-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): MCP server instrumentation #16807
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
Conversation
…improved test isolation
…racing and monitoring
…r MCP server instrumentation
…ibute names to match OTEL draft semantic convention
…r improved instrumentation
…or stdio and SSE transports
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.
Generally this is looks to be the right direction! The only comment I would make is to do mv core/src/mcp-server.ts core/src/integrations/mcp-server/index.ts
and move all of the utils/mcp-server
files into that integrations folder. This matches our existing structure.
I do have some low prio items, but that I can get to once this PR gets out of draft.
attributes['mcp.resource.uri'] = String(params.uri); | ||
// Extract protocol from URI | ||
try { | ||
const url = new URL(String(params.uri)); |
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 have a url parsing helper in the core package that we should use here. It also handles relative URLs.
Thanks @AbhiPrasad! I'm closing this in favor of #16817, that avoids I'll apply your suggestions on #16817 |
An improvement on MCP server tracing. It follows:
This is WIP, and will later add:
Project with this new spans:
yarn lint
) & (yarn test
).