mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(otel v2): read the tenant's stored callback config the way the sibling parser does
Three divergences between the destination resolver and `convert_key_logging_metadata_to_callback`, which read the same stored config: - A key whose callbacks are disabled stores an empty list, and `or` treated that as "the key configured nothing", so the request inherited the team's destination. The sibling parser treats an empty list as configured. - Two entries naming one backend now merge their `callback_vars` last-wins, matching the sibling, instead of the resolver taking the first entry and the per-request tracer routing taking the last. - `credential_gated_exporters` dropped any exporter whose kind had no transport, which also dropped an `in_memory` exporter the operator asked for. The placeholder is the spec with every field still at its default, so that is what the predicate now says. Arize's `allow_missing_credentials` branch was unreachable: `get_arize_config` resolves every credential with `os.environ.get` and always supplies an endpoint, so it never raises. Dropped it and corrected the protocol docstring.
This commit is contained in:
parent
f5fb73f716
commit
1779fcf4a7
5 changed files with 100 additions and 56 deletions
|
|
@ -11,10 +11,7 @@ from litellm.integrations.otel.model.config import (
|
|||
ExporterSpec,
|
||||
OpenTelemetryV2Config,
|
||||
)
|
||||
from litellm.integrations.otel.presets.utils import (
|
||||
credential_gated_exporters,
|
||||
ensure_mappers,
|
||||
)
|
||||
from litellm.integrations.otel.presets.utils import ensure_mappers
|
||||
from litellm.types.utils import StandardCallbackDynamicParams
|
||||
|
||||
|
||||
|
|
@ -33,17 +30,7 @@ def arize_preset(
|
|||
) -> OpenTelemetryV2Config:
|
||||
base: Final = config_overrides or OpenTelemetryV2Config()
|
||||
mappers: Final = ensure_mappers(base.mapper_names, "openinference")
|
||||
try:
|
||||
arize_cfg: Final = _V1ArizeLogger.get_arize_config()
|
||||
except Exception:
|
||||
if not allow_missing_credentials:
|
||||
raise
|
||||
return base.model_copy(
|
||||
update={ # mutable-ok: pydantic model_copy takes a plain update mapping
|
||||
"exporters": credential_gated_exporters(base.exporters, ExporterOwner.ARIZE_AX),
|
||||
"mapper_names": mappers,
|
||||
}
|
||||
)
|
||||
arize_cfg: Final = _V1ArizeLogger.get_arize_config()
|
||||
headers: Final = _arize_headers(arize_cfg)
|
||||
return base.model_copy(
|
||||
update={
|
||||
|
|
|
|||
|
|
@ -19,9 +19,9 @@ class Preset(Protocol):
|
|||
``config_overrides`` lets one preset layer onto another's config (or onto
|
||||
test-supplied defaults); the factory calls presets with no arguments.
|
||||
|
||||
``allow_missing_credentials`` lets a credential-mandatory backend (langfuse /
|
||||
arize / weave) degrade to an exporter-less, mapper-only config instead of
|
||||
raising when the operator set no env credentials of their own. That is a real
|
||||
``allow_missing_credentials`` lets a credential-mandatory backend (langfuse and
|
||||
weave) degrade to an exporter-less, mapper-only config instead of raising when the
|
||||
operator set no env credentials of their own. That is a real
|
||||
deployment: every team brings its own account and the operator keeps none, and
|
||||
without it the whole V2 path silently falls back to the legacy integration, so
|
||||
no team destination is ever reached. Credential-optional backends ignore it.
|
||||
|
|
|
|||
|
|
@ -19,9 +19,7 @@ def ensure_mappers(mapper_names: Iterable[str], *names: str) -> list[str]:
|
|||
return result
|
||||
|
||||
|
||||
def credential_gated_exporters(
|
||||
exporters: "Iterable[ExporterSpec]", owner: "ExporterOwner"
|
||||
) -> "tuple[ExporterSpec, ...]":
|
||||
def credential_gated_exporters(exporters: "Iterable[ExporterSpec]", owner: "ExporterOwner") -> "list[ExporterSpec]":
|
||||
"""``exporters`` with the operator's destination replaced by a header-gated one.
|
||||
|
||||
Used when a credential-mandatory backend is asked to build without the operator's
|
||||
|
|
@ -31,31 +29,18 @@ def credential_gated_exporters(
|
|||
span would be printed to stdout, and the gated spec keeps the owner so the
|
||||
override filter still recognises which backend this provider speaks for.
|
||||
"""
|
||||
return (
|
||||
*(spec for spec in exporters if not _is_stdout_placeholder(spec)),
|
||||
return [
|
||||
*(spec for spec in exporters if not _is_unconfigured_placeholder(spec)),
|
||||
ExporterSpec(owner=owner, requires_headers=True),
|
||||
)
|
||||
]
|
||||
|
||||
|
||||
#: The fields ``OpenTelemetryV2Config._normalize`` fills the synthesized spec from.
|
||||
_SHORTHAND_FIELDS: Final = frozenset({"kind", "endpoint", "headers"})
|
||||
def _is_unconfigured_placeholder(spec: "ExporterSpec") -> bool:
|
||||
"""Whether ``spec`` is the one ``_normalize`` folds in when nothing was configured.
|
||||
|
||||
|
||||
def _is_stdout_placeholder(spec: "ExporterSpec") -> bool:
|
||||
"""Whether ``spec`` is the placeholder ``_normalize`` folds in for an empty list.
|
||||
|
||||
Two conditions. It must have nowhere to send a span, which is what
|
||||
``exporter_transport`` answers: an unrecognized or misspelled kind falls back to the
|
||||
console exporter, so comparing against the literal ``"console"`` would miss it. And
|
||||
every non-shorthand field must still be at its default, which is what says the
|
||||
operator did not ask for it: an exporter they configured survives, and so does the
|
||||
gated spec this module appends, which would otherwise eat itself when one preset
|
||||
layers onto another.
|
||||
Every field at its default is what says the operator asked for nothing: an exporter
|
||||
they did configure survives, whatever its kind, and so does the gated spec this
|
||||
module appends, which would otherwise eat itself when one preset layers onto
|
||||
another.
|
||||
"""
|
||||
from litellm.integrations.otel.plumbing.providers import exporter_transport
|
||||
|
||||
return (
|
||||
exporter_transport(spec.kind) == "headerless"
|
||||
and spec.endpoint is None
|
||||
and spec.model_dump(exclude_defaults=True).keys() <= _SHORTHAND_FIELDS
|
||||
)
|
||||
return not spec.model_dump(exclude_defaults=True)
|
||||
|
|
|
|||
|
|
@ -994,9 +994,14 @@ def resolve_tenant_otel_destinations(
|
|||
|
||||
Key settings win over team settings outright, the same precedence
|
||||
``_get_dynamic_logging_metadata`` applies, so one caller never exports the same
|
||||
backend to two accounts. Returns empty when OTEL V2 is off, when neither level
|
||||
named a destination-capable backend, or when the config is incomplete, and the
|
||||
request then keeps the operator's own exporters.
|
||||
backend to two accounts. An empty key-level list counts as configured, since that
|
||||
is what disabling a key's callbacks writes. Returns empty when OTEL V2 is off, when
|
||||
neither level named a destination-capable backend, or when the config is
|
||||
incomplete, and the request then keeps the operator's own exporters.
|
||||
|
||||
Two entries naming the same backend merge their ``callback_vars`` last-wins, the
|
||||
way ``convert_key_logging_metadata_to_callback`` merges them, so the destination
|
||||
and the per-request tracer routing cannot read one config two ways.
|
||||
|
||||
A ``failure``-only entry is skipped: a destination is resolved during auth, before
|
||||
the request has an outcome, so honouring the filter would mean holding every span
|
||||
|
|
@ -1009,23 +1014,33 @@ def resolve_tenant_otel_destinations(
|
|||
|
||||
if not is_otel_v2_enabled():
|
||||
return ()
|
||||
entries: Final = KeyAndTeamLoggingSettings.get_key_dynamic_logging_settings(
|
||||
user_api_key_dict
|
||||
) or KeyAndTeamLoggingSettings.get_team_dynamic_logging_settings(user_api_key_dict)
|
||||
key_entries: Final = KeyAndTeamLoggingSettings.get_key_dynamic_logging_settings(user_api_key_dict)
|
||||
entries: Final = (
|
||||
key_entries
|
||||
if key_entries is not None
|
||||
else KeyAndTeamLoggingSettings.get_team_dynamic_logging_settings(user_api_key_dict)
|
||||
)
|
||||
if not entries:
|
||||
return ()
|
||||
resolved: Final = tuple(
|
||||
destination
|
||||
callbacks: Final = tuple(
|
||||
callback
|
||||
for item in entries
|
||||
if (callback := _get_validated_callback_metadata(item=item, source="otel-destination")) is not None
|
||||
if callback.callback_type != "failure"
|
||||
if (destination := destination_for(callback.callback_name, _tenant_otel_params(callback.callback_vars)))
|
||||
is not None
|
||||
)
|
||||
merged: Final = {
|
||||
name: {
|
||||
var: value
|
||||
for callback in callbacks
|
||||
if callback.callback_name == name
|
||||
for var, value in callback.callback_vars.items()
|
||||
}
|
||||
for name in dict.fromkeys(callback.callback_name for callback in callbacks)
|
||||
}
|
||||
return tuple(
|
||||
destination
|
||||
for index, destination in enumerate(resolved)
|
||||
if destination.callback_name not in tuple(earlier.callback_name for earlier in resolved[:index])
|
||||
for name, callback_vars in merged.items()
|
||||
if (destination := destination_for(name, _tenant_otel_params(callback_vars))) is not None
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -507,6 +507,52 @@ class TestCallbackTypeFilter:
|
|||
assert resolve_tenant_otel_destinations(self._auth("failure")) == ()
|
||||
|
||||
|
||||
class TestTenantConfigAgreement:
|
||||
"""The destination resolver and ``convert_key_logging_metadata_to_callback`` read
|
||||
the same stored config, so they must not read it two different ways."""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _v2_on(self, monkeypatch):
|
||||
monkeypatch.setenv("LITELLM_OTEL_V2", "true")
|
||||
monkeypatch.setattr(
|
||||
litellm, "provider_url_destination_allowed_hosts", ["team.local", "key.local"], raising=False
|
||||
)
|
||||
is_otel_v2_enabled.cache_clear()
|
||||
yield
|
||||
is_otel_v2_enabled.cache_clear()
|
||||
|
||||
@staticmethod
|
||||
def _entry(host, **extra):
|
||||
return {
|
||||
"callback_name": "langfuse_otel",
|
||||
"callback_vars": {"langfuse_public_key": "pk", "langfuse_secret_key": "sk", "langfuse_host": host, **extra},
|
||||
}
|
||||
|
||||
def test_a_key_that_disabled_its_callbacks_does_not_fall_back_to_the_team(self):
|
||||
"""Disabling a key's callbacks stores an empty list, which the sibling parser
|
||||
reads as 'the key configured none'."""
|
||||
auth = UserAPIKeyAuth(
|
||||
metadata={"logging": []},
|
||||
team_metadata={"logging": [self._entry("http://team.local")]},
|
||||
)
|
||||
|
||||
assert resolve_tenant_otel_destinations(auth) == ()
|
||||
|
||||
def test_two_entries_for_one_backend_merge_their_vars_last_wins(self):
|
||||
auth = UserAPIKeyAuth(
|
||||
team_metadata={
|
||||
"logging": [
|
||||
self._entry("http://team.local"),
|
||||
{"callback_name": "langfuse_otel", "callback_vars": {"langfuse_host": "http://key.local"}},
|
||||
]
|
||||
}
|
||||
)
|
||||
|
||||
destinations = resolve_tenant_otel_destinations(auth)
|
||||
|
||||
assert [d.endpoint for d in destinations] == ["http://key.local/api/public/otel"]
|
||||
|
||||
|
||||
class TestEvictionSafety:
|
||||
def test_an_evicted_processor_is_retired_rather_than_shut_down(self):
|
||||
"""``on_end`` hands a processor back and exports outside the lock, so shutting
|
||||
|
|
@ -602,6 +648,17 @@ class TestCredentialGatedExporters:
|
|||
|
||||
assert kept[0] == operator_otlp
|
||||
|
||||
def test_an_in_memory_exporter_the_operator_asked_for_survives(self):
|
||||
"""``OTEL_EXPORTER=in_memory`` stores spans, so it is a destination the operator
|
||||
chose, not the placeholder that stands in for choosing nothing."""
|
||||
from litellm.integrations.otel.presets.utils import credential_gated_exporters
|
||||
|
||||
operator_memory = ExporterSpec(kind="in_memory", endpoint=None, headers=None)
|
||||
|
||||
kept = credential_gated_exporters((operator_memory,), ExporterOwner.LANGFUSE_OTEL)
|
||||
|
||||
assert kept[0] == operator_memory
|
||||
|
||||
def test_the_synthesized_stdout_placeholder_is_dropped(self):
|
||||
from litellm.integrations.otel.presets.utils import credential_gated_exporters
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue