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); + }); });