Skip to content

Commit 2928290

Browse files
sanchitrkclaude
sanchitrk
and
claude
committed
fix: address telemetry review findings
- Export sandbox provisioning: CLI.selectSandbox and Sandbox.resolve are Info, and the server RPCs that provision a sandbox start their own Info trace, since RpcServer's Debug request span would drop them. - Record tokens and cost of an interrupted model call from the terminal the consumer drains. - E2E covers CODEWORK_TRACE_LEVEL=debug; the instruction test asserts stat only runs for existing files. - Drop comments that restated the code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a0a05f8 commit 2928290

12 files changed

Lines changed: 122 additions & 117 deletions

File tree

‎packages/codework/src/cli/cmd/handlers/run.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const awaitMessage = Effect.fn("CLI.awaitMessage")(function* (ended: Queue.Queue
6666
}
6767
});
6868

69-
const selectSandbox = Effect.fn("CLI.selectSandbox")(function* (input: Sandbox.Selection) {
69+
const selectSandbox = Effect.fn("CLI.selectSandbox", { level: "Info" })(function* (input: Sandbox.Selection) {
7070
const selection = yield* Sandbox.resolve(input);
7171
if (selection.created && selection.info !== undefined) {
7272
const id = selection.info.id;

‎packages/codework/src/framework/otel.ts‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@ import { FetchHttpClient } from "effect/http";
33
import { OtlpMetrics, OtlpSerialization, OtlpTracer } from "effect/observability";
44
import { uuidv7 } from "uuidv7";
55

6-
/**
7-
* OTLP export of the harness's spans and metrics, configured by the standard
8-
* `OTEL_*` environment variables alone. Without an endpoint every layer is
9-
* empty; a configuration that fails to build runs the command without export.
10-
*/
6+
// Configured by the standard `OTEL_*` variables alone. A configuration that fails
7+
// to build runs the command without export rather than failing it.
118

129
// The OTel spec defaults `OTEL_{TRACES,METRICS}_EXPORTER` to `otlp`; Effect reads an
1310
// unset list as none, so an endpoint alone would export nothing.
@@ -36,13 +33,9 @@ const fiberMetrics = Layer.unwrap(
3633
}),
3734
);
3835

39-
/**
40-
* Export only the spans marked `level: "Info"`: the command, the agent loop, model
41-
* and tool calls, and sandbox provisioning. Every other span, Effect's own `sql.*`
42-
* among them, defaults to `Debug`; `CODEWORK_TRACE_LEVEL=debug` exports those too.
43-
* Effect drops a span's children with it, so every span on the path from a command
44-
* to a model call must be `Info`.
45-
*/
36+
// Only spans marked `level: "Info"` are exported; the rest, Effect's own `sql.*`
37+
// among them, default to `Debug`. Effect drops a span's children with it, so every
38+
// span on the path from a command to a model or tool call must be `Info`.
4639
const levels = { all: "All", trace: "Trace", debug: "Debug", info: "Info", warn: "Warn", error: "Error" } as const;
4740

4841
const traceLevels = Layer.unwrap(
@@ -62,11 +55,7 @@ interface OtlpResource {
6255
readonly attributes: Record<string, string>;
6356
}
6457

65-
/**
66-
* Serialize by `OTEL_EXPORTER_OTLP_PROTOCOL`, defaulting to the spec's
67-
* `http/protobuf`. Effect has no gRPC exporter, so `grpc` exports nothing
68-
* rather than sending HTTP to a gRPC port.
69-
*/
58+
// Effect has no gRPC exporter: `grpc` exports nothing rather than sending HTTP to a gRPC port.
7059
const exporters = (resource: OtlpResource) =>
7160
Layer.unwrap(
7261
Effect.gen(function* () {
@@ -83,7 +72,6 @@ const exporters = (resource: OtlpResource) =>
8372
}),
8473
);
8574

86-
/** The release channel a version was published to, from its prerelease tag. */
8775
const channel = (version: string) =>
8876
version.includes("-canary.") ? "canary" : version.includes("-dev.") ? "dev" : "latest";
8977

‎packages/codework/src/framework/runtime.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ export function run(root: Spec.Any, handlers: ReadonlyArray<LazyHandler>, option
7676
return Command.run(provide(root, handlers, { version: options.version, path: [] }), options);
7777
}
7878

79-
/** Each handler runs inside its own OTLP export, named by its command path below the root. */
8079
function provide(
8180
node: Spec.Any,
8281
handlers: ReadonlyArray<LazyHandler>,

‎packages/codework/src/server/handlers.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ const sessionInfo = Effect.fn("Server.sessionInfo")(function* (sessionId: Sessio
4040
return toSessionInfo(yield* found.value.info);
4141
});
4242

43+
// RpcServer's request span has no level, so it is Debug and drops every Info span under it.
44+
// Provisioning a sandbox is worth exporting, so it starts a trace of its own.
45+
const exported = <A, E, R>(name: string, self: Effect.Effect<A, E, R>) =>
46+
self.pipe(Effect.withSpan(name, { level: "Info", root: true }));
47+
4348
const releaseOnFailure = (selection: Effect.Success<ReturnType<typeof Sandbox.resolve>> | undefined) =>
4449
selection?.created && selection.info !== undefined
4550
? Sandbox.stop(selection.info.id).pipe(Effect.catchCause(Effect.logError))
@@ -61,7 +66,8 @@ export const layer = Contract.Api.toLayer(
6166
"auth.refresh": ({ provider }) => oauth.refresh(provider),
6267
"auth.logout": ({ provider }) => oauth.logout(provider),
6368
"session.create": Effect.fnUntraced(function* ({ title, directory, hostDir, sandbox, runtime }) {
64-
const selection = sandbox === undefined ? undefined : yield* Sandbox.resolve(sandbox);
69+
const selection =
70+
sandbox === undefined ? undefined : yield* exported("Server.session.create", Sandbox.resolve(sandbox));
6571
const selected = selection?.info;
6672
const handle = yield* Session.create({
6773
...(title === undefined ? {} : { title }),
@@ -93,7 +99,8 @@ export const layer = Contract.Api.toLayer(
9399
return toSessionInfo(yield* handle.info);
94100
}),
95101
"session.relink": Effect.fnUntraced(function* ({ sessionId, sandbox, directory }) {
96-
const selection = sandbox === undefined ? undefined : yield* Sandbox.resolve(sandbox);
102+
const selection =
103+
sandbox === undefined ? undefined : yield* exported("Server.session.relink", Sandbox.resolve(sandbox));
97104
const selected = selection?.info;
98105
const handle = yield* Session.relink({
99106
sessionId,
@@ -132,7 +139,7 @@ export const layer = Contract.Api.toLayer(
132139
return (yield* Sandbox.list()).map(toSandboxInfo);
133140
}),
134141
"sandbox.create": Effect.fnUntraced(function* ({ driver }) {
135-
return toSandboxInfo(yield* Sandbox.create({ driver }));
142+
return toSandboxInfo(yield* exported("Server.sandbox.create", Sandbox.create({ driver })));
136143
}),
137144
"sandbox.register": Effect.fnUntraced(function* ({ driver, providerResourceId }) {
138145
return toSandboxInfo(yield* Sandbox.register({ driver, providerResourceId }));

‎packages/codework/test/__artifacts__/telemetry.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
},
99
"exported": [
1010
"(root) > CLI.run",
11+
"CLI.run > CLI.selectSandbox",
1112
"CLI.run > Loop.run",
13+
"CLI.selectSandbox > Sandbox.resolve",
1214
"Loop.run > Loop.runTurn",
1315
"Loop.runTurn > Loop.runTurnAttempt",
1416
"Loop.runTurnAttempt > LLM.run",

‎packages/codework/test/telemetry.test.ts‎

Lines changed: 83 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* @effect-diagnostics nodeBuiltinImport:off -- this suite spawns the CLI and serves a fake provider and collector. */
22
/* @effect-diagnostics globalTimers:off -- the subprocess deadline runs outside any Effect. */
33
import { spawn } from "node:child_process";
4-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
4+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
55
import { createServer, type IncomingMessage } from "node:http";
66
import type { AddressInfo } from "node:net";
77
import { tmpdir } from "node:os";
@@ -301,68 +301,81 @@ const exportedSpans = (exports: ReadonlyArray<Export>) => {
301301
].sort();
302302
};
303303

304-
/** Every exported resource, traces and metrics alike, must describe the same process. */
305304
const resources = (exports: ReadonlyArray<Export>) =>
306305
exports.flatMap((entry) => [...(entry.body.resourceSpans ?? []), ...(entry.body.resourceMetrics ?? [])]);
307306

307+
/** Run the CLI once against `collector` as its provider and OTLP endpoint, with no other `OTEL_*` set. */
308+
const exportRun = async (collector: Awaited<ReturnType<typeof serve>>, traceLevel?: string) => {
309+
const home = mkdtempSync(join(tmpdir(), "codework-otel-"));
310+
temps.push(home);
311+
const project = join(home, "project");
312+
const settings = join(home, "settings");
313+
mkdirSync(project);
314+
mkdirSync(settings);
315+
writeFileSync(join(project, "note.txt"), "hello from the note\n");
316+
writeFileSync(
317+
join(settings, "settings.jsonc"),
318+
JSON.stringify({
319+
retry: { baseDelayMs: 10 },
320+
model: { provider: "otel", id: "otel-model" },
321+
models: [
322+
{
323+
id: "otel-model",
324+
name: "otel-model",
325+
provider: { id: "otel", name: "Otel", source: "config", env: [] },
326+
baseUrl: `${collector.url}/v1`,
327+
reasoning: false,
328+
input: ["text"],
329+
cost: { input: 3, output: 15, cacheRead: 0, cacheWrite: 0 },
330+
contextWindow: 32768,
331+
maxTokens: 8192,
332+
protocol: "openai-compatible",
333+
},
334+
],
335+
}),
336+
);
337+
338+
const env: NodeJS.ProcessEnv = {
339+
...process.env,
340+
HOME: home,
341+
CODEWORK_MODELS_FILE: models,
342+
OTEL_EXPORTER_OTLP_ENDPOINT: collector.url,
343+
OTEL_EXPORTER_OTLP_HEADERS: "x-otel-test=present",
344+
// The collector reads JSON; protobuf is the default otherwise.
345+
OTEL_EXPORTER_OTLP_PROTOCOL: "http/json",
346+
};
347+
const configured = new Set([
348+
"OTEL_EXPORTER_OTLP_ENDPOINT",
349+
"OTEL_EXPORTER_OTLP_HEADERS",
350+
"OTEL_EXPORTER_OTLP_PROTOCOL",
351+
]);
352+
for (const key of Object.keys(env)) {
353+
if (key.startsWith("OTEL_") && !configured.has(key)) delete env[key];
354+
}
355+
delete env.CODEWORK_TRACE_LEVEL;
356+
if (traceLevel !== undefined) env.CODEWORK_TRACE_LEVEL = traceLevel;
357+
358+
const result = await runCli(
359+
["--home", home, "--database", ":memory:", "--user-config-dir", settings, "run", "What does note.txt say?"],
360+
{ cwd: project, env },
361+
);
362+
expect(result.status, result.stderr).toBe(0);
363+
expect(result.stdout).toContain("The note says hello.");
364+
365+
// No prompt, command, file content, or model output may leave in any span, attribute, or event.
366+
const traces = JSON.stringify(collector.exports.filter((entry) => entry.path === "/v1/traces"));
367+
for (const content of ["What does note.txt say?", "cat note.txt", "hello from the note", "The note says hello."]) {
368+
expect(traces).not.toContain(content);
369+
}
370+
};
371+
372+
const artifact = fileURLToPath(new URL("./__artifacts__/telemetry.json", import.meta.url));
373+
308374
describe("OTLP export", () => {
309375
it("exports GenAI spans and metrics for a run through the standard OTEL_* environment", async () => {
310376
const collector = await serve();
311377
try {
312-
const home = mkdtempSync(join(tmpdir(), "codework-otel-"));
313-
temps.push(home);
314-
const project = join(home, "project");
315-
const settings = join(home, "settings");
316-
mkdirSync(project);
317-
mkdirSync(settings);
318-
writeFileSync(join(project, "note.txt"), "hello from the note\n");
319-
writeFileSync(
320-
join(settings, "settings.jsonc"),
321-
JSON.stringify({
322-
retry: { baseDelayMs: 10 },
323-
model: { provider: "otel", id: "otel-model" },
324-
models: [
325-
{
326-
id: "otel-model",
327-
name: "otel-model",
328-
provider: { id: "otel", name: "Otel", source: "config", env: [] },
329-
baseUrl: `${collector.url}/v1`,
330-
reasoning: false,
331-
input: ["text"],
332-
cost: { input: 3, output: 15, cacheRead: 0, cacheWrite: 0 },
333-
contextWindow: 32768,
334-
maxTokens: 8192,
335-
protocol: "openai-compatible",
336-
},
337-
],
338-
}),
339-
);
340-
341-
const env: NodeJS.ProcessEnv = {
342-
...process.env,
343-
HOME: home,
344-
CODEWORK_MODELS_FILE: models,
345-
OTEL_EXPORTER_OTLP_ENDPOINT: collector.url,
346-
OTEL_EXPORTER_OTLP_HEADERS: "x-otel-test=present",
347-
// The collector reads JSON; protobuf is the default otherwise.
348-
OTEL_EXPORTER_OTLP_PROTOCOL: "http/json",
349-
};
350-
const configured = new Set([
351-
"OTEL_EXPORTER_OTLP_ENDPOINT",
352-
"OTEL_EXPORTER_OTLP_HEADERS",
353-
"OTEL_EXPORTER_OTLP_PROTOCOL",
354-
]);
355-
for (const key of Object.keys(env)) {
356-
if (key.startsWith("OTEL_") && !configured.has(key)) delete env[key];
357-
}
358-
delete env.CODEWORK_TRACE_LEVEL;
359-
360-
const result = await runCli(
361-
["--home", home, "--database", ":memory:", "--user-config-dir", settings, "run", "What does note.txt say?"],
362-
{ cwd: project, env },
363-
);
364-
expect(result.status, result.stderr).toBe(0);
365-
expect(result.stdout).toContain("The note says hello.");
378+
await exportRun(collector);
366379

367380
expect(collector.exports.length).toBeGreaterThan(0);
368381
expect(new Set(collector.exports.map((entry) => entry.path))).toEqual(new Set(["/v1/traces", "/v1/metrics"]));
@@ -371,17 +384,6 @@ describe("OTLP export", () => {
371384
const exported = resources(collector.exports).map((entry) => JSON.stringify(entry.resource));
372385
expect(new Set(exported).size, "every export carries one resource").toBe(1);
373386

374-
// No prompt, file content, or model output may leave in any span, attribute, or event.
375-
const traces = JSON.stringify(collector.exports.filter((entry) => entry.path === "/v1/traces"));
376-
for (const content of [
377-
"What does note.txt say?",
378-
"cat note.txt",
379-
"hello from the note",
380-
"The note says hello.",
381-
]) {
382-
expect(traces).not.toContain(content);
383-
}
384-
385387
await expect(
386388
`${JSON.stringify(
387389
{
@@ -398,4 +400,18 @@ describe("OTLP export", () => {
398400
await collector.close();
399401
}
400402
}, 60_000);
403+
404+
it("exports every span, Effect's own sql.* included, with CODEWORK_TRACE_LEVEL=debug", async () => {
405+
const collector = await serve();
406+
try {
407+
await exportRun(collector, "debug");
408+
const spans = exportedSpans(collector.exports);
409+
expect(spans.some((pair) => pair.endsWith("> sql.execute"))).toBe(true);
410+
// Debug adds spans; it never loses the ones exported by default.
411+
const defaults: ReadonlyArray<string> = JSON.parse(readFileSync(artifact, "utf8")).exported;
412+
expect(spans).toEqual(expect.arrayContaining([...defaults]));
413+
} finally {
414+
await collector.close();
415+
}
416+
}, 60_000);
401417
});

‎packages/harness/src/effect/sandbox.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export const Selection = Schema.Union([
7474
]);
7575
export type Selection = typeof Selection.Type;
7676

77-
export const resolve = Effect.fn("Sandbox.resolve")(function* (input: Selection) {
77+
export const resolve = Effect.fn("Sandbox.resolve", { level: "Info" })(function* (input: Selection) {
7878
if ("id" in input) {
7979
const found = yield* get(input.id);
8080
if (Option.isNone(found)) return yield* new SandboxNotFoundError({ id: input.id });

‎packages/harness/src/plugin/builtin/prompt/instruction.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ const ancestors = (directory: string): string[] => {
4747
/**
4848
* Which of `paths` are files. Absence is the expected answer and stays silent; any other failure
4949
* (permissions, a remote blip) is logged so a dropped file is not a mystery, then counts as absent.
50-
*
51-
* `exists` answers absence without failing, so only the few paths that exist are stat'ed. A stat
52-
* on a missing path fails its span, and most candidates are missing: probing them all with `stat`
53-
* filled every trace with errors that were never errors.
5450
*/
5551
const present = (fs: SandboxFileSystem.Interface, paths: ReadonlyArray<string>) =>
5652
Effect.forEach(

‎packages/harness/src/runner/llm.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,12 @@ const requestErrorType = (input: RequestInput, exit: Exit.Exit<LLMEventPublisher
231231
return providerError(input, messageFailure(exit.value.message)).reason._tag;
232232
};
233233

234-
/** Span attributes and metrics for one settled provider request. */
235234
const observe = Effect.fnUntraced(function* (
236235
input: RequestInput,
237236
timing: { readonly start: number; readonly firstToken: number | undefined },
238237
exit: Exit.Exit<LLMEventPublisher.Terminal, unknown>,
238+
// An interrupted request still drains aikit's terminal, which carries the usage it spent.
239+
drained: LLMEventPublisher.Terminal | undefined,
239240
) {
240241
const now = yield* Effect.clockWith((clock) => clock.currentTimeMillis);
241242
const labels = { provider: input.provider, model: input.model };
@@ -251,9 +252,10 @@ const observe = Effect.fnUntraced(function* (
251252
yield* Effect.annotateCurrentSpan("gen_ai.response.time_to_first_chunk", ttft);
252253
}
253254
if (errorType !== undefined) yield* Effect.annotateCurrentSpan("error.type", errorType);
254-
if (Exit.isFailure(exit)) return;
255+
const terminal = Exit.isSuccess(exit) ? exit.value : drained;
256+
if (terminal === undefined) return;
255257

256-
const { message } = exit.value;
258+
const { message } = terminal;
257259
const { usage } = message;
258260
// aikit's `input` excludes cached tokens; input counts them, as the GenAI conventions do.
259261
const inputTokens = usage.input + usage.cacheRead + usage.cacheWrite;
@@ -271,7 +273,7 @@ const observe = Effect.fnUntraced(function* (
271273
yield* Effect.annotateCurrentSpan({
272274
"gen_ai.response.model": message.responseModel ?? message.model,
273275
...(message.responseId === undefined ? {} : { "gen_ai.response.id": message.responseId }),
274-
"gen_ai.response.finish_reasons": [exit.value.reason],
276+
"gen_ai.response.finish_reasons": [terminal.reason],
275277
"gen_ai.usage.input_tokens": inputTokens,
276278
"gen_ai.usage.output_tokens": usage.output,
277279
"gen_ai.usage.cache_read.input_tokens": usage.cacheRead,
@@ -308,6 +310,7 @@ export const make = (openStream: Open): Request => {
308310
});
309311
const start = yield* Effect.clockWith((clock) => clock.currentTimeMillis);
310312
let firstToken: number | undefined;
313+
let drained: LLMEventPublisher.Terminal | undefined;
311314
const markToken = (event: AikitEvent.LLMMessageEvent) =>
312315
firstToken !== undefined || !isToken(event)
313316
? Effect.void
@@ -350,8 +353,9 @@ export const make = (openStream: Open): Request => {
350353
);
351354

352355
if (Exit.isFailure(awaited)) {
353-
const drained = yield* Fiber.await(consumer);
354-
if (Exit.isFailure(drained)) return yield* Effect.failCause(drained.cause);
356+
const consumed = yield* Fiber.await(consumer);
357+
if (Exit.isFailure(consumed)) return yield* Effect.failCause(consumed.cause);
358+
drained = consumed.value;
355359
return yield* Effect.failCause(awaited.cause);
356360
}
357361

@@ -360,7 +364,9 @@ export const make = (openStream: Open): Request => {
360364
return completed.value;
361365
}),
362366
);
363-
const terminal = yield* settled.pipe(Effect.onExit((exit) => observe(input, { start, firstToken }, exit)));
367+
const terminal = yield* settled.pipe(
368+
Effect.onExit((exit) => observe(input, { start, firstToken }, exit, drained)),
369+
);
364370
if (terminal.outcome === "failed") return yield* new FailedTerminal({ terminal });
365371
return terminal;
366372
});

0 commit comments

Comments
 (0)