Skip to content

Commit 2bc20c8

Browse files
NERLOEclaude
andcommitted
fix(core): mint the fallback external trace id per run
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 c043c4a 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) <noreply@anthropic.com>
1 parent 98cdf89 commit 2bc20c8

3 files changed

Lines changed: 153 additions & 13 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@trigger.dev/core": patch
3+
---
4+
5+
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.

packages/core/src/v3/otel/tracingSDK.ts

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -393,21 +393,67 @@ function setLogLevel(level: TracingDiagnosticLogLevel) {
393393
diag.setLogger(new DiagConsoleLogger(), diagLogLevel);
394394
}
395395

396+
/**
397+
* The external trace id used by runs that carry no external trace context,
398+
* minted once per run.
399+
*
400+
* It has to change per run for the same reason the wrappers read the external
401+
* context live: with `processKeepAlive` the `TracingSDK` — and so the wrappers
402+
* — outlive the run, so an id captured at construction merges every run on the
403+
* process into one trace. The manager's trace context object is reassigned per
404+
* run, which makes its identity the run boundary.
405+
*/
406+
class FallbackExternalTraceId {
407+
private traceId: string;
408+
private seenTraceContext: unknown;
409+
410+
constructor(
411+
private seed: string,
412+
private traceIdGenerator: Pick<RandomIdGenerator, "generateTraceId"> = idGenerator
413+
) {
414+
this.traceId = seed;
415+
this.seenTraceContext = traceContext.getTraceContext();
416+
}
417+
418+
get(): string {
419+
// An empty seed means external export is disabled — leave it that way
420+
// rather than minting an id and switching the feature on.
421+
if (!this.seed) {
422+
return this.seed;
423+
}
424+
425+
const currentTraceContext = traceContext.getTraceContext();
426+
427+
if (currentTraceContext !== this.seenTraceContext) {
428+
this.seenTraceContext = currentTraceContext;
429+
this.traceId = this.traceIdGenerator.generateTraceId();
430+
}
431+
432+
return this.traceId;
433+
}
434+
}
435+
396436
export class ExternalSpanExporterWrapper {
437+
private fallback: FallbackExternalTraceId;
438+
397439
constructor(
398440
private underlyingExporter: SpanExporter,
399-
private externalTraceId: string
400-
) {}
441+
externalTraceId: string,
442+
traceIdGenerator?: Pick<RandomIdGenerator, "generateTraceId">
443+
) {
444+
this.fallback = new FallbackExternalTraceId(externalTraceId, traceIdGenerator);
445+
}
401446

