refactor: sync isFirstUse w/ ClientId

Problem:

We were seeing some cases where the ClientId was seen as new, but
ifFirstUse was reporting as not new.

Solution:

Validate that if we have a new ClientId, that isFirstUse MUST be true.
Otherwise we emit an error, that should help us debug certain cases.

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

Author: nkomonen-amazon

packages/core/src/auth/utils.ts

@@ -58,7 +58,7 @@ import { EcsCredentialsProvider } from './providers/ecsCredentialsProvider'
 import { EnvVarsCredentialsProvider } from './providers/envVarsCredentialsProvider'
 import { showMessageWithUrl } from '../shared/utilities/messages'
 import { credentialHelpUrl } from '../shared/constants'
-import { ExtStartUpSources, ExtStartUpSource } from '../shared/telemetry/util'
+import { ExtStartUpSources, ExtStartUpSource, hadClientIdOnStartup } from '../shared/telemetry/util'
 
 // iam-only excludes Builder ID and IAM Identity Center from the list of valid connections
 // TODO: Understand if "iam" should include these from the list at all
@@ -695,6 +695,18 @@ export class ExtensionUse {
             return this.isFirstUseCurrentSession
         }
 
+        /**
+         * SANITY CHECK: If the clientId already existed on startup, then isFirstUse MUST be false. So
+         * there is a bug in the state.
+         */
+        if (hadClientIdOnStartup(globals.globalState)) {
+            telemetry.function_call.emit({
+                result: 'Failed',
+                functionName: 'isFirstUse',
+                reason: 'ClientIdAlreadyExisted',
+            })
+        }
+
         if (isAmazonQ()) {
             this.isFirstUseCurrentSession = true
         } else {

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

@@ -30,6 +30,7 @@ import { asStringifiedStack, FunctionEntry } from './spans'
 import { telemetry } from './telemetry'
 import { v5 as uuidV5 } from 'uuid'
 import { ToolkitError } from '../errors'
+import { GlobalState } from '../globalState'
 
 const legacySettingsTelemetryValueDisable = 'Disable'
 const legacySettingsTelemetryValueEnable = 'Enable'
@@ -177,6 +178,8 @@ export const getClientId = memoize(
             const localClientId = globalState.tryGet('telemetryClientId', String) // local to extension, despite accessing "global" state
             let clientId: string
 
+            _hadClientIdOnStartup = !!globalClientId || !!localClientId
+
             if (isWeb()) {
                 const machineId = vscode.env.machineId
                 clientId = localClientId ?? machineId
@@ -210,6 +213,17 @@ export const getClientId = memoize(
     }
 )
 
+let _hadClientIdOnStartup = false
+/**
+ * Returns true if the ClientID existed before this session started
+ */
+export const hadClientIdOnStartup = (globalState: GlobalState) => {
+    // triggers the flow that will update the state, if not done already
+    getClientId(globalState)
+
+    return _hadClientIdOnStartup
+}
+
 export const platformPair = () => `${env.appName.replace(/\s/g, '-')}/${version}`
 
 /**

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

@@ -10,6 +10,7 @@ import {
     convertLegacy,
     getClientId,
     getUserAgent,
+    hadClientIdOnStartup,
     platformPair,
     SessionId,
     telemetryClientIdEnvKey,
@@ -128,20 +129,24 @@ describe('getSessionId', function () {
 
 describe('getClientId', function () {
     before(function () {
-        delete process.env[telemetryClientIdEnvKey]
+        setClientIdEnvVar(undefined)
     })
 
     afterEach(function () {
-        delete process.env[telemetryClientIdEnvKey]
+        setClientIdEnvVar(undefined)
     })
 
     function testGetClientId(globalState: GlobalState) {
         return getClientId(globalState, true, false, randomUUID())
     }
 
+    function setClientIdEnvVar(val: string | undefined) {
+        process.env[telemetryClientIdEnvKey] = val
+    }
+
     it('generates a unique id if no other id is available', function () {
         const c1 = testGetClientId(new GlobalState(new FakeMemento()))
-        delete process.env[telemetryClientIdEnvKey]
+        setClientIdEnvVar(undefined)
         const c2 = testGetClientId(new GlobalState(new FakeMemento()))
         assert.notStrictEqual(c1, c2)
     })
@@ -160,7 +165,7 @@ describe('getClientId', function () {
 
         const e = new GlobalState(new FakeMemento())
         await e.update('telemetryClientId', randomUUID())
-        process.env[telemetryClientIdEnvKey] = expectedClientId
+        setClientIdEnvVar(expectedClientId)
 
         assert.strictEqual(testGetClientId(new GlobalState(new FakeMemento())), expectedClientId)
     })
@@ -218,6 +223,25 @@ describe('getClientId', function () {
         const clientId = getClientId(new GlobalState(new FakeMemento()), false, false)
         assert.strictEqual(clientId, '11111111-1111-1111-1111-111111111111')
     })
+
+    describe('hadClientIdOnStartup', async function () {
+        it('returns false when no existing clientId', async function () {
+            const globalState = new GlobalState(new FakeMemento())
+            assert.strictEqual(hadClientIdOnStartup(globalState), false)
+        })
+
+        it('returns true when existing env var clientId', async function () {
+            const globalState = new GlobalState(new FakeMemento())
+            setClientIdEnvVar('aaa-111')
+            assert.strictEqual(hadClientIdOnStartup(globalState), true)
+        })
+
+        it('returns true when existing state clientId', async function () {
+            const globalState = new GlobalState(new FakeMemento())
+            await globalState.update('telemetryClientId', 'bbb-222')
+            assert.strictEqual(hadClientIdOnStartup(globalState), true)
+        })
+    })
 })
 
 describe('getUserAgent', function () {