From 1e33b2fede1cb972743dd605c14b01b4657e1cab Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Sat, 5 Sep 2026 14:25:49 -0700 Subject: [PATCH] fix(otel): redact tenant URL query parameters --- litellm/integrations/otel/mappers/legacy.py | 3 +- litellm/integrations/otel/model/semconv.py | 3 + .../integrations/otel/plumbing/providers.py | 29 +++++++-- .../otel/test_otel_v2_destinations.py | 59 +++++++++++++++++++ 4 files changed, 87 insertions(+), 7 deletions(-) diff --git a/litellm/integrations/otel/mappers/legacy.py b/litellm/integrations/otel/mappers/legacy.py index 37475acb8f7..d25c25cd127 100644 --- a/litellm/integrations/otel/mappers/legacy.py +++ b/litellm/integrations/otel/mappers/legacy.py @@ -23,6 +23,7 @@ from litellm.integrations.otel.model.payloads import ( ServiceSpanData, ToolDefinition, ) +from litellm.integrations.otel.model.semconv import Error # Attribute keys in the semconv-ai / Traceloop vocabulary. _LEGACY_SYSTEM: Final = "gen_ai.system" @@ -36,7 +37,7 @@ _LEGACY_PRESENCE_PENALTY: Final = "llm.presence_penalty" _LEGACY_STOP_SEQUENCES: Final = "llm.chat.stop_sequences" _LEGACY_SERVICE: Final = "service" _LEGACY_CALL_TYPE: Final = "call_type" -_LEGACY_ERROR: Final = "error" +_LEGACY_ERROR: Final = Error.MESSAGE_LEGACY class LegacyMapper: diff --git a/litellm/integrations/otel/model/semconv.py b/litellm/integrations/otel/model/semconv.py index af5327cbd41..d3628005bac 100644 --- a/litellm/integrations/otel/model/semconv.py +++ b/litellm/integrations/otel/model/semconv.py @@ -204,6 +204,9 @@ class Error: TYPE: Final = "error.type" MESSAGE: Final = "error.message" + # The same text under the bare key the semconv-ai / Traceloop vocabulary uses + # (see ``LegacyMapper``), so anything reading or redacting error text covers both. + MESSAGE_LEGACY: Final = "error" class LiteLLMError: diff --git a/litellm/integrations/otel/plumbing/providers.py b/litellm/integrations/otel/plumbing/providers.py index db06a467080..70948f466ff 100644 --- a/litellm/integrations/otel/plumbing/providers.py +++ b/litellm/integrations/otel/plumbing/providers.py @@ -355,10 +355,16 @@ _DATASTORE_ENDPOINT_KEYS: Final = frozenset({Server.ADDRESS, Server.PORT, DB.NAM # MCP call, the guardrail), so its error text is theirs to see. Every other span is # the proxy's own work, whose error text names the operator's infrastructure. _TENANT_OWNED_KEYS: Final = frozenset({GenAI.OPERATION_NAME, MCP.METHOD_NAME, LiteLLM.GUARDRAIL_NAME}) +_PROXY_ERROR_TEXT_KEYS: Final = frozenset({Error.MESSAGE, Error.MESSAGE_LEGACY}) # Attribute prefixes the FastAPI instrumentor uses for headers the operator opted to # capture (``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_*``). The request # side carries the caller's bearer token verbatim. _CAPTURED_HEADER_PREFIXES: Final = ("http.request.header.", "http.response.header.") +# The instrumentor stamps the request URL on the server span with its query string, +# under the old convention and the new one, and litellm accepts a virtual key as a +# ``?key=`` query parameter. +_URL_KEYS: Final = frozenset({"http.url", "http.target", "url.full"}) +_URL_QUERY_KEY: Final = "url.query" class _TenantSpanView(ReadableSpan): @@ -397,11 +403,21 @@ def _is_tenant_owned_span(attributes: Mapping[str, AttributeValue]) -> bool: def _tenant_visible(key: str, database: bool, owned: bool) -> bool: - if key.startswith(_CAPTURED_HEADER_PREFIXES) or key == LiteLLMError.STACK_TRACE: + if key.startswith(_CAPTURED_HEADER_PREFIXES) or key in (LiteLLMError.STACK_TRACE, _URL_QUERY_KEY): return False if database and key in _DATASTORE_ENDPOINT_KEYS: return False - return owned or key != Error.MESSAGE + return owned or key not in _PROXY_ERROR_TEXT_KEYS + + +def _without_query(key: str, value: AttributeValue) -> AttributeValue: + if key not in _URL_KEYS or not isinstance(value, str): + return value + return value.partition("?")[0] + + +def _same_attributes(kept: Mapping[str, AttributeValue], attributes: Mapping[str, AttributeValue]) -> bool: + return len(kept) == len(attributes) and all(kept[key] is value for key, value in attributes.items()) def _without_stack_trace(event: Event) -> Event: @@ -426,19 +442,20 @@ def _for_destination(span: ReadableSpan, destination: "OtelDestination") -> Read spells out the operator's Postgres endpoint. A database span loses that endpoint too. Stack traces walk the operator's install and come off every span, as do the headers the operator captures on the server span, whose request side holds the - caller's bearer token. The span itself stays, so the tenant still gets the whole - trace tree. + caller's bearer token, and the query string of the request URL, which can hold + the same key. The span itself stays, so the tenant still gets the whole trace + tree. """ extra: Final = destination.resource_attributes attributes: Final = span.attributes or _NO_ATTRIBUTES database: Final = _is_database_span(attributes) owned: Final = _is_tenant_owned_span(attributes) kept: Final = MappingProxyType( - {key: value for key, value in attributes.items() if _tenant_visible(key, database, owned)} + {key: _without_query(key, value) for key, value in attributes.items() if _tenant_visible(key, database, owned)} ) recorded: Final = span.events events: Final = tuple(_without_stack_trace(event) for event in recorded) if owned else () - unchanged: Final = owned and len(kept) == len(attributes) and all(a is b for a, b in zip(events, recorded)) + unchanged: Final = owned and _same_attributes(kept, attributes) and all(a is b for a, b in zip(events, recorded)) if not extra and unchanged: return span resource: Final = span.resource.merge(Resource(extra)) if extra else span.resource diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_destinations.py b/tests/test_litellm/integrations/otel/test_otel_v2_destinations.py index 212b646d8fa..c98144d1776 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_destinations.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_destinations.py @@ -534,6 +534,7 @@ class TestFanOut: "db.namespace": "litellm", "error.type": "PrismaError", "error.message": unreachable, + "error": unreachable, "litellm.provider.error.stack_trace": f"Traceback: {unreachable}", } ) @@ -566,9 +567,67 @@ class TestFanOut: assert operator_db.attributes["server.port"] == 15400 assert operator_db.attributes["db.namespace"] == "litellm" assert operator_db.attributes["error.message"] == unreachable + assert operator_db.attributes["error"] == unreachable assert operator_db.status.description == unreachable assert [event.name for event in operator_db.events] == ["exception"] + def test_the_callers_key_in_the_query_string_does_not_ride_along_to_the_tenant(self): + """A Google AI Studio style request authenticates with ``?key=``, + and the instrumentor stamps the full request URL on the server span. The + tenant keeps the URL up to the query string, and the operator's copy keeps it + whole.""" + dest_exporter, operator_exporter = InMemorySpanExporter(), InMemorySpanExporter() + provider = TracerProvider() + provider.add_span_processor(SimpleSpanProcessor(operator_exporter)) + provider.add_span_processor( + TenantFanOutSpanProcessor(processor_factory=lambda _d: SimpleSpanProcessor(dest_exporter)) + ) + tracer = get_tracer(provider, "litellm") + path = "/v1beta/models/gemini-2.5-flash:generateContent" + query = "key=sk-another-members-virtual-key&alt=sse" + + def run(): + set_request_destinations((LANGFUSE_DEST,)) + with tracer.start_as_current_span(f"POST {path}") as server_span: + server_span.set_attributes( + { + "http.method": "POST", + "http.route": path, + "http.target": f"{path}?{query}", + "http.url": f"http://proxy.example:4000{path}?{query}", + "url.path": path, + "url.query": query, + "http.status_code": 200, + } + ) + with tracer.start_as_current_span("generate_content gemini-2.5-flash") as llm_span: + llm_span.set_attributes( + { + "gen_ai.operation.name": "generate_content", + "url.full": f"https://generativelanguage.googleapis.com{path}?key=AIza-operator-provider-key", + } + ) + + in_fresh_context(run) + + tenant = {s.name: s for s in dest_exporter.get_finished_spans()} + assert dict(tenant[f"POST {path}"].attributes) == { + "http.method": "POST", + "http.route": path, + "http.target": path, + "http.url": f"http://proxy.example:4000{path}", + "url.path": path, + "http.status_code": 200, + } + assert "sk-another-members-virtual-key" not in tenant[f"POST {path}"].to_json() + assert tenant["generate_content gemini-2.5-flash"].attributes["url.full"] == ( + f"https://generativelanguage.googleapis.com{path}" + ), "the tenant's own span keeps its error text, and still loses a query string" + operator = {s.name: s for s in operator_exporter.get_finished_spans()} + assert operator[f"POST {path}"].attributes["http.url"] == f"http://proxy.example:4000{path}?{query}" + assert operator[f"POST {path}"].attributes["url.query"] == query + assert "AIza-operator-provider-key" in operator["generate_content gemini-2.5-flash"].to_json() + def test_captured_request_headers_do_not_ride_along_to_the_tenant(self): """With ``OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST`` set, the server span carries the caller's bearer token. A team admin's collector must