revert: fix(integrations): ensure that GEN_AI_AGENT_NAME is properly …#5718
revert: fix(integrations): ensure that GEN_AI_AGENT_NAME is properly …#5718alexander-alderman-webb wants to merge 1 commit into
Conversation
…set for GEN_AI spans under an invoke_agent span
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Anthropic
Documentation 📚
Internal Changes 🔧
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 6.61s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 0.00%. Project has 14333 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 25.33% 30.31% +4.98%
==========================================
Files 189 189 —
Lines 20613 20566 -47
Branches 6738 6728 -10
==========================================
+ Hits 5222 6233 +1011
- Misses 15391 14333 -1058
- Partials 429 474 +45Generated by Codecov Action |
|
Closing, will do this piece-by-piece |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
|
|
||
| old_iterator = result | ||
|
|
||
| def new_iterator() -> "Iterator[Any]": | ||
| exc_info: "tuple[Any, Any, Any]" = (None, None, None) | ||
| try: | ||
| for event in old_iterator: | ||
| yield event | ||
| def new_iterator(): | ||
| # type: () -> Iterator[Any] | ||
| for event in old_iterator: | ||
| yield event | ||
|
|
||
| try: | ||
| output = event.get("output") | ||
| except Exception: | ||
| output = None | ||
|
|
||
| if ( | ||
| output is not None | ||
| and should_send_default_pii() | ||
| and integration.include_prompts | ||
| ): | ||
| set_data_normalized(span, SPANDATA.GEN_AI_RESPONSE_TEXT, output) | ||
| except Exception: | ||
| exc_info = sys.exc_info() | ||
| set_span_errored(span) | ||
| raise | ||
| finally: | ||
| # Ensure cleanup happens even if iterator is abandoned or fails | ||
| _pop_agent() | ||
| span.__exit__(*exc_info) | ||
|
|
||
| async def new_iterator_async() -> "AsyncIterator[Any]": | ||
| exc_info: "tuple[Any, Any, Any]" = (None, None, None) | ||
| try: | ||
| async for event in old_iterator: | ||
| yield event | ||
| output = event.get("output") | ||
| except Exception: | ||
| output = None | ||
|
|
||
| try: | ||
| output = event.get("output") | ||
| except Exception: | ||
| output = None | ||
|
|
||
| if ( | ||
| output is not None | ||
| and should_send_default_pii() | ||
| and integration.include_prompts | ||
| ): | ||
| set_data_normalized(span, SPANDATA.GEN_AI_RESPONSE_TEXT, output) | ||
| if ( | ||
| output is not None | ||
| and should_send_default_pii() | ||
| and integration.include_prompts | ||
| ): | ||
| set_data_normalized(span, SPANDATA.GEN_AI_RESPONSE_TEXT, output) | ||
|
|
||
| span.__exit__(None, None, None) |
There was a problem hiding this comment.
Iterator exception handling removed - span errors not captured during streaming
The revert removes exception handling from both new_iterator() and new_iterator_async(). Previously, exceptions were caught with exc_info = sys.exc_info(), the span was marked as errored via set_span_errored(span), and the exception info was passed to span.__exit__(*exc_info). Now, span.__exit__(None, None, None) is called unconditionally, meaning if an exception occurs during iteration, the span will be closed as successful rather than errored, and Sentry will not capture the error context.
Verification
Read the current code at lines 1030-1047 (new_iterator) and 1049-1066 (new_iterator_async). Confirmed the diff removes try/except/finally blocks that previously called set_span_errored and passed exc_info to span.exit. The current code always calls span.exit(None, None, None).
Also found at 1 additional location
sentry_sdk/integrations/langchain.py:1049-1066
Identified by Warden code-review · FHU-PUV
| Callable, | ||
| Dict, | ||
| Iterator, | ||
| List, | ||
| Optional, | ||
| Union, |
There was a problem hiding this comment.
Removed Iterator and AsyncIterator imports are still used in type comments
This change removes Iterator and AsyncIterator from the TYPE_CHECKING imports, but these types are still referenced in type comments at lines 1031 (# type: () -> Iterator[Any]) and 1050 (# type: () -> AsyncIterator[Any]). While this won't cause runtime errors (type comments are not evaluated at runtime), it will break type checking with tools like mypy, causing undefined name errors during static analysis.
Verification
Read the full langchain.py file and used grep to confirm Iterator is referenced at lines 1031 and 1050 as type comments. Verified these imports were in the TYPE_CHECKING block being modified by this diff.
Suggested fix: Keep the Iterator and AsyncIterator imports in the TYPE_CHECKING block since they are used in type comments later in the file.
| Callable, | |
| Dict, | |
| Iterator, | |
| List, | |
| Optional, | |
| Union, | |
| AsyncIterator, | |
| Iterator, |
Identified by Warden code-review · KCC-CXG
| return llm_type | ||
|
|
||
|
|
||
| @pytest.mark.xfail |
There was a problem hiding this comment.
Main integration test marked as xfail without explanation
The test_langchain_agent test is marked with @pytest.mark.xfail, meaning it is expected to fail and won't cause CI failures if it does. This test validates critical functionality including token usage recording, prompt capturing, tool calls, and PII handling. The PR description is empty, providing no explanation for why this test is being disabled or when it will be fixed. This reduces effective test coverage for the LangChain integration.
Verification
Read tests/integrations/langchain/test_langchain.py to verify the test function scope (lines 69-318). The test covers 12 parameter combinations (4 send_default_pii/include_prompts x 3 system_instructions_content), validates token usage, response text, tool calls, finish reasons, and available tools. Confirmed this is a significant integration test being disabled.
Identified by Warden code-review · YW2-EYP
| assert SPANDATA.GEN_AI_REQUEST_MESSAGES not in tool_exec_span.get("data", {}) | ||
| assert SPANDATA.GEN_AI_RESPONSE_TEXT not in tool_exec_span.get("data", {}) |
There was a problem hiding this comment.
Test assertions no longer verify tool span PII protection
The assertions are changed from checking GEN_AI_TOOL_INPUT and GEN_AI_TOOL_OUTPUT to GEN_AI_REQUEST_MESSAGES and GEN_AI_RESPONSE_TEXT. However, the langchain integration's on_tool_start and on_tool_end methods only set GEN_AI_TOOL_INPUT and GEN_AI_TOOL_OUTPUT on tool execution spans (lines 615, 631 in langchain.py). The new assertions check for attributes that are never set on tool spans, so the test will always pass trivially without actually verifying that tool input/output is hidden when PII is disabled. This removes effective test coverage for PII protection on tool execution spans.
Verification
Verified by reading langchain.py: on_tool_start sets SPANDATA.GEN_AI_TOOL_INPUT at line 615 and on_tool_end sets SPANDATA.GEN_AI_TOOL_OUTPUT at line 631. Neither method ever sets GEN_AI_REQUEST_MESSAGES or GEN_AI_RESPONSE_TEXT on tool spans. The test at lines 243-244 still expects GEN_AI_TOOL_INPUT and GEN_AI_TOOL_OUTPUT to be present when PII is enabled, confirming these are the correct attributes for tool spans.
Identified by Warden code-review · TAS-M8H
1 participant
…set for GEN_AI spans under an invoke_agent span
Description
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)