fix(langchain): handle ToolNode object in create_react_agent instrumentation#3846
fix(langchain): handle ToolNode object in create_react_agent instrumentation#3846SanjanaB123 wants to merge 3 commits intotraceloop:mainfrom
Conversation
ToolNode (from langgraph-prebuilt) is not iterable, but patch.py assumed tools was always a list. Check for tools_by_name attribute to extract tools from ToolNode objects. Fixes traceloop#3841
📝 WalkthroughWalkthroughThis change updates the agent wrapper to handle LangGraph Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-langchain/tests/test_langgraph.py (2)
827-838: Consider extractingMockChatModelto reduce duplication.
MockChatModelis defined identically in three tests (test_create_react_agent_span,test_create_agent_with_system_prompt, and this test). Extracting it to a module-level fixture or helper class would reduce maintenance burden.♻️ Proposed refactor
Add a module-level mock class or fixture at the top of the file:
class MockChatModel(BaseChatModel): """Mock chat model for testing agent creation.""" `@property` def _llm_type(self) -> str: return "mock" def _generate(self, messages, stop=None, run_manager=None, **kwargs): return ChatResult( generations=[ChatGeneration(message=AIMessage(content="Mock"))] ) def bind_tools(self, tools, **kwargs): return selfThen use
MockChatModel()directly in each test without redefining it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-langchain/tests/test_langgraph.py` around lines 827 - 838, Extract the duplicate MockChatModel class into a single module-level helper or fixture so tests reuse it instead of redefining; create the class named MockChatModel once at top of the file (or as a pytest fixture returning MockChatModel()), then update each test that currently defines MockChatModel (test_create_react_agent_span, test_create_agent_with_system_prompt, and test_langgraph.py's current test) to instantiate or reference that shared MockChatModel rather than declaring the class locally.
811-871: Well-structured regression test for issue#3841.The test properly validates that
ToolNodeobjects are handled without raisingTypeError, and verifies that tool definitions are correctly extracted viatools_by_name.values().Consider adding a comment or separate test case for edge cases like an empty
ToolNode(i.e.,ToolNode([])) to ensure the instrumentation handles that gracefully, though this is optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-langchain/tests/test_langgraph.py` around lines 811 - 871, Add a new regression test similar to test_create_react_agent_with_toolnode that constructs an empty ToolNode (ToolNode([])), calls create_react_agent with model=MockChatModel() and tools=that empty ToolNode, then assert no exception is raised and that the created span (find by "create_agent" in spans) contains GenAIAttributes.GEN_AI_TOOL_DEFINITIONS (which should parse to an empty list/object); reuse the same MockChatModel, span_exporter usage and JSON-parsing pattern from test_create_react_agent_with_toolnode to validate the instrumentation handles empty ToolNode gracefully.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/opentelemetry-instrumentation-langchain/tests/test_langgraph.py`:
- Around line 827-838: Extract the duplicate MockChatModel class into a single
module-level helper or fixture so tests reuse it instead of redefining; create
the class named MockChatModel once at top of the file (or as a pytest fixture
returning MockChatModel()), then update each test that currently defines
MockChatModel (test_create_react_agent_span,
test_create_agent_with_system_prompt, and test_langgraph.py's current test) to
instantiate or reference that shared MockChatModel rather than declaring the
class locally.
- Around line 811-871: Add a new regression test similar to
test_create_react_agent_with_toolnode that constructs an empty ToolNode
(ToolNode([])), calls create_react_agent with model=MockChatModel() and
tools=that empty ToolNode, then assert no exception is raised and that the
created span (find by "create_agent" in spans) contains
GenAIAttributes.GEN_AI_TOOL_DEFINITIONS (which should parse to an empty
list/object); reuse the same MockChatModel, span_exporter usage and JSON-parsing
pattern from test_create_react_agent_with_toolnode to validate the
instrumentation handles empty ToolNode gracefully.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bd08649-87a6-4003-a96c-847d60eaa0e7
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-langchain/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-langchain/tests/test_langgraph.py
Summary
ToolNode(fromlanggraph-prebuilt) is not iterable, butpatch.pyassumedtoolswas always a listhasattr(tools, 'tools_by_name')check to extract tools viatools_by_name.values()when aToolNodeis passedtest_create_react_agent_with_toolnodeReproduction
feat(instrumentation): ...orfix(instrumentation): ....Summary by CodeRabbit
Bug Fixes
Tests