Skip to content

Commit f837ccc

Browse files
NERLOEclaude
andcommitted
refactor(core): let the trace context manager mark the run boundary
Detecting the run boundary by reference-identity of the trace context was a heuristic, and guarding it against the noop manager's fresh `{}` meant testing the context for emptiness. That traded an unreachable bug for a reachable one: `traceContext` is `z.record(z.unknown())`, so a run whose context is empty would stop reminting and silently merge back into the previous run's trace. Replace the inference with a fact. `StandardTraceContextManager.traceContext` becomes an accessor pair that advances an epoch whenever the context is replaced, which is exactly what starting a run does, so no call site changes. The noop manager reports a constant epoch, so with no manager registered there are no boundaries to react to and nothing churns. Drops the emptiness heuristic entirely, and covers the case it would have broken. Also renames `get()` to `forCurrentRun()` and the shared instance to `fallbackTraceId`, since the old name read as a string, and exports the log wrapper so a test can prove a run's spans and logs stay on one id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 81cf2a5 commit f837ccc

5 files changed

Lines changed: 145 additions & 84 deletions

File tree

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

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -163,12 +163,12 @@ export class TracingSDK {
163163
);
164164

165165
// Shared by every wrapper below so a run's spans and logs agree on the id.
166-
const externalTraceId = new FallbackExternalTraceId(idGenerator.generateTraceId());
166+
const fallbackTraceId = new FallbackExternalTraceId(idGenerator.generateTraceId());
167167

