Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions cmd/credentialUtil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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
Comment on lines +608 to +609
Copy link

Copilot AI Jun 9, 2025

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.

Suggested change
if len(azcopyTelemetryValue) > 0 {
userAgent = azcopyTelemetryValue + " " + userAgent
trimmedTelemetryValue := strings.TrimSpace(azcopyTelemetryValue)
if len(trimmedTelemetryValue) > 0 {
userAgent = trimmedTelemetryValue + " " + userAgent

Copilot uses AI. Check for mistakes.
}

// Add environment variable prefix if set
userAgent = common.AddUserAgentPrefix(userAgent)
Copy link
Member

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.

Copy link
Contributor Author

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.


return userAgent
}
3 changes: 3 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ var azcopySkipVersionCheck bool
var isPipeDownload bool
var retryStatusCodes string
var debugMemoryProfile string
var azcopyTelemetryValue string

type jobLoggerInfo struct {
jobID common.JobID
Expand Down Expand Up @@ -287,6 +288,8 @@ func init() {

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.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.")

Copy link
Contributor Author

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)


// Note: this is due to Windows not supporting signals properly
rootCmd.PersistentFlags().BoolVar(&cancelFromStdin, "cancel-from-stdin", false, "Used by partner teams to send in `cancel` through stdin to stop a job.")

Expand Down
58 changes: 58 additions & 0 deletions cmd/telemetry_integration_test.go
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
Copy link

Copilot AI Jun 9, 2025

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.

Suggested change
baseUserAgent := "AzCopy/10.29.1" // Use static version for predictable testing
baseUserAgent := common.UserAgent // Dynamically derive AzCopy version for testing

Copilot uses AI. Check for mistakes.

// Test case 1: Only CLI telemetry parameter
azcopyTelemetryValue = "MyCompany/1.0"
os.Unsetenv("AZCOPY_USER_AGENT_PREFIX")
result := buildUserAgentWithTelemetry(baseUserAgent)
expected := "MyCompany/1.0 AzCopy/10.29.1"
if result != expected {
t.Errorf("CLI telemetry only: expected %s, got %s", expected, result)
}

// Test case 2: Only environment variable
azcopyTelemetryValue = ""
os.Setenv("AZCOPY_USER_AGENT_PREFIX", "EnvPrefix/1.0")
result = buildUserAgentWithTelemetry(baseUserAgent)
expected = "EnvPrefix/1.0 AzCopy/10.29.1"
if result != expected {
t.Errorf("Env prefix only: expected %s, got %s", expected, result)
}

// Test case 3: Both CLI telemetry and environment variable
azcopyTelemetryValue = "MyCompany/1.0"
os.Setenv("AZCOPY_USER_AGENT_PREFIX", "EnvPrefix/1.0")
result = buildUserAgentWithTelemetry(baseUserAgent)
expected = "EnvPrefix/1.0 MyCompany/1.0 AzCopy/10.29.1"
if result != expected {
t.Errorf("Both CLI and env: expected %s, got %s", expected, result)
}

// Test case 4: Neither CLI nor environment variable
azcopyTelemetryValue = ""
os.Unsetenv("AZCOPY_USER_AGENT_PREFIX")
result = buildUserAgentWithTelemetry(baseUserAgent)
expected = "AzCopy/10.29.1"
if result != expected {
t.Errorf("Neither CLI nor env: expected %s, got %s", expected, result)
}
}
40 changes: 40 additions & 0 deletions cmd/telemetry_test.go
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
Copy link

Copilot AI Jun 9, 2025

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.

Suggested change
// 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 uses AI. Check for mistakes.
}