Repository navigation
feat: built-in OpenTelemetry tracing and metrics - #3115
mattzcarey wants to merge 7 commits into
Conversation
Client, Server and McpServer take instrumentation: { send, receive }. The SDK
calls it for every request and notification it sends or receives, merges
trace-context keys the hook writes into params._meta, runs the transport send
or the handler inside the returned scope, and ends the scope exactly once
with how the message settled. A throwing hook is reported via onerror and
never changes the outcome. Without the option nothing changes on the wire.
Refs #1264.
🦋 Changeset detectedLatest commit: cfb8190 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
…ion option
openTelemetry() turns every MCP request and notification into spans and
mcp.{client,server}.operation.duration histograms following the OpenTelemetry
semantic conventions for MCP, and propagates W3C trace context and baggage
through params._meta. Content capture is opt-in. Adds the docs page
(docs/advanced/opentelemetry.md) with a runnable companion, the
examples/opentelemetry story over stdio and Streamable HTTP, and the design spec.
Refs #1264.
Client, Server and McpServer call the OpenTelemetry API directly: register an SDK and MCP traffic shows up; without one the API is a no-op and messages are unchanged. @opentelemetry/api is a dependency of client and server, the MCP mapping moves into core-internal, and the instrumentation hook becomes package-internal (attached via setProtocolInstrumentation). Drops the @modelcontextprotocol/otel package, the public instrumentation option and captureContent. Instrumentation scopes are @modelcontextprotocol/client and @modelcontextprotocol/server. Tests and examples use OpenTelemetry SDK 3. Refs #1264.
Sentry's propagator only injects the active context, so injecting before the span was made active left params._meta empty and broke cross-process traces under Sentry. Inject inside context.with(spanContext).
There was a problem hiding this comment.
3 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.
Findings marked 🟡 are optional suggestions and need no follow-up push.
- Record a received method name only for spec methods or methods with their own handler; fallback-handled methods are client-chosen and become _OTHER. - Keep a server-side tool or prompt name only once its handler recognised it, so a cancelled unknown-tool call no longer records the name. - Record a requested protocolVersion on the receive side only when supported. - Bucket application-defined error codes from peers as _OTHER on client metrics. - Inject trace context from the span's own context, so propagation works without a context manager while Sentry's propagator still sees it active. - Recreate the duration histograms when the global meter provider changes. - @opentelemetry/api is now a peer dependency of client and server, so the SDK shares the application's copy (a second copy silently ignored the app's SDK). - Changeset and docs: baggage reaches peers, strict custom schemas need an optional _meta, notifications/message is skipped, what spans do record. - Trim internal JSDoc; drop the design spec from this PR.
Errors the protocol sends before dispatch (-32601, -32022, the envelope's -32602) carry no thrown cause, so a tools/call to an instance with no tool handler no longer records the client-chosen name. Changeset lists every install setup that doesn't add the new peer dependency automatically.
|
@mattzcarey a suggestion for us to consider before this lands: keep the zero-config default, and add one optional override for the cases the app's global OpenTelemetry setup can't express. It brings back part of // On ProtocolOptions, so Client, Server and McpServer all accept it
openTelemetry?: false | {
propagator?: TextMapPropagator; // _meta inject (send) and extract (receive); default: the global propagator
tracerProvider?: TracerProvider; // default: the global tracer provider
meterProvider?: MeterProvider; // default: the global meter provider
};
Later, in its own PR: The conventions list What I'd leave to OpenTelemetry and document instead
If we want the smallest step, |
…kers caveats Filtering the MCP scopes orphans handler spans, so point Sentry users at removing its McpServer integration instead. Say _meta follows the global propagator like HTTP headers, and that Workers trace flags don't reflect sampling.
| ::: warning | ||
| `params._meta` gets whatever your global propagator writes, as HTTP headers do. If it includes W3C Baggage (the `NodeSDK` default), baggage entries reach every MCP server you call, including third-party ones. Register `W3CTraceContextPropagator` on its own to send trace context only. | ||
|
|
||
| On Cloudflare Workers, a span's trace flags don't reflect sampling, so a W3C propagator sends `traceparent` with flags `00` and peers may treat the request as unsampled. |
There was a problem hiding this comment.
🟡 nit (optional): Readers who run the OpenTelemetry JS SDK on Cloudflare Workers are told their traceparent always carries flags 00, which is not true for that setup. docs/advanced/opentelemetry.md:91 says it for all of Workers. The guide never names the setup it applies to: Workers' own tracing behind registerOpenTelemetry() from agents/observability. With the SDK setup this page shows, the sampler sets the flags (test/integration/test/otel/sdkSetupOrder.test.ts:48 expects -01). Fix: scope the sentence to that bridge by name and say what a reader should do about it, outside the baggage warning.
Why this was flagged
No runtime trigger; this is a wording check on a line this push adds. The only setup the guide documents is the JS SDK: TracerProvider from @ opentelemetry/sdk-trace plus W3CTraceContextPropagator (docs/advanced/opentelemetry.md:13-21), and Cloudflare Workers appears nowhere else on the page. In that setup the span's trace flags come from the SDK sampler on any runtime, and send injects from that span context (packages/core-internal/src/shared/openTelemetry.ts:159-162), so a sampled span sends -01, as test/integration/test/otel/sdkSetupOrder.test.ts:48 asserts. Line 91 says "On Cloudflare Workers, a span's trace flags don't reflect sampling, so a W3C propagator sends traceparent with flags 00" with no qualifier. A Workers reader on the documented SDK setup is told their peers see unsampled requests when they do not, and a reader on the bridge cannot tell the caveat is theirs or what to do. The base branch has no such sentence. No safeguard applies to doc prose.
Verification: The sentence at docs/advanced/opentelemetry.md:91 is unconditional. test/integration/test/otel/sdkSetupOrder.test.ts:17-19 registers TracerProvider from @ opentelemetry/sdk-trace plus W3CTraceContextPropagator, and line 48 expects traceparent: 00-${traceId}-${spanId}-01. A reader on Workers with the JS SDK is told their requests go out unsampled when the code sends -01.
Finding ID: f-90d751b6
Client,ServerandMcpServernow emit OpenTelemetry spans and duration metrics for every MCP request and notification, following the MCP semantic conventions, and carry trace context inparams._meta(SEP-414). There is nothing to configure: whatever OpenTelemetry setup the app already has picks MCP up. Implements #1264.On Cloudflare Workers,
registerOpenTelemetry()fromagents/observabilityputs the OpenTelemetry API on Workers tracing, so MCP spans land in Workers traces next to the platform's own spans. Register once at module scope as above:and turn traces on in
wrangler.jsonc:@opentelemetry/apiis a no-op until an app registers an SDK onglobalThis, so the SDK calls it unconditionally. Without one nothing is recorded and messages are unchanged.@opentelemetry/api(^1.9.0) is a new peer dependency ofclientandserver, so the SDK shares the app's copy; a second copy would silently ignore the app's SDK. There is no new public API.Each setup above ran against packed builds of this branch and delivered the MCP spans to its backend; with Sentry, a client and server in separate processes joined one trace. Tests ran (
pnpm check:all, package, integration and e2e suites), including e2e propagation on every transport.