fix(langchain): detach existing SpanHolder token before overwrite in _create_llm_span

[saivedant169]

Closes #3957

• I have added tests that cover my changes.
• If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
• PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
• (If applicable) I have updated the documentation accordingly.

What

In _create_llm_span(), when a SpanHolder already exists for a given run_id, the old entry was being replaced directly. The previous holder's token (from context_api.attach(set_value(SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY, True))) was dropped on the floor without ever being detached, leaving an orphaned entry on the OTel context stack.

This is the same class of bug as the ones fixed in #3526 and #3807, just on a different code path. It was flagged during review of #3807 and tracked separately in #3957.

Fix

Before overwriting self.spans[run_id], check if an existing holder is present and detach its token through the existing _safe_detach_context() helper.

existing_holder = self.spans.get(run_id)
if existing_holder is not None and existing_holder.token is not None:
    self._safe_detach_context(existing_holder.token)

Scoped strictly to _create_llm_span() as described in #3957. The same pattern exists in _create_span() and _create_task_span(), but those were not called out in the issue, so leaving them out of this PR to keep it tight.

Tests

No new tests added — this is a defensive fix for a race between the runtime overwriting the registry entry and the old token remaining attached, which is hard to cover with cassette-based tests. Happy to add one if a maintainer suggests a good angle.

AI Disclosure

Took help of an AI tool to draft the fix and this description.

Summary by CodeRabbit

• Bug Fixes
  • Improved OpenTelemetry instrumentation context management to prevent resource leaks during concurrent span operations by properly detaching prior context attachments before reassignment.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc6e3602-1990-46a1-ab5a-8d33abb39ee6

📥 Commits

Reviewing files that changed from the base of the PR and between 0a25803 and 6940005.

📒 Files selected for processing (1)

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

---

📝 Walkthrough

Walkthrough

The _create_llm_span() method in the langchain instrumentation callback handler now validates and properly detaches existing context tokens before overwriting a SpanHolder entry. This prevents silent context corruption from orphaned context_api.attach() tokens when the same run_id triggers multiple span creation calls.

Changes

Cohort / File(s)	Summary
Context Token Cleanup packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py	Added defensive check in _create_llm_span() to detect and safely detach existing SpanHolder context tokens before overwriting the span registry entry for a given run_id.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• 🐛 Bug Report: Orphaned context_api.attach() Calls Corrupt OpenTelemetry Context Stack #3526: Orphaned context_api.attach() calls—this fix extends the same token detachment pattern previously applied in PR #3807 to the _create_llm_span() code path.

Poem

🐰 A context token floats adrift,
Lost when spans would overlap and shift,
But now we check before we store,
Detaching tokens as before,
No orphans left behind! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR partially addresses the linked issue #3957 by detaching the token from existing SpanHolder before overwrite, but does not detach the association_properties_token also mentioned in the issue.	Add logic to detach existing_holder.association_properties_token before overwriting the SpanHolder, as specified in issue #3957.
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 (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: detaching an existing SpanHolder token before overwriting it in _create_llm_span, which is the core fix in the PR.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objective of detaching context tokens when overwriting SpanHolder entries in _create_llm_span().

_{✏️ 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.}