refactor(telemetry): merge duplicate session-start and goal events (#885)
Some checks are pending
CI / build (push) Waiting to run
CI / test (push) Waiting to run
CI / lint (push) Waiting to run
CI / typecheck (push) Waiting to run
Nix Build / Check flake.nix workspace sync (push) Waiting to run
Nix Build / nix build .#kimi-code (push) Blocked by required conditions
Release / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions

* refactor(telemetry): merge duplicate session-start and goal events

* test: align telemetry tests with merged session-start events

- run-shell: assert sessionStartedProperties plumbing instead of the removed 'started' event, drop the now-redundant resumed-lifecycle test, and fix the startup_perf call-count assertion

- node-sdk: cover process-level and session-level sessionStartedProperties merging on session_started

* fix(telemetry): keep session_started canonical fields authoritative

Caller-supplied sessionStartedProperties were merged after the canonical fields (client_name, client_version, ui_mode, resumed), so a caller could silently override them via the public SDK options. Reorder so the harness-owned canonical fields always win, while session-level properties still override process-level ones for non-canonical keys.
This commit is contained in:
7Sageer 2026-06-18 17:38:02 +08:00 committed by GitHub
parent 8ab9e96963
commit 42d648655a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 161 additions and 108 deletions

View file

@ -83,6 +83,7 @@ export async function runPrompt(
}
track('oauth_refresh', { success: false, reason: outcome.reason });
},
sessionStartedProperties: { yolo: false, plan: false, afk: true },
});
log.info('kimi-code starting', {
version,
@ -115,7 +116,7 @@ export async function runPrompt(
for (const warning of (await harness.getConfigDiagnostics()).warnings) {
stderr.write(`Warning: ${warning}\n`);
}
const { session, resumed, restorePermission, telemetryModel, goalModel } =
const { session, restorePermission, telemetryModel, goalModel } =
await resolvePromptSession(
harness,
opts,
@ -138,13 +139,6 @@ export async function runPrompt(
});
setCrashPhase('runtime');
withTelemetryContext({ sessionId: session.id }).track('started', {
resumed,
yolo: false,
plan: false,
afk: true,
});
const outputFormat = opts.outputFormat ?? 'text';
// Headless goal mode: `kimi -p "/goal <objective>"`. The goal driver keeps
// the turn-run alive across continuation turns, so the normal prompt-turn

View file

@ -72,6 +72,7 @@ export async function runShell(
reason: outcome.reason,
});
},
sessionStartedProperties: { yolo: opts.yolo, auto: opts.auto, plan: opts.plan, afk: false },
});
log.info('kimi-code starting', {
version,
@ -116,7 +117,6 @@ export async function runShell(
});
setCrashPhase('runtime');
const resumed = opts.continue || opts.session !== undefined;
const trackLifecycleForSession = (
sessionId: string,
event: string,
@ -161,13 +161,6 @@ export async function runShell(
const initStartedAt = Date.now();
await tui.start();
const initMs = Date.now() - initStartedAt;
trackLifecycle('started', {
resumed,
yolo: opts.yolo,
auto: opts.auto,
plan: opts.plan,
afk: false,
});
const startupSessionId = tui.getCurrentSessionId();
const mcpMs = await tui.getStartupMcpMs();
trackLifecycleForSession(startupSessionId, 'startup_perf', {

View file

@ -412,7 +412,6 @@ async function startGoal(
if (options.beforeSend !== undefined && !(await options.beforeSend())) {
return false;
}
host.track('goal_create', { replace: parsed.replace });
host.state.transcriptContainer.addChild(new GoalSetMessageComponent());
host.state.ui.requestRender();
if (options.sendInput !== undefined) {

View file

@ -191,6 +191,7 @@ describe('runShell', () => {
userAgentProduct: 'kimi-code-cli',
version: '1.2.3-test',
}),
sessionStartedProperties: { yolo: true, auto: false, plan: true, afk: false },
}),
);
expect(mocks.harnessEnsureConfigFile).toHaveBeenCalledOnce();
@ -228,15 +229,7 @@ describe('runShell', () => {
workDir: process.cwd(),
});
expect(mocks.tuiStart).toHaveBeenCalledOnce();
expect(mocks.harnessTrack).not.toHaveBeenCalledWith('started', expect.anything());
expect(mocks.withTelemetryContext).toHaveBeenCalledWith({ sessionId: 'ses-startup' });
expect(mocks.lifecycleTrack).toHaveBeenCalledWith('started', {
resumed: false,
yolo: true,
auto: false,
plan: true,
afk: false,
});
expect(mocks.lifecycleTrack).toHaveBeenCalledWith('startup_perf', {
duration_ms: expect.any(Number),
config_ms: expect.any(Number),
@ -327,39 +320,6 @@ describe('runShell', () => {
expect(mocks.harnessTrack).toHaveBeenCalledWith('first_launch');
});
it('marks resumed lifecycle starts from session flags', async () => {
mocks.loadTuiConfig.mockResolvedValue({
theme: 'dark',
editorCommand: null,
notifications: { enabled: true, condition: 'unfocused' },
});
mocks.tuiStart.mockResolvedValue(undefined);
mocks.tuiGetCurrentSessionId.mockReturnValue('ses-1');
await runShell(
{
session: 'ses-1',
continue: false,
yolo: false,
auto: false,
plan: false,
model: undefined,
outputFormat: undefined,
prompt: undefined,
skillsDirs: [],
},
'1.2.3-test',
);
expect(mocks.lifecycleTrack).toHaveBeenCalledWith('started', {
resumed: true,
yolo: false,
auto: false,
plan: false,
afk: false,
});
});
it('binds startup_perf to the session captured before MCP metrics resolve', async () => {
mocks.loadTuiConfig.mockResolvedValue({
theme: 'dark',
@ -389,9 +349,9 @@ describe('runShell', () => {
'1.2.3-test',
);
expect(mocks.withTelemetryContext).toHaveBeenNthCalledWith(1, { sessionId: 'ses-startup' });
expect(mocks.withTelemetryContext).toHaveBeenNthCalledWith(2, { sessionId: 'ses-startup' });
expect(mocks.lifecycleTrack).toHaveBeenNthCalledWith(2, 'startup_perf', {
expect(mocks.withTelemetryContext).toHaveBeenCalledWith({ sessionId: 'ses-startup' });
expect(mocks.withTelemetryContext).not.toHaveBeenCalledWith({ sessionId: 'ses-later' });
expect(mocks.lifecycleTrack).toHaveBeenCalledWith('startup_perf', {
duration_ms: expect.any(Number),
config_ms: expect.any(Number),
init_ms: expect.any(Number),

View file

@ -251,7 +251,6 @@ describe('handleGoalCommand', () => {
expect(session.createGoal).toHaveBeenCalledWith(
expect.objectContaining({ objective: 'Ship feature X', replace: false }),
);
expect(host.track).toHaveBeenCalledWith('goal_create', { replace: false });
expect(host.sendNormalUserInput).toHaveBeenCalledWith('Ship feature X');
expect(host.sendNormalUserInput).not.toHaveBeenCalledWith('/goal Ship feature X');
});

View file

@ -185,7 +185,6 @@ describe('SessionEventHandler goal queue promotion', () => {
text: 'Ship queued goal',
});
expect(host.sendNormalUserInput).not.toHaveBeenCalled();
expect(host.track).toHaveBeenCalledWith('goal_create', { replace: false });
});
it('waits for queued user input to drain before promoting the next queued goal', async () => {

View file

@ -288,6 +288,7 @@ export class AcpServer implements Agent {
workDir: params.cwd,
kaos: acpKaos,
persistenceKaos,
sessionStartedProperties: { mode: 'new' },
// @ts-expect-error — `mcpServers` is a kernel-side extension
// (agent-core `CreateSessionPayload`) the SDK transparently
// forwards via spread. See block comment above.
@ -305,11 +306,6 @@ export class AcpServer implements Agent {
currentThinkingEnabled,
);
this.sessions.set(session.id, acpSession);
// Telemetry breadcrumb so we can observe ACP adoption (number of
// sessions started via this surface, vs. TUI / SDK direct). The
// property set is deliberately minimal: `mode` distinguishes
// `newSession` from `loadSession`; no user content / PII.
this.trackSessionStarted(session.id, 'new');
// Phase 14 (PLAN D11) advertises both the model and mode pickers as
// a unified `configOptions: SessionConfigOption[]` surface. The
// dedicated Phase 12 `modes:` field is gone — see
@ -363,10 +359,8 @@ export class AcpServer implements Agent {
cwd: params.cwd,
sessionId: params.sessionId,
mcpServers: params.mcpServers,
mode: 'load',
});
// Same telemetry breadcrumb as `newSession`, but `mode: 'load'`
// so we can distinguish session creation from resumption. No PII.
this.trackSessionStarted(session.id, 'load');
// Synchronously replay history — the response must not settle
// until every historical `session/update` has been pushed,
// otherwise the client would race the load completion against
@ -401,11 +395,8 @@ export class AcpServer implements Agent {
cwd: params.cwd,
sessionId: params.sessionId,
mcpServers: params.mcpServers,
mode: 'resume',
});
// Telemetry breadcrumb — distinguishes resume from new/load so we
// can observe which clients adopt the lighter-weight resume
// surface vs the history-replaying load surface. No PII.
this.trackSessionStarted(session.id, 'resume');
this.scheduleAvailableCommandsUpdate(session.id);
return { configOptions };
}
@ -437,6 +428,7 @@ export class AcpServer implements Agent {
cwd: string;
sessionId: string;
mcpServers?: ReadonlyArray<McpServer>;
mode: 'load' | 'resume';
}): Promise<{
session: Session;
acpSession: AcpSession;
@ -466,6 +458,7 @@ export class AcpServer implements Agent {
id: params.sessionId,
kaos: acpKaos,
persistenceKaos,
sessionStartedProperties: { mode: params.mode },
// @ts-expect-error — see block comment above; mcpServers is a
// kernel-only field that the SDK forwards via spread.
mcpServers,
@ -786,8 +779,7 @@ export class AcpServer implements Agent {
* throwing) adapter-level unit tests routinely construct minimal
* `KimiHarness` shapes that only stub `auth.status` + `createSession`.
* Production callers always supply a real harness with both methods;
* the swallow-and-fallback path exists purely for test ergonomics
* (matches the `track?.bind(...)` pattern at `trackSessionStarted`).
* the swallow-and-fallback path exists purely for test ergonomics.
*
* Logged at `warn` when a fallback fires so a dev who forgot to set
* `default_model = ...` sees a breadcrumb in the agent log.
@ -867,7 +859,7 @@ export class AcpServer implements Agent {
* Build a {@link TelemetryTrackFn} wrapper bound to the underlying
* harness so the {@link AcpSession} (and its reverse-RPC bridges in
* Phase 13) can emit PII-free breadcrumbs through the same
* `harness.track` channel `trackSessionStarted` uses. The wrapper
* `harness.track` channel. The wrapper
* shape is required by the broader `Record<string, unknown>` properties
* type {@link TelemetryTrackFn} uses the harness's own `track` is
* typed against the narrower `TelemetryProperties` (a
@ -928,31 +920,6 @@ export class AcpServer implements Agent {
}
}
/**
* Emit the single ACP-adapter telemetry event.
*
* Wraps {@link KimiHarness.track} so a partial harness stub
* (common in unit tests see `auth-gate.test.ts`, `session-new.test.ts`)
* cannot crash the request path. Production callers always supply a
* real harness with `.track`; the swallow-and-log fallback exists
* purely for test ergonomics.
*
* Property set is deliberately minimal: `sessionId` + `mode`. No
* user content, no IDE identity, no client capabilities keeping
* the breadcrumb PII-free.
*/
private trackSessionStarted(sessionId: string, mode: 'new' | 'load' | 'resume'): void {
const track = this.harness.track?.bind(this.harness);
if (typeof track !== 'function') return;
try {
track('acp_session_started', { sessionId, mode });
} catch (err) {
log.warn('acp: telemetry track failed', {
sessionId,
error: err instanceof Error ? err.message : String(err),
});
}
}
}
/**

View file

@ -37,6 +37,7 @@ export interface KimiHarnessRuntimeOptions {
readonly telemetry: TelemetryClient;
readonly ensureConfigFile: () => Promise<void>;
readonly onClose: () => void | Promise<void>;
readonly sessionStartedProperties?: TelemetryProperties;
}
export class KimiHarness {
@ -50,6 +51,7 @@ export class KimiHarness {
private readonly activeSessions = new Map<string, Session>();
private readonly ensureConfigFileImpl: () => Promise<void>;
private readonly closeImpl: () => void | Promise<void>;
private readonly sessionStartedProperties: TelemetryProperties;
constructor(
private readonly rpc: SDKRpcClientBase,
@ -63,6 +65,7 @@ export class KimiHarness {
this.auth = options.auth;
this.ensureConfigFileImpl = options.ensureConfigFile;
this.closeImpl = options.onClose;
this.sessionStartedProperties = options.sessionStartedProperties ?? {};
}
get sessions(): ReadonlyMap<string, Session> {
@ -86,7 +89,7 @@ export class KimiHarness {
}
async createSession(options: CreateSessionOptions): Promise<Session> {
const { planMode, kaos, persistenceKaos, ...coreOptions } = options;
const { planMode, kaos, persistenceKaos, sessionStartedProperties, ...coreOptions } = options;
const summary =
kaos === undefined && persistenceKaos === undefined
? await this.rpc.createSession(coreOptions)
@ -104,7 +107,7 @@ export class KimiHarness {
if (planMode === true) {
await session.setPlanMode(true);
}
this.trackSessionStarted(summary.id, false);
this.trackSessionStarted(summary.id, false, sessionStartedProperties);
this.trackSessionEvent(session.id, 'session_new');
return session;
}
@ -112,7 +115,7 @@ export class KimiHarness {
async resumeSession(input: ResumeSessionInput): Promise<Session> {
const id = normalizeSessionId(input.id);
const active = this.activeSessions.get(id);
const { kaos, persistenceKaos, ...resumeInput } = input;
const { kaos, persistenceKaos, sessionStartedProperties, ...resumeInput } = input;
if (active !== undefined) {
if (kaos !== undefined || persistenceKaos !== undefined) {
await this.rpc.resumeSessionWithKaos({ ...resumeInput, id }, kaos ?? persistenceKaos as Kaos, persistenceKaos);
@ -134,7 +137,7 @@ export class KimiHarness {
},
});
this.activeSessions.set(session.id, session);
this.trackSessionStarted(summary.id, true);
this.trackSessionStarted(summary.id, true, sessionStartedProperties);
this.trackSessionEvent(session.id, 'session_resume');
return session;
}
@ -246,8 +249,16 @@ export class KimiHarness {
withTelemetryContext(this.telemetry, { sessionId: eventSessionId }).track(event);
}
private trackSessionStarted(eventSessionId: string, resumed: boolean): void {
private trackSessionStarted(
eventSessionId: string,
resumed: boolean,
sessionScoped?: TelemetryProperties,
): void {
withTelemetryContext(this.telemetry, { sessionId: eventSessionId }).track('session_started', {
...this.sessionStartedProperties,
...sessionScoped,
// Canonical fields are owned by the harness and must win over any
// caller-supplied sessionStartedProperties that happen to share a key.
client_name: this.identity?.userAgentProduct ?? null,
client_version: this.identity?.version ?? null,
ui_mode: this.uiMode,

View file

@ -139,5 +139,6 @@ export function createKimiHarness(options: KimiHarnessOptions): KimiHarness {
telemetry: rpc.telemetry,
ensureConfigFile: () => rpc.ensureConfigFile(),
onClose: () => rpc.close(),
sessionStartedProperties: options.sessionStartedProperties,
});
}

View file

@ -88,6 +88,7 @@ export interface KimiHarnessOptions {
readonly skillDirs?: readonly string[];
readonly telemetry?: TelemetryClient | undefined;
readonly onOAuthRefresh?: ((outcome: OAuthRefreshOutcome) => void) | undefined;
readonly sessionStartedProperties?: TelemetryProperties;
}
export interface CreateSessionOptions {
@ -100,6 +101,7 @@ export interface CreateSessionOptions {
readonly metadata?: JsonObject | undefined;
readonly kaos?: Kaos | undefined;
readonly persistenceKaos?: Kaos | undefined;
readonly sessionStartedProperties?: TelemetryProperties;
}
export interface RenameSessionInput {
@ -111,6 +113,7 @@ export interface ResumeSessionInput {
readonly id: string;
readonly kaos?: Kaos | undefined;
readonly persistenceKaos?: Kaos | undefined;
readonly sessionStartedProperties?: TelemetryProperties;
}
export interface ForkSessionInput {

View file

@ -175,6 +175,133 @@ describe('KimiHarness.createSession transport link', () => {
}
});
it('merges process-level sessionStartedProperties into session_started', async () => {
const homeDir = await makeTempDir();
const workDir = await makeTempDir();
const records: TelemetryRecord[] = [];
const harness = createKimiHarness({
identity: TEST_IDENTITY,
homeDir,
telemetry: recordingTelemetry(records),
sessionStartedProperties: { yolo: true, plan: false },
});
try {
const session = await harness.createSession({
id: 'ses_process_props',
workDir,
});
expect(records).toContainEqual({
event: 'session_started',
sessionId: session.id,
properties: {
client_name: 'kimi-code-cli',
client_version: '0.0.0-test',
ui_mode: 'shell',
resumed: false,
yolo: true,
plan: false,
},
});
} finally {
await harness.close();
}
});
it('merges session-level sessionStartedProperties and overrides process-level ones', async () => {
const homeDir = await makeTempDir();
const workDir = await makeTempDir();
const records: TelemetryRecord[] = [];
const harness = createKimiHarness({
identity: TEST_IDENTITY,
homeDir,
telemetry: recordingTelemetry(records),
sessionStartedProperties: { mode: 'process', source: 'process' },
});
try {
const session = await harness.createSession({
id: 'ses_scoped_props',
workDir,
sessionStartedProperties: { mode: 'new' },
});
expect(records).toContainEqual({
event: 'session_started',
sessionId: session.id,
properties: {
client_name: 'kimi-code-cli',
client_version: '0.0.0-test',
ui_mode: 'shell',
resumed: false,
mode: 'new',
source: 'process',
},
});
await session.close();
await harness.resumeSession({
id: session.id,
sessionStartedProperties: { mode: 'load' },
});
expect(records).toContainEqual({
event: 'session_started',
sessionId: session.id,
properties: {
client_name: 'kimi-code-cli',
client_version: '0.0.0-test',
ui_mode: 'shell',
resumed: true,
mode: 'load',
source: 'process',
},
});
} finally {
await harness.close();
}
});
it('does not let sessionStartedProperties override canonical session_started fields', async () => {
const homeDir = await makeTempDir();
const workDir = await makeTempDir();
const records: TelemetryRecord[] = [];
const harness = createKimiHarness({
identity: TEST_IDENTITY,
homeDir,
telemetry: recordingTelemetry(records),
});
try {
const session = await harness.createSession({
id: 'ses_reserved_keys',
workDir,
sessionStartedProperties: {
client_name: 'evil',
client_version: 'evil',
ui_mode: 'evil',
resumed: true,
extra: 'kept',
},
});
expect(records).toContainEqual({
event: 'session_started',
sessionId: session.id,
properties: {
client_name: 'kimi-code-cli',
client_version: '0.0.0-test',
ui_mode: 'shell',
resumed: false,
extra: 'kept',
},
});
} finally {
await harness.close();
}
});
it('emits session_fork with the forked session context', async () => {
const homeDir = await makeTempDir();
const workDir = await makeTempDir();