Fixing review comments

This commit is contained in:
Srinath Padmanabhan
2026-03-05 16:06:08 -08:00
parent 79dd2bb535
commit 192b1a630f
3 changed files with 32 additions and 33 deletions
+6 -6
View File
@@ -27,8 +27,8 @@ import {
authEvents, authEvents,
} from './oauth2.js'; } from './oauth2.js';
import { import {
recordOnboardingStart, recordGoogleAuthStart,
recordOnboardingEnd, recordGoogleAuthEnd,
} from '../telemetry/metrics.js'; } from '../telemetry/metrics.js';
import { UserAccountManager } from '../utils/userAccountManager.js'; import { UserAccountManager } from '../utils/userAccountManager.js';
import * as fs from 'node:fs'; import * as fs from 'node:fs';
@@ -110,8 +110,8 @@ vi.mock('../mcp/token-storage/hybrid-token-storage.js', () => ({
})); }));
vi.mock('../telemetry/metrics.js', () => ({ vi.mock('../telemetry/metrics.js', () => ({
recordOnboardingStart: vi.fn(), recordGoogleAuthStart: vi.fn(),
recordOnboardingEnd: vi.fn(), recordGoogleAuthEnd: vi.fn(),
})); }));
const mockConfig = { const mockConfig = {
@@ -1415,8 +1415,8 @@ describe('oauth2', () => {
await getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfig); await getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfig);
expect(recordOnboardingStart).toHaveBeenCalledWith(mockConfig); expect(recordGoogleAuthStart).toHaveBeenCalledWith(mockConfig);
expect(recordOnboardingEnd).toHaveBeenCalledWith(mockConfig); expect(recordGoogleAuthEnd).toHaveBeenCalledWith(mockConfig);
}); });
it('should NOT record onboarding events for other auth types', async () => { it('should NOT record onboarding events for other auth types', async () => {
+8 -9
View File
@@ -22,8 +22,8 @@ import open from 'open';
import path from 'node:path'; import path from 'node:path';
import { promises as fs } from 'node:fs'; import { promises as fs } from 'node:fs';
import { import {
recordOnboardingStart, recordGoogleAuthStart,
recordOnboardingEnd, recordGoogleAuthEnd,
} from '../telemetry/metrics.js'; } from '../telemetry/metrics.js';
import type { Config } from '../config/config.js'; import type { Config } from '../config/config.js';
import { import {
@@ -117,9 +117,9 @@ async function initOauthClient(
authType: AuthType, authType: AuthType,
config: Config, config: Config,
): Promise<AuthClient> { ): Promise<AuthClient> {
const recordOnboardingEndIfApplicable = () => { const recordGoogleAuthEndIfApplicable = () => {
if (authType === AuthType.LOGIN_WITH_GOOGLE) { if (authType === AuthType.LOGIN_WITH_GOOGLE) {
recordOnboardingEnd(config); recordGoogleAuthEnd(config);
} }
}; };
@@ -154,7 +154,7 @@ async function initOauthClient(
}); });
if (authType === AuthType.LOGIN_WITH_GOOGLE) { if (authType === AuthType.LOGIN_WITH_GOOGLE) {
recordOnboardingStart(config); recordGoogleAuthStart(config);
} }
const useEncryptedStorage = getUseEncryptedStorageFlag(); const useEncryptedStorage = getUseEncryptedStorageFlag();
@@ -167,7 +167,6 @@ async function initOauthClient(
access_token: process.env['GOOGLE_CLOUD_ACCESS_TOKEN'], access_token: process.env['GOOGLE_CLOUD_ACCESS_TOKEN'],
}); });
await fetchAndCacheUserInfo(client); await fetchAndCacheUserInfo(client);
recordOnboardingEndIfApplicable();
return client; return client;
} }
@@ -204,7 +203,7 @@ async function initOauthClient(
debugLogger.log('Loaded cached credentials.'); debugLogger.log('Loaded cached credentials.');
await triggerPostAuthCallbacks(credentials as Credentials); await triggerPostAuthCallbacks(credentials as Credentials);
recordOnboardingEndIfApplicable(); recordGoogleAuthEndIfApplicable();
return client; return client;
} }
} catch (error) { } catch (error) {
@@ -296,7 +295,7 @@ async function initOauthClient(
} }
await triggerPostAuthCallbacks(client.credentials); await triggerPostAuthCallbacks(client.credentials);
recordOnboardingEndIfApplicable(); recordGoogleAuthEndIfApplicable();
} else { } else {
// In ACP mode, we skip the interactive consent and directly open the browser // In ACP mode, we skip the interactive consent and directly open the browser
if (!config.getAcpMode()) { if (!config.getAcpMode()) {
@@ -403,7 +402,7 @@ async function initOauthClient(
}); });
await triggerPostAuthCallbacks(client.credentials); await triggerPostAuthCallbacks(client.credentials);
recordOnboardingEndIfApplicable(); recordGoogleAuthEndIfApplicable();
} }
return client; return client;
+18 -18
View File
@@ -50,8 +50,8 @@ const KEYCHAIN_AVAILABILITY_COUNT = 'gemini_cli.keychain.availability.count';
const TOKEN_STORAGE_TYPE_COUNT = 'gemini_cli.token_storage.type.count'; const TOKEN_STORAGE_TYPE_COUNT = 'gemini_cli.token_storage.type.count';
const OVERAGE_OPTION_COUNT = 'gemini_cli.overage_option.count'; const OVERAGE_OPTION_COUNT = 'gemini_cli.overage_option.count';
const CREDIT_PURCHASE_COUNT = 'gemini_cli.credit_purchase.count'; const CREDIT_PURCHASE_COUNT = 'gemini_cli.credit_purchase.count';
const EVENT_ONBOARDING_START = 'gemini_cli.onboarding.start'; const EVENT_GOOGLE_AUTH_START = 'gemini_cli.google_auth.start';
const EVENT_ONBOARDING_END = 'gemini_cli.onboarding.end'; const EVENT_GOOGLE_AUTH_END = 'gemini_cli.google_auth.end';
// Agent Metrics // Agent Metrics
const AGENT_RUN_COUNT = 'gemini_cli.agent.run.count'; const AGENT_RUN_COUNT = 'gemini_cli.agent.run.count';
@@ -290,16 +290,16 @@ const COUNTER_DEFINITIONS = {
model: string; model: string;
}, },
}, },
[EVENT_ONBOARDING_START]: { [EVENT_GOOGLE_AUTH_START]: {
description: 'Counts onboarding start events.', description: 'Counts auth "Login with Google" started.',
valueType: ValueType.INT, valueType: ValueType.INT,
assign: (c: Counter) => (onboardingStartCounter = c), assign: (c: Counter) => (googleAuthStartCounter = c),
attributes: {} as Record<string, never>, attributes: {} as Record<string, never>,
}, },
[EVENT_ONBOARDING_END]: { [EVENT_GOOGLE_AUTH_END]: {
description: 'Counts onboarding end events.', description: 'Counts auth "Login with Google" succeeded.',
valueType: ValueType.INT, valueType: ValueType.INT,
assign: (c: Counter) => (onboardingEndCounter = c), assign: (c: Counter) => (googleAuthEndCounter = c),
attributes: {} as Record<string, never>, attributes: {} as Record<string, never>,
}, },
} as const; } as const;
@@ -642,8 +642,8 @@ let keychainAvailabilityCounter: Counter | undefined;
let tokenStorageTypeCounter: Counter | undefined; let tokenStorageTypeCounter: Counter | undefined;
let overageOptionCounter: Counter | undefined; let overageOptionCounter: Counter | undefined;
let creditPurchaseCounter: Counter | undefined; let creditPurchaseCounter: Counter | undefined;
let onboardingStartCounter: Counter | undefined; let googleAuthStartCounter: Counter | undefined;
let onboardingEndCounter: Counter | undefined; let googleAuthEndCounter: Counter | undefined;
// OpenTelemetry GenAI Semantic Convention Metrics // OpenTelemetry GenAI Semantic Convention Metrics
let genAiClientTokenUsageHistogram: Histogram | undefined; let genAiClientTokenUsageHistogram: Histogram | undefined;
@@ -817,22 +817,22 @@ export function recordLinesChanged(
// --- New Metric Recording Functions --- // --- 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 { export function recordGoogleAuthStart(config: Config): void {
if (!onboardingStartCounter || !isMetricsInitialized) return; if (!googleAuthStartCounter || !isMetricsInitialized) return;
onboardingStartCounter.add( googleAuthStartCounter.add(
1, 1,
baseMetricDefinition.getCommonAttributes(config), 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 { export function recordGoogleAuthEnd(config: Config): void {
if (!onboardingEndCounter || !isMetricsInitialized) return; if (!googleAuthEndCounter || !isMetricsInitialized) return;
onboardingEndCounter.add(1, baseMetricDefinition.getCommonAttributes(config)); googleAuthEndCounter.add(1, baseMetricDefinition.getCommonAttributes(config));
} }
/** /**