From 81151d12b1fb80658c12fae09ecc537136fdad2f Mon Sep 17 00:00:00 2001 From: Srinath Padmanabhan Date: Thu, 5 Mar 2026 16:06:08 -0800 Subject: [PATCH] Fixing review comments --- packages/core/src/code_assist/oauth2.test.ts | 12 +++---- packages/core/src/code_assist/oauth2.ts | 17 +++++---- packages/core/src/telemetry/metrics.ts | 36 ++++++++++---------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/packages/core/src/code_assist/oauth2.test.ts b/packages/core/src/code_assist/oauth2.test.ts index bce6fc1bcd..2db30c7fd3 100644 --- a/packages/core/src/code_assist/oauth2.test.ts +++ b/packages/core/src/code_assist/oauth2.test.ts @@ -15,8 +15,8 @@ import { authEvents, } from './oauth2.js'; import { - recordOnboardingStart, - recordOnboardingEnd, + recordGoogleAuthStart, + recordGoogleAuthEnd, } from '../telemetry/metrics.js'; import { UserAccountManager } from '../utils/userAccountManager.js'; import { OAuth2Client, Compute, GoogleAuth } from 'google-auth-library'; @@ -96,8 +96,8 @@ vi.mock('../mcp/token-storage/hybrid-token-storage.js', () => ({ })); vi.mock('../telemetry/metrics.js', () => ({ - recordOnboardingStart: vi.fn(), - recordOnboardingEnd: vi.fn(), + recordGoogleAuthStart: vi.fn(), + recordGoogleAuthEnd: vi.fn(), })); const mockConfig = { @@ -1378,8 +1378,8 @@ describe('oauth2', () => { await getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfig); - expect(recordOnboardingStart).toHaveBeenCalledWith(mockConfig); - expect(recordOnboardingEnd).toHaveBeenCalledWith(mockConfig); + expect(recordGoogleAuthStart).toHaveBeenCalledWith(mockConfig); + expect(recordGoogleAuthEnd).toHaveBeenCalledWith(mockConfig); }); it('should NOT record onboarding events for other auth types', async () => { diff --git a/packages/core/src/code_assist/oauth2.ts b/packages/core/src/code_assist/oauth2.ts index 862f01ea8f..8f766d4e02 100644 --- a/packages/core/src/code_assist/oauth2.ts +++ b/packages/core/src/code_assist/oauth2.ts @@ -20,8 +20,8 @@ import open from 'open'; import path from 'node:path'; import { promises as fs } from 'node:fs'; import { - recordOnboardingStart, - recordOnboardingEnd, + recordGoogleAuthStart, + recordGoogleAuthEnd, } from '../telemetry/metrics.js'; import type { Config } from '../config/config.js'; import { @@ -115,9 +115,9 @@ async function initOauthClient( authType: AuthType, config: Config, ): Promise { - const recordOnboardingEndIfApplicable = () => { + const recordGoogleAuthEndIfApplicable = () => { if (authType === AuthType.LOGIN_WITH_GOOGLE) { - recordOnboardingEnd(config); + recordGoogleAuthEnd(config); } }; @@ -152,7 +152,7 @@ async function initOauthClient( }); if (authType === AuthType.LOGIN_WITH_GOOGLE) { - recordOnboardingStart(config); + recordGoogleAuthStart(config); } const useEncryptedStorage = getUseEncryptedStorageFlag(); @@ -165,7 +165,6 @@ async function initOauthClient( access_token: process.env['GOOGLE_CLOUD_ACCESS_TOKEN'], }); await fetchAndCacheUserInfo(client); - recordOnboardingEndIfApplicable(); return client; } @@ -202,7 +201,7 @@ async function initOauthClient( debugLogger.log('Loaded cached credentials.'); await triggerPostAuthCallbacks(credentials as Credentials); - recordOnboardingEndIfApplicable(); + recordGoogleAuthEndIfApplicable(); return client; } } catch (error) { @@ -287,7 +286,7 @@ async function initOauthClient( } await triggerPostAuthCallbacks(client.credentials); - recordOnboardingEndIfApplicable(); + recordGoogleAuthEndIfApplicable(); } else { // In Zed integration, we skip the interactive consent and directly open the browser if (!config.getExperimentalZedIntegration()) { @@ -394,7 +393,7 @@ async function initOauthClient( }); await triggerPostAuthCallbacks(client.credentials); - recordOnboardingEndIfApplicable(); + recordGoogleAuthEndIfApplicable(); } return client; diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index 347497dd2e..22ea23df90 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -43,8 +43,8 @@ const KEYCHAIN_AVAILABILITY_COUNT = 'gemini_cli.keychain.availability.count'; const TOKEN_STORAGE_TYPE_COUNT = 'gemini_cli.token_storage.type.count'; const OVERAGE_OPTION_COUNT = 'gemini_cli.overage_option.count'; const CREDIT_PURCHASE_COUNT = 'gemini_cli.credit_purchase.count'; -const EVENT_ONBOARDING_START = 'gemini_cli.onboarding.start'; -const EVENT_ONBOARDING_END = 'gemini_cli.onboarding.end'; +const EVENT_GOOGLE_AUTH_START = 'gemini_cli.google_auth.start'; +const EVENT_GOOGLE_AUTH_END = 'gemini_cli.google_auth.end'; // Agent Metrics const AGENT_RUN_COUNT = 'gemini_cli.agent.run.count'; @@ -283,16 +283,16 @@ const COUNTER_DEFINITIONS = { model: string; }, }, - [EVENT_ONBOARDING_START]: { - description: 'Counts onboarding start events.', + [EVENT_GOOGLE_AUTH_START]: { + description: 'Counts auth "Login with Google" started.', valueType: ValueType.INT, - assign: (c: Counter) => (onboardingStartCounter = c), + assign: (c: Counter) => (googleAuthStartCounter = c), attributes: {} as Record, }, - [EVENT_ONBOARDING_END]: { - description: 'Counts onboarding end events.', + [EVENT_GOOGLE_AUTH_END]: { + description: 'Counts auth "Login with Google" succeeded.', valueType: ValueType.INT, - assign: (c: Counter) => (onboardingEndCounter = c), + assign: (c: Counter) => (googleAuthEndCounter = c), attributes: {} as Record, }, } as const; @@ -635,8 +635,8 @@ let keychainAvailabilityCounter: Counter | undefined; let tokenStorageTypeCounter: Counter | undefined; let overageOptionCounter: Counter | undefined; let creditPurchaseCounter: Counter | undefined; -let onboardingStartCounter: Counter | undefined; -let onboardingEndCounter: Counter | undefined; +let googleAuthStartCounter: Counter | undefined; +let googleAuthEndCounter: Counter | undefined; // OpenTelemetry GenAI Semantic Convention Metrics let genAiClientTokenUsageHistogram: Histogram | undefined; @@ -810,22 +810,22 @@ export function recordLinesChanged( // --- New Metric Recording Functions --- /** - * Records a metric for when the onboarding process starts. + * Records a metric for when the Google auth process starts. */ -export function recordOnboardingStart(config: Config): void { - if (!onboardingStartCounter || !isMetricsInitialized) return; - onboardingStartCounter.add( +export function recordGoogleAuthStart(config: Config): void { + if (!googleAuthStartCounter || !isMetricsInitialized) return; + googleAuthStartCounter.add( 1, baseMetricDefinition.getCommonAttributes(config), ); } /** - * Records a metric for when the onboarding process ends successfully. + * Records a metric for when the Google auth process ends successfully. */ -export function recordOnboardingEnd(config: Config): void { - if (!onboardingEndCounter || !isMetricsInitialized) return; - onboardingEndCounter.add(1, baseMetricDefinition.getCommonAttributes(config)); +export function recordGoogleAuthEnd(config: Config): void { + if (!googleAuthEndCounter || !isMetricsInitialized) return; + googleAuthEndCounter.add(1, baseMetricDefinition.getCommonAttributes(config)); } /**