Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable custom UserAgent #451

Merged
merged 4 commits into from
Oct 15, 2020
Merged

Conversation

bishtawi
Copy link
Contributor

@bishtawi bishtawi commented Oct 1, 2020

@roidelapluie
Copy link
Collaborator

gitlab/provider.go Outdated Show resolved Hide resolved
@roidelapluie
Copy link
Collaborator

How much work is it to go to the v2 of the sdk?

@bishtawi
Copy link
Contributor Author

bishtawi commented Oct 1, 2020

Not sure but there is a lot of breaking changes between 1.x and 2.x: https://github.com/hashicorp/terraform-plugin-sdk/blob/master/CHANGELOG.md

@roidelapluie
Copy link
Collaborator

In 1.0 we have httpclient which has the function we want

@roidelapluie
Copy link
Collaborator

@bishtawi
Copy link
Contributor Author

bishtawi commented Oct 1, 2020

Nice good find!

@nicholasklick
Copy link
Collaborator

@bishtawi thanks for this. So what is the strong generated by c.TerraformUserAgent?
Does it include something unique to the provider? Like gitlab-terraform-provider.

Basically our goal here is to be able to identify API requests originating from the terraform gitlab provider.


// NOTE: httpclient.TerraformUserAgent is deprecated and removed in Terraform SDK v2
// After upgrading the SDK to v2 replace with p.UserAgent()
TerraformUserAgent: httpclient.TerraformUserAgent(p.TerraformVersion),
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client := provider.Client()
client.UserAgent = httpclient.TerraformUserAgent(p.TerraformVersion)
return client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Personally I am not a fan of the providerConfigure function customizing the client as that seems to be the responsibility of the Client() function but the code is indeed simpler.


ConfigureFunc: providerConfigure,
provider.ConfigureFunc = func(d *schema.ResourceData) (interface{}, error) {
return providerConfigure(provider, d)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think this is not needed? The providerConfigure function needs the provider object to get the Terraform version number. Wrapping the providerConfigure function allows us to do this. Is there another way to accomplish it?

@roidelapluie
Copy link
Collaborator

It looks like this pull request is way too complicated and it could all be done in gitlab/provider.go

@bishtawi
Copy link
Contributor Author

@roidelapluie Sure I can simplify it; I was just trying to keep with the existing code structure by having all the specific Gitlab Client customizations in the Client() function rather than having the provider also customize the client.

@nicholasklick No, the httpclient.TerraformUserAgent() function only includes Terraform specific info. We have to add Gitlab provider info ourselves. I'll add that in.

@bishtawi
Copy link
Contributor Author

bishtawi commented Oct 13, 2020

Useragent will look like

HashiCorp Terraform/0.13.4 (+https://www.terraform.io) Terraform Plugin SDK/1.15.0 terraform-provider-gitlab

@nicholasklick
Copy link
Collaborator

@bishtawi that useragent string looks good thanks!

@nicholasklick
Copy link
Collaborator

@roidelapluie @armsnyder if this looks good I would love to merge and ship a new release.

Copy link
Collaborator

@armsnyder armsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A possible future improvement is to include the terraform-provider-gitlab provider version in the string somewhere. GoReleaser supports setting a version variable at release time, using ldflags, which could do this. https://goreleaser.com/customization/build/

@nicholasklick nicholasklick merged commit f082dd9 into gitlabhq:master Oct 15, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Enable custom UserAgent for metrics purposes
4 participants