-
Notifications
You must be signed in to change notification settings - Fork 3k
Add pluggable instrumentation interface and request_id logging #1693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dgenio
wants to merge
7
commits into
modelcontextprotocol:main
Choose a base branch
from
dgenio:feature/instrumentation-interface
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add pluggable instrumentation interface and request_id logging #1693
dgenio
wants to merge
7
commits into
modelcontextprotocol:main
from
dgenio:feature/instrumentation-interface
+1,057
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a pluggable instrumentation interface for monitoring MCP request/response lifecycle. This lays groundwork for OpenTelemetry and other observability integrations. Changes: - Define Instrumenter protocol with on_request_start, on_request_end, and on_error hooks - Add NoOpInstrumenter as default implementation with minimal overhead - Wire instrumenter into ServerSession and ClientSession constructors - Add instrumentation calls in Server._handle_request for server-side monitoring - Add request_id to log records via extra field for correlation - Add comprehensive tests for instrumentation protocol - Add documentation with examples and best practices Addresses modelcontextprotocol#421
This change addresses feedback on the instrumentation interface design. The updated API now uses a token-based approach where on_request_start() returns a token that is passed to on_request_end() and on_error(). This enables instrumenters to maintain state (like OpenTelemetry spans) without external storage or side-channels. Changes: - Updated Instrumenter protocol to return token from on_request_start() - Modified on_request_end() and on_error() to accept token as first parameter - Updated server.py to capture and pass instrumentation tokens - Updated all tests to match new API - Added complete OpenTelemetry example implementation - Updated documentation with token-based examples Fixes modelcontextprotocol#421
Author
|
Updated the instrumentation interface to use a token-based API based on community feedback. This enables proper OpenTelemetry integration without external storage. See updated PR description for details. |
…ection Pytest was trying to collect TestInstrumenter as a test class because it starts with 'Test', but it's actually a helper class with an __init__ constructor. Renaming to MockInstrumenter resolves the PytestCollectionWarning.
Added full type hints to MockInstrumenter class to resolve pyright type checking errors. This ensures the test helper class properly implements the Instrumenter protocol with correct types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
Request for a new feature that's not currently supported
P2
Moderate issues affecting some users, edge cases, potentially valuable feature
v2
Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a pluggable instrumentation interface with a token-based API for the MCP Python SDK, enabling OpenTelemetry and other observability integrations as requested in #421.
Key Innovation: Token-Based API
The instrumentation interface uses a token-based approach that solves a critical design problem:
This design enables instrumenters to maintain state (like OpenTelemetry spans) without external storage or side-channels, addressing feedback from the community on API design best practices.
Changes
Core Interface
Instrumenterprotocol with token-based hooksNoOpInstrumenteras default implementation with minimal overheadIntegration
instrumenterparameter toServerSessionandClientSessionconstructorsServer._handle_request()to track:request_idto loggingextrafields for correlationOpenTelemetry Example
OpenTelemetryInstrumenterimplementation inexamples/opentelemetry_instrumentation.pyTesting
request_idis consistent across lifecycleDocumentation
docs/instrumentation.mdwith:Benefits
spans = {}dictionariesFollow-up Work
pip install mcp[opentelemetry])params._meta.traceparentpropagationFixes #421