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

Set User-Agent in http.Client used in govmomi lib #2693

Closed
embano1 opened this issue Jan 6, 2022 · 8 comments · Fixed by #2872
Closed

Set User-Agent in http.Client used in govmomi lib #2693

embano1 opened this issue Jan 6, 2022 · 8 comments · Fixed by #2872
Assignees

Comments

@embano1
Copy link
Contributor

embano1 commented Jan 6, 2022

Is your feature request related to a problem? Please describe.
Set a default value for User-Agent in http.Client when using the library (govc already allows this) to identify requests coming from govmomi SDK (currently it only shows a generic Go client in stats). Useful for debugging, statistics, etc.

Describe the solution you'd like
Set a default value for User-Agent in http.Client

UserAgent string

Describe alternatives you've considered
Keep as is

Additional context
Should not be considered a breaking change IMHO?

@embano1
Copy link
Contributor Author

embano1 commented Jan 12, 2022

Example (adopted from aws-sdk-go):

// file: version.go
package govmomi

// SDKName is the name of this library
const LibraryName = "govmomi"

// SDKVersion is the version of the library
const LibraryVersion = "0.28.0"

Source: https://github.com/aws/aws-sdk-go/blob/main/aws/version.go

@dougm thoughts?
I would have to rework the automation and RELEASE.md to continuously update the const before creating a new tag.

@dougm
Copy link
Member

dougm commented Jan 12, 2022

When/how would the automation update the LibraryVersion ?

Another option might be to update LibraryVersion manually and automate creating the tag from that?

But I'm good with adopting a common pattern from what other projects are doing these days.

@embano1
Copy link
Contributor Author

embano1 commented Jan 13, 2022

When/how would the automation update the LibraryVersion ?

My idea: create a dispatch Github Actions workflow which is triggered before creating a release by a maintainer. It will patch the version based on user input (all within Github UI) and automatically create a PR for review. I would also update the release workflow to check whether the tag matches version and fail a release if not.

Another option might be to update LibraryVersion manually and automate creating the tag from that?

Thought about manual way too, but this would require someone to file a PR (since we don't directly push to master). The above would not require this.

@dougm
Copy link
Member

dougm commented Jan 13, 2022

When/how would the automation update the LibraryVersion ?

My idea: create a dispatch Github Actions workflow which is triggered before creating a release by a maintainer. It will patch the version based on user input (all within Github UI) and automatically create a PR for review. I would also update the release workflow to check whether the tag matches version and fail a release if not.

That sounds cool. Maybe someday I'll learn more about GH Actions, in the meantime thanks for all your efforts around this!

Another option might be to update LibraryVersion manually and automate creating the tag from that?

Thought about manual way too, but this would require someone to file a PR (since we don't directly push to master). The above would not require this.

True, in the old days I would bump the const version as part of the PR for release docs update (usage, changelog, contributors, etc). Would be much better to automate the version bump.

@atc0005
Copy link
Contributor

atc0005 commented Jan 13, 2022

Question (likely from ignorance):

Once this feature is developed/implemented, will it block setting a more specific user agent?

Snippet from a project where I'm using this library:

	c, err := govmomi.NewClient(ctx, u, trustCert)
	if err != nil {
		return nil, err
	}

	// Override default user agent
	c.Client.UserAgent = userAgent

	// provide credentials *after* we create the client so that the desired
	// User Agent value can be set before logging in.
	u.User = url.UserPassword(username, password)

	// Login, supplying our custom user agent in place of the default
	authErr := c.Login(ctx, u.User)
	if authErr != nil {
		return nil, authErr
	}

where:

  • trustCert is a user-specified choice of whether they wish to trust the cert
  • userAgent is a project specific name/version string (based on git tag)
    • intended to identify the source of the tool logging into a vSphere environment

@embano1
Copy link
Contributor Author

embano1 commented Jan 14, 2022

Thx for the great feedback @atc0005 ! This is really useful and my plan was to definitely allow for customization/overwriting the agent, and providing a meaningful default value when not.

@github-actions
Copy link
Contributor

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Mark as fresh by adding the comment /remove-lifecycle stale.

@embano1
Copy link
Contributor Author

embano1 commented Apr 17, 2022

/remove-lifecycle stale

@embano1 embano1 self-assigned this Apr 25, 2022
embano1 pushed a commit to embano1/govmomi that referenced this issue Jun 10, 2022
Closes: vmware#2693
Signed-off-by: Michael Gasch <mgasch@vmware.com>
embano1 pushed a commit to embano1/govmomi that referenced this issue Jun 10, 2022
Closes: vmware#2693
Signed-off-by: Michael Gasch <mgasch@vmware.com>
embano1 pushed a commit to embano1/govmomi that referenced this issue Jun 14, 2022
Closes: vmware#2693
Signed-off-by: Michael Gasch <mgasch@vmware.com>
embano1 pushed a commit to embano1/govmomi that referenced this issue Jun 15, 2022
Closes: vmware#2693
Signed-off-by: Michael Gasch <mgasch@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants