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

Include a more specific user agent string in HTTP calls #90

Closed
willsmythe opened this issue Mar 21, 2017 · 4 comments
Closed

Include a more specific user agent string in HTTP calls #90

willsmythe opened this issue Mar 21, 2017 · 4 comments
Assignees

Comments

@willsmythe
Copy link
Contributor

Need to include a more descriptive user agent string in calls from this client. This will help us better understand how/where people are using the library.

The user-agent string should always include information about the client library, but also information about the tool/app/service using the library (optionally). This might be specified like this:

var requestSettings: IWebApiRequestSettings = {
 
    productName = "MyTool",
    productVersion = "12.322"
   
};

var webApi  = new WebApi(connectionUrl, creds, requestSettings);

if product name is specified ...

   {ToolName}/{ToolVersion} (vsts-node-api {VstsNodeApiVersion}; {osname} {osversion})

Otherwise ...

   vsts-node-api/{VstsNodeApiVersion} ({osname} {osversion})

Examples:

  • vsts-node-api/6.1.1-preivew (Windows 10.01232)
  • tfx-cli/0.3.49 (vsts-node-api 14.1223.2; Windows 10.1232)
@willsmythe
Copy link
Contributor Author

@scottdallamura / @bryanmacfarlane - take a look and let's close on this. We can evolve it over time as more information is needed. The version of the client, the OS, and optional "tool" information is the main info we need to start seeing in the telemetry on the server.

@stephenmichaelf
Copy link
Member

Working on this now, can create Issue in tfx-cli too.

@stephenmichaelf
Copy link
Member

And add L0 tests once that infrastructure is setup. Good candidate for a Nock test to make sure the header is set correctly with/without tool being specified.

@damccorm
Copy link

@stephenmichaelf I added a PR to address this, I'm going to hold off on adding testing until #218 is approved (adding a setup for testing) to avoid issues merging them together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants