From 466b4ddae31beb94635f72adc796d731b485930f Mon Sep 17 00:00:00 2001 From: Aneesh-Fiddler Date: Tue, 28 Apr 2026 13:33:33 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20address=20review=20comments=20=E2=80=94?= =?UTF-8?q?=20Pydantic=20compat,=20falsy=20fallthrough,=20per-tool-call=20?= =?UTF-8?q?attrs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- litellm/integrations/opentelemetry.py | 62 ++++++- .../integrations/test_opentelemetry.py | 171 ++++++++++++++++++ 2 files changed, 227 insertions(+), 6 deletions(-) diff --git a/litellm/integrations/opentelemetry.py b/litellm/integrations/opentelemetry.py index 8084a9d0f22..90e647d86bf 100644 --- a/litellm/integrations/opentelemetry.py +++ b/litellm/integrations/opentelemetry.py @@ -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", "") diff --git a/tests/test_litellm/integrations/test_opentelemetry.py b/tests/test_litellm/integrations/test_opentelemetry.py index 5f294005641..56aba4bc5ed 100644 --- a/tests/test_litellm/integrations/test_opentelemetry.py +++ b/tests/test_litellm/integrations/test_opentelemetry.py @@ -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" + )