fix(mcp): Nest MCP spans under HTTP transactions #5292
Conversation
| def use_isolation_scope(isolation_scope: "Scope") -> "Generator[Scope, None, None]": | ||
| def use_isolation_scope(isolation_scope: "S") -> "Generator[S, None, None]": | ||
| """ | ||
| .. versionadded:: 2.0.0 | ||
| Context manager that uses the given `isolation_scope` and runs the wrapped code in it. | ||
| The current scope is also forked to not bleed data into the existing current scope. | ||
| After the wrapped code is executed, the original scopes are restored. | ||
| Example Usage: | ||
| .. code-block:: python | ||
| import sentry_sdk | ||
| with sentry_sdk.isolation_scope() as scope: | ||
| scope.set_tag("color", "green") | ||
| sentry_sdk.capture_message("hello") # will include `color` tag. | ||
| sentry_sdk.capture_message("hello, again") # will NOT include `color` tag. | ||
| """ | ||
| if isolation_scope is None: | ||
| yield isolation_scope | ||
| return |
There was a problem hiding this comment.
I'm not sure about accepting None in the context managers. It's convenient in the MCP integration since it helps avoid code duplication (or the need to break stuff into more functions).
We could also just check if the scopes are None, so I can adjust if needed.
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| _set_span_output_data(span, result, result_data_key, handler_type) | ||
| return result | ||
| return result |
There was a problem hiding this comment.
Sync handlers missing HTTP scope nesting
Medium Severity
The _async_handler_wrapper was updated to use use_isolation_scope and use_scope context managers for nesting MCP spans under HTTP transactions, but _sync_handler_wrapper was not similarly updated. Sync MCP handlers using HTTP-based transports (like SSE) will not have their spans properly nested under the HTTP transaction, while async handlers will. This creates inconsistent behavior depending on whether a handler is sync or async, even though both can be used with HTTP transport.
Additional Locations (1)
There was a problem hiding this comment.
The _sync_handler_wrapper probably shouldn't exist in the first place. The decorators we are wrapping are for async functions only.
Description
Patch
StreamableHTTPServerTransport.handle_request()to store isolation and current scopes on the Starlette request object'sscopeattribute.Run MCP handlers in the context of the stored scopes, if these are available.
Modify
use_isolation_scope()anduse_scope()context managers to acceptNone. The MCP protocol allows for transport layers that are not HTTP-based, so the scopes may not be available.The Python MCP SDK uses an in-memory queue of requests. The HTTP propagation context is not active in the MCP decorators we patch, since they run after requests are retrieved from the queue.
Issues
Closes #5272
Closes PY-2022
Closes TET-1688
Reminders
tox -e linters.feat:,fix:,ref:,meta:)