-
Notifications
You must be signed in to change notification settings - Fork 266
CI information in UserAgent header #14717
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
base: dev
Are you sure you want to change the base?
Conversation
accepted/2026/ci-useragent-nuget.md
Outdated
|
|
||
| ## Drawbacks | ||
|
|
||
| 1. **Limited CI/CD Coverage**: Only GitHub Actions and Azure DevOps are detected initially. Other CI systems are not detected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SDK already detects CI for telemetry purposes, and supports several other environment variables: https://github.com/dotnet/sdk/blob/1b58d7631b1ed3cd26f7fa7415c075f686ae0f1a/src/Cli/dotnet/Telemetry/CIEnvironmentDetectorForTelemetry.cs#L13
Since NuGet is a component of the SDK, I think it should be consistent with the rest of the SDK. The technical challenge is how to detect when the SDK changes the list so that NuGet can adapt and use the same values
| # User-Agent Telemetry Enrichment for NuGet Commands | ||
|
|
||
| - Author: [@mruizmares](https://github.com/mruizmares) | ||
| - GitHub Issue: https://github.com/NuGet/Client.Engineering/issues/3467 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we link to private issues in public specs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO no, we shouldn't.
accepted/2026/ci-useragent-nuget.md
Outdated
|
|
||
| The enriched User-Agent follows the format: | ||
| ``` | ||
| {base-user-agent} NuGet/{ClientId} CI/{CiClient} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@agr could you identify if this change will impact NuGet.org statistics processing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joelverhagen @agr I updated the format to {base-user-agent} NuGet/{ClientId; CIClient}
accepted/2026/ci-useragent-nuget.md
Outdated
| ``` | ||
|
|
||
| Examples: | ||
| - `NuGet xplat/6.10.0 (Microsoft Windows NT; GitHubActions)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we should distinguish the CI values with a "CI" tag, that ensures the set of CI strings will not overlap with other strings put in this parenthesis (called a comment in UA language).
So, maybe NuGet xplat/6.10.0 (Microsoft Windows NT; CI: GitHub Actions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could imagine this being used via a regex like CI: ([^\);]+)
|
|
||
| **After (running in GitHub Actions, push command):** | ||
| ``` | ||
| NuGet Command Line/6.10.0 (Microsoft Windows NT 10.0.22631.0; CI: GithubActions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| NuGet Command Line/6.10.0 (Microsoft Windows NT 10.0.22631.0; CI: GithubActions) | |
| NuGet Command Line/6.10.0 (Microsoft Windows NT 10.0.22631.0; CI: GitHub Actions) |
| Examples: | ||
| - `NuGet xplat/6.10.0 (Microsoft Windows NT; CI: GitHubActions)` | ||
| - `NuGet xplat/6.10.0 (Linux; CI: AzureDevOps)` | ||
| - `NuGet xplat/6.10.0 (Linux; CI: CI)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - `NuGet xplat/6.10.0 (Linux; CI: CI)` | |
| - `NuGet xplat/6.10.0 (Linux; CI)` |
might be a little less ugly :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or CI: other
joelverhagen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor comments but looks good.
zivkan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be happy to approve once the unresolved question is resolved and moved out of the unresolved questions section.
| # User-Agent Telemetry Enrichment for NuGet Commands | ||
|
|
||
| - Author: [@mruizmares](https://github.com/mruizmares) | ||
| - GitHub Issue: https://github.com/NuGet/Client.Engineering/issues/3467 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO no, we shouldn't.
|
|
||
| ## Summary | ||
|
|
||
| This proposal adds telemetry enrichment to the NuGet User-Agent header for push and restore commands. The enriched User-Agent header includes contextual information about the client environment (GitHub Actions, Azure DevOps). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In several pull requests on the docs repo and design specs, I've suggested that markdown files should have one sentence per line, as it enriches the pull request experience. I can enrich my feedback by easily commenting on a single line, rather than the full paragraph, and the output when rendered in html is not modified. In the past github didn't highlight changes within long lines, so splitting markdown paragraphs to short lines enriches the diff experience, but that's more helpful in the docs repo where modifications are common.
also nitpick: I have no problem with the word enrich, but it's not a very commonly used word, so using it twice in two sentences feels weird 😁
|
|
||
| The enriched User-Agent follows the format: | ||
| ``` | ||
| {base-user-agent} NuGet/{ClientId; CI: ([^\);]+)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I find this very difficult to understand. I know only the very basics of regex. The examples below help, but I personally don't find this line helpful in understanding the proposed change. I suspect that it could be explained in a more clear way.
We'll add
CI: {detected environment}to the user agent, following the operating system information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should have the regex in the spec. I provided it in the comment above as an example. I agree with you.
|
|
||
| The enrichment happens automatically: | ||
| 1. **Detection**: NuGet detects if it's running in a known CI/CD environment (GitHub Actions or Azure DevOps) by checking environment variables. | ||
| 2. **User-Agent Enrichment**: HTTP requests include this context in the User-Agent header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there be a header if no CI was detected?
Is that gonna be the assumption by omission or are we gonna be explicit about it?
No description provided.