From a1dcd802adf89a0425881bd42be9107f69232a54 Mon Sep 17 00:00:00 2001 From: Filipe Andujar Date: Thu, 24 Sep 2026 15:48:29 -0300 Subject: [PATCH] fix(proxy): traceparent/baggage fallback must not override caller metadata 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. --- litellm/proxy/litellm_pre_call_utils.py | 24 ++++++-- .../proxy/test_litellm_pre_call_utils.py | 55 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 8fc5faee2c9..60748234873 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -150,6 +150,21 @@ def _session_id_from_baggage(baggage: str) -> str | None: return None +def _client_steered_trace_field(data: dict, field: str) -> bool: + """Did the caller explicitly steer ``field`` in the request body? + + ``metadata`` (``litellm_metadata`` on ``LITELLM_METADATA_ROUTES``) is the + documented way to set ``trace_id`` / ``session_id``, so a value there is an + explicit caller choice and must outrank the traceparent/baggage fallback + below, which only guards on the top-level ``litellm_*`` keys. + """ + for variable_name in ("metadata", "litellm_metadata"): + container = data.get(variable_name) + if isinstance(container, dict) and container.get(field) is not None: + return True + return False + + def _stampable_key_hash(user_api_key_dict: UserAPIKeyAuth) -> str | None: """Only proxy-validated keys are stamped, proven by the unforgeable via_virtual_key marker AND a known non-secret shape: the sha256 hex digest @@ -1574,12 +1589,13 @@ class LiteLLMProxyRequestSetup: # Last-resort fallback: the W3C standards for trace/session propagation # (https://www.w3.org/TR/trace-context/, https://www.w3.org/TR/baggage/). # Lower priority than everything above - only fires when neither the - # explicit litellm headers nor the Anthropic-metadata path found - # anything - but lets a caller's existing traceparent/baggage headers + # explicit litellm headers, the Anthropic-metadata path, nor the + # caller's own request-body metadata set the field - but lets a + # caller's existing traceparent/baggage headers # (from real OTel instrumentation) correlate with litellm's own logs # instead of generating an unrelated trace_id. normalized_headers: Final = MappingProxyType({k.lower(): v for k, v in headers.items() if isinstance(k, str)}) - if "litellm_trace_id" not in data: + if "litellm_trace_id" not in data and not _client_steered_trace_field(data, "trace_id"): traceparent: Final = normalized_headers.get("traceparent") if isinstance(traceparent, str): trace_id_from_traceparent: Final = _trace_id_from_traceparent(traceparent) @@ -1589,7 +1605,7 @@ class LiteLLMProxyRequestSetup: verbose_proxy_logger.debug( "Extracted trace_id from W3C traceparent header: %s", trace_id_from_traceparent ) - if "litellm_session_id" not in data: + if "litellm_session_id" not in data and not _client_steered_trace_field(data, "session_id"): baggage: Final = normalized_headers.get("baggage") if isinstance(baggage, str): session_id_from_baggage: Final = _session_id_from_baggage(baggage) diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index b2241191ced..bd350f56410 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -3648,6 +3648,61 @@ def test_add_litellm_metadata_from_request_headers_explicit_trace_id_beats_trace assert data["litellm_session_id"] == "explicit-trace-id-value" +def test_add_litellm_metadata_from_request_headers_body_trace_id_beats_traceparent(): + """A caller that set metadata.trace_id keeps it: the traceparent fallback is + documented as last-resort, so it must not overwrite an explicit choice. + + This matters in practice because some platforms (e.g. GCP) inject a + traceparent into every inbound request, so the fallback would otherwise fire + on traffic whose caller never sent the header at all.""" + headers = {"traceparent": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"} + data = {"metadata": {"trace_id": "caller-chosen-trace-id"}} + LiteLLMProxyRequestSetup.add_litellm_metadata_from_request_headers( + headers=headers, data=data, _metadata_variable_name="metadata" + ) + assert data["metadata"]["trace_id"] == "caller-chosen-trace-id" + assert "litellm_trace_id" not in data + + +def test_add_litellm_metadata_from_request_headers_body_session_id_beats_baggage(): + """Same for session_id and the baggage header.""" + headers = {"baggage": "session.id=baggage-session-42"} + data = {"metadata": {"session_id": "caller-chosen-session-id"}} + LiteLLMProxyRequestSetup.add_litellm_metadata_from_request_headers( + headers=headers, data=data, _metadata_variable_name="metadata" + ) + assert data["metadata"]["session_id"] == "caller-chosen-session-id" + assert "litellm_session_id" not in data + + +def test_add_litellm_metadata_from_request_headers_body_steering_is_per_field(): + """Steering one field must not suppress the fallback for the other: + a caller setting only trace_id still gets session_id from baggage.""" + headers = { + "traceparent": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + "baggage": "session.id=baggage-session-42", + } + data = {"metadata": {"trace_id": "caller-chosen-trace-id"}} + LiteLLMProxyRequestSetup.add_litellm_metadata_from_request_headers( + headers=headers, data=data, _metadata_variable_name="metadata" + ) + assert data["metadata"]["trace_id"] == "caller-chosen-trace-id" + assert data["litellm_session_id"] == "baggage-session-42" + + +def test_add_litellm_metadata_from_request_headers_litellm_metadata_steering_honoured(): + """Routes in LITELLM_METADATA_ROUTES (/v1/responses, /v1/messages, batches, + files) carry their metadata in litellm_metadata, so steering there counts + the same as steering in metadata.""" + headers = {"traceparent": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"} + data = {"litellm_metadata": {"trace_id": "caller-chosen-trace-id"}} + LiteLLMProxyRequestSetup.add_litellm_metadata_from_request_headers( + headers=headers, data=data, _metadata_variable_name="litellm_metadata" + ) + assert data["litellm_metadata"]["trace_id"] == "caller-chosen-trace-id" + assert "litellm_trace_id" not in data + + def _otel_span_with_trace_id(trace_id: int) -> NonRecordingSpan: return NonRecordingSpan(SpanContext(trace_id=trace_id, span_id=0x00F067AA0BA902B7, is_remote=False))