402447
private transformSpan(span: ReadableSpan): ReadableSpan | undefined {
403448
// Read external context live, so per-run reassignment of
404449
// standardTraceContextManager.traceContext is honoured on warm-started
405450
// workers that reuse a single TracingSDK across runs.
406451
const externalTraceContext = traceContext.getExternalTraceContext();
452+
const fallbackTraceId = this.fallback.get();
407453

408454
const isExternallySampled = externalTraceContext
409455
? isTraceFlagSampled(externalTraceContext.traceFlags)
410-
: !!this.externalTraceId;
456+
: !!fallbackTraceId;
411457

412458
if (!isExternallySampled) {
413459
return;
@@ -419,7 +465,7 @@ export class ExternalSpanExporterWrapper {
419465

420466
const externalTraceId = externalTraceContext
421467
? externalTraceContext.traceId
422-
: this.externalTraceId;
468+
: fallbackTraceId;
423469

424470
const isAttemptSpan = span.attributes[SemanticInternalAttributes.SPAN_ATTEMPT];
425471

@@ -478,25 +524,33 @@ export class ExternalSpanExporterWrapper {
478524
}
479525

480526
class ExternalLogRecordExporterWrapper {
527+
private fallback: FallbackExternalTraceId;
528+
481529
constructor(
482530
private underlyingExporter: LogRecordExporter,
483-
private externalTraceId: string
484-
) {}
531+
externalTraceId: string,
532+
traceIdGenerator?: Pick<RandomIdGenerator, "generateTraceId">
533+
) {
534+
this.fallback = new FallbackExternalTraceId(externalTraceId, traceIdGenerator);
535+
}
485536

486537
export(logs: any[], resultCallback: (result: any) => void): void {
487538
const externalTraceContext = traceContext.getExternalTraceContext();
539+
const fallbackTraceId = this.fallback.get();
488540

489541
const isExternallySampled = externalTraceContext
490542
? isTraceFlagSampled(externalTraceContext.traceFlags)
491-
: !!this.externalTraceId;
543+
: !!fallbackTraceId;
492544

493545
if (!isExternallySampled) {
494546
this.underlyingExporter.export([], resultCallback);
495547

496548
return;
497549
}
498550

499-
const modifiedLogs = logs.map((log) => this.transformLogRecord(log, externalTraceContext));
551+
const modifiedLogs = logs.map((log) =>
552+
this.transformLogRecord(log, externalTraceContext, fallbackTraceId)
553+
);
500554

501555
this.underlyingExporter.export(modifiedLogs, resultCallback);
502556
}
@@ -517,13 +571,13 @@ class ExternalLogRecordExporterWrapper {
517571
logRecord: ReadableLogRecord,
518572
externalTraceContext:
519573
| { traceId: string; spanId: string; tracestate?: string; traceFlags: number }
520-
| undefined
574+
| undefined,
575+
fallbackTraceId: string
521576
): ReadableLogRecord {
522577
// Capture externalTraceId for use within the proxy's scope.
523-
// Use externalTraceContext.traceId if available, otherwise fall back to generated externalTraceId
524-
const externalTraceId = externalTraceContext
525-
? externalTraceContext.traceId
526-
: this.externalTraceId;
578+
// Use externalTraceContext.traceId if available, otherwise fall back to the
579+
// per-run generated id.
580+
const externalTraceId = externalTraceContext ? externalTraceContext.traceId : fallbackTraceId;
527581

528582
// If there's no spanContext, or if the externalTraceId is not set, return the original logRecord.
529583
if (!logRecord.spanContext || !externalTraceId) {

packages/core/test/externalSpanExporterWrapper.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ describe("ExternalSpanExporterWrapper warm-start regression", () => {
5353
let manager: StandardTraceContextManager;
5454

5555
beforeEach(() => {
56+
// `setGlobalManager` delegates to `registerGlobal`, which ignores a second
57+
// registration — without disabling first, every test after the first would
58+
// keep mutating the first test's manager.
59+
traceContext.disable();
5660
manager = new StandardTraceContextManager();
5761
traceContext.setGlobalManager(manager);
5862
});
@@ -77,4 +81,81 @@ describe("ExternalSpanExporterWrapper warm-start regression", () => {
7781
expect(span.parentSpanContext?.traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
7882
expect(span.spanContext().traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
7983
});
84+
85+
// Runs triggered internally — a schedule, or one task triggering another —
86+
// carry no external trace context and so take the generated fallback. That id
87+
// was captured at construction, which on a warm-started worker meant every run
88+
// on the process shared a single trace id.
89+
it("mints a new fallback trace id per run when there is no external context", () => {
90+
const { exporter, captured } = makeCapturingExporter();
91+
92+
let generated = 0;
93+
const idGenerator = {
94+
generateTraceId: () => `${++generated}`.padStart(32, "0"),
95+
};
96+
97+
manager.traceContext = { traceparent: TRACEPARENT_RUN_A };
98+
99+
const wrapper = new ExternalSpanExporterWrapper(
100+
exporter,
101+
"ffffffffffffffffffffffffffffffff",
102+
idGenerator
103+
);
104+
105+
wrapper.export([createAttemptSpan()], () => {});
106+
107+
// A second run on the same warm process: the manager is reassigned, so the
108+
// fallback has to be reminted.
109+
manager.traceContext = { traceparent: TRACEPARENT_RUN_B };
110+
111+
wrapper.export([createAttemptSpan()], () => {});
112+
113+
const runATraceId = captured[0]![0]!.spanContext().traceId;
114+
const runBTraceId = captured[1]![0]!.spanContext().traceId;
115+
116+
expect(runATraceId).toBe("ffffffffffffffffffffffffffffffff");
117+
expect(runBTraceId).not.toBe(runATraceId);
118+
expect(runBTraceId).toBe("00000000000000000000000000000001");
119+
});
120+
121+
it("keeps one fallback trace id across every export within a run", () => {
122+
const { exporter, captured } = makeCapturingExporter();
123+
124+
let generated = 0;
125+
const idGenerator = {
126+
generateTraceId: () => `${++generated}`.padStart(32, "0"),
127+
};
128+
129+
manager.traceContext = { traceparent: TRACEPARENT_RUN_A };
130+
131+
const wrapper = new ExternalSpanExporterWrapper(
132+
exporter,
133+
"ffffffffffffffffffffffffffffffff",
134+
idGenerator
135+
);
136+
137+
wrapper.export([createAttemptSpan()], () => {});
138+
wrapper.export([createAttemptSpan()], () => {});
139+
140+
expect(captured[1]![0]!.spanContext().traceId).toBe(captured[0]![0]!.spanContext().traceId);
141+
expect(generated).toBe(0);
142+
});
143+
144+
it("leaves external export off when no external trace id was configured", () => {
145+
const { exporter, captured } = makeCapturingExporter();
146+
147+
const idGenerator = {
148+
generateTraceId: () => "00000000000000000000000000000001",
149+
};
150+
151+
manager.traceContext = { traceparent: TRACEPARENT_RUN_A };
152+
153+
const wrapper = new ExternalSpanExporterWrapper(exporter, "", idGenerator);
154+
155+
wrapper.export([createAttemptSpan()], () => {});
156+
157+
// Minting an id here would switch external export on for a deployment that
158+
// never asked for it.
159+
expect(captured[0]).toHaveLength(0);
160+
});
80161
});

0 commit comments

Comments
 (0)