Skip to content

fix(instrumentation): preserve FastMCP client-only imports - #4611

Open
wangjiawei-vegetable wants to merge 3 commits into
traceloop:mainfrom
wangjiawei-vegetable:fix/fastmcp-client-only-imports
Open

wangjiawei-vegetable wants to merge 3 commits into
traceloop:mainfrom
wangjiawei-vegetable:fix/fastmcp-client-only-imports

Conversation

@wangjiawei-vegetable

@wangjiawei-vegetable wangjiawei-vegetable commented Oct 12, 2026 •

Copy link
Copy Markdown

Enabling McpInstrumentor breaks OAuth and Client imports when only fastmcp-slim[client]==3.4.8 is installed. The constructor hook runs as soon as the top-level fastmcp package loads and resolves its lazy FastMCP attribute, pulling in optional server dependencies and raising FastMCP server support is not installed.

Fixes #4612

Register the constructor hook on fastmcp.server.server instead. Server-name capture still works when the server module is imported before or after instrumentation, while client-only imports keep the server unloaded and retain client instrumentation.

Minimal reproduction with the published package:

uv venv --python 3.12
uv pip install 'opentelemetry-instrumentation-mcp==0.62.4' 'fastmcp-slim[client]==3.4.8' 'mcp<2'
.venv/bin/python -c 'from opentelemetry.instrumentation.mcp import McpInstrumentor; McpInstrumentor().instrument(); from fastmcp.client.auth import OAuth; from fastmcp import Client'

Validation:

  • Before the fix, both client import-order tests fail with the missing uncalled_for / server-support error.
  • After the fix, client-only import tests pass and verify that server modules remain unloaded and client methods remain instrumented.
  • Full-install tests cover both import orders, the public and direct server imports, and positional/keyword server names.
  • The existing package suite passes with locked FastMCP 2.14.3: 31 passed, 2 skipped (the two split-distribution cases).
  • The six local import-test combinations pass on Python 3.10, 3.11, and 3.12: client-only 2 passed, 4 skipped per version; full FastMCP 3.4.8 6 passed per version. A dedicated CI matrix runs the same combinations.
  • Ruff and actionlint pass; sdist/wheel build succeeds; the built wheel passes the client-only import reproduction.

Scope of the FastMCP 3.4.8 checks: the four existing server integration tests fail with both the upstream constructor hook and this change (resource reads, server spans, and workflow attributes). These failures also occur before this fix. The 3.4.8 matrix here validates imports and constructor name capture; the package suite validates existing server tracing with the repository's FastMCP 2.14.3 test dependency.

  • I have added tests that cover my changes.
  • Observability screenshots: not applicable; this changes import behavior and preserves existing span output, verified by the package tests.
  • PR name follows conventional commits format.
  • Documentation changes: not applicable; no public API changes.

Summary by CodeRabbit

  • Bug Fixes

    • Improved FastMCP server instrumentation so server names are captured with the .mcp suffix regardless of import order or import path.
    • Verified that client instrumentation supports both import orders without loading server components.
  • Tests

    • Added automated compatibility checks across Python 3.10–3.12 and supported FastMCP distributions.

@CLAassistant

CLAassistant commented Oct 12, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

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: 2d42b2d4-c60f-45de-8b8c-0b2d849a26df
Commits

Reviewing files that changed from the base of the PR and between a6e2ac1 and e92e1cd.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

FastMCP server initialization instrumentation now targets the server module. Subprocess tests cover client and server import orders. A GitHub Actions workflow runs these tests across Python 3.10–3.12 and two FastMCP distributions.

Changes: FastMCP import instrumentation

Layer / File(s) Summary
Server hook and import-order validation
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_imports.py, .github/workflows/mcp-imports.yml
The server initialization hook now targets fastmcp.server.server. Subprocess tests check client import orders and server-name capture across import paths. The workflow runs these tests across Python 3.10–3.12 with fastmcp-slim[client]==3.4.8 and fastmcp==3.4.8.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · previous review a6e2a

No actionable merge-blocking risk remains; the configured checks cover client-only imports and server-name capture.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 477f7

The change avoids loading optional server support during client-only imports. No new privilege, authentication bypass, or data exposure was identified. Failure recovery and repeated or concurrent instrumentation remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed hook affects applications enabling MCP instrumentation and importing FastMCP server support within their Python process. Its target is a fixed module and constructor name, not an attacker-selected execution target or a new network entrypoint.

Trust Boundaries and Controls

  • observed — Tool arguments and results continue to flow into telemetry through the existing conditional capture paths. The PR does not change those controls, the delegated tool operation, or its error-recording path.

Resilience and Maintainability Implications

  • observed — The test helper launches fresh Python processes with a 30-second timeout, preventing cached modules and installed import hooks from leaking between import-order cases. Its scripts come from the test definitions rather than a production request source.
Pre-merge checks: Passed 5

Passed 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 concisely describes the main change: preserving FastMCP client-only imports while instrumentation is enabled.
Linked Issues check Passed The change satisfies the coding requirements in [#4612]. fastmcp_instrumentation.py registers the FastMCP.__init__ hook on fastmcp.server.server, so instrumentation does not resolve the top-leve…
Out of Scope Changes check Passed The changes stay within [#4612]. The workflow runs the new import regression tests across supported Python versions and split/full FastMCP distributions. The test additions provide regression coverage…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
Finishing Touches

🧪 Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
ℹ️ Recent review info

⚙️ Run configuration

  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 76f48dce-5239-4279-a053-997c17eaa3d1
📥 Commits

Reviewing files that changed from the base of the PR and between 477f748 and a6e2ac1.

📒 Files selected for processing (1)

  • packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_imports.py
🚧 Files skipped from review as they are similar to previous changes (1)

  • packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_imports.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.

@charan-rathore charan-rathore left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ran this branch locally against the issue repro. On main, instrumenting with only fastmcp-slim[client] installed raises "ImportError: FastMCP server support is not installed" as soon as the client import happens. On this branch the new import tests pass (2 passed, 4 server-variant skips in a slim-only env) and no fastmcp.server modules get loaded. Moving the post-import hook to fastmcp.server.server keeps server-name capture intact for full installs in both import orders, and uninstrument is untouched (it never unwrapped the post-import hook anyway). Nice, focused fix.

@wangjiawei-vegetable
wangjiawei-vegetable force-pushed the fix/fastmcp-client-only-imports branch from a6e2ac1 to e92e1cd Compare November 13, 2026 09:04
@wangjiawei-vegetable

Copy link
Copy Markdown
Author

Hi @doronkopit5, a gentle follow-up on this PR fixing FastMCP client-only imports. Could you take a look when you have a chance? Happy to address any review feedback. Thanks!

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: MCP instrumentation breaks fastmcp-slim client-only imports

3 participants