mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-29 06:33:28 +00:00
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.
This commit is contained in:
parent
7af6c8dcc0
commit
7d0a0ddfa5
6 changed files with 123 additions and 59 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
(
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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",)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue