mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
fix(otel/v2): detach credential-routed tenant spans into their own trace (#38847)
* fix(otel/v2): detach credential-routed tenant spans into their own trace Multi-tenant OTel v2 routes a team or key's LLM-call span to that tenant's own vendor account (New Relic, Arize, Langfuse, Weave) via dynamic OTLP credential headers, while the request-root, auth, and db spans stay on the operator's default backend. The span was still parented into the request trace, so the tenant account received a child whose parent it never got, and New Relic rendered it as a fragmented trace with a missing parent. Detach a credential-routed span the same way a project-routed (Phoenix) span already detaches: root a fresh trace in the tenant account and link back to the request trace for correlation. Service-name routing keeps parenting, since it only relabels service.name on the same operator backend where the parent is present. Guard the detach on the callback actually owning an OTLP exporter the credentials can reach: a callback owning only a console or in_memory exporter has nowhere to stamp them, so the span would export to the default backend unchanged and detaching would orphan it on the very backend that holds its parent. In that case warn once and keep the default tracer. * fix(otel/v2): derive tenant-route routability from resolved exporter transport A denylist classified an owned exporter as routable whenever its kind was not console/in_memory, so a typo'd or unavailable kind (e.g. "otlp", "grcp") passed the check while _exporter_from_spec falls it back to a header-ignoring console exporter. Detaching such a span would root a fresh trace that only ever reaches the operator console, never the tenant backend, orphaning it on both sides. Route on a shared exporter_transport() predicate that resolves the kind the same way _exporter_from_spec builds it (registered factories + otlp_http aliases -> http, otlp_grpc aliases -> grpc, else headerless), so an unresolvable kind is headerless and stays parented. Fixes the same latent gap in project routability.
This commit is contained in:
yucheng-berri
GitHub
parent
d619b1c227
commit
3c2fa5fafb
3 changed files with 216 additions and 17 deletions
|
|
@ -1,7 +1,7 @@
|
|||
"""Provider / exporter factory + the Baggage span processor."""
|
||||
|
||||
from collections.abc import Callable, Iterable
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
from typing import TYPE_CHECKING, Any, Final, Literal
|
||||
|
||||
from opentelemetry import _logs, baggage, metrics
|
||||
from opentelemetry._events import EventLogger
|
||||
|
|
@ -135,14 +135,36 @@ def parse_headers(raw: str | None) -> dict[str, str]:
|
|||
return dict(parse_env_headers(raw, liberal=True))
|
||||
|
||||
|
||||
_IN_MEMORY_KINDS: Final = ("in_memory", "inmemory", "memory")
|
||||
_OTLP_HTTP_KINDS: Final = ("otlp_http", "http", "http/protobuf", "http/json")
|
||||
_OTLP_GRPC_KINDS: Final = ("otlp_grpc", "grpc")
|
||||
|
||||
|
||||
def exporter_transport(kind: str) -> Literal["http", "grpc", "headerless"]:
|
||||
"""How an exporter of this ``kind`` carries credentials, per ``_exporter_from_spec``.
|
||||
|
||||
``http``/``grpc`` exporters (and any registered factory, which builds an
|
||||
OTLP exporter) stamp ``spec.headers``; ``console``, ``in_memory``, and any
|
||||
unrecognized kind (which falls back to a header-ignoring console exporter)
|
||||
are ``headerless``. Routability decisions must read this rather than a
|
||||
denylist, so a typo'd or unavailable kind is not mistaken for OTLP.
|
||||
"""
|
||||
resolved: Final = kind.lower()
|
||||
if resolved in _OTLP_HTTP_KINDS or resolved in _EXPORTER_FACTORIES:
|
||||
return "http"
|
||||
if resolved in _OTLP_GRPC_KINDS:
|
||||
return "grpc"
|
||||
return "headerless"
|
||||
|
||||
|
||||
def _exporter_from_spec(spec: ExporterSpec) -> SpanExporter:
|
||||
kind: Final = (spec.kind or "console").lower()
|
||||
factory: Final = _EXPORTER_FACTORIES.get(kind)
|
||||
if factory is not None:
|
||||
return factory(spec)
|
||||
if kind in ("in_memory", "inmemory", "memory"):
|
||||
if kind in _IN_MEMORY_KINDS:
|
||||
return InMemorySpanExporter()
|
||||
if kind in ("otlp_http", "http", "http/protobuf", "http/json"):
|
||||
if kind in _OTLP_HTTP_KINDS:
|
||||
from opentelemetry.exporter.otlp.proto.http.trace_exporter import (
|
||||
OTLPSpanExporter as HTTPExporter,
|
||||
)
|
||||
|
|
@ -151,7 +173,7 @@ def _exporter_from_spec(spec: ExporterSpec) -> SpanExporter:
|
|||
endpoint=_otlp_traces_endpoint(spec.endpoint),
|
||||
headers=parse_headers(spec.headers),
|
||||
)
|
||||
if kind in ("otlp_grpc", "grpc"):
|
||||
if kind in _OTLP_GRPC_KINDS:
|
||||
from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import (
|
||||
OTLPSpanExporter as GRPCExporter,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ from litellm.constants import OTEL_SERVICE_NAME_METADATA_KEYS
|
|||
from litellm.integrations.otel.model.config import ExporterSpec, OpenTelemetryV2Config
|
||||
from litellm.integrations.otel.plumbing.providers import (
|
||||
build_tracer_provider,
|
||||
exporter_transport,
|
||||
get_tracer,
|
||||
)
|
||||
from litellm.integrations.otel.presets import (
|
||||
|
|
@ -121,13 +122,27 @@ def _encoded_header_string(headers: Mapping[str, str]) -> str:
|
|||
class TenantRoute:
|
||||
"""The tracer to create a span on, plus whether it must root its own trace.
|
||||
|
||||
``detached`` is True when project routing engaged. Phoenix assigns a whole
|
||||
``detached`` is True when the routed span exports to a DIFFERENT backend
|
||||
than the request's root span, which always exports through the default
|
||||
tracer. A detached span roots a fresh trace with a link back to the request
|
||||
trace for correlation, so the destination account is not left holding a
|
||||
child whose parent it never received. It is driven by whether routing
|
||||
headers were actually applied to an owned exporter, not merely requested:
|
||||
a credential or project route whose callback owns no exporter those headers
|
||||
can reach exports through the default backend unchanged, so it stays
|
||||
parented like an unrouted span.
|
||||
|
||||
Credential routing (a team/key's own vendor account) is one detaching case:
|
||||
the root, auth, and db spans stay on the operator's default backend while
|
||||
the LLM-call span exports to the tenant's account, so parenting it into the
|
||||
request trace makes the tenant account show a fragmented span with a missing
|
||||
parent. Project routing (Phoenix) is the other: Phoenix assigns a whole
|
||||
trace to one project by whichever of its spans arrives first, so a
|
||||
project-routed span parented into the request trace gets dragged into the
|
||||
project of the default-exported request spans and the header does nothing.
|
||||
The span must therefore start a fresh trace (with a link back to the
|
||||
request trace for correlation) — which is also how the v1 Phoenix logger
|
||||
behaved, exporting each request under its own Phoenix-local parent span.
|
||||
Both mirror the v1 loggers, which exported each request under its own
|
||||
backend-local root. Service-name routing does NOT detach: it relabels
|
||||
``service.name`` on the SAME operator backend, where the parent is present.
|
||||
"""
|
||||
|
||||
tracer: Tracer
|
||||
|
|
@ -161,11 +176,20 @@ class TenantTracerCache:
|
|||
self._open_span_counts: dict[TracerProvider, int] = {} # mutable-ok: live refcount state
|
||||
# Oldest-first so an overflow of draining providers sheds the stalest.
|
||||
self._retired: OrderedDict[TracerProvider, None] = OrderedDict() # mutable-ok: draining evicted providers
|
||||
self._project_routable = any(
|
||||
spec.owner == callback_name and spec.kind.lower() not in (*_NON_OTLP_KINDS, *_GRPC_KINDS)
|
||||
for spec in config.exporters
|
||||
# An owned exporter is routable only when its kind actually resolves to a
|
||||
# header-carrying OTLP exporter. A denylist would accept a typo'd or
|
||||
# unavailable kind, which ``_exporter_from_spec`` falls back to a
|
||||
# header-ignoring console exporter: detaching such a span would strand it
|
||||
# on the operator's console, never reaching the tenant backend. Project
|
||||
# headers are HTTP-only; credentials ride gRPC metadata too (Arize's
|
||||
# default exporter is gRPC), so they accept either OTLP transport.
|
||||
owned_transports: Final = tuple(
|
||||
exporter_transport(spec.kind) for spec in config.exporters if spec.owner == callback_name
|
||||
)
|
||||
self._project_routable = "http" in owned_transports
|
||||
self._credential_routable = "http" in owned_transports or "grpc" in owned_transports
|
||||
self._warned_project_unroutable = False
|
||||
self._warned_credential_unroutable = False
|
||||
|
||||
def release(self, provider: TracerProvider | None) -> None:
|
||||
"""Drop one open-span count; shut a retired provider down once drained.
|
||||
|
|
@ -207,7 +231,7 @@ class TenantTracerCache:
|
|||
concurrent overflow eviction can't shut it down between selection and
|
||||
the caller's span start. The caller must ``release`` it exactly once.
|
||||
"""
|
||||
credential_headers: Final = dynamic_otlp_headers(self._callback_name, dynamic_params) or _NO_HEADERS
|
||||
credential_headers: Final = self._credential_headers(dynamic_params)
|
||||
project_headers: Final = self._project_headers(auth_metadata)
|
||||
service_name: Final = tenant_service_name(auth_metadata)
|
||||
if not credential_headers and not project_headers and service_name is None:
|
||||
|
|
@ -231,7 +255,7 @@ class TenantTracerCache:
|
|||
_shutdown_provider(evicted)
|
||||
return TenantRoute(
|
||||
tracer=get_tracer(provider, self._tracer_name),
|
||||
detached=bool(project_headers),
|
||||
detached=bool(project_headers) or bool(credential_headers),
|
||||
provider=provider,
|
||||
)
|
||||
|
||||
|
|
@ -275,6 +299,26 @@ class TenantTracerCache:
|
|||
self._open_span_counts.pop(overflowed, None)
|
||||
return overflowed
|
||||
|
||||
def _credential_headers(self, dynamic_params: StandardCallbackDynamicParams | None) -> Mapping[str, str]:
|
||||
"""The per-request dynamic OTLP credentials, if this cache can apply them.
|
||||
|
||||
A callback owning only a console/in_memory exporter has nowhere to stamp
|
||||
them, so the span would export to the operator's default backend
|
||||
unchanged; routing there and detaching would orphan it on the very
|
||||
backend that holds its parent. Warn once and keep the default tracer.
|
||||
"""
|
||||
requested: Final = dynamic_otlp_headers(self._callback_name, dynamic_params) or _NO_HEADERS
|
||||
if not requested or self._credential_routable:
|
||||
return requested
|
||||
if not self._warned_credential_unroutable:
|
||||
self._warned_credential_unroutable = True
|
||||
verbose_logger.warning(
|
||||
"OTel V2: %s request carries dynamic credentials, but the callback owns no "
|
||||
"OTLP exporter to stamp them onto; spans export to the default backend.",
|
||||
self._callback_name,
|
||||
)
|
||||
return _NO_HEADERS
|
||||
|
||||
def _project_headers(self, auth_metadata: Mapping[str, str] | None) -> Mapping[str, str]:
|
||||
"""The per-request project-routing headers, if this cache can apply them.
|
||||
|
||||
|
|
|
|||
|
|
@ -2,21 +2,33 @@
|
|||
|
||||
import base64
|
||||
|
||||
|
||||
import pytest
|
||||
from opentelemetry.trace import NoOpTracer
|
||||
|
||||
from litellm.integrations.otel.model.config import ExporterSpec, OpenTelemetryV2Config
|
||||
from litellm.integrations.otel.plumbing.providers import parse_headers
|
||||
from litellm.integrations.otel.plumbing.routing import TenantTracerCache
|
||||
from litellm.integrations.otel.presets import (
|
||||
DYNAMIC_HEADERS_BY_CALLBACK,
|
||||
dynamic_otlp_endpoint,
|
||||
dynamic_otlp_headers,
|
||||
project_routing_headers,
|
||||
)
|
||||
from litellm.integrations.otel.plumbing.providers import parse_headers
|
||||
from litellm.integrations.otel.plumbing.routing import TenantTracerCache
|
||||
|
||||
|
||||
def _cache(callback_name, exporters=None):
|
||||
cfg = OpenTelemetryV2Config(exporters=exporters or [ExporterSpec(kind="in_memory")])
|
||||
# A credential-routing callback always contributes an owned OTLP exporter
|
||||
# from its preset, so default the fixture to one (a simple processor, no
|
||||
# background flush thread); otherwise its dynamic credentials have nowhere
|
||||
# to stamp and the route stays on the default tracer.
|
||||
if exporters is None:
|
||||
owned = (
|
||||
[ExporterSpec(kind="otlp_http", owner=callback_name, use_simple_processor=True)]
|
||||
if callback_name in DYNAMIC_HEADERS_BY_CALLBACK
|
||||
else []
|
||||
)
|
||||
exporters = [ExporterSpec(kind="in_memory"), *owned]
|
||||
cfg = OpenTelemetryV2Config(exporters=exporters)
|
||||
return TenantTracerCache(cfg, callback_name, "litellm")
|
||||
|
||||
|
||||
|
|
@ -507,6 +519,127 @@ def test_newrelic_provider_cached_per_key_and_region():
|
|||
assert len(cache._providers) == 3
|
||||
|
||||
|
||||
# --- credential routes must detach: their tenant backend never receives the --- #
|
||||
# --- operator-side request-root span, so a parented LLM span is orphaned. --- #
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"callback, dynamic_params",
|
||||
[
|
||||
("newrelic", {"newrelic_api_key": "NRAL-KEY"}),
|
||||
("arize", {"arize_space_id": "S", "arize_api_key": "K"}),
|
||||
("langfuse_otel", {"langfuse_public_key": "pk", "langfuse_secret_key": "sk"}),
|
||||
("weave_otel", {"wandb_api_key": "w", "weave_project_id": "p"}),
|
||||
],
|
||||
)
|
||||
def test_credential_route_detaches_from_request_trace(callback, dynamic_params):
|
||||
# The request root, auth, guardrail and db spans stay on the operator's
|
||||
# default backend; a credential-routed LLM span exports to the tenant's own
|
||||
# account, which never sees that root. Parenting it there leaves it
|
||||
# orphaned ("Missing parent"/fragmented), so a credential route must root
|
||||
# its own trace and link back, exactly as a Phoenix project route does.
|
||||
cache = _cache(
|
||||
callback,
|
||||
exporters=[
|
||||
ExporterSpec(kind="in_memory"),
|
||||
ExporterSpec(kind="otlp_http", owner=callback, use_simple_processor=True),
|
||||
],
|
||||
)
|
||||
default = NoOpTracer()
|
||||
routed = cache.route_for(default, dynamic_params)
|
||||
assert routed.tracer is not default
|
||||
assert routed.detached is True # own trace + link back, never parented cross-account
|
||||
cache.release(routed.provider)
|
||||
|
||||
|
||||
def test_credential_route_without_owned_otlp_exporter_stays_parented():
|
||||
# A callback owning only a console/in_memory exporter has nowhere to stamp
|
||||
# the dynamic credentials, so the span exports to the operator's default
|
||||
# backend unchanged. Detaching there would orphan it on the very backend
|
||||
# that holds its parent, so it must stay parented (mirrors the project guard).
|
||||
cache = _cache("newrelic", exporters=[ExporterSpec(kind="in_memory")])
|
||||
default = NoOpTracer()
|
||||
routed = cache.route_for(default, {"newrelic_api_key": "NRAL-KEY"})
|
||||
assert routed.tracer is default # no scoped provider built
|
||||
assert routed.detached is False
|
||||
assert cache._providers == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("typo_kind", ["otlp", "grcp", "htttp", "otlphttp"])
|
||||
def test_credential_route_with_unresolvable_exporter_kind_stays_parented(typo_kind):
|
||||
# An owned exporter whose kind does not resolve to a real OTLP exporter
|
||||
# (a typo or an unavailable protocol) falls back to a header-ignoring
|
||||
# console exporter, so the dynamic credentials never reach a tenant backend.
|
||||
# A denylist would wrongly treat it as routable and detach the span onto the
|
||||
# operator's console, orphaning it; routability must instead follow the same
|
||||
# kind resolution the exporter build uses.
|
||||
cache = _cache(
|
||||
"newrelic",
|
||||
exporters=[ExporterSpec(kind="in_memory"), ExporterSpec(kind=typo_kind, owner="newrelic")],
|
||||
)
|
||||
default = NoOpTracer()
|
||||
routed = cache.route_for(default, {"newrelic_api_key": "NRAL-KEY"})
|
||||
assert routed.tracer is default # no scoped provider built
|
||||
assert routed.detached is False
|
||||
assert cache._providers == {}
|
||||
|
||||
|
||||
def test_credential_routed_span_roots_new_trace_and_links_back():
|
||||
# Beyond the detached flag: an emitted credential-routed span must actually
|
||||
# root its own trace (a fresh trace id, no parent) and carry a link back to
|
||||
# the request trace, so the tenant account can correlate it without holding
|
||||
# the operator-side root it never received.
|
||||
from opentelemetry import trace
|
||||
from opentelemetry.sdk.trace import TracerProvider
|
||||
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
|
||||
from opentelemetry.sdk.trace.export.in_memory_span_exporter import (
|
||||
InMemorySpanExporter,
|
||||
)
|
||||
|
||||
from litellm.integrations.otel.logger import _request_trace_links
|
||||
|
||||
default_exporter = InMemorySpanExporter()
|
||||
default_provider = TracerProvider()
|
||||
default_provider.add_span_processor(SimpleSpanProcessor(default_exporter))
|
||||
default = default_provider.get_tracer("litellm")
|
||||
|
||||
cache = _cache(
|
||||
"newrelic",
|
||||
exporters=[ExporterSpec(kind="otlp_http", owner="newrelic", use_simple_processor=True)],
|
||||
)
|
||||
with default.start_as_current_span("chat gemini-flash") as request_root:
|
||||
request_ctx = trace.set_span_in_context(request_root)
|
||||
route = cache.route_for(default, {"newrelic_api_key": "NRAL-KEY"})
|
||||
assert route.detached is True
|
||||
from opentelemetry.trace import INVALID_SPAN, set_span_in_context
|
||||
|
||||
with route.tracer.start_as_current_span(
|
||||
"chat gemini-flash",
|
||||
context=set_span_in_context(INVALID_SPAN, request_ctx),
|
||||
links=_request_trace_links(request_ctx),
|
||||
) as tenant_span:
|
||||
tenant_ctx = tenant_span.get_span_context()
|
||||
cache.release(route.provider)
|
||||
|
||||
root_ctx = request_root.get_span_context()
|
||||
assert tenant_ctx.trace_id != root_ctx.trace_id # fresh trace, not parented
|
||||
(link,) = tenant_span.links
|
||||
assert link.context.trace_id == root_ctx.trace_id # linked back to the request trace
|
||||
|
||||
|
||||
def test_service_name_route_stays_parented_unlike_credential_route():
|
||||
# Guard the boundary the fix must NOT cross: service.name routing relabels
|
||||
# the span on the SAME operator backend, where the request root is present,
|
||||
# so it stays parented. Only credential/project routes (different backend)
|
||||
# detach.
|
||||
cache = _cache("otel")
|
||||
default = NoOpTracer()
|
||||
routed = cache.route_for(default, None, {"otel_service_name": "payments-gateway"})
|
||||
assert routed.tracer is not default
|
||||
assert routed.detached is False
|
||||
cache.release(routed.provider)
|
||||
|
||||
|
||||
def test_requires_headers_spec_skipped_without_headers():
|
||||
from litellm.integrations.otel.plumbing.providers import build_tracer_provider
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue