fix: missing mcp otel attributes (#29554)
This commit is contained in:
parent
0a767ed14f
commit
8fbdfc7f0d
@ -340,7 +340,7 @@ class MCPToolCallSpanData:
|
||||
def from_standard_logging_payload(
|
||||
cls, payload: "StandardLoggingPayload", capture_content: bool = False
|
||||
) -> "MCPToolCallSpanData":
|
||||
meta = cast(Mapping[str, object], payload.get("mcp_tool_call_metadata") or {})
|
||||
meta = _mcp_tool_call_metadata(cast(Mapping[str, object], payload))
|
||||
return cls(
|
||||
operation=resolve_operation(as_str(payload.get("call_type"))),
|
||||
method=MCPMethod.TOOLS_CALL.value,
|
||||
@ -363,11 +363,22 @@ class MCPToolCallSpanData:
|
||||
)
|
||||
|
||||
|
||||
def _mcp_tool_call_metadata(payload: Mapping[str, object]) -> Mapping[str, object]:
|
||||
"""The MCP gateway's tool-call metadata, which lives under
|
||||
``StandardLoggingPayload.metadata`` (a ``StandardLoggingMetadata`` key), not
|
||||
at the payload's top level."""
|
||||
metadata = payload.get("metadata")
|
||||
if not isinstance(metadata, Mapping):
|
||||
return {}
|
||||
meta = metadata.get("mcp_tool_call_metadata")
|
||||
return meta if isinstance(meta, Mapping) else {}
|
||||
|
||||
|
||||
def is_mcp_tool_call(payload: Mapping[str, object]) -> bool:
|
||||
"""Whether a closed request's payload is an MCP tool call rather than an LLM
|
||||
call — true when the MCP gateway stamped its tool-call metadata, or the call
|
||||
type says so on a path that hasn't populated the metadata yet."""
|
||||
return bool(payload.get("mcp_tool_call_metadata")) or (
|
||||
return bool(_mcp_tool_call_metadata(payload)) or (
|
||||
payload.get("call_type") == "call_mcp_tool"
|
||||
)
|
||||
|
||||
|
||||
@ -249,15 +249,17 @@ def _mcp_payload(**overrides):
|
||||
"status": "success",
|
||||
"litellm_call_id": "mcp_1",
|
||||
"response_cost": 0.01,
|
||||
"metadata": {"user_api_key_team_id": "t1"},
|
||||
"hidden_params": {},
|
||||
"mcp_tool_call_metadata": {
|
||||
"name": "get_weather",
|
||||
"arguments": {"city": "Paris"},
|
||||
"result": {"temp_c": 21},
|
||||
"mcp_server_name": "weather-mcp",
|
||||
"mcp_session_id": "sess-abc123",
|
||||
"metadata": {
|
||||
"user_api_key_team_id": "t1",
|
||||
"mcp_tool_call_metadata": {
|
||||
"name": "get_weather",
|
||||
"arguments": {"city": "Paris"},
|
||||
"result": {"temp_c": 21},
|
||||
"mcp_server_name": "weather-mcp",
|
||||
"mcp_session_id": "sess-abc123",
|
||||
},
|
||||
},
|
||||
"hidden_params": {},
|
||||
}
|
||||
payload.update(overrides)
|
||||
return payload
|
||||
@ -302,7 +304,7 @@ def test_mcp_tool_call_stateless_omits_session_id():
|
||||
``mcp.session.id`` rather than stamping an empty or ``None`` value."""
|
||||
logger, exporter = _logger()
|
||||
payload = _mcp_payload()
|
||||
del payload["mcp_tool_call_metadata"]["mcp_session_id"]
|
||||
del payload["metadata"]["mcp_tool_call_metadata"]["mcp_session_id"]
|
||||
asyncio.run(
|
||||
logger.async_log_success_event(
|
||||
{"standard_logging_object": payload}, None, None, None
|
||||
@ -360,6 +362,31 @@ def test_mcp_tool_call_deduped_on_repeat():
|
||||
assert len(exporter.get_finished_spans()) == 1
|
||||
|
||||
|
||||
def test_mcp_tool_call_metadata_read_from_nested_metadata_not_top_level():
|
||||
"""``mcp_tool_call_metadata`` lives under ``StandardLoggingPayload.metadata``;
|
||||
a top-level copy (the pre-fix shape the reader used to look at) must be ignored
|
||||
so the reader can't silently regress to producing an empty ``tools/call`` span
|
||||
with no session id, tool name, or server name."""
|
||||
logger, exporter = _logger()
|
||||
payload = _mcp_payload()
|
||||
# Move the real metadata to the top level only, mirroring the old buggy read
|
||||
# location. ``call_type`` still classifies this as an MCP call, so the span is
|
||||
# emitted, but none of its fields are reachable from the wrong nesting level.
|
||||
payload["mcp_tool_call_metadata"] = payload["metadata"].pop(
|
||||
"mcp_tool_call_metadata"
|
||||
)
|
||||
asyncio.run(
|
||||
logger.async_log_success_event(
|
||||
{"standard_logging_object": payload}, None, None, None
|
||||
)
|
||||
)
|
||||
(span,) = exporter.get_finished_spans()
|
||||
assert span.name == "tools/call"
|
||||
assert "mcp.session.id" not in span.attributes
|
||||
assert "gen_ai.tool.name" not in span.attributes
|
||||
assert LiteLLM.MCP_SERVER_NAME not in span.attributes
|
||||
|
||||
|
||||
def test_pre_call_idempotent_keeps_first_span():
|
||||
"""A retried call may re-enter ``pre_call`` with the same call id; the first
|
||||
span (with the true start time) is kept, not replaced."""
|
||||
|
||||
@ -195,15 +195,17 @@ def _mcp_payload(capture=False, **overrides):
|
||||
"status": "success",
|
||||
"litellm_call_id": "mcp_call_1",
|
||||
"response_cost": 0.01,
|
||||
"metadata": {"user_api_key_team_id": "t1"},
|
||||
"hidden_params": {},
|
||||
"mcp_tool_call_metadata": {
|
||||
"name": "get_weather",
|
||||
"arguments": {"city": "Paris"},
|
||||
"result": {"temp_c": 21},
|
||||
"mcp_server_name": "weather-mcp",
|
||||
"mcp_session_id": "sess-abc123",
|
||||
"metadata": {
|
||||
"user_api_key_team_id": "t1",
|
||||
"mcp_tool_call_metadata": {
|
||||
"name": "get_weather",
|
||||
"arguments": {"city": "Paris"},
|
||||
"result": {"temp_c": 21},
|
||||
"mcp_server_name": "weather-mcp",
|
||||
"mcp_session_id": "sess-abc123",
|
||||
},
|
||||
},
|
||||
"hidden_params": {},
|
||||
}
|
||||
payload.update(overrides)
|
||||
return payload
|
||||
|
||||
Loading…
Reference in New Issue
Block a user