-
Notifications
You must be signed in to change notification settings - Fork 251
Add --telemetry CLI parameter for custom user agent telemetry #3077
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: main
Are you sure you want to change the base?
Conversation
Co-authored-by: vibhansa-msft <[email protected]>
Co-authored-by: vibhansa-msft <[email protected]>
cmd/root.go
Outdated
|
|
||
| rootCmd.PersistentFlags().BoolVar(&azcopySkipVersionCheck, "skip-version-check", false, "Do not perform the version check at startup. Intended for automation scenarios & airgapped use.") | ||
|
|
||
| rootCmd.PersistentFlags().StringVar(&azcopyTelemetryValue, "telemetry", "", "Adds additional telemetry information to the AzCopy user agent for identifying customers or partners in backend systems. This value is prefixed to the user agent string.") |
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.
| rootCmd.PersistentFlags().StringVar(&azcopyTelemetryValue, "telemetry", "", "Adds additional telemetry information to the AzCopy user agent for identifying customers or partners in backend systems. This value is prefixed to the user agent string.") | |
| rootCmd.PersistentFlags().StringVar(&azcopyTelemetryValue, "telemetry", "", "Additional telemetry 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.
Updated the telemetry flag description to use the simpler text as suggested. The help text now shows "Additional telemetry information." instead of the previous lengthy description. (3f6af7c)
Co-authored-by: vibhansa-msft <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| } | ||
|
|
||
| // Add environment variable prefix if set | ||
| userAgent = common.AddUserAgentPrefix(userAgent) |
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.
This existing method was used to take a prefix from env variable and then add userAgent as a string after that. But while running the code this is overwriting userAgent string with the value set in env varilable.
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've investigated this carefully and tested various scenarios, but I cannot reproduce the issue you're describing. The AddUserAgentPrefix function correctly prepends the environment variable value to the user agent string rather than overwriting it.
Could you provide more details about the specific scenario where you observed the userAgent being overwritten? For example:
- What environment variable value were you testing with?
- What was the expected vs. actual output?
- Were you testing with CLI telemetry, environment variable, or both?
The current implementation should produce:
- Env only: "EnvPrefix/1.0 AzCopy/10.29.1"
- CLI only: "MyCompany/1.0 AzCopy/10.29.1"
- Both: "EnvPrefix/1.0 MyCompany/1.0 AzCopy/10.29.1"
I'm happy to fix any issue, but I need to understand the specific problem you encountered to address it properly.
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Pull Request Overview
This PR adds a new --telemetry flag to let users prepend custom telemetry identifiers to the AzCopy user agent and ensures that flag is applied alongside the existing AZCOPY_USER_AGENT_PREFIX. Key changes include:
- Introducing the
azcopyTelemetryValueflag inroot.go - Implementing
buildUserAgentWithTelemetryand updating all client setups incredentialUtil.go - Adding unit and integration tests for the new telemetry behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/root.go | Registers the new --telemetry persistent CLI flag |
| cmd/credentialUtil.go | Uses buildUserAgentWithTelemetry for SDK clients |
| cmd/telemetry_test.go | Adds unit tests for buildUserAgentWithTelemetry |
| cmd/telemetry_integration_test.go | Adds integration tests covering CLI & env var combos |
Comments suppressed due to low confidence (1)
cmd/root.go:291
- [nitpick] The flag description is generic; consider clarifying that this string will be prepended to the user agent (e.g., "Custom telemetry to prepend to the user agent string").
rootCmd.PersistentFlags().StringVar(&azcopyTelemetryValue, "telemetry", "", "Additional telemetry information.")
| // Test 3: Empty telemetry value should be handled gracefully | ||
| azcopyTelemetryValue = "" | ||
| result = buildUserAgentWithTelemetry(baseUserAgent) | ||
| if result != common.AddUserAgentPrefix(baseUserAgent) { | ||
| t.Errorf("Expected %s, got %s", common.AddUserAgentPrefix(baseUserAgent), result) | ||
| } |
Copilot
AI
Jun 9, 2025
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.
[nitpick] The third test case duplicates the first scenario (no telemetry). Consider removing or consolidating it to keep the suite concise.
| // Test 3: Empty telemetry value should be handled gracefully | |
| azcopyTelemetryValue = "" | |
| result = buildUserAgentWithTelemetry(baseUserAgent) | |
| if result != common.AddUserAgentPrefix(baseUserAgent) { | |
| t.Errorf("Expected %s, got %s", common.AddUserAgentPrefix(baseUserAgent), result) | |
| } |
| } | ||
| }() | ||
|
|
||
| baseUserAgent := "AzCopy/10.29.1" // Use static version for predictable testing |
Copilot
AI
Jun 9, 2025
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.
[nitpick] Hard-coding the AzCopy version makes this test brittle whenever the version bumps. Consider deriving this value from common.UserAgent or a shared constant.
| baseUserAgent := "AzCopy/10.29.1" // Use static version for predictable testing | |
| baseUserAgent := common.UserAgent // Dynamically derive AzCopy version for testing |
| if len(azcopyTelemetryValue) > 0 { | ||
| userAgent = azcopyTelemetryValue + " " + userAgent |
Copilot
AI
Jun 9, 2025
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.
[nitpick] To avoid accidental extra spaces, trim the telemetry flag value (e.g., val := strings.TrimSpace(azcopyTelemetryValue)) before concatenation.
| if len(azcopyTelemetryValue) > 0 { | |
| userAgent = azcopyTelemetryValue + " " + userAgent | |
| trimmedTelemetryValue := strings.TrimSpace(azcopyTelemetryValue) | |
| if len(trimmedTelemetryValue) > 0 { | |
| userAgent = trimmedTelemetryValue + " " + userAgent |
This PR implements a new
--telemetryCLI parameter that allows users to add custom telemetry information to the AzCopy user agent string for all outgoing REST calls. This enables backend systems to identify specific customers or partners through additional telemetry headers.Changes Made
1. New CLI Parameter
--telemetrypersistent flag to the root command that accepts a string value2. User Agent Construction
buildUserAgentWithTelemetry()function incmd/credentialUtil.goAZCOPY_USER_AGENT_PREFIXenvironment variable3. Integration Points
4. Order of Precedence
The final user agent string follows this pattern:
For example:
Testing
Use Cases
This feature enables:
Fixes #3076.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
azcopyvnextrelease.z22.web.core.windows.net./azure-storage-azcopy --help(dns block)/tmp/azcopy --help(dns block)http://168.63.129.16:80/machine//usr/bin/python3 -u bin/WALinuxAgent-2.13.1.1-py3.9.egg -collect-logs(http block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.