mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-15 22:33:05 -07:00
fix(telemetry): strip high-cardinality attributes from metrics
Decouples generic OpenTelemetry attributes from metric-specific attributes to resolve a Cloud Monitoring 'Internal error encountered' caused by a cardinality explosion. High-cardinality values like session.id, installation.id, user.email, and experiments.ids are now excluded from time series metric labels, but they are still preserved on traces and logs. Also refactors getCommonAttributes to use getCommonMetricAttributes to reduce repetition, and adds robust unit tests covering the truncation of the experiments.ids array and fallback cases.
This commit is contained in:
@@ -121,8 +121,10 @@ describe('Telemetry Metrics', () => {
|
||||
return actualApi;
|
||||
});
|
||||
|
||||
const { getCommonAttributes } = await import('./telemetryAttributes.js');
|
||||
(getCommonAttributes as Mock).mockReturnValue({
|
||||
const { getCommonMetricAttributes } = await import(
|
||||
'./telemetryAttributes.js'
|
||||
);
|
||||
(getCommonMetricAttributes as Mock).mockReturnValue({
|
||||
'session.id': 'test-session-id',
|
||||
'installation.id': 'test-installation-id',
|
||||
'user.email': 'test@example.com',
|
||||
|
||||
@@ -24,7 +24,7 @@ import type {
|
||||
TokenStorageInitializationEvent,
|
||||
} from './types.js';
|
||||
import { AuthType } from '../core/contentGenerator.js';
|
||||
import { getCommonAttributes } from './telemetryAttributes.js';
|
||||
import { getCommonMetricAttributes } from './telemetryAttributes.js';
|
||||
import { sanitizeHookName } from './sanitize.js';
|
||||
|
||||
const EVENT_CHAT_COMPRESSION = 'gemini_cli.chat_compression';
|
||||
@@ -104,7 +104,7 @@ const EXIT_FAIL_COUNT = 'gemini_cli.exit.fail.count';
|
||||
const PLAN_EXECUTION_COUNT = 'gemini_cli.plan.execution.count';
|
||||
|
||||
const baseMetricDefinition = {
|
||||
getCommonAttributes,
|
||||
getCommonAttributes: getCommonMetricAttributes,
|
||||
};
|
||||
|
||||
const COUNTER_DEFINITIONS = {
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
|
||||
import {
|
||||
getCommonAttributes,
|
||||
getCommonMetricAttributes,
|
||||
} from './telemetryAttributes.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { UserAccountManager } from '../utils/userAccountManager.js';
|
||||
import { InstallationManager } from '../utils/installationManager.js';
|
||||
|
||||
vi.mock('../utils/userAccountManager.js');
|
||||
vi.mock('../utils/installationManager.js');
|
||||
|
||||
describe('telemetryAttributes', () => {
|
||||
let mockConfig: Partial<Config>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
|
||||
mockConfig = {
|
||||
getSessionId: vi.fn().mockReturnValue('mock-session-id'),
|
||||
isInteractive: vi.fn().mockReturnValue(true),
|
||||
getExperiments: vi.fn().mockReturnValue(undefined),
|
||||
getContentGeneratorConfig: vi.fn().mockReturnValue(undefined),
|
||||
};
|
||||
|
||||
(
|
||||
UserAccountManager.prototype.getCachedGoogleAccount as Mock
|
||||
).mockReturnValue(undefined);
|
||||
(InstallationManager.prototype.getInstallationId as Mock).mockReturnValue(
|
||||
'mock-install-id',
|
||||
);
|
||||
});
|
||||
|
||||
describe('getCommonMetricAttributes', () => {
|
||||
it('should return interactive and auth_type when defined', () => {
|
||||
mockConfig.getContentGeneratorConfig = vi
|
||||
.fn()
|
||||
.mockReturnValue({ authType: 'oauth-personal' });
|
||||
|
||||
const attributes = getCommonMetricAttributes(mockConfig as Config);
|
||||
|
||||
expect(attributes).toEqual({
|
||||
interactive: true,
|
||||
auth_type: 'oauth-personal',
|
||||
});
|
||||
});
|
||||
|
||||
it('should return only interactive when auth_type is not defined', () => {
|
||||
const attributes = getCommonMetricAttributes(mockConfig as Config);
|
||||
|
||||
expect(attributes).toEqual({
|
||||
interactive: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCommonAttributes', () => {
|
||||
it('should include all common attributes', () => {
|
||||
(
|
||||
UserAccountManager.prototype.getCachedGoogleAccount as Mock
|
||||
).mockReturnValue('test@google.com');
|
||||
mockConfig.getExperiments = vi
|
||||
.fn()
|
||||
.mockReturnValue({ experimentIds: [123, 456] });
|
||||
mockConfig.getContentGeneratorConfig = vi
|
||||
.fn()
|
||||
.mockReturnValue({ authType: 'adc' });
|
||||
|
||||
const attributes = getCommonAttributes(mockConfig as Config);
|
||||
|
||||
expect(attributes).toEqual({
|
||||
'session.id': 'mock-session-id',
|
||||
'installation.id': 'mock-install-id',
|
||||
interactive: true,
|
||||
'user.email': 'test@google.com',
|
||||
auth_type: 'adc',
|
||||
'experiments.ids': '123,456',
|
||||
});
|
||||
});
|
||||
|
||||
it('should safely truncate experiments string to not exceed 1000 characters and not cut mid-ID', () => {
|
||||
// Generate a list of experiment IDs that will produce a string > 1000 chars
|
||||
const expIds = [];
|
||||
for (let i = 0; i < 200; i++) {
|
||||
// e.g., 100000000 -> 9 chars + 1 comma = 10 chars per ID
|
||||
expIds.push(100000000 + i);
|
||||
}
|
||||
mockConfig.getExperiments = vi
|
||||
.fn()
|
||||
.mockReturnValue({ experimentIds: expIds });
|
||||
|
||||
const attributes = getCommonAttributes(mockConfig as Config);
|
||||
const expString = attributes['experiments.ids'] as string;
|
||||
|
||||
expect(expString.length).toBeLessThanOrEqual(1000);
|
||||
|
||||
// Verify the last ID is complete (not cut off) by checking if it's one of our expected IDs
|
||||
const ids = expString.split(',');
|
||||
const lastIdStr = ids[ids.length - 1];
|
||||
const lastIdNumber = parseInt(lastIdStr, 10);
|
||||
|
||||
expect(lastIdNumber).toBeGreaterThanOrEqual(100000000);
|
||||
expect(lastIdNumber).toBeLessThan(100000200);
|
||||
|
||||
// Also ensure no trailing comma
|
||||
expect(expString.endsWith(',')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -12,19 +12,36 @@ import { UserAccountManager } from '../utils/userAccountManager.js';
|
||||
const userAccountManager = new UserAccountManager();
|
||||
const installationManager = new InstallationManager();
|
||||
|
||||
export function getCommonMetricAttributes(config: Config): Attributes {
|
||||
const authType = config.getContentGeneratorConfig()?.authType;
|
||||
|
||||
return {
|
||||
interactive: config.isInteractive(),
|
||||
...(authType && { auth_type: authType }),
|
||||
};
|
||||
}
|
||||
|
||||
export function getCommonAttributes(config: Config): Attributes {
|
||||
const email = userAccountManager.getCachedGoogleAccount();
|
||||
const experiments = config.getExperiments();
|
||||
const authType = config.getContentGeneratorConfig()?.authType;
|
||||
|
||||
let experimentsIdsStr = '';
|
||||
if (experiments && experiments.experimentIds.length > 0) {
|
||||
experimentsIdsStr = experiments.experimentIds.join(',');
|
||||
if (experimentsIdsStr.length > 1000) {
|
||||
experimentsIdsStr = experimentsIdsStr.substring(0, 1000);
|
||||
const lastCommaIndex = experimentsIdsStr.lastIndexOf(',');
|
||||
if (lastCommaIndex > 0) {
|
||||
experimentsIdsStr = experimentsIdsStr.substring(0, lastCommaIndex);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
...getCommonMetricAttributes(config),
|
||||
'session.id': config.getSessionId(),
|
||||
'installation.id': installationManager.getInstallationId(),
|
||||
interactive: config.isInteractive(),
|
||||
...(email && { 'user.email': email }),
|
||||
...(authType && { auth_type: authType }),
|
||||
...(experiments &&
|
||||
experiments.experimentIds.length > 0 && {
|
||||
'experiments.ids': experiments.experimentIds,
|
||||
}),
|
||||
...(experimentsIdsStr && { 'experiments.ids': experimentsIdsStr }),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user