From a1c608d3ae122df955fa9a4c47254ca01e24c3c2 Mon Sep 17 00:00:00 2001 From: Vira Shreder Date: Mon, 3 Nov 2025 11:32:09 +0100 Subject: [PATCH] Fix unhashable attributes crash in OpenAI streaming instrumentation (#3428) This commit resolves a critical bug where OpenAI streaming instrumentation crashes with "TypeError: unhashable type: 'list'" when tool definitions contain lists or other unhashable types. **Problem:** - OpenAI tool definitions contain lists (e.g., "required": ["param1", "param2"]) - These unhashable values propagate to metric attributes via Config.get_common_metrics_attributes() - OpenTelemetry metrics require hashable attributes for aggregation keys - When creating frozenset(attributes.items()), TypeError occurs and crashes user code **Solution:** - Added _sanitize_attributes_for_metrics() function to convert unhashable values to JSON strings - Applied sanitization in all metric recording paths: - ChatStream._shared_attributes() (main streaming path) - _set_chat_metrics() (non-streaming path) - _build_from_streaming_response() (legacy OpenAI v0 compatibility) - _abuild_from_streaming_response() (async legacy compatibility) **Testing:** - Added comprehensive test suite covering edge cases - Verified fix preserves critical attributes while converting problematic values - All hashable values remain unchanged, unhashable values become JSON strings Fixes #3428 --- .../openai/shared/chat_wrappers.py | 41 +++- .../tests/test_unhashable_attributes_fix.py | 177 ++++++++++++++++++ 2 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 packages/opentelemetry-instrumentation-openai/tests/test_unhashable_attributes_fix.py diff --git a/packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py b/packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py index 515723044c..6e885c272c 100644 --- a/packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py +++ b/packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py @@ -64,6 +64,37 @@ logger = logging.getLogger(__name__) +def _sanitize_attributes_for_metrics(attributes): + """ + Sanitize attributes to ensure they are hashable for metric recording. + OpenTelemetry metrics require hashable attributes for aggregation keys. + + Args: + attributes (dict): Dictionary of attributes to sanitize + + Returns: + dict: Dictionary with all values converted to hashable types + """ + if not isinstance(attributes, dict): + return attributes + + sanitized = {} + for key, value in attributes.items(): + try: + # Test if the value is hashable by creating a frozenset + frozenset({key: value}.items()) + sanitized[key] = value + except TypeError: + # Value is not hashable, convert to JSON string + try: + sanitized[key] = json.dumps(value, sort_keys=True) + except (TypeError, ValueError): + # If JSON serialization fails, convert to string + sanitized[key] = str(value) + + return sanitized + + @_with_chat_telemetry_wrapper def chat_wrapper( tracer: Tracer, @@ -375,6 +406,8 @@ def _set_chat_metrics( server_address=_get_openai_base_url(instance), is_streaming=is_streaming, ) + # Sanitize attributes to ensure they are hashable for metric recording + shared_attributes = _sanitize_attributes_for_metrics(shared_attributes) # token metrics usage = response_dict.get("usage") # type: dict @@ -727,7 +760,7 @@ def _process_item(self, item): _accumulate_stream_items(item, self._complete_response) def _shared_attributes(self): - return metric_shared_attributes( + attributes = metric_shared_attributes( response_model=self._complete_response.get("model") or self._request_kwargs.get("model") or None, @@ -735,6 +768,8 @@ def _shared_attributes(self): server_address=_get_openai_base_url(self._instance), is_streaming=True, ) + # Sanitize attributes to ensure they are hashable for metric recording + return _sanitize_attributes_for_metrics(attributes) @dont_throw def _process_complete_response(self): @@ -894,6 +929,8 @@ def _build_from_streaming_response( "server.address": _get_openai_base_url(instance), "stream": True, } + # Sanitize attributes to ensure they are hashable for metric recording + shared_attributes = _sanitize_attributes_for_metrics(shared_attributes) _set_streaming_token_metrics( request_kwargs, complete_response, span, token_counter, shared_attributes @@ -965,6 +1002,8 @@ async def _abuild_from_streaming_response( "server.address": _get_openai_base_url(instance), "stream": True, } + # Sanitize attributes to ensure they are hashable for metric recording + shared_attributes = _sanitize_attributes_for_metrics(shared_attributes) _set_streaming_token_metrics( request_kwargs, complete_response, span, token_counter, shared_attributes diff --git a/packages/opentelemetry-instrumentation-openai/tests/test_unhashable_attributes_fix.py b/packages/opentelemetry-instrumentation-openai/tests/test_unhashable_attributes_fix.py new file mode 100644 index 0000000000..5e80c8a82b --- /dev/null +++ b/packages/opentelemetry-instrumentation-openai/tests/test_unhashable_attributes_fix.py @@ -0,0 +1,177 @@ +""" +Test to reproduce and verify the fix for the unhashable attributes issue #3428. + +This test simulates the scenario where OpenAI tool definitions with lists +cause TypeError when streaming chat completions are used. +""" +import pytest +from unittest.mock import MagicMock, patch +from opentelemetry.instrumentation.openai.shared.chat_wrappers import ( + _sanitize_attributes_for_metrics, + ChatStream, +) +from opentelemetry.instrumentation.openai.shared.config import Config + + +class TestUnhashableAttributesFix: + def test_sanitize_attributes_with_lists(self): + """Test that _sanitize_attributes_for_metrics handles unhashable values""" + # Test attributes with unhashable types (lists, dicts) + attributes_with_lists = { + "model": "gpt-4", + "tools_required": ["param1", "param2"], # This list causes the issue + "tool_params": {"type": "object", "properties": {"location": {"type": "string"}}}, # Dict + "simple_string": "test", + "simple_number": 42, + } + + # This should not raise an exception + sanitized = _sanitize_attributes_for_metrics(attributes_with_lists) + + # Verify all values are now hashable + for key, value in sanitized.items(): + try: + # Test if the value is hashable by creating a frozenset + frozenset({key: value}.items()) + except TypeError: + pytest.fail(f"Value for key '{key}' is still not hashable: {value}") + + # Verify the content is preserved (as JSON strings for complex types) + assert sanitized["model"] == "gpt-4" + assert sanitized["simple_string"] == "test" + assert sanitized["simple_number"] == 42 + assert '"param1"' in sanitized["tools_required"] # Should be JSON string + assert '"location"' in sanitized["tool_params"] # Should be JSON string + + def test_sanitize_attributes_with_none_values(self): + """Test that None values and empty values are handled correctly""" + attributes = { + "none_value": None, + "empty_list": [], + "empty_dict": {}, + "normal_value": "test" + } + + sanitized = _sanitize_attributes_for_metrics(attributes) + + # Verify all are hashable + for key, value in sanitized.items(): + try: + frozenset({key: value}.items()) + except TypeError: + pytest.fail(f"Value for key '{key}' is still not hashable: {value}") + + def test_sanitize_attributes_preserves_hashable_values(self): + """Test that already hashable values are preserved as-is""" + hashable_attributes = { + "string": "test", + "int": 42, + "float": 3.14, + "bool": True, + "tuple": (1, 2, 3), # Tuples are hashable + } + + sanitized = _sanitize_attributes_for_metrics(hashable_attributes) + + # All values should remain the same + assert sanitized == hashable_attributes + + def test_config_get_common_metrics_attributes_with_unhashable_values(self): + """Test that the fix works when Config.get_common_metrics_attributes returns unhashable values""" + # Mock Config.get_common_metrics_attributes to return unhashable data + original_fn = Config.get_common_metrics_attributes + + def mock_get_common_attributes(): + return { + "tool_definitions": [ + { + "type": "function", + "function": { + "name": "get_weather", + "parameters": { + "type": "object", + "properties": {"location": {"type": "string"}}, + "required": ["location"] # This list causes the crash + } + } + } + ], + "other_config": {"nested": {"data": "value"}} + } + + try: + Config.get_common_metrics_attributes = mock_get_common_attributes + + # Create a mock ChatStream instance + mock_span = MagicMock() + mock_response = MagicMock() + mock_instance = MagicMock() + + # Create ChatStream with mocked objects + chat_stream = ChatStream( + span=mock_span, + response=mock_response, + instance=mock_instance, + start_time=1234567890.0, + request_kwargs={"model": "gpt-4"} + ) + + # This should not raise TypeError: unhashable type: 'list' + attributes = chat_stream._shared_attributes() + + # Verify all attributes are hashable + for key, value in attributes.items(): + try: + frozenset({key: value}.items()) + except TypeError: + pytest.fail(f"Attribute '{key}' with value '{value}' is not hashable") + + finally: + # Restore original function + Config.get_common_metrics_attributes = original_fn + + def test_original_issue_reproduction_simulation(self): + """ + Simulate the original issue scenario from the bug report. + This test reproduces the conditions that led to the TypeError. + """ + # Simulate the problematic attributes that would come from OpenAI tool definitions + problematic_attributes = { + "gen_ai.system": "openai", + "gen_ai.response.model": "gpt-4", + "gen_ai.operation.name": "chat", + "server.address": "https://api.openai.com/v1", + "stream": True, + # This simulates what happens when tool definitions leak into attributes + "tool_required_params": ["location", "unit"], # This causes the crash + "tool_schema": { + "type": "object", + "properties": { + "location": {"type": "string"}, + "unit": {"type": "string", "enum": ["celsius", "fahrenheit"]} + }, + "required": ["location"] # Another list that causes issues + } + } + + # Before the fix, this would fail when trying to create a frozenset + try: + frozenset(problematic_attributes.items()) + pytest.fail("Expected TypeError was not raised - test setup may be incorrect") + except TypeError: + # This is expected - the original issue + pass + + # After applying our sanitization, it should work + sanitized = _sanitize_attributes_for_metrics(problematic_attributes) + + # This should not raise an exception + try: + frozenset(sanitized.items()) + except TypeError as e: + pytest.fail(f"Sanitization failed to fix the issue: {e}") + + # Verify important attributes are preserved + assert sanitized["gen_ai.system"] == "openai" + assert sanitized["gen_ai.response.model"] == "gpt-4" + assert sanitized["stream"] is True \ No newline at end of file