mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-13 03:59:12 +00:00
fix(otel): harden destination callback var validation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
37addcfb6f
commit
95be09c25c
6 changed files with 74 additions and 18 deletions
|
|
@ -311,8 +311,9 @@ 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`). Unknown names are logged
|
||||
as an error and ignored. A span is withheld when its `db.system.name` /
|
||||
`db.system.name` spellings `redis` / `postgresql`). Unknown global names are logged
|
||||
as an error and ignored. Team and key callback values are validated when saved,
|
||||
and invalid values are rejected. 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. Team and key destinations can replace the
|
||||
global set with `excluded_services` in their own `callback_vars`; `"none"`
|
||||
|
|
|
|||
|
|
@ -104,7 +104,7 @@ def validate_excluded_services_value(value: str) -> None:
|
|||
|
||||
names: Final = tuple(name.strip().lower() for name in value.split(","))
|
||||
invalid_names: Final = invalid_excluded_service_names(value)
|
||||
allowed: Final = "redis, postgres, postgresql, batch_write_to_db, redis-prefixed service names, or none"
|
||||
allowed: Final = "redis, postgres, postgresql, batch_write_to_db, redis_-prefixed service names, or none"
|
||||
if invalid_names or ("none" in names and len(names) != 1):
|
||||
raise ValueError(
|
||||
f"Invalid excluded_services {value!r}: use {allowed}; 'none' must be used alone and names cannot be empty"
|
||||
|
|
|
|||
|
|
@ -8,13 +8,10 @@ per-integration checks here.
|
|||
import math
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from types import MappingProxyType
|
||||
from typing import Final
|
||||
|
||||
from pydantic import TypeAdapter
|
||||
from typing import Final, cast # noqa: TID251 # preserve arbitrary keys from validated callback mappings
|
||||
|
||||
from litellm.types.utils import CAPTURE_MESSAGE_CONTENT_VAR, EXCLUDED_SERVICES_VAR
|
||||
|
||||
_STRING_OBJECT_MAPPING_ADAPTER: Final = TypeAdapter(dict[str, object])
|
||||
_NEWRELIC_CALLBACK: Final = "newrelic"
|
||||
_NEWRELIC_VAR_PREFIX: Final = "newrelic_"
|
||||
_LANGFUSE_OTEL_CALLBACK: Final = "langfuse_otel"
|
||||
|
|
@ -338,20 +335,28 @@ def _entry_callback_vars(entry: object) -> Mapping[str, str]:
|
|||
def _logging_entry_error(entry: object) -> str | None:
|
||||
if not isinstance(entry, Mapping):
|
||||
return None
|
||||
typed_entry: Final[Mapping[str, object]] = _STRING_OBJECT_MAPPING_ADAPTER.validate_python(entry)
|
||||
typed_entry: Final[Mapping[object, object]] = (
|
||||
cast( # cast-ok: entry is Mapping-checked but may have non-string keys
|
||||
Mapping[object, object], entry
|
||||
)
|
||||
)
|
||||
callback_name: Final = typed_entry.get("callback_name")
|
||||
callback_vars_raw: Final = typed_entry.get("callback_vars")
|
||||
if not isinstance(callback_name, str) or not isinstance(callback_vars_raw, Mapping):
|
||||
return None
|
||||
callback_vars: Final[Mapping[str, object]] = _STRING_OBJECT_MAPPING_ADAPTER.validate_python(callback_vars_raw)
|
||||
if CAPTURE_MESSAGE_CONTENT_VAR in callback_vars:
|
||||
capture_message_content: Final = callback_vars[CAPTURE_MESSAGE_CONTENT_VAR]
|
||||
if not isinstance(capture_message_content, str):
|
||||
return f"{CAPTURE_MESSAGE_CONTENT_VAR} must be a string, got {type(capture_message_content).__name__}"
|
||||
if EXCLUDED_SERVICES_VAR in callback_vars:
|
||||
excluded_services: Final = callback_vars[EXCLUDED_SERVICES_VAR]
|
||||
if not isinstance(excluded_services, str):
|
||||
return f"{EXCLUDED_SERVICES_VAR} must be a string, got {type(excluded_services).__name__}"
|
||||
callback_vars: Final[Mapping[object, object]] = cast( # cast-ok: Mapping check preserves arbitrary key types
|
||||
Mapping[object, object], callback_vars_raw
|
||||
)
|
||||
destination_var_error: Final = next(
|
||||
(
|
||||
f"{variable_name} must be a string, got {type(callback_vars[variable_name]).__name__}"
|
||||
for variable_name in (CAPTURE_MESSAGE_CONTENT_VAR, EXCLUDED_SERVICES_VAR)
|
||||
if variable_name in callback_vars and not isinstance(callback_vars[variable_name], str)
|
||||
),
|
||||
None,
|
||||
)
|
||||
if destination_var_error is not None:
|
||||
return destination_var_error
|
||||
callback_type_raw: Final = typed_entry.get("callback_type")
|
||||
return callback_config_error(
|
||||
callback_name,
|
||||
|
|
|
|||
|
|
@ -596,7 +596,9 @@ class TestFanOut:
|
|||
def run():
|
||||
set_request_destinations(
|
||||
(
|
||||
OtelDestination(endpoint="http://team.local", excluded_db_systems=frozenset({"redis", "postgresql"})),
|
||||
OtelDestination(
|
||||
endpoint="http://team.local", excluded_db_systems=frozenset({"redis", "postgresql"})
|
||||
),
|
||||
OtelDestination(endpoint="http://inherited.local"),
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -360,6 +360,21 @@ def test_key_logging_metadata_rejects_non_string_destination_callback_vars(callb
|
|||
assert error == f"{callback_var} must be a string, got NoneType"
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("otel_v2_on")
|
||||
def test_key_logging_metadata_with_non_string_callback_var_key_does_not_raise() -> None:
|
||||
error: Final = logging_metadata_config_error(
|
||||
{
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langfuse_otel",
|
||||
"callback_vars": {1: "x", "excluded_services": "redis"},
|
||||
}
|
||||
]
|
||||
}
|
||||
)
|
||||
assert error is None
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("otel_v2_on")
|
||||
def test_each_entry_keeps_its_own_capture_message_content() -> None:
|
||||
error: Final = logging_metadata_config_error(
|
||||
|
|
|
|||
|
|
@ -1686,6 +1686,39 @@ async def test_a_second_entry_may_not_flip_the_span_scope(patched_prisma, caller
|
|||
patched_prisma.db.litellm_teamtable.update.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("otel_v2_on")
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_second_newrelic_entry_may_not_change_excluded_services(patched_prisma) -> None:
|
||||
patched_prisma.get_data = AsyncMock(
|
||||
return_value=_team_row(
|
||||
metadata={
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "newrelic",
|
||||
"callback_type": "success",
|
||||
"callback_vars": {"newrelic_api_key": "license-key", "excluded_services": "redis"},
|
||||
}
|
||||
]
|
||||
}
|
||||
)
|
||||
)
|
||||
data: Final = AddTeamCallback(
|
||||
callback_name="newrelic",
|
||||
callback_type="success_and_failure",
|
||||
callback_vars={"newrelic_api_key": "license-key", "excluded_services": "postgres"},
|
||||
)
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await add_team_callbacks(
|
||||
data=data,
|
||||
http_request=Mock(spec=Request),
|
||||
team_id="team-victim",
|
||||
user_api_key_dict=_admin_auth(),
|
||||
)
|
||||
assert exc.value.status_code == 400
|
||||
assert "excluded_services" in str(exc.value.detail)
|
||||
patched_prisma.db.litellm_teamtable.update.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_team_callbacks_rejects_out_of_range_arize_sampling_rate(patched_prisma):
|
||||
data = AddTeamCallback(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue