Skip to content

Commit c043c4a

Browse files
nicktrnnicktrn
authored
fix(core): external trace context leaks across warm-started runs (#3768)
On warm-started workers with `processKeepAlive` enabled, every run's attempt span was exported with the first run's `traceId` and `parentSpanId`. `ExternalSpanExporterWrapper` and `ExternalLogRecordExporterWrapper` captured `externalTraceContext` at `TracingSDK` construction, and the SDK is memoized for the worker's lifetime - so the per-run reset of `StandardTraceContextManager.traceContext` never reached the wrappers. Reported by a customer running v4.4.x: 33 distinct runs on the same host/pid showed up in their APM as siblings of one parent span. Fix: the wrappers now read external context live from the trace context manager per export. Runs without an external trace context fall through to the unchanged `externalTraceId` fallback - no behaviour change for them. Regression test in `packages/core/test/externalSpanExporterWrapper.test.ts` asserts that reassigning the manager between exports produces correctly-parented spans.
1 parent 816986d commit c043c4a

3 files changed

Lines changed: 132 additions & 49 deletions

File tree

‎.changeset/warm-start-external-trace-context-leak.md‎

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+
Fix external trace context leaking across runs on warm-started workers with `processKeepAlive` enabled. Every subsequent run's attempt span was being exported with the first run's `traceId` and `parentSpanId`, breaking causal-chain navigation in external APM tools. Runs without an external trace context are unaffected.

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

Lines changed: 44 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,12 @@ export class TracingSDK {
171171
);
172172

173173
const externalTraceId = idGenerator.generateTraceId();
174-
const externalTraceContext = traceContext.getExternalTraceContext();
175174

176175
for (const exporter of config.exporters ?? []) {
177176
spanProcessors.push(
178177
getEnvVar("TRIGGER_OTEL_BATCH_PROCESSING_ENABLED") === "1"
179178
? new BatchSpanProcessor(
180-
new ExternalSpanExporterWrapper(exporter, externalTraceId, externalTraceContext),
179+
new ExternalSpanExporterWrapper(exporter, externalTraceId),
181180
{
182181
maxExportBatchSize: parseInt(
183182
getEnvVar("TRIGGER_OTEL_SPAN_MAX_EXPORT_BATCH_SIZE") ?? "64"
@@ -192,7 +191,7 @@ export class TracingSDK {
192191
}
193192
)
194193
: new SimpleSpanProcessor(
195-
new ExternalSpanExporterWrapper(exporter, externalTraceId, externalTraceContext)
194+
new ExternalSpanExporterWrapper(exporter, externalTraceId)
196195
)
197196
);
198197
}
@@ -245,11 +244,7 @@ export class TracingSDK {
245244
logProcessors.push(
246245
getEnvVar("TRIGGER_OTEL_BATCH_PROCESSING_ENABLED") === "1"
247246
? new BatchLogRecordProcessor(
248-
new ExternalLogRecordExporterWrapper(
249-
externalLogExporter,
250-
externalTraceId,
251-
externalTraceContext
252-
),
247+
new ExternalLogRecordExporterWrapper(externalLogExporter, externalTraceId),
253248
{
254249
maxExportBatchSize: parseInt(
255250
getEnvVar("TRIGGER_OTEL_LOG_MAX_EXPORT_BATCH_SIZE") ?? "64"
@@ -264,11 +259,7 @@ export class TracingSDK {
264259
}
265260
)
266261
: new SimpleLogRecordProcessor(
267-
new ExternalLogRecordExporterWrapper(
268-
externalLogExporter,
269-
externalTraceId,
270-
externalTraceContext
271-
)
262+
new ExternalLogRecordExporterWrapper(externalLogExporter, externalTraceId)
272263
)
273264
);
274265
}
@@ -417,32 +408,32 @@ function setLogLevel(level: TracingDiagnosticLogLevel) {
417408
diag.setLogger(new DiagConsoleLogger(), diagLogLevel);
418409
}
419410

420-
class ExternalSpanExporterWrapper {
421-
private readonly _isExternallySampled: boolean;
422-
411+
export class ExternalSpanExporterWrapper {
423412
constructor(
424413
private underlyingExporter: SpanExporter,
425-
private externalTraceId: string,
426-
private externalTraceContext:
427-
| { traceId: string; spanId: string; traceFlags: number; tracestate?: string }
428-
| undefined
429-
) {
430-
this._isExternallySampled = externalTraceContext
431-
? isTraceFlagSampled(externalTraceContext.traceFlags)
432-
: !!externalTraceId;
433-
}
414+
private externalTraceId: string
415+
) {}
434416

435417
private transformSpan(span: ReadableSpan): ReadableSpan | undefined {
436-
if (!this._isExternallySampled) {
418+
// Read external context live, so per-run reassignment of
419+
// standardTraceContextManager.traceContext is honoured on warm-started
420+
// workers that reuse a single TracingSDK across runs.
421+
const externalTraceContext = traceContext.getExternalTraceContext();
422+
423+
const isExternallySampled = externalTraceContext
424+
? isTraceFlagSampled(externalTraceContext.traceFlags)
425+
: !!this.externalTraceId;
426+
427+
if (!isExternallySampled) {
437428
return;
438429
}
439430

440431
if (isSpanInternalOnly(span)) {
441432
return;
442433
}
443434

444-
const externalTraceId = this.externalTraceContext
445-
? this.externalTraceContext.traceId
435+
const externalTraceId = externalTraceContext
436+
? externalTraceContext.traceId
446437
: this.externalTraceId;
447438

448439
const isAttemptSpan = span.attributes[SemanticInternalAttributes.SPAN_ATTEMPT];
@@ -457,15 +448,15 @@ class ExternalSpanExporterWrapper {
457448
};
458449
}
459450

460-
if (isAttemptSpan && this.externalTraceContext) {
451+
if (isAttemptSpan && externalTraceContext) {
461452
parentSpanContext = {
462453
...parentSpanContext,
463454
traceId: externalTraceId,
464-
spanId: this.externalTraceContext.spanId,
465-
traceState: this.externalTraceContext.tracestate
466-
? new TraceState(this.externalTraceContext.tracestate)
455+
spanId: externalTraceContext.spanId,
456+
traceState: externalTraceContext.tracestate
457+
? new TraceState(externalTraceContext.tracestate)
467458
: undefined,
468-
traceFlags: this.externalTraceContext.traceFlags,
459+
traceFlags: externalTraceContext.traceFlags,
469460
};
470461
} else if (isAttemptSpan) {
471462
parentSpanContext = undefined;
@@ -502,28 +493,27 @@ class ExternalSpanExporterWrapper {
502493
}
503494

504495
class ExternalLogRecordExporterWrapper {
505-
private readonly _isExternallySampled: boolean;
506-
507496
constructor(
508497
private underlyingExporter: LogRecordExporter,
509-
private externalTraceId: string,
510-
private externalTraceContext:
511-
| { traceId: string; spanId: string; tracestate?: string; traceFlags: number }
512-
| undefined
513-
) {
514-
this._isExternallySampled = externalTraceContext
515-
? isTraceFlagSampled(externalTraceContext.traceFlags)
516-
: !!externalTraceId;
517-
}
498+
private externalTraceId: string
499+
) {}
518500

519501
export(logs: any[], resultCallback: (result: any) => void): void {
520-
if (!this._isExternallySampled) {
502+
const externalTraceContext = traceContext.getExternalTraceContext();
503+
504+
const isExternallySampled = externalTraceContext
505+
? isTraceFlagSampled(externalTraceContext.traceFlags)
506+
: !!this.externalTraceId;
507+
508+
if (!isExternallySampled) {
521509
this.underlyingExporter.export([], resultCallback);
522510

523511
return;
524512
}
525513

526-
const modifiedLogs = logs.map(this.transformLogRecord.bind(this));
514+
const modifiedLogs = logs.map((log) =>
515+
this.transformLogRecord(log, externalTraceContext)
516+
);
527517

528518
this.underlyingExporter.export(modifiedLogs, resultCallback);
529519
}
@@ -532,11 +522,16 @@ class ExternalLogRecordExporterWrapper {
532522
return this.underlyingExporter.shutdown();
533523
}
534524

535-
transformLogRecord(logRecord: ReadableLogRecord): ReadableLogRecord {
525+
transformLogRecord(
526+
logRecord: ReadableLogRecord,
527+
externalTraceContext:
528+
| { traceId: string; spanId: string; tracestate?: string; traceFlags: number }
529+
| undefined
530+
): ReadableLogRecord {
536531
// Capture externalTraceId for use within the proxy's scope.
537532
// Use externalTraceContext.traceId if available, otherwise fall back to generated externalTraceId
538-
const externalTraceId = this.externalTraceContext
539-
? this.externalTraceContext.traceId
533+
const externalTraceId = externalTraceContext
534+
? externalTraceContext.traceId
540535
: this.externalTraceId;
541536

542537
// If there's no spanContext, or if the externalTraceId is not set, return the original logRecord.

‎packages/core/test/externalSpanExporterWrapper.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { SpanKind, SpanStatusCode, TraceFlags } from "@opentelemetry/api";
2+
import type { ReadableSpan, SpanExporter } from "@opentelemetry/sdk-trace-node";
3+
import { beforeEach, describe, expect, it } from "vitest";
4+
import { ExternalSpanExporterWrapper } from "../src/v3/otel/tracingSDK.js";
5+
import { SemanticInternalAttributes } from "../src/v3/semanticInternalAttributes.js";
6+
import { traceContext } from "../src/v3/trace-context-api.js";
7+
import { StandardTraceContextManager } from "../src/v3/traceContext/manager.js";
8+
9+
const TRACEPARENT_RUN_A = "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-1111111111111111-01";
10+
const TRACEPARENT_RUN_B = "00-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-2222222222222222-01";
11+
12+
function createAttemptSpan(): ReadableSpan {
13+
const spanCtx = {
14+
traceId: "cccccccccccccccccccccccccccccccc",
15+
spanId: "3333333333333333",
16+
traceFlags: TraceFlags.SAMPLED,
17+
};
18+
return {
19+
name: "Attempt 1",
20+
kind: SpanKind.CONSUMER,
21+
spanContext: () => spanCtx,
22+
parentSpanContext: undefined,
23+
startTime: [0, 0],
24+
endTime: [0, 0],
25+
status: { code: SpanStatusCode.UNSET },
26+
attributes: { [SemanticInternalAttributes.SPAN_ATTEMPT]: true },
27+
links: [],
28+
events: [],
29+
duration: [0, 0],
30+
ended: true,
31+
resource: {} as any,
32+
instrumentationLibrary: { name: "test" } as any,
33+
droppedAttributesCount: 0,
34+
droppedEventsCount: 0,
35+
droppedLinksCount: 0,
36+
} as unknown as ReadableSpan;
37+
}
38+
39+
function makeCapturingExporter(): { exporter: SpanExporter; captured: ReadableSpan[][] } {
40+
const captured: ReadableSpan[][] = [];
41+
const exporter: SpanExporter = {
42+
export: (spans, cb) => {
43+
captured.push(spans);
44+
cb({ code: 0 } as any);
45+
},
46+
shutdown: () => Promise.resolve(),
47+
forceFlush: () => Promise.resolve(),
48+
};
49+
return { exporter, captured };
50+
}
51+
52+
describe("ExternalSpanExporterWrapper warm-start regression", () => {
53+
let manager: StandardTraceContextManager;
54+
55+
beforeEach(() => {
56+
manager = new StandardTraceContextManager();
57+
traceContext.setGlobalManager(manager);
58+
});
59+
60+
it("rewrites attempt spans using the manager's current external context, not the value captured at construction", () => {
61+
const { exporter, captured } = makeCapturingExporter();
62+
63+
manager.traceContext = { external: { traceparent: TRACEPARENT_RUN_A } };
64+
65+
const wrapper = new ExternalSpanExporterWrapper(
66+
exporter,
67+
"ffffffffffffffffffffffffffffffff"
68+
);
69+
70+
manager.traceContext = { external: { traceparent: TRACEPARENT_RUN_B } };
71+
72+
wrapper.export([createAttemptSpan()], () => {});
73+
74+
expect(captured).toHaveLength(1);
75+
expect(captured[0]).toHaveLength(1);
76+
77+
const span = captured[0]![0]!;
78+
expect(span.parentSpanContext?.spanId).toBe("2222222222222222");
79+
expect(span.parentSpanContext?.spanId).not.toBe("1111111111111111");
80+
expect(span.parentSpanContext?.traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
81+
expect(span.spanContext().traceId).toBe("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
82+
});
83+
});

0 commit comments

Comments
 (0)