chore: Improves user agent in the provider to include Terraform CLI version #2085

maastha · 2024-03-26T14:24:22Z

Description

Improves user agent in the provider to include Terraform CLI version

Link to any related issue(s): CLOUDP-237005

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contribution guidelines
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals, I defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

TF Plugin Framework based resource API call logs:

2024/04/05 15:17:50 
GET /api/atlas/v2/groups/6602bedbc32461337d92e2a5/settings HTTP/1.1
Host: cloud-dev.mongodb.com
User-Agent: terraform-provider-mongodbatlas/devel Terraform/1.7.0-rc1
Accept: application/vnd.atlas.2023-01-01+json
Accept-Encoding: gzip

TF Plugin SDK based resource API call logs:

2024/04/05 15:17:52 
POST /api/atlas/v2/groups/6602d0208210bb7b10f5f8a8/clusters HTTP/1.1
Host: cloud-dev.mongodb.com
User-Agent: terraform-provider-mongodbatlas/devel Terraform/1.7.0-rc1
Content-Length: 302
Accept: application/vnd.atlas.2023-02-01+json

internal/config/client.go

oarbusi

Great work! LGTM

AgustinBettati

LGTM, just some follow up doubts

internal/provider/provider_sdk2.go

AgustinBettati · 2024-03-26T16:29:08Z

internal/provider/provider.go

@@ -231,6 +231,7 @@ func (p *MongodbtlasProvider) Configure(ctx context.Context, req provider.Config
 		PrivateKey:   data.PrivateKey.ValueString(),
 		BaseURL:      data.BaseURL.ValueString(),
 		RealmBaseURL: data.RealmBaseURL.ValueString(),
+		UserAgent:    config.TerraformVersionUserAgentInfo(req.TerraformVersion),


[q] Could both the provider version and the terraform version be defined in a single place so it is easy to see the complete user agent format?

it can be but I have kept it separate. Ideally user agent should be taken care of when creating new client (like we already do today) but in this case I had to set the new Terraform version user agent in the Provider because that's only where it's available.
Defining both in single place would mean we take provider version out of the client creation which I don't think we should do

I see, one option could be to define a property TerraformVersion in the Config struct (instead of UserAgent), and then when creating a new client define the user agent with both the terraform and provider versions available. Let me know if you feel this makes it clearer or not.

I was thinking the same thing after spending some time reviewing this. Having TerraformVersion would help a lot in terms of readability. In fact during client creation you can "compose" the user agent in-place without even having to have a special append function IMO.

Also this solution would help to handle logs / problems: e.g. if TerraformVersion is empty during the client creation phase then we should handle it with a warning probably since it's something that needs attention

I have added TerraformVersion in the Config struct, let me know what you think

internal/config/client.go

Co-authored-by: Agustin Bettati <[email protected]>

lantoli · 2024-03-26T19:49:33Z

internal/config/client.go

 )

 const (
 	toolName = "terraform-provider-mongodbatlas"
 )

 var (
-	userAgent = fmt.Sprintf("%s/%s", toolName, version.ProviderVersion)
+	userAgentProviderVersion = fmt.Sprintf("%s/%s", toolName, version.ProviderVersion)


would it be useful to add some tests in the PR?

github-actions · 2024-04-01T00:22:16Z

This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy!

internal/config/client.go

AgustinBettati

LGTM, just to confirm the conclusion with analytics team was to have the provider version first and then the terraform version?

maastha · 2024-04-05T15:25:12Z

LGTM, just to confirm the conclusion with analytics team was to have the provider version first and then the terraform version?

This was Gus's suggestion, yes. I have updated the testing example in the PR accordingly.

marcosuma

Awesome job!

maastha added 6 commits March 26, 2024 12:08

initial

075332a

update user agent for plugin-sdk

821acc4

cleanup

4b6a35b

Merge branch 'master' into CLOUDP-237005-enrich-user-agent

523571e

minor

a413506

cleanup

672a472

maastha marked this pull request as ready for review March 26, 2024 15:15

maastha requested a review from a team as a code owner March 26, 2024 15:15

maastha requested review from AgustinBettati and oarbusi March 26, 2024 15:49

oarbusi reviewed Mar 26, 2024

View reviewed changes

internal/config/client.go Outdated Show resolved Hide resolved

oarbusi approved these changes Mar 26, 2024

View reviewed changes

AgustinBettati approved these changes Mar 26, 2024

View reviewed changes

Update internal/provider/provider_sdk2.go

debbbb0

Co-authored-by: Agustin Bettati <[email protected]>

lantoli reviewed Mar 26, 2024

View reviewed changes

github-actions bot added the stale label Apr 1, 2024

oarbusi removed the stale label Apr 2, 2024

EspenAlbert approved these changes Apr 2, 2024

View reviewed changes

marcosuma reviewed Apr 5, 2024

View reviewed changes

internal/config/client.go Outdated Show resolved Hide resolved

add TerraformVersion to Config

53666b3

AgustinBettati approved these changes Apr 5, 2024

View reviewed changes

marcosuma approved these changes Apr 5, 2024

View reviewed changes

maastha merged commit ac9b66b into master Apr 5, 2024
48 checks passed

maastha deleted the CLOUDP-237005-enrich-user-agent branch April 5, 2024 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Improves user agent in the provider to include Terraform CLI version #2085

chore: Improves user agent in the provider to include Terraform CLI version #2085

maastha commented Mar 26, 2024 •

edited

Loading

oarbusi left a comment

AgustinBettati left a comment

AgustinBettati Mar 26, 2024

maastha Mar 26, 2024

AgustinBettati Mar 26, 2024

marcosuma Apr 5, 2024

maastha Apr 5, 2024

lantoli Mar 26, 2024 •

edited

Loading

github-actions bot commented Apr 1, 2024

AgustinBettati left a comment

maastha commented Apr 5, 2024

marcosuma left a comment

chore: Improves user agent in the provider to include Terraform CLI version #2085

chore: Improves user agent in the provider to include Terraform CLI version #2085

Conversation

maastha commented Mar 26, 2024 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

oarbusi left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

AgustinBettati Mar 26, 2024

Choose a reason for hiding this comment

maastha Mar 26, 2024

Choose a reason for hiding this comment

AgustinBettati Mar 26, 2024

Choose a reason for hiding this comment

marcosuma Apr 5, 2024

Choose a reason for hiding this comment

maastha Apr 5, 2024

Choose a reason for hiding this comment

lantoli Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 1, 2024

AgustinBettati left a comment

Choose a reason for hiding this comment

maastha commented Apr 5, 2024

marcosuma left a comment

Choose a reason for hiding this comment

maastha commented Mar 26, 2024 •

edited

Loading

lantoli Mar 26, 2024 •

edited

Loading