From a0622a2aab51e4891ee19a332bb9bf1054070ee4 Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Tue, 28 Jul 2026 16:40:03 -0700 Subject: [PATCH] fix(otel/v2): skip phantom LLM span on rejected requests; harden telemetry setup A request rejected before any upstream call (rate limit, budget, pre-call guardrail) fires the failure callback with a built payload while the auth-hoisted destinations are still resolved, so _close_llm_call's carrier-None branch fabricated a gen-AI span for a call that never reached a provider. It now re-reads call.is_no_upstream_call, the same marker log_pre_api_call already honors, so a rejected request no longer lands a fake "chat " span in the tenant's destination. Reproduced live against a real destination (a 429 emitted a chat span in Arize before the fix, none after) and pinned by a regression test that fails on the pre-fix code The pre-call resolver re-run in _apply_admin_logging_exporters was not wrapped, unlike the auth-time hoist, so a non-HTTPException from the org fallback lookup could abort a real request; it is now best-effort so admin-owned telemetry setup can never break request handling Also drops the explanatory inline comments this feature added across the otel v2 modules, keeping only docstrings and lint suppressions per the repository convention that new code carries no comments --- litellm/constants.py | 3 -- litellm/integrations/otel/logger.py | 14 +---- .../integrations/otel/model/destination.py | 3 -- litellm/integrations/otel/model/metadata.py | 4 -- litellm/integrations/otel/mount.py | 6 --- litellm/integrations/otel/plumbing/context.py | 9 ---- .../integrations/otel/plumbing/providers.py | 8 --- litellm/integrations/otel/plumbing/routing.py | 42 --------------- litellm/integrations/otel/presets/agentops.py | 3 -- litellm/integrations/otel/presets/arize.py | 4 -- .../integrations/otel/presets/destinations.py | 17 ------- litellm/integrations/otel/presets/langfuse.py | 5 -- litellm/integrations/otel/presets/levo.py | 4 -- litellm/integrations/otel/presets/phoenix.py | 4 -- litellm/integrations/otel/presets/weave.py | 6 --- litellm/litellm_core_utils/litellm_logging.py | 10 ---- litellm/proxy/_types.py | 4 -- litellm/proxy/auth/user_api_key_auth.py | 7 --- .../proxy/credential_endpoints/endpoints.py | 4 -- litellm/proxy/litellm_pre_call_utils.py | 20 ++------ .../key_management_endpoints.py | 2 - .../management_endpoints/team_endpoints.py | 5 -- litellm/types/utils.py | 4 -- .../integrations/otel/test_otel_v2_logger.py | 51 +++++++++++++++++++ .../proxy/test_litellm_pre_call_utils.py | 31 +++++++++++ 25 files changed, 87 insertions(+), 183 deletions(-) diff --git a/litellm/constants.py b/litellm/constants.py index 167e1426ba1..170106f01e8 100644 --- a/litellm/constants.py +++ b/litellm/constants.py @@ -440,9 +440,6 @@ DD_TRACER_STREAMING_CHUNK_YIELD_RESOURCE = os.getenv( LITELLM_HTTP_STATUS_CLIENT_DISCONNECTED = 499 -# Reserved key/team logging callback var that binds the callback to a named OTEL -# credential in the registry (an admin-owned reference resolved server-side into a -# trace destination, never forwarded as a request parameter). LITELLM_LOGGING_CREDENTIAL_NAME_KEY = "litellm_logging_credential_name" EMAIL_BUDGET_ALERT_TTL = int(os.getenv("EMAIL_BUDGET_ALERT_TTL", 24 * 60 * 60)) # 24 hours in seconds diff --git a/litellm/integrations/otel/logger.py b/litellm/integrations/otel/logger.py index df768b98326..43a4ede1fe5 100644 --- a/litellm/integrations/otel/logger.py +++ b/litellm/integrations/otel/logger.py @@ -166,10 +166,6 @@ class OpenTelemetryV2(CustomLogger): ) self._tenant_tracers = TenantTracerCache(self.config, callback_name, LITELLM_TRACER_NAME) self._open_llm_calls: OrderedDict[str, _LLMCallSpan] = OrderedDict() - # call_ids for which the LLM-call span has already been emitted; lets - # _close_llm_call no-op on duplicate callbacks (success + failure both - # firing, or success firing twice) instead of double-exporting the - # deferred-emit span. Bounded LRU, same size as _open_llm_calls. self._closed_call_ids: OrderedDict[str, None] = OrderedDict() # mutable-ok: bounded LRU of emitted call ids self._init_otel_logger_on_litellm_proxy() @@ -273,9 +269,6 @@ class OpenTelemetryV2(CustomLogger): if call_id in self._open_llm_calls: return start_time_ns = to_ns(datetime.now()) - # One live span per destination Resource group (Arize routing two projects - # yields two; header-routed backends yield one). Empty until a recordable - # parent is confirmed. spans: tuple[Span, ...] = () # Parent to the request's anchored root span (stable across the request), # falling back to ambient on the SDK path. Open the span live only when @@ -445,11 +438,6 @@ class OpenTelemetryV2(CustomLogger): call = LLMCallEvent.from_dict(kwargs) call_id = call.call_id - # Dedup guard: a normal close pops the carrier and emits a span. If a - # second close fires for the same call_id (success + failure callbacks - # are wired separately, custom callbacks can fan out, etc.), the - # carrier is already gone and a payload+destinations combination would - # otherwise emit a second deferred span. if call_id and call_id in self._closed_call_ids: return None @@ -458,7 +446,7 @@ class OpenTelemetryV2(CustomLogger): if carrier is None: destinations = self._destinations_for_backend(call) - if payload is None or not destinations: + if call.is_no_upstream_call or payload is None or not destinations: return None self._mark_closed(call_id) return self._emit_deferred_llm_call( diff --git a/litellm/integrations/otel/model/destination.py b/litellm/integrations/otel/model/destination.py index 51eff030807..57f64ef34e3 100644 --- a/litellm/integrations/otel/model/destination.py +++ b/litellm/integrations/otel/model/destination.py @@ -18,9 +18,6 @@ class OtelDestination(BaseModel): endpoint: str headers: Mapping[str, str] = Field(default_factory=dict) resource_attributes: Mapping[str, str] = Field(default_factory=dict) - # The OTEL backend (callback_name) this destination belongs to, so a request - # that fans out across backends routes each destination to the logger that - # owns its attribute vocabulary. None for the legacy single-destination path. callback_name: str | None = None def header_string(self) -> str: diff --git a/litellm/integrations/otel/model/metadata.py b/litellm/integrations/otel/model/metadata.py index 5198c0ea5f3..4db37996301 100644 --- a/litellm/integrations/otel/model/metadata.py +++ b/litellm/integrations/otel/model/metadata.py @@ -194,10 +194,6 @@ class LLMCallEvent: # at ``pre_call``, or when the call closed before any payload materialized (so # there is nothing to stamp on the span). payload: "StandardLoggingPayload | None" - # The admin-resolved OTLP destinations (endpoint + auth headers) for this call's - # identity chain, fanned out to. Empty when none are assigned. Read from the - # server-only request ContextVar the proxy anchors at auth time, so it is never - # request-derived and never travels through the request or provider body. otel_destinations: tuple[OtelDestination, ...] # True for synthetic proxy-gate logs (auth / rate-limit rejections): they fire # the ``pre_call`` hook but never made an upstream call, so they get no span. diff --git a/litellm/integrations/otel/mount.py b/litellm/integrations/otel/mount.py index 074fc928e5e..983c9eb1582 100644 --- a/litellm/integrations/otel/mount.py +++ b/litellm/integrations/otel/mount.py @@ -104,12 +104,6 @@ def instrument_fastapi_app(app: Any) -> None: if not is_otel_v2_enabled(): return - # Lazy: only the V2-enabled path needs the optional - # ``opentelemetry-instrumentation-fastapi`` package. Importing it at module top - # would make ``proxy_server``'s unconditional ``import`` of this module crash when - # the package is absent, even with the gate off. When V2 IS on, a missing package - # is a real misconfiguration -- without the server span the trace has no root and - # admin-owned destination traces are orphaned -- so it must be loud, not silent. try: from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor except ImportError: diff --git a/litellm/integrations/otel/plumbing/context.py b/litellm/integrations/otel/plumbing/context.py index dc9d8568283..d221503b4bb 100644 --- a/litellm/integrations/otel/plumbing/context.py +++ b/litellm/integrations/otel/plumbing/context.py @@ -39,15 +39,6 @@ _PROPAGATOR = TraceContextTextMapPropagator() # request task, so there is nothing to leak. _request_root_span: "ContextVar[Span | None]" = ContextVar("litellm_otel_request_root_span", default=None) -# Per-request admin-resolved destinations. Set once at the auth boundary (the -# earliest point a request's identity is known) and read by the global-provider -# fan-out processor at ``on_end`` time, so every span the proxy emits for this -# request -- the FastAPI server span, the ``auth`` phase, DB lookups, the -# batch-write cost ledger -- ships to every per-tenant destination the admin -# assigned. Lives on a ``ContextVar`` so it follows the request task across -# ``asyncio.create_task`` children (the success/failure logging callbacks close -# the LLM span in a worker copied from the request context). Request-scoped: the -# contextvar dies with the request task, so nothing leaks across requests. _request_destinations: 'ContextVar[tuple["OtelDestination", ...]]' = ContextVar( "litellm_otel_request_destinations", default=() ) diff --git a/litellm/integrations/otel/plumbing/providers.py b/litellm/integrations/otel/plumbing/providers.py index f8917d5ab71..d32263c4afa 100644 --- a/litellm/integrations/otel/plumbing/providers.py +++ b/litellm/integrations/otel/plumbing/providers.py @@ -113,10 +113,6 @@ def _otlp_traces_endpoint(endpoint: str | None) -> str | None: if not endpoint: return endpoint endpoint = endpoint.rstrip("/") - # Some vendors expose a complete traces ingest path that is NOT the OTLP-standard - # ``/v1/traces`` base: Splunk Observability uses ``/v2/trace/otlp`` and Langtrace - # ingests at ``/api/trace``. Appending ``/v1/traces`` to those 404s, so never - # rewrite them. if endpoint.endswith("/v1/traces") or "/v2/trace/otlp" in endpoint or endpoint.endswith("/api/trace"): return endpoint for other_signal in ("/v1/logs", "/v1/metrics"): @@ -125,10 +121,6 @@ def _otlp_traces_endpoint(endpoint: str | None) -> str | None: return endpoint + "/v1/traces" -# Backends whose OTLP transport is gRPC. Arize's OTLP endpoint -# (``otlp.arize.com``) speaks gRPC; every other current preset speaks OTLP/HTTP. -# Single source of truth shared by the per-tenant fan-out processor and the -# ``TenantTracerCache`` so the two never disagree on a destination's transport. _GRPC_BACKENDS = frozenset({"arize"}) diff --git a/litellm/integrations/otel/plumbing/routing.py b/litellm/integrations/otel/plumbing/routing.py index 15768523cc5..423dc17ff2c 100644 --- a/litellm/integrations/otel/plumbing/routing.py +++ b/litellm/integrations/otel/plumbing/routing.py @@ -39,14 +39,8 @@ from litellm.integrations.otel.plumbing.providers import ( get_tracer, ) -# Exporter kinds that ignore endpoint/headers — never rewritten with a destination. _NON_OTLP_KINDS = ("console", "in_memory", "inmemory", "memory") -# Cap on distinct destination-scoped providers held at once. Destinations are -# admin-owned (one per key/team), so this is resource hygiene rather than an -# anti-abuse bound: it keeps the working set of active tenants resident. Evicted -# providers are dropped without a synchronous shutdown (see _evict_if_full); their -# exporter threads drain on their own and are reclaimed at process exit. _MAX_CACHED_PROVIDERS = 256 @@ -231,27 +225,8 @@ class TenantTracerCache: ) -# --- Proxy-internal span fan-out ------------------------------------------- # -# -# ``TenantTracerCache`` above routes the gen-AI LLM-call span (and its MCP-tool -# sibling) to per-tenant destinations through clone providers. The processor below -# handles the OTHER span class: the proxy-internal spans (FastAPI server span, the -# ``auth`` phase, DB lookups, the post-call cost ledger) emitted on the MAIN -# provider. It forwards each to every admin-resolved destination for the request, -# reading them from the same server-only contextvar the cache's callers set, so both -# routing paths share one source of truth for where a request's traces go. - -# Bound on cached per-destination processors. One processor per -# ``(endpoint, sorted(headers))`` pair, so the working set is one entry per -# admin-resolved tenant credential -- a real-world deployment with hundreds of -# tenants stays well under this. Evicted entries are dropped (not shut down; see -# the eviction site) and reclaimed at process exit. _MAX_CACHED_PROCESSORS = 256 -# Attribute set on every gen-AI LLM-call span by the v2 emitter. Used as the -# unambiguous skip signal: only the LLM-call span carries this, and the per-backend -# v2 logger already routes it to per-tenant destinations through the -# TenantTracerCache clone provider's appended exporter. _GENAI_SPAN_ATTR = "gen_ai.operation.name" @@ -325,19 +300,8 @@ class TenantFanOutSpanProcessor(SpanProcessor): destinations = request_destinations() if not destinations: return - # The gen-AI LLM-call span (and the MCP tool-call sibling) is already routed - # to per-tenant destinations by the per-backend v2 logger via - # ``TenantTracerCache`` -- the logger picks the right attribute mapper - # (OpenInference for arize, GenAI semconv for langfuse_otel) and ships through - # the clone provider's appended exporter. Forwarding it here too would deliver - # a SECOND copy with the wrong vocabulary and a fresh span_id, surfacing in the - # destination as an orphaned duplicate. Skip. if _is_genai_span(span): return - # Proxy-internal spans (FastAPI server, ``auth`` phase, postgres lookups, - # post-call cost ledger) are generic OTel semantic-convention spans with no - # backend-specific vocabulary, so they ship to EVERY admin-resolved destination - # this request fans out to, regardless of the destination's ``callback_name``. for destination in destinations: processor = self._processor_for(destination) if processor is None: @@ -401,11 +365,5 @@ class TenantFanOutSpanProcessor(SpanProcessor): return None self._processors[key] = processor if len(self._processors) > _MAX_CACHED_PROCESSORS: - # Evict the LRU entry but do NOT shut it down here: a - # ``BatchSpanProcessor`` may still hold spans queued on its exporter - # thread, and calling ``shutdown`` synchronously can drop or raise on those - # in-flight spans. Dropping the reference lets the worker drain naturally - # and be reclaimed at process exit. The cache is bounded, so the - # un-shut-down working set stays bounded. self._processors.popitem(last=False) return processor diff --git a/litellm/integrations/otel/presets/agentops.py b/litellm/integrations/otel/presets/agentops.py index 94568c4122c..f4bef67d092 100644 --- a/litellm/integrations/otel/presets/agentops.py +++ b/litellm/integrations/otel/presets/agentops.py @@ -50,9 +50,6 @@ def agentops_preset( """ settings = _AgentOpsSettings() base = config_overrides or OpenTelemetryV2Config() - # Contribute the global AgentOps exporter only when an API key is configured. - # Without it the lazy-auth exporter has nothing to mint a JWT from and every - # export fails; admin-owned destinations carry their own credentials. global_exporter = ( ( ExporterSpec( diff --git a/litellm/integrations/otel/presets/arize.py b/litellm/integrations/otel/presets/arize.py index 7b58234bd2a..c65ec1bf12f 100644 --- a/litellm/integrations/otel/presets/arize.py +++ b/litellm/integrations/otel/presets/arize.py @@ -28,10 +28,6 @@ def arize_preset( arize_cfg = _V1ArizeLogger.get_arize_config() headers = _arize_headers(arize_cfg) base = config_overrides or OpenTelemetryV2Config() - # Contribute the global Arize exporter only when Arize credentials are - # configured. Without them it points at the Arize cloud with no auth and every - # export fails PERMISSION_DENIED; admin-owned destinations carry their own - # credentials and are appended by the router instead. global_exporter = ( ( ExporterSpec( diff --git a/litellm/integrations/otel/presets/destinations.py b/litellm/integrations/otel/presets/destinations.py index 01a7c1b905d..0ca5b5bd2ac 100644 --- a/litellm/integrations/otel/presets/destinations.py +++ b/litellm/integrations/otel/presets/destinations.py @@ -21,9 +21,6 @@ from litellm.integrations.langfuse.langfuse_otel import ( from litellm.integrations.otel.model.destination import OtelDestination from litellm.integrations.weave.weave_otel import _get_weave_authorization_header -#: Reserved ``callback_vars`` key binding a key/team's logging callback to a named -#: credential in the registry. It is a reference, resolved server-side; it is never -#: forwarded as a request parameter. LOGGING_CREDENTIAL_NAME_KEY = LITELLM_LOGGING_CREDENTIAL_NAME_KEY @@ -54,12 +51,6 @@ def _arize_destination(values: Mapping[str, str]) -> OtelDestination | None: if not space or not api_key: return None endpoint = values.get("arize_endpoint") or "https://otlp.arize.com/v1" - # Arize routes a trace to a project via the ``model_id`` Resource attribute - # (OpenInference convention), NOT an auth header like langfuse/weave do, so the - # project must ride the span Resource. Prefer the credential's own project, then - # fall back to the proxy-global ``ARIZE_PROJECT_NAME`` so an arize credential - # that omits the project still lands somewhere deterministic. Backends that route - # by header declare no resource_attributes; this stays arize-local. project = values.get("arize_project_name") or values.get("project_name") or os.environ.get("ARIZE_PROJECT_NAME") resource_attributes = {"model_id": project, "arize.project.name": project} if project else {} return OtelDestination( @@ -73,13 +64,6 @@ def _weave_destination(values: Mapping[str, str]) -> OtelDestination | None: api_key = values.get("wandb_api_key") if not api_key: return None - # Weave's OTLP path is ``/otel/v1/traces`` (not the bare ``/v1/traces`` the - # generic exporter would append), so a host like ``https://trace.wandb.ai`` - # must be completed here -- otherwise the export 404s and silently drops. The - # host itself defaults to the Weave cloud base (only dedicated/self-hosted wandb - # differs), so the endpoint is optional. Mirror the v1 integration's - # WEAVE_BASE_URL / WEAVE_OTEL_ENDPOINT and stay idempotent if the caller already - # supplied the full path or the ``/otel`` prefix. from litellm.integrations.weave.weave_otel import ( WEAVE_BASE_URL, WEAVE_OTEL_ENDPOINT, @@ -109,7 +93,6 @@ _ADAPTERS: Mapping[str, Callable[[Mapping[str, str]], OtelDestination | None]] = "weave_otel": _weave_destination, } -#: OTEL v2 callbacks that can be routed to a per-key/team admin destination. OTEL_V2_DESTINATION_CALLBACKS = frozenset(_ADAPTERS) diff --git a/litellm/integrations/otel/presets/langfuse.py b/litellm/integrations/otel/presets/langfuse.py index b2b5442409b..9fd27f0187e 100644 --- a/litellm/integrations/otel/presets/langfuse.py +++ b/litellm/integrations/otel/presets/langfuse.py @@ -18,11 +18,6 @@ def langfuse_preset( ) -> OpenTelemetryV2Config: base = config_overrides or OpenTelemetryV2Config() mappers = ensure_mappers(base.mapper_names, "langfuse") - # ``get_langfuse_otel_config()`` raises without Langfuse keys. Propagate that raise - # for a global callback so a misconfigured deployment fails loud, but when an - # admin-owned Langfuse destination is the reason for construction it carries its own - # per-tenant keys, so degrade to a (global-exporter-less) mapper-only config -- or - # the gen-AI span falls to the generic logger and never reaches it. try: cfg = _V1Langfuse.get_langfuse_otel_config() except Exception: diff --git a/litellm/integrations/otel/presets/levo.py b/litellm/integrations/otel/presets/levo.py index eeeadc9d961..ba090119acc 100644 --- a/litellm/integrations/otel/presets/levo.py +++ b/litellm/integrations/otel/presets/levo.py @@ -14,10 +14,6 @@ def levo_preset( allow_missing_credentials: bool = False, ) -> OpenTelemetryV2Config: base = config_overrides or OpenTelemetryV2Config() - # ``get_levo_config()`` raises without Levo credentials. Propagate that raise for a - # global callback so a misconfigured deployment fails loud, but when an admin-owned - # Levo destination is the reason for construction it carries its own per-tenant - # credentials, so degrade to a global-exporter-less config rather than raising. try: cfg = _V1Levo.get_levo_config() except Exception: diff --git a/litellm/integrations/otel/presets/phoenix.py b/litellm/integrations/otel/presets/phoenix.py index ecd055cbac4..ab7f7bdde02 100644 --- a/litellm/integrations/otel/presets/phoenix.py +++ b/litellm/integrations/otel/presets/phoenix.py @@ -39,10 +39,6 @@ def phoenix_preset( ) -> OpenTelemetryV2Config: project_name = _PhoenixSettings().project_name base = config_overrides or OpenTelemetryV2Config() - # Contribute the global Phoenix exporter only when Phoenix is configured (a - # cloud API key or a collector endpoint). Otherwise the config defaults to - # http://localhost:6006 and would export there even when the operator only - # uses admin-owned Phoenix destinations. if any(os.environ.get(v) for v in _PHOENIX_ENV_VARS): cfg = _V1Phoenix.get_arize_phoenix_config() headers = cfg.otlp_auth_headers if hasattr(cfg, "otlp_auth_headers") else None diff --git a/litellm/integrations/otel/presets/weave.py b/litellm/integrations/otel/presets/weave.py index 4c149b2ec2b..1be6603dae6 100644 --- a/litellm/integrations/otel/presets/weave.py +++ b/litellm/integrations/otel/presets/weave.py @@ -15,13 +15,7 @@ def weave_preset( allow_missing_credentials: bool = False, ) -> OpenTelemetryV2Config: base = config_overrides or OpenTelemetryV2Config() - # Weave consumes OpenInference + a small Weave-specific overlay. mappers = ensure_mappers(base.mapper_names, "openinference", "weave") - # ``get_weave_otel_config()`` raises without W&B credentials. Propagate that raise - # for a global callback so a misconfigured deployment fails loud, but when an - # admin-owned Weave destination is the reason for construction it carries its own - # per-tenant credentials, so degrade to a (global-exporter-less) mapper-only config - # -- otherwise the gen-AI span falls to the generic logger and never reaches Weave. try: weave_cfg = get_weave_otel_config() except Exception: diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index 540697f8969..dd3ba6eb456 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -4058,11 +4058,6 @@ def _init_custom_logger_compatible_class( _in_memory_loggers.append(_otel_logger) return _otel_logger # type: ignore elif logging_integration == "generic": - # Generic OTLP passthrough: a vendor-neutral OpenTelemetryV2 logger whose - # per-destination exporter is attached by the admin-owned destination, so a - # ``generic`` destination gets the full trace (incl. the gen-AI span), not - # just proxy-internal spans. Only meaningful as an admin-owned destination, - # so there is no legacy fallback: None when no v2 logger is constructed. return _maybe_construct_otel_v2("generic", _in_memory_loggers) elif logging_integration == "pagerduty": for callback in _in_memory_loggers: @@ -4232,11 +4227,6 @@ def _maybe_construct_otel_v2(callback_name: str, _in_memory_loggers: list) -> Op if isinstance(callback, OpenTelemetryV2) and getattr(callback, "callback_name", None) == callback_name: return callback try: - # An admin-owned destination carries its own per-tenant credentials, so a - # credential-mandatory preset may degrade rather than raise. A purely global - # callback with no destination must still raise on missing credentials; the - # raise is swallowed here so the caller defers to the legacy path and customers - # get the same loud error story they had before V2 landed. config = preset_fn(allow_missing_credentials=has_admin_dest) except Exception: return None diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 15922c9fbb4..25427a7f281 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -2859,8 +2859,6 @@ class LiteLLM_OrganizationTableWithMembers(LiteLLM_OrganizationTable): litellm_budget_table: Optional[LiteLLM_BudgetTable] = None created_at: datetime updated_at: datetime - # Destination names that will receive this org's traces (own logging_exporters - # plus auto-enabled destinations whose access grants the org). Names only. resolved_logging_exporters: Sequence[str] | None = None @@ -3876,8 +3874,6 @@ class TeamInfoResponseObjectTeamTable(LiteLLM_TeamTable): access_group_models: Optional[List[str]] = None access_group_mcp_server_ids: Optional[List[str]] = None access_group_agent_ids: Optional[List[str]] = None - # Destination names that will receive this team's traces (own logging_exporters - # plus auto-enabled destinations whose access grants the team). Names only. resolved_logging_exporters: Sequence[str] | None = None diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 9457b31dc15..2a092a5a50c 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -2592,13 +2592,6 @@ async def user_api_key_auth( ) user_api_key_auth_obj.budget_reservation = None - # Admin-resolved OTEL destinations: anchor them on this request's task - # context BEFORE downstream spans close, so the global-provider fan-out - # processor forwards every span (server, auth, db, batch-write) to the - # admin-assigned per-tenant backends -- not just the gen-AI span the - # ``TenantTracerCache`` already routes. Also stashed on ``request.state`` - # so ``_apply_admin_logging_exporters`` reuses the result instead of - # re-resolving. await _hoist_request_destinations(request, user_api_key_auth_obj) ## ENSURE DISABLE ROUTE WORKS ACROSS ALL USER AUTH FLOWS ## diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index 26605fddee6..5c10aa1261f 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -313,10 +313,6 @@ def update_db_credential( merged_credential.credential_values.update(encrypted_params) - # Merge the patch into the existing credential_info so a partial update (e.g. only - # access.teams) preserves credential_type/description/host AND the untouched - # access subfields (global/orgs/other teams in access). See - # _merge_credential_info for the surgical-access reasoning. if encrypted_credential.credential_info: if merged_credential.credential_info is None: merged_credential.credential_info = {} diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 08766ebae71..2f9bd5c2d72 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -808,13 +808,13 @@ async def _apply_admin_logging_exporters( ) ) else: - destinations, backends = await _resolve_logging_exporters(user_api_key_dict) + try: + destinations, backends = await _resolve_logging_exporters(user_api_key_dict) + except Exception: # noqa: BLE001 # best-effort telemetry setup must never break the request + return if not destinations: return _set_request_otel_destinations(destinations) - # Register on both success and failure: an admin-owned destination must - # capture a failed upstream call (its error gen-AI span) as well as a - # successful one, otherwise a 401/timeout lands a trace with no LLM-call span. existing_success = data.get("success_callback") or [] data["success_callback"] = list(dict.fromkeys([*existing_success, *backends])) existing_failure = data.get("failure_callback") or [] @@ -1984,13 +1984,6 @@ async def add_litellm_data_to_request( ) # Team Callbacks controls - # OTEL destinations are admin-owned and resolved server-side below; a client must - # never set or override them. Drop any client value from every metadata carrier - # before the resolver runs so injection is inert regardless of the slot used: the - # top level, the request metadata key, and litellm_metadata (where the resolver - # stashes the admin-resolved value the dynamic-params reader later picks up). The - # server sets only litellm_metadata at resolve time, so wiping the others here - # removes client input only. data.pop("otel_destinations", None) for _carrier_key in (_metadata_variable_name, "litellm_metadata"): carrier = data.get(_carrier_key) @@ -2007,11 +2000,6 @@ async def add_litellm_data_to_request( for k, v in callback_settings_obj.callback_vars.items(): data[k] = v - # Admin-owned exporter assignment: resolve the union of exporters assigned across - # the request's identity chain (key + team + org) into fan-out destinations and - # activate their backends. Default-deny: an unassigned identity gets none. Reuse - # the result ``user_api_key_auth`` already cached on ``request.state`` so the - # resolver runs once per request, not twice. cached = getattr(getattr(request, "state", None), "otel_destinations", None) await _apply_admin_logging_exporters(data, user_api_key_dict, cached_destinations=cached) diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 63140689097..1bed6c34e34 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -4751,8 +4751,6 @@ async def regenerate_key_fn( # noqa: C901 # single endpoint handling many opti detail={"error": "You are not authorized to regenerate this key"}, ) - # Look up the key's team once (the body may omit team_id); shared by the - # access-group, object-permission, and logging-exporter gates below. regenerate_team_table: LiteLLM_TeamTableCachedObj | None = None if _key_in_db.team_id is not None: try: diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index ef4beaaea51..979ad25c7c9 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -1022,8 +1022,6 @@ async def new_team( user_api_key_cache, ) - # logging_exporters is proxy-admin only. Skip the check when the field - # isn't being written so unrelated /team/new calls stay cheap. if data.logging_exporters is not None: validate_logging_exporter_field(data.logging_exporters, user_api_key_dict) @@ -1735,9 +1733,6 @@ async def update_team( user_api_key_dict=user_api_key_dict, ) - # logging_exporters on /team/update is proxy-admin only. Pass the stored - # column value so the validator's no-op check sees a real change and a - # non-admin cannot clear an admin-assigned value. if data.logging_exporters is not None: validate_logging_exporter_field( data.logging_exporters, diff --git a/litellm/types/utils.py b/litellm/types/utils.py index c09ccfb4c69..4f029e8de95 100644 --- a/litellm/types/utils.py +++ b/litellm/types/utils.py @@ -3086,10 +3086,6 @@ class StandardCallbackDynamicParams(TypedDict, total=False): turn_off_message_logging: Optional[bool] # when true will not log messages litellm_disabled_callbacks: Optional[List[str]] - # Admin-owned OTEL v2 destinations, resolved server-side from the exporters - # assigned to the request's identity chain (key/team/user/org), fanned out to. - # Never request-settable: absent from the request-read whitelist in - # initialize_dynamic_callback_params, so a request body/metadata cannot set it. otel_destinations: Optional[Sequence[OtelDestinationParams]] 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 99ad3622d20..72f1ea5ca7d 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_logger.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_logger.py @@ -986,6 +986,57 @@ def test_lazy_activation_emits_llm_span_when_destination_resolves(monkeypatch): assert len(dests) == 1 and dests[0].endpoint == "https://otlp.example.com/v1" +def test_no_upstream_reject_emits_no_deferred_span_even_with_destinations(monkeypatch): + """LIT-3850 regression: a post-auth rejection (rate-limit/budget/guardrail) fires + the failure callback with the ``LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL`` marker set, + a real ``standard_logging_object`` payload, AND admin-resolved destinations already + hoisted at auth time. This is the exact intersection the lazy-activation close path + misses: because destinations are present, the ``not destinations`` guard does not + fire, so only re-reading ``call.is_no_upstream_call`` keeps the close from + fabricating a ``chat`` span for a call that never reached a provider. It mirrors + ``test_lazy_activation_emits_llm_span_when_destination_resolves`` (which emits) with + the marker added; without the no-upstream check this close emits a phantom span into + the tenant's destination (live-reproduced: a 429 rate-limited request produced a + ``chat gflash`` span in the tenant sink).""" + from litellm.constants import LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL + + logger, exporter = _logger() + monkeypatch.setattr(logger, "callback_name", "in_memory") + server = logger._emitter.start_span( + SpanRole.PROXY_REQUEST, LITELLM_PROXY_REQUEST_SPAN_NAME + ) + set_request_root_span(server) + + tracer_for_calls: list[tuple] = [] + + def _fake_tracers_for(default, destinations): + tracer_for_calls.append(destinations) + return (default,) + + monkeypatch.setattr(logger._tenant_tracers, "tracers_for", _fake_tracers_for) + _anchor([ + { + "callback_name": "in_memory", + "endpoint": "https://otlp.example.com/v1", + "headers": {"api_key": "k"}, + } + ]) + payload = _payload( + status="failure", + error_information={"error_class": "ProxyException", "error_code": "429"}, + ) + kwargs = _kwargs(payload=payload) + kwargs[LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL] = True + + assert "call_1" not in logger._open_llm_calls + asyncio.run(logger.async_log_failure_event(kwargs, None, None, None)) + server.end() + + names = [s.name for s in exporter.get_finished_spans()] + assert "chat gpt-4o" not in names + assert tracer_for_calls == [] + + def test_second_close_after_opened_call_does_not_emit_duplicate(monkeypatch): logger, exporter = _logger() monkeypatch.setattr(logger, "callback_name", "in_memory") 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 bd7a8814c28..9054b65b86b 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -5325,6 +5325,37 @@ async def test_apply_admin_logging_exporters_stamps_and_activates( _request_destinations.reset(token) +@pytest.mark.asyncio +async def test_apply_admin_logging_exporters_swallows_resolver_failure(monkeypatch): + """LIT-3850 regression: admin-owned telemetry setup is best-effort and must never + break a real request. The pre-call path re-runs the resolver when nothing was cached + at auth (the auth hoist failed, or the SDK path has no ``request.state``); the auth + hoist wraps the resolver in ``except Exception`` but this call site did not, so a + non-``HTTPException`` there (e.g. a cache-backend error surfacing through the org + fallback lookup in ``_effective_org_id``) propagated out of + ``add_litellm_data_to_request`` and 500'd the request. With the guard the request + proceeds: no exception escapes, no backend is activated, and request data is + untouched. Without it this raises.""" + import litellm.proxy.litellm_pre_call_utils as pcu + from litellm.integrations.otel.plumbing.context import ( + _request_destinations, + request_destinations, + ) + + async def _boom(_uapk): + raise RuntimeError("cache backend exploded") + + monkeypatch.setattr(pcu, "_resolve_logging_exporters", _boom) + token = _request_destinations.set(()) + data: dict = {} + try: + await pcu._apply_admin_logging_exporters(data, _auth(), cached_destinations=None) + assert data == {} + assert request_destinations() == () + finally: + _request_destinations.reset(token) + + @pytest.mark.asyncio async def test_client_cannot_control_otel_destinations(_seeded_logging_credentials): """Y3 spoofing guard: a client cannot control OTEL export destinations.