-
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?
Changes from all commits
3e2e2ec
65868b6
256cf58
3f6af7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,7 +366,7 @@ func isPublic(ctx context.Context, blobResourceURL string, cpkOptions common.Cpk | |
| RetryDelay: ste.UploadRetryDelay, | ||
| MaxRetryDelay: ste.UploadMaxRetryDelay, | ||
| }, policy.TelemetryOptions{ | ||
| ApplicationID: common.AddUserAgentPrefix(common.UserAgent), | ||
| ApplicationID: buildUserAgentWithTelemetry(common.UserAgent), | ||
| }, nil, ste.LogOptions{}, nil, nil) | ||
|
|
||
| blobClient, _ := blob.NewClientWithNoCredential(bURLParts.String(), &blob.ClientOptions{ClientOptions: clientOptions}) | ||
|
|
@@ -399,7 +399,7 @@ func mdAccountNeedsOAuth(ctx context.Context, blobResourceURL string, cpkOptions | |
| RetryDelay: ste.UploadRetryDelay, | ||
| MaxRetryDelay: ste.UploadMaxRetryDelay, | ||
| }, policy.TelemetryOptions{ | ||
| ApplicationID: common.AddUserAgentPrefix(common.UserAgent), | ||
| ApplicationID: buildUserAgentWithTelemetry(common.UserAgent), | ||
| }, nil, ste.LogOptions{}, nil, nil) | ||
|
|
||
| blobClient, _ := blob.NewClientWithNoCredential(blobResourceURL, &blob.ClientOptions{ClientOptions: clientOptions}) | ||
|
|
@@ -593,8 +593,24 @@ func createClientOptions(logger common.ILoggerResetable, srcCred *common.ScopedT | |
| RetryDelay: ste.UploadRetryDelay, | ||
| MaxRetryDelay: ste.UploadMaxRetryDelay, | ||
| }, policy.TelemetryOptions{ | ||
| ApplicationID: common.AddUserAgentPrefix(common.UserAgent), | ||
| ApplicationID: buildUserAgentWithTelemetry(common.UserAgent), | ||
| }, ste.NewAzcopyHTTPClient(frontEndMaxIdleConnectionsPerHost), logOptions, srcCred, reauthCred) | ||
| } | ||
|
|
||
| const frontEndMaxIdleConnectionsPerHost = http.DefaultMaxIdleConnsPerHost | ||
|
|
||
| // buildUserAgentWithTelemetry creates the user agent string including both | ||
| // environment variable prefix and CLI telemetry parameter | ||
| func buildUserAgentWithTelemetry(baseUserAgent string) string { | ||
| userAgent := baseUserAgent | ||
|
|
||
| // Add CLI telemetry parameter if provided | ||
| if len(azcopyTelemetryValue) > 0 { | ||
| userAgent = azcopyTelemetryValue + " " + userAgent | ||
| } | ||
|
|
||
| // Add environment variable prefix if set | ||
| userAgent = common.AddUserAgentPrefix(userAgent) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Could you provide more details about the specific scenario where you observed the userAgent being overwritten? For example:
The current implementation should produce:
I'm happy to fix any issue, but I need to understand the specific problem you encountered to address it properly. |
||
|
|
||
| return userAgent | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,58 @@ | ||||||
| package cmd | ||||||
|
|
||||||
| import ( | ||||||
| "testing" | ||||||
| "os" | ||||||
| ) | ||||||
|
|
||||||
| func TestTelemetryIntegration(t *testing.T) { | ||||||
| // Save original values to restore later | ||||||
| originalTelemetry := azcopyTelemetryValue | ||||||
| originalEnv := os.Getenv("AZCOPY_USER_AGENT_PREFIX") | ||||||
| defer func() { | ||||||
| azcopyTelemetryValue = originalTelemetry | ||||||
| if originalEnv != "" { | ||||||
| os.Setenv("AZCOPY_USER_AGENT_PREFIX", originalEnv) | ||||||
| } else { | ||||||
| os.Unsetenv("AZCOPY_USER_AGENT_PREFIX") | ||||||
| } | ||||||
| }() | ||||||
|
|
||||||
| baseUserAgent := "AzCopy/10.29.1" // Use static version for predictable testing | ||||||
|
||||||
| baseUserAgent := "AzCopy/10.29.1" // Use static version for predictable testing | |
| baseUserAgent := common.UserAgent // Dynamically derive AzCopy version for testing |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||
| package cmd | ||||||||||||||
|
|
||||||||||||||
| import ( | ||||||||||||||
| "testing" | ||||||||||||||
| "github.com/Azure/azure-storage-azcopy/v10/common" | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| func TestBuildUserAgentWithTelemetry(t *testing.T) { | ||||||||||||||
| // Save original value to restore later | ||||||||||||||
| originalTelemetry := azcopyTelemetryValue | ||||||||||||||
| defer func() { | ||||||||||||||
| azcopyTelemetryValue = originalTelemetry | ||||||||||||||
| }() | ||||||||||||||
|
|
||||||||||||||
| baseUserAgent := common.UserAgent | ||||||||||||||
|
|
||||||||||||||
| // Test 1: No telemetry value | ||||||||||||||
| azcopyTelemetryValue = "" | ||||||||||||||
| result := buildUserAgentWithTelemetry(baseUserAgent) | ||||||||||||||
| // Should have only the base user agent (possibly with env prefix) | ||||||||||||||
| if result != common.AddUserAgentPrefix(baseUserAgent) { | ||||||||||||||
| t.Errorf("Expected %s, got %s", common.AddUserAgentPrefix(baseUserAgent), result) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Test 2: With telemetry value | ||||||||||||||
| azcopyTelemetryValue = "MyCompany/1.0" | ||||||||||||||
| result = buildUserAgentWithTelemetry(baseUserAgent) | ||||||||||||||
| // Should have telemetry prefix followed by user agent | ||||||||||||||
| expected := common.AddUserAgentPrefix("MyCompany/1.0 " + baseUserAgent) | ||||||||||||||
| if result != expected { | ||||||||||||||
| t.Errorf("Expected %s, got %s", expected, result) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // 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) | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+34
to
+39
|
||||||||||||||
| // 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) | |
| } |
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.