mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(otel v2): leave callback init and boot untouched when excluded_services is unset
Read callback_settings.otel.excluded_services directly instead of making the otel callback build its own logger, and drop the new boot-time parse of callback_settings.otel, so a proxy without the setting behaves exactly as on main Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
b17c4b3d25
commit
cf472733a9
7 changed files with 67 additions and 77 deletions
|
|
@ -905,26 +905,24 @@ def publish_global_otel_v2_provider(
|
|||
attach_tenant_fan_out(
|
||||
logger.tracer_provider,
|
||||
*_v2_configs(in_memory_loggers, logger),
|
||||
excluded_db_systems=_excluded_db_systems(in_memory_loggers, logger),
|
||||
excluded_db_systems=_excluded_db_systems(logger),
|
||||
)
|
||||
set_global_provider(logger.tracer_provider)
|
||||
_published_v2_provider = logger.tracer_provider # rebind-ok: startup records the one provider carrying the fan-out
|
||||
return logger
|
||||
|
||||
|
||||
def _excluded_db_systems(in_memory_loggers: Sequence[object], logger: "OpenTelemetryV2") -> frozenset[str]:
|
||||
def _excluded_db_systems(logger: "OpenTelemetryV2") -> frozenset[str]:
|
||||
"""The datastore services withheld from tenant destinations.
|
||||
|
||||
Proxy-wide the set lives on exactly one config: the ``otel`` callback's, the
|
||||
only one ``callback_settings.otel`` writes. A preset (langfuse_otel, …) builds
|
||||
its config env-only, so reading ``excluded_services`` off every logger and
|
||||
unioning them reintroduces the ``LITELLM_OTEL_EXCLUDED_SERVICES`` value that
|
||||
``callback_settings.otel`` already overrode on the ``otel`` config. Falls back
|
||||
to ``logger`` when no ``otel`` callback exists.
|
||||
``callback_settings.otel.excluded_services`` wins over the env var whichever
|
||||
logger got published: with ``callbacks: [langfuse_otel, otel]`` the ``otel``
|
||||
callback folds into the preset, whose config is env-only.
|
||||
"""
|
||||
loggers: Final = tuple(cb for cb in in_memory_loggers if isinstance(cb, OpenTelemetryV2))
|
||||
owner: Final = next((cb for cb in (logger, *loggers) if cb.callback_name == "otel"), logger)
|
||||
return owner.config.excluded_services
|
||||
configured: Final = litellm.callback_settings.get("otel", {}).get("excluded_services")
|
||||
if configured is None:
|
||||
return logger.config.excluded_services
|
||||
return OpenTelemetryV2Config(excluded_services=configured).excluded_services
|
||||
|
||||
|
||||
def _v2_configs(in_memory_loggers: Sequence[object], logger: "OpenTelemetryV2") -> tuple[OpenTelemetryV2Config, ...]:
|
||||
|
|
@ -986,12 +984,10 @@ def fan_out_provider() -> ApiTracerProvider:
|
|||
return published
|
||||
logger: Final = _registered_v2_logger()
|
||||
if logger is not None:
|
||||
from litellm.litellm_core_utils.litellm_logging import _in_memory_loggers
|
||||
|
||||
attach_tenant_fan_out(
|
||||
logger.tracer_provider,
|
||||
logger.config,
|
||||
excluded_db_systems=_excluded_db_systems(_in_memory_loggers, logger),
|
||||
excluded_db_systems=_excluded_db_systems(logger),
|
||||
)
|
||||
return logger.tracer_provider
|
||||
return get_tracer_provider()
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
"""Typed configuration for the OpenTelemetry instrumentation."""
|
||||
|
||||
from collections.abc import Mapping
|
||||
from enum import Enum
|
||||
from functools import lru_cache
|
||||
from typing import Annotated, Any, Final
|
||||
|
|
@ -374,15 +373,3 @@ def _db_system_for_excluded_service(service: str) -> str | None:
|
|||
"excluded_services: %r is not a datastore service; ignored. Allowed: postgres, redis", service
|
||||
)
|
||||
return resolved
|
||||
|
||||
|
||||
def validate_otel_v2_callback_settings(settings: object) -> None:
|
||||
"""Parse ``callback_settings.otel`` so a malformed block fails proxy boot.
|
||||
|
||||
Logger construction is lazy and swallows init errors, so without this a bad
|
||||
value in the shared settings only surfaces as a dropped callback at request
|
||||
time.
|
||||
"""
|
||||
if not is_otel_v2_enabled() or not isinstance(settings, Mapping):
|
||||
return
|
||||
OpenTelemetryV2Config(**settings)
|
||||
|
|
|
|||
|
|
@ -1178,9 +1178,8 @@ def attach_tenant_fan_out(
|
|||
so exactly one fan-out lands. ``configs`` name the operator's own exporters, one
|
||||
config per v2 logger since each keeps its own provider and still writes its
|
||||
account, so an additive destination pointing at any of them is delivered once
|
||||
rather than twice. ``excluded_db_systems`` comes from the ``otel`` callback's
|
||||
config alone (see ``_excluded_db_systems``); unioning it across every logger's
|
||||
config would reintroduce the env value that ``callback_settings.otel`` overrode.
|
||||
rather than twice. ``excluded_db_systems`` only filters what the fan-out
|
||||
delivers, never the operator's own exporters.
|
||||
"""
|
||||
with _FAN_OUT_ATTACH_LOCK:
|
||||
if any(isinstance(processor, TenantFanOutSpanProcessor) for processor in _attached_processors(provider)):
|
||||
|
|
|
|||
|
|
@ -4764,13 +4764,11 @@ def _init_custom_logger_compatible_class(
|
|||
from litellm.integrations.otel.model.config import OpenTelemetryV2Config
|
||||
|
||||
for callback in _in_memory_loggers:
|
||||
if isinstance(callback, OpenTelemetryV2) and callback.callback_name == "otel":
|
||||
if isinstance(callback, OpenTelemetryV2):
|
||||
return callback
|
||||
otel_settings: Final = _get_custom_logger_settings_from_proxy_server(callback_name=logging_integration)
|
||||
otel_logger_v2: Final = build_otel_v2_logger(
|
||||
config=OpenTelemetryV2Config(**otel_settings),
|
||||
callback_name=logging_integration,
|
||||
settings=otel_settings,
|
||||
config=OpenTelemetryV2Config(**otel_settings), settings=otel_settings
|
||||
)
|
||||
_in_memory_loggers.append(otel_logger_v2)
|
||||
_maybe_auto_initialize_arize_phoenix(_in_memory_loggers)
|
||||
|
|
|
|||
|
|
@ -174,11 +174,6 @@ def initialize_callbacks_on_proxy(
|
|||
imported_list.append(code_interpreter_interception_obj)
|
||||
continue
|
||||
|
||||
if isinstance(callback, str) and callback == "otel":
|
||||
from litellm.integrations.otel.model.config import validate_otel_v2_callback_settings
|
||||
|
||||
validate_otel_v2_callback_settings(callback_specific_params.get("otel"))
|
||||
|
||||
# check if callback is a custom logger compatible callback
|
||||
if isinstance(callback, str):
|
||||
callback = LoggingCallbackManager._add_custom_callback_generic_api_str(callback)
|
||||
|
|
|
|||
|
|
@ -211,6 +211,27 @@ def test_excluded_services_drops_db_spans_at_tenant_only(
|
|||
assert not any("batch_write_to_db" in name for name in names), f"spend writer reached tenant: {names}"
|
||||
|
||||
|
||||
@pytest.mark.timeout(180)
|
||||
def test_without_excluded_services_the_tenant_still_gets_redis_and_postgres_spans(
|
||||
gateway: Gateway,
|
||||
audit_sinks: SpanSinks,
|
||||
otel_audit_config: AuditConfigWriter,
|
||||
langfuse_vars: dict[str, JsonValue],
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
config: Final = _config_with(tmp_path, otel_audit_config, extra=_guardrail_block)
|
||||
with owned_proxy(gateway, tmp_path, {"LITELLM_OTEL_V2": "1"}, config=config, workers=2) as candidate:
|
||||
tenant_start, _ = recorded_spans(audit_sinks.tenant)
|
||||
traffic: Final = _drive(candidate, langfuse_vars)
|
||||
_await_db_span(audit_sinks.tenant, None, "batch_write_to_db", seconds=60, since=tenant_start)
|
||||
tenant_trace: Final = _trace_id(audit_sinks.tenant, traffic)
|
||||
_await_db_span(audit_sinks.tenant, tenant_trace, "redis", seconds=60)
|
||||
_assert_core_spans_present(_trace_spans(audit_sinks.tenant, tenant_trace, seconds=15))
|
||||
_, all_tenant = recorded_spans(audit_sinks.tenant, tenant_start)
|
||||
systems: Final = _db_systems(all_tenant)
|
||||
assert {"redis", "postgresql"} <= systems, f"datastore spans missing at tenant: {systems}"
|
||||
|
||||
|
||||
def test_env_excluded_services_drops_only_redis(
|
||||
gateway: Gateway,
|
||||
audit_sinks: SpanSinks,
|
||||
|
|
|
|||
|
|
@ -1069,29 +1069,19 @@ class TestProviderWiring:
|
|||
assert kinds(published).count("TenantFanOutSpanProcessor") == 1
|
||||
assert "TenantFanOutSpanProcessor" not in kinds(other)
|
||||
|
||||
def test_excluded_services_come_from_the_otel_callback_config_only(self):
|
||||
"""A preset builds its config env-only, so unioning ``excluded_services``
|
||||
across loggers reintroduces the env value ``callback_settings.otel``
|
||||
overrode. The fan-out must take the set from the ``otel`` config alone."""
|
||||
otel = OpenTelemetryV2(
|
||||
config=OpenTelemetryV2Config(exporters=[ExporterSpec(kind="in_memory")], excluded_services=["postgres"]),
|
||||
callback_name="otel",
|
||||
)
|
||||
preset = OpenTelemetryV2(
|
||||
config=OpenTelemetryV2Config(exporters=[ExporterSpec(kind="in_memory")], excluded_services=["redis"]),
|
||||
callback_name="langfuse_otel",
|
||||
)
|
||||
|
||||
publish_global_otel_v2_provider([preset], lambda _p: None, registered=otel)
|
||||
|
||||
fan_out = next(
|
||||
@staticmethod
|
||||
def _fan_out_of(logger: OpenTelemetryV2) -> TenantFanOutSpanProcessor:
|
||||
return next(
|
||||
processor
|
||||
for processor in otel._tracer_provider._active_span_processor._span_processors
|
||||
for processor in logger._tracer_provider._active_span_processor._span_processors
|
||||
if isinstance(processor, TenantFanOutSpanProcessor)
|
||||
)
|
||||
assert fan_out._excluded_db_systems == frozenset({"postgresql"})
|
||||
|
||||
def test_excluded_services_fall_back_to_the_published_logger_without_an_otel_callback(self):
|
||||
def test_callback_settings_excluded_services_win_over_the_published_preset_env_config(self, monkeypatch):
|
||||
"""A preset builds its config env-only, so the fan-out must read
|
||||
``callback_settings.otel.excluded_services`` itself rather than the
|
||||
published logger's config, or the env value would win."""
|
||||
monkeypatch.setattr(litellm, "callback_settings", {"otel": {"excluded_services": ["postgres"]}}, raising=False)
|
||||
preset = OpenTelemetryV2(
|
||||
config=OpenTelemetryV2Config(exporters=[ExporterSpec(kind="in_memory")], excluded_services=["redis"]),
|
||||
callback_name="langfuse_otel",
|
||||
|
|
@ -1099,25 +1089,30 @@ class TestProviderWiring:
|
|||
|
||||
publish_global_otel_v2_provider([], lambda _p: None, registered=preset)
|
||||
|
||||
fan_out = next(
|
||||
processor
|
||||
for processor in preset._tracer_provider._active_span_processor._span_processors
|
||||
if isinstance(processor, TenantFanOutSpanProcessor)
|
||||
)
|
||||
assert fan_out._excluded_db_systems == frozenset({"redis"})
|
||||
assert self._fan_out_of(preset)._excluded_db_systems == frozenset({"postgresql"})
|
||||
|
||||
def test_otel_callback_builds_its_own_logger_after_a_preset(self, monkeypatch):
|
||||
"""With ``callbacks: [langfuse_otel, otel]`` the otel branch reused any
|
||||
V2 logger, so ``callback_settings.otel`` (excluded_services) was dropped
|
||||
onto the preset's env-only config."""
|
||||
from litellm.integrations.otel.logger import _excluded_db_systems
|
||||
def test_excluded_services_fall_back_to_the_published_logger_config_without_callback_settings(self, monkeypatch):
|
||||
monkeypatch.setattr(litellm, "callback_settings", {"otel": {"exporter": "in_memory"}}, raising=False)
|
||||
preset = OpenTelemetryV2(
|
||||
config=OpenTelemetryV2Config(exporters=[ExporterSpec(kind="in_memory")], excluded_services=["redis"]),
|
||||
callback_name="langfuse_otel",
|
||||
)
|
||||
|
||||
publish_global_otel_v2_provider([], lambda _p: None, registered=preset)
|
||||
|
||||
assert self._fan_out_of(preset)._excluded_db_systems == frozenset({"redis"})
|
||||
|
||||
def test_otel_after_a_preset_reuses_it_and_still_takes_callback_settings_exclusions(self, monkeypatch):
|
||||
"""``callbacks: [langfuse_otel, otel]`` keeps one v2 logger, exactly as
|
||||
before ``excluded_services`` existed, and the exclusion still comes from
|
||||
``callback_settings.otel`` rather than the preset's env-only config."""
|
||||
from litellm.litellm_core_utils import litellm_logging as logging_module
|
||||
|
||||
logging_module._in_memory_loggers.clear()
|
||||
monkeypatch.setenv("LITELLM_OTEL_V2", "true")
|
||||
monkeypatch.setenv("LANGFUSE_PUBLIC_KEY", "pk")
|
||||
monkeypatch.setenv("LANGFUSE_SECRET_KEY", "sk")
|
||||
monkeypatch.delenv("LITELLM_OTEL_EXCLUDED_SERVICES", raising=False)
|
||||
monkeypatch.setenv("LITELLM_OTEL_EXCLUDED_SERVICES", "redis")
|
||||
is_otel_v2_enabled.cache_clear()
|
||||
monkeypatch.setattr(litellm, "callback_settings", {"otel": {"excluded_services": ["postgres"]}}, raising=False)
|
||||
try:
|
||||
|
|
@ -1133,13 +1128,12 @@ class TestProviderWiring:
|
|||
preset = init("langfuse_otel")
|
||||
otel_cb = init("otel")
|
||||
|
||||
assert otel_cb is not None and otel_cb is not preset
|
||||
v2_names = {
|
||||
cb.callback_name for cb in logging_module._in_memory_loggers if isinstance(cb, OpenTelemetryV2)
|
||||
}
|
||||
assert {"langfuse_otel", "otel"} <= v2_names, v2_names
|
||||
resolved = _excluded_db_systems(logging_module._in_memory_loggers, otel_cb)
|
||||
assert resolved == frozenset({"postgresql"}), resolved
|
||||
assert isinstance(preset, OpenTelemetryV2)
|
||||
assert otel_cb is preset
|
||||
v2_loggers = [cb for cb in logging_module._in_memory_loggers if isinstance(cb, OpenTelemetryV2)]
|
||||
assert v2_loggers == [preset], v2_loggers
|
||||
publish_global_otel_v2_provider(logging_module._in_memory_loggers, lambda _p: None, registered=preset)
|
||||
assert self._fan_out_of(preset)._excluded_db_systems == frozenset({"postgresql"})
|
||||
finally:
|
||||
logging_module._in_memory_loggers.clear()
|
||||
is_otel_v2_enabled.cache_clear()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue