Skip to content

Commit 35502be

Browse files
telemetry(auth): Update metrics to better debug Auth dropoff (#6625)
## Problem We noticed that there was an auth dropoff between `session_start` with a brand new clientId, versus when the `auth_userState` metric indicated `isFirstUse` meaning the user is net new. We went from 12.7k for `session_start` to 9.8k for `auth_userState`, these should have been basically the same. ## Solution Add in certain metrics to help debug where the discrepancy is coming from: - When we determine the user is a first time user, we will also check if the clientId is newly generated. If this is not the case we know there is a discrepancy here - The relevant metric will be `function_call` with a `functionName: isFirstUse`, `result: Failed`, and a `reason: ClientIdAlreadyExisted` - When we determine the user is a first time user, if we detected that they had previous auth connections, this will indicate a likely cause for the discrepancy - The relevant metric will be `function_call` with `reason: UnexpectedConnections` - We will emit metrics when the Auth Login page loads since that also had a discrepancy and the telemetry did not exist - The relevant metric is `webview_load` and it will indicate when the Auth Login/Reauth page has actually loaded - Previously we were observing the telemetry for the command `aws.amazonq.focusChat`, but all this did was emit when called and didn't confirm the UI actually loaded. - We will also add the `isFirstUse` metric source value in to some other existing metrics --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon <[email protected]>
1 parent bf6e097 commit 35502be

File tree

11 files changed

+151
-22
lines changed

11 files changed

+151
-22
lines changed

packages/amazonq/src/extension.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ export async function activateAmazonQCommon(context: vscode.ExtensionContext, is
142142
// Give time for the extension to finish initializing.
143143
globals.clock.setTimeout(async () => {
144144
CommonAuthWebview.authSource = ExtStartUpSources.firstStartUp
145-
void focusAmazonQPanel.execute(placeholder, 'firstStartUp')
145+
void focusAmazonQPanel.execute(placeholder, ExtStartUpSources.firstStartUp)
146146
}, 1000)
147147
}
148148
}

packages/core/src/auth/utils.ts

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import { EcsCredentialsProvider } from './providers/ecsCredentialsProvider'
5858
import { EnvVarsCredentialsProvider } from './providers/envVarsCredentialsProvider'
5959
import { showMessageWithUrl } from '../shared/utilities/messages'
6060
import { credentialHelpUrl } from '../shared/constants'
61-
import { ExtStartUpSources, ExtStartUpSource } from '../shared/telemetry/util'
61+
import { ExtStartUpSources, ExtStartUpSource, hadClientIdOnStartup } from '../shared/telemetry/util'
6262

6363
// iam-only excludes Builder ID and IAM Identity Center from the list of valid connections
6464
// TODO: Understand if "iam" should include these from the list at all
@@ -688,17 +688,43 @@ export class ExtensionUse {
688688
return this.isFirstUseCurrentSession
689689
}
690690

691-
this.isFirstUseCurrentSession = globals.globalState.get('isExtensionFirstUse')
692-
if (this.isFirstUseCurrentSession === undefined) {
691+
// This is for sure not their first use
692+
const isFirstUse = globals.globalState.tryGet('isExtensionFirstUse', Boolean)
693+
if (isFirstUse === false) {
694+
this.isFirstUseCurrentSession = isFirstUse
695+
return this.isFirstUseCurrentSession
696+
}
697+
698+
/**
699+
* SANITY CHECK: If the clientId already existed on startup, then isFirstUse MUST be false. So
700+
* there is a bug in the state.
701+
*/
702+
if (hadClientIdOnStartup(globals.globalState)) {
703+
telemetry.function_call.emit({
704+
result: 'Failed',
705+
functionName: 'isFirstUse',
706+
reason: 'ClientIdAlreadyExisted',
707+
})
708+
}
709+
710+
if (isAmazonQ()) {
711+
this.isFirstUseCurrentSession = true
712+
if (hasExistingConnections()) {
713+
telemetry.function_call.emit({
714+
result: 'Failed',
715+
functionName: 'isFirstUse',
716+
reason: 'UnexpectedConnections',
717+
})
718+
}
719+
} else {
693720
// The variable in the store is not defined yet, fallback to checking if they have existing connections.
694721
this.isFirstUseCurrentSession = !hasExistingConnections()
695-
696-
getLogger().debug(
697-
`isFirstUse: State not found, marking user as '${
698-
this.isFirstUseCurrentSession ? '' : 'NOT '
699-
}first use' since they 'did ${this.isFirstUseCurrentSession ? 'NOT ' : ''}have existing connections'.`
700-
)
701722
}
723+
getLogger().debug(
724+
`isFirstUse: State not found, marking user as '${
725+
this.isFirstUseCurrentSession ? '' : 'NOT '
726+
}first use' since they 'did ${this.isFirstUseCurrentSession ? 'NOT ' : ''}have existing connections'.`
727+
)
702728

703729
// Update state, so next time it is not first use
704730
this.updateMemento('isExtensionFirstUse', false)

packages/core/src/login/webview/commonAuthViewProvider.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { AuthSources } from './util'
4646
import { AuthFlowStates } from './vue/types'
4747
import { getTelemetryMetadataForConn } from '../../auth/connection'
4848
import { AuthUtil } from '../../codewhisperer/util/authUtil'
49+
import { ExtensionUse } from '../../auth/utils'
4950

5051
export class CommonAuthViewProvider implements WebviewViewProvider {
5152
public readonly viewType: string
@@ -83,14 +84,22 @@ export class CommonAuthViewProvider implements WebviewViewProvider {
8384
) {
8485
// Our callback won't fire on the first view.
8586
if (webviewView.visible) {
86-
telemetry.auth_signInPageOpened.emit({ result: 'Succeeded', passive: true })
87+
telemetry.auth_signInPageOpened.emit({
88+
result: 'Succeeded',
89+
passive: true,
90+
source: ExtensionUse.instance.sourceForTelemetry(),
91+
})
8792
}
8893

8994
// This will fire whenever the user opens or closes the login page from 'somewhere else'
9095
// i.e. NOT when switching from/to the chat window, which uses the same view area.
9196
webviewView.onDidChangeVisibility(async () => {
9297
if (webviewView.visible) {
93-
telemetry.auth_signInPageOpened.emit({ result: 'Succeeded', passive: true })
98+
telemetry.auth_signInPageOpened.emit({
99+
result: 'Succeeded',
100+
passive: true,
101+
source: ExtensionUse.instance.sourceForTelemetry(),
102+
})
94103
} else {
95104
telemetry.auth_signInPageClosed.emit({ result: 'Succeeded', passive: true })
96105

packages/core/src/login/webview/vue/backend.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,22 @@ export abstract class CommonAuthWebview extends VueWebview {
6161
return globals.regionProvider.getRegions().reverse()
6262
}
6363

64+
private didCall: { login: boolean; reauth: boolean } = { login: false, reauth: false }
65+
public setUiReady(state: 'login' | 'reauth') {
66+
// Prevent telemetry spam, since showing/hiding chat triggers this each time.
67+
// So only emit once.
68+
if (this.didCall[state]) {
69+
return
70+
}
71+
72+
telemetry.webview_load.emit({
73+
passive: true,
74+
webviewName: state,
75+
result: 'Succeeded',
76+
})
77+
this.didCall[state] = true
78+
}
79+
6480
/**
6581
* This wraps the execution of the given setupFunc() and handles common errors from the SSO setup process.
6682
*

packages/core/src/login/webview/vue/login.vue

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ export default defineComponent({
368368
// Pre-select the first available login option
369369
await this.preselectLoginOption()
370370
await this.handleUrlInput() // validate the default startUrl
371+
372+
await client.setUiReady('login')
371373
},
372374
methods: {
373375
toggleItemSelection(itemId: number) {

packages/core/src/login/webview/vue/reauthenticate.vue

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ export default defineComponent({
127127
128128
this.doShow = true
129129
},
130+
async mounted() {
131+
await client.setUiReady('reauth')
132+
},
130133
methods: {
131134
async reauthenticate() {
132135
client.emitUiClick('auth_reauthenticate')

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { telemetry, MetricBase } from './telemetry'
2222
import fs from '../fs/fs'
2323
import fsNode from 'fs/promises'
2424
import * as collectionUtil from '../utilities/collectionUtils'
25+
import { ExtensionUse } from '../../auth/utils'
2526

2627
export type TelemetryService = ClassToInterfaceType<DefaultTelemetryService>
2728

@@ -98,7 +99,9 @@ export class DefaultTelemetryService {
9899
// TODO: `readEventsFromCache` should be async
99100
this._eventQueue.push(...(await DefaultTelemetryService.readEventsFromCache(this.persistFilePath)))
100101
this._endOfCache = this._eventQueue[this._eventQueue.length - 1]
101-
telemetry.session_start.emit()
102+
telemetry.session_start.emit({
103+
source: ExtensionUse.instance.sourceForTelemetry(),
104+
})
102105
this.startFlushInterval()
103106
}
104107

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { asStringifiedStack, FunctionEntry } from './spans'
3030
import { telemetry } from './telemetry'
3131
import { v5 as uuidV5 } from 'uuid'
3232
import { ToolkitError } from '../errors'
33+
import { GlobalState } from '../globalState'
3334

3435
const legacySettingsTelemetryValueDisable = 'Disable'
3536
const legacySettingsTelemetryValueEnable = 'Enable'
@@ -177,6 +178,8 @@ export const getClientId = memoize(
177178
const localClientId = globalState.tryGet('telemetryClientId', String) // local to extension, despite accessing "global" state
178179
let clientId: string
179180

181+
_hadClientIdOnStartup = !!globalClientId || !!localClientId
182+
180183
if (isWeb()) {
181184
const machineId = vscode.env.machineId
182185
clientId = localClientId ?? machineId
@@ -210,6 +213,22 @@ export const getClientId = memoize(
210213
}
211214
)
212215

216+
let _hadClientIdOnStartup = false
217+
/**
218+
* Returns true if the ClientID existed before this session started
219+
*/
220+
export const hadClientIdOnStartup = (
221+
globalState: GlobalState,
222+
update = (globalState: GlobalState) => {
223+
getClientId(globalState)
224+
}
225+
) => {
226+
// triggers the flow that will update the state, if not done already
227+
update(globalState)
228+
229+
return _hadClientIdOnStartup
230+
}
231+
213232
export const platformPair = () => `${env.appName.replace(/\s/g, '-')}/${version}`
214233

215234
/**

packages/core/src/shared/telemetry/vscodeTelemetry.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,17 @@
11311131
],
11321132
"passive": true
11331133
},
1134+
{
1135+
"name": "auth_signInPageOpened",
1136+
"description": "When the Amazon Q sign in page is opened and focused.",
1137+
"metadata": [
1138+
{
1139+
"type": "source",
1140+
"required": true
1141+
}
1142+
],
1143+
"passive": true
1144+
},
11341145
{
11351146
"name": "function_call",
11361147
"description": "Represents a function call. In most cases this should wrap code with a run(), then you can add context.",
@@ -1207,6 +1218,17 @@
12071218
}
12081219
]
12091220
},
1221+
{
1222+
"name": "session_start",
1223+
"description": "Called when starting the plugin",
1224+
"metadata": [
1225+
{
1226+
"type": "source",
1227+
"required": false
1228+
}
1229+
],
1230+
"passive": true
1231+
},
12101232
{
12111233
"name": "session_end",
12121234
"description": "Called when stopping the IDE on a best effort basis",

packages/core/src/test/credentials/utils.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,23 @@ import globals from '../../shared/extensionGlobals'
1111

1212
describe('ExtensionUse.isFirstUse()', function () {
1313
let instance: ExtensionUse
14+
const notHasExistingConnections = () => false
1415

1516
beforeEach(async function () {
1617
instance = new ExtensionUse()
17-
await globals.globalState.update(ExtensionUse.instance.isExtensionFirstUseKey, true)
18+
await makeStateValueNotExist()
1819
})
1920

2021
it('is true only on first startup', function () {
21-
assert.strictEqual(instance.isFirstUse(), true, 'Failed on first call.')
22-
assert.strictEqual(instance.isFirstUse(), true, 'Failed on second call.')
22+
assert.strictEqual(instance.isFirstUse(notHasExistingConnections), true, 'Failed on first call.')
23+
assert.strictEqual(instance.isFirstUse(notHasExistingConnections), true, 'Failed on second call.')
2324

2425
const nextStartup = nextExtensionStartup()
25-
assert.strictEqual(nextStartup.isFirstUse(), false, 'Failed on new startup.')
26+
assert.strictEqual(nextStartup.isFirstUse(notHasExistingConnections), false, 'Failed on new startup.')
2627
})
2728

2829
it('true when: (state value not exists + NOT has existing connections)', async function () {
2930
await makeStateValueNotExist()
30-
const notHasExistingConnections = () => false
3131
assert.strictEqual(
3232
instance.isFirstUse(notHasExistingConnections),
3333
true,

0 commit comments

Comments
 (0)