fix: Fix OTEL trace context propagation race - #2450
Merged
Merged
Conversation
David Elner (delner)
requested review from
Luca Forstner (lforst) and
Andrew Kent (realark)
September 9, 2026 14:34
Luca Forstner (lforst)
marked this pull request as ready for review
September 10, 2026 13:59
Luca Forstner (lforst)
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Span.inject()/injectTraceContext()are synchronous, but the experiment id they need is not._getOtelParent()builds thebraintrust.parentbaggage entry fromparentObjectId.getSync(), which staysundefineduntilPOST /api/experiment/registerreturns. An inject that happens before then emits atraceparentwith nobaggage: trace identity with no destination.The failure lands in a different process. The receiving side warns ("Received traceparent without a braintrust.parent ... Starting a fresh local span instead") and starts a fresh local trace, so spans from a subprocess agent land in project logs instead of the experiment. Discovery happens while debugging the consumer, not the producer. It is also first-span-only: the id caches once resolved, so it breaks the first turn of a run and then quietly stops, which is how it ships broken and goes unnoticed.
Why this fixes it
The precondition belongs at experiment init, not in the injector, so the race becomes structurally impossible rather than narrower:
Eval()already awaitedexperiment._waitForId(), but only whenglobalThis.BRAINTRUST_CONTEXT_MANAGER !== undefined(the opt-in@braintrust/otelintegration). That gate was correct when written and stopped being correct onceinjectstarted depending on the same resolved id. Ungating it means no span can exist before the id does.inject()stays synchronous, matching the OTel propagator API, which is sync by spec.SpanImpl.inject()now warns when it has nobraintrust.parent, symmetric with the receive-side warning. It does not prevent the failure, but it moves discovery to the process that caused it, which matters for the paths eager resolution does not cover.js/src/framework.tsawait experiment._waitForId()inEval()js/src/logger.tsSpanImpl.inject()whenbraintrustParentis emptyTesting
js/src/framework.test.ts:injectTraceContext()inside the firstEval()task must carrybraintrust.parent. Verified against the old gate, where it fails withexpected undefined to be 'experiment_id:experiment-id'for every row, not just the first: concurrent tasks all inject before the first flush resolves the id.js/src/propagation.test.ts: unresolved experiment id warns and omits the parent; resolved id injectsexperiment_id:<id>.trace-context-and-continuation,trace-primitives-basic,test-framework-evals-node,otel-compat-mixed-tracing,durable-eval-webhookpass, run twice.Notes
_get_parent_info()already falls back to a blockingparent_object_id.get()for experiments, and Python'sLazyValue.get()computes on the calling thread, so a syncinject()there resolves the id rather than dropping it.init()plusstartSpan()plus inject is still racy, sinceinit()is fully lazy andlazyIdis not kicked off until the first flush. It now warns instead of failing silently. Closing it would mean either firinglazyId.get()fromExperiment.startSpan(narrows the window, does not close it) or documentingawait experiment.idbefore injecting.