From 2bc481a3b57414ef59703873282af2fe0e360ec5 Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 16 Apr 2026 23:01:46 +0000 Subject: [PATCH] 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. --- packages/core/src/telemetry/metrics.test.ts | 6 +- packages/core/src/telemetry/metrics.ts | 4 +- .../src/telemetry/telemetryAttributes.test.ts | 115 ++++++++++++++++++ .../core/src/telemetry/telemetryAttributes.ts | 31 +++-- 4 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/telemetry/telemetryAttributes.test.ts diff --git a/packages/core/src/telemetry/metrics.test.ts b/packages/core/src/telemetry/metrics.test.ts index 0bca699b16..4e306af406 100644 --- a/packages/core/src/telemetry/metrics.test.ts +++ b/packages/core/src/telemetry/metrics.test.ts @@ -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', diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index 377479c1e4..227f3fa858 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -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 = { diff --git a/packages/core/src/telemetry/telemetryAttributes.test.ts b/packages/core/src/telemetry/telemetryAttributes.test.ts new file mode 100644 index 0000000000..075d55e659 --- /dev/null +++ b/packages/core/src/telemetry/telemetryAttributes.test.ts @@ -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; + + 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); + }); + }); +}); diff --git a/packages/core/src/telemetry/telemetryAttributes.ts b/packages/core/src/telemetry/telemetryAttributes.ts index d2139e32df..0632fc453c 100644 --- a/packages/core/src/telemetry/telemetryAttributes.ts +++ b/packages/core/src/telemetry/telemetryAttributes.ts @@ -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 }), }; }