SEP-414: Document OpenTelemetry Trace Context Propagation Conventions#414
Conversation
|
I would love the text in the spec to include more of what you have written in the PR description, and that the recommendation is to use It could be labelled as "Optional: OpenTelemetry context". I hope that SDKs decide to implement it where they have existing OTel ecosystems that they can leverage. MCP clients are not required to send it, and MCP servers that do not support OTel can safely ignore those parameters. This may be too much for the index file, but they could be added to the typescript and documentation. |
|
@samsp-msft, thanks for your thoughtful feedback! I’ve updated the "Basic" section to keep it neutral and avoid implying MCP defines OpenTelemetry (OTel) specifics: Updated Rationale:This keeps Specifically, we want to make room in otel for what's likely an inevitable OTEP for MCP. I say inevitable because the other discussion includes transport details (e.g. OTLP which is not propagation). This feels a lot like past discussions that led to specification clarifications including env variable propagation and how to handle messaging. In fact, there's already work in Otel for semantic conventions (thanks for participating in that). Ideally an engineer will be able to see the otel details coherently in one place (e.g. an OTEP). Long story short, deferring OTel keeps MCP focused on protocol mechanics, not telemetry details. By adding an example, we hint of how to hand-over to otel for clarity on telemetry. Does this address your concern? |
6c55543 to
2081ae2
Compare
|
@dsp-ant @jspahrsummers So, for anecdotal context. What drove me to the discussion leading to this and the PR itself was your podcast on latent space with @FanaHOVA and @swyxio Was a knock-out episode, and since working in oss since 2008 this made me feel the best:
So, my goal with this change was to make the absolutely least change possible, with the highest rigor. Even if it is a no, all good. Thanks for inspiring me to give it a go. I love the do the work, then let's talk approach to change. |
LGTM There is a balance between what needs to go into the base specification - providing the place to pass context - and having agreement amongst the SDK as to how they will use the extensibility mechanisms to implement telemetry propagation. The details of which key values pairs should be used and their values can probably be delegated to docs in the OTel space. It can probably go in the docs for the semantic conventions. I am hoping that we can get common agreement amongst most MCP SDKs about how to incorporate telemetry so that they can interoperate nicely, and it doesn't need major retrofits later. |
|
@samsp-msft thanks for the support. PS I raised a PR to csharp-sdk to change the carrier from |
757a83e to
8a5e71f
Compare
|
Earlier I mis-attributed I've revised the spec change to link to the progress spec, and also shored up the description accordingly. |
|
@dsp-ant do you have any advice for us on how to progress this PR? I'm happy to revise it, but it seems stalled. |
|
Right now, by naming convention ( Specifically, this PR describes some use cases:
Whereas in the schema today, there are 13 occurrences of this advice on
The recent work by @findleyr and friends on the Go SDK design implies code generation, and there's a small gap you can see if you look carefully here.
There are a number of ways to address code generation coherency.. we could make a single "meta" type and use that for Finally, we can decide to not solve it strictly in the schema. Rather, stick with advice here and mention to code generator authors that there's a relationship with anything ending in Request, Response etc that should have special casing. I don't have a preferred way out, but I would like to help close out this topic. Any thoughts? |
|
added a section to the description that it is possible a future JSON-RPC 2.1 could formalize "_meta". That said, I don't expect this to change any current practice. cc @mpcm |
|
@codefromthecrypt See recently posted: https://groups.google.com/g/json-rpc/c/pFFuI0JN8Cs |
|
In the JSON-RPC group discussion, I mentioned that responses could benefit from a The protocol currently uses |
|
@jonathanhefner Thanks for the suggestion - personally, if the compatibility issue is acceptable I would suggest top level |
|
We have a similar need. I too would prefer something outside of the params, although it's a good way to experiment for the time being. |
|
going to close this out as it hasn't moved forward in a month. happy to re-open when maintainers are interested in a change. Meanwhile, per the description, there are enough artifacts here and there to suggest |
|
FWIW, i was able to abuse the protocol's sending: https://github.com/dylibso/mcp-otel/blob/2407c736c92d6a5e71b454845d839f33dabbbfca/src/agent.ts#L122 I think this should be safe if you do your best to avoid name collisions, but going to ask around. Doing the actual context propagation and naming of everything works, but is a little tedious if you're not familiar with it. This could perhaps be part of an otel adapter for the SDKs, or one day adopted with the SDKs if some things can be agreed upon. |
|
@bhelx thx for the feedback. I will add to the description your use of this pattern which aligns with others mentioned there including Arize Openinference which is an otel SDK. That way folks don't have to scroll through comments should there be a desire to formalize this later. |
c3e532e to
8712009
Compare
|
thanks @dsp-ant makes sense. I added text I believe highlights this more clearly in the last commit. |
Document OpenTelemetry trace context propagation conventions for _meta keys (traceparent, tracestate, baggage) in the draft spec, including the rationale for the DNS prefix exception. Signed-off-by: Adrian Cole <adrian@tetrate.io>
|
ok added the spec and changelog. 🚢 when ready or tell me what's next. Thanks, all! |
- Status: Final (accepted with reference implementations in org SDKs) - Created: 2025-04-25 (PR creation date per convention) Signed-off-by: Adrian Cole <adrian@tetrate.io>
|
Polish notes: I set this status to Final as it is accepted and reverse documents the existing c# SDK implementation, which at the same time serves as a reference. This is inline with other SEPs all marked final, as well. For the SEP date, I noticed another is using the PR creation date, so went with that. |
Maintainer Activity CheckHi @Kludex! You're assigned to this SEP but there hasn't been any activity from you in 19 days. Please provide an update on:
If you're no longer able to sponsor this SEP, please let us know so we can find another maintainer. This is an automated message from the SEP lifecycle bot. |
|
happy to rebase if this is keen to be merged if I do |
|
I have been following this issue and luckily am not in a situation to be hurt either way so will say it - if multiple anthropic employees needs to sign off on a change, this doesn't seem to be a community project? Happy to be proven wrong - I am not suspecting malice at all but governance is important and I think we need to see it clearly through these examples. |
|
@Kludex it's accepted, so go ahead and land once the merge conflicts are resolved. Thanks! @anuraaga you can read about MCP governance here: https://modelcontextprotocol.io/community/governance |
|
@pja-ant docs are easy to write, or vibe. What matters is what actually happens in practice. If this was just a confirmation point, great, but still the optics are
So the suggestion is to not approve the PR for these situations to make clear there isn't gate keeping (can't take it back so no worries this time), and if possible remove These are suggestions and if you disagree, feel free to ignore - I have a feeling that's what will happen. But I have seen many good and bad oss communities before, and am hoping this ends up on the better side. |
Summary
This PR introduces SEP-414, documenting OpenTelemetry Trace Context Propagation Conventions
SEP
seps/414-request-meta.mdSponsor
@Kludex
Notes
This PR converts the earlier doc change into a standards-track SEP. Discussion links and rationale are in the SEP itself.