Fixing review comments

This commit is contained in:
Srinath Padmanabhan
2026-03-05 16:06:08 -08:00
parent 0cbec42ed8
commit 81151d12b1
3 changed files with 32 additions and 33 deletions

View File

@@ -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 () => {

View File

@@ -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<AuthClient> {
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;

View File

@@ -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<string, never>,
},
[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<string, never>,
},
} 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));
}
/**