From 762b9eb1b4c37d49cecf41908c040cdb98472a6d Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Tue, 28 Jul 2026 21:24:41 -0700 Subject: [PATCH] chore(otel/v2): trim narrative test docstrings and stale assignment wording The two regression-test docstrings recounted the bug history and repro, which belongs in the PR description rather than the code; they now state only what the test checks. Also drops a leftover section comment and updates a few docstring phrases that still said "assignment" to describe the access grant, since access is the only routing input now. --- litellm/proxy/litellm_pre_call_utils.py | 2 +- .../integrations/otel/test_otel_v2_logger.py | 15 ++++----------- .../test_logging_exporter_access.py | 6 ------ .../proxy/test_litellm_pre_call_utils.py | 18 ++++++------------ 4 files changed, 11 insertions(+), 30 deletions(-) diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 71730f85d52..2f4582a1273 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -723,7 +723,7 @@ async def _apply_admin_logging_exporters( The destinations are set on a server-only ContextVar (never on ``data``), so they are neither request-shaped nor reachable by the provider body; the OTEL v2 router and the fan-out processor both read them from that ContextVar. Default-deny - means an identity with no assignment gets no per-tenant destination here. + means an identity no destination's access grants gets no per-tenant destination here. ``cached_destinations`` -- when ``user_api_key_auth`` already resolved the destinations on this request (the FastAPI path), reuse the result instead of 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 72f1ea5ca7d..95d6b598d5e 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_logger.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_logger.py @@ -987,17 +987,10 @@ def test_lazy_activation_emits_llm_span_when_destination_resolves(monkeypatch): 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).""" + """A close for a no-upstream request (``LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL`` set) + with a payload and resolved destinations must emit no gen-AI span. Mirrors + ``test_lazy_activation_emits_llm_span_when_destination_resolves`` with the marker + set: that one emits, this one must not.""" from litellm.constants import LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL logger, exporter = _logger() 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 64c471474ba..71a276fe447 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 @@ -97,12 +97,6 @@ def test_access_grants_not_global_when_false(): # --- routing scope decided entirely by access ------------------------------- -# -# Routing is access-only: a destination fires for exactly the identities its -# access grants. Empty access fires for no one (deny-all); proxy-wide routing -# must be requested explicitly with access.global=True. - - def test_empty_access_is_deny_all(): """Empty access grants no one: not proxy-wide.""" info = CredentialInfo(credential_type="logging") 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 dbaa69fc42a..d481f49d456 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -5341,15 +5341,9 @@ async def test_apply_admin_logging_exporters_stamps_and_activates( @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.""" + """Telemetry setup is best-effort: when the pre-call resolver raises a + non-``HTTPException``, ``_apply_admin_logging_exporters`` swallows it so the request + proceeds with no exception escaping, no backend activated, and request data untouched.""" import litellm.proxy.litellm_pre_call_utils as pcu from litellm.integrations.otel.plumbing.context import ( _request_destinations, @@ -5375,8 +5369,8 @@ async def test_client_cannot_control_otel_destinations(_seeded_logging_credentia """Y3 spoofing guard: a client cannot control OTEL export destinations. A request injects ``otel_destinations`` at the top level AND inside - ``litellm_metadata`` pointing at an attacker endpoint, for an identity with no - admin assignment. Destinations are admin-owned and resolved server-side, so the + ``litellm_metadata`` pointing at an attacker endpoint, for an identity no + destination grants. Destinations are admin-owned and resolved server-side, so the client value is wiped before the resolver runs; default-deny then adds nothing. The attacker endpoint must appear nowhere in the outgoing request. This drives the full ``add_litellm_data_to_request`` so the wipe-then-resolve ORDER is under @@ -5415,7 +5409,7 @@ async def test_client_cannot_control_otel_destinations(_seeded_logging_credentia user_api_key_dict = UserAPIKeyAuth( api_key="hashed-key", metadata={}, - team_metadata={}, # no logging_exporters assigned anywhere + team_metadata={}, spend=0.0, max_budget=100.0, model_max_budget={},