-
Notifications
You must be signed in to change notification settings - Fork 275
MCP semantic conventions #2083
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?
MCP semantic conventions #2083
Conversation
I realize this is dangling, but one thing I wanted to mention here: you should keep mcp.transport. Abstracting this purely to the network transport removes the value conventions add. For example, how would I identify the difference between http streaming and sse? We (Sentry) are already implemented a variation of this spec, so I'm going to suggest to the team to capture that too. We'll probably just do "http" and "sse" and stick w/ the conventions used in the SDKs. |
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.
Pull Request Overview
Adds the first version of Model Context Protocol (MCP) semantic conventions, including span definitions, attribute and metric schemas, documentation, and repository configuration.
- Introduce
model/mcp/
with common attributes, spans, and metrics YAML definitions - Generate and link corresponding documentation under
docs/registry/attributes/mcp.md
anddocs/gen-ai/mcp.md
- Update templates, issue forms, and CODEOWNERS to include MCP
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
templates/registry/markdown/weaver.yaml | Added MCP to known acronyms |
model/mcp/common.yaml | Defined common MCP attributes |
model/mcp/spans.yaml | Added client/server span definitions |
model/mcp/registry.yaml | Registered MCP attributes and method enums |
model/mcp/metrics.yaml | Defined MCP operation and session metrics |
docs/registry/attributes/mcp.md | Auto-generated attribute reference page |
docs/registry/attributes/README.md | Added MCP to attribute namespace listing |
docs/gen-ai/mcp.md | New semantic conventions guide for MCP |
docs/gen-ai/README.md | Linked MCP guide in Gen-AI overview |
.github/ISSUE_TEMPLATE/{change_proposal,bug_report}.yaml | Added area: mcp in issue templates |
.github/CODEOWNERS | Assigned owners for model/mcp/ |
.chloggen/2083.yaml | Added changelog entry for MCP enhancement |
Comments suppressed due to low confidence (2)
model/mcp/registry.yaml:139
- This inline comment reflects internal uncertainty. Consider moving it into a formal
note
field or resolving the overlap betweenmcp.tool.name
andgen_ai.tool.name
instead of leaving it as a code comment.
# This is effectively the same as `gen_ai.tool.name`, but it's not clear
docs/gen-ai/mcp.md:223
- This manual duplication of the server span note diverges from the autogenerated snippet and is outdated. Remove the redundant block and rely on the generated content to keep docs in sync.
<!-- This is a dup of yaml note and should no longer be necessary after https://github.com/open-telemetry/semantic-conventions/pull/1505 -->
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.
Amazing work on this! Excited to clean up a few small areas and get it merged in.
stability: development | ||
- id: ping | ||
value: ping | ||
brief: > |
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.
Would you mind also updating the brief
for ping
? It doesn't provide any progress updates, it just expects an empty payload in response.
- id: mcp.session.id | ||
type: string | ||
brief: Identifies MCP session. | ||
stability: development | ||
examples: ["191c4850af6c49e08843a3f6c80e5046"] |
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.
Could we use the existing session.id
attribute?
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 this going either way, but it's worth noting that the definition of MCP's session
is somewhat distinct from OTel's:
MCP:
An MCP “session” consists of logically related interactions between a client and a server, beginning with the initialization phase. To support servers which want to establish stateful sessions: ...
Session is defined as the period of time encompassing all activities performed by the application and the actions executed by the end user. ... When a session reaches end of life, typically due to user inactivity or session timeout, a new session identifier will be assigned. ...
session
for MCP seems more closely aligned with gen_ai.conversation.id, which is its own attribute in the gen_ai
group.
- id: mcp.request.id | ||
type: string | ||
brief: > | ||
This is a unique identifier for the request. | ||
stability: development | ||
examples: ["42"] |
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.
Thoughts on using an attribute like peer.request.id when describing it from the client?
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.
From what I see in the context of OTel peer
means a separate process, usually over the network. Since MCP client-server relationships have the option to run on stdio, IMO it doesn't always meet the definition of a peer. Happy to get a second opinion though.
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.
For me a peer is any service that you are interacting with hence it is important to note peer doesn't define any addressing attributes this is left to namespaces such as destination & server.
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.
Understood, that makes sense. To be clear, you would completely replace mcp.request.id
with peer.request.id
? What about the mcp.request.argument.<key>
attribute?
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 would leave it as I see them as comparable to http arguments etc.
- id: metric.mcp.client.session.duration | ||
type: metric | ||
metric_name: mcp.client.session.duration | ||
brief: The duration of the MCP session as observed on the MCP client. | ||
unit: s | ||
instrument: histogram | ||
stability: development | ||
extends: mcp.session.metrics.attributes | ||
attributes: | ||
- ref: server.address | ||
- ref: server.port | ||
requirement_level: | ||
recommended: When `server.address` is set | ||
|
||
- id: metric.mcp.server.session.duration | ||
type: metric | ||
metric_name: mcp.server.session.duration | ||
brief: The duration of the MCP session as observed on the MCP server. | ||
unit: s | ||
instrument: histogram | ||
stability: development | ||
extends: mcp.session.metrics.attributes |
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.
Would these fit in the session namespace with perhaps a session type attribute?
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.
Thank you for this PR!
| `quic` | QUIC |  | | ||
| `tcp` | TCP |  | | ||
| `udp` | UDP |  | | ||
| `unix` | Unix domain socket |  | |
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 not go with 'uds'?
| [`network.transport`](/docs/registry/attributes/network.md) | string | The transport protocol used for the MCP session. [4] | `tcp`; `udp` | `Recommended` |  | | ||
| [`server.address`](/docs/registry/attributes/server.md) | string | Server domain name if available without reverse DNS lookup; otherwise, IP address or Unix domain socket name. [5] | `example.com`; `10.1.2.80`; `/tmp/my.sock` | `Recommended` |  | | ||
| [`server.port`](/docs/registry/attributes/server.md) | int | Server port number. [6] | `80`; `8080`; `443` | `Recommended` When `server.address` is set |  | | ||
| [`mcp.request.argument.<key>`](/docs/registry/attributes/mcp.md) | string | Additional arguments passed to the request within `params` object. `<key>` being the normalized argument name (lowercase), the value being the argument value. [7] | `mcp.request.argument.location="Seattle, WA"`; `mcp.request.argument.a="42"` | `Opt-In` |  | |
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 is the '' needed here? I don't see a specific reason to parse the first layer of arguments, especially since the input could be a nested JSON object.
See https://github.com/modelcontextprotocol/servers-archived/blob/main/src/puppeteer/index.ts
| [`server.address`](/docs/registry/attributes/server.md) | string | Server domain name if available without reverse DNS lookup; otherwise, IP address or Unix domain socket name. [5] | `example.com`; `10.1.2.80`; `/tmp/my.sock` | `Recommended` |  | | ||
| [`server.port`](/docs/registry/attributes/server.md) | int | Server port number. [6] | `80`; `8080`; `443` | `Recommended` When `server.address` is set |  | | ||
| [`mcp.request.argument.<key>`](/docs/registry/attributes/mcp.md) | string | Additional arguments passed to the request within `params` object. `<key>` being the normalized argument name (lowercase), the value being the argument value. [7] | `mcp.request.argument.location="Seattle, WA"`; `mcp.request.argument.a="42"` | `Opt-In` |  | | ||
|
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.
Maybe we need 'mcp.response.output' here with opt-in level, which is the same as chat history.
This is based on the work in open-telemetry/semantic-conventions#2083 with the goal to be standardised monitoring of this MCP server.
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.
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.
Fixes #2043
Adds the first version of MCP conventions including spans and basic metrics.
Implementations/prototypes: