diff --git a/src/cli/commands/telemetry/actions.ts b/src/cli/commands/telemetry/actions.ts index 696608e01..c83a6f425 100644 --- a/src/cli/commands/telemetry/actions.ts +++ b/src/cli/commands/telemetry/actions.ts @@ -1,4 +1,9 @@ -import { GLOBAL_CONFIG_DIR, GLOBAL_CONFIG_FILE, updateGlobalConfig } from '../../../lib/schemas/io/global-config.js'; +import { + GLOBAL_CONFIG_DIR, + GLOBAL_CONFIG_FILE, + readGlobalConfig, + updateGlobalConfig, +} from '../../../lib/schemas/io/global-config.js'; import { resolveTelemetryPreference } from '../../telemetry/config.js'; export async function handleTelemetryDisable( @@ -20,7 +25,8 @@ export async function handleTelemetryEnable( } export async function handleTelemetryStatus(configFile = GLOBAL_CONFIG_FILE): Promise { - const pref = await resolveTelemetryPreference(configFile); + const globalConfig = await readGlobalConfig(configFile); + const pref = await resolveTelemetryPreference(globalConfig); const status = pref.enabled ? 'Enabled' : 'Disabled'; const sourceLabel = diff --git a/src/cli/telemetry/__tests__/resolve.test.ts b/src/cli/telemetry/__tests__/resolve.test.ts index bdb326f42..458b2a023 100644 --- a/src/cli/telemetry/__tests__/resolve.test.ts +++ b/src/cli/telemetry/__tests__/resolve.test.ts @@ -1,32 +1,29 @@ -import { createTempConfig } from '../../__tests__/helpers/temp-config'; -import { resolveTelemetryPreference } from '../config'; -import { writeFile } from 'fs/promises'; -import { join } from 'node:path'; -import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'; - -const tmp = createTempConfig('resolve'); +import { + resolveAuditEnabled, + resolveTelemetryEndpoint, + resolveTelemetryPreference, + validateEndpointUrl, +} from '../config'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; describe('resolveTelemetryPreference', () => { const originalEnv = process.env; - beforeEach(async () => { + beforeEach(() => { process.env = { ...originalEnv }; delete process.env.AGENTCORE_TELEMETRY_DISABLED; - await tmp.setup(); }); afterEach(() => { process.env = originalEnv; }); - afterAll(() => tmp.cleanup()); - describe('AGENTCORE_TELEMETRY_DISABLED env var', () => { it('disables telemetry for any non-false/non-0 value', async () => { for (const val of ['true', 'TRUE', '1', 'yes']) { process.env.AGENTCORE_TELEMETRY_DISABLED = val; - const result = await resolveTelemetryPreference(tmp.configFile); + const result = await resolveTelemetryPreference(); expect(result).toMatchObject({ enabled: false, source: 'environment' }); expect(result.envVar).toEqual({ name: 'AGENTCORE_TELEMETRY_DISABLED', value: val }); @@ -37,7 +34,7 @@ describe('resolveTelemetryPreference', () => { for (const val of ['false', '0']) { process.env.AGENTCORE_TELEMETRY_DISABLED = val; - const result = await resolveTelemetryPreference(tmp.configFile); + const result = await resolveTelemetryPreference(); expect(result).toMatchObject({ enabled: true, source: 'environment' }); expect(result.envVar).toEqual({ name: 'AGENTCORE_TELEMETRY_DISABLED', value: val }); @@ -46,18 +43,15 @@ describe('resolveTelemetryPreference', () => { }); describe('global config', () => { - it('uses config file when no env vars set', async () => { - await writeFile(tmp.configFile, JSON.stringify({ telemetry: { enabled: false } })); - - const result = await resolveTelemetryPreference(tmp.configFile); + it('uses config when telemetry.enabled is false', async () => { + const result = await resolveTelemetryPreference({ telemetry: { enabled: false } }); expect(result).toEqual({ enabled: false, source: 'global-config' }); }); it('ignores non-boolean enabled values in config', async () => { - await writeFile(tmp.configFile, JSON.stringify({ telemetry: { enabled: 'false' } })); - - const result = await resolveTelemetryPreference(tmp.configFile); + // @ts-expect-error — intentionally invalid + const result = await resolveTelemetryPreference({ telemetry: { enabled: 'false' } }); expect(result).toEqual({ enabled: true, source: 'default' }); }); @@ -65,9 +59,106 @@ describe('resolveTelemetryPreference', () => { describe('default', () => { it('defaults to enabled when no env vars or config', async () => { - const result = await resolveTelemetryPreference(join(tmp.testDir, 'nonexistent.json')); + const result = await resolveTelemetryPreference({}); expect(result).toEqual({ enabled: true, source: 'default' }); }); }); }); + +describe('validateEndpointUrl', () => { + it('returns success with normalized URL for valid https endpoint', () => { + const result = validateEndpointUrl('https://telemetry.example.com/v1/'); + expect(result).toEqual({ success: true, url: 'https://telemetry.example.com/v1' }); + }); + + it('returns success for http endpoint', () => { + const result = validateEndpointUrl('http://localhost:4318'); + expect(result).toEqual({ success: true, url: 'http://localhost:4318' }); + }); + + it('strips trailing slashes', () => { + const result = validateEndpointUrl('https://example.com/'); + expect(result).toEqual({ success: true, url: 'https://example.com' }); + }); + + it('returns failure for non-http protocol', () => { + const result = validateEndpointUrl('file:///etc/passwd'); + expect(result.success).toBe(false); + expect(!result.success && result.error.message).toContain('Unsupported protocol'); + }); + + it('returns failure for malformed URL', () => { + const result = validateEndpointUrl('not-a-url'); + expect(result.success).toBe(false); + expect(!result.success && result.error.message).toContain('Invalid URL'); + }); +}); + +describe('resolveTelemetryEndpoint', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + delete process.env.AGENTCORE_TELEMETRY_ENDPOINT; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('returns endpoint from env var', async () => { + process.env.AGENTCORE_TELEMETRY_ENDPOINT = 'https://env.example.com'; + + const result = await resolveTelemetryEndpoint({}); + + expect(result).toEqual({ success: true, url: 'https://env.example.com' }); + }); + + it('falls back to config endpoint', async () => { + const result = await resolveTelemetryEndpoint({ telemetry: { endpoint: 'https://config.example.com' } }); + + expect(result).toEqual({ success: true, url: 'https://config.example.com' }); + }); + + it('returns failure when no endpoint configured', async () => { + const result = await resolveTelemetryEndpoint({}); + + expect(result.success).toBe(false); + }); + + it('returns failure for invalid env endpoint', async () => { + process.env.AGENTCORE_TELEMETRY_ENDPOINT = 'not-a-url'; + + const result = await resolveTelemetryEndpoint({}); + + expect(result.success).toBe(false); + }); +}); + +describe('resolveAuditEnabled', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + delete process.env.AGENTCORE_TELEMETRY_AUDIT; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('returns true when env var is "1"', async () => { + process.env.AGENTCORE_TELEMETRY_AUDIT = '1'; + + expect(await resolveAuditEnabled({})).toBe(true); + }); + + it('returns true when config audit is true', async () => { + expect(await resolveAuditEnabled({ telemetry: { audit: true } })).toBe(true); + }); + + it('returns false when neither env nor config enables audit', async () => { + expect(await resolveAuditEnabled({})).toBe(false); + }); +}); diff --git a/src/cli/telemetry/client-accessor.ts b/src/cli/telemetry/client-accessor.ts index 04172dae5..53a7ddb46 100644 --- a/src/cli/telemetry/client-accessor.ts +++ b/src/cli/telemetry/client-accessor.ts @@ -1,8 +1,15 @@ import { GLOBAL_CONFIG_DIR, readGlobalConfig } from '../../lib/schemas/io/global-config.js'; import { TelemetryClient } from './client.js'; -import { resolveAuditFilePath, resolveResourceAttributes } from './config.js'; +import { + resolveAuditEnabled, + resolveAuditFilePath, + resolveResourceAttributes, + resolveTelemetryEndpoint, + resolveTelemetryPreference, +} from './config.js'; import { FileSystemSink } from './sinks/filesystem-sink.js'; import { CompositeSink } from './sinks/metric-sink.js'; +import { OtelMetricSink } from './sinks/otel-metric-sink.js'; import { join } from 'path'; /** @@ -24,8 +31,12 @@ export class TelemetryClientAccessor { static async shutdown(): Promise { if (this.clientPromise) { - const client = await this.clientPromise; - await client.shutdown(); + try { + const client = await this.clientPromise; + await client.shutdown(); + } catch { + // Telemetry is best-effort — don't propagate init or shutdown failures + } } } } @@ -33,8 +44,13 @@ export class TelemetryClientAccessor { async function createClient(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): Promise { const [resource, config] = await Promise.all([resolveResourceAttributes(mode), readGlobalConfig()]); + const [{ enabled }, endpointResult, audit] = await Promise.all([ + resolveTelemetryPreference(config), + resolveTelemetryEndpoint(config), + resolveAuditEnabled(config), + ]); + const sinks = []; - const audit = process.env.AGENTCORE_TELEMETRY_AUDIT === '1' || config.telemetry?.audit === true; if (audit) { const filePath = resolveAuditFilePath( @@ -45,5 +61,9 @@ async function createClient(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): Pr sinks.push(new FileSystemSink({ filePath, resource })); } + if (endpointResult.success && enabled) { + sinks.push(new OtelMetricSink({ endpoint: endpointResult.url, resource })); + } + return new TelemetryClient(new CompositeSink(sinks)); } diff --git a/src/cli/telemetry/config.ts b/src/cli/telemetry/config.ts index fbaa3fb13..d14133484 100644 --- a/src/cli/telemetry/config.ts +++ b/src/cli/telemetry/config.ts @@ -1,4 +1,5 @@ -import { getOrCreateInstallationId, readGlobalConfig } from '../../lib/schemas/io/global-config.js'; +import type { Result } from '../../lib/result.js'; +import { type GlobalConfig, getOrCreateInstallationId, readGlobalConfig } from '../../lib/schemas/io/global-config.js'; import { PACKAGE_VERSION } from '../constants.js'; import { type ResourceAttributes, ResourceAttributesSchema } from './schemas/common-attributes.js'; import { randomUUID } from 'crypto'; @@ -17,7 +18,7 @@ export interface TelemetryPreference { const ENV_VAR_NAME = 'AGENTCORE_TELEMETRY_DISABLED'; -export async function resolveTelemetryPreference(configFile?: string): Promise { +export async function resolveTelemetryPreference(config?: GlobalConfig): Promise { const agentcoreEnv = process.env[ENV_VAR_NAME]; if (agentcoreEnv !== undefined) { const normalized = agentcoreEnv.toLowerCase().trim(); @@ -29,9 +30,9 @@ export async function resolveTelemetryPreference(configFile?: string): Promise { + if (process.env.AGENTCORE_TELEMETRY_AUDIT === '1') return true; + const resolved = config ?? (await readGlobalConfig()); + return resolved.telemetry?.audit === true; +} + +/** + * Validate that a string is a well-formed HTTP(S) URL suitable for an OTLP endpoint. + * Returns the normalized URL (trailing slashes stripped) on success. + */ +export function validateEndpointUrl(endpoint: string): Result<{ url: string }> { + try { + const parsed = new URL(endpoint); + if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { + return { success: false, error: new Error(`Unsupported protocol: ${parsed.protocol}`) }; + } + return { success: true, url: parsed.origin + parsed.pathname.replace(/\/+$/, '') }; + } catch { + return { success: false, error: new Error(`Invalid URL: ${endpoint}`) }; + } +} + +/** + * Resolve the telemetry endpoint from env var or global config. + * Returns a failure Result if no endpoint is configured or the value is invalid. + */ +export async function resolveTelemetryEndpoint(config?: GlobalConfig): Promise> { + const envEndpoint = process.env.AGENTCORE_TELEMETRY_ENDPOINT; + if (envEndpoint) { + return validateEndpointUrl(envEndpoint); + } + const resolved = config ?? (await readGlobalConfig()); + const configEndpoint = resolved.telemetry?.endpoint; + if (configEndpoint) { + return validateEndpointUrl(configEndpoint); + } + return { success: false, error: new Error('No telemetry endpoint found.') }; +} diff --git a/src/cli/telemetry/sinks/otel-metric-sink.ts b/src/cli/telemetry/sinks/otel-metric-sink.ts index 0bc6721f5..734220e8c 100644 --- a/src/cli/telemetry/sinks/otel-metric-sink.ts +++ b/src/cli/telemetry/sinks/otel-metric-sink.ts @@ -17,11 +17,13 @@ export class OtelMetricSink implements MetricSink { constructor(config: OtelMetricSinkConfig) { const resource = resourceFromAttributes(config.resource); + const url = config.endpoint.endsWith('/v1/metrics') ? config.endpoint : `${config.endpoint}/v1/metrics`; const exporter = new OTLPMetricExporter({ - url: `${config.endpoint}/v1/metrics`, + url, headers: { 'X-Installation-Id': config.resource['agentcore-cli.installation_id'] }, temporalityPreference: AggregationTemporality.DELTA, }); + this.meterProvider = new MeterProvider({ resource, readers: [