add graph_structure #3508
Open
add graph_structure #3508
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
WalkthroughThis PR extends OpenTelemetry instrumentation for LangChain by adding LangGraph graph structure extraction and serialization capabilities. A new helper module provides utilities to extract compiled state graphs from the Python call stack and convert them to JSON structures, while the callback handler integrates this to annotate spans with graph structure data when detecting LangGraph tasks. Changes
Sequence DiagramsequenceDiagram
participant Callback as Callback Handler
participant Detector as is_langgraph_task
participant Helper as LangGraph Helper
participant Stack as Call Stack
participant Span as OTel Span
Callback->>Callback: on_chain_start(chain_name)
Callback->>Detector: is_langgraph_task(chain_name)
Detector-->>Callback: true/false
alt Is LangGraph Task
Callback->>Helper: get_graph_structure()
Helper->>Stack: get_compiled_graph()
Stack-->>Helper: CompiledStateGraph or None
alt Graph Found
Helper->>Helper: build_node_graph(graph)
Helper-->>Callback: JSON string (nodes + edges)
else Graph Not Found
Helper-->>Callback: "{}"
end
Callback->>Span: set_attribute(graph_structure, data)
end
Callback-->>Span: chain/task span created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 4561359 in 2 minutes and 0 seconds. Click for details.
- Reviewed
137lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4draft 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-langchain/opentelemetry/instrumentation/langchain/callback_handler.py:438
- Draft comment:
When setting the graph_structure attribute, consider verifying that the returned JSON isn’t empty. A comment on the expected structure could improve clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is making suggestions about code quality improvements (checking for empty values, adding documentation). However, it's not pointing out a definite bug or issue. The comment uses "consider" which is speculative language. Without seeing the implementation ofget_graph_structure(), I can't determine if empty values are actually a problem or if the function already handles this appropriately. The suggestion to add a comment about expected structure is a documentation suggestion, not a code issue. According to the rules, I should not keep speculative comments or comments that just suggest improvements without clear evidence of an issue. The comment might be valid ifget_graph_structure()can return empty or invalid data that would cause issues downstream. Without seeing the implementation or knowing if span attributes handle empty values gracefully, I might be dismissing a legitimate concern about data validation. Even if empty values could be problematic, the comment uses "consider" which makes it speculative rather than definitive. The rules state to only comment if it's definitely an issue, not "if X, then Y is an issue". Additionally, asking for documentation comments about expected structure is not a code change requirement. Without strong evidence that this is a real bug, this should be deleted. This comment should be deleted because it's speculative ("consider"), suggests documentation improvements rather than code fixes, and doesn't provide strong evidence of a definite issue. The rules explicitly state not to make speculative comments.
2. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py:9
- Draft comment:
The is_langgraph_task() function strictly checks for an exact match ('LangGraph'). Consider making this more flexible or configurable if variations are possible. - 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% The comment is suggesting a potential improvement ("consider making this more flexible") but doesn't point to any concrete problem. It's speculative - "if variations are possible" - which violates the rule about not making speculative comments. There's no evidence in the diff that variations are needed or that this exact match is problematic. The function name and implementation are straightforward. Without seeing how this function is used elsewhere or evidence that variations exist, this is just a "nice to have" suggestion rather than identifying a clear issue. This type of comment ("consider making this more flexible") is not actionable without specific requirements. Perhaps the author has knowledge from other parts of the codebase that there are indeed variations of "LangGraph" task names that need to be handled. Maybe this is based on actual usage patterns in the instrumentation library. Even if there were variations, the comment doesn't provide any evidence of them or specific examples. The comment is conditional ("if variations are possible") which makes it speculative. If the author knew of actual variations, they should have been mentioned. Without concrete evidence in the diff or the comment itself, this is just a general suggestion for flexibility. This comment should be deleted. It's speculative ("if variations are possible"), not actionable without specific requirements, and doesn't identify a concrete problem with the code. It's a general suggestion for flexibility without evidence that flexibility is needed.
3. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py:16
- Draft comment:
Using inspect.stack() can be costly. If get_compiled_graph() is called frequently, consider caching the result or optimizing the stack inspection. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is speculative in nature - it says "can be costly" and "if called frequently, consider...". This is a conditional suggestion ("If X, then consider Y") rather than pointing out a definite problem. The comment doesn't provide evidence that this function IS called frequently, nor does it demonstrate that there's an actual performance issue. It's more of a general performance tip rather than identifying a concrete problem with the implementation. According to the rules, speculative comments like "If X, then Y is an issue" should not be kept - we should only comment if it's definitely an issue. However,inspect.stack()is indeed known to be expensive in Python, and this is a legitimate code quality concern. The comment could be seen as a reasonable performance optimization suggestion rather than pure speculation. It's providing actionable advice about a known performance characteristic of the code. Whileinspect.stack()is known to be expensive, the comment is still speculative because it's conditional on the function being "called frequently" - which we don't know from the context. The comment doesn't demonstrate that there's an actual performance problem in this specific use case. It's a "consider this" suggestion rather than identifying a definite issue that needs fixing. This comment should be deleted. It's a speculative performance suggestion conditional on the function being called frequently, which we have no evidence of. The rules explicitly state not to make speculative comments like "If X, then Y is an issue" - only comment if it's definitely an issue.
4. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py:69
- Draft comment:
In build_node_graph(), branch.ends is assumed to be a mapping. Consider adding type checks or error handling to ensure robustness if the structure changes. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment is suggesting defensive programming - adding type checks "in case the structure changes." This is speculative by nature - it's saying "consider adding checks to ensure robustness if the structure changes." This is a classic example of a speculative comment that says "If X changes, then Y could be an issue." The rules explicitly state: "Do NOT make speculative comments, such as 'If X, then Y is an issue'. Only comment if it is definitely an issue." The comment doesn't identify a current bug or problem with the code as written - it's suggesting preemptive error handling for potential future API changes. Without evidence thatbranch.endsmight not be a mapping in the current langgraph API, this is purely speculative. Could this actually be a real issue? Maybe the langgraph library doesn't guarantee thatbranch.endsis always a mapping, and this could fail at runtime. Without seeing the langgraph library's type definitions or documentation, I can't be 100% sure this is speculative. While it's possible thatbranch.endsmight not always be a mapping, the comment explicitly frames this as a future concern ("if the structure changes") rather than a current issue. The author is clearly working with the langgraph API and presumably tested this code. Ifbranch.endswasn't a mapping, this would fail immediately at runtime during testing. This comment is speculative - it's suggesting defensive programming for potential future changes rather than identifying a current bug. According to the rules, speculative comments should be removed. The comment should be deleted.
Workflow ID: wflow_XciEvEP54svfHbUa
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: 1
🧹 Nitpick comments (5)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
438-441: Consider using a constant for the attribute name.The implementation correctly guards graph extraction with
is_langgraph_task(). However, other span attributes in this file use constants fromSpanAttributes. Consider defininggraph_structureas a constant for consistency and maintainability.- span.set_attribute("graph_structure", graph_structure) + span.set_attribute(SpanAttributes.GRAPH_STRUCTURE, graph_structure)This would require adding the constant to the appropriate attributes module.
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py (4)
13-24: Clean up frame references to avoid potential memory issues.
inspect.stack()creates references to frame objects. In some Python versions, holding these references can prevent garbage collection of local variables. Consider usingtry/finallyto ensure cleanup.def get_compiled_graph(): """ Get the compiled graph from the call stack """ graph = None invocation_methods = ["Pregel.invoke", "Pregel.ainvoke", "Pregel.stream", "Pregel.astream"] frames = inspect.stack() - for frame_info in frames[1:]: - if frame_info.frame.f_code.co_qualname in invocation_methods: - local_vars = frame_info.frame.f_locals - graph = local_vars.get("self", None) - graph = graph if isinstance(graph, CompiledStateGraph) else None - break - return graph + try: + for frame_info in frames[1:]: + if frame_info.frame.f_code.co_qualname in invocation_methods: + local_vars = frame_info.frame.f_locals + graph = local_vars.get("self", None) + graph = graph if isinstance(graph, CompiledStateGraph) else None + break + finally: + del frames + return graph
27-35: UseTuplefromtypingfor Python 3.8 compatibility.The lowercase
tuple[str, ...]syntax requires Python 3.9+. For broader compatibility, useTuplefrom thetypingmodule which is already imported.+from typing import List, Dict, Union, Any, Tuple + def _normalize_endpoint_names( - names: Union[str, List[str], tuple[str, ...]] + names: Union[str, List[str], Tuple[str, ...]] ) -> List[str]:
68-75: Fix incomplete type annotation and add defensive attribute access.The type annotation on line 69 is incomplete (
Dict[str, Dict[str]]), and accessingbuilder.brancheswithout checking could fail if the graph structure varies.# Branches - branches: Dict[str, Dict[str]] = builder.branches - for source, branch_map in branches.items(): - for branch in branch_map.values(): - # branch.ends is expected to be a mapping; we use its values as destinations - dest_names = list(branch.ends.values()) - # Source is a single node here - edges.append([[source], dest_names]) + branches: Dict[str, Dict[str, Any]] = getattr(builder, "branches", {}) + for source, branch_map in branches.items(): + for branch in branch_map.values(): + ends = getattr(branch, "ends", None) + if ends: + dest_names = list(ends.values()) + edges.append([[source], dest_names])
89-104: Add exception handling for robustness.If
build_node_graph()raises an exception (e.g., due to unexpected graph structure), it will propagate up. While the caller has@dont_throw, handling exceptions here provides cleaner fallback behavior.def get_graph_structure() -> str: """ Get graph structure as a JSON string. Returns: JSON string with structure: { "nodes": [...], "edges": [[[...], [...]], ...] } """ graph_structure: Dict[str, Any] = {} - graph = get_compiled_graph() - if graph: - graph_structure = build_node_graph(graph) + try: + graph = get_compiled_graph() + if graph: + graph_structure = build_node_graph(graph) + except Exception: + pass # Return empty structure on any failure return json.dumps(graph_structure)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py(2 hunks)packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.pypackages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py (2)
is_langgraph_task(9-10)get_graph_structure(89-104)
🪛 Ruff (0.14.7)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py
35-35: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)
36-39: LGTM!The import is properly grouped with other local module imports.
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py (1)
9-10: LGTM!Simple and effective check for LangGraph tasks.
Comment on lines
+1
to
+6
| import inspect | ||
| import json | ||
| from langgraph.graph.state import CompiledStateGraph | ||
|
|
||
|
|
||
| from typing import List, Dict, Union, Any |
There was a problem hiding this comment.
Handle optional langgraph dependency gracefully.
The unconditional import of CompiledStateGraph will raise ImportError if langgraph is not installed. Since this is an instrumentation library that may be used in environments where langgraph isn't present, consider using a lazy import pattern.
import inspect
import json
-from langgraph.graph.state import CompiledStateGraph
-
-
from typing import List, Dict, Union, Any
+
+try:
+ from langgraph.graph.state import CompiledStateGraph
+ HAS_LANGGRAPH = True
+except ImportError:
+ CompiledStateGraph = None
+ HAS_LANGGRAPH = FalseThen guard the functions accordingly:
def get_compiled_graph():
if not HAS_LANGGRAPH:
return None
# ... rest of implementation🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/langgraph_helper.py
around lines 1-6, the direct import of CompiledStateGraph will raise ImportError
when langgraph is absent; change to a guarded lazy-import pattern by wrapping
the import in a try/except ImportError that sets a module-level HAS_LANGGRAPH
boolean (False on exception) and only import CompiledStateGraph when available
(or import it inside functions that need it). Update public helper functions
(e.g., get_compiled_graph) to check HAS_LANGGRAPH at the start and return None
(or no-op) when False, so callers won’t error in environments without langgraph.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(instrumentation): ...orfix(instrumentation): ....Important
Adds graph structure extraction and integration in Langchain instrumentation, attaching graph data as span attributes for specific tasks.
on_chain_start()incallback_handler.pyusingget_graph_structure().is_langgraph_task()returns true.langgraph_helper.pyfile withis_langgraph_task(),get_compiled_graph(),build_node_graph(), andget_graph_structure()functions.get_graph_structure()returns a JSON string of nodes and edges fromCompiledStateGraph.is_langgraph_taskandget_graph_structureincallback_handler.py.This description was created by
for 4561359. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.