fix(mcp): restore uninstrumentation lifecycle by harshb16 · Pull Request #4654 · traceloop/openllmetry

harshb16 · 2026-10-22T20:14:54Z

Closes #4653.

After McpInstrumentor().uninstrument(), completed MCP calls continued exporting spans. Re-enabling added another tracing layer even though each tool executed only once. The issue's in-memory reproducer now returns to the original tracing behavior:

Phase	Before: total / tool spans	After: total / tool spans
Enabled	6 / 2	6 / 2
Disabled	6 / 2	0 / 0
Re-enabled	11 / 4	6 / 2

The two baseline tool spans are the legitimate client and server spans.

Changes

Track all MCP and FastMCP patches and restore owned layers during teardown. Deferred imports stay inert while disabled and use the current callback after re-enabling.
Preserve patch ownership across repeated singleton construction. Copied/bound references bypass tracing while disabled and resume the current callback when the underlying target is unchanged.
Preserve other wrappers and original callable behavior when targets are decorated, replaced, or deleted.
Add regressions for repeated cycles, existing/fresh clients, import timing, tracer replacement, all twelve patched targets, and wrapper interoperability.

Scope is fully completed client sessions. Toggling instrumentation during an active session remains unsupported.

Validation

62 MCP package tests passed with FastMCP 2.12.2 and 2.14.3, using Python 3.11, MCP 1.26.0, OpenTelemetry SDK 1.42.1, and instrumentation 0.63b1.
The trimmed regressions still fail against the original production code and detect temporarily reintroduced lifecycle/retargeting defects.
Ruff, whitespace checks, and wheel build passed. The wheel includes the new helper.
Original issue reproducer passed; tool execution count and result remain unchanged.
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.

Summary by CodeRabbit

New Features
- MCP tracing can now be stopped and restarted, including with a different tracer provider, without producing duplicate spans.
Bug Fixes
- Stopping MCP tracing now removes its instrumentation cleanly. Calls made while tracing is stopped do not generate spans, and restarting tracing restores span collection.
Documentation
- Clarified that active client sessions must be finished before stopping MCP tracing. Switching instrumentation while a session remains open is not supported.

Track patches and deferred imports so disabling MCP stops tracing and re-enabling restores one layer with the current tracer. Preserve copied callables and wrappers installed by other code. Closes traceloop#4653

coderabbitai · 2026-10-22T20:15:11Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: c5cbb2c3-2880-4235-982a-17c23b387088

📥 Commits

Reviewing files that changed from the base of the PR and between 8d9c93f and e8c435e.

📒 Files selected for processing: 7

packages/opentelemetry-instrumentation-mcp/README.md
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/_wrapping.py
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py
packages/opentelemetry-instrumentation-mcp/tests/conftest.py
packages/opentelemetry-instrumentation-mcp/tests/test_instrumentation_lifecycle.py
packages/opentelemetry-instrumentation-mcp/tests/test_wrapping.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The MCP instrumentors now use a shared helper to manage method wrappers. Uninstrumentation disables and removes wrappers. Tests cover wrapper behavior and repeated instrumentation cycles, and the README documents session requirements for stopping and restarting tracing.

Changes: MCP instrumentation lifecycle

Layer / File(s)	Summary
Method patch management and wrapper behavior `packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/_wrapping.py`, `packages/opentelemetry-instrumentation-mcp/tests/test_wrapping.py`	The shared helper tracks wrappers, handles deferred imports and changed targets, and disables or removes patches during teardown. Tests cover wrapper chains, cached references, rewrapping, and cleanup.
Instrumentor lifecycle integration `packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/instrumentation.py`, `packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py`, `packages/opentelemetry-instrumentation-mcp/tests/test_instrumentation_lifecycle.py`, `packages/opentelemetry-instrumentation-mcp/tests/conftest.py`, `packages/opentelemetry-instrumentation-mcp/README.md`	Both instrumentors register and remove wrappers through the shared helper. Lifecycle tests cover import timing, repeated instrumentation cycles, span counts, and method restoration. The README documents session requirements for stopping and restarting tracing.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant McpInstrumentor
  participant InstrumentedMethods
  participant MCPMethods
  participant TracerProvider
  McpInstrumentor->>InstrumentedMethods: register method wrappers
  InstrumentedMethods->>MCPMethods: install wrappers when targets resolve
  MCPMethods->>TracerProvider: export spans while instrumentation is active
  McpInstrumentor->>InstrumentedMethods: disable and remove wrappers

Suggested reviewers: fali007, nirga

Merge Risk: ⚪ Minimal · up to e8c43

The completed-session enable, disable, and re-enable behavior has no identified merge-blocking issue. Mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 6 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix(mcp): restore uninstrumentation lifecycle" matches the main change. The PR fixes MCP spans that kept exporting after uninstrument(), and fixes duplicate tracing after re-enabling. It ad…
Linked Issues check		Issue `#4653` requires disabled completed MCP calls to export no MCP spans, re-enabling to restore one tracing layer, and application behavior to remain unchanged. The lifecycle regression covers three …
Out of Scope Changes check		The changes stay within issue `#4653`. The patch helper, wrapper tests, lifecycle tests, fixture comment, and README describe or verify MCP patch ownership, teardown, retargeting, deferred imports, and …

Full details: Title check

Explanation

The title "fix(mcp): restore uninstrumentation lifecycle" matches the main change. The PR fixes MCP spans that kept exporting after uninstrument(), and fixes duplicate tracing after re-enabling. It adds tracking of patches for teardown and deferred imports, so that disabling stops tracing and re-enabling restores a single layer. The title is short, uses a conventional commit prefix, and names the scope (mcp) and the lifecycle area that changed.

Full details: Linked Issues check

Explanation

Issue #4653 requires disabled completed MCP calls to export no MCP spans, re-enabling to restore one tracing layer, and application behavior to remain unchanged. The lifecycle regression covers three enable/disable cycles, fresh and persistent clients, deferred imports, provider replacement, tool result 8, one execution per call, zero spans while disabled, stable span counts while enabled, and restoration of original methods. _InstrumentedMethods removes owned patches and updates the active callback, which addresses duplicate tracing and stale callbacks. The implementation matches the issue scope of completed sessions.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #4653. The patch helper, wrapper tests, lifecycle tests, fixture comment, and README describe or verify MCP patch ownership, teardown, retargeting, deferred imports, and repeated instrumentation. These changes support the lifecycle fix. No unrelated product behavior or unrelated package changes are shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 6 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

CLAassistant · 2026-10-22T20:30:55Z

All committers have signed the CLA.

fix(mcp): restore uninstrumentation lifecycle

e8c435e

Track patches and deferred imports so disabling MCP stops tracing and re-enabling restores one layer with the current tracer. Preserve copied callables and wrappers installed by other code. Closes traceloop#4653

Navigation Menu

Repository navigation

fix(mcp): restore uninstrumentation lifecycle - #4654

fix(mcp): restore uninstrumentation lifecycle#4654
harshb16 wants to merge 1 commit into
traceloop:mainfrom
harshb16:fix-mcp-uninstrument-4653

harshb16 commented Oct 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 22, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

CLAassistant commented Oct 22, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

harshb16 commented Oct 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

CLAassistant commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

harshb16 commented Oct 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 22, 2026 •

edited

Loading

CLAassistant commented Oct 22, 2026 •

edited

Loading