fix(langchain): detach orphaned context_api.attach() calls that corrupt OTel context#3807
fix(langchain): detach orphaned context_api.attach() calls that corrupt OTel context#3807saivedant169 wants to merge 2 commits intotraceloop:mainfrom
Conversation
…pt OTel context Two orphaned context_api.attach() calls were leaving stale contexts on the OpenTelemetry stack, causing trace.get_current_span() to return ended spans after LangChain execution. 1. _create_span(): association_properties attach token was never saved or detached. Now stored in SpanHolder.association_properties_token and detached in _end_span(). 2. on_chain_end(): redundant attach to reset SUPPRESS key — already handled by _end_span() detaching SpanHolder.token. Removed. Fixes traceloop#3526
|
|
📝 WalkthroughWalkthroughThis pull request fixes orphaned Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `pylint` configuration to improve the quality of Python code reviews.Add a pylint configuration file to your project to customize how CodeRabbit runs |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py (1)
368-385:⚠️ Potential issue | 🟠 MajorFix double-encoding of OpenAI-style tool call arguments.
Lines 373-385 incorrectly handle pre-serialized arguments from OpenAI API responses. When tool_args comes from
function.arguments(the fallback path), it's already a JSON string. Applyingjson.dumps()to a string wraps it in additional quotes/escapes, producing invalid span attributes. Check the type before serializing:Suggested fix
- _set_span_attribute( - span, - f"{tool_call_prefix}.arguments", - json.dumps(tool_args, cls=CallbackFilteredJSONEncoder), - ) + serialized_args = ( + tool_args + if isinstance(tool_args, str) or tool_args is None + else json.dumps(tool_args, cls=CallbackFilteredJSONEncoder) + ) + _set_span_attribute(span, f"{tool_call_prefix}.arguments", serialized_args)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py` around lines 368 - 385, _in _set_chat_tool_calls, pre-serialized OpenAI function.arguments (a JSON string) is being passed through json.dumps which double-encodes it; update the logic in _set_chat_tool_calls to check the type of tool_args and only call json.dumps(tool_args, cls=CallbackFilteredJSONEncoder) when tool_args is not a str (i.e., when it is a dict/list), otherwise pass the string verbatim to _set_span_attribute for f"{tool_call_prefix}.arguments"; use the existing CallbackFilteredJSONEncoder only for non-string objects and keep using _set_span_attribute to set the final attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py`:
- Around line 459-465: The code is overwriting the SpanHolder in _create_span(),
losing previously saved span and metadata attachment tokens; instead of
replacing self.spans[run_id] with a new SpanHolder, add a suppression_token
field to the existing SpanHolder (or construct the holder with suppression_token
set) and set that field when _safe_attach_context() returns the suppression
token; update SpanHolder to include suppression_token and modify _end_span() to
detach tokens in reverse attach order: first suppression_token, then the span
attachment token, then association_properties_token so all context attachments
are properly unwound.
---
Outside diff comments:
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py`:
- Around line 368-385: _in _set_chat_tool_calls, pre-serialized OpenAI
function.arguments (a JSON string) is being passed through json.dumps which
double-encodes it; update the logic in _set_chat_tool_calls to check the type of
tool_args and only call json.dumps(tool_args, cls=CallbackFilteredJSONEncoder)
when tool_args is not a str (i.e., when it is a dict/list), otherwise pass the
string verbatim to _set_span_attribute for f"{tool_call_prefix}.arguments"; use
the existing CallbackFilteredJSONEncoder only for non-string objects and keep
using _set_span_attribute to set the final attribute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f74a231e-4bad-4a06-864a-f4fd7074d97f
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pypackages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py
| token = context_api.attach(context_api.set_value(SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY, True)) | ||
| except Exception: | ||
| # If context setting fails, continue without suppression token | ||
| token = None | ||
|
|
||
| self.spans[run_id] = SpanHolder( | ||
| span, token, None, [], workflow_name, None, entity_path | ||
| ) | ||
| self.spans[run_id] = SpanHolder(span, token, None, [], workflow_name, None, entity_path) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "callback_handler.py" -path "*/instrumentation/langchain/*" | head -5Repository: traceloop/openllmetry
Length of output: 175
🏁 Script executed:
wc -l packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 178
🏁 Script executed:
# Find SpanHolder class definition and key methods
rg "class SpanHolder" -A 10 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 47
🏁 Script executed:
# Find _create_span method
rg "def _create_span" -A 30 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 1434
🏁 Script executed:
# Find _create_llm_span method
rg "def _create_llm_span" -A 40 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 1617
🏁 Script executed:
# Find _end_span method
rg "def _end_span" -A 20 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 921
🏁 Script executed:
# Find SpanHolder namedtuple/class definition
rg "SpanHolder" packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | head -20Repository: traceloop/openllmetry
Length of output: 343
🏁 Script executed:
# Get more context around _create_span to see where the holder is stored
rg "def _create_span" -A 60 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | tail -40Repository: traceloop/openllmetry
Length of output: 2025
🏁 Script executed:
# Check around line 450-465 to see the exact code
sed -n '440,475p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 1398
🏁 Script executed:
# Find SpanHolder definition/import
rg -B 5 "SpanHolder" packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | grep -A 10 "class SpanHolder\|namedtuple.*SpanHolder\|SpanHolder ="Repository: traceloop/openllmetry
Length of output: 47
🏁 Script executed:
# Get the complete _create_span method
sed -n '194,250p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 2595
🏁 Script executed:
# Check what imports are at the top
head -50 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | grep -i spanholdRepository: traceloop/openllmetry
Length of output: 80
🏁 Script executed:
# Look for SpanHolder in the models/types files
find packages/opentelemetry-instrumentation-langchain -name "*.py" -exec grep -l "SpanHolder" {} \;Repository: traceloop/openllmetry
Length of output: 374
🏁 Script executed:
# Find SpanHolder definition in patch.py or span_utils.py
rg "class SpanHolder|SpanHolder\s*=" packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.pyRepository: traceloop/openllmetry
Length of output: 82
🏁 Script executed:
# Check patch.py
rg "class SpanHolder|SpanHolder\s*=" packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.pyRepository: traceloop/openllmetry
Length of output: 47
🏁 Script executed:
# Look for imports in callback_handler.py
head -100 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | grep -i importRepository: traceloop/openllmetry
Length of output: 1379
🏁 Script executed:
# Get the full SpanHolder class definition from span_utils.py
rg "class SpanHolder" -A 20 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.pyRepository: traceloop/openllmetry
Length of output: 624
🏁 Script executed:
# Also check if SpanHolder is a dataclass or namedtuple
sed -n '1,100p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/span_utils.py | grep -A 20 "SpanHolder"Repository: traceloop/openllmetry
Length of output: 1470
🏁 Script executed:
# Get complete _create_span method to see if it stores the holder
sed -n '194,250p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 2595
🏁 Script executed:
# Check the end of _create_span where it returns
sed -n '220,250p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 1544
🏁 Script executed:
# Find where _create_span stores the SpanHolder to self.spans
rg "def _create_span" -A 80 packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py | head -100Repository: traceloop/openllmetry
Length of output: 3585
🏁 Script executed:
# Search for all lines where self.spans[run_id] is assigned in _create_span
sed -n '257,335p' packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.pyRepository: traceloop/openllmetry
Length of output: 3514
Don't replace the SpanHolder after _create_span() has saved its tokens.
_create_span() attaches the span context via _safe_attach_context() and stores the attachment token in the holder. When metadata is present, it also attaches and stores the association_properties_token. However, line 464 overwrites that holder with a new SpanHolder that replaces the span attachment token with only the suppression token and loses the metadata attachment token. When _end_span() runs, it cannot unwind the original span attachment or the metadata attachment—only the suppression token—leaving stale context attachments on the OpenTelemetry stack.
Suggested direction
- try:
- token = context_api.attach(context_api.set_value(SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY, True))
- except Exception:
- # If context setting fails, continue without suppression token
- token = None
-
- self.spans[run_id] = SpanHolder(span, token, None, [], workflow_name, None, entity_path)
+ try:
+ suppression_token = context_api.attach(
+ context_api.set_value(SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY, True)
+ )
+ except Exception:
+ suppression_token = None
+
+ self.spans[run_id].suppression_token = suppression_tokenSpanHolder will need a dedicated suppression_token field, and _end_span() should detach in reverse attach order: suppression token, span token, then association_properties_token.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py`
around lines 459 - 465, The code is overwriting the SpanHolder in
_create_span(), losing previously saved span and metadata attachment tokens;
instead of replacing self.spans[run_id] with a new SpanHolder, add a
suppression_token field to the existing SpanHolder (or construct the holder with
suppression_token set) and set that field when _safe_attach_context() returns
the suppression token; update SpanHolder to include suppression_token and modify
_end_span() to detach tokens in reverse attach order: first suppression_token,
then the span attachment token, then association_properties_token so all context
attachments are properly unwound.
|
Thanks for the thorough analysis @coderabbitai. The point about The Sourcery review approved the changes. Happy to address any maintainer feedback. |
|
Tip For best results, initiate chat on the files or code changes.
Would you like me to open a separate GitHub issue to track the |
2 participants
Fixes #3526
Description
The LangChain instrumentation has two code paths where
context_api.attach()is called without a correspondingcontext_api.detach(), leaving orphaned contexts on the OpenTelemetry stack. After LangChain execution completes,trace.get_current_span()returns an ended span instead of the parent span, breaking downstream trace context propagation and log correlation.Root cause
1.
_create_span()— association_properties token lostFix: Save the token and store it in a new
SpanHolder.association_properties_tokenfield. Detach it in_end_span()alongside the span token.2.
on_chain_end()— redundant attach without detachFix: Removed entirely. The suppression token is already properly detached by
_end_span()viaSpanHolder.token, so this second attach was both redundant and harmful.Changes
span_utils.py: Addedassociation_properties_tokenfield toSpanHolderdataclasscallback_handler.py: Save association_properties attach token and detach in_end_span()callback_handler.py: Remove redundant orphaned attach inon_chain_end()Testing
ruff checkandruff formatpass on all changed filesSummary by CodeRabbit