feat(mcp-js): move instrumentation to transport layer #1555
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
dosubot
bot
added
the
size:XL
codefromthecrypt
approved these changes
Apr 24, 2025
Contributor
codefromthecrypt
left a comment
codefromthecrypt
left a comment
There was a problem hiding this comment.
Thanks so much for this @anuraaga! I had hoped both vercel and inspector would be traceable with openinference by my next talk (cloud native summit early next week in auckland)
Bravo and thanks for the support!
@mikeldking fyi another MCP win
mikeldking
approved these changes
Apr 24, 2025
Collaborator
mikeldking
left a comment
mikeldking
left a comment
There was a problem hiding this comment.
This is great @anuraaga. Cool to see the new streamable module too.
GeLi2001
pushed a commit
that referenced
this pull request
May 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This reworks the context propagation instrumentation for MCP to instrument
Transportinstead ofClient/Server. I had intentionally picked the latter at first due to concerns of the implications of propagating back from a server to a client, but with more thought, it was too much concern. Notably, if treating as two nodes talking to each other, it seems just fine to have A -> B -> A since the second A just happens to be the same server, but is not the same RPC method. This is the point notably brought up in #1524 (review) where MCP explicitly supports a server calling back to the client. This PR adds this usage to the test cases to demonstrate.Aside from server calling client, we found a couple of other advantages of instrumenting
TransportTransportfrom@modelcontextprotocol/sdkwhile using hand-written clients. By instrumentingTransport, this will be instrumented without explicitly instrumenting vercel AIClient/Serverat all to set up an MCP stream. Notably, proxies such as in mcp-inspector do this. InstrumentingTransporthandles this case too.The downside is that for any new transport, instrumentation needs to be updated - this is probably not a frequent event though. The instrumentation logic is the same so it is mostly a registration step.
/cc @codefromthecrypt