fix(fetch): preserve explicit trace context during propagation - #733
fix(fetch): preserve explicit trace context during propagation#733SamP231004 wants to merge 1 commit into
Conversation
|
Hey @SamP231004, I took a look at this PR while checking the same issue. The core change in I opened a cleaner, minimal version with a single focused integration test in #734 if you'd like to take a look. |
|
Thanks for reviewing this @mandar1045 . That’s fair—the broader coverage helped verify the behavior, but I should have trimmed the permutations and SDK dependency before opening the PR. I looked at #734. The smaller test is easier to review. One detail: the implementation keeps the same propagator guard as #733, while the description says it changes I’ll simplify #733 to a focused regression test while retaining a small check that automatic propagation still works. |
|
ahh!, yes i've updated it |
When an Edge Function supplies
traceparenttofetch()while runtime tracing is enabled, the runtime appends its own context to the same header. The callee receives two comma-separated contexts, which is not a valid W3Ctraceparent. This change preserves explicitly supplied trace context while retaining automatic trace propagation for requests without it.Related to supabase/supabase#50006.
Reproduction and cause
The new integration fixture runs a caller and callee as real user workers, with tracing enabled through
otelConfig. The caller uses an explicit trace ID different from the incoming request's active trace and invokes the callee throughfetch(url, options),fetch(Request), andsupabase.functions.invoke()using@supabase/supabase-js@2.112.3.With the original runtime code, the traced regression fails because the callee receives:
The expected value is the caller's original header:
The SDK preserves the supplied headers. The duplication occurs in the instrumented runtime
fetch(), whose propagator setter callsHeaders.append().Change
Before injecting a propagator that owns
traceparent, check whether the request already contains that header. If so, skip that propagator. Header lookup is case-insensitive.Skipping the trace-context propagator preserves
traceparentand its associatedtracestatetogether, including an empty or absenttracestate. It also prevents state from a different active trace being attached to an explicit caller context. Other propagators remain eligible, and requests without an explicittraceparentcontinue to receive runtime-generated context.Regression coverage
Two native integration tests exercise traced and untraced callers. Each checks all three invocation methods with present, empty, and absent
tracestate, plus a request without explicit context. Assertions cover:traceparent, including mixed-case input header names.tracestate.Validation
Verified locally on Ubuntu 22.04 under WSL with the repository's Rust 1.98.0 toolchain and ONNX Runtime 1.20.1:
git diff --checkpass.These are targeted tests; the full repository test suite was not run.
Hosted verification
The native tests establish the duplicate-injection defect and its fix. They do not establish the reporter's deployed runtime configuration or prove that a hosted gateway replaces the malformed header with a new trace ID. Hosted verification and rollout remain necessary for the linked issue. Automatic inheritance of inbound baggage/tracestate into runtime spans is outside this change.