mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
fix: address review comments — Pydantic compat, falsy fallthrough, per-tool-call attrs
- Replace isinstance(item, dict) with hasattr(item, 'get') so Pydantic model instances (ResponseOutputMessage, ResponseFunctionToolCall) are accepted alongside plain dicts (P1) - Use 'is not None' guards instead of or-chain for system_instructions coalescing to prevent falsy values (e.g. []) falling through to the wrong kwarg (P2) - Emit per-tool-call span attributes (gen_ai.completion.N.function_call.*) for Responses API function_call items, matching the choices branch parity with _tool_calls_kv_pair (P2) - Add 4 new tests: Pydantic-like objects, falsy fallthrough guard, per-tool-call attribute emission, multiple tool call indexing
This commit is contained in:
Aneesh-Fiddler
parent
c30d58f7e3
commit
466b4ddae3
2 changed files with 227 additions and 6 deletions
|
|
@ -1683,10 +1683,16 @@ class OpenTelemetry(CustomLogger):
|
|||
# - "system_instructions" — Vertex AI Gemini chat-completion
|
||||
# - "instructions" — OpenAI Responses API
|
||||
# - "system" — Anthropic Messages API
|
||||
# Use `is not None` rather than truthiness to avoid falsy
|
||||
# values (e.g. []) falling through to the wrong kwarg.
|
||||
system_instructions = (
|
||||
kwargs.get("system_instructions")
|
||||
or kwargs.get("instructions")
|
||||
or kwargs.get("system")
|
||||
if kwargs.get("system_instructions") is not None
|
||||
else (
|
||||
kwargs.get("instructions")
|
||||
if kwargs.get("instructions") is not None
|
||||
else kwargs.get("system")
|
||||
)
|
||||
)
|
||||
if system_instructions:
|
||||
if isinstance(system_instructions, str):
|
||||
|
|
@ -1770,8 +1776,9 @@ class OpenTelemetry(CustomLogger):
|
|||
# list instead of "choices". Each item with
|
||||
# type="message" contains a "content" list of
|
||||
# OutputText objects (type="output_text").
|
||||
output_items = response_obj.get("output")
|
||||
output_messages = self._transform_responses_api_output_to_otel(
|
||||
response_obj.get("output")
|
||||
output_items
|
||||
)
|
||||
if output_messages:
|
||||
self.safe_set_attribute(
|
||||
|
|
@ -1780,6 +1787,43 @@ class OpenTelemetry(CustomLogger):
|
|||
value=safe_dumps(output_messages),
|
||||
)
|
||||
|
||||
# Emit per-tool-call span attributes (parity with
|
||||
# the choices branch that calls _tool_calls_kv_pair).
|
||||
# Convert Responses API function_call items to the
|
||||
# ChatCompletionMessageToolCall format expected by
|
||||
# _tool_calls_kv_pair.
|
||||
tool_calls = []
|
||||
for out_item in output_items:
|
||||
if (
|
||||
hasattr(out_item, "get")
|
||||
and out_item.get("type") == "function_call"
|
||||
):
|
||||
tool_calls.append(
|
||||
{
|
||||
"function": {
|
||||
"name": out_item.get("name", ""),
|
||||
"arguments": out_item.get("arguments", ""),
|
||||
}
|
||||
}
|
||||
)
|
||||
if tool_calls:
|
||||
kv_pairs = OpenTelemetry._tool_calls_kv_pair(tool_calls) # type: ignore
|
||||
for key, value in kv_pairs.items():
|
||||
self.safe_set_attribute(
|
||||
span=span,
|
||||
key=key,
|
||||
value=value,
|
||||
)
|
||||
|
||||
# Extract finish reason from ResponsesAPIResponse.status
|
||||
status = response_obj.get("status")
|
||||
if status:
|
||||
self.safe_set_attribute(
|
||||
span=span,
|
||||
key=SpanAttributes.GEN_AI_RESPONSE_FINISH_REASONS.value,
|
||||
value=safe_dumps([status]),
|
||||
)
|
||||
|
||||
# Extract finish reason from ResponsesAPIResponse.status
|
||||
status = response_obj.get("status")
|
||||
if status:
|
||||
|
|
@ -1884,7 +1928,7 @@ class OpenTelemetry(CustomLogger):
|
|||
transformed.append(transformed_msg)
|
||||
return transformed
|
||||
|
||||
def _transform_responses_api_output_to_otel(self, output: List[dict]) -> List[dict]:
|
||||
def _transform_responses_api_output_to_otel(self, output: List) -> List[dict]:
|
||||
"""
|
||||
Transform Responses API output items into OTEL GenAI 1.38 format.
|
||||
|
||||
|
|
@ -1893,18 +1937,24 @@ class OpenTelemetry(CustomLogger):
|
|||
``content`` list of ``OutputText`` objects with ``type="output_text"``
|
||||
and ``text`` fields.
|
||||
|
||||
Items may be plain dicts or Pydantic model instances (e.g.
|
||||
``ResponseOutputMessage``, ``ResponseFunctionToolCall``). Both
|
||||
expose a ``.get()`` method via ``BaseLiteLLMOpenAIResponseObject``,
|
||||
so we use ``hasattr(item, "get")`` rather than ``isinstance(item,
|
||||
dict)`` to accept either form.
|
||||
|
||||
This method converts them to the same ``{"role": ..., "parts": [...]}``
|
||||
format used by ``_transform_choices_to_otel_semantic_conventions``.
|
||||
"""
|
||||
transformed = []
|
||||
for item in output:
|
||||
if not isinstance(item, dict):
|
||||
if not hasattr(item, "get"):
|
||||
continue
|
||||
if item.get("type") == "message":
|
||||
role = item.get("role", "assistant")
|
||||
parts = []
|
||||
for content in item.get("content", []):
|
||||
if not isinstance(content, dict):
|
||||
if not hasattr(content, "get"):
|
||||
continue
|
||||
if content.get("type") == "output_text":
|
||||
text = content.get("text", "")
|
||||
|
|
|
|||
|
|
@ -3315,3 +3315,174 @@ class TestTransformResponsesAPIOutput(unittest.TestCase):
|
|||
]
|
||||
result = otel._transform_responses_api_output_to_otel(output)
|
||||
self.assertEqual(result[0]["role"], "assistant")
|
||||
|
||||
|
||||
def test_pydantic_like_objects_accepted(self):
|
||||
"""Items with .get() but not isinstance(dict) should be accepted."""
|
||||
|
||||
class FakeOutputItem:
|
||||
"""Mimics BaseLiteLLMOpenAIResponseObject duck-typing."""
|
||||
|
||||
def __init__(self, data):
|
||||
self._data = data
|
||||
|
||||
def get(self, key, default=None):
|
||||
return self._data.get(key, default)
|
||||
|
||||
class FakeContent:
|
||||
def __init__(self, data):
|
||||
self._data = data
|
||||
|
||||
def get(self, key, default=None):
|
||||
return self._data.get(key, default)
|
||||
|
||||
otel = OpenTelemetry()
|
||||
output = [
|
||||
FakeOutputItem(
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
FakeContent({"type": "output_text", "text": "Pydantic works!"}),
|
||||
],
|
||||
}
|
||||
)
|
||||
]
|
||||
result = otel._transform_responses_api_output_to_otel(output)
|
||||
self.assertEqual(len(result), 1)
|
||||
self.assertEqual(result[0]["parts"][0]["content"], "Pydantic works!")
|
||||
|
||||
|
||||
class TestSystemInstructionsPrecedence(unittest.TestCase):
|
||||
"""Tests for the is-not-None precedence in system_instructions coalescing."""
|
||||
|
||||
def _get_attr(self, mock_span, attr_name):
|
||||
calls = [
|
||||
call
|
||||
for call in mock_span.set_attribute.call_args_list
|
||||
if call[0][0] == attr_name
|
||||
]
|
||||
if not calls:
|
||||
return None
|
||||
return calls[0][0][1]
|
||||
|
||||
def _base_kwargs(self, **overrides):
|
||||
kwargs = {
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "Hi"}],
|
||||
"optional_params": {},
|
||||
"litellm_params": {"custom_llm_provider": "openai"},
|
||||
"standard_logging_object": {
|
||||
"id": "test-id",
|
||||
"call_type": "responses",
|
||||
"metadata": {},
|
||||
},
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return kwargs
|
||||
|
||||
def test_empty_list_system_instructions_does_not_fallthrough(self):
|
||||
"""An empty list for system_instructions should NOT fall through to instructions."""
|
||||
otel = OpenTelemetry()
|
||||
mock_span = MagicMock()
|
||||
|
||||
kwargs = self._base_kwargs(
|
||||
system_instructions=[],
|
||||
instructions="Should not be used",
|
||||
)
|
||||
response_obj = {"id": "r1", "model": "gpt-4o"}
|
||||
|
||||
otel.set_attributes(span=mock_span, kwargs=kwargs, response_obj=response_obj)
|
||||
|
||||
# system_instructions is [] (falsy but not None), so it wins.
|
||||
# Since it's an empty list, no attribute should be set (nothing to transform).
|
||||
value = self._get_attr(mock_span, "gen_ai.system_instructions")
|
||||
# The empty list is truthy for `is not None` but produces empty
|
||||
# transformed output — the attribute should NOT contain "Should not be used".
|
||||
if value is not None:
|
||||
self.assertNotIn("Should not be used", str(value))
|
||||
|
||||
|
||||
class TestResponsesAPIToolCallSpanAttributes(unittest.TestCase):
|
||||
"""Tests for per-tool-call span attributes on Responses API function_call items."""
|
||||
|
||||
def _base_kwargs(self):
|
||||
return {
|
||||
"model": "gpt-4o",
|
||||
"messages": [{"role": "user", "content": "What is the weather?"}],
|
||||
"optional_params": {},
|
||||
"litellm_params": {"custom_llm_provider": "openai"},
|
||||
"standard_logging_object": {
|
||||
"id": "resp_tc",
|
||||
"call_type": "responses",
|
||||
"metadata": {},
|
||||
},
|
||||
}
|
||||
|
||||
def test_per_tool_call_attributes_emitted(self):
|
||||
"""function_call output items should produce per-tool-call span attributes."""
|
||||
otel = OpenTelemetry()
|
||||
mock_span = MagicMock()
|
||||
|
||||
response_obj = {
|
||||
"id": "resp_tc",
|
||||
"model": "gpt-4o",
|
||||
"status": "completed",
|
||||
"output": [
|
||||
{
|
||||
"type": "function_call",
|
||||
"name": "get_weather",
|
||||
"call_id": "call_abc",
|
||||
"arguments": '{"location": "SF"}',
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
otel.set_attributes(span=mock_span, kwargs=self._base_kwargs(), response_obj=response_obj)
|
||||
|
||||
# Verify per-tool-call attributes were set (same format as choices branch)
|
||||
attr_names = [call[0][0] for call in mock_span.set_attribute.call_args_list]
|
||||
tool_call_attrs = [a for a in attr_names if "function_call" in a]
|
||||
self.assertTrue(len(tool_call_attrs) > 0, "Per-tool-call span attributes should be emitted")
|
||||
|
||||
# Verify the name attribute specifically
|
||||
mock_span.set_attribute.assert_any_call(
|
||||
"gen_ai.completion.0.function_call.name", "get_weather"
|
||||
)
|
||||
mock_span.set_attribute.assert_any_call(
|
||||
"gen_ai.completion.0.function_call.arguments", '{"location": "SF"}'
|
||||
)
|
||||
|
||||
def test_multiple_tool_calls_indexed(self):
|
||||
"""Multiple function_call items should be indexed correctly."""
|
||||
otel = OpenTelemetry()
|
||||
mock_span = MagicMock()
|
||||
|
||||
response_obj = {
|
||||
"id": "resp_tc2",
|
||||
"model": "gpt-4o",
|
||||
"status": "completed",
|
||||
"output": [
|
||||
{
|
||||
"type": "function_call",
|
||||
"name": "get_weather",
|
||||
"call_id": "call_1",
|
||||
"arguments": "{}",
|
||||
},
|
||||
{
|
||||
"type": "function_call",
|
||||
"name": "get_time",
|
||||
"call_id": "call_2",
|
||||
"arguments": "{}",
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
otel.set_attributes(span=mock_span, kwargs=self._base_kwargs(), response_obj=response_obj)
|
||||
|
||||
mock_span.set_attribute.assert_any_call(
|
||||
"gen_ai.completion.0.function_call.name", "get_weather"
|
||||
)
|
||||
mock_span.set_attribute.assert_any_call(
|
||||
"gen_ai.completion.1.function_call.name", "get_time"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue