Skip to content

Commit ee95f5f

Browse files
telemetry: Add sessionId to metrics (aws#5510)
### Do not merge until service changes are live ## Problem: I have filtered all the metrics in Kibana by `clientId: "XXXXXX"`, but if they had multiple IDE windows opened at the same time sending metrics I cannot know which window they are coming from since they share the same clientID. ## Solution: This adds a new field to the telemetry service called `sessionId` which uniquely identifies a part of the UI in an IDE window. It is adjacent to `clientId`. Now in my metrics I will be able to differentiate metrics by their sessionID since they both share the same clientId by design. ## Implementation Details: Changes in the Telemetry service need to be merged before this change can since they update the endpoint to accept this new field. The updates of this commit only mirror the upstream, so this process is manual. --- <!--- REMINDER: Ensure that your PR meets the guidelines in CONTRIBUTING.md --> License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: Nikolas Komonen <[email protected]>
1 parent 01d166b commit ee95f5f

File tree

8 files changed

+106
-8
lines changed

8 files changed

+106
-8
lines changed

packages/core/src/shared/crypto.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,15 @@ export function randomUUID(): `${string}-${string}-${string}-${string}-${string}
3030

3131
return require('crypto').randomUUID()
3232
}
33+
34+
/**
35+
* Returns true if the given string is a UUID
36+
*
37+
* NOTE: There are different UUID versions, this function does not discriminate between them.
38+
* See: https://stackoverflow.com/questions/7905929/how-to-test-valid-uuid-guid
39+
*/
40+
export function isUuid(uuid: string): boolean {
41+
// NOTE: This pattern must match, or at least be a subset of the "Session ID" pattern in `telemetry/service-2.json`
42+
const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
43+
return uuidPattern.test(uuid)
44+
}

packages/core/src/shared/telemetry/activation.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { DefaultTelemetryService } from './telemetryService'
1414
import { getLogger } from '../logger'
1515
import { getComputeRegion, isAmazonQ, isCloud9, productName } from '../extensionUtilities'
1616
import { openSettingsId, Settings } from '../settings'
17-
import { TelemetryConfig } from './util'
17+
import { getSessionId, TelemetryConfig } from './util'
1818
import { isAutomation, isReleaseVersion } from '../vscode/env'
1919
import { AWSProduct } from './clienttelemetry'
2020
import { DefaultTelemetryClient } from './telemetryClient'
@@ -76,6 +76,12 @@ export async function activate(
7676
}
7777

7878
await globals.telemetry.start()
79+
80+
if (globals.telemetry.telemetryEnabled) {
81+
// Only log the IDs if telemetry is enabled, so that users who have it disabled do not think we are sending events.
82+
getLogger().info(`Telemetry Client ID: ${globals.telemetry.clientId}`)
83+
getLogger().info(`Telemetry Session ID: ${getSessionId()}`)
84+
}
7985
} catch (e) {
8086
// Only throw in a production build because:
8187
// 1. Telemetry must never prevent normal Toolkit operation.

packages/core/src/shared/telemetry/service-2.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@
5858
"type":"string",
5959
"pattern":"^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$"
6060
},
61+
"SessionID":{
62+
"type":"string",
63+
"pattern":"^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$"
64+
},
6165
"Command":{
6266
"type":"string",
6367
"max":2000
@@ -207,6 +211,7 @@
207211
"AWSProduct":{"shape":"AWSProduct"},
208212
"AWSProductVersion":{"shape":"AWSProductVersion"},
209213
"ClientID":{"shape":"ClientID"},
214+
"SessionID":{"shape":"SessionID"},
210215
"OS":{"shape":"Value"},
211216
"OSVersion":{"shape":"Value"},
212217
"ComputeEnv": {"shape":"ComputeEnv"},

packages/core/src/shared/telemetry/telemetryClient.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { ServiceConfigurationOptions } from 'aws-sdk/lib/service'
1717
import globals from '../extensionGlobals'
1818
import { DevSettings } from '../settings'
1919
import { ClassToInterfaceType } from '../utilities/tsUtils'
20-
import { getComputeEnvType } from './util'
20+
import { getComputeEnvType, getSessionId } from './util'
2121

2222
export const accountMetadataKey = 'awsAccount'
2323
export const regionKey = 'awsRegion'
@@ -106,6 +106,7 @@ export class DefaultTelemetryClient implements TelemetryClient {
106106
AWSProduct: DefaultTelemetryClient.productName,
107107
AWSProductVersion: extensionVersion,
108108
ClientID: this.clientId,
109+
SessionID: getSessionId(),
109110
OS: os.platform(),
110111
OSVersion: os.release(),
111112
ComputeEnv: await getComputeEnvType(),

packages/core/src/shared/telemetry/telemetryService.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ export class DefaultTelemetryService {
133133
getLogger().verbose(`Telemetry is ${value ? 'enabled' : 'disabled'}`)
134134
}
135135

136+
private _clientId: string | undefined
137+
/** Returns the client ID, creating one if it does not exist. */
138+
public get clientId(): string {
139+
return (this._clientId ??= getClientId(globals.globalState))
140+
}
141+
136142
public get timer(): NodeJS.Timer | undefined {
137143
return this._timer
138144
}
@@ -224,8 +230,6 @@ export class DefaultTelemetryService {
224230

225231
private async createDefaultPublisher(): Promise<TelemetryPublisher | undefined> {
226232
try {
227-
// grab our clientId and generate one if it doesn't exist
228-
const clientId = getClientId(globals.globalState)
229233
// grab our Cognito identityId
230234
const poolId = DefaultTelemetryClient.config.identityPool
231235
const identityMapJson = globals.globalState.tryGet(
@@ -240,7 +244,7 @@ export class DefaultTelemetryService {
240244

241245
// if we don't have an identity, get one
242246
if (!identity) {
243-
const identityPublisherTuple = await DefaultTelemetryPublisher.fromDefaultIdentityPool(clientId)
247+
const identityPublisherTuple = await DefaultTelemetryPublisher.fromDefaultIdentityPool(this.clientId)
244248

245249
// save it
246250
identityMap.set(poolId, identityPublisherTuple.cognitoIdentityId)
@@ -252,7 +256,7 @@ export class DefaultTelemetryService {
252256
// return the publisher
253257
return identityPublisherTuple.publisher
254258
} else {
255-
return DefaultTelemetryPublisher.fromIdentityId(clientId, identity)
259+
return DefaultTelemetryPublisher.fromIdentityId(this.clientId, identity)
256260
}
257261
} catch (err) {
258262
getLogger().error(`Got ${err} while initializing telemetry publisher`)

packages/core/src/shared/telemetry/util.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { env, version } from 'vscode'
88
import * as os from 'os'
99
import { getLogger } from '../logger'
1010
import { fromExtensionManifest, migrateSetting, Settings } from '../settings'
11-
import { memoize } from '../utilities/functionUtils'
11+
import { memoize, once } from '../utilities/functionUtils'
1212
import {
1313
isInDevEnv,
1414
extensionVersion,
@@ -28,6 +28,7 @@ import { randomUUID } from '../crypto'
2828
import { ClassToInterfaceType } from '../utilities/tsUtils'
2929
import { FunctionEntry, type TelemetryTracer } from './spans'
3030
import { telemetry } from './telemetry'
31+
import { v5 as uuidV5 } from 'uuid'
3132

3233
const legacySettingsTelemetryValueDisable = 'Disable'
3334
const legacySettingsTelemetryValueEnable = 'Enable'
@@ -90,6 +91,41 @@ export function convertLegacy(value: unknown): boolean {
9091
}
9192
}
9293

94+
/**
95+
* Returns an identifier that uniquely identifies a single application
96+
* instance/window of a specific IDE. I.e if I have multiple VS Code
97+
* windows open each one will have a unique session ID. This session ID
98+
* can be used in conjunction with the client ID to differntiate between
99+
* different VS Code windows on a users machine.
100+
*
101+
* ### Rules:
102+
*
103+
* - On startup of a new application instance, a new session ID must be created.
104+
* - This identifier must be a `UUID`, it is enforced by the telemetry service.
105+
* - The session ID must be different from all other IDs on the machine
106+
* - A session ID exists until the application instance is terminated.
107+
* It should never be used again after termination.
108+
* - All extensions on the same application instance MUST return the same session ID.
109+
* - This will allow us to know which of our extensions (eg Q vs Toolkit) are in the
110+
* same VS Code window.
111+
*
112+
* `vscode.env.sessionId` behaves as described aboved, EXCEPT its
113+
* value looks close to a UUID by does not exactly match it (has additional characters).
114+
* As a result we process it through uuidV5 which creates a proper UUID from it.
115+
* uuidV5 is idempotent, so as long as `vscode.env.sessionId` returns the same value,
116+
* we will get the same UUID.
117+
*/
118+
export const getSessionId = once(() => uuidV5(vscode.env.sessionId, sessionIdNonce))
119+
/**
120+
* This is an arbitrary nonce that is used in creating a v5 UUID for Session ID. We only
121+
* have this since the spec requires it.
122+
* - This should ONLY be used by {@link getSessionId}.
123+
* - This value MUST NOT change during runtime, otherwise {@link getSessionId} will lose its
124+
* idempotency. But, if there was a reason to change the value in a PR, it would not be an issue.
125+
* - This is exported only for testing.
126+
*/
127+
export const sessionIdNonce = '44cfdb20-b30b-4585-a66c-9f48f24f99b5' as const
128+
93129
/**
94130
* Calculates the clientId for the current profile. This calculation is performed once
95131
* on first call and the result is stored for the remainder of the session.

packages/core/src/test/shared/crypto.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import assert from 'assert'
7-
import { randomUUID } from '../../shared/crypto'
7+
import { isUuid, randomUUID } from '../../shared/crypto'
88

99
describe('crypto', function () {
1010
describe('randomUUID()', function () {
@@ -25,4 +25,22 @@ describe('crypto', function () {
2525
assert(uuidPattern.test('not-a-uuid') === false)
2626
})
2727
})
28+
29+
describe('isUuid()', function () {
30+
assert.equal(isUuid(''), false)
31+
assert.equal(isUuid('not-a-uuid'), false)
32+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971'), false)
33+
assert.equal(isUuid('47fe01cf_f37a_4e7c-b971-@10fe5897763'), false)
34+
// The '9' in '9e7c' must actually be between 1-5 based on the UUID spec: https://stackoverflow.com/a/38191104
35+
assert.equal(isUuid('47fe01cf-f37a-9e7c-b971-d10fe5897763'), false)
36+
assert.equal(isUuid(' 47fe01cf-f37a-4e7c-b971-d10fe5897763'), false) // leading whitespace
37+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763 '), false) // trailing whitespace
38+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763z'), false) // one extra character
39+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763 blah'), false) // trailing word
40+
41+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763'), true)
42+
// The telemetry services indicates that per postel's law, uppercase is valid to pass in
43+
// as they will lowerCase when necessary.
44+
assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763'.toUpperCase()), true)
45+
})
2846
})

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,18 @@ import { Settings } from '../../../shared/settings'
99
import {
1010
convertLegacy,
1111
getClientId,
12+
getSessionId,
1213
getUserAgent,
1314
platformPair,
15+
sessionIdNonce,
1416
telemetryClientIdEnvKey,
1517
TelemetryConfig,
1618
} from '../../../shared/telemetry/util'
1719
import { extensionVersion } from '../../../shared/vscode/env'
1820
import { FakeMemento } from '../../fakeExtensionContext'
1921
import { GlobalState } from '../../../shared/globalState'
2022
import { randomUUID } from 'crypto'
23+
import { isUuid } from '../../../shared/crypto'
2124

2225
describe('TelemetryConfig', function () {
2326
const settingKey = 'aws.telemetry'
@@ -107,6 +110,19 @@ describe('TelemetryConfig', function () {
107110
})
108111
})
109112

113+
describe('getSessionId', function () {
114+
it('returns a stable UUID', function () {
115+
const result = getSessionId()
116+
117+
assert.deepStrictEqual(isUuid(result), true)
118+
assert.deepStrictEqual(getSessionId(), result, 'Subsequent call did not return the same UUID')
119+
})
120+
121+
it('nonce is the same as always', function () {
122+
assert.deepStrictEqual(sessionIdNonce, '44cfdb20-b30b-4585-a66c-9f48f24f99b5')
123+
})
124+
})
125+
110126
describe('getClientId', function () {
111127
before(function () {
112128
delete process.env[telemetryClientIdEnvKey]

0 commit comments

Comments
 (0)