Skip to content

feat: Add telemetry interop mode for FastMCP#4046

Open
strawgate wants to merge 4 commits into
mainfrom
codex/otel-interop-v2
Open

feat: Add telemetry interop mode for FastMCP#4046
strawgate wants to merge 4 commits into
mainfrom
codex/otel-interop-v2

Conversation

@strawgate
Copy link
Copy Markdown
Collaborator

@strawgate strawgate commented Apr 26, 2026

When another instrumentation layer (e.g. an MCP-aware OTEL library or a service mesh) already owns the MCP span hierarchy, FastMCP's native spans become duplicates. This PR adds a minimal interop mode that lets users suppress FastMCP's own spans while keeping trace context propagation through _meta fully functional.

import fastmcp

# Global: suppress all FastMCP spans
fastmcp.settings.telemetry_mode = "propagation_only"

# Or per-scope: suppress only within a block
from fastmcp.telemetry import suppress_fastmcp_telemetry

with suppress_fastmcp_telemetry():
    result = await client.call_tool("search", {"query": "otel"})

The setting (FASTMCP_TELEMETRY_MODE) controls all three span helpers (server_span, client_span, delegate_span). In propagation_only mode, incoming trace context from _meta is still extracted and attached so downstream user spans inherit the correct parent—only FastMCP's own MCP operation spans are skipped.

This follows the universal pattern seen in FastAPI, Django, Flask, Sentry, and gRPC instrumentation libraries: extract→attach→skip span→detach.

@marvin-context-protocol marvin-context-protocol Bot added enhancement Improvement to existing functionality. For issues and smaller PR improvements. client Related to the FastMCP client SDK or client-side functionality. server Related to FastMCP server implementation or server-side functionality. labels Apr 26, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@strawgate strawgate force-pushed the codex/otel-interop-v2 branch from 0d4639f to 79382bf Compare April 26, 2026 00:32
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

marvin-context-protocol Bot commented Apr 26, 2026

tl;dr: A pre-existing test, test_refresh_expires_in_zero_issues_refresh_token, is now failing because the PR's change to proxy.py sets refresh_expires_in = None for the raw_value == 0 branch, which causes downstream gating logic to silently skip issuing the refresh token. Fix by using the fallback expiry instead of None in that branch.


(Edited to reflect latest CI failure — this replaces the earlier ruff format report.)

Root Cause: In src/fastmcp/server/auth/oauth_proxy/proxy.py, the new raw_value == 0 branch sets refresh_expires_in = None (to represent "never expires"). But two downstream checks gate refresh token issuance and storage on refresh_expires_in being truthy:

# line 1128 — None is falsy → fastmcp_refresh_token stays None
if refresh_jti and refresh_expires_in:
    fastmcp_refresh_token = self.jwt_issuer.issue_refresh_token(...)

# line 1159 — fastmcp_refresh_token is None → no metadata stored
if fastmcp_refresh_token and refresh_expires_in:
    await self._refresh_token_store.put(...)

result.refresh_token ends up None, failing the assertion at test_tokens.py:874.

Fix: In the raw_value == 0 branch of the new code, use the configured fallback instead of None. This matches the test's own docstring: "refresh_expires_in=0 should fall back to the configured default."

# Before (PR's new code — broken):
if raw_value == 0:
    refresh_expires_in = None
    refresh_token_expires_at = None

# After (correct):
if raw_value == 0:
    refresh_expires_in = self._fallback_refresh_token_expiry_seconds
    refresh_token_expires_at = time.time() + refresh_expires_in
Log excerpt
FAILED tests/server/auth/oauth_proxy/test_tokens.py::TestUpstreamTokenStorageTTL::test_refresh_expires_in_zero_issues_refresh_token
AssertionError: assert None is not None
 +  where None = OAuthToken(access_token='***', token_type='Bearer', expires_in=3600, scope='read write', refresh_token=None).refresh_token

Reproduced across all failing matrix jobs (Python 3.10 / 3.13 on ubuntu and windows, plus lowest-direct-deps).

Related files
  • src/fastmcp/server/auth/oauth_proxy/proxy.py — lines 1064–1085 (new refresh_expires_in=0 branch), 1128, 1159
  • tests/server/auth/oauth_proxy/test_tokens.pyTestUpstreamTokenStorageTTL::test_refresh_expires_in_zero_issues_refresh_token (line 825)

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@strawgate strawgate added the DON'T MERGE PR is not ready for merging. Used by authors to prevent premature merging. label Apr 26, 2026
@strawgate strawgate force-pushed the codex/otel-interop-v2 branch from fe9b3fb to 118a5a6 Compare May 13, 2026 03:42
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d90859a4d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fastmcp_slim/fastmcp/server/telemetry.py
strawgate added a commit that referenced this pull request May 17, 2026
…4046)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 <[email protected]>
strawgate and others added 4 commits May 18, 2026 22:12
Add propagation_only telemetry mode and suppress_fastmcp_telemetry()
context manager. When active, FastMCP skips its own MCP spans but
still propagates trace context through _meta so an external
instrumentation layer can own the span hierarchy.

Scope reduced from ~370 lines to ~130 lines by removing:
- Client-side _meta injection on list_* calls
- Custom _initialize_session_with_meta() using private SDK APIs
- Ambient span context ASGI scope plumbing
- Complex Link-based parent context resolution

Co-authored-by: Copilot <[email protected]>
🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 <[email protected]>
…4046)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 <[email protected]>
…ion marker

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@strawgate strawgate force-pushed the codex/otel-interop-v2 branch from ac2e964 to 289150f Compare May 19, 2026 03:13
@strawgate strawgate removed the DON'T MERGE PR is not ready for merging. Used by authors to prevent premature merging. label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Related to the FastMCP client SDK or client-side functionality. enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant