-
Notifications
You must be signed in to change notification settings - Fork 619
feat(instrumentation-mcp): add OpenTelemetry instrumentation for MCP SDK #3186
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
base: main
Are you sure you want to change the base?
Conversation
Add automatic OpenTelemetry instrumentation for the Model Context Protocol (MCP) SDK to enable distributed tracing across MCP client–server boundaries. Features 1. Automatic span creation for client requests and server handlers. 2. OpenTelemetry context propagation via request metadata. 3. Support for MCP semantic conventions. Reference: MCP semantic conventions open-telemetry/semantic-conventions#2083 Tests 1. End-to-end test with HTTP-based MCP client and server 2. End-to-end test with stdio-based MCP client and server 3. All unit tests pass Implementation Notes According to the MCP SDK documentation https://github.com/modelcontextprotocol/typescript-sdk/blob/main/README.md, the following modules are used to create the MCP client and server: Client Module name: '@modelcontextprotocol/sdk/client/index.js' basedir: '/Users/someone/http-client-server/client/node_modules/@modelcontextprotocol/sdk' Server Module name: '@modelcontextprotocol/sdk/server/mcp.js' basedir: '/Users/someone/http-client-server/server/node_modules/@modelcontextprotocol/sdk' The existing InstrumentationBase could not wrap both modules from the @modelcontextprotocol/sdk package. This PR introduces a custom hook within the MCP instrumentation library to support both.
|
@lukeina2z is this a duplicate of #3175? maybe close the other one to avoid reviews in different places |
|
|
||
| ### Added | ||
|
|
||
| - Initial release of MCP (Model Context Protocol) instrumentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually add the PR link and title in the changelog, not the specific details , Ex:
Add OpenTelemetry instrumentation for MCP SDK 3186
| @@ -0,0 +1,55 @@ | |||
| Apache License | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks different to other LICENSE files in this repo, it would be good to use the same for consistency
|
|
||
| ### Known Limitations | ||
|
|
||
| - ESM (ECMAScript Modules) not supported |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ESM is not supported? I believe it would be easier to add these kind of limitations in main readme file of the package
| ``` | ||
|
|
||
| ## Configuration | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see you have debugLogFile config there, we should document all configuration options here
|
|
||
| ## Semantic Conventions | ||
|
|
||
| This instrumentation uses MCP-specific semantic conventions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other instrumentation refer to specific Semantic Convention version instead of the actual values, it would be easier to update.
Ex:
Semantic Conventions
This package implements Semantic Convention Version 1.38.0.
| [![NPM Published Version][npm-img]][npm-url] | ||
| [![Apache License][license-image]][license-image] | ||
|
|
||
| This module provides automatic instrumentation for the [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) SDK for Node.js applications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to mention what kind of telemetry it will be generated
|
|
||
| This module provides automatic instrumentation for the [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) SDK for Node.js applications. | ||
|
|
||
| ## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are adding this in auto instrumentation node package, it should mention that approach, you can use other readme as reference like bunyan https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/packages/instrumentation-bunyan/README.md
Add automatic OpenTelemetry instrumentation for the Model Context Protocol (MCP) SDK to enable distributed tracing across MCP client–server boundaries.
Features
Reference:
MCP semantic conventions
open-telemetry/semantic-conventions#2083
Tests
Implementation Notes
According to the MCP SDK documentation https://github.com/modelcontextprotocol/typescript-sdk/blob/main/README.md, the following modules are used to create the MCP client and server:
Client
Module name: '@modelcontextprotocol/sdk/client/index.js' basedir: '/Users/someone/http-client-server/client/node_modules/@modelcontextprotocol/sdk'
Server
Module name: '@modelcontextprotocol/sdk/server/mcp.js' basedir: '/Users/someone/http-client-server/server/node_modules/@modelcontextprotocol/sdk'
The existing InstrumentationBase could not wrap both modules from the @modelcontextprotocol/sdk package. This PR introduces a custom hook within the MCP instrumentation library to support both.