From 87392ce0258e08a6f4c30aee27457254fe7110b7 Mon Sep 17 00:00:00 2001 From: Roland Schlaefli Date: Wed, 9 Sep 2026 23:04:48 +0200 Subject: [PATCH 1/2] docs(project): plan nonfatal request telemetry --- .../2026-09-09-nonfatal-telemetry-plan.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 docs/project/2026-09-09-nonfatal-telemetry-plan.md diff --git a/docs/project/2026-09-09-nonfatal-telemetry-plan.md b/docs/project/2026-09-09-nonfatal-telemetry-plan.md new file mode 100644 index 00000000..71b370e3 --- /dev/null +++ b/docs/project/2026-09-09-nonfatal-telemetry-plan.md @@ -0,0 +1,55 @@ +# Keep Code Interpreter telemetry outside domain failures + +## Approval and outcome + +Continue W2 of the already approved optional-telemetry roadmap. Contain optional SDK setup, +propagation, span callbacks and request instrumentation failures without changing domain results, +error identity, invocation count, healthy trace propagation, privacy or shutdown behavior. +Source fixes, synthetic tests, reviews, ordinary fork-branch push and draft PR are authorized. +No merge, release, deployment, dependency, shutdown expansion, inference or gitlink change. + +Repository: maintained `uzh-bf/code-interpreter` fork, remote `uzh`, target `main` at +`d2382d66491b05f1e9000ad6756868a66ea47e1d`. Branch `rs/nonfatal-telemetry`, worktree +`upstream/trees/rs/nonfatal-telemetry`. The `origin` remote is not the delivery target. +PR 21 owns disjoint OpenAPI/replay/worker paths. Parent retains integration and external effects. +Full-path package; terminal is reviewed source with passing checks and a draft PR. + +## One implementation slice + +Route: main. Execution-tier skip reason: unhealthy route. `ocx ready --json` reports +`ready:false,status:failed`; liveness is healthy. Native planner completed successfully, which +proves only that planner operation. No service mutation or executor dispatch is assumed. + +Write only `shared/telemetry-core.ts`, `shared/telemetry-test-suite.ts`, +`api/src/telemetry.test.ts`, `service/src/telemetry.test.ts` plus this plan. Guard SDK preparation +before invoking domain callbacks. Never retry domain work. Failed extraction uses ROOT_CONTEXT; +failed injection discards partially written carrier fields but preserves caller headers. +Initialize once, preserve available propagation after exporter failure, and enable spans only +after complete setup. Span facade operations are individually nonthrowing; error status wins +and end is attempted at most once. Middleware calls next once, preserving its thrown value. +Protect only instrumentation around emitter binding and completion hooks, never application +listener execution. Preserve existing shutdown rejection, timeout and shared-promise semantics. +Do not add a reset API or telemetry factory for tests. A partial-resource ownership issue that +requires shutdown changes returns to the parent before that expansion. + +## Acceptance and review + +Use synthetic configured SDK dependencies in isolated Bun processes for initialization state. +Prove setup and propagation failure isolation, result and error identity, partial injection +removal, middleware next exactly once and callback/end failures. Retain healthy privacy and +stream-context tests in both package entrypoints. No real exporter or application needed. +Run pinned Bun 1.3.14 tests for api/src/telemetry.test.ts and service/src/telemetry.test.ts, +then applicable package builds/tests and CI. Preserve lockfiles. Required simplifier, risk review +and integrated final review precede completed source delivery. No browser-only contract changes. + +## Planning disposition + +Native planner Tesla returned DONE_WITH_CONCERNS on the complete finite scope. Main accepts +ROOT_CONTEXT fallback, one-shot degradation, per-operation facade guards and isolated synthetic +fault tests. Shutdown contract remains unchanged; broad new shutdown testing is deferred unless +changed code crosses it. Existing healthy tests remain. This is a correction to existing internal +instrumentation, with no new product primitive, trust boundary or forward-looking ADR decision. + +## Progress + +Ownership and target confirmed; source unchanged. Dependencies absent in this new worktree. From cbe08a91a4a34c75aec85b3276f69bdb67d42897 Mon Sep 17 00:00:00 2001 From: Roland Schlaefli Date: Wed, 9 Sep 2026 23:11:23 +0200 Subject: [PATCH 2/2] fix(telemetry): isolate optional request instrumentation failures --- api/src/telemetry.test.ts | 4 +- .../2026-09-09-nonfatal-telemetry-plan.md | 14 +- service/src/telemetry.test.ts | 4 +- shared/telemetry-core.ts | 147 ++++++++++-------- shared/telemetry-test-suite.ts | 99 ++++++++++++ 5 files changed, 201 insertions(+), 67 deletions(-) diff --git a/api/src/telemetry.test.ts b/api/src/telemetry.test.ts index 6199395e..632164fc 100644 --- a/api/src/telemetry.test.ts +++ b/api/src/telemetry.test.ts @@ -1,4 +1,4 @@ -import { runTelemetryPrivacyGuardTests } from '../../shared/telemetry-test-suite'; +import { runTelemetryFailureTests, runTelemetryPrivacyGuardTests } from '../../shared/telemetry-test-suite'; import { captureTraceCarrier, injectTraceHeaders, @@ -14,3 +14,5 @@ runTelemetryPrivacyGuardTests({ traceHttpRequest, withTraceContext, }); + +runTelemetryFailureTests(); diff --git a/docs/project/2026-09-09-nonfatal-telemetry-plan.md b/docs/project/2026-09-09-nonfatal-telemetry-plan.md index 71b370e3..830c828a 100644 --- a/docs/project/2026-09-09-nonfatal-telemetry-plan.md +++ b/docs/project/2026-09-09-nonfatal-telemetry-plan.md @@ -52,4 +52,16 @@ instrumentation, with no new product primitive, trust boundary or forward-lookin ## Progress -Ownership and target confirmed; source unchanged. Dependencies absent in this new worktree. +Ownership and target confirmed. Main implemented four scoped files after the planner pass. +Sixteen initial enabled fault scenarios failed before the guards. Expanded focused coverage now +has 52 passing tests across both package entrypoints, including successful telemetry and processor/ +resource failures. Full API suite386 and service suite577 passed before the final fixture-only +scenario additions; their affected suite was rerun. Both package builds pass. Service build emits +an existing replay-state type-cast warning outside this scope. Bun1.3.14 dependencies installed +from unchanged lockfiles. Source is awaiting committed slice reviews and final review. + +The source guards configuration, one-shot initialization, propagation and facade callbacks. +Middleware preparation is separate from next; finish/close end once, failed listener setup ends +best effort. Explicit missing-dependency configuration remains an invariant. No shutdown change. +Substantive diff188 additions/66 deletions across four source/test paths before final review. +Generated api/.build is untracked and excluded from publication; service build output is ignored. diff --git a/service/src/telemetry.test.ts b/service/src/telemetry.test.ts index 6199395e..632164fc 100644 --- a/service/src/telemetry.test.ts +++ b/service/src/telemetry.test.ts @@ -1,4 +1,4 @@ -import { runTelemetryPrivacyGuardTests } from '../../shared/telemetry-test-suite'; +import { runTelemetryFailureTests, runTelemetryPrivacyGuardTests } from '../../shared/telemetry-test-suite'; import { captureTraceCarrier, injectTraceHeaders, @@ -14,3 +14,5 @@ runTelemetryPrivacyGuardTests({ traceHttpRequest, withTraceContext, }); + +runTelemetryFailureTests(); diff --git a/shared/telemetry-core.ts b/shared/telemetry-core.ts index 17847459..17129a28 100644 --- a/shared/telemetry-core.ts +++ b/shared/telemetry-core.ts @@ -75,7 +75,15 @@ export function configureTelemetry(config: TelemetryConfig): void { resolveServiceName: config.resolveServiceName, }; otelDependencies = config.otel; - tracer = config.otel.trace.getTracer('codeapi.manual.telemetry'); + tracer = optionalTelemetry(() => config.otel.trace.getTracer('codeapi.manual.telemetry')); +} + +function optionalTelemetry(operation: () => T): T | undefined { + try { + return operation(); + } catch { + return undefined; + } } function otel(): OtelDependencies { @@ -138,22 +146,24 @@ function ensureTelemetryInitialized(): void { if (initialized) return; initialized = true; const deps = otel(); - deps.propagation.setGlobalPropagator(new deps.W3CTraceContextPropagator()); + optionalTelemetry(() => deps.propagation.setGlobalPropagator(new deps.W3CTraceContextPropagator())); if (!tracingEnabled()) return; - const exporter = new deps.OTLPTraceExporter(); - const processor = new deps.BatchSpanProcessor(exporter); - telemetryProvider = new deps.BasicTracerProvider({ - resource: telemetryResource(), - spanLimits: { - attributeValueLengthLimit: MAX_ATTRIBUTE_LENGTH, - }, - spanProcessors: [processor], + optionalTelemetry(() => { + const exporter = new deps.OTLPTraceExporter(); + const processor = new deps.BatchSpanProcessor(exporter); + telemetryProvider = new deps.BasicTracerProvider({ + resource: telemetryResource(), + spanLimits: { + attributeValueLengthLimit: MAX_ATTRIBUTE_LENGTH, + }, + spanProcessors: [processor], + }); + deps.trace.setGlobalTracerProvider(telemetryProvider); + tracer = deps.trace.getTracer('codeapi.manual.telemetry'); + tracingInitialized = true; }); - deps.trace.setGlobalTracerProvider(telemetryProvider); - tracer = deps.trace.getTracer('codeapi.manual.telemetry'); - tracingInitialized = true; } export async function shutdownTelemetry( @@ -203,10 +213,10 @@ function sanitizedCarrier(carrier: TraceCarrier | undefined): TraceCarrier { export function extractTraceContext(carrier: TraceCarrier | undefined): ContextLike { ensureTelemetryInitialized(); const deps = otel(); - return deps.propagation.extract(deps.ROOT_CONTEXT, sanitizedCarrier(carrier), { + return optionalTelemetry(() => deps.propagation.extract(deps.ROOT_CONTEXT, sanitizedCarrier(carrier), { get: carrierGetter, keys: (c: TraceCarrier) => Object.keys(c), - }); + })) ?? deps.ROOT_CONTEXT; } export function withTraceContext(carrier: TraceCarrier | undefined, fn: () => T): T { @@ -216,9 +226,11 @@ export function withTraceContext(carrier: TraceCarrier | undefined, fn: () => export function captureTraceCarrier(): Record { ensureTelemetryInitialized(); const deps = otel(); - const carrier: Record = {}; - deps.propagation.inject(store.getStore() ?? deps.ROOT_CONTEXT, carrier); - return carrier; + return optionalTelemetry(() => { + const carrier: Record = {}; + deps.propagation.inject(store.getStore() ?? deps.ROOT_CONTEXT, carrier); + return carrier; + }) ?? {}; } export function injectTraceHeaders>(headers?: T): T & Record { @@ -276,26 +288,26 @@ function createSpanFacade(span: SpanLike): TelemetrySpan { let errorStatusSet = false; return { setAttribute: (key, value) => { - setCleanAttribute(span, key, value); + optionalTelemetry(() => setCleanAttribute(span, key, value)); }, setStatus: status => { if (status.code === 'ERROR') { errorStatusSet = true; - span.setStatus({ code: SpanStatusCode.ERROR, message: safeStatusMessage(status.message) }); + optionalTelemetry(() => span.setStatus({ code: SpanStatusCode.ERROR, message: safeStatusMessage(status.message) })); return; } - if (!errorStatusSet) span.setStatus({ code: SpanStatusCode.OK }); + if (!errorStatusSet) optionalTelemetry(() => span.setStatus({ code: SpanStatusCode.OK })); }, recordException: error => { errorStatusSet = true; - span.recordException?.(error instanceof Error ? error : String(error)); - span.setAttribute('exception.type', exceptionType(error)); - span.setStatus({ code: SpanStatusCode.ERROR }); + optionalTelemetry(() => span.recordException?.(error instanceof Error ? error : String(error))); + optionalTelemetry(() => span.setAttribute('exception.type', exceptionType(error))); + optionalTelemetry(() => span.setStatus({ code: SpanStatusCode.ERROR })); }, end: () => { if (ended) return; ended = true; - span.end(); + optionalTelemetry(() => span.end()); }, }; } @@ -347,15 +359,22 @@ export async function withSpan( ensureTelemetryInitialized(); if (!tracingInitialized) return fn(noopSpan()); - const deps = otel(); - const activeTracer = tracer ?? deps.trace.getTracer('codeapi.manual.telemetry'); - const parentContext = store.getStore() ?? deps.otelContext.active(); - const span = activeTracer.startSpan(name, { - attributes: cleanAttributes(attributes), - kind: otelSpanKind(kind), - }, parentContext); - const spanContext = deps.trace.setSpan(parentContext, span); - const telemetrySpan = createSpanFacade(span); + let span: SpanLike; + const prepared = optionalTelemetry(() => { + const deps = otel(); + const activeTracer = tracer ?? deps.trace.getTracer('codeapi.manual.telemetry'); + const parentContext = store.getStore() ?? deps.otelContext.active(); + span = activeTracer.startSpan(name, { + attributes: cleanAttributes(attributes), + kind: otelSpanKind(kind), + }, parentContext); + return { context: deps.trace.setSpan(parentContext, span), facade: createSpanFacade(span) }; + }); + if (!prepared) { + optionalTelemetry(() => span?.end()); + return fn(noopSpan()); + } + const { context: spanContext, facade: telemetrySpan } = prepared; return store.run(spanContext, async () => { try { @@ -382,27 +401,23 @@ export function traceHttpRequest(name = 'codeapi.http.request') { statusCode?: number; }, next: () => void): void => { const parentContext = extractTraceContext(req.headers); - const deps = otel(); - const route = normalizeTracePath(req.path ?? req.originalUrl ?? req.url ?? '/'); - if (!tracingInitialized) { - store.run(parentContext, () => { - bindEmitterToTraceContext(req, parentContext); - bindEmitterToTraceContext(res, parentContext); - next(); - }); - return; - } - - const activeTracer = tracer ?? deps.trace.getTracer('codeapi.manual.telemetry'); - const span = activeTracer.startSpan(name, { - attributes: cleanAttributes({ - 'http.request.method': req.method ?? 'UNKNOWN', - 'url.path': route, - }), - kind: deps.OtelSpanKind.SERVER, - }, parentContext); - const context = deps.trace.setSpan(parentContext, span); - const telemetrySpan = createSpanFacade(span); + let span: SpanLike; + const prepared = tracingInitialized ? optionalTelemetry(() => { + const deps = otel(); + const route = normalizeTracePath(req.path ?? req.originalUrl ?? req.url ?? '/'); + const activeTracer = tracer ?? deps.trace.getTracer('codeapi.manual.telemetry'); + span = activeTracer.startSpan(name, { + attributes: cleanAttributes({ + 'http.request.method': req.method ?? 'UNKNOWN', + 'url.path': route, + }), + kind: deps.OtelSpanKind.SERVER, + }, parentContext); + return { context: deps.trace.setSpan(parentContext, span), facade: createSpanFacade(span) }; + }) : undefined; + if (!prepared) optionalTelemetry(() => span?.end()); + const context = prepared?.context ?? parentContext; + const telemetrySpan = prepared?.facade ?? noopSpan(); let finished = false; const finish = (): void => { if (finished) return; @@ -415,15 +430,19 @@ export function traceHttpRequest(name = 'codeapi.http.request') { }; store.run(context, () => { - bindEmitterToTraceContext(req, context); - bindEmitterToTraceContext(res, context); - if (res.once) { - res.once('finish', finish); - res.once('close', finish); - } else { - res.on?.('finish', finish); - res.on?.('close', finish); - } + optionalTelemetry(() => bindEmitterToTraceContext(req, context)); + optionalTelemetry(() => bindEmitterToTraceContext(res, context)); + const registered = optionalTelemetry(() => { + if (res.once) { + res.once('finish', finish); + res.once('close', finish); + } else { + res.on?.('finish', finish); + res.on?.('close', finish); + } + return true; + }); + if (!registered) telemetrySpan.end(); next(); }); }; diff --git a/shared/telemetry-test-suite.ts b/shared/telemetry-test-suite.ts index 3f36edf8..dc10f5c9 100644 --- a/shared/telemetry-test-suite.ts +++ b/shared/telemetry-test-suite.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test'; import { EventEmitter } from 'events'; +import { spawnSync } from 'node:child_process'; type TraceMiddleware = ( req: EventEmitter & { @@ -114,3 +115,101 @@ export function runTelemetryPrivacyGuardTests(telemetry: TelemetryTestSubject): }); }); } + + +export function runTelemetryFailureTests(): void { + const faults = ['healthy', 'getTracer', 'propagator', 'exporter', 'processor', 'resource', 'provider', 'registration', + 'extract', 'inject', 'active', 'startSpan', 'setSpan', 'sanitize', + 'setAttribute', 'setStatus', 'recordException', 'end', 'listener']; + for (const fault of faults) { + test(`optional telemetry ${fault} failure preserves domain outcomes`, () => { + const script = ` + import assert from 'node:assert/strict'; + import { EventEmitter } from 'node:events'; + import * as telemetry from ${JSON.stringify(import.meta.resolve('./telemetry-core'))}; + const fault = ${JSON.stringify(fault)}; + const sentinel = new Error('synthetic domain failure'); + const result = { ok: true }; + const root = {}; + const fail = name => { if (name === fault) throw new Error('synthetic telemetry fault'); }; + let endCalls = 0; + const span = { + setAttribute() { fail('setAttribute'); }, + setStatus() { fail('setStatus'); }, + recordException() { fail('recordException'); }, + end() { endCalls++; fail('end'); }, + }; + const tracer = { startSpan() { fail('startSpan'); return span; } }; + const resource = { merge() { return resource; } }; + telemetry.configureTelemetry({ defaultServiceName: 'synthetic', otel: { + ROOT_CONTEXT: root, OtelSpanKind: { INTERNAL: 0, SERVER: 1 }, + SpanStatusCode: { OK: 1, ERROR: 2 }, + otelContext: { active() { fail('active'); return root; } }, + propagation: { + setGlobalPropagator() { fail('propagator'); }, + extract() { fail('extract'); return root; }, + inject(context, carrier) { carrier.traceparent = 'partial'; fail('inject'); }, + }, + trace: { + getTracer() { fail('getTracer'); return tracer; }, + setGlobalTracerProvider() { fail('registration'); }, + setSpan(context) { fail('setSpan'); return context; }, + }, + W3CTraceContextPropagator: class {}, + OTLPTraceExporter: class { constructor() { fail('exporter'); } }, + BatchSpanProcessor: class { constructor() { fail('processor'); } }, + BasicTracerProvider: class { constructor() { fail('provider'); } async shutdown() {} }, + defaultResource: () => resource, detectResources() { fail('resource'); return resource; }, + envDetector: {}, resourceFromAttributes: () => resource, + sanitizeAttributes(attrs) { fail('sanitize'); return attrs; }, + }}); + let calls = 0; + const actual = await telemetry.withSpan('synthetic', {}, facade => { + calls++; + facade.setAttribute('synthetic', true); + facade.setStatus({ code: 'ERROR' }); + facade.recordException(sentinel); + return result; + }); + assert.equal(actual, result); assert.equal(calls, 1); + for (const asyncFailure of [false, true]) { + calls = 0; + try { + await telemetry.withSpan('synthetic', {}, () => { + calls++; + if (asyncFailure) return Promise.reject(sentinel); + throw sentinel; + }); + assert.fail('domain error was suppressed'); + } catch (error) { assert.equal(error, sentinel); } + assert.equal(calls, 1); + } + calls = 0; + assert.equal(telemetry.withTraceContext({}, () => { calls++; return result; }), result); + assert.equal(calls, 1); + const headers = telemetry.injectTraceHeaders({ 'x-synthetic': 'preserved' }); + assert.equal(headers['x-synthetic'], 'preserved'); + if (fault === 'inject') assert.equal(headers.traceparent, undefined); + for (const domainFails of [false, true]) { + const req = new EventEmitter(); req.headers = {}; req.path = '/health'; + const res = new EventEmitter(); res.statusCode = 200; + if (fault === 'listener') res.once = () => { throw new Error('synthetic listener failure'); }; + calls = 0; + try { + telemetry.traceHttpRequest()(req, res, () => { calls++; if (domainFails) throw sentinel; }); + if (domainFails) assert.fail('next error was suppressed'); + } catch (error) { if (!domainFails) throw error; assert.equal(error, sentinel); } + assert.equal(calls, 1); + const endsBefore = endCalls; + res.emit('finish'); res.emit('close'); + assert.ok(endCalls - endsBefore <= 1); + } + `; + const child = spawnSync(process.execPath, ['--eval', script], { + env: { PATH: process.env.PATH, OTEL_TRACING_ENABLED: 'true' }, + encoding: 'utf8', timeout: 10000, + }); + expect({ status: child.status, stderr: child.stderr }).toEqual({ status: 0, stderr: '' }); + }); + } +}