From 75f95c9e39b78f852edfa3364dfbe0da2f2433cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Nerl=C3=B8e?= Date: Fri, 7 Aug 2026 10:53:59 +0200 Subject: [PATCH] fix(core): mint the fallback external trace id per run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs that carry no external trace context (schedules, task-to-task triggers) fall back to a trace id generated once in the TracingSDK constructor. With `experimental_processKeepAlive` the TracingSDK outlives the run, so every run on a warm process was exported to the external OTLP endpoint under that one id — merging unrelated runs into a single trace. This is the same warm-start hazard c043c4a6a fixed for the external context path, which read the context live but deliberately left the fallback captured at construction. Remint the fallback when the trace context manager's context object is reassigned, which is the run boundary. An empty configured id still means external export is off and is left alone rather than switched on. The test harness needed a fix too: `setGlobalManager` delegates to `registerGlobal`, which ignores a second registration, so every test after the first was mutating the first test's manager. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/external-trace-id-per-run.md | 5 ++ packages/core/src/v3/otel/tracingSDK.ts | 80 +++++++++++++++--- .../test/externalSpanExporterWrapper.test.ts | 81 +++++++++++++++++++ 3 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 .changeset/external-trace-id-per-run.md diff --git a/.changeset/external-trace-id-per-run.md b/.changeset/external-trace-id-per-run.md new file mode 100644 index 00000000000..f6e7c335c44 --- /dev/null +++ b/.changeset/external-trace-id-per-run.md @@ -0,0 +1,5 @@ +--- +"@trigger.dev/core": patch +--- + +Mint the fallback external trace id per run rather than once per `TracingSDK`. Runs that carry no external trace context fall back to a generated trace id, and with `experimental_processKeepAlive` the `TracingSDK` outlives the run — so every run on a warm process was exported to the external OTLP endpoint under one shared trace id, merging unrelated runs into a single trace. diff --git a/packages/core/src/v3/otel/tracingSDK.ts b/packages/core/src/v3/otel/tracingSDK.ts index 0b3a66a87b4..e3dd023b3ca 100644 --- a/packages/core/src/v3/otel/tracingSDK.ts +++ b/packages/core/src/v3/otel/tracingSDK.ts @@ -393,21 +393,67 @@ function setLogLevel(level: TracingDiagnosticLogLevel) { diag.setLogger(new DiagConsoleLogger(), diagLogLevel); } +/** + * The external trace id used by runs that carry no external trace context, + * minted once per run. + * + * It has to change per run for the same reason the wrappers read the external + * context live: with `processKeepAlive` the `TracingSDK` — and so the wrappers + * — outlive the run, so an id captured at construction merges every run on the + * process into one trace. The manager's trace context object is reassigned per + * run, which makes its identity the run boundary. + */ +class FallbackExternalTraceId { + private traceId: string; + private seenTraceContext: unknown; + + constructor( + private seed: string, + private traceIdGenerator: Pick = idGenerator + ) { + this.traceId = seed; + this.seenTraceContext = traceContext.getTraceContext(); + } + + get(): string { + // An empty seed means external export is disabled — leave it that way + // rather than minting an id and switching the feature on. + if (!this.seed) { + return this.seed; + } + + const currentTraceContext = traceContext.getTraceContext(); + + if (currentTraceContext !== this.seenTraceContext) { + this.seenTraceContext = currentTraceContext; + this.traceId = this.traceIdGenerator.generateTraceId(); + } + + return this.traceId; + } +} + export class ExternalSpanExporterWrapper { + private fallback: FallbackExternalTraceId; + constructor( private underlyingExporter: SpanExporter, - private externalTraceId: string - ) {} + externalTraceId: string, + traceIdGenerator?: Pick + ) { + this.fallback = new FallbackExternalTraceId(externalTraceId, traceIdGenerator); + } private transformSpan(span: ReadableSpan): ReadableSpan | undefined { // Read external context live, so per-run reassignment of // standardTraceContextManager.traceContext is honoured on warm-started // workers that reuse a single TracingSDK across runs. const externalTraceContext = traceContext.getExternalTraceContext(); + const fallbackTraceId = this.fallback.get(); const isExternallySampled = externalTraceContext ? isTraceFlagSampled(externalTraceContext.traceFlags) - : !!this.externalTraceId; + : !!fallbackTraceId; if (!isExternallySampled) { return; @@ -419,7 +465,7 @@ export class ExternalSpanExporterWrapper { const externalTraceId = externalTraceContext ? externalTraceContext.traceId - : this.externalTraceId; + : fallbackTraceId; const isAttemptSpan = span.attributes[SemanticInternalAttributes.SPAN_ATTEMPT]; @@ -478,17 +524,23 @@ export class ExternalSpanExporterWrapper { } class ExternalLogRecordExporterWrapper { + private fallback: FallbackExternalTraceId; + constructor( private underlyingExporter: LogRecordExporter, - private externalTraceId: string - ) {} + externalTraceId: string, + traceIdGenerator?: Pick + ) { + this.fallback = new FallbackExternalTraceId(externalTraceId, traceIdGenerator); + } export(logs: any[], resultCallback: (result: any) => void): void { const externalTraceContext = traceContext.getExternalTraceContext(); + const fallbackTraceId = this.fallback.get(); const isExternallySampled = externalTraceContext ? isTraceFlagSampled(externalTraceContext.traceFlags) - : !!this.externalTraceId; + : !!fallbackTraceId; if (!isExternallySampled) { this.underlyingExporter.export([], resultCallback); @@ -496,7 +548,9 @@ class ExternalLogRecordExporterWrapper { return; } - const modifiedLogs = logs.map((log) => this.transformLogRecord(log, externalTraceContext)); + const modifiedLogs = logs.map((log) => + this.transformLogRecord(log, externalTraceContext, fallbackTraceId) + ); this.underlyingExporter.export(modifiedLogs, resultCallback); } @@ -517,13 +571,13 @@ class ExternalLogRecordExporterWrapper { logRecord: ReadableLogRecord, externalTraceContext: | { traceId: string; spanId: string; tracestate?: string; traceFlags: number } - | undefined + | undefined, + fallbackTraceId: string ): ReadableLogRecord { // Capture externalTraceId for use within the proxy's scope. - // Use externalTraceContext.traceId if available, otherwise fall back to generated externalTraceId - const externalTraceId = externalTraceContext - ? externalTraceContext.traceId - : this.externalTraceId; + // Use externalTraceContext.traceId if available, otherwise fall back to the + // per-run generated id. + const externalTraceId = externalTraceContext ? externalTraceContext.traceId : fallbackTraceId; // If there's no spanContext, or if the externalTraceId is not set, return the original logRecord. if (!logRecord.spanContext || !externalTraceId) { diff --git a/packages/core/test/externalSpanExporterWrapper.test.ts b/packages/core/test/externalSpanExporterWrapper.test.ts index 9b51653a1ec..8880daad92e 100644 --- a/packages/core/test/externalSpanExporterWrapper.test.ts +++ b/packages/core/test/externalSpanExporterWrapper.test.ts @@ -53,6 +53,10 @@ describe("ExternalSpanExporterWrapper warm-start regression", () => { let manager: StandardTraceContextManager; beforeEach(() => { + // `setGlobalManager` delegates to `registerGlobal`, which ignores a second + // registration — without disabling first, every test after the first would + // keep mutating the first test's manager. + traceContext.disable(); manager = new StandardTraceContextManager(); traceContext.setGlobalManager(manager); }); @@ -77,4 +81,81 @@ describe("ExternalSpanExporterWrapper warm-start regression", () => { expect(span.parentSpanContext?.traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"); expect(span.spanContext().traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"); }); + + // Runs triggered internally — a schedule, or one task triggering another — + // carry no external trace context and so take the generated fallback. That id + // was captured at construction, which on a warm-started worker meant every run + // on the process shared a single trace id. + it("mints a new fallback trace id per run when there is no external context", () => { + const { exporter, captured } = makeCapturingExporter(); + + let generated = 0; + const idGenerator = { + generateTraceId: () => `${++generated}`.padStart(32, "0"), + }; + + manager.traceContext = { traceparent: TRACEPARENT_RUN_A }; + + const wrapper = new ExternalSpanExporterWrapper( + exporter, + "ffffffffffffffffffffffffffffffff", + idGenerator + ); + + wrapper.export([createAttemptSpan()], () => {}); + + // A second run on the same warm process: the manager is reassigned, so the + // fallback has to be reminted. + manager.traceContext = { traceparent: TRACEPARENT_RUN_B }; + + wrapper.export([createAttemptSpan()], () => {}); + + const runATraceId = captured[0]![0]!.spanContext().traceId; + const runBTraceId = captured[1]![0]!.spanContext().traceId; + + expect(runATraceId).toBe("ffffffffffffffffffffffffffffffff"); + expect(runBTraceId).not.toBe(runATraceId); + expect(runBTraceId).toBe("00000000000000000000000000000001"); + }); + + it("keeps one fallback trace id across every export within a run", () => { + const { exporter, captured } = makeCapturingExporter(); + + let generated = 0; + const idGenerator = { + generateTraceId: () => `${++generated}`.padStart(32, "0"), + }; + + manager.traceContext = { traceparent: TRACEPARENT_RUN_A }; + + const wrapper = new ExternalSpanExporterWrapper( + exporter, + "ffffffffffffffffffffffffffffffff", + idGenerator + ); + + wrapper.export([createAttemptSpan()], () => {}); + wrapper.export([createAttemptSpan()], () => {}); + + expect(captured[1]![0]!.spanContext().traceId).toBe(captured[0]![0]!.spanContext().traceId); + expect(generated).toBe(0); + }); + + it("leaves external export off when no external trace id was configured", () => { + const { exporter, captured } = makeCapturingExporter(); + + const idGenerator = { + generateTraceId: () => "00000000000000000000000000000001", + }; + + manager.traceContext = { traceparent: TRACEPARENT_RUN_A }; + + const wrapper = new ExternalSpanExporterWrapper(exporter, "", idGenerator); + + wrapper.export([createAttemptSpan()], () => {}); + + // Minting an id here would switch external export on for a deployment that + // never asked for it. + expect(captured[0]).toHaveLength(0); + }); });