Skip to content

Commit 965cffb

Browse files
authored
telemetry: update SAM CLI UA env var to something more generic (#3314)
## Problem The name `SAM_CLI_TELEMETRY_FROM_IDE` is too specific. We're also sending more than what SAM CLI expects. ## Solution Use `AWS_TOOLING_USER_AGENT `instead and remove extraneous pairs
1 parent d2983c2 commit 965cffb

File tree

5 files changed

+27
-8
lines changed

5 files changed

+27
-8
lines changed

src/shared/awsClientBuilder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class DefaultAWSClientBuilder implements AWSClientBuilder {
126126
}
127127

128128
if (userAgent && !opt.customUserAgent) {
129-
opt.customUserAgent = await getUserAgent({ includeClientId: true })
129+
opt.customUserAgent = await getUserAgent({ includePlatform: true, includeClientId: true })
130130
}
131131

132132
const apiConfig = (opt as { apiConfig?: { metadata?: Record<string, string> } } | undefined)?.apiConfig

src/shared/sam/cli/samCliInvokerUtils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export async function addTelemetryEnvVar(options: SpawnOptions | undefined): Pro
105105
return {
106106
...options,
107107
env: {
108-
SAM_CLI_TELEMETRY_FROM_IDE: await getUserAgent({ includeClientId: false }),
108+
AWS_TOOLING_USER_AGENT: await getUserAgent({ includeClientId: false }),
109109
...options?.env,
110110
},
111111
}

src/shared/telemetry/util.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,22 @@ export const getClientId = shared(
6161
}
6262
)
6363

64+
export const platformPair = () => `${env.appName.replace(/\s/g, '-')}/${version}`
65+
6466
/**
6567
* Returns a string that should be used as the extension's user agent.
6668
*
67-
* Omits the `ClientId` pair by default.
69+
* Omits the platform and `ClientId` pairs by default.
6870
*/
6971
export async function getUserAgent(
70-
opt?: { includeClientId?: boolean },
72+
opt?: { includePlatform?: boolean; includeClientId?: boolean },
7173
globalState = globals.context.globalState
7274
): Promise<string> {
73-
const platformName = env.appName.replace(/\s/g, '-')
74-
const pairs = [`AWS-Toolkit-For-VSCode/${extensionVersion}`, `${platformName}/${version}`]
75+
const pairs = [`AWS-Toolkit-For-VSCode/${extensionVersion}`]
76+
77+
if (opt?.includePlatform) {
78+
pairs.push(platformPair())
79+
}
7580

7681
if (opt?.includeClientId) {
7782
const clientId = await getClientId(globalState)

src/test/shared/sam/cli/samCliInvokerUtils.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('addTelemetryEnvVar', async function () {
9393
assert.deepStrictEqual(result, {
9494
cwd: '/foo',
9595
env: {
96-
SAM_CLI_TELEMETRY_FROM_IDE: result.env?.['SAM_CLI_TELEMETRY_FROM_IDE'],
96+
AWS_TOOLING_USER_AGENT: result.env?.['AWS_TOOLING_USER_AGENT'],
9797
AWS_REGION: 'us-east-1',
9898
},
9999
})

src/test/shared/telemetry/util.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as assert from 'assert'
77
import { Memento, ConfigurationTarget } from 'vscode'
88
import { Settings } from '../../../shared/settings'
9-
import { convertLegacy, getClientId, getUserAgent, TelemetryConfig } from '../../../shared/telemetry/util'
9+
import { convertLegacy, getClientId, getUserAgent, platformPair, TelemetryConfig } from '../../../shared/telemetry/util'
1010
import { extensionVersion } from '../../../shared/vscode/env'
1111
import { FakeMemento } from '../../fakeExtensionContext'
1212

@@ -166,6 +166,12 @@ describe('getUserAgent', function () {
166166
assert.ok(lastPair?.startsWith(`AWS-Toolkit-For-VSCode/${extensionVersion}`))
167167
})
168168

169+
it('includes only one pair by default', async function () {
170+
const userAgent = await getUserAgent()
171+
const pairs = userAgent.split(' ')
172+
assert.strictEqual(pairs.length, 1)
173+
})
174+
169175
it('omits `ClientId` by default', async function () {
170176
const userAgent = await getUserAgent()
171177
assert.ok(!userAgent.includes('ClientId'))
@@ -176,4 +182,12 @@ describe('getUserAgent', function () {
176182
const lastPair = userAgent.split(' ').pop()
177183
assert.ok(lastPair?.startsWith('ClientId/'))
178184
})
185+
186+
it('includes the platform before `ClientId` if opted in', async function () {
187+
const userAgent = await getUserAgent({ includePlatform: true, includeClientId: true })
188+
const pairs = userAgent.split(' ')
189+
const clientPairIndex = pairs.findIndex(pair => pair.startsWith('ClientId/'))
190+
const beforeClient = pairs[clientPairIndex - 1]
191+
assert.strictEqual(beforeClient, platformPair())
192+
})
179193
})

0 commit comments

Comments
 (0)