From 7d0a0ddfa542d541695908df7de34a650788faea Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Fri, 31 Jul 2026 12:59:25 -0700 Subject: [PATCH] fix(otel/v2): keep global exporters off destination Resources; disclose only buildable destinations Two gauntlet findings on the destination fan-out. The configured/global exporters were folded into the first destination Resource group's provider, so that provider's Resource carried the group's destination attributes. An operator running a global OTLP exporter alongside an admin-owned Arize (or any Resource-routed) destination therefore saw their own collector receive gen-AI spans stamped with that tenant's project, e.g. arize.project.name and model_id. The global exporters now ride their own clean-Resource provider and are never folded into a destination group, so each destination still gets its correctly-tagged span while the global export stays untouched; the global continues to receive the span exactly once. resolved_logging_exporter_names, which backs resolved_logging_exporters on /team/info and /organization/info, selected on access alone while the request-time resolver additionally requires a destination to build (a backend named by credential_info.description whose values build_destination accepts). A granted-but-unbuildable destination was therefore advertised while receiving no traces. Both sides now share one destination_for_credential helper, so disclosure and routing cannot drift apart again. --- litellm/integrations/otel/plumbing/routing.py | 14 ++++--- litellm/proxy/litellm_pre_call_utils.py | 21 ++-------- .../logging_exporter_access.py | 36 ++++++++++++++-- .../integrations/otel/test_otel_v2_dynamic.py | 41 +++++++++++-------- .../integrations/otel/test_otel_v2_logger.py | 35 ++++++++++------ .../test_logging_exporter_access.py | 35 +++++++++++++++- 6 files changed, 123 insertions(+), 59 deletions(-) diff --git a/litellm/integrations/otel/plumbing/routing.py b/litellm/integrations/otel/plumbing/routing.py index 87233526ddf..314d3a06005 100644 --- a/litellm/integrations/otel/plumbing/routing.py +++ b/litellm/integrations/otel/plumbing/routing.py @@ -88,15 +88,19 @@ class TenantTracerCache: """The tracers for this request's gen-AI span, one per distinct Resource group. Destinations are grouped by ``destination_resource_attrs`` (a backend like Arize selects its - project from the Resource); the configured/global exporters ride the first group only, and - ``include_base_on_first=False`` drops them when a credential-scoped tracer already carries them. + project from the Resource). The configured/global exporters ride their own clean-Resource + provider (``default``), never folded into a destination group, so a Resource-routed + destination cannot stamp its own attributes (e.g. Arize's project) onto the global export. + ``include_base_on_first=False`` drops ``default`` when a credential-scoped tracer already + carries the configured exporters. """ if not destinations: return (default,) - return tuple( - self._tracer_for_group(resource_key, group, include_base=include_base_on_first and index == 0) - for index, (resource_key, group) in enumerate(self._group_by_resource(destinations)) + groups = tuple( + self._tracer_for_group(resource_key, group, include_base=False) + for resource_key, group in self._group_by_resource(destinations) ) + return (default, *groups) if include_base_on_first else groups def genai_tracers_for( self, diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 05fa88d06bc..d09cb759f10 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -637,14 +637,14 @@ async def _resolve_logging_exporters( ``credential_info.access`` is the sole routing determinant: a destination is selected when its ``access`` grants the caller's team/org. Empty access grants no one, so an empty-access destination never fires (proxy-wide requires - ``access.global``). Each survivor is built via ``build_destination`` and deduped on + ``access.global``). Each survivor is built via ``destination_for_credential`` and deduped on (endpoint, headers, resource attributes). Returns ([], []) when nothing is selected (default-deny). """ from litellm.integrations.otel.model.config import is_otel_v2_enabled - from litellm.integrations.otel.presets.destinations import build_destination from litellm.proxy.management_endpoints.logging_exporter_access import ( access_grants, + destination_for_credential, identity_scope, parse_credential_info, ) @@ -671,26 +671,11 @@ async def _resolve_logging_exporters( return False return access_grants(info.access, team_ids, org_ids) - def _build( - credential: "CredentialItem", - ) -> "tuple[str, OtelDestination] | None": - backend = (credential.credential_info or {}).get("description") - if not backend: - return None - # Drop unset (``None``) values rather than stringifying them: ``str(None)`` is the - # literal ``"None"``, which would land in the exporter endpoint/headers and break - # the export (e.g. an empty ``otel_endpoint`` becoming the URL ``"None"``). - values = { - str(key): str(value) for key, value in (credential.credential_values or {}).items() if value is not None - } - destination = build_destination(backend, values) - return None if destination is None else (backend, destination) - built = tuple( result for credential in litellm.credential_list if _selected(credential) - if (result := _build(credential)) is not None + if (result := destination_for_credential(credential)) is not None ) deduped = { ( diff --git a/litellm/proxy/management_endpoints/logging_exporter_access.py b/litellm/proxy/management_endpoints/logging_exporter_access.py index 7507bdaf6be..dda467086b3 100644 --- a/litellm/proxy/management_endpoints/logging_exporter_access.py +++ b/litellm/proxy/management_endpoints/logging_exporter_access.py @@ -12,9 +12,15 @@ one-element scope built with ``identity_scope``. from pydantic import BaseModel, ConfigDict, ValidationError +from typing import TYPE_CHECKING + import litellm from litellm.integrations.otel.model.config import is_otel_v2_enabled -from litellm.models.credentials import CredentialAccess, CredentialInfo +from litellm.integrations.otel.presets.destinations import build_destination +from litellm.models.credentials import CredentialAccess, CredentialInfo, CredentialItem + +if TYPE_CHECKING: + from litellm.integrations.otel.model.destination import OtelDestination class _LoggingDestinationTag(BaseModel): @@ -74,9 +80,10 @@ def resolved_logging_exporter_names( """Destination names that will receive this identity's traces, for disclosure on the team/org info pages. - Mirrors the request-time resolver's selection: a logging destination is included - when its ``access`` grants the identity. Names only; endpoints, headers, and the - access map itself stay proxy-admin information. + Mirrors the request-time resolver's selection so it never advertises an exporter that + receives no traces: a destination is disclosed only when its ``access`` grants the + identity AND it actually builds (see ``_builds``). Names only; endpoints, headers, and + the access map itself stay proxy-admin information. Gated on ``is_otel_v2_enabled`` for parity with the resolver, which returns nothing when the flag is off: disclosing a destination the request path would never fire @@ -91,10 +98,31 @@ def resolved_logging_exporter_names( if (info := parse_credential_info(credential.credential_info)) is not None and info.credential_type == "logging" and access_grants(info.access, team_ids, org_ids) + and destination_for_credential(credential) is not None ) return tuple(dict.fromkeys(selected)) +def destination_for_credential(credential: CredentialItem) -> 'tuple[str, "OtelDestination"] | None': + """The ``(backend, destination)`` this logging credential resolves to, or ``None`` when it + resolves to nothing. + + A credential builds only when it names a backend (``credential_info.description``) and + ``build_destination`` accepts its values. Shared by the request-time resolver (which fans + out to the built destinations) and the team/org disclosure (which must not advertise a + destination that resolves to nothing), so the two cannot drift apart. + """ + backend = (credential.credential_info or {}).get("description") + if not backend: + return None + # Drop unset (``None``) values rather than stringifying them: ``str(None)`` is the + # literal ``"None"``, which would land in the exporter endpoint/headers and break + # the export (e.g. an empty ``otel_endpoint`` becoming the URL ``"None"``). + values = {str(key): str(value) for key, value in (credential.credential_values or {}).items() if value is not None} + destination = build_destination(backend, values) + return None if destination is None else (backend, destination) + + def access_grants( access: CredentialAccess | None, team_ids: frozenset[str], diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py b/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py index 986725de9ad..f3a6c1777d6 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_dynamic.py @@ -474,11 +474,13 @@ def test_tracers_for_empty_returns_default_only(): def test_tracers_for_single_destination_one_group(): - """No regression: a single Arize destination yields one tracer/provider carrying - its project (same as the old single-merged path).""" + """A single Arize destination yields the clean base tracer (``default``) plus one + destination provider carrying its project; base is never folded into the project group.""" + default = NoOpTracer() cache = _cache("arize") - tracers = cache.tracers_for(NoOpTracer(), (_arize_dest("solo"),)) - assert len(tracers) == 1 + tracers = cache.tracers_for(default, (_arize_dest("solo"),)) + assert tracers[0] is default # base rides its own clean tracer, returned first + assert len(tracers) == 2 # default + the one project group assert {_provider_project(p) for p in cache._providers.values()} == {"solo"} @@ -490,7 +492,7 @@ def test_tracers_for_two_arize_projects_split_into_separate_groups(): tracers = cache.tracers_for( NoOpTracer(), (_arize_dest("projA"), _arize_dest("projB")) ) - assert len(tracers) == 2 # one tracer per project group + assert len(tracers) == 3 # default (clean base) + one tracer per project group assert {_provider_project(p) for p in cache._providers.values()} == { "projA", "projB", @@ -523,36 +525,39 @@ def test_header_routed_destinations_stay_one_group_with_two_exporters(): NoOpTracer(), (_dest("https://a/v1", "Basic A"), _dest("https://b/v1", "Basic B")), ) - assert len(tracers) == 1 # single empty-Resource group + assert len(tracers) == 2 # default (clean base) + the single empty-Resource group assert len(cache._providers) == 1 (provider,) = cache._providers.values() endpoints = " ".join( str(getattr(getattr(sp, "span_exporter", None), "_endpoint", "")) for sp in provider._active_span_processor._span_processors ) - # global + both destinations all live on the one provider (OTLP normalizes the - # endpoint by appending /v1/traces, so match on the host+path prefix) + # both destinations live on the one group provider (the global exporter rides ``default``, + # not this group); OTLP normalizes the endpoint by appending /v1/traces, so match on prefix assert "https://a/v1" in endpoints and "https://b/v1" in endpoints -def test_base_exporters_attach_to_first_group_only(): - """When a backend splits into multiple Resource groups, the configured/global - exporters must ride exactly ONE group, so the global receives the gen-AI span once - rather than once per project.""" +def test_base_exporters_never_ride_a_destination_group(): + """The configured/global exporters must ride their own clean-Resource tracer + (``default``), never a destination group: folding them into a group made the global + export inherit that destination's Resource (e.g. Arize's project), so an operator's + own collector saw spans stamped with a tenant's project. The global still receives the + gen-AI span exactly once (via ``default``), not once per project.""" cache = _cache( "arize", exporters=[ExporterSpec(kind="in_memory", endpoint=None, owner=None)], ) - cache.tracers_for(NoOpTracer(), (_arize_dest("projA"), _arize_dest("projB"))) - base_counts = {} + default = NoOpTracer() + tracers = cache.tracers_for(default, (_arize_dest("projA"), _arize_dest("projB"))) + assert tracers[0] is default # global rides its own clean tracer, exactly once for provider in cache._providers.values(): - project = _provider_project(provider) - base_counts[project] = sum( + base_count = sum( type(getattr(sp, "span_exporter", sp)).__name__ == "InMemorySpanExporter" for sp in provider._active_span_processor._span_processors ) - # exactly one group carries the global in_memory exporter; the other carries none - assert sorted(base_counts.values()) == [0, 1] + assert base_count == 0, "no destination group may carry the configured/global exporters" + # and each group's Resource stays its own project only + assert _provider_project(provider) in {"projA", "projB"} def test_generic_backend_resolves_generic_destination(): diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_logger.py b/tests/test_litellm/integrations/otel/test_otel_v2_logger.py index 2400f3a741e..b634e30ca57 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_logger.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_logger.py @@ -466,8 +466,10 @@ def test_mcp_tool_call_routes_to_admin_destination(): service_name="litellm-proxy", exporters=[ExporterSpec(kind="in_memory")], ) - logger = OpenTelemetryV2(config=cfg, callback_name="generic", tracer_provider=build_tracer_provider(cfg)) - dest = OtelDestination(callback_name="generic", endpoint="http://collector/v1/traces", headers={"x": "1"}) + base_provider = build_tracer_provider(cfg) + logger = OpenTelemetryV2(config=cfg, callback_name="generic", tracer_provider=base_provider) + endpoint = "http://127.0.0.1:1/v1/traces" + dest = OtelDestination(callback_name="generic", endpoint=endpoint, headers={"x": "1"}) token = _request_destinations.set((dest,)) try: @@ -476,18 +478,27 @@ def test_mcp_tool_call_routes_to_admin_destination(): finally: _request_destinations.reset(token) - # The destination's clone provider (base in_memory exporter + the destination) must - # have exported the tool-call span. + # Routing: a destination provider was built and points at the destination's endpoint. If the + # tool-call span took the global-only path (no fan-out), no destination provider would exist. assert logger._tenant_tracers._providers, "no destination provider was built for the tool-call" - captured = [] - for provider in logger._tenant_tracers._providers.values(): - provider.force_flush() - for proc in provider._active_span_processor._span_processors: - exporter = getattr(proc, "span_exporter", None) - if isinstance(exporter, InMemorySpanExporter): - captured += exporter.get_finished_spans() + destination_endpoints = " ".join( + str(getattr(getattr(proc, "span_exporter", None), "_endpoint", "")) + for provider in logger._tenant_tracers._providers.values() + for proc in provider._active_span_processor._span_processors + ) + assert endpoint in destination_endpoints, "no exporter pointed at the admin destination" + + # Emission: the tool-call span was really emitted (the configured/global exporter rides its own + # clean provider, so it is captured there rather than on a destination group). + base_provider.force_flush() + captured = [ + span + for proc in base_provider._active_span_processor._span_processors + if isinstance(getattr(proc, "span_exporter", None), InMemorySpanExporter) + for span in proc.span_exporter.get_finished_spans() + ] assert any(s.attributes.get("mcp.method.name") == "tools/call" for s in captured), ( - "tool-call span did not reach the admin destination's provider" + "tool-call span was not emitted" ) diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py index 996455fc9ac..287d6c85663 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py @@ -185,11 +185,17 @@ def test_identity_scope_empty_for_none(): # --- resolved_logging_exporter_names: the /team/info + /organization/info disclosure -- -def _cred(name, access=None, ctype="logging"): +def _cred(name, access=None, ctype="logging", buildable=True): info = {"credential_type": ctype} if access is not None: info["access"] = access - return CredentialItem(credential_name=name, credential_values={}, credential_info=info) + values = {} + if buildable: + # a generic OTLP backend with an endpoint builds a destination, so disclosure + # (which now mirrors the resolver's buildability) includes it. + info["description"] = "generic" + values = {"otel_endpoint": "http://collector.example/v1/traces"} + return CredentialItem(credential_name=name, credential_values=values, credential_info=info) def test_resolved_names_are_access_only(monkeypatch): @@ -245,3 +251,28 @@ def test_resolved_names_gated_off_when_v2_disabled(monkeypatch): monkeypatch.setenv("LITELLM_OTEL_V2", "true") is_otel_v2_enabled.cache_clear() assert resolved_logging_exporter_names("t1", None) == ("team-granted",) + + +def test_resolved_names_excludes_unbuildable(monkeypatch): + """Disclosure mirrors the resolver's buildability, not access alone: a granted + destination that names no backend, or a backend with incomplete values, resolves to + nothing at request time and must not be advertised on /team/info.""" + monkeypatch.setenv("LITELLM_OTEL_V2", "true") + monkeypatch.setattr( + litellm, + "credential_list", + [ + _cred("buildable-generic", access={"teams": ["t1"]}), + _cred("no-backend", access={"teams": ["t1"]}, buildable=False), + CredentialItem( + credential_name="langfuse-missing-secret", + credential_values={"langfuse_public_key": "pk-only"}, + credential_info={ + "credential_type": "logging", + "access": {"teams": ["t1"]}, + "description": "langfuse_otel", + }, + ), + ], + ) + assert resolved_logging_exporter_names("t1", None) == ("buildable-generic",)