From cf472733a91d86e647ea01c3076462cc1fc80298 Mon Sep 17 00:00:00 2001 From: yucheng Date: Mon, 28 Sep 2026 21:30:11 +0000 Subject: [PATCH] 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> --- litellm/integrations/otel/logger.py | 24 +++---- litellm/integrations/otel/model/config.py | 13 ---- .../integrations/otel/plumbing/providers.py | 5 +- litellm/litellm_core_utils/litellm_logging.py | 6 +- litellm/proxy/common_utils/callback_utils.py | 5 -- .../test_otel_excluded_services.py | 21 ++++++ .../otel/test_otel_v2_destinations.py | 70 +++++++++---------- 7 files changed, 67 insertions(+), 77 deletions(-) diff --git a/litellm/integrations/otel/logger.py b/litellm/integrations/otel/logger.py index 13d3b49e074..fa5770f9b81 100644 --- a/litellm/integrations/otel/logger.py +++ b/litellm/integrations/otel/logger.py @@ -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() diff --git a/litellm/integrations/otel/model/config.py b/litellm/integrations/otel/model/config.py index 763f095e666..cde366b099e 100644 --- a/litellm/integrations/otel/model/config.py +++ b/litellm/integrations/otel/model/config.py @@ -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) diff --git a/litellm/integrations/otel/plumbing/providers.py b/litellm/integrations/otel/plumbing/providers.py index 66b69b7c729..25878e8a302 100644 --- a/litellm/integrations/otel/plumbing/providers.py +++ b/litellm/integrations/otel/plumbing/providers.py @@ -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)): diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index e3d7eefb4bb..28d72702f3e 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -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) diff --git a/litellm/proxy/common_utils/callback_utils.py b/litellm/proxy/common_utils/callback_utils.py index cbe5dca2edc..cb8b51d092e 100644 --- a/litellm/proxy/common_utils/callback_utils.py +++ b/litellm/proxy/common_utils/callback_utils.py @@ -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) diff --git a/tests/integration/observability/test_otel_excluded_services.py b/tests/integration/observability/test_otel_excluded_services.py index 7f1288978fd..16864f1635c 100644 --- a/tests/integration/observability/test_otel_excluded_services.py +++ b/tests/integration/observability/test_otel_excluded_services.py @@ -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, diff --git a/tests/unit/integrations/otel/test_otel_v2_destinations.py b/tests/unit/integrations/otel/test_otel_v2_destinations.py index bb6a992f7b4..87e89626ad9 100644 --- a/tests/unit/integrations/otel/test_otel_v2_destinations.py +++ b/tests/unit/integrations/otel/test_otel_v2_destinations.py @@ -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()