fix(anthropic): various fixes around tools parsing #3204
Conversation
WalkthroughThe changes introduce enhanced support for Anthropic tool usage events within OpenTelemetry instrumentation. Internal logic is updated to distinguish and process tool use blocks and streaming tool calls, with structured attributes and JSON parsing. The test suite is expanded with new scenarios and shared tool definitions, and multiple test cassettes are added for comprehensive coverage. Changes
Sequence Diagram(s)sequenceDiagram participant User participant Instrumentation participant AnthropicAPI User->>Instrumentation: Send message with tool usage Instrumentation->>AnthropicAPI: Forward message (with tools) AnthropicAPI-->>Instrumentation: Streaming response events loop For each event Instrumentation->>Instrumentation: Parse event alt event.type == "tool_use" Instrumentation->>Instrumentation: Parse JSON input, create ToolCall Instrumentation->>Instrumentation: Emit ChoiceEvent with tool_calls else event.type != "tool_use" Instrumentation->>Instrumentation: Emit ChoiceEvent with message content end end Instrumentation-->>User: Emit structured events and span attributes Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 81dcefa in 2 minutes and 21 seconds. Click for details.
- Reviewed
2471lines of code in10files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:2454
- Draft comment:
Consider using a partial or subset dictionary comparison in assert_message_in_logs rather than full equality. This would reduce brittleness if the log body includes extra keys in the future. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:115
- Draft comment:
Multiple tests assert hardcoded response IDs (e.g. 'msg_01TPXhkPo8jy6yQMrMhjpiAE'). Consider documenting or dynamically verifying these values to reduce fragility if cassette responses change. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:85
- Draft comment:
Instead of catching a generic Exception when calling messages.create() with an 'unknown_parameter', consider using pytest.raises with a specific exception type. This makes the intended failure mode explicit. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:1890
- Draft comment:
The expected user message log is asserted as an empty dictionary ({}). It would help to document why an empty body is expected in this case to clarify the behavior when there is no content. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to document why an empty dictionary is expected, which falls under asking for clarification or confirmation of intention. This violates the rule against asking the author to explain or confirm their intention.
5. packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/span_utils.py:186
- Draft comment:
Typo: 'Antrhopic' should be spelled as 'Anthropic'. - Reason this comment was not posted:
Comment was on unchanged code.
6. packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_with_events_with_no_content.yaml:5
- Draft comment:
Typographical note: The string "I''ll help you get the weather and current time in San Francisco." contains a double apostrophe (I''ll). Please confirm if this is intentional or if it should be a single apostrophe (I'll). - Reason this comment was not posted:
Comment looked like it was already resolved.
7. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:1456
- Draft comment:
There's a typographical error in the comment on this line. It currently reads "Validate the tool messages input vent" — likely it was meant to be "Validate the tool messages input event". - Reason this comment was not posted:
Comment was on unchanged code.
8. packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py:2266
- Draft comment:
Typo: In the comment "Validate the tool messages input vent", "vent" should be corrected to "event". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment correctly identifies a typo, it's a very minor issue in a test file comment. Test file comments are internal documentation and don't affect functionality. The rules state to only keep comments that require clear code changes, and to not make comments that are obvious or unimportant. A typo in a test comment is not a significant issue that requires immediate attention. The typo could theoretically cause confusion for future developers reading the test. Documentation clarity has some value. While documentation clarity has value, this is an extremely minor typo that doesn't meaningfully impact understanding - "vent" vs "event" in this context is obvious from context. The cost of the PR review comment outweighs the minor benefit. Delete the comment. The typo is too minor of an issue to warrant a PR comment, especially in test file documentation.
Workflow ID: wflow_aF7XfQnljGTgD9sk
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (1)
131-164: Add error handling for JSON parsing of tool arguments.The tool call parsing logic is well-structured, but the
json.loads()call on Line 138 could raise aJSONDecodeErrorif the input contains malformed JSON.Apply this diff to add proper error handling:
- "arguments": json.loads(message.get("input", '{}')), + "arguments": self._safe_parse_json(message.get("input", '{}')),And add this helper method:
def _safe_parse_json(self, json_str): try: return json.loads(json_str) except (json.JSONDecodeError, TypeError): logger.warning("Failed to parse tool call arguments as JSON: %s", json_str) return {}
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py (1)
1646-1651: Consider using contextlib.suppress for cleaner exception handling.The static analysis tool correctly identifies that
contextlib.suppress(Exception)would be cleaner than try-except-pass blocks.+from contextlib import suppress + # Then replace patterns like: -try: - anthropic_client.messages.create( - unknown_parameter="unknown", - ) -except Exception: - pass +with suppress(Exception): + anthropic_client.messages.create( + unknown_parameter="unknown", + )Also applies to: 1806-1811, 1909-1914, 1988-1993, 2120-2125, 2233-2238
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py(3 hunks)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/span_utils.py(3 hunks)packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/streaming.py(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_legacy.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_with_events_with_content.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_with_events_with_no_content.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_legacy.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_with_events_with_content.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_with_events_with_no_content.yaml(1 hunks)packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py
1646-1651: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
1806-1811: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
1909-1914: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
1988-1993: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
2120-2125: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
2233-2238: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
🔇 Additional comments (15)
packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_with_events_with_no_content.yaml (1)
1-121: LGTM! Test cassette properly captures tool interaction scenario.This test cassette accurately records the HTTP interaction for testing tool usage with no initial content, providing good test coverage for the instrumentation changes.
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/streaming.py (2)
43-46: LGTM! Proper initialization of tool use events.The code correctly initializes tool use content blocks with the necessary fields (
id,name, and emptyinputstring) to support incremental tool input construction during streaming.
53-54: LGTM! Correct handling of incremental JSON input.The addition of
input_json_deltahandling properly accumulates partial JSON data for tool inputs during streaming, which is essential for the tool parsing fixes mentioned in the PR objectives.packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_with_events_with_content.yaml (1)
1-121: LGTM! Test cassette provides comprehensive tool interaction coverage.This cassette complements the test suite by covering tool usage scenarios with content, ensuring thorough testing of the enhanced tool parsing logic.
packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_history_legacy.yaml (1)
1-116: LGTM! Legacy compatibility test coverage is valuable.This cassette ensures that the tool parsing enhancements maintain backward compatibility with legacy tool usage formats, which is crucial for the robustness of the changes.
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/event_emitter.py (1)
3-3: LGTM! JSON import needed for tool argument parsing.The addition of the
jsonimport is necessary for parsing tool call arguments in the streaming response events.packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_with_events_with_content.yaml (1)
1-304: LGTM! Test cassette file looks complete.This test cassette properly captures the streaming interaction with Anthropic's API including tool usage for weather and time queries. The recorded response includes all necessary streaming events and metadata.
packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_with_events_with_no_content.yaml (1)
1-335: LGTM! Test cassette properly captures the "no content" streaming scenario.This cassette complements the previous one by testing the edge case where tool inputs initially contain no content and are incrementally built through streaming deltas.
packages/opentelemetry-instrumentation-anthropic/tests/cassettes/test_messages/test_anthropic_tools_streaming_legacy.yaml (1)
1-300: LGTM! Legacy streaming cassette ensures backward compatibility.This cassette properly captures the legacy streaming format interactions, which is important for maintaining compatibility with older Anthropic API integrations.
packages/opentelemetry-instrumentation-anthropic/opentelemetry/instrumentation/anthropic/span_utils.py (4)
115-135: Good separation of tool use blocks from content.The logic correctly separates tool use blocks from other content blocks when processing messages. This ensures tool calls are recorded with proper attributes while maintaining the regular content flow.
136-152: Tool call attributes are properly structured.The implementation correctly records tool call details including ID, name, and JSON-serialized arguments under the appropriate span attribute paths.
188-189: Good defensive check for text attribute existence.Adding
hasattr(content, "text")prevents potential AttributeError when processing content blocks that may not have a text attribute.
270-299: Improved streaming response handling with explicit event type processing.The refactored code is cleaner and more maintainable:
- Explicit index management for completions
- Proper handling of thinking events as separate completions
- Clear distinction between tool_use events and regular text events
The removal of the try-except block makes the code flow more predictable.
packages/opentelemetry-instrumentation-anthropic/tests/test_messages.py (2)
33-68: Excellent refactoring - centralizing tool definitions improves maintainability.Moving tool definitions to a global
TOOLSlist eliminates duplication across tests and ensures consistency. This makes it easier to update tool schemas in one place.
1605-2312: Comprehensive test coverage for tool usage scenarios.The new tests thoroughly cover:
- Tool usage with legacy attributes
- Streaming tool calls with events
- Multi-turn conversations with tool results
- Both synchronous and asynchronous operations
Each test properly verifies span attributes, metrics, and event logs for tool-related interactions.
Co-authored-by: Nir Gazit <nirga@users.noreply.github.com>
2 participants
Fixes the following:
feat(instrumentation): ...orfix(instrumentation): ....Important
Fixes tool usage recording issues in Anthropic instrumentation for both streaming and non-streaming modes, with added tests for verification.
event_emitter.pyandstreaming.py.span_utils.py.test_messages.pyto verify tool usage recording in various scenarios, including legacy and streaming modes.test_anthropic_tools_history_legacy.yamlandtest_anthropic_tools_streaming_with_events_with_content.yaml.This description was created by
for 81dcefa. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Tests