Skip to content

[DNM] OTEL: Instrument all MCP methods#3890

Draft
strawgate wants to merge 4 commits intoPrefectHQ:mainfrom
strawgate:strawgate/otel-span-coverage
Draft

[DNM] OTEL: Instrument all MCP methods#3890
strawgate wants to merge 4 commits intoPrefectHQ:mainfrom
strawgate:strawgate/otel-span-coverage

Conversation

@strawgate
Copy link
Copy Markdown
Collaborator

Summary

Adds OpenTelemetry span coverage for MCP list operations and enriches delegate spans with method names. Refs #3887.

  • Server spans for list operations: Wraps list_tools, list_resources, list_resource_templates, and list_prompts in server_span() in src/fastmcp/server/server.py
  • Client spans for list operations: Wraps list_tools_mcp, list_resources_mcp, list_resource_templates_mcp, and list_prompts_mcp in client_span() in client mixins
  • mcp.method.name on delegate spans: Adds optional method parameter to delegate_span() in src/fastmcp/server/telemetry.py and propagates the MCP method name from all callers in src/fastmcp/server/providers/fastmcp_provider.py
  • Tests: New test files for server list spans, client list spans, and delegate span method attributes

Test plan

  • uv run ruff check src/ passes
  • uv run pytest tests/server/telemetry tests/client/telemetry -x -- all 46 tests pass (18 new + 28 existing)

🤖 Generated with Claude Code

- Add server_span wrappers to list_tools, list_resources,
  list_resource_templates, and list_prompts in FastMCP server
- Add client_span wrappers to list_tools_mcp, list_resources_mcp,
  list_resource_templates_mcp, and list_prompts_mcp in client mixins
- Add optional `method` parameter to delegate_span in telemetry.py
  and set mcp.method.name on delegate spans
- Update all delegate_span callers in fastmcp_provider.py to pass
  the MCP method name (tools/call, resources/read, prompts/get)
- Add tests for new server list spans, client list spans, and
  delegate span method attributes

Refs: PrefectHQ#3887

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marvin-context-protocol marvin-context-protocol bot added DON'T MERGE PR is not ready for merging. Used by authors to prevent premature merging. enhancement Improvement to existing functionality. For issues and smaller PR improvements. server Related to FastMCP server implementation or server-side functionality. client Related to the FastMCP client SDK or client-side functionality. labels Apr 13, 2026
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

marvin-context-protocol bot commented Apr 13, 2026

Test Failure Analysis

Summary: The ruff format check failed because 2 files in this PR have formatting that does not match ruff's style.

Root Cause: Two files were reformatted by ruff format during the CI run, indicating they were not formatted before pushing:

  • src/fastmcp/server/server.py: Two server_span(...) calls are written across 3 lines each but ruff collapses them to single lines (at lines ~621 and ~1026).
  • tests/client/telemetry/test_client_list_tracing.py: A list comprehension condition split across 2 lines needs to be joined onto one line (around line 70).

Suggested Solution: Run ruff format locally and commit the result:

uv run ruff format src/fastmcp/server/server.py tests/client/telemetry/test_client_list_tracing.py

Or run the full pre-commit suite to catch all issues at once:

uv run prek run --all-files

Then commit and push the reformatted files.

Detailed Analysis

The ruff format hook failed with:

ruff format.............................................Failed
- hook id: ruff-format
- files were modified by this hook
  2 files reformatted, 731 files left unchanged

Diff applied by ruff in src/fastmcp/server/server.py:

@@ -621,9 +621,7 @@
             # Core logic: list tools
-            with server_span(
-                "tools/list", "tools/list", self.name, "tool", ""
-            ):
+            with server_span("tools/list", "tools/list", self.name, "tool", ""):

@@ -1028,9 +1026,7 @@
             # Core logic: list prompts
-            with server_span(
-                "prompts/list", "prompts/list", self.name, "prompt", ""
-            ):
+            with server_span("prompts/list", "prompts/list", self.name, "prompt", ""):

Diff applied by ruff in tests/client/telemetry/test_client_list_tracing.py:

@@ -70,8 +70,7 @@
             (
                 s
                 for s in tools_list_spans
-                if s.attributes is not None
-                and "fastmcp.server.name" in s.attributes
+                if s.attributes is not None and "fastmcp.server.name" in s.attributes
             ),

(Comment edited to reflect latest analysis from workflow run 24329066788)

Related Files
  • src/fastmcp/server/server.py — Two server_span() calls need to be collapsed to single lines
  • tests/client/telemetry/test_client_list_tracing.py — List comprehension condition needs to be joined to one line

Move server_span() inside the else branch (after middleware check) for
list_tools, list_resources, list_resource_templates, and list_prompts,
matching the existing pattern in call_tool/read_resource/render_prompt.
Fix 2-space indentation to 4-space to match the rest of the file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
strawgate and others added 2 commits April 13, 2026 01:29
Wrap attribute-setting blocks in server_span, delegate_span, and
client_span with `if span.is_recording():` to avoid unnecessary work
on non-recording spans. Add error.type attribute using
`type(e).__qualname__` for proper exception class identification.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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. DON'T MERGE PR is not ready for merging. Used by authors to prevent premature merging. 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