mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix(otel v2): compare the effective span scope across callback entries
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
d2cd70f8f0
commit
00195b1788
6 changed files with 93 additions and 38 deletions
|
|
@ -406,7 +406,6 @@ _SCOPES: Final[tuple[OtelSpanScope, ...]] = get_args(OtelSpanScope)
|
|||
|
||||
|
||||
def tenant_span_scope_default() -> OtelSpanScope:
|
||||
"""The ``span_scope`` a tenant destination gets when its own callback vars name none."""
|
||||
import litellm
|
||||
|
||||
configured: Final = litellm.otel_tenant_span_scope or os.environ.get(OTEL_TENANT_SPAN_SCOPE_ENV)
|
||||
|
|
|
|||
|
|
@ -106,11 +106,6 @@ def _otel_span_scope_error(callback_name: str | None, callback_vars: Mapping[str
|
|||
|
||||
|
||||
def _alias_conflict_error(callback_vars: Mapping[str, str]) -> str | None:
|
||||
"""Reject one entry that names both scope aliases with different values.
|
||||
|
||||
``otel_span_scope`` outranks ``langfuse_span_scope`` when they disagree, so
|
||||
storing both would export the one and silently drop the other.
|
||||
"""
|
||||
langfuse_scope: Final = callback_vars.get(_LANGFUSE_SPAN_SCOPE_VAR)
|
||||
otel_scope: Final = callback_vars.get(_OTEL_SPAN_SCOPE_VAR)
|
||||
if langfuse_scope is None or otel_scope is None or langfuse_scope == otel_scope:
|
||||
|
|
@ -211,36 +206,28 @@ def cross_entry_family_error(
|
|||
)
|
||||
|
||||
|
||||
def _effective_span_scope(callback_name: str | None, callback_vars: Mapping[str, str]) -> str | None:
|
||||
langfuse_scope: Final = (
|
||||
callback_vars.get(_LANGFUSE_SPAN_SCOPE_VAR) if callback_name == _LANGFUSE_OTEL_CALLBACK else None
|
||||
)
|
||||
return callback_vars.get(_OTEL_SPAN_SCOPE_VAR) or langfuse_scope
|
||||
|
||||
|
||||
def conflicting_span_scope_error(
|
||||
callback_name: str | None,
|
||||
callback_vars: Mapping[str, str] | None,
|
||||
stored_vars_by_entry: Sequence[Mapping[str, str]],
|
||||
) -> str | None:
|
||||
return _conflicting_var_error(_LANGFUSE_SPAN_SCOPE_VAR, callback_vars, stored_vars_by_entry)
|
||||
|
||||
|
||||
def conflicting_otel_span_scope_error(
|
||||
callback_vars: Mapping[str, str] | None,
|
||||
stored_vars_by_entry: Sequence[Mapping[str, str]],
|
||||
) -> str | None:
|
||||
"""``stored_vars_by_entry`` must hold only the entries of the same callback: the
|
||||
request merges the var per backend, so two backends may legitimately disagree."""
|
||||
return _conflicting_var_error(_OTEL_SPAN_SCOPE_VAR, callback_vars, stored_vars_by_entry)
|
||||
|
||||
|
||||
def _conflicting_var_error(
|
||||
var: str,
|
||||
callback_vars: Mapping[str, str] | None,
|
||||
stored_vars_by_entry: Sequence[Mapping[str, str]],
|
||||
) -> str | None:
|
||||
incoming: Final = None if callback_vars is None else callback_vars.get(var)
|
||||
incoming: Final = None if callback_vars is None else _effective_span_scope(callback_name, callback_vars)
|
||||
if incoming is None:
|
||||
return None
|
||||
return next(
|
||||
(
|
||||
f"{var} is already set to {stored!r} by another callback entry. "
|
||||
f"span scope is already set to {stored!r} by another {callback_name} callback entry "
|
||||
f"({_LANGFUSE_SPAN_SCOPE_VAR} and {_OTEL_SPAN_SCOPE_VAR} name the same setting). "
|
||||
f"Every entry shares one value: remove that entry or send the same value."
|
||||
for entry in stored_vars_by_entry
|
||||
if (stored := entry.get(var)) not in (None, incoming)
|
||||
if (stored := _effective_span_scope(callback_name, entry)) not in (None, incoming)
|
||||
),
|
||||
None,
|
||||
)
|
||||
|
|
@ -260,9 +247,9 @@ def logging_metadata_config_error(metadata: Mapping[str, object] | None) -> str
|
|||
error
|
||||
for error in (
|
||||
*(_logging_entry_error(entry) for entry in entries),
|
||||
*(conflicting_span_scope_error(entry_vars[i], entry_vars[:i]) for i in range(len(entry_vars))),
|
||||
*(
|
||||
conflicting_otel_span_scope_error(
|
||||
conflicting_span_scope_error(
|
||||
entry_names[i],
|
||||
entry_vars[i],
|
||||
tuple(vars_ for vars_, name in zip(entry_vars[:i], entry_names[:i]) if name == entry_names[i]),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@ from litellm.proxy._types import (
|
|||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.callback_config_validation import (
|
||||
callback_config_error,
|
||||
conflicting_otel_span_scope_error,
|
||||
conflicting_span_scope_error,
|
||||
cross_entry_family_error,
|
||||
)
|
||||
|
|
@ -354,10 +353,8 @@ async def add_team_callbacks(
|
|||
stored_entry_vars: Final = [ # mutable-ok: read-only input to the checks, never stored
|
||||
entry.get("callback_vars") or {} for entry in stored_entries
|
||||
]
|
||||
scope_error: Final = conflicting_span_scope_error(data.callback_vars, stored_entry_vars)
|
||||
if scope_error is not None:
|
||||
raise _callback_config_error(scope_error)
|
||||
otel_scope_error: Final = conflicting_otel_span_scope_error(
|
||||
scope_error: Final = conflicting_span_scope_error(
|
||||
data.callback_name,
|
||||
data.callback_vars,
|
||||
tuple(
|
||||
entry_vars
|
||||
|
|
@ -365,8 +362,8 @@ async def add_team_callbacks(
|
|||
if entry.get("callback_name") == data.callback_name
|
||||
),
|
||||
)
|
||||
if otel_scope_error is not None:
|
||||
raise _callback_config_error(otel_scope_error)
|
||||
if scope_error is not None:
|
||||
raise _callback_config_error(scope_error)
|
||||
# One entry has to own a credential family end to end. The entries are
|
||||
# flattened into one dict before a request reads them, so an entry
|
||||
# naming only a destination would pair with a key written on another
|
||||
|
|
|
|||
|
|
@ -579,6 +579,23 @@ def test_langfuse_alias_and_otel_scope_disagree_rejected(gateway: Gateway, langf
|
|||
assert "span_scope" in response.text, response.text
|
||||
|
||||
|
||||
def test_split_scope_aliases_on_two_entries_rejected(gateway: Gateway, langfuse_vars: dict[str, JsonValue]) -> None:
|
||||
response: Final = gateway.request(
|
||||
"POST",
|
||||
"/team/new",
|
||||
{
|
||||
"metadata": {
|
||||
"logging": [
|
||||
*_key_logging_entry({**langfuse_vars, "langfuse_span_scope": "full"}),
|
||||
*_key_logging_entry({**langfuse_vars, SPAN_SCOPE_VAR: "llm_only"}),
|
||||
]
|
||||
}
|
||||
},
|
||||
)
|
||||
assert response.status_code == 400, f"expected 400, got {response.status_code}: {response.text}"
|
||||
assert "span_scope" in response.text, response.text
|
||||
|
||||
|
||||
def test_additive_mode_operator_full_tenant_no_internal(gateway: Gateway, audit_sinks: SpanSinks, langfuse_vars: dict[str, JsonValue], otel_audit_config: AuditConfigWriter, tmp_path: Path) -> None:
|
||||
with _candidate(gateway, tmp_path, audit_sinks, otel_audit_config, settings={"otel_tenant_destination_mode": "additive"}) as candidate:
|
||||
with candidate.scenario() as scenario:
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ import pytest
|
|||
|
||||
from litellm.proxy.common_utils.callback_config_validation import (
|
||||
callback_config_error,
|
||||
conflicting_otel_span_scope_error,
|
||||
conflicting_span_scope_error,
|
||||
cross_entry_family_error,
|
||||
logging_metadata_config_error,
|
||||
|
|
@ -60,7 +59,7 @@ def test_a_bad_span_scope_is_reported_even_when_the_environment_is_fine():
|
|||
def test_one_span_scope_per_team(new_vars, stored, rejected):
|
||||
"""The entries flatten last-wins, so a second scope would export whichever entry
|
||||
was stored last. An entry that names no scope leaves the stored one in charge."""
|
||||
error = conflicting_span_scope_error(new_vars, stored)
|
||||
error = conflicting_span_scope_error("langfuse_otel", new_vars, stored)
|
||||
assert (error is not None) is rejected
|
||||
if rejected:
|
||||
assert "langfuse_span_scope" in error and stored[-1]["langfuse_span_scope"] in error
|
||||
|
|
@ -166,7 +165,22 @@ def test_key_logging_entries_of_one_backend_may_not_disagree_on_span_scope():
|
|||
],
|
||||
)
|
||||
def test_one_span_scope_value_per_backend(new_vars, stored, rejected):
|
||||
error = conflicting_otel_span_scope_error(new_vars, stored)
|
||||
error = conflicting_span_scope_error("arize", new_vars, stored)
|
||||
assert (error is not None) is rejected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"callback_name, new_vars, stored, rejected",
|
||||
[
|
||||
("langfuse_otel", {"otel_span_scope": "llm_only"}, [{"langfuse_span_scope": "full"}], True),
|
||||
("newrelic", {"otel_span_scope": "llm_only"}, [{"otel_span_scope": "full"}], True),
|
||||
("langfuse_otel", {"otel_span_scope": "llm_only"}, [{"langfuse_span_scope": "llm_only"}], False),
|
||||
("langfuse_otel", {"otel_span_scope": "llm_only"}, [{"otel_span_scope": "llm_only"}], False),
|
||||
("langfuse_otel", {"langfuse_span_scope": "llm_only"}, [{"otel_span_scope": "llm_only"}], False),
|
||||
],
|
||||
)
|
||||
def test_split_span_scope_aliases_conflict_across_entries(callback_name, new_vars, stored, rejected):
|
||||
error = conflicting_span_scope_error(callback_name, new_vars, stored)
|
||||
assert (error is not None) is rejected
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1730,6 +1730,47 @@ async def test_a_second_entry_may_not_flip_span_scope_but_another_backend_may(pa
|
|||
assert [entry["callback_vars"]["otel_span_scope"] for entry in saved["logging"]] == ["no_internal", "full"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_split_span_scope_aliases_across_entries_are_rejected(patched_prisma):
|
||||
"""langfuse_span_scope and otel_span_scope name the same setting, so a stored
|
||||
full under one name may not be flipped to llm_only under the other."""
|
||||
patched_prisma.get_data = AsyncMock(
|
||||
return_value=_team_row(
|
||||
metadata={
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langfuse_otel",
|
||||
"callback_type": "success",
|
||||
"callback_vars": {
|
||||
"langfuse_public_key": "pk",
|
||||
"langfuse_secret_key": "sk",
|
||||
"langfuse_span_scope": "full",
|
||||
},
|
||||
}
|
||||
]
|
||||
}
|
||||
)
|
||||
)
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await add_team_callbacks(
|
||||
data=AddTeamCallback(
|
||||
callback_name="langfuse_otel",
|
||||
callback_type="failure",
|
||||
callback_vars={
|
||||
"langfuse_public_key": "pk",
|
||||
"langfuse_secret_key": "sk",
|
||||
"otel_span_scope": "llm_only",
|
||||
},
|
||||
),
|
||||
http_request=Mock(spec=Request),
|
||||
team_id="team-victim",
|
||||
user_api_key_dict=_admin_auth(),
|
||||
)
|
||||
assert exc.value.status_code == 400
|
||||
assert "span_scope" in str(exc.value.detail) and "'full'" 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