bug(telemetry): sessionId not unique

Problem:

There seems to be a bug with vscode.env.sessionId since we observed in telemetry
multiple clientIds who has the same sessionId.

Solution:

Generate our own sessionId and share it between extensions through globalThis

Signed-off-by: nkomonen-amazon <[email protected]>

Author: nkomonen-amazon

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

@@ -79,8 +79,8 @@ export async function activate(
 
         if (globals.telemetry.telemetryEnabled) {
             // Only log the IDs if telemetry is enabled, so that users who have it disabled do not think we are sending events.
-            getLogger().info(`Telemetry Client ID: ${globals.telemetry.clientId}`)
-            getLogger().info(`Telemetry Session ID: ${getSessionId()}`)
+            getLogger().info(`Telemetry clientId: ${globals.telemetry.clientId}`)
+            getLogger().info(`Telemetry sessionId: ${getSessionId()}`)
         }
     } catch (e) {
         // Only throw in a production build because:

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

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+/* eslint-disable aws-toolkits/no-console-log */
+
 import * as vscode from 'vscode'
 import { env, version } from 'vscode'
 import * as os from 'os'
@@ -28,7 +30,7 @@ import { randomUUID } from '../crypto'
 import { ClassToInterfaceType } from '../utilities/tsUtils'
 import { FunctionEntry, type TelemetryTracer } from './spans'
 import { telemetry } from './telemetry'
-import { v5 as uuidV5 } from 'uuid'
+import { v4 as uuidV4, v5 as uuidV5 } from 'uuid'
 
 const legacySettingsTelemetryValueDisable = 'Disable'
 const legacySettingsTelemetryValueEnable = 'Enable'
@@ -98,33 +100,62 @@ export function convertLegacy(value: unknown): boolean {
  * can be used in conjunction with the client ID to differntiate between
  * different VS Code windows on a users machine.
  *
- * ### Rules:
- *
- * - On startup of a new application instance, a new session ID must be created.
- *   - This identifier must be a `UUID`, it is enforced by the telemetry service.
- * - The session ID must be different from all other IDs on the machine
- * - A session ID exists until the application instance is terminated.
- *   It should never be used again after termination.
- * - All extensions on the same application instance MUST return the same session ID.
- *   - This will allow us to know which of our extensions (eg Q vs Toolkit) are in the
- *     same VS Code window.
- *
- * `vscode.env.sessionId` behaves as described aboved, EXCEPT its
- * value looks close to a UUID by does not exactly match it (has additional characters).
- * As a result we process it through uuidV5 which creates a proper UUID from it.
- * uuidV5 is idempotent, so as long as `vscode.env.sessionId` returns the same value,
- * we will get the same UUID.
- */
-export const getSessionId = once(() => uuidV5(vscode.env.sessionId, sessionIdNonce))
-/**
- * This is an arbitrary nonce that is used in creating a v5 UUID for Session ID. We only
- * have this since the spec requires it.
- * - This should ONLY be used by {@link getSessionId}.
- * - This value MUST NOT change during runtime, otherwise {@link getSessionId} will lose its
- *   idempotency. But, if there was a reason to change the value in a PR, it would not be an issue.
- * - This is exported only for testing.
+ * See spec: https://quip-amazon.com/9gqrAqwO5FCE
  */
-export const sessionIdNonce = '44cfdb20-b30b-4585-a66c-9f48f24f99b5' as const
+export const getSessionId = once(() => SessionId.getSessionId())
+
+class SessionId {
+    public static getSessionId() {
+        // This implementation does not work in web
+        if (!isWeb()) {
+            return this._getSessionId()
+        }
+        // A best effort at a sessionId just for web mode
+        return this._getVscSessionId()
+    }
+
+    /**
+     * This implementation assumes that the `globalThis` is shared between extensions in the same
+     * Extension Host, so we can share a global variable that way.
+     *
+     * This does not seem to work on web mode since the `globalThis` is not shared due to WebWorker design
+     */
+    private static _getSessionId() {
+        const global = globalThis as any
+        getLogger().debug('getSessionId: global.amzn_sessionId=%s', global.amzn_sessionId)
+        if (global.amzn_sessionId === undefined) {
+            getLogger().debug('getSessionId: creating new sessionId')
+            ;(globalThis as any).amzn_sessionId = uuidV4()
+        }
+        getLogger().debug('getSessionId: returning %s', global.amzn_sessionId)
+        return global.amzn_sessionId
+    }
+
+    /**
+     * `vscode.env.sessionId` looks close to a UUID by does not exactly match it (has additional characters).
+     * As a result we process it through uuidV5 which creates a proper UUID from it.
+     * uuidV5 is idempotent, so as long as `vscode.env.sessionId` returns the same value,
+     * we will get the same UUID.
+     *
+     * We were initially using this implementation for all session ids, but it has some caveats:
+     * - If the extension host crashes, sesionId stays the same since the parent VSC process defines it and that does not crash.
+     *   We wanted it to generate a new sessionId on ext host crash.
+     * - This value may not be reliable, see the following sessionId in telemetry, it contains many events
+     *   all from different client ids: `sessionId: cabea8e7-a8a1-5e51-a60e-07218f4a5937`
+     */
+    private static _getVscSessionId() {
+        return uuidV5(vscode.env.sessionId, this.sessionIdNonce)
+    }
+    /**
+     * This is an arbitrary nonce that is used in creating a v5 UUID for Session ID. We only
+     * have this since the spec requires it.
+     * - This should ONLY be used by {@link getSessionId}.
+     * - This value MUST NOT change during runtime, otherwise {@link getSessionId} will lose its
+     *   idempotency. But, if there was a reason to change the value in a PR, it would not be an issue.
+     * - This is exported only for testing.
+     */
+    private static readonly sessionIdNonce = '44cfdb20-b30b-4585-a66c-9f48f24f99b5'
+}
 
 /**
  * Calculates the clientId for the current profile. This calculation is performed once

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

@@ -12,7 +12,6 @@ import {
     getSessionId,
     getUserAgent,
     platformPair,
-    sessionIdNonce,
     telemetryClientIdEnvKey,
     TelemetryConfig,
 } from '../../../shared/telemetry/util'
@@ -117,10 +116,6 @@ describe('getSessionId', function () {
         assert.deepStrictEqual(isUuid(result), true)
         assert.deepStrictEqual(getSessionId(), result, 'Subsequent call did not return the same UUID')
     })
-
-    it('nonce is the same as always', function () {
-        assert.deepStrictEqual(sessionIdNonce, '44cfdb20-b30b-4585-a66c-9f48f24f99b5')
-    })
 })
 
 describe('getClientId', function () {