From 212d6610c66cc68e479f5689f92b0c3d1fff8979 Mon Sep 17 00:00:00 2001 From: yucheng Date: Sat, 26 Sep 2026 20:18:38 +0000 Subject: [PATCH] fix(otel): log and drop unknown excluded_services instead of failing boot Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/integrations/otel/README.md | 7 +- litellm/integrations/otel/model/config.py | 40 +++---- litellm/proxy/common_utils/callback_utils.py | 11 -- .../test_otel_excluded_services.py | 104 ++++++++++-------- ..._v2_config_baggage_parenting_guardrails.py | 68 +++--------- 5 files changed, 96 insertions(+), 134 deletions(-) diff --git a/litellm/integrations/otel/README.md b/litellm/integrations/otel/README.md index 33fa38f8848..1ce23a9f74f 100644 --- a/litellm/integrations/otel/README.md +++ b/litellm/integrations/otel/README.md @@ -210,9 +210,10 @@ nothing here imports outside it: `LITELLM_OTEL_EXCLUDED_SERVICES` (comma-separated) or `excluded_services` (a YAML list) under `callback_settings.otel`, naming the datastore services to withhold (`redis`, `postgres`, `batch_write_to_db`, `redis_*`, or their - `db.system.name` spellings `redis` / `postgresql`). A span is withheld when - its `db.system.name` / `db.system` attribute is in the set, so request root, - auth, guardrail and model spans can never be excluded. + `db.system.name` spellings `redis` / `postgresql`). Unknown names are logged + as an error and ignored. A span is withheld when its `db.system.name` / + `db.system` attribute is in the set, so request root, auth, guardrail and + model spans can never be excluded. - [`baggage.py`](./model/baggage.py) — the single definition of which request-identity values are promoted into Baggage (so child spans inherit them) and under which attribute keys. diff --git a/litellm/integrations/otel/model/config.py b/litellm/integrations/otel/model/config.py index 528ac76256f..763f095e666 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.""" -import os from collections.abc import Mapping from enum import Enum from functools import lru_cache @@ -9,6 +8,7 @@ from typing import Annotated, Any, Final from pydantic import AliasChoices, BaseModel, Field, field_validator, model_validator from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict +from litellm._logging import verbose_logger from litellm.integrations.otel.model.baggage import ( BAGGAGE_PROMOTED_KEYS, DEFAULT_BAGGAGE_METADATA_KEYS, @@ -358,36 +358,22 @@ def _normalize_excluded_services(services: frozenset[str]) -> frozenset[str]: ``postgres`` and ``postgresql`` name the same system, as do every ``ServiceTypes`` member that ``db_system`` maps. Anything else means the - operator pointed the setting at a span family it cannot cover. + operator pointed the setting at a span family it cannot cover; those names + are logged and dropped so a typo cannot take the proxy down. """ - return frozenset(_db_system_for_excluded_service(service) for service in services) - - -def _db_system_for_excluded_service(service: str) -> str: - resolved: Final = db_system(service) if service != POSTGRESQL else POSTGRESQL - if resolved is None: - raise ValueError(f"excluded_services: {service!r} is not a datastore service; allowed: postgres, redis") + resolved: Final = frozenset( + system for service in services if (system := _db_system_for_excluded_service(service)) is not None + ) return resolved -def validate_otel_v2_excluded_services_env(settings: object) -> None: - """Validate ``LITELLM_OTEL_EXCLUDED_SERVICES`` at boot even with no ``otel`` callback. - - Preset-only deployments build env-only configs through a path that swallows - init errors, so a bogus value would otherwise degrade to the legacy callback - silently. Splitting and normalizing here raises the same ``ValueError`` the - field raises. An explicit ``callback_settings.otel.excluded_services`` wins - over the env var, so the caller skips this check only when no V2 preset - callback that would still parse the env is configured. - """ - if not is_otel_v2_enabled(): - return - if isinstance(settings, Mapping) and "excluded_services" in settings: - return - raw: Final = os.environ.get("LITELLM_OTEL_EXCLUDED_SERVICES") - if not raw: - return - _normalize_excluded_services(frozenset(item.strip() for item in raw.split(",") if item.strip())) +def _db_system_for_excluded_service(service: str) -> str | None: + resolved: Final = db_system(service) if service != POSTGRESQL else POSTGRESQL + if resolved is None: + verbose_logger.error( + "excluded_services: %r is not a datastore service; ignored. Allowed: postgres, redis", service + ) + return resolved def validate_otel_v2_callback_settings(settings: object) -> None: diff --git a/litellm/proxy/common_utils/callback_utils.py b/litellm/proxy/common_utils/callback_utils.py index 837361e7232..cbe5dca2edc 100644 --- a/litellm/proxy/common_utils/callback_utils.py +++ b/litellm/proxy/common_utils/callback_utils.py @@ -148,11 +148,6 @@ def initialize_callbacks_on_proxy( verbose_proxy_logger.debug("%sinitializing callbacks=%s on proxy%s", blue_color_code, value, reset_color_code) if isinstance(value, list): - from litellm.integrations.otel.presets import PRESET_BY_CALLBACK - - preset_present: Final = any( - isinstance(entry, str) and entry != "otel" and entry in PRESET_BY_CALLBACK for entry in value - ) imported_list: Final[list[Any]] = [] for callback in value: # ["presidio", ] if isinstance(callback, str) and callback == "compression_interception": @@ -184,12 +179,6 @@ def initialize_callbacks_on_proxy( validate_otel_v2_callback_settings(callback_specific_params.get("otel")) - from litellm.integrations.otel.model.config import validate_otel_v2_excluded_services_env - - validate_otel_v2_excluded_services_env( - callback_specific_params.get("otel") if "otel" in value and not preset_present else None - ) - # 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 0473a99a7ef..572c81b1e93 100644 --- a/tests/integration/observability/test_otel_excluded_services.py +++ b/tests/integration/observability/test_otel_excluded_services.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os import time import uuid from collections.abc import Callable, Iterator, Mapping @@ -273,20 +272,27 @@ def test_excluded_services_applies_with_preset_ordered_first( ) -def test_bogus_excluded_service_fails_proxy_start( - gateway: Gateway, otel_audit_config: AuditConfigWriter, tmp_path: Path +def test_bogus_excluded_service_logs_error_and_drops_at_proxy_start( + 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, otel={"excluded_services": ["auth"]}) - log_dir: Final = Path(os.environ.get("INTEGRATION_RESULTS_DIR", str(tmp_path))) - before: Final = frozenset(log_dir.glob("owned-proxy-*.log")) - with pytest.raises(AssertionError, match="readiness"): - with owned_proxy_process(gateway, tmp_path, {"LITELLM_OTEL_V2": "1"}, config=config, workers=2): - pass - logs: Final = [path.read_text() for path in frozenset(log_dir.glob("owned-proxy-*.log")) - before] - assert logs, "no owned proxy log written" - text: Final = "\n".join(logs) - assert "'auth' is not a datastore service" in text, text[-3000:] - assert "postgres, redis" in text, text[-3000:] + config: Final = _config_with(tmp_path, otel_audit_config, otel={"excluded_services": ["auth", "postgres"]}) + with owned_proxy_process(gateway, tmp_path, {"LITELLM_OTEL_V2": "1"}, config=config, workers=2) as owned: + assert "'auth' is not a datastore service; ignored" in owned.log.read_text(), owned.log.read_text()[-3000:] + start, _ = recorded_spans(audit_sinks.tenant) + traffic: Final = _drive(owned.gateway, langfuse_vars) + tenant_trace: Final = _trace_id(audit_sinks.tenant, traffic) + _await_db_span(audit_sinks.tenant, tenant_trace, "redis", seconds=60) + tenant_spans: Final = _trace_spans(audit_sinks.tenant, tenant_trace, seconds=15) + _, all_tenant = recorded_spans(audit_sinks.tenant, start) + systems: Final = _db_systems(tenant_spans) + assert "redis" in systems, f"redis spans missing at tenant: {systems}" + assert "postgresql" not in _db_systems(all_tenant), ( + f"postgresql spans reached tenant: {_db_systems(all_tenant)}" + ) def test_valid_config_excluded_services_tolerates_bogus_env( @@ -313,46 +319,58 @@ def test_valid_config_excluded_services_tolerates_bogus_env( ) -def test_bogus_excluded_services_env_fails_proxy_start_with_preset_alongside_otel( - gateway: Gateway, otel_audit_config: AuditConfigWriter, tmp_path: Path +def test_bogus_excluded_services_env_logs_and_drops_with_preset_alongside_otel( + gateway: Gateway, + audit_sinks: SpanSinks, + otel_audit_config: AuditConfigWriter, + langfuse_vars: dict[str, JsonValue], + tmp_path: Path, ) -> None: def with_langfuse_otel(config: dict) -> None: config["litellm_settings"]["callbacks"] = ["otel", "langfuse_otel"] - config: Final = _config_with( - tmp_path, otel_audit_config, otel={"excluded_services": ["postgres"]}, extra=with_langfuse_otel - ) - log_dir: Final = Path(os.environ.get("INTEGRATION_RESULTS_DIR", str(tmp_path))) - before: Final = frozenset(log_dir.glob("owned-proxy-*.log")) - with pytest.raises(AssertionError, match="readiness"): - with owned_proxy_process( - gateway, tmp_path, {"LITELLM_OTEL_V2": "1", "LITELLM_OTEL_EXCLUDED_SERVICES": "auth"}, config=config, workers=2 - ): - pass - logs: Final = [path.read_text() for path in frozenset(log_dir.glob("owned-proxy-*.log")) - before] - assert logs, "no owned proxy log written" - text: Final = "\n".join(logs) - assert "'auth' is not a datastore service" in text, text[-3000:] + config: Final = _config_with(tmp_path, otel_audit_config, extra=with_langfuse_otel) + overrides: Final = {"LITELLM_OTEL_V2": "1", "LITELLM_OTEL_EXCLUDED_SERVICES": "auth,postgres"} + with owned_proxy_process(gateway, tmp_path, overrides, config=config, workers=2) as owned: + assert "'auth' is not a datastore service; ignored" in owned.log.read_text(), owned.log.read_text()[-3000:] + start, _ = recorded_spans(audit_sinks.tenant) + traffic: Final = _drive(owned.gateway, langfuse_vars) + tenant_trace: Final = _trace_id(audit_sinks.tenant, traffic) + _await_db_span(audit_sinks.tenant, tenant_trace, "redis", seconds=60) + tenant_spans: Final = _trace_spans(audit_sinks.tenant, tenant_trace, seconds=15) + _, all_tenant = recorded_spans(audit_sinks.tenant, start) + systems: Final = _db_systems(tenant_spans) + assert "redis" in systems, f"redis spans missing at tenant: {systems}" + assert "postgresql" not in _db_systems(all_tenant), ( + f"postgresql spans reached tenant: {_db_systems(all_tenant)}" + ) -def test_bogus_excluded_services_env_fails_proxy_start_without_otel_callback( - gateway: Gateway, otel_audit_config: AuditConfigWriter, tmp_path: Path +def test_bogus_excluded_services_env_logs_and_drops_without_otel_callback( + gateway: Gateway, + audit_sinks: SpanSinks, + otel_audit_config: AuditConfigWriter, + langfuse_vars: dict[str, JsonValue], + tmp_path: Path, ) -> None: def presets_only(config: dict) -> None: config["litellm_settings"]["callbacks"] = ["langfuse_otel"] config: Final = _config_with(tmp_path, otel_audit_config, extra=presets_only) - log_dir: Final = Path(os.environ.get("INTEGRATION_RESULTS_DIR", str(tmp_path))) - before: Final = frozenset(log_dir.glob("owned-proxy-*.log")) - with pytest.raises(AssertionError, match="readiness"): - with owned_proxy_process( - gateway, tmp_path, {"LITELLM_OTEL_V2": "1", "LITELLM_OTEL_EXCLUDED_SERVICES": "auth"}, config=config, workers=2 - ): - pass - logs: Final = [path.read_text() for path in frozenset(log_dir.glob("owned-proxy-*.log")) - before] - assert logs, "no owned proxy log written" - text: Final = "\n".join(logs) - assert "'auth' is not a datastore service" in text, text[-3000:] + overrides: Final = {"LITELLM_OTEL_V2": "1", "LITELLM_OTEL_EXCLUDED_SERVICES": "auth,postgres"} + with owned_proxy_process(gateway, tmp_path, overrides, config=config, workers=2) as owned: + assert "'auth' is not a datastore service; ignored" in owned.log.read_text(), owned.log.read_text()[-3000:] + start, _ = recorded_spans(audit_sinks.tenant) + traffic: Final = _drive(owned.gateway, langfuse_vars) + tenant_trace: Final = _trace_id(audit_sinks.tenant, traffic) + _await_db_span(audit_sinks.tenant, tenant_trace, "redis", seconds=60) + tenant_spans: Final = _trace_spans(audit_sinks.tenant, tenant_trace, seconds=15) + _, all_tenant = recorded_spans(audit_sinks.tenant, start) + systems: Final = _db_systems(tenant_spans) + assert "redis" in systems, f"redis spans missing at tenant: {systems}" + assert "postgresql" not in _db_systems(all_tenant), ( + f"postgresql spans reached tenant: {_db_systems(all_tenant)}" + ) def test_postgres_exclusion_covers_batch_write_to_db( diff --git a/tests/unit/integrations/otel/test_otel_v2_config_baggage_parenting_guardrails.py b/tests/unit/integrations/otel/test_otel_v2_config_baggage_parenting_guardrails.py index c85394e8233..3ec088eca0b 100644 --- a/tests/unit/integrations/otel/test_otel_v2_config_baggage_parenting_guardrails.py +++ b/tests/unit/integrations/otel/test_otel_v2_config_baggage_parenting_guardrails.py @@ -11,6 +11,7 @@ """ import asyncio +import logging import pytest @@ -22,17 +23,17 @@ from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( # noqa: E4 ) from litellm.integrations.otel import LiteLLM, OpenTelemetryV2Config # noqa: E402 -from litellm.integrations.otel.plumbing import providers # noqa: E402 +from litellm.integrations.otel.logger import OpenTelemetryV2 # noqa: E402 from litellm.integrations.otel.model.baggage import ( # noqa: E402 BAGGAGE_PROMOTED_KEYS, DEFAULT_BAGGAGE_METADATA_KEYS, ) -from litellm.integrations.otel.logger import OpenTelemetryV2 # noqa: E402 from litellm.integrations.otel.model.payloads import GuardrailSpanData # noqa: E402 from litellm.integrations.otel.model.spans import ( # noqa: E402 LITELLM_PROXY_REQUEST_SPAN_NAME, SpanRole, ) +from litellm.integrations.otel.plumbing import providers # noqa: E402 # --------------------------------------------------------------------------- # # Area 1 — baggage allowlists configurable @@ -74,13 +75,11 @@ def test_baggage_keys_from_config_yaml_kwargs(): def test_baggage_processor_allowlist_uses_config_keys(): - cfg = OpenTelemetryV2Config( - exporter="in_memory", baggage_promoted_keys=[LiteLLM.TEAM_ID] - ) + cfg = OpenTelemetryV2Config(exporter="in_memory", baggage_promoted_keys=[LiteLLM.TEAM_ID]) provider, exporter = providers.in_memory_provider(cfg) - from litellm.integrations.otel.plumbing import context as ctx_mod from litellm.integrations.otel.emitter import SpanEmitter from litellm.integrations.otel.model.payloads import ServiceSpanData + from litellm.integrations.otel.plumbing import context as ctx_mod engine = SpanEmitter(providers.get_tracer(provider, "t"), cfg) ctx = ctx_mod.set_request_baggage({LiteLLM.TEAM_ID: "t1", LiteLLM.TEAM_ALIAS: "ta"}) @@ -115,46 +114,19 @@ def test_excluded_services_config_wins_over_env(monkeypatch): assert OpenTelemetryV2Config(excluded_services=["postgres"]).excluded_services == frozenset({"postgresql"}) -def test_excluded_services_rejects_a_non_datastore_service(): - with pytest.raises(Exception, match="'auth' is not a datastore service; allowed: postgres, redis"): - OpenTelemetryV2Config(excluded_services=["auth"]) +def test_excluded_services_drops_a_non_datastore_service_and_logs(caplog): + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + config = OpenTelemetryV2Config(excluded_services=["auth", "redis"]) + assert config.excluded_services == frozenset({"redis"}) + assert any("'auth' is not a datastore service; ignored" in record.message for record in caplog.records) -def test_excluded_services_env_is_validated_at_boot_when_enabled(monkeypatch): - from litellm.integrations.otel.model.config import is_otel_v2_enabled, validate_otel_v2_excluded_services_env - - monkeypatch.setenv("LITELLM_OTEL_V2", "1") - monkeypatch.setenv("LITELLM_OTEL_EXCLUDED_SERVICES", "auth") - is_otel_v2_enabled.cache_clear() - try: - with pytest.raises(ValueError, match="'auth' is not a datastore service; allowed: postgres, redis"): - validate_otel_v2_excluded_services_env(None) - finally: - is_otel_v2_enabled.cache_clear() - - -def test_excluded_services_env_validation_accepts_datastore_names(monkeypatch): - from litellm.integrations.otel.model.config import is_otel_v2_enabled, validate_otel_v2_excluded_services_env - - monkeypatch.setenv("LITELLM_OTEL_V2", "1") - monkeypatch.setenv("LITELLM_OTEL_EXCLUDED_SERVICES", "redis, postgres") - is_otel_v2_enabled.cache_clear() - try: - validate_otel_v2_excluded_services_env(None) - finally: - is_otel_v2_enabled.cache_clear() - - -def test_excluded_services_env_bad_value_is_inert_when_config_wins(monkeypatch): - from litellm.integrations.otel.model.config import is_otel_v2_enabled, validate_otel_v2_excluded_services_env - - monkeypatch.setenv("LITELLM_OTEL_V2", "1") - monkeypatch.setenv("LITELLM_OTEL_EXCLUDED_SERVICES", "auth") - is_otel_v2_enabled.cache_clear() - try: - validate_otel_v2_excluded_services_env({"excluded_services": ["postgres"]}) - finally: - is_otel_v2_enabled.cache_clear() +def test_excluded_services_env_drops_a_bad_value_and_logs(monkeypatch, caplog): + monkeypatch.setenv("LITELLM_OTEL_EXCLUDED_SERVICES", "auth,postgres") + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + config = OpenTelemetryV2Config() + assert config.excluded_services == frozenset({"postgresql"}) + assert any("'auth' is not a datastore service; ignored" in record.message for record in caplog.records) # --------------------------------------------------------------------------- # @@ -191,9 +163,7 @@ def test_passthrough_llm_span_parents_to_ambient_server_span(): later (possibly detached) success callback only closes the already-parented span, so it never becomes a separate root trace.""" logger, exporter = _logger() - server = logger._emitter.start_span( - SpanRole.PROXY_REQUEST, LITELLM_PROXY_REQUEST_SPAN_NAME - ) + server = logger._emitter.start_span(SpanRole.PROXY_REQUEST, LITELLM_PROXY_REQUEST_SPAN_NAME) kwargs = { "standard_logging_object": _payload(), "litellm_params": {"metadata": {}}, @@ -217,9 +187,7 @@ def test_llm_span_unaffected_by_phase_span_active_at_close(): successor to the old auth-failure-401 case where the LLM log nested under ``auth``: the span is now born after auth, parented to the request root.""" logger, exporter = _logger() - server = logger._emitter.start_span( - SpanRole.PROXY_REQUEST, LITELLM_PROXY_REQUEST_SPAN_NAME - ) + server = logger._emitter.start_span(SpanRole.PROXY_REQUEST, LITELLM_PROXY_REQUEST_SPAN_NAME) kwargs = { "standard_logging_object": _payload(), "litellm_params": {"metadata": {}},