Conversation
Test Failure AnalysisSummary: Test times out on Windows when logging output triggers a lazy import of Rich library's Windows renderer module. Root Cause: On Windows, when Rich's logging handler attempts to output debug logs, it lazily imports The issue is specific to Windows and occurs when:
Suggested Solution: Pre-import Specific changes needed: # tests/conftest.py, lines 30-35
@pytest.fixture(autouse=True)
def import_rich_rule():
# What a hack
import rich.rule # noqa: F401
# Pre-import Windows renderer to avoid lazy import timeout with beartype on Windows
if sys.platform == "win32":
try:
import rich._windows_renderer # noqa: F401
except ImportError:
pass # Module might not exist in all Rich versions
yieldThis will ensure the problematic import happens during fixture setup (where it doesn't timeout) rather than during test execution. Detailed AnalysisStack Trace Analysis: The timeout occurs at this call stack: Why the existing workaround helps: The comment "What a hack" at line 32 of conftest.py acknowledges that pre-importing Rich modules works around similar issues. This PR adds more logging (specifically the debug logs at oauth_proxy.py:1742-1749), which triggers more Rich console operations and exposes this Windows-specific race condition. Why this only affects Windows: The Related Issues:
Related Files
|
Test Failure AnalysisSummary: Test Root Cause: On Windows, when Rich's logging handler attempts to output debug logs, it lazily imports The issue is specific to Windows and occurs when:
Suggested Solution: Pre-import Specific changes needed: # tests/conftest.py, lines 30-35
@pytest.fixture(autouse=True)
def import_rich_rule():
# What a hack
import rich.rule # noqa: F401
# Pre-import Windows renderer to avoid lazy import timeout with beartype on Windows
if sys.platform == "win32":
try:
import rich._windows_renderer # noqa: F401
except ImportError:
pass # Module might not exist in all Rich versions
yieldThis will ensure the problematic import happens during fixture setup (where it doesn't timeout) rather than during test execution. Detailed AnalysisStack Trace Analysis: The timeout occurs at this call stack: Why the existing workaround helps: The comment "What a hack" at line 32 of conftest.py acknowledges that pre-importing Rich modules works around similar issues. This PR adds more logging (specifically the debug logs at oauth_proxy.py:1742-1749), which triggers more Rich console operations and exposes this Windows-specific race condition. Why this only affects Windows: The Related Issues:
Related Files
|
| icon: chart-line | ||
| --- | ||
|
|
||
| FastMCP includes native OpenTelemetry instrumentation for observability. Traces are automatically generated for all MCP operations, providing visibility into server behavior, request handling, and provider delegation chains. |
There was a problem hiding this comment.
I'd say for all tool, prompt, resource, and resource template operations. We're deferring the other lower-level stuff to the mcp lower-level SDK
|
|
||
| ## How It Works | ||
|
|
||
| FastMCP uses the OpenTelemetry API (not the SDK) for instrumentation. This means: |
There was a problem hiding this comment.
No need to say "not the SDK" in this
| @@ -0,0 +1 @@ | |||
| """FastMCP Diagnostics example - for testing tracing, errors, and observability.""" | |||
There was a problem hiding this comment.
I don't think we need this file, this isn't used as a package
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request introduces native OpenTelemetry instrumentation throughout FastMCP. It adds telemetry context managers and span creation utilities at the core infrastructure level ( Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/diagnostics/__init__.py (1)
1-1: Unnecessary__init__.pyfor examples directory.As noted in previous comments, this file can be removed since the examples directory isn't used as an importable package.
🧹 Nitpick comments (8)
examples/run_with_tracing.py (2)
22-25: Add return type annotation tomainfunction.Per coding guidelines, Python ≥3.10 code requires full type annotations.
Suggested fix
-def main(): +def main() -> None:
54-54: Use spread operator for cleaner list construction.As flagged by static analysis (Ruff RUF005), the spread operator is more idiomatic.
Suggested fix
- sys.argv = ["fastmcp", "run"] + sys.argv[1:] + sys.argv = ["fastmcp", "run", *sys.argv[1:]]docs/servers/telemetry.mdx (2)
291-301: Add return type annotation to test helper function.Per coding guidelines, all Python code examples should include full type annotations.
Suggested fix
-async def test_tool_creates_span(trace_exporter): +async def test_tool_creates_span(trace_exporter: InMemorySpanExporter) -> None: mcp = FastMCP("test") `@mcp.tool`() def hello() -> str: return "world"
269-289: Testing section provides practical guidance.The in-memory exporter fixture pattern is a good practice for testing telemetry without external dependencies. Consider adding
trace.set_tracer_provider(TracerProvider())in teardown to reset to a no-op provider, preventing state leakage between tests.Enhanced fixture with cleanup
`@pytest.fixture` def trace_exporter(): exporter = InMemorySpanExporter() provider = TracerProvider() provider.add_span_processor(SimpleSpanProcessor(exporter)) + original_provider = trace.get_tracer_provider() trace.set_tracer_provider(provider) yield exporter exporter.clear() + trace.set_tracer_provider(original_provider)examples/diagnostics/server.py (2)
46-52: Synchronous blocking call in async context manager.The readiness check uses synchronous
httpx.get()andtime.sleep()inside an async context manager. This blocks the event loop. Consider usinghttpx.AsyncClientandasyncio.sleep().Proposed async readiness check
+import asyncio + # Wait for server to be ready -for _ in range(50): - try: - httpx.get(f"http://localhost:{ECHO_SERVER_PORT}/sse", timeout=0.1) - break - except Exception: - time.sleep(0.1) +async with httpx.AsyncClient() as http_client: + for _ in range(50): + try: + await http_client.get(f"http://localhost:{ECHO_SERVER_PORT}/sse", timeout=0.1) + break + except Exception: + await asyncio.sleep(0.1)
58-62: Subprocess output not captured or logged on termination failure.If the subprocess fails to terminate within the timeout, the exception propagates without any diagnostics. Consider logging
proc.stdout/proc.stderron failure for debugging.Suggested improvement
try: yield finally: - proc.terminate() - proc.wait(timeout=5) + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + stdout, stderr = proc.communicate() + print(f"Echo server did not terminate gracefully. stderr: {stderr.decode()}")src/fastmcp/server/providers/proxy.py (2)
116-154: Synchronous context manager wrapping async code.
client_spanis a synchronous context manager (from the external snippet showing@contextmanager), but the code inside containsawaitstatements. This works but the span will remain open during I/O suspension. This is expected behavior for tracing async operations, but ensureclient_spandoesn't acquire any non-reentrant locks.Additionally, the
contextparameter at line 113 is shadowed at line 123 bycontext = get_context(). The original parameter is unused.Consider removing unused parameter shadowing
async def run( self, arguments: dict[str, Any], - context: Context | None = None, + context: Context | None = None, # noqa: ARG002 - required by interface ) -> ToolResult:Or if the interface doesn't require it:
async def run( self, arguments: dict[str, Any], - context: Context | None = None, ) -> ToolResult:
336-358: Code duplication: resource content processing repeated.The content processing loop (lines 336-358) is nearly identical to the one in
ProxyResource.read(lines 237-257). Consider extracting a helper function.Suggested helper extraction
def _process_resource_contents( result: Sequence[TextResourceContents | BlobResourceContents], ) -> list[ResourceContent]: """Convert MCP resource contents to FastMCP ResourceContent.""" contents: list[ResourceContent] = [] for item in result: if isinstance(item, TextResourceContents): contents.append( ResourceContent( content=item.text, mime_type=item.mimeType, meta=item.meta, ) ) elif isinstance(item, BlobResourceContents): contents.append( ResourceContent( content=base64.b64decode(item.blob), mime_type=item.mimeType, meta=item.meta, ) ) else: raise ResourceError(f"Unsupported content type: {type(item)}") return contentsThen use it in both
ProxyResource.readandProxyTemplate.create_resource.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
loq.tomlis excluded by none and included by nonepyproject.tomlis excluded by none and included by nonetests/client/telemetry/__init__.pyis excluded by none and included by nonetests/client/telemetry/test_client_tracing.pyis excluded by none and included by nonetests/conftest.pyis excluded by none and included by nonetests/server/telemetry/__init__.pyis excluded by none and included by nonetests/server/telemetry/test_provider_tracing.pyis excluded by none and included by nonetests/server/telemetry/test_server_tracing.pyis excluded by none and included by nonetests/telemetry/__init__.pyis excluded by none and included by nonetests/telemetry/test_module.pyis excluded by none and included by nonetests/test_mcp_config.pyis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (19)
docs/docs.jsondocs/servers/telemetry.mdxexamples/diagnostics/__init__.pyexamples/diagnostics/client_with_tracing.pyexamples/diagnostics/server.pyexamples/run_with_tracing.pysrc/fastmcp/client/client.pysrc/fastmcp/client/telemetry.pysrc/fastmcp/client/transports.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/resources/resource.pysrc/fastmcp/resources/template.pysrc/fastmcp/server/providers/fastmcp_provider.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/server.pysrc/fastmcp/server/telemetry.pysrc/fastmcp/telemetry.pysrc/fastmcp/tools/tool.pysrc/fastmcp/utilities/components.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python ≥3.10 with full type annotations required for all code
Never use bareexcept- be specific with exception types in Python code
Files:
src/fastmcp/utilities/components.pysrc/fastmcp/client/transports.pyexamples/diagnostics/client_with_tracing.pysrc/fastmcp/resources/template.pysrc/fastmcp/client/telemetry.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/server/telemetry.pysrc/fastmcp/tools/tool.pysrc/fastmcp/client/client.pysrc/fastmcp/resources/resource.pysrc/fastmcp/telemetry.pyexamples/run_with_tracing.pyexamples/diagnostics/__init__.pysrc/fastmcp/server/providers/fastmcp_provider.pyexamples/diagnostics/server.pysrc/fastmcp/server/server.py
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/telemetry.mdx
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
**/__init__.py: Be intentional about module re-exports - only re-export fundamental types to fastmcp.*; prefer users importing from specific submodules
Core types that define a module's purpose should be exported; specialized features can live in submodules
Files:
examples/diagnostics/__init__.py
🧠 Learnings (6)
📚 Learning: 2026-01-12T16:24:55.006Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:55.006Z
Learning: Maintain consistency across all four MCP object types (Tools, Resources, Resource Templates, and Prompts) when implementing similar features
Applied to files:
docs/servers/telemetry.mdx
📚 Learning: 2026-01-12T16:24:55.006Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:55.006Z
Learning: Applies to src/tools/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Tools (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
docs/servers/telemetry.mdx
📚 Learning: 2026-01-12T16:24:55.006Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:55.006Z
Learning: Applies to src/prompts/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Prompts (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
docs/servers/telemetry.mdx
📚 Learning: 2026-01-12T16:24:55.006Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:55.006Z
Learning: Applies to src/resources/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Resources (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
docs/servers/telemetry.mdx
📚 Learning: 2026-01-13T03:11:40.917Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T03:11:40.917Z
Learning: Applies to **/__init__.py : Be intentional about module re-exports - only re-export fundamental types to fastmcp.*; prefer users importing from specific submodules
Applied to files:
src/fastmcp/telemetry.pyexamples/diagnostics/__init__.py
📚 Learning: 2025-11-03T17:36:13.363Z
Learnt from: jlowin
Repo: jlowin/fastmcp PR: 2355
File: docs/clients/client.mdx:226-246
Timestamp: 2025-11-03T17:36:13.363Z
Learning: In FastMCP documentation, prefer showing the happy path in onboarding examples without over-explaining edge cases or adding defensive checks, as this reduces cognitive burden for new users learning the API.
Applied to files:
examples/diagnostics/__init__.py
🧬 Code graph analysis (11)
examples/diagnostics/client_with_tracing.py (1)
examples/run_with_tracing.py (1)
main(22-55)
src/fastmcp/resources/template.py (6)
src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/resource.py (1)
get_span_attributes(412-416)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/tools/tool.py (1)
get_span_attributes(389-393)src/fastmcp/utilities/components.py (1)
get_span_attributes(186-191)
src/fastmcp/client/telemetry.py (1)
src/fastmcp/telemetry.py (1)
get_tracer(38-47)
src/fastmcp/server/providers/proxy.py (5)
src/fastmcp/client/telemetry.py (1)
client_span(12-37)src/fastmcp/exceptions.py (2)
ToolError(18-19)ResourceError(14-15)src/fastmcp/resources/resource.py (1)
get_span_attributes(412-416)src/fastmcp/resources/template.py (1)
get_span_attributes(318-322)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)
src/fastmcp/prompts/prompt.py (6)
src/fastmcp/resources/resource.py (1)
get_span_attributes(412-416)src/fastmcp/resources/template.py (1)
get_span_attributes(318-322)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/tools/tool.py (1)
get_span_attributes(389-393)src/fastmcp/utilities/components.py (1)
get_span_attributes(186-191)
src/fastmcp/server/telemetry.py (3)
src/fastmcp/telemetry.py (2)
extract_trace_context(82-111)get_tracer(38-47)src/fastmcp/server/dependencies.py (2)
get_access_token(376-427)get_context(283-290)src/fastmcp/server/context.py (2)
client_id(431-437)request_context(264-290)
src/fastmcp/tools/tool.py (6)
src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/resource.py (1)
get_span_attributes(412-416)src/fastmcp/resources/template.py (1)
get_span_attributes(318-322)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/utilities/components.py (1)
get_span_attributes(186-191)
src/fastmcp/client/client.py (2)
src/fastmcp/client/telemetry.py (1)
client_span(12-37)src/fastmcp/telemetry.py (1)
inject_trace_context(50-73)
src/fastmcp/resources/resource.py (6)
src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/template.py (1)
get_span_attributes(318-322)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/tools/tool.py (1)
get_span_attributes(389-393)src/fastmcp/utilities/components.py (1)
get_span_attributes(186-191)
src/fastmcp/server/providers/fastmcp_provider.py (7)
src/fastmcp/server/telemetry.py (1)
delegate_span(93-116)src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/resource.py (1)
get_span_attributes(412-416)src/fastmcp/resources/template.py (1)
get_span_attributes(318-322)src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/tools/tool.py (1)
get_span_attributes(389-393)src/fastmcp/utilities/components.py (1)
get_span_attributes(186-191)
src/fastmcp/server/server.py (6)
src/fastmcp/server/telemetry.py (1)
server_span(55-89)src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/resource.py (2)
get_span_attributes(412-416)key(381-383)src/fastmcp/resources/template.py (2)
get_span_attributes(318-322)key(285-287)src/fastmcp/tools/tool.py (1)
get_span_attributes(389-393)src/fastmcp/utilities/components.py (2)
get_span_attributes(186-191)key(94-102)
🪛 Ruff (0.14.11)
examples/diagnostics/client_with_tracing.py
96-96: Do not catch blind exception: Exception
(BLE001)
104-104: Do not catch blind exception: Exception
(BLE001)
111-111: Do not catch blind exception: Exception
(BLE001)
120-120: Do not catch blind exception: Exception
(BLE001)
132-132: Do not catch blind exception: Exception
(BLE001)
139-139: Do not catch blind exception: Exception
(BLE001)
145-145: Do not catch blind exception: Exception
(BLE001)
152-152: Do not catch blind exception: Exception
(BLE001)
src/fastmcp/server/providers/proxy.py
232-234: Avoid specifying long messages outside the exception class
(TRY003)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
examples/run_with_tracing.py
54-54: Consider ["fastmcp", "run", *sys.argv[1:]] instead of concatenation
Replace with ["fastmcp", "run", *sys.argv[1:]]
(RUF005)
examples/diagnostics/server.py
30-30: subprocess call: check for execution of untrusted input
(S603)
31-40: Starting a process with a partial executable path
(S607)
51-51: Do not catch blind exception: Exception
(BLE001)
106-106: Avoid specifying long messages outside the exception class
(TRY003)
112-112: Avoid specifying long messages outside the exception class
(TRY003)
118-118: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/server.py
1494-1494: Avoid specifying long messages outside the exception class
(TRY003)
1495-1495: Avoid specifying long messages outside the exception class
(TRY003)
1588-1588: Avoid specifying long messages outside the exception class
(TRY003)
1589-1589: Avoid specifying long messages outside the exception class
(TRY003)
1595-1595: Avoid specifying long messages outside the exception class
(TRY003)
1609-1609: Avoid specifying long messages outside the exception class
(TRY003)
1610-1610: Avoid specifying long messages outside the exception class
(TRY003)
1699-1699: Avoid specifying long messages outside the exception class
(TRY003)
1700-1700: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (41)
src/fastmcp/client/transports.py (2)
125-128: LGTM!Clean extension point for transports to expose session IDs. The default
Nonereturn is appropriate for transports that don't support session identification.
311-317: LGTM!Defensive exception handling is appropriate here since session ID retrieval is optional telemetry metadata - failures shouldn't propagate and break the transport.
src/fastmcp/utilities/components.py (1)
185-191: LGTM!Well-designed extension point for telemetry. The pattern of calling
super()and merging attributes enables consistent span metadata across the component hierarchy.src/fastmcp/resources/resource.py (1)
412-416: LGTM!Consistent implementation following the established telemetry pattern across components. Properly chains to
super()and adds resource-specific span attributes.src/fastmcp/resources/template.py (1)
318-322: LGTM!Consistent telemetry implementation matching the pattern used by other components. The
resource_templatetype clearly distinguishes templates from concrete resources in traces.src/fastmcp/tools/tool.py (1)
389-393: LGTM! Consistent span attributes implementation.The implementation correctly follows the established pattern across other components (Prompt, Resource, ResourceTemplate) by calling
super().get_span_attributes()and merging component-specific attributes using the dict union operator.docs/docs.json (1)
131-131: LGTM! Navigation entry correctly added.The new telemetry documentation page is properly positioned alphabetically within the Features group.
src/fastmcp/prompts/prompt.py (1)
395-399: LGTM! Consistent implementation matching sibling components.The
get_span_attributesmethod follows the same pattern asTool,Resource, andResourceTemplate, ensuring uniform telemetry attributes across all component types. Based on learnings, this maintains consistency across all four MCP object types.examples/run_with_tracing.py (1)
40-45: LGTM with note:insecure=Trueis appropriate for local development.The OTLP exporter configuration with
insecure=Trueis correct for local development scenarios demonstrated in this example. The script's docstring clearly indicates this is for localhost usage withotel-desktop-viewer.docs/servers/telemetry.mdx (5)
1-6: LGTM! Proper frontmatter with required fields.The YAML frontmatter correctly includes the required
titleanddescriptionfields per coding guidelines.
8-17: Clear and accurate introduction.The explanation of OpenTelemetry API vs SDK usage and the "bring your own SDK" approach is well-articulated. The list of supported backends sets appropriate expectations.
56-75: Well-structured server spans documentation.The tables clearly document span names and attributes with appropriate examples. The component types (
tool,resource,resource_template,prompt) align with theget_span_attributesimplementations across the codebase.
223-231: Add type annotation to error handling example.Per coding guidelines, code examples should include proper type annotations.
Suggested fix
`@mcp.tool`() -def risky_operation() -> str: +def risky_operation() -> str: # Already has return annotation, but no param annotations needed here raise ValueError("Something went wrong")Actually, this example is correct - no parameters means no parameter annotations needed, and the return type is present. No change required.
113-130: Clear span hierarchy visualization.The ASCII diagrams effectively illustrate the parent-child relationship between spans for mounted servers and proxy providers. This helps users understand trace propagation across provider boundaries.
src/fastmcp/client/telemetry.py (1)
1-40: LGTM! Clean implementation of client-side telemetry helper.The
client_spancontext manager correctly:
- Creates a CLIENT span with appropriate MCP attributes
- Records exceptions on the span before re-raising
- Conditionally includes session ID when available
The use of bare
Exceptionin Line 34 is acceptable here since the purpose is telemetry recording, and the exception is always re-raised.src/fastmcp/client/client.py (3)
882-917: LGTM! Proper trace context propagation for resource reads.The instrumentation correctly:
- Creates a CLIENT span with resource URI as the component key
- Injects trace context into the meta dict for server-side propagation
- Falls back to the simpler
session.read_resourcewhen no trace context is available
1103-1148: LGTM! Consistent instrumentation pattern for prompt operations.The implementation follows the same well-structured pattern as
read_resource_mcp, with proper span creation and trace context propagation.
1395-1419: LGTM! Tool call instrumentation is well implemented.The implementation correctly wraps the tool call with a CLIENT span and propagates trace context.
Minor observation: Line 1416 could be simplified from
meta=propagated_meta if propagated_meta else Noneto justmeta=propagated_metasinceinject_trace_contextalready returnsNonewhen there's nothing to inject. However, this is a very minor stylistic point and the current code is clear about intent.src/fastmcp/server/server.py (3)
1476-1495: LGTM! Server-side tool call tracing correctly implemented.The span correctly:
- Encompasses the entire tool execution including lookup
- Sets component-specific attributes from
tool.get_span_attributes()- Preserves existing exception handling patterns (FastMCPError, ValidationError, generic Exception)
The
server_spancontext manager will automatically record exceptions and set error status before re-raising.
1570-1610: LGTM! Resource read tracing handles both concrete and template paths.The implementation correctly:
- Wraps the entire read operation in a single span
- Sets appropriate span attributes for whichever path succeeds (concrete resource or template)
- Preserves the existing fallback logic from resources to templates
- Re-raises
NotFoundErrorwith proper context when neither is found
1684-1700: LGTM! Prompt rendering tracing follows the established pattern.The implementation is consistent with tool and resource tracing, correctly wrapping the prompt render operation with appropriate span attributes.
examples/diagnostics/client_with_tracing.py (2)
32-45: LGTM! Standard OpenTelemetry setup for examples.The
setup_tracing()function correctly configures OTLP export with sensible defaults for local development. Theinsecure=Trueparameter is appropriate for localhost development scenarios documented in the usage instructions.
91-153: BareExceptioncatches are acceptable for this diagnostic example.The static analysis flags
except Exceptionat multiple locations. In this context, catching broad exceptions is intentional - the script is designed to exercise all server components and report which ones succeed/fail, continuing execution regardless of individual failures.For production client code, more specific exception handling (e.g.,
ToolError,ResourceError,McpError) would be preferable, but for a diagnostic/example script demonstrating tracing behavior, the current approach is appropriate.src/fastmcp/server/providers/fastmcp_provider.py (4)
115-140: LGTM! Tool delegation tracing correctly implemented.The
delegate_spanwraps_run()appropriately, andget_span_attributes()provides useful debugging context (original name). Therun()method correctly remains unwrapped since it calls_server.call_toolwhich creates its own span.
193-204: LGTM! Resource delegation tracing follows the established pattern.
265-290: LGTM! Prompt delegation tracing correctly mirrors the tool pattern.
372-421: LGTM! Resource template delegation tracing is complete and consistent.The
get_span_attributes()correctly includesoriginal_uri_templateto distinguish template-based reads from concrete resource reads.src/fastmcp/server/telemetry.py (3)
12-26: LGTM - Auth span attributes extraction is well-structured.The function correctly handles the
RuntimeErrorthat can occur when no context is available. The deferred import ofget_access_tokenavoids circular dependencies.
54-89: LGTM - Server span context manager is well-implemented.The span correctly sets RPC semantic conventions, records exceptions, and sets error status. Exception re-raising after recording is correct. Passing
Noneto thecontextparameter when_get_parent_trace_context()returnsNoneis valid as OpenTelemetry'sstart_as_current_spanacceptsNoneand uses the current context as fallback.
92-116: LGTM - Delegate span context manager follows the same pattern.Consistent with
server_span, correctly records exceptions and sets error status. The implicitINTERNALspan kind (default) is appropriate for internal delegation.src/fastmcp/telemetry.py (5)
1-22: LGTM - Well-documented module with clear SDK configuration example.The docstring clearly explains that telemetry is a no-op without an SDK, and provides a practical example for users to configure OpenTelemetry.
38-47: LGTM - Tracer accessor correctly delegates to OpenTelemetry API.The function properly wraps
otel_get_tracerwith the instrumentation name, providing no-op behavior when no SDK is configured.
50-73: LGTM - Trace context injection handles edge cases well.The function correctly returns
Nonewhen there's no trace context to inject and the inputmetawasNone, avoiding unnecessary empty dicts in requests.
76-79: LGTM - Utility for recording span errors.Simple helper that encapsulates the common pattern of recording an exception and setting error status.
82-111: LGTM - Trace context extraction correctly preserves existing spans.The check for an already-valid span context (e.g., from HTTP propagation) before extracting from meta prevents accidental context overwrites. This is the correct behavior for distributed tracing.
examples/diagnostics/server.py (1)
70-118: LGTM - Example components are well-structured for testing observability.The components cover successful operations and intentional failures across tools, resources, templates, and prompts. The
ValueErrorexceptions with descriptive messages are appropriate for this diagnostics/testing use case. The static analysis warnings about long exception messages (TRY003) can be safely ignored here since these are intentional test fixtures.src/fastmcp/server/providers/proxy.py (5)
156-160: LGTM - Span attributes correctly expose proxy metadata.The
get_span_attributesmethod properly merges parent attributes with proxy-specific attributes including backend name.
223-258: LGTM - Resource read with proper span wrapping and content processing.The span correctly wraps the remote resource read, and the content processing handles both text and blob resource contents appropriately. The base64 decoding for blob content is correct.
260-264: LGTM - Resource span attributes properly include backend URI.
375-379: LGTM - Template span attributes include backend URI template.
435-459: LGTM - Prompt rendering with span wrapping is well-implemented.The span correctly captures the prompt operation, and the message conversion properly maps backend prompt messages to local Message objects. The
get_span_attributesmethod follows the same pattern as other proxy components.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Adds opt-in distributed tracing via OpenTelemetry for observability into FastMCP server and client operations. Server spans are created for tool calls, resource reads, and prompt renders with attributes like component key, component type, provider type, session ID, and auth context. Client spans wrap outgoing calls with trace context propagation via W3C headers in request meta. Components provide their own span attributes through a `get_span_attributes()` method that subclasses override - this lets LocalProvider, FastMCPProvider, and ProxyProvider each include relevant context (original names, backend URIs). To enable: configure an OpenTelemetry SDK with a TracerProvider before importing fastmcp. Traces export to any OTLP-compatible backend. Closes ENG-2813 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test was checking caplog.records length but OpenTelemetry emits internal warning logs that were getting captured. Filter to only the test's logger to avoid flaky failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Lead with opentelemetry-instrument as the default approach - Move programmatic configuration lower in the page - Remove unimplemented metrics section - Fix attribute values (resource_template not template) - Add auth attributes (enduser.id, enduser.scope) - Add provider-specific delegation attributes - Link to OpenTelemetry Python docs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Lightweight single-binary alternative to Jaeger for local development. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
A few improvements based on code review:
- Don't override existing trace context in `extract_trace_context` - if we're
already in a valid trace (e.g., from HTTP propagation), preserve it rather
than extracting from MCP meta
- Add exception recording to `delegate_span` to match `server_span` pattern
- Remove unused `get_meter` function (metrics not implemented yet)
- Return `None` instead of `{}` from `inject_trace_context` when nothing to inject
- Clean up trivial tests that were just testing OpenTelemetry's own API
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Be specific about which operations are traced (tools, prompts, resources, resource templates) - Remove "(not the SDK)" parenthetical - Consolidate attribute documentation - remove redundancy in Tracing section - Delete unnecessary examples/diagnostics/__init__.py 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
7f356c4 to
473b0e3
Compare
- Add return type annotation to main() in run_with_tracing.py - Use spread operator for argv construction - Add type annotations to docs test example - Use async httpx client and asyncio.sleep in diagnostics server - Improve subprocess termination handling with timeout fallback 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test Failure AnalysisSummary: Type checking failure in - attempting to directly await a union type. Root Cause: The test at attempts to directly , but the type checker correctly identifies that has type . When called, this returns , which is a union type that isn't directly awaitable - you need to check if it's awaitable first. Suggested Solution: Update the test to follow the established pattern used throughout the codebase (see ProxyTool._get_client, ProxyResource._get_client, etc.). Replace the direct await with: # At line 217 in tests/server/providers/proxy/test_proxy_server.py
client = proxy.client_factory()
if inspect.isawaitable(client):
client = await client
assert isinstance(client, Client)This matches the pattern used in 15+ other places in the codebase where Detailed AnalysisType Error Details: Established Pattern in Codebase:
All of these follow the same pattern: call the factory, check Related Files
|
Test Failure AnalysisSummary: Type checking failure in Root Cause: The test at Suggested Solution: Update the test to follow the established pattern used throughout the codebase (see ProxyTool._get_client, ProxyResource._get_client, etc.). Replace the direct await with: # At line 217 in tests/server/providers/proxy/test_proxy_server.py
client = proxy.client_factory()
if inspect.isawaitable(client):
client = await client
assert isinstance(client, Client)This matches the pattern used in 15+ other places in the codebase where Detailed AnalysisType Error Details: Established Pattern in Codebase:
All of these follow the same pattern: call the factory, check Related Files
|
- Fix potential None session_id in span attributes - Add return type annotation to _get_parent_trace_context - Fix type checker issue with ClientFactoryT await pattern 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f356c461f
ℹ️ 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".
| # Inject trace context into meta for propagation to server | ||
| propagated_meta = inject_trace_context(meta) | ||
|
|
||
| result = await self._await_with_session_monitoring( | ||
| self.session.call_tool( |
There was a problem hiding this comment.
Propagate trace context for task-based calls
Tracing is only injected on the standard request path in call_tool_mcp, but call_tool(..., task=True) routes to _call_tool_as_task which builds a CallToolRequest directly and never reaches this instrumentation. That means background task invocations won’t emit client spans or carry fastmcp.traceparent to the server, so SEP-1686 task traces become orphaned even when tracing is enabled. Consider wrapping the task helpers (_call_tool_as_task, _read_resource_as_task, _get_prompt_as_task) in client_span and passing inject_trace_context(...) into request meta.
Useful? React with 👍 / 👎.
Validates that session_id is captured on both client and server spans when using HTTP transport, and that they share the same ID. 🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
examples/diagnostics/server.py (2)
54-55: Avoid catching bareException- use specific exception types.Per coding guidelines, bare
exceptor overly broadexcept Exceptionshould be avoided. For HTTP connection retries, catching specific exceptions is clearer and prevents masking unexpected errors.♻️ Suggested fix
try: await client.get( f"http://localhost:{ECHO_SERVER_PORT}/sse", timeout=0.1 ) break - except Exception: + except (httpx.RequestError, httpx.TimeoutException): await asyncio.sleep(0.1)
46-59: Consider checking subprocess health and raising on startup failure.If the subprocess crashes during startup, the loop completes without error and the proxy mount proceeds anyway, leading to confusing failures later. Consider checking
proc.poll()in the loop and raising an error if the server never becomes ready.♻️ Suggested improvement
# Wait for server to be ready (async to avoid blocking event loop) + server_ready = False async with httpx.AsyncClient() as client: for _ in range(50): + if proc.poll() is not None: + stderr = proc.stderr.read() if proc.stderr else b"" + raise RuntimeError(f"Echo server exited unexpectedly: {stderr.decode()}") try: await client.get( f"http://localhost:{ECHO_SERVER_PORT}/sse", timeout=0.1 ) + server_ready = True break - except Exception: + except (httpx.RequestError, httpx.TimeoutException): await asyncio.sleep(0.1) + + if not server_ready: + proc.terminate() + raise RuntimeError("Echo server failed to start within timeout") # Mount proxy to the running echo serverexamples/diagnostics/client_with_tracing.py (2)
32-45: Add return type annotation.Per coding guidelines, Python ≥3.10 requires full type annotations. The
setup_tracingfunction is missing its return type.Proposed fix
-def setup_tracing(): +def setup_tracing() -> None: """Set up OpenTelemetry tracing with OTLP export."""
48-48: Add return type annotation.The
mainfunction is missing its return type annotation.Proposed fix
-async def main(): +async def main() -> None:src/fastmcp/server/providers/proxy.py (1)
236-258: Consider extracting content processing into a shared helper.The content processing logic (lines 237-256) is duplicated nearly verbatim in
ProxyTemplate.create_resource()(lines 337-356). Extracting this into a helper function would reduce duplication and centralize future maintenance.Example helper extraction
def _process_resource_contents( result: Sequence[TextResourceContents | BlobResourceContents], source_uri: str, ) -> list[ResourceContent]: """Convert MCP resource contents to ResourceContent objects.""" contents: list[ResourceContent] = [] for item in result: if isinstance(item, TextResourceContents): contents.append( ResourceContent(content=item.text, mime_type=item.mimeType, meta=item.meta) ) elif isinstance(item, BlobResourceContents): contents.append( ResourceContent(content=base64.b64decode(item.blob), mime_type=item.mimeType, meta=item.meta) ) else: raise ResourceError(f"Unsupported content type: {type(item)}") return contents
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
loq.tomlis excluded by none and included by nonepyproject.tomlis excluded by none and included by nonetests/client/telemetry/__init__.pyis excluded by none and included by nonetests/client/telemetry/test_client_tracing.pyis excluded by none and included by nonetests/conftest.pyis excluded by none and included by nonetests/server/providers/proxy/test_proxy_server.pyis excluded by none and included by nonetests/server/telemetry/__init__.pyis excluded by none and included by nonetests/server/telemetry/test_provider_tracing.pyis excluded by none and included by nonetests/server/telemetry/test_server_tracing.pyis excluded by none and included by nonetests/telemetry/__init__.pyis excluded by none and included by nonetests/telemetry/test_module.pyis excluded by none and included by nonetests/test_mcp_config.pyis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (18)
docs/docs.jsondocs/servers/telemetry.mdxexamples/diagnostics/client_with_tracing.pyexamples/diagnostics/server.pyexamples/run_with_tracing.pysrc/fastmcp/client/client.pysrc/fastmcp/client/telemetry.pysrc/fastmcp/client/transports.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/resources/resource.pysrc/fastmcp/resources/template.pysrc/fastmcp/server/providers/fastmcp_provider.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/server.pysrc/fastmcp/server/telemetry.pysrc/fastmcp/telemetry.pysrc/fastmcp/tools/tool.pysrc/fastmcp/utilities/components.py
🚧 Files skipped from review as they are similar to previous changes (10)
- src/fastmcp/client/telemetry.py
- src/fastmcp/prompts/prompt.py
- examples/run_with_tracing.py
- src/fastmcp/tools/tool.py
- src/fastmcp/client/transports.py
- src/fastmcp/resources/resource.py
- docs/servers/telemetry.mdx
- src/fastmcp/client/client.py
- src/fastmcp/resources/template.py
- src/fastmcp/telemetry.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python ≥3.10 with full type annotations required for all code
Never use bareexcept- be specific with exception types in Python code
Files:
examples/diagnostics/client_with_tracing.pysrc/fastmcp/server/providers/fastmcp_provider.pysrc/fastmcp/server/telemetry.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/server.pysrc/fastmcp/utilities/components.pyexamples/diagnostics/server.py
🧬 Code graph analysis (4)
examples/diagnostics/client_with_tracing.py (1)
examples/run_with_tracing.py (1)
main(22-55)
src/fastmcp/server/providers/proxy.py (4)
src/fastmcp/client/telemetry.py (1)
client_span(12-37)src/fastmcp/exceptions.py (2)
ToolError(18-19)ResourceError(14-15)src/fastmcp/resources/resource.py (3)
get_span_attributes(412-416)ResourceContent(36-115)ResourceResult(118-206)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)
src/fastmcp/server/server.py (2)
src/fastmcp/server/telemetry.py (1)
server_span(56-90)src/fastmcp/utilities/components.py (2)
get_span_attributes(186-191)key(94-102)
src/fastmcp/utilities/components.py (6)
src/fastmcp/server/providers/proxy.py (4)
get_span_attributes(156-160)get_span_attributes(260-264)get_span_attributes(375-379)get_span_attributes(455-459)src/fastmcp/prompts/prompt.py (1)
get_span_attributes(395-399)src/fastmcp/resources/resource.py (2)
get_span_attributes(412-416)key(381-383)src/fastmcp/resources/template.py (2)
get_span_attributes(318-322)key(285-287)src/fastmcp/server/providers/fastmcp_provider.py (4)
get_span_attributes(136-140)get_span_attributes(200-204)get_span_attributes(286-290)get_span_attributes(417-421)src/fastmcp/tools/tool.py (1)
get_span_attributes(397-401)
🪛 Ruff (0.14.11)
examples/diagnostics/client_with_tracing.py
96-96: Do not catch blind exception: Exception
(BLE001)
104-104: Do not catch blind exception: Exception
(BLE001)
111-111: Do not catch blind exception: Exception
(BLE001)
120-120: Do not catch blind exception: Exception
(BLE001)
132-132: Do not catch blind exception: Exception
(BLE001)
139-139: Do not catch blind exception: Exception
(BLE001)
145-145: Do not catch blind exception: Exception
(BLE001)
152-152: Do not catch blind exception: Exception
(BLE001)
src/fastmcp/server/providers/proxy.py
232-234: Avoid specifying long messages outside the exception class
(TRY003)
256-256: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/server.py
1493-1493: Avoid specifying long messages outside the exception class
(TRY003)
1494-1494: Avoid specifying long messages outside the exception class
(TRY003)
1587-1587: Avoid specifying long messages outside the exception class
(TRY003)
1588-1588: Avoid specifying long messages outside the exception class
(TRY003)
1594-1594: Avoid specifying long messages outside the exception class
(TRY003)
1608-1608: Avoid specifying long messages outside the exception class
(TRY003)
1609-1609: Avoid specifying long messages outside the exception class
(TRY003)
1698-1698: Avoid specifying long messages outside the exception class
(TRY003)
1699-1699: Avoid specifying long messages outside the exception class
(TRY003)
examples/diagnostics/server.py
30-30: subprocess call: check for execution of untrusted input
(S603)
31-40: Starting a process with a partial executable path
(S607)
54-54: Do not catch blind exception: Exception
(BLE001)
113-113: Avoid specifying long messages outside the exception class
(TRY003)
119-119: Avoid specifying long messages outside the exception class
(TRY003)
125-125: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
🔇 Additional comments (22)
docs/docs.json (1)
131-131: LGTM!The navigation entry for the telemetry documentation is correctly placed in alphabetical order within the Features group and follows the existing naming convention.
examples/diagnostics/server.py (2)
77-98: LGTM!The successful components are well-typed with clear docstrings. Good examples for demonstrating tracing of successful operations.
104-125: LGTM!The error components are properly typed and serve their purpose of demonstrating error tracing. Using
ValueErrorwith inline messages is acceptable for this diagnostic/example context.src/fastmcp/utilities/components.py (1)
186-191: LGTM!The base implementation provides the correct extensibility hook for telemetry span attributes. The pattern of returning
{"fastmcp.component.key": self.key}and encouraging subclasses to merge viasuper().get_span_attributes() | {...}is consistent across all subclass implementations (Tool, Prompt, Resource, ResourceTemplate, and provider wrappers).examples/diagnostics/client_with_tracing.py (1)
91-121: BareExceptioncatches are acceptable here.Static analysis flags the
except Exceptionblocks, but this is intentional for an example script demonstrating how errors appear in traces. The script explicitly handles expected failures to show tracing behavior across success and error scenarios. No changes needed.src/fastmcp/server/providers/fastmcp_provider.py (5)
115-120: LGTM!The
delegate_spanwrapping correctly instruments the delegation to the child server'scall_tool. Using a synchronous context manager around an async call is appropriate since the span creation/cleanup is synchronous.
136-140: LGTM!The span attributes correctly extend the base class with provider-specific context (
FastMCPProvidertype and the original tool name before any namespace transforms).
193-204: LGTM!Resource delegation and span attributes follow the same consistent pattern as tools.
265-290: LGTM!Prompt delegation and span attributes implementation is consistent with tools and resources.
372-375: LGTM!Resource template delegation correctly uses the expanded
original_urifor the span name while including the template pattern in span attributes for traceability.Also applies to: 417-421
src/fastmcp/server/server.py (3)
1475-1494: LGTM!The
server_spanintegration correctly instruments tool calls with:
- Span name including the tool name for easy identification
- Standard MCP attributes (rpc.system, rpc.service, rpc.method)
- Component-specific attributes via
tool.get_span_attributes()- Automatic exception recording via the context manager
The existing exception handling is preserved and the span will properly record errors.
1569-1609: LGTM!The resource read instrumentation correctly handles both concrete resources and template fallback within a single span. The
span.set_attributes()call appropriately updates attributes based on whether a concrete resource or template is used, ensuring the final span reflects the actual component that served the request.
1683-1699: LGTM!Prompt rendering instrumentation follows the same consistent pattern as tool calls, with proper span attributes and exception handling.
src/fastmcp/server/telemetry.py (5)
13-27: LGTM!The function correctly extracts auth attributes with appropriate null checks and graceful error handling when no token is available.
30-41: LGTM!Correctly addresses the past review comment - now checks both
ctx.request_context is not Noneandctx.session_id is not Nonebefore adding the session ID attribute.
44-52: LGTM!The function correctly extracts parent trace context for distributed tracing propagation, with proper type annotation and error handling.
55-90: LGTM!The
server_spancontext manager provides comprehensive SERVER span instrumentation with:
- Standard OpenTelemetry RPC semantic conventions (
rpc.system,rpc.service,rpc.method)- FastMCP-specific attributes for component identification
- Auth and session context enrichment
- Automatic exception recording with proper status propagation
The bare
Exceptioncatch is intentional here - it ensures all exceptions are recorded on the span for observability before being re-raised.
93-117: LGTM!The
delegate_spanprovides appropriate INTERNAL span instrumentation for provider delegation, with consistent exception handling patterns.src/fastmcp/server/providers/proxy.py (4)
156-160: LGTM!The
get_span_attributes()method follows the established pattern from other providers and correctly exposes the backend identifier for telemetry.
260-264: LGTM!Consistent with the established span attribute pattern.
375-379: LGTM, but verify telemetry coverage forcreate_resource.The
get_span_attributes()implementation is correct. However, note thatProxyTemplate.create_resource()(lines 313-373) does not wrap itsclient.read_resource()call withclient_span, unlikeProxyResource.read(),ProxyTool.run(), andProxyPrompt.render(). If tracing template resource creation is desired, consider adding telemetry wrapping there as well.
435-459: LGTM!The telemetry wrapping and message conversion are implemented correctly. The
get_span_attributes()method follows the established pattern.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if result.isError: | ||
| raise ToolError(cast(mcp.types.TextContent, result.content[0]).text) |
There was a problem hiding this comment.
Unsafe cast assumes error content structure.
If the backend returns isError=True with an empty content list or non-TextContent item, this will raise IndexError or fail the cast. Consider defensive handling:
Suggested fix
if result.isError:
- raise ToolError(cast(mcp.types.TextContent, result.content[0]).text)
+ error_text = "Tool execution failed"
+ if result.content and hasattr(result.content[0], "text"):
+ error_text = result.content[0].text
+ raise ToolError(error_text)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if result.isError: | |
| raise ToolError(cast(mcp.types.TextContent, result.content[0]).text) | |
| if result.isError: | |
| error_text: str = "Tool execution failed" | |
| if result.content and hasattr(result.content[0], "text"): | |
| error_text = result.content[0].text | |
| raise ToolError(error_text) |
Adds OpenTelemetry tracing for observability into FastMCP server and client operations.
Server spans are created for tool calls, resource reads, and prompt renders with attributes like component key, component type, provider type, session ID, and auth context. Client spans wrap outgoing calls with trace context propagation via W3C headers in request meta.
Components provide their own span attributes through a
get_span_attributes()method that subclasses override - this lets LocalProvider, FastMCPProvider, and ProxyProvider each include relevant context (original names, backend URIs, etc).Closes #2813
🤖 Generated with Claude Code