diff --git a/instrumentation-genai/opentelemetry-instrumentation-openai-v2/CHANGELOG.md b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/CHANGELOG.md index c75cdf2f27..76d2deabc4 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-openai-v2/CHANGELOG.md +++ b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Fix `ChoiceBuffer` crash on streaming tool-call deltas with `arguments=None` + ([#4350](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4350)) - Fix `StreamWrapper` missing `.headers` and other attributes when using `with_raw_response` streaming ([#4113](https://github.com/open-telemetry/opentelemetry-python-contrib/issues/4113)) - Add opt-in support for latest experimental semantic conventions (v1.37.0). Set diff --git a/instrumentation-genai/opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py index 3bc42f103c..2c8c1b4c71 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py @@ -582,7 +582,8 @@ def __init__(self, index, tool_call_id, function_name): self.arguments = [] def append_arguments(self, arguments): - self.arguments.append(arguments) + if arguments is not None: + self.arguments.append(arguments) class ChoiceBuffer: @@ -601,13 +602,16 @@ def append_tool_call(self, tool_call): for _ in range(len(self.tool_calls_buffers), idx + 1): self.tool_calls_buffers.append(None) + function = tool_call.function if not self.tool_calls_buffers[idx]: self.tool_calls_buffers[idx] = ToolCallBuffer( - idx, tool_call.id, tool_call.function.name + idx, + tool_call.id, + function.name if function else None, ) - self.tool_calls_buffers[idx].append_arguments( - tool_call.function.arguments - ) + + if function: + self.tool_calls_buffers[idx].append_arguments(function.arguments) class BaseStreamWrapper: diff --git a/instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/test_choice_buffer.py b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/test_choice_buffer.py new file mode 100644 index 0000000000..7717ff73b2 --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/test_choice_buffer.py @@ -0,0 +1,182 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit tests for ChoiceBuffer and ToolCallBuffer classes.""" + +from openai.types.chat.chat_completion_chunk import ( + ChoiceDeltaToolCall, + ChoiceDeltaToolCallFunction, +) + +from opentelemetry.instrumentation.openai_v2.patch import ( + ChoiceBuffer, + ToolCallBuffer, +) + + +def test_toolcallbuffer_append_arguments_with_string(): + buf = ToolCallBuffer(0, "call_1", "get_weather") + buf.append_arguments('{"city":') + buf.append_arguments(' "NYC"}') + assert "".join(buf.arguments) == '{"city": "NYC"}' + + +def test_toolcallbuffer_append_arguments_with_none_is_skipped(): + """Regression test for issue #4344. + + Some OpenAI-compatible providers (vLLM, TGI, etc.) send + arguments=None on tool-call delta chunks instead of arguments="". + This must not crash when joining the arguments list. + """ + buf = ToolCallBuffer(0, "call_1", "get_weather") + buf.append_arguments(None) + buf.append_arguments('{"city": "NYC"}') + buf.append_arguments(None) + assert "".join(buf.arguments) == '{"city": "NYC"}' + + +def test_toolcallbuffer_append_arguments_all_none(): + buf = ToolCallBuffer(0, "call_1", "get_weather") + buf.append_arguments(None) + buf.append_arguments(None) + assert "".join(buf.arguments) == "" + + +def test_toolcallbuffer_append_arguments_empty_string(): + buf = ToolCallBuffer(0, "call_1", "get_weather") + buf.append_arguments("") + buf.append_arguments('{"city": "NYC"}') + assert "".join(buf.arguments) == '{"city": "NYC"}' + + +def test_choicebuffer_append_tool_call_with_none_arguments(): + """End-to-end regression test for issue #4344. + + Simulates the exact scenario from the bug report where a provider + sends arguments=None on the first tool-call delta chunk. + """ + buf = ChoiceBuffer(0) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + id="call_1", + type="function", + function=ChoiceDeltaToolCallFunction( + name="get_weather", arguments=None + ), + ) + ) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + function=ChoiceDeltaToolCallFunction(arguments='{"city": "NYC"}'), + ) + ) + + # This must not raise TypeError + result = "".join(buf.tool_calls_buffers[0].arguments) + assert result == '{"city": "NYC"}' + + +def test_choicebuffer_append_tool_call_normal_flow(): + """Standard OpenAI flow where arguments="" on first delta.""" + buf = ChoiceBuffer(0) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + id="call_1", + type="function", + function=ChoiceDeltaToolCallFunction( + name="get_weather", arguments="" + ), + ) + ) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + function=ChoiceDeltaToolCallFunction(arguments='{"city": "NYC"}'), + ) + ) + + result = "".join(buf.tool_calls_buffers[0].arguments) + assert result == '{"city": "NYC"}' + + +def test_choicebuffer_append_multiple_tool_calls_with_none_arguments(): + """Multiple tool calls where some have arguments=None.""" + buf = ChoiceBuffer(0) + + # First tool call + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + id="call_1", + type="function", + function=ChoiceDeltaToolCallFunction( + name="get_weather", arguments=None + ), + ) + ) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + function=ChoiceDeltaToolCallFunction(arguments='{"city": "NYC"}'), + ) + ) + + # Second tool call + buf.append_tool_call( + ChoiceDeltaToolCall( + index=1, + id="call_2", + type="function", + function=ChoiceDeltaToolCallFunction( + name="get_time", arguments=None + ), + ) + ) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=1, + function=ChoiceDeltaToolCallFunction(arguments='{"tz": "EST"}'), + ) + ) + + assert "".join(buf.tool_calls_buffers[0].arguments) == '{"city": "NYC"}' + assert "".join(buf.tool_calls_buffers[1].arguments) == '{"tz": "EST"}' + + +def test_choicebuffer_append_tool_call_with_none_function(): + """Handle delta chunks where function is None.""" + buf = ChoiceBuffer(0) + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + id="call_1", + type="function", + function=ChoiceDeltaToolCallFunction( + name="get_weather", arguments='{"city": "NYC"}' + ), + ) + ) + # Subsequent delta with function=None should not crash + buf.append_tool_call( + ChoiceDeltaToolCall( + index=0, + function=None, + ) + ) + + result = "".join(buf.tool_calls_buffers[0].arguments) + assert result == '{"city": "NYC"}'