168168
for (const exporter of config.exporters ?? []) {
169169
spanProcessors.push(
170170
getEnvVar("TRIGGER_OTEL_BATCH_PROCESSING_ENABLED") === "1"
171-
? new BatchSpanProcessor(new ExternalSpanExporterWrapper(exporter, externalTraceId), {
171+
? new BatchSpanProcessor(new ExternalSpanExporterWrapper(exporter, fallbackTraceId), {
172172
maxExportBatchSize: parseInt(
173173
getEnvVar("TRIGGER_OTEL_SPAN_MAX_EXPORT_BATCH_SIZE") ?? "64"
174174
),
@@ -180,7 +180,7 @@ export class TracingSDK {
180180
),
181181
maxQueueSize: parseInt(getEnvVar("TRIGGER_OTEL_SPAN_MAX_QUEUE_SIZE") ?? "512"),
182182
})
183-
: new SimpleSpanProcessor(new ExternalSpanExporterWrapper(exporter, externalTraceId))
183+
: new SimpleSpanProcessor(new ExternalSpanExporterWrapper(exporter, fallbackTraceId))
184184
);
185185
}
186186

@@ -232,7 +232,7 @@ export class TracingSDK {
232232
logProcessors.push(
233233
getEnvVar("TRIGGER_OTEL_BATCH_PROCESSING_ENABLED") === "1"
234234
? new BatchLogRecordProcessor(
235-
new ExternalLogRecordExporterWrapper(externalLogExporter, externalTraceId),
235+
new ExternalLogRecordExporterWrapper(externalLogExporter, fallbackTraceId),
236236
{
237237
maxExportBatchSize: parseInt(
238238
getEnvVar("TRIGGER_OTEL_LOG_MAX_EXPORT_BATCH_SIZE") ?? "64"
@@ -247,7 +247,7 @@ export class TracingSDK {
247247
}
248248
)
249249
: new SimpleLogRecordProcessor(
250-
new ExternalLogRecordExporterWrapper(externalLogExporter, externalTraceId)
250+
new ExternalLogRecordExporterWrapper(externalLogExporter, fallbackTraceId)
251251
)
252252
);
253253
}
@@ -394,55 +394,41 @@ function setLogLevel(level: TracingDiagnosticLogLevel) {
394394
diag.setLogger(new DiagConsoleLogger(), diagLogLevel);
395395
}
396396

397-
/**
398-
* Identity of the current run's trace context, or undefined when no run is
399-
* active.
400-
*
401-
* An empty context is not a run: the noop manager returns a fresh `{}` on every
402-
* call, so treating it as a run would mint a new id on every export.
403-
*/
404-
function currentRunTraceContext(): object | undefined {
405-
const current = traceContext.getTraceContext();
406-
407-
return current && Object.keys(current).length > 0 ? current : undefined;
408-
}
409-
410397
/**
411398
* The external trace id used by runs that carry no external trace context,
412399
* minted once per run.
413400
*
414401
* It has to change per run for the same reason the wrappers read the external
415402
* context live: with `processKeepAlive` the `TracingSDK` — and so the wrappers
416403
* — outlive the run, so an id captured at construction merges every run on the
417-
* process into one trace. The manager's trace context object is reassigned per
418-
* run, which makes its identity the run boundary.
404+
* process into one trace.
419405
*
420406
* One instance is shared by every wrapper, so a run's spans and logs still
421407
* agree on the id after a remint.
422408
*/
423409
export class FallbackExternalTraceId {
424410
private traceId: string;
425-
private seenTraceContext: object | undefined;
411+
private seenEpoch: number;
426412

427413
constructor(
428414
private seed: string,
429415
private traceIdGenerator: Pick<RandomIdGenerator, "generateTraceId"> = idGenerator
430416
) {
431417
this.traceId = seed;
432-
this.seenTraceContext = currentRunTraceContext();
418+
this.seenEpoch = traceContext.getTraceContextEpoch();
433419
}
434420

435-
get(): string {
421+
forCurrentRun(): string {
436422
// An empty seed means external export is disabled — leave it that way
437423
// rather than minting an id and switching the feature on.
438424
if (!this.seed) {
439425
return this.seed;
440426
}
441427

442-
const current = currentRunTraceContext();
428+
const epoch = traceContext.getTraceContextEpoch();
443429

444-
if (current && current !== this.seenTraceContext) {
445-
this.seenTraceContext = current;
430+
if (epoch !== this.seenEpoch) {
431+
this.seenEpoch = epoch;
446432
this.traceId = this.traceIdGenerator.generateTraceId();
447433
}
448434

@@ -461,7 +447,7 @@ export class ExternalSpanExporterWrapper {
461447
// standardTraceContextManager.traceContext is honoured on warm-started
462448
// workers that reuse a single TracingSDK across runs.
463449
const externalTraceContext = traceContext.getExternalTraceContext();
464-
const fallbackTraceId = this.fallback.get();
450+
const fallbackTraceId = this.fallback.forCurrentRun();
465451

466452
const isExternallySampled = externalTraceContext
467453
? isTraceFlagSampled(externalTraceContext.traceFlags)
@@ -533,15 +519,15 @@ export class ExternalSpanExporterWrapper {
533519
}
534520
}
535521

536-
class ExternalLogRecordExporterWrapper {
522+
export class ExternalLogRecordExporterWrapper {
537523
constructor(
538524
private underlyingExporter: LogRecordExporter,
539525
private fallback: FallbackExternalTraceId
540526
) {}
541527

542528
export(logs: any[], resultCallback: (result: any) => void): void {
543529
const externalTraceContext = traceContext.getExternalTraceContext();
544-
const fallbackTraceId = this.fallback.get();
530+
const fallbackTraceId = this.fallback.forCurrentRun();
545531

546532
const isExternallySampled = externalTraceContext
547533
? isTraceFlagSampled(externalTraceContext.traceFlags)

packages/core/src/v3/traceContext/api.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ class NoopTraceContextManager implements TraceContextManager {
1010
return {};
1111
}
1212

13+
// Never advances: with no manager registered there are no runs to separate.
14+
getTraceContextEpoch() {
15+
return 0;
16+
}
17+
1318
reset() {}
1419

1520
getExternalTraceContext() {
@@ -57,6 +62,10 @@ export class TraceContextAPI implements TraceContextManager {
5762
return this.#getManager().getTraceContext();
5863
}
5964

65+
public getTraceContextEpoch() {
66+
return this.#getManager().getTraceContextEpoch();
67+
}
68+
6069
public getExternalTraceContext() {
6170
return this.#getManager().getExternalTraceContext();
6271
}

packages/core/src/v3/traceContext/manager.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,29 @@ import { parseTraceParent } from "@opentelemetry/core";
44
import type { TraceContextManager } from "./types.js";
55

66
export class StandardTraceContextManager implements TraceContextManager {
7-
public traceContext: Record<string, unknown> = {};
7+
#traceContext: Record<string, unknown> = {};
8+
#epoch = 0;
9+
10+
// An accessor rather than a plain field so that replacing the context, which
11+
// is what starting a run does, is what advances the epoch. Call sites are
12+
// unchanged.
13+
get traceContext(): Record<string, unknown> {
14+
return this.#traceContext;
15+
}
16+
17+
set traceContext(value: Record<string, unknown>) {
18+
this.#traceContext = value;
19+
this.#epoch++;
20+
}
821

922
getTraceContext() {
1023
return this.traceContext;
1124
}
1225

26+
getTraceContextEpoch() {
27+
return this.#epoch;
28+
}
29+
1330
reset() {
1431
this.traceContext = {};
1532
}

packages/core/src/v3/traceContext/types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@ import type { Context } from "@opentelemetry/api";
22

33
export interface TraceContextManager {
44
getTraceContext(): Record<string, unknown>;
5+
/**
6+
* Increments every time the trace context is replaced, which on a worker that
7+
* reuses one process across runs is the run boundary. Long-lived consumers
8+
* compare it to tell "still the same run" from "a new run started".
9+
*/
10+
getTraceContextEpoch(): number;
511
extractContext(): Context;
612
reset(): void;
713
getExternalTraceContext():

0 commit comments

Comments
 (0)