Skip to content

fix(mcp): give each FastMCP server its own workflow name - #4646

Open
feiiiiii5 wants to merge 1 commit into
traceloop:mainfrom
feiiiiii5:hunt/openllmetry-d160
Open

feiiiiii5 wants to merge 1 commit into
traceloop:mainfrom
feiiiiii5:hunt/openllmetry-d160

Conversation

@feiiiiii5

@feiiiiii5 feiiiiii5 commented Oct 20, 2026 •

Copy link
Copy Markdown

Summary

FastMCPInstrumentor stores a single _server_name for the whole instrumentor and overwrites it every time a FastMCP server is constructed. A process that hosts more than one server therefore labels the spans of every server with the name of the last one constructed. An unnamed server also inherits the previous server's name instead of the generated name FastMCP gave it.

The name is now resolved from the server object after __init__ finishes (instance.name, which covers positional, keyword, and generated names) and kept per server in a WeakKeyDictionary keyed by that server's tool manager, which is the object the tool-call wrapper receives.

Reproduction: create FastMCP("first-server") with a tool, then construct FastMCP("second-server"), then call the first server's tool. On the base commit the server-side spans carry traceloop.workflow.name=second-server.mcp; with the change they carry first-server.mcp. The second test covers an unnamed server, whose generated name was previously replaced by the last named server.

Tests

Added tests/test_fastmcp_server_names.py:

  • constructing a second server does not rename the first server's spans;
  • an unnamed server reports its own generated name rather than a previous server's.

Both fail on the base commit with the wrong traceloop.workflow.name value and pass with the change.

Validation in the package's locked environment (uv sync --frozen --all-groups, Python 3.12):

  • pytest tests -> 51 passed.
  • ruff check on both changed files -> clean.

Checklist

  • 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.

The change is visible in span attributes, which the tests assert with an in-memory exporter; no hosted platform is involved.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed workflow names for applications using multiple FastMCP servers in the same process.
    • Spans now consistently use each server’s configured or generated name without being overwritten by another server.
    • Improved name tracking for servers initialized with positional arguments, keyword arguments, or no explicit name.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7a64be63-661a-43ee-b172-b3a55ec20508
📥 Commits

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

📒 Files selected for processing: 2
  • packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
  • packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_server_names.py

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

📝 Walkthrough

FastMCP instrumentation now tracks resolved workflow names separately for each tool manager. It applies the matching name to the parent MCP span and nested tool span. New tests cover multiple named servers and generated names.

Changes: FastMCP workflow names

Layer / File(s) Summary
Associate resolved names with tool managers
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
The instrumentation stores each initialized server’s resolved name, suffixed with .mcp, in a weak-key mapping keyed by its tool manager.
Apply names to spans and test server isolation
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_server_names.py
Tool execution applies the workflow name associated with its tool manager to both spans. Tests check that servers retain distinct names and that an unnamed server uses its generated name.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: nina-kollman

Merge Risk: ⚪ Minimal · up to 3d5d3

FastMCP servers now keep their own workflow names on spans, including generated names for unnamed servers. No actionable merge risk was identified, and tests cover the changed behavior.

Security architecture risk: 🔵 Low · up to 3d5d3

The change prevents one server from relabeling another server’s spans without changing tool execution authority. Compatibility of the new weak-key association with all relevant FastMCP versions and lifecycle paths remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated effect is confined to attribution of instrumented server-side tool spans within a process. Server operators can influence their workflow labels through server naming; the inspected change does not give tool callers a new execution target or additional privilege. Downstream uses of workflow labels were not established.

Trust Boundaries and Controls

  • observed — Tool names and arguments still flow to the wrapped FastMCP operation. Input and output capture remain gated by the existing content-capture check, and automatic exception recording remains disabled on these spans. These controls are unchanged by the attribution fix.

Resilience and Maintainability Implications

  • inferred — If an upstream manager cannot serve as a weak key, construction-time bookkeeping errors are caught by the existing decorator, but the new unguarded lookup could prevent the wrapped tool call from running. No incompatible manager implementation was observed, so this remains a failure-containment uncertainty rather than a retained finding. The existing no-op uninstrumentation behavior predates this PR.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 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: assigning each FastMCP server its own workflow name.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ 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.

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.

1 participant