Repository navigation
feat(mcp): add mcp server instrumentation for mcp 1.x and 2.x - #1024
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
> 🤖 **This text was AI-generated.**
## Problem
MCP servers built with the Python SDK produced no Braintrust spans. A tool
call that ran an LLM or other traced work showed up as disconnected root
spans with no record of the MCP request around them.
```
(before) anthropic.messages.create <- orphan root, no MCP context
```
## Solution
Add `MCPIntegration` and `wrap_mcp_server(server, capture_content=True)`,
following the Braintrust MCP server spec. Each incoming request or
notification gets one span covering receipt, validation, the handler, and
sending the response. Nested work becomes its child:
```
(after) tools/call ask_llm type=tool
└─ anthropic.messages.create type=llm
```
Automatic setup traces every MCP server in the process:
```python
from braintrust.integrations.mcp import MCPIntegration
MCPIntegration.setup()
```
Or wrap a single server manually:
```python
from mcp.server.fastmcp import FastMCP # or MCPServer, or fastmcp.FastMCP
from braintrust.integrations.mcp import wrap_mcp_server
server = wrap_mcp_server(FastMCP("demo"))
```
Supported: `mcp>=1.28.1,<3` (low-level `Server`, bundled `FastMCP` on 1.x,
`MCPServer` on 2.x) and standalone `fastmcp` 2.x–4.x, including
`fastmcp-slim`.
Spans include:
- names like `tools/call <tool>`, `prompts/get <prompt>`, `resources/read`
- arguments as `input` and the MCP result as `output` (content capture is on
by default; `capture_content=False` keeps only names, timing, metadata, and
error classification)
- spec metadata: `mcp.method.name`, `jsonrpc.request.id`, `mcp.session.id`,
protocol version, client/server identity, and HTTP network fields
- `error.type = "tool_error"` for `isError` results; JSON-RPC caller errors
record `rpc.response.status_code` without failing the span
- `mcp.cancelled` for cancelled requests, and an error for requests left
pending when the connection closes
- inline images, audio, and resource blobs as attachments
- outgoing notifications as children of the request that sent them
- parentage from `traceparent`/`baggage` in `params._meta`; the ambient span
never becomes the parent
MCP 1.x and 2.x share all span shaping in `tracing.py`. Only the lifecycle
hooks differ:
```
1.x ServerSession streams ──► read: start span (before validation)
write: record result, end after send
Server._handle_message ─► span is current during the handler
2.x ServerRunner._on_request/_on_notify ──► start span, run handler
JSONRPCDispatcher._handle_request ────► end after the response write
```
Payloads are logged as the dicts MCP already produces. They are copied only
where `_meta` must be dropped or media replaced, and all logging happens once,
after the response is sent.
Tests use real MCP and fastmcp clients and servers in-process, over
streamable HTTP, and over stdio to a real subprocess. Cassettes record only
the nested Anthropic call. New nox sessions: `test_mcp`, `test_fastmcp`,
`test_fastmcp_slim`.
## Limitations and next steps
- `network.transport: "pipe"` is not set for stdio. Neither MCP version
exposes the transport type to the server session.
- On 2.x single-exchange HTTP and in-process direct dispatch, spans end when
the handler returns rather than after the HTTP write.
- OTel `mcp.server.operation.duration` metrics are not emitted.
- Client instrumentation and connected traces (PR 2) and `auto_instrument()`
wiring plus docs (PR 3) follow in separate changes.
Refs #1023
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
edecdd5 to
6a16726
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edecdd559b
ℹ️ 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 (@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 (@codex) address that feedback".
> 🤖 **This text was AI-generated.** ## Problem With MCP 1.x and `raise_exceptions=True`, a request handler exception propagates instead of becoming an error response. The server span never saw it: the request stayed pending until the session closed, so the span was labeled with the wrong error. ``` prompts/get broken error.type = ConnectionClosedError <- real error lost ``` Separately, `test_fastmcp(2.14.7)` failed on the Python 3.15 Windows CI job. fastmcp 2.x depends on `lupa`, which has no cp315 wheels, and its source build fails on Windows. ## Solution `Server._handle_message`'s wrapper now records the exception and ends the span before re-raising, if no response has been sent for that request: ``` prompts/get broken error.type = ValueError error = "...prompt exploded" ``` `nox_session_constraints.py` caps fastmcp releases below 3.0.0 at Python < 3.15. The constraints file can't express a Windows-only skip, so 2.14.7 also skips on 3.15 for Linux and macOS; it is still tested on 3.10–3.14. Addresses review feedback on #1024. Refs #1023 Co-Authored-By: Claude Opus 5.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: 46d74b4ffa
ℹ️ 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 (@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 (@codex) address that feedback".
|
|
||
| def __init__(self, session: Any, config: MCPServerConfig): | ||
| self._session_ref = weakref.ref(session) | ||
| self.config = config |
There was a problem hiding this comment.
Apply capture setting changes to active MCP 1.x sessions
When wrap_mcp_server(server, capture_content=False) is called after an MCP 1.x connection has opened, this trace retains the original frozen configuration while wrap_mcp_server only updates the low-level server attribute. Subsequent requests on that existing connection therefore continue logging arguments and results despite the documented settings update, potentially retaining content the caller intended to disable; active session traces should receive the new configuration or consult the server's current setting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6e1c7bc. MCP 1.x connections no longer keep the settings they were opened with. Each one holds a weak reference to its server and reads the server's current settings for every incoming message and outgoing notification, the same way MCP 2.x already does. Re-wrapping with capture_content=False now applies to clients that are already connected.
Added test_rewrap_updates_open_connections: it makes one call, re-wraps the server with capture_content=False, then makes a second call on the same connection and checks that the second span has no input or output. It failed on mcp 1.30 and fastmcp 3 before the fix, and passes on all supported mcp and fastmcp versions now.
> 🤖 **This text was AI-generated.**
## Problem
On MCP 1.x, each connection kept the tracing settings it was opened with.
Re-wrapping a server while a client was connected did not reach that client:
```python
server = wrap_mcp_server(FastMCP("demo"))
# ... client connects and calls a tool: input/output captured ...
wrap_mcp_server(server, capture_content=False)
# ... same client calls again: input/output still captured
```
MCP 2.x was not affected because it reads the server's settings on every
request.
## Solution
A 1.x connection now keeps a weak reference to its server and reads the
server's current settings for each incoming message and outgoing
notification, matching 2.x:
```
before: session ──► config (frozen at connect)
after: session ──► weakref(server) ──► server's current config
```
Connections created while only `MCPIntegration.setup()` is active keep
following the global setting, and a server started from inside another
server's handler is still not attributed to the outer server.
Addresses review feedback on #1024.
Refs #1023
Co-Authored-By: Claude Opus 5.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: 6e1c7bcc30
ℹ️ 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 (@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 (@codex) address that feedback".
…ettings > 🤖 **This text was AI-generated.** ## Problem Two ways content still reached spans after `capture_content=False`: 1. JSON-RPC error messages and exception messages were copied into the span's `error`. They can echo application data: ``` error = "MCP error 0: Error rendering prompt broken: prompt exploded" ``` 2. On MCP 2.x, outgoing notifications used a cached copy of the settings from the connection's last request. Re-wrapping the server only took effect once another request arrived, so a notification sent in between logged its payload. ## Solution 1. With content capture off, error responses log only their code (`MCP error 0`) and exceptions log only their type name. `error.type` and `rpc.response.status_code` are unchanged. 2. Each 2.x dispatcher now keeps a weak reference to the server it serves, and notifications read that server's current settings: ``` before: dispatcher ──► config (from the last request) after: dispatcher ──► weakref(server) ──► server's current config ``` Addresses review feedback on #1024. Refs #1023 Co-Authored-By: Claude Opus 5.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: f9d719bfde
ℹ️ 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 (@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 (@codex) address that feedback".
> 🤖 **This text was AI-generated.** ## Problem The MCP tests checked in 18 cassettes (one pair per mcp/fastmcp version), but none of them recorded MCP traffic. They only replayed an Anthropic call made inside a test tool, to show a nested LLM span under the MCP tool span: ``` tools/call ask_llm └─ anthropic.messages.create <- the only thing the cassettes replayed ``` That cost an `anthropic` install in every MCP session, an API key to re-record, and a new cassette pair for every version added to the matrix. ## Solution Remove the `ask_llm` tool, the cassettes, the VCR setup in the stdio subprocess, the `anthropic` session dependency, and the MCP entry in `[tool.braintrust.cassette-dirs]`. Nesting is still covered on every transport by the manual span the `add` tool creates: ``` tools/call add └─ nested-work <- in-process, streamable HTTP, and stdio ``` MCP traffic was never recorded and stays real: clients talk to servers in-process, over streamable HTTP on localhost, and over stdio to a real subprocess. ## Limitations There is no longer a check that another integration's spans (for example `wrap_anthropic`) nest under MCP tool spans. This also departs from #1023's request for cassette-backed tests, since the MCP tests no longer need any recorded traffic. Refs #1023 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
> 🤖 **This text was AI-generated.** Stacked on [#1024](#1024), which adds MCP server instrumentation. This is PR 2 of the MCP instrumentation work. ## Problem Server instrumentation records MCP execution, but the caller has no span for the round trip. Without automatic context propagation, the agent and server appear in separate traces: ```text agent # caller trace tools/call add # server trace └─ nested-work ``` ## Solution Add `wrap_mcp_client(client_or_session, capture_content=True)` to trace outgoing requests and propagate W3C trace context and Braintrust baggage through `params._meta`. ```python from braintrust.integrations.mcp import wrap_mcp_client client = wrap_mcp_client(client) ``` Each side works independently. With both client and server wrapped, nested work joins the caller's trace: ```text agent └─ tools/call add (client) └─ tools/call add (server) └─ nested-work ``` Supports official `mcp>=1.28.1,<3` clients and sessions, standalone `fastmcp>=2.14.7,<5` clients, and corresponding fastmcp-slim releases. MCP dependencies remain optional. - Client spans share server names, types, and payload shaping. `metadata["mcp.span.kind"] = "client"` identifies the round trip. - Content capture is enabled by default. Pass `capture_content=False` to omit arguments and results; discovery inventories are always omitted. - Propagation preserves the original request and existing baggage. Concurrent requests keep their own parentage. - Errors, timeouts, cancellation, and disconnects finalize spans without changing application behavior. Returned tool errors retain their results; cancellation alone does not mark the span as failed. Wrap before connecting to include negotiation, or wrap an active client/session. Repeated wrapping updates settings without adding spans, and FastMCP reconnects inherit those settings. Framework tool spans can parent the MCP round trip. When `opentelemetry-instrumentation-mcp` is enabled, its instrumentor owns client spans, propagation, and capture settings; Braintrust adds no duplicate client span. Integration tests use real clients and servers in memory, over localhost HTTP, and over stdio subprocesses. They assert span payloads, parentage, and received trace metadata directly, without cassettes or external LLM calls. ## Scope and next steps One-way notifications, session lifetimes, and detached task APIs do not create client operation spans. Automatic client setup and documentation follow in PR 3. `MCPIntegration.setup()` continues to enable server tracing independently. Refs #1023
PR 1 for #1023
Problem
MCP servers built with the Python SDK produced no Braintrust spans. A tool
call that ran an LLM or other traced work showed up as disconnected root
spans with no record of the MCP request around them.
Solution
Add
MCPIntegrationandwrap_mcp_server(server, capture_content=True),following the Braintrust MCP server spec. Each incoming request or
notification gets one span covering receipt, validation, the handler, and
sending the response. Nested work becomes its child:
Automatic setup traces every MCP server in the process:
Or wrap a single server manually:
Supported:
mcp>=1.28.1,<3(low-levelServer, bundledFastMCPon 1.x,MCPServeron 2.x) and standalonefastmcp2.x–4.x, includingfastmcp-slim.Spans include:
tools/call <tool>,prompts/get <prompt>,resources/readinputand the MCP result asoutput(content capture is onby default;
capture_content=Falsekeeps only names, timing, metadata, anderror classification)
mcp.method.name,jsonrpc.request.id,mcp.session.id,protocol version, client/server identity, and HTTP network fields
error.type = "tool_error"forisErrorresults; JSON-RPC caller errorsrecord
rpc.response.status_codewithout failing the spanmcp.cancelledfor cancelled requests, and an error for requests leftpending when the connection closes
traceparent/baggageinparams._meta; the ambient spannever becomes the parent
MCP 1.x and 2.x share all span shaping in
tracing.py. Only the lifecyclehooks differ:
Payloads are logged as the dicts MCP already produces. They are copied only
where
_metamust be dropped or media replaced, and all logging happens once,after the response is sent.
Tests use real MCP and fastmcp clients and servers in-process, over
streamable HTTP, and over stdio to a real subprocess. Cassettes record only
the nested Anthropic call. New nox sessions:
test_mcp,test_fastmcp,test_fastmcp_slim.Limitations and next steps
network.transport: "pipe"is not set for stdio. Neither MCP versionexposes the transport type to the server session.
the handler returns rather than after the HTTP write.
mcp.server.operation.durationmetrics are not emitted.auto_instrument()wiring plus docs (PR 3) follow in separate changes.