mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(otel): don't crash set_attributes on non-dict (MCP) response_obj (#30660)
* fix(otel): don't crash set_attributes on non-dict (MCP) response_obj MCP tool calls pass a Pydantic CallToolResult, but set_attributes accesses response_obj via .get() throughout. The AttributeError was caught but skipped writing the span output. Normalize a non-dict response_obj to a dict (model_dump) so the span is fully emitted. Fixes #30651. Co-Authored-By: Chenglun Hu <chenglunhu@gmail.com> * test(otel): cover model_dump-raises and non-serializable fallbacks for #30651 * style: black-format the non-dict response test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
c5b7fc5d22
commit
aad4d335de
2 changed files with 63 additions and 0 deletions
|
|
@ -2117,6 +2117,19 @@ class OpenTelemetry(OTELGenAISemconvMixin, CustomLogger):
|
|||
if standard_logging_payload is None:
|
||||
raise ValueError("standard_logging_object not found in kwargs")
|
||||
|
||||
# MCP tool calls pass a Pydantic response (e.g. mcp.types.CallToolResult)
|
||||
# rather than a dict. set_attributes accesses response_obj via .get()
|
||||
# throughout, so a non-dict payload raises AttributeError and the span
|
||||
# output is silently dropped. Normalize it to a dict here. See #30651.
|
||||
if response_obj is not None and not hasattr(response_obj, "get"):
|
||||
if hasattr(response_obj, "model_dump"):
|
||||
try:
|
||||
response_obj = response_obj.model_dump()
|
||||
except Exception:
|
||||
response_obj = None
|
||||
else:
|
||||
response_obj = None
|
||||
|
||||
# https://github.com/open-telemetry/semantic-conventions/blob/main/model/registry/gen-ai.yaml
|
||||
# Following Conventions here: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/llm-spans.md
|
||||
#############################################
|
||||
|
|
|
|||
|
|
@ -5852,3 +5852,53 @@ class TestOpenTelemetryMetricAttributeFiltering(unittest.TestCase):
|
|||
exporter="console", attributes=attributes
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
class PydanticLikeToolResult:
|
||||
"""Mimics mcp.types.CallToolResult: a Pydantic-style object with model_dump
|
||||
and no dict .get (the shape that crashed set_attributes — see #30651)."""
|
||||
|
||||
def model_dump(self):
|
||||
return {"content": [{"type": "text", "text": "ok"}], "isError": False}
|
||||
|
||||
|
||||
def test_set_attributes_does_not_crash_on_non_dict_response_obj():
|
||||
from litellm.integrations.opentelemetry import OpenTelemetry
|
||||
|
||||
otel = OpenTelemetry()
|
||||
otel.tracer = MagicMock()
|
||||
span = MagicMock()
|
||||
from collections import defaultdict
|
||||
|
||||
# defaultdict(dict) keeps the many standard_logging_payload[...] subscripts in
|
||||
# set_attributes safe so execution reaches the response_obj.get(...) path,
|
||||
# which is where the non-dict crash (#30651) happened.
|
||||
kwargs = {
|
||||
"standard_logging_object": defaultdict(dict, {"id": "log-1"}),
|
||||
"litellm_params": {},
|
||||
"optional_params": {},
|
||||
}
|
||||
|
||||
def _set_attr_error_count(response_obj):
|
||||
with patch("litellm.integrations.opentelemetry.verbose_logger") as vl:
|
||||
otel.set_attributes(span, kwargs, response_obj)
|
||||
return sum(1 for c in vl.exception.call_args_list if "set_attributes" in str(c))
|
||||
|
||||
# A dict response is the known-good baseline; a Pydantic (non-dict) response
|
||||
# used to raise "'CallToolResult' object has no attribute 'get'" on top of it.
|
||||
# After the fix the non-dict path must not add any set_attributes error.
|
||||
baseline = _set_attr_error_count({"id": "resp-1"})
|
||||
pydantic = _set_attr_error_count(PydanticLikeToolResult())
|
||||
assert pydantic <= baseline, (pydantic, baseline)
|
||||
|
||||
# also cover the fallbacks: model_dump raising, and an object with neither
|
||||
# .get nor model_dump — both normalize to None without adding errors.
|
||||
class _RaisingDump:
|
||||
def model_dump(self):
|
||||
raise RuntimeError("boom")
|
||||
|
||||
class _Opaque:
|
||||
pass
|
||||
|
||||
assert _set_attr_error_count(_RaisingDump()) <= baseline
|
||||
assert _set_attr_error_count(_Opaque()) <= baseline
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue