mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
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.
This commit is contained in:
parent
d3462c65b5
commit
a1dcd802ad
2 changed files with 75 additions and 4 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue