From 85582a12cdd7183a8870b9eac6962c792570b312 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Wed, 29 Apr 2026 14:09:16 +0000 Subject: [PATCH 1/2] feat: add FilesystemSink for telemetry audit mode --- .../__tests__/filesystem-sink.test.ts | 95 +++++++++++++++++++ src/cli/telemetry/config.ts | 5 + src/cli/telemetry/index.ts | 3 +- src/cli/telemetry/sinks/filesystem-sink.ts | 42 ++++++++ 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 src/cli/telemetry/__tests__/filesystem-sink.test.ts create mode 100644 src/cli/telemetry/sinks/filesystem-sink.ts diff --git a/src/cli/telemetry/__tests__/filesystem-sink.test.ts b/src/cli/telemetry/__tests__/filesystem-sink.test.ts new file mode 100644 index 000000000..19a6043ca --- /dev/null +++ b/src/cli/telemetry/__tests__/filesystem-sink.test.ts @@ -0,0 +1,95 @@ +import { createTempConfig } from '../../__tests__/helpers/temp-config'; +import { resolveAuditFilePath } from '../config'; +import { FilesystemSink } from '../sinks/filesystem-sink'; +import { readFile } from 'fs/promises'; +import { join } from 'node:path'; +import { afterAll, beforeEach, describe, expect, it } from 'vitest'; + +const tmp = createTempConfig('fs-sink'); +const outputDir = join(tmp.configDir, 'telemetry'); + +function createSink(opts: { dir?: string; log?: (msg: string) => void } = {}) { + const filePath = join(opts.dir ?? outputDir, 'test-session.json'); + return new FilesystemSink({ filePath, log: opts.log }); +} + +function readJsonl(path: string): Promise { + return readFile(path, 'utf-8').then(data => + data + .trim() + .split('\n') + .map(line => JSON.parse(line)) + ); +} + +describe('FilesystemSink', () => { + beforeEach(() => tmp.setup()); + afterAll(() => tmp.cleanup()); + + it('writes each record as a JSONL line on disk', async () => { + const sink = createSink(); + sink.record(42, { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }); + await sink.flush(); + + const entries = await readJsonl(join(outputDir, 'test-session.json')); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + value: 42, + attrs: { command_group: 'deploy', command: 'deploy', exit_reason: 'success' }, + }); + }); + + it('appends multiple records as separate lines', async () => { + const sink = createSink(); + sink.record(10, { command_group: 'add', command: 'add.agent' }); + sink.record(20, { command_group: 'add', command: 'add.memory' }); + await sink.flush(); + + const entries = await readJsonl(join(outputDir, 'test-session.json')); + expect(entries).toHaveLength(2); + expect(entries[0]).toMatchObject({ value: 10 }); + expect(entries[1]).toMatchObject({ value: 20 }); + }); + + it('creates output directory if it does not exist', async () => { + const nested = join(tmp.testDir, 'deep', 'nested', 'telemetry'); + const filePath = join(nested, 'test.json'); + const sink = new FilesystemSink({ filePath }); + sink.record(1, { command_group: 'status', command: 'status' }); + await sink.flush(); + + const entries = await readJsonl(filePath); + expect(entries).toHaveLength(1); + }); + + it('flush is a no-op when no records exist', async () => { + const sink = createSink(); + await expect(sink.flush()).resolves.toBeUndefined(); + }); + + it('shutdown logs audit message when records were written', async () => { + const logged: string[] = []; + const sink = createSink({ log: msg => logged.push(msg) }); + sink.record(99, { command_group: 'invoke', command: 'invoke' }); + await sink.shutdown(); + + expect(logged).toHaveLength(1); + expect(logged[0]).toContain('[audit mode]'); + expect(logged[0]).toContain('test-session.json'); + }); + + it('shutdown does not log when no records were written', async () => { + const logged: string[] = []; + const sink = createSink({ log: msg => logged.push(msg) }); + await sink.shutdown(); + + expect(logged).toHaveLength(0); + }); +}); + +describe('resolveAuditFilePath', () => { + it('joins outputDir, entrypoint, and sessionId into a JSON file path', () => { + const path = resolveAuditFilePath('/home/user/.agentcore/telemetry', 'deploy', 'abc-123'); + expect(path).toBe('/home/user/.agentcore/telemetry/deploy-abc-123.json'); + }); +}); diff --git a/src/cli/telemetry/config.ts b/src/cli/telemetry/config.ts index 5bee94eff..364d57f68 100644 --- a/src/cli/telemetry/config.ts +++ b/src/cli/telemetry/config.ts @@ -3,6 +3,7 @@ import { getOrCreateInstallationId, readGlobalConfig } from '../global-config.js import { type ResourceAttributes, ResourceAttributesSchema } from './schemas/common-attributes.js'; import { randomUUID } from 'crypto'; import os from 'os'; +import { join } from 'path'; // --------------------------------------------------------------------------- // Telemetry preference (opt-in / opt-out) @@ -59,3 +60,7 @@ export async function resolveResourceAttributes(mode: 'cli' | 'tui'): Promise void; +} + +export class FilesystemSink implements MetricSink { + private readonly filePath: string; + private readonly log: (message: string) => void; + private hasRecords = false; + + constructor(config: FilesystemSinkConfig) { + this.filePath = config.filePath; + this.log = config.log ?? (msg => console.log(msg)); + } + + record(value: number, attrs: Record): void { + this.hasRecords = true; + this.pendingWrite = this.pendingWrite.then(() => this.appendEntry({ value, attrs })); + } + + async flush(): Promise { + await this.pendingWrite; + } + + async shutdown(): Promise { + await this.pendingWrite; + if (this.hasRecords) { + this.log(`[audit mode] Telemetry written to ${this.filePath}`); + } + } + + private pendingWrite: Promise = Promise.resolve(); + + private async appendEntry(entry: { value: number; attrs: Record }): Promise { + await mkdir(dirname(this.filePath), { recursive: true }); + await appendFile(this.filePath, JSON.stringify(entry) + '\n'); + } +} From 9db18ad0d1e908645effaff4775b5b540a93f15c Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Wed, 29 Apr 2026 14:43:26 +0000 Subject: [PATCH 2/2] feat: instrument help.modes with telemetry, add audit integ test --- integ-tests/help.test.ts | 63 ++++++++++++++++++- src/cli/cli.ts | 8 ++- src/cli/commands/help/command.tsx | 19 ++++-- .../__tests__/filesystem-sink.test.ts | 8 +-- src/cli/telemetry/client-accessor.ts | 49 +++++++++++++++ src/cli/telemetry/index.ts | 3 +- src/cli/telemetry/schemas/command-run.ts | 1 + src/cli/telemetry/sinks/filesystem-sink.ts | 14 +++-- 8 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 src/cli/telemetry/client-accessor.ts diff --git a/integ-tests/help.test.ts b/integ-tests/help.test.ts index af99b8ce4..052605c7a 100644 --- a/integ-tests/help.test.ts +++ b/integ-tests/help.test.ts @@ -1,5 +1,10 @@ +import { spawnAndCollect } from '../src/test-utils/cli-runner.js'; import { runCLI } from '../src/test-utils/index.js'; -import { describe, expect, it } from 'vitest'; +import { readdirSync } from 'node:fs'; +import { mkdir, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; const COMMANDS = [ 'create', @@ -38,3 +43,59 @@ describe('CLI help', () => { } }); }); + +describe('help modes telemetry', () => { + let testConfigDir: string; + const cliPath = join(__dirname, '..', 'dist', 'cli', 'index.mjs'); + + beforeAll(async () => { + testConfigDir = join(tmpdir(), `agentcore-help-telemetry-${Date.now()}`); + await mkdir(testConfigDir, { recursive: true }); + }); + afterAll(() => rm(testConfigDir, { recursive: true, force: true })); + + function run(args: string[], extraEnv: Record = {}) { + return spawnAndCollect('node', [cliPath, ...args], tmpdir(), { + AGENTCORE_SKIP_INSTALL: '1', + AGENTCORE_CONFIG_DIR: testConfigDir, + ...extraEnv, + }); + } + + it('writes JSONL audit file when audit is enabled via env var', async () => { + const result = await run(['help', 'modes'], { AGENTCORE_TELEMETRY_AUDIT: '1' }); + expect(result.exitCode).toBe(0); + + const telemetryDir = join(testConfigDir, 'telemetry'); + const files = readdirSync(telemetryDir).filter(f => f.startsWith('help-')); + expect(files).toHaveLength(1); + + const content = await readFile(join(telemetryDir, files[0]!), 'utf-8'); + const entry = JSON.parse(content.trim()); + expect(entry.attrs).toMatchObject({ + 'service.name': 'agentcore-cli', + 'agentcore-cli.mode': 'cli', + command_group: 'help', + command: 'help.modes', + exit_reason: 'success', + }); + expect(entry.attrs['agentcore-cli.session_id']).toBeDefined(); + expect(entry.attrs['os.type']).toBeDefined(); + expect(entry.value).toBeGreaterThanOrEqual(0); + }); + + it('does not write audit file when audit is not enabled', async () => { + const telemetryDir = join(testConfigDir, 'telemetry'); + await rm(telemetryDir, { recursive: true, force: true }); + + const result = await run(['help', 'modes']); + expect(result.exitCode).toBe(0); + + try { + const files = readdirSync(telemetryDir); + expect(files).toHaveLength(0); + } catch { + // telemetry dir doesn't exist — correct + } + }); +}); diff --git a/src/cli/cli.ts b/src/cli/cli.ts index 72cf5c50d..b8100c0d8 100644 --- a/src/cli/cli.ts +++ b/src/cli/cli.ts @@ -21,6 +21,7 @@ import { registerValidate } from './commands/validate'; import { PACKAGE_VERSION } from './constants'; import { getOrCreateInstallationId } from './global-config'; import { ALL_PRIMITIVES } from './primitives'; +import { TelemetryClientAccessor } from './telemetry'; import { App } from './tui/App'; import { LayoutProvider } from './tui/context'; import { COMMAND_DESCRIPTIONS } from './tui/copy'; @@ -222,7 +223,12 @@ export const main = async (argv: string[]) => { printTelemetryNotice(); } - await program.parseAsync(argv); + TelemetryClientAccessor.init(args[0] ?? 'unknown'); + try { + await program.parseAsync(argv); + } finally { + await TelemetryClientAccessor.shutdown(); + } // Telemetry notice already printed above; only run update check here. await printPostCommandNotices(false, updateCheck); diff --git a/src/cli/commands/help/command.tsx b/src/cli/commands/help/command.tsx index 48d771854..338684d7e 100644 --- a/src/cli/commands/help/command.tsx +++ b/src/cli/commands/help/command.tsx @@ -1,3 +1,4 @@ +import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js'; import type { Command } from '@commander-js/extra-typings'; const MODES_HELP = ` @@ -41,15 +42,23 @@ export const registerHelp = (program: Command) => { const helpCmd = program .command('help') .description('Display help topics') - .action(() => { - console.log('Available help topics: modes'); - console.log('Run `agentcore help ` for details.'); + .action(async () => { + const client = await TelemetryClientAccessor.get(); + await client.withCommandRun('help', () => { + console.log('Available help topics: modes'); + console.log('Run `agentcore help ` for details.'); + return {}; + }); }); helpCmd .command('modes') .description('Explain interactive vs non-interactive modes') - .action(() => { - console.log(MODES_HELP); + .action(async () => { + const client = await TelemetryClientAccessor.get(); + await client.withCommandRun('help.modes', () => { + console.log(MODES_HELP); + return {}; + }); }); }; diff --git a/src/cli/telemetry/__tests__/filesystem-sink.test.ts b/src/cli/telemetry/__tests__/filesystem-sink.test.ts index 19a6043ca..50d8d4620 100644 --- a/src/cli/telemetry/__tests__/filesystem-sink.test.ts +++ b/src/cli/telemetry/__tests__/filesystem-sink.test.ts @@ -1,6 +1,6 @@ import { createTempConfig } from '../../__tests__/helpers/temp-config'; import { resolveAuditFilePath } from '../config'; -import { FilesystemSink } from '../sinks/filesystem-sink'; +import { FileSystemSink } from '../sinks/filesystem-sink'; import { readFile } from 'fs/promises'; import { join } from 'node:path'; import { afterAll, beforeEach, describe, expect, it } from 'vitest'; @@ -10,7 +10,7 @@ const outputDir = join(tmp.configDir, 'telemetry'); function createSink(opts: { dir?: string; log?: (msg: string) => void } = {}) { const filePath = join(opts.dir ?? outputDir, 'test-session.json'); - return new FilesystemSink({ filePath, log: opts.log }); + return new FileSystemSink({ filePath, log: opts.log }); } function readJsonl(path: string): Promise { @@ -22,7 +22,7 @@ function readJsonl(path: string): Promise { ); } -describe('FilesystemSink', () => { +describe('FileSystemSink', () => { beforeEach(() => tmp.setup()); afterAll(() => tmp.cleanup()); @@ -54,7 +54,7 @@ describe('FilesystemSink', () => { it('creates output directory if it does not exist', async () => { const nested = join(tmp.testDir, 'deep', 'nested', 'telemetry'); const filePath = join(nested, 'test.json'); - const sink = new FilesystemSink({ filePath }); + const sink = new FileSystemSink({ filePath }); sink.record(1, { command_group: 'status', command: 'status' }); await sink.flush(); diff --git a/src/cli/telemetry/client-accessor.ts b/src/cli/telemetry/client-accessor.ts new file mode 100644 index 000000000..c41c261df --- /dev/null +++ b/src/cli/telemetry/client-accessor.ts @@ -0,0 +1,49 @@ +import { GLOBAL_CONFIG_DIR, readGlobalConfig } from '../global-config.js'; +import { TelemetryClient } from './client.js'; +import { resolveAuditFilePath, resolveResourceAttributes } from './config.js'; +import { FileSystemSink } from './sinks/filesystem-sink.js'; +import { CompositeSink } from './sinks/metric-sink.js'; +import { join } from 'path'; + +/** + * Manages a singleton TelemetryClient. Call init() at startup to configure, + * get() from command handlers to obtain the client, and shutdown() on exit. + * get() lazily initializes if init() was never called. + */ +export class TelemetryClientAccessor { + private static clientPromise: Promise | undefined; + + static init(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): void { + this.clientPromise = createClient(entrypoint, mode); + } + + static get(): Promise { + this.clientPromise ??= createClient('unknown'); + return this.clientPromise; + } + + static async shutdown(): Promise { + if (this.clientPromise) { + const client = await this.clientPromise; + await client.shutdown(); + } + } +} + +async function createClient(entrypoint: string, mode: 'cli' | 'tui' = 'cli'): Promise { + const [resource, config] = await Promise.all([resolveResourceAttributes(mode), readGlobalConfig()]); + + const sinks = []; + const audit = process.env.AGENTCORE_TELEMETRY_AUDIT === '1' || config.telemetry?.audit === true; + + if (audit) { + const filePath = resolveAuditFilePath( + join(GLOBAL_CONFIG_DIR, 'telemetry'), + entrypoint, + resource['agentcore-cli.session_id'] + ); + sinks.push(new FileSystemSink({ filePath, resource })); + } + + return new TelemetryClient(new CompositeSink(sinks)); +} diff --git a/src/cli/telemetry/index.ts b/src/cli/telemetry/index.ts index 960983477..778376713 100644 --- a/src/cli/telemetry/index.ts +++ b/src/cli/telemetry/index.ts @@ -1,7 +1,8 @@ export { resolveTelemetryPreference, resolveResourceAttributes, resolveAuditFilePath } from './config.js'; export type { TelemetryPreference } from './config.js'; +export { TelemetryClientAccessor } from './client-accessor.js'; export { TelemetryClient, CANCELLED } from './client.js'; export { type MetricSink, CompositeSink } from './sinks/metric-sink.js'; export { OtelMetricSink, type OtelMetricSinkConfig } from './sinks/otel-metric-sink.js'; -export { FilesystemSink, type FilesystemSinkConfig } from './sinks/filesystem-sink.js'; +export { FileSystemSink, type FileSystemSinkConfig } from './sinks/filesystem-sink.js'; export { classifyError, isUserError } from './error-classification.js'; diff --git a/src/cli/telemetry/schemas/command-run.ts b/src/cli/telemetry/schemas/command-run.ts index f8a6df436..dfd127a98 100644 --- a/src/cli/telemetry/schemas/command-run.ts +++ b/src/cli/telemetry/schemas/command-run.ts @@ -193,6 +193,7 @@ export const COMMAND_SCHEMAS = { package: NoAttrs, validate: NoAttrs, 'help.modes': NoAttrs, + help: NoAttrs, 'remove.agent': NoAttrs, 'remove.memory': NoAttrs, 'remove.credential': NoAttrs, diff --git a/src/cli/telemetry/sinks/filesystem-sink.ts b/src/cli/telemetry/sinks/filesystem-sink.ts index c62359660..a9868f2b9 100644 --- a/src/cli/telemetry/sinks/filesystem-sink.ts +++ b/src/cli/telemetry/sinks/filesystem-sink.ts @@ -2,24 +2,29 @@ import type { MetricSink } from './metric-sink.js'; import { appendFile, mkdir } from 'fs/promises'; import { dirname } from 'path'; -export interface FilesystemSinkConfig { +export interface FileSystemSinkConfig { filePath: string; + resource?: Record; log?: (message: string) => void; } -export class FilesystemSink implements MetricSink { +export class FileSystemSink implements MetricSink { private readonly filePath: string; + private readonly resource: Record; private readonly log: (message: string) => void; private hasRecords = false; - constructor(config: FilesystemSinkConfig) { + constructor(config: FileSystemSinkConfig) { this.filePath = config.filePath; + this.resource = config.resource ?? {}; this.log = config.log ?? (msg => console.log(msg)); } record(value: number, attrs: Record): void { this.hasRecords = true; - this.pendingWrite = this.pendingWrite.then(() => this.appendEntry({ value, attrs })); + this.pendingWrite = this.pendingWrite.then(() => + this.appendEntry({ value, attrs: { ...this.resource, ...attrs } }) + ); } async flush(): Promise { @@ -33,6 +38,7 @@ export class FilesystemSink implements MetricSink { } } + // Promise chain that serializes async writes so record() can stay synchronous. private pendingWrite: Promise = Promise.resolve(); private async appendEntry(entry: { value: number; attrs: Record }): Promise {