Fix unhashable attributes crash in OpenAI streaming instrumentation
Fixes #3428
[!IMPORTANT] Fixes unhashable attributes crash in OpenAI streaming by sanitizing attributes for metric recording.
- Behavior:
- Adds
_sanitize_attributes_for_metrics()inchat_wrappers.pyto convert unhashable attributes to hashable types for metric recording.- Applies sanitization in
_set_chat_metrics(),_shared_attributes(),_build_from_streaming_response(), and_abuild_from_streaming_response().- Tests:
- New test file
test_unhashable_attributes_fix.pyadded to verify the fix.- Tests include handling of lists, dicts,
Nonevalues, and preserving hashable values.- Simulates original issue conditions to ensure the fix prevents crashes.
This description was created by
for a1c608d3ae122df955fa9a4c47254ca01e24c3c2. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
-
Bug Fixes
- Fixed an issue where OpenTelemetry metrics collection could fail when encountering certain attribute value types in OpenAI instrumentation.
-
Tests
- Added comprehensive test coverage for metrics attribute handling with various data types and edge cases.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
Walkthrough
This PR introduces a _sanitize_attributes_for_metrics() helper function to convert non-hashable metric attribute values (lists, dicts) to JSON strings or string representations, ensuring OpenTelemetry metric compatibility. The function is integrated into five metric recording locations across the ChatStream class and related functions.
Changes
| Cohort / File(s) | Summary |
|---|---|
Metric attribute sanitization packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py |
Added _sanitize_attributes_for_metrics() helper to convert non-hashable types (lists, dicts) to JSON strings. Applied sanitization in _set_chat_metrics(), ChatStream._shared_attributes(), ChatStream._process_complete_response(), and streaming response builders before metric recording. |
Test suite for sanitization fix packages/opentelemetry-instrumentation-openai/tests/test_unhashable_attributes_fix.py |
New test module validating sanitization behavior across edge cases: lists, dicts, None/empty values, and pre-existing hashable values. Includes reproduction of original TypeError crash and verification that sanitized attributes remain hashable. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Sanitization logic correctness: Verify JSON serialization handles edge cases and fallback string conversion is appropriate
-
Integration points: Confirm all five metric recording locations are covered and sanitization is applied before OpenTelemetry
record()calls -
Test coverage: Validate test scenarios comprehensively cover unhashable types (lists, dicts) and ensure mocking of
Config.get_common_metrics_attributes()accurately reproduces the production crash -
Streaming path: Pay special attention to
_process_item()and streaming builders where the crash originally occurred
Possibly related PRs
-
PR
#3155: Modifieschat_wrappers.pyand ChatStream methods including_process_complete_response()and streaming metric emission—direct overlap in implementation areas.
Poem
🐰 Lists and dicts caused quite a fright,
Making metrics crash in streaming's night,
But JSON strings save the day,
Now hashable hops keep crashes at bay! ✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'Fix unhashable attributes crash in OpenAI streaming instrumentation' directly and accurately summarizes the main change: fixing a critical bug where unhashable attribute values (lists/dicts) crash metric recording in OpenAI streaming instrumentation. |
| Linked Issues check | ✅ Passed | The PR successfully addresses all key coding requirements from issue #3428: adds _sanitize_attributes_for_metrics helper to convert unhashable values to hashable forms, integrates sanitization into metric recording paths (_set_chat_metrics, ChatStream._shared_attributes, _process_complete_response, streaming builders), and includes comprehensive tests validating the fix with various attribute types. |
| Out of Scope Changes check | ✅ Passed | All changes are scoped to fixing the unhashable attributes issue: the new sanitization function, its integration points for metric recording, and the corresponding test module. No unrelated modifications to other functionality or files are present. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.