Skip to content

fix(langchain): remove orphaned context_api.attach() in on_chain_end#4100

Merged
dvirski merged 2 commits into
mainfrom
dr/fix(langchain)-remove-orphaned-context_api.attach()-in-on_chain_end
May 14, 2026
Merged

fix(langchain): remove orphaned context_api.attach() in on_chain_end#4100
dvirski merged 2 commits into
mainfrom
dr/fix(langchain)-remove-orphaned-context_api.attach()-in-on_chain_end

Conversation

@dvirski
Copy link
Copy Markdown
Contributor

@dvirski dvirski commented May 10, 2026

Fixes #3526.

The SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY=False attach at the end
of on_chain_end() was redundant — _end_span() already detaches the
suppression token via SpanHolder. The orphaned attach was corrupting
the OTel context stack on every root chain completion.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented a context leak so suppression state is not left behind after a top-level chain ends, improving instrumentation correctness.
  • Tests
    • Added regression tests to ensure suppression state remains absent after root chains and repeated root runs do not accumulate leaked context.

Review Change Stack

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 10, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Caution

Review failed

An error occurred during the review process. Please try again later.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51907c7f-be2d-444e-8581-4750f6da5c65

📥 Commits

Reviewing files that changed from the base of the PR and between 11b718c and 226d5dd.

⛔ Files ignored due to path filters (1)
  • packages/traceloop-sdk/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
  • packages/opentelemetry-instrumentation-langchain/tests/test_context_token_lifecycle.py
💤 Files with no reviewable changes (1)
  • packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/opentelemetry-instrumentation-langchain/tests/test_context_token_lifecycle.py

📝 Walkthrough

Walkthrough

The PR removes an orphaned context_api.attach() in TraceloopCallbackHandler.on_chain_end that previously set SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY without detaching. It adds two regression tests ensuring the suppression context key is not left attached or accumulated across root chains.

Changes

Context Stack Corruption Fix

Layer / File(s) Summary
Context Attachment Removal
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py
Removed top-level-only context_api.attach(...) that set SUPPRESS_LANGUAGE_MODEL_INSTRUMENTATION_KEY=False without detaching; span end/status/output emission remains unchanged.
Regression tests for context token lifecycle
packages/opentelemetry-instrumentation-langchain/tests/test_context_token_lifecycle.py
Adds test_on_chain_end_does_not_leak_context_frame and test_on_chain_end_context_stack_does_not_accumulate to assert the suppression key remains absent (None) after root chain completion and does not accumulate across repeated root chains.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • galzilber
  • nina-kollman
  • max-deygin-traceloop
  • netanel-tl
  • doronkopit5

Poem

🐰 A stray attach I spied one day,
I nudged it gently far away,
Chains finish, spans sleep tight,
The context stack stays right,
Hops of traces homeward sway.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 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 main change: removing an orphaned context_api.attach() call from on_chain_end to fix context stack corruption.
Linked Issues check ✅ Passed The PR directly addresses issue #3526 by removing the orphaned context_api.attach() in on_chain_end that was leaving orphaned contexts on the OpenTelemetry stack, preventing corruption of context and restoring correct active-span behavior.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to fixing the on_chain_end orphaned attach issue. Test changes focus on validating the fix and updating existing tests to match the revised code path, both directly related to the stated objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dr/fix(langchain)-remove-orphaned-context_api.attach()-in-on_chain_end

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

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

Copy link
Copy Markdown
Member

@doronkopit5 doronkopit5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can yuu add some reproduce test for this, a test that fails without your code

@dvirski
Copy link
Copy Markdown
Contributor Author

dvirski commented May 14, 2026

recheck

@dvirski dvirski force-pushed the dr/fix(langchain)-remove-orphaned-context_api.attach()-in-on_chain_end branch from 11b718c to 226d5dd Compare May 14, 2026 11:52
@dvirski dvirski merged commit 4e3e25d into main May 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: Orphaned context_api.attach() Calls Corrupt OpenTelemetry Context Stack

3 participants