Repository navigation
fix(proxy): traceparent/baggage fallback must not override caller metadata - #43688
Conversation
…adata
The W3C traceparent/baggage fallback in
add_litellm_metadata_from_request_headers documents itself as last-resort:
Lower priority than everything above - only fires when neither the
explicit litellm headers nor the Anthropic-metadata path found anything
But it guards on the top-level `litellm_trace_id` / `litellm_session_id` body
keys and never checks `metadata`, which is the documented way callers set
trace_id / session_id (`metadata: {"trace_id": ...}` on /chat/completions).
A caller that explicitly sets metadata.trace_id has it silently replaced by the
header value, so the implementation contradicts its own stated precedence.
This is not a corner case on managed platforms: GCP's front end injects a
traceparent into every inbound request, so the fallback fires on traffic whose
caller never sent the header. The request still returns 200 and the trace still
reaches the logging backend, just under an id the caller never chose, so any
caller correlating by its own id silently fails to find its trace.
Guard both fallbacks on the caller's request-body metadata as well.
Deliberately narrow:
- x-litellm-trace-id still outranks the body (documented priority #1)
- a request that steers neither field still adopts traceparent/baggage exactly
as before
- steering is per-field: setting only trace_id still lets session_id come from
baggage
- litellm_metadata is checked too, for LITELLM_METADATA_ROUTES (/v1/responses,
/v1/messages, batches, files)
Also corrects the comment, which understated the guard.
4 new tests; each fails without the source change. The existing traceparent and
baggage tests are unchanged and still pass.
… values Addresses review. The first version treated any value in either `metadata` or `litellm_metadata` as caller steering. On LITELLM_METADATA_ROUTES the body `metadata` is provider-facing and is only promoted later, so a session_id there suppressed baggage before apply_missing_session_id_policy ran, and a request with a usable baggage session id got a 400 under `missing_session_id: reject`. An empty session_id did the same, since the policy treats "" as absent Now only the active metadata container counts, and only a truthy value, which matches how apply_missing_session_id_policy decides a session id is present. The helper takes a typed `object` rather than a bare dict Adds regression tests for both cases, including the end to end reject path, and drops the test docstrings
Addresses review. On LITELLM_METADATA_ROUTES the proxy promotes the caller's trace control fields (trace_id, session_id, ...) from `metadata` into `litellm_metadata`, but only after the header fallback runs. Checking only `litellm_metadata` let traceparent and baggage fill those fields first, and the promotion then skipped them because they were already set, so the trace was recorded under the header ids The check now also reads `metadata` for fields in LITELLM_TRACE_CONTROL_METADATA_FIELDS on those routes, and apply_missing_session_id_policy uses the same check, so `reject` no longer refuses a request whose session id is about to be promoted The earlier test asserting that a session_id in `metadata` must not block baggage on /v1/responses had the premise wrong, since that value is promoted. It is replaced by tests that assert the promoted caller ids win on /v1/responses and /v1/messages, with no policy, reject, and generate
…oted one Addresses review. Promotion copies a trace control field from `metadata` into `litellm_metadata` only when the key is absent, so a key that is present but empty in `litellm_metadata` wins and the `metadata` value is never used. The check still looked at `metadata` in that case, which let `missing_session_id: reject` accept a request that ends up with an empty session id, and let an empty trace_id or session_id block the headers The check now follows the same rule as promotion. If the active container has the key, its value decides. Only when the key is absent does the promoted `metadata` value count. This makes the conflicting-bucket cases behave exactly as on main
The type-discipline gate failed: this branch added 4 unsuppressed `cast()` calls and LIT006 was already at its limit. Each cast now carries a `# cast-ok` reason. The two in the helper follow an `isinstance` check that proves the Mapping. The two at the call sites stay because the method's `data` parameter is a bare `dict`, and dropping those casts adds basedpyright unknown-type errors instead. No behavior change
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…cedence upstreams Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ller values The _caller_trace_field gating added for caller-metadata precedence was presence-aware, which regressed three pre-call behaviors versus main: - Equal ids in W3C headers and body metadata suppressed the traceparent/baggage branch entirely, leaving the root litellm_trace_id/litellm_session_id unset so router fallbacks minted a fresh uuid4 per request. The header branch now also fires when the caller value equals the header-derived id, so equal ids stamp the root fields exactly like main. - A truthy but non-string metadata value (e.g. session_id 4815162342) counted as caller-supplied and suppressed the baggage fallback, so downstream str-only consumers (code interpreter sandbox reuse) got a new sandbox per turn. _caller_trace_field now counts only non-empty string values, and the generate policy falls through to generation when the caller value is not usable. - On litellm_metadata routes a usable caller session id suppressed the missing_session_id policies while never landing on the root field, so the root session stayed unset. The policy now promotes the caller's usable session id to litellm_session_id instead of leaving it stranded.
…allback_precedence Main renamed tests/test_litellm/proxy/test_litellm_pre_call_utils.py to tests/unit/proxy/test_litellm_pre_call_utils.py; the branch's test additions (caller-metadata precedence steering plus the equal-ids, agentic-route generate and non-string session regression cases) carry onto the new path.
|
@greptileai review latest head |
|
bugbot run |
|
@greptileai please review the current head 7855fde (regression-fix commit 99cf509 plus a merge of main that only resolves the test-path rename) |
|
bugbot run |
The audit cell compared a headers leg against a headerless leg for equality, which only held while a truthy non-string id suppressed the W3C fallback on both legs. With unusable values ignored again, the headers leg resolves to the header ids while the headerless leg cannot, so assert the headers leg's concrete outcome instead, mirroring the empty/null cell.
|
@greptileai please review the current head 6b4e319 — the audit cells you flagged are rewritten in test_audit_non_string_caller_ids_fall_back_to_w3c: non-string ids now assert the concrete W3C-header outcome (mirroring the empty/null cell) instead of comparing a headers leg against a headerless leg |
|
bugbot run |
|
@greptileai please review the current head 250b1a5 |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 250b1a5. Configure here.
REVIEWER MUST KNOW BEFORE APPROVING
Review loop converged at
250b1a50d2: Greptile 5/5 with no findings (the P1 audit-cell finding fixed in6b4e3198cf, the P2 comment finding in250b1a50d2), Bugbot clean, no veria on this repo, all review threads resolvedFollow-up fix
99cf509980repairs three P2 regressions the first cut introduced versus the merge base, found by a fresh base-vs-head validation run on 2026-10-04: equal ids in headers and metadata now stamp the root ids again (spend session had become a fresh uuid4 per call), a non-stringmetadata.session_idno longer suppresses thebaggagefallback (sandbox-reuse consumers need a str), and a usable caller session on Responses/Messages now lands on the root session field instead of leaving it unset. All steering tests from the first cut still passCarries @fandujar's fix from fix(proxy): traceparent/baggage fallback must not override caller metadata #43020 unchanged (their 6 commits keep their authorship), plus a merge of
main, test-only commits (incl. the audit cells below), and one follow-up fix somissing_session_id: generateon Responses and Messages derives the session from the caller'smetadata.trace_id(found by Greptile, confirmed live)Moved to an internal
litellm_branch so the CircleCI integration shards run on it; fix(proxy): traceparent/baggage fallback must not override caller metadata #43020 gets closed as superseded with a pointer hereA caller that sets
metadata.trace_id/metadata.session_idnow keeps them overtraceparent/baggage: intended, it is the ticket, but it changes which id wins for those callersTLDR
Problem this solves:
metadata.trace_idgets replaced by thetraceparentheader's trace idmetadata.session_idand thebaggageheader, on chat, Responses and Messagesmissing_session_id: reject, a Responses or Messages call withmetadata.session_idgets a 400How it solves it:
metadatacount as caller setmissing_session_id: generate, a Responses or Messages call that only setsmetadata.trace_idgets that trace as its session id, same as chat, instead of a fresh uuid per callIntentional product change: callers who send their own trace or session id through
metadatanow see those in Langfuse, spend logs and session-based routing instead of the W3C header idsUser Flow
Before: a caller behind a load balancer that injects W3C trace headers loses the session id they set
"metadata": {"trace_id": "my-trace-id", "session_id": "my-session-id"}traceparentandbaggage: session.id=baggage-session-42, and the call comes back 200baggage-session-42, not theirsmissing_session_id: reject, the same /v1/responses or /v1/messages call withoutbaggagecomes back 400 "Request has no session id"After: the same requests keep the ids the caller chose
my-session-idunder the trace formy-trace-idmissing_session_id: reject, the /v1/responses or /v1/messages call withoutbaggagecomes back 200; a call with no session id anywhere is still 400Relevant issues
Supersedes #43020 (fix by @fandujar)
Affected release
Regression in v1.98.0, where #34418 added the
traceparent/baggagefallback without checking caller metadata. Confirmed live on v1.97.1, v1.98.0 and this head, see Regression check belowLinear ticket
Resolves LIT-8561
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Latest run at head
250b1a50d2(audit pass, 2026-10-04). Real OpenAIgpt-4o-minion every call, real Postgres per proxy, real Redis, Langfuse OTLP export into a local wire-faithful OTLP recorder sink (disclosed substitute for a cloud Langfuse: same OTLP/HTTP protocol, records every exported span). Four proxies from the tip: default policy withrequest_correlation_in_logs: true, correlation off,missing_session_id: generate, andmissing_session_id: reject. Spend rows are read back per request id; OTLP spans per session attribute.Before (4d3940c, prior head; the merge base 85dc7cb already behaved like After)
equal ids in headers and metadata (chat)
curl /v1/chat/completionswithtraceparent: 00-4bf92f3577b34da6a3ce929d0e0e4736-...+baggage: session.id=matching-session-42and"metadata":{"trace_id":"4bf92f3577b34da6a3ce929d0e0e4736","session_id":"matching-session-42"}HTTP 200; pre-call probe: rootlitellm_trace_id=None, rootlitellm_session_id=None4bf92f3577b34da6a3ce929d0e0e4736)non-string metadata session_id with baggage (chat)
baggage: session.id=header-session-42and"metadata":{"session_id":4815162342}HTTP 200; probe: rootlitellm_session_id=None(merge base:header-session-42),metadata.session_idstays the intheader-free generate on /v1/responses (missing_session_id: generate)
curl /v1/responseswith no trace headers, body"litellm_trace_id":"root-trace-42","metadata":{"session_id":"caller-session-42"}HTTP 200; probe: rootlitellm_session_id=None(merge base and the fix: set)After (250b1a5)
equal ids in headers and metadata (chat, and /v1/responses)
HTTP 200; spend log session4bf92f3577b34da6a3ce929d0e0e4736on both endpoints, byte-identical to the merge-base legs; the header-only control matches on all sides; OTLP span trace4bf92f35..., sessionmatching-session-42non-string metadata session_id with baggage (chat; also list, bool, nested object in unit tests)
HTTP 200; spend session4bf92f35...(trace preferred with correlation on), OTLP span sessionbaggage-session-audit- the baggage fallback fires again, exactly like the merge base; undergeneratethe unusable value falls through to generation (rootroot-trace-42, marked generated)header-free generate on /v1/responses (missing_session_id: generate)
HTTP 200; rootlitellm_session_id=caller-session-42(the caller's own id, so session consumers keep one stable id), spend sessionroot-trace-42, OTLP sessioncaller-session-42; a generate call with onlymetadata.trace_idstill derives the session from the caller trace (caller-trace-42, kept from the first cut)full audit matrix at the tip (all
HTTP 200unless stated, one real provider call per cell)traceparentis ignored whilebaggagestill supplies the sessiongenerate(caller session, caller trace, and no ids at all),reject(caller session 200, no ids 400 with the session message in the body, baggage session 200)se-*and baggagebag-session-auditland in the spend session column directly (session-first ordering), equal ids landmatch-sessintegration suite at the tip
tests/integration/observability/test_langfuse_delivery.py: 92 passed locally at250b1a50d2against the real rig (own proxy per module, scripted upstream, OTLP sink, Postgres, Redis), after the audit cells were fixed in6b4e3198cf(the six non-string cells pinned the old suppression and were rewritten to assert the concrete header outcome; verified failing on the prior head, passing on the fix). CircleCI owns this suite in CI; its pipelines have been stalled org-wide since ~09:30 today (five unrelated PRs pushed after that also show zero CircleCI statuses), so the branch's CircleCI columns are empty - GitHub Actions is fully green at250b1a50d2and the local run above is the substitute until the org queue recoversUI evidence
Bundled dashboard at the audited proxy (login admin + master key), Logs page showing the audited requests under the W3C-correlated trace/session, and the request detail:
Animated flow (4 steps):
Full screen recording of the flow (login, Logs, request detail): audit_recording.webm
Type
🐛 Bug Fix
✅ Test
Caveats (if any)
Low
traceparent, Langfuse follows the caller'strace_idmetadata.session_id(e.g.456) never counts as caller-supplied:baggagewins when present, andgeneratederives from the trace otherwise (merge-base behavior, pinned by99cf509980)Final Attestation
Link to Devin session: https://app.devin.ai/sessions/e9f90c2f75c14ded84f606aa00756857
Open in Devin Desktop: https://app.devin.ai/desktop/session/e9f90c2f75c14ded84f606aa00756857?variant=devin
Requested by: @yucheng-berri