feat(node): Add first-party Mastra integration - #23823
Conversation
b5821be to
2c85c6e
Compare
size-limit report 📦
|
8beaa13 to
18578a1
Compare
Name `handler` spans after the route they serve when span streaming is enabled, or `Request handler` if no route set. Static mode left as is. Drop Hapi method, as the template dictates. NestJS resolves no route when the span starts. The NestJS callback name stays on `nestjs.callback`. Elysia sets `context.route` when the request enters the compiled handler, which is before the `Handle` phase reports. Read it in the trace listener so streamed handler spans carry the route instead of the `Request handler` fallback. The fallback now applies only when the context has no route. Set `code.function.name` only when the handler has a name. Static mode keeps the handler name in the span name, so the attribute adds nothing there, and an anonymous handler has no name to record. Adding convention for `code.function.name` in general is tracked in #23823. Register the Fastify test route from a plugin. Fastify installs the SDK's `onRoute` hook when it flushes its plugin list, which is after root-level routes are in place. A root-level route therefore produces no route handler span, and the test never reached that code path. Also: correct `REQUEST_HANDLER_SPAN_NAME_FALLBACK`: the conventions spell the fallback `Request handler`, and its `@see` link pointed at the resource section. closes #23533 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mastra is exporter-based, so we hook the constructor, register a Sentry exporter, and bootstrap an observability pipeline when the app has not configured one.
9f48b17 to
a45a5db
Compare
- Stop calling `_INTERNAL_skipAiProviderWrapping`. That was copied from the LangChain exporter, where `@langchain/openai` and `@langchain/anthropic` depend on the `openai` / `@anthropic-ai/sdk` clients, so LangChain really does double-instrument. `@mastra/core` reaches providers through the fetch-based `@ai-sdk/*` packages and never imports those SDKs, so there was no duplicate span to suppress — only the app's own direct SDK calls were being dropped, permanently and process-wide, from the first Mastra event on. - Store a parentless dropped span under its own id instead of `''`. `LRUMap.remove` only deletes truthy values, so the empty-string sentinel was never evicted; those entries accumulated to the 1000-entry cap and pushed out live dropped-span -> parent mappings, breaking re-parenting. - Emit `gen_ai.request.stop_sequences` as a list rather than a JSON string, to match the `string[]` type in sentry-conventions and the Bedrock integration. `gen_ai.response.finish_reasons` stays string-encoded: it is typed `string` and the OpenAI integration encodes it the same way. - Replace the per-span `keys().includes()` scan in `_trackSpan` with a `size` check, and end the previous span on a duplicate `span_started` — `LRUMap.set` evicts the oldest entry even when overwriting an existing key, which dropped a span without ending it. - Gate `isExportedSpanType` on `hasOwnProperty` so span types named `constructor` or `toString` are not treated as exported. - Widen the orchestrion `filePath` to any `mastra*` chunk under `dist/` so a chunk rename inside the supported version range does not silently disable instrumentation. Matches the same two files in 1.63.2. - Revert the unrelated `.unordered()` added to the Cloudflare Workflow test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J1cK2XXFg4tCaQYQ5yJjND
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1c544d9. Configure here.
|
I know this looks big, but the reviewable part is not. most of the diff is Node integration tests and a @mastra/* yarn.lock bump. The product decisions are in three files listed in the description, start with those and the rest should fall into place 
|
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
Quite some big stuff. Added some comments
|
|
||
| public readonly [MASTRA_EXPORTER_BRAND] = true; | ||
|
|
||
| private readonly _spans = new LRUMap<string, TrackedSpan>(MAX_TRACKED_MASTRA_SPANS); |
There was a problem hiding this comment.
m: Do we really want to store the spans in an LRUMap? This would mean that we might just drop random spans right? I could imagine that some child span has then no parent at all, as this might be dropped already
There was a problem hiding this comment.
Yah the cap is intentional (prisma does something similar) so a span that never ends cannot leak. dropping is oldest-first so it is not really random, and we end() the span before dropping it. a later child uses getActiveSpan() (the request span if one is active) or becomes a new root, it doesn't keep a dangling parent id
| } | ||
| } | ||
|
|
||
| private _onSpanStarted(span: MastraExportedSpan): void { |
There was a problem hiding this comment.
super-l: Maybe we can already start using private class methods instead in newer functions? Then we would already modernize our codebase a bit
| private _onSpanStarted(span: MastraExportedSpan): void { | |
| #onSpanStarted(span: MastraExportedSpan): void { |
There was a problem hiding this comment.
tbh i had this first and switched it, almost everything in server-utils (and the SDK generally) uses private, i wouldn't wanna introduce any new repo-wide convention in such a big PR
| - `DenoMongoose` => `Mongoose` | ||
| - `DenoMysql` => `Mysql` | ||
| - `DenoPostgres` => `Postgres` | ||
| - feat(node): Add first-party Mastra integration ([#23823](https://github.com/getsentry/sentry-javascript/pull/23823)). Enabled by default; disable with `defaultIntegrations: integrations => integrations.filter(i => i.name !== 'Mastra')`. |
There was a problem hiding this comment.
m: Can we enable it by default? It seems Mastra 1.63.2 has an engine of 22.13+: https://github.com/mastra-ai/mastra/blob/003e75745c5fd6a7af8464ece1d2930f81dd15af/packages/core/package.json#L987
Maybe it wouldn't matter as users can't use 1.63.2 anyways when they're not on the supported version and we don't import it in our integration anyways. Just wanted to double check
There was a problem hiding this comment.
There was a problem hiding this comment.
yah It's already on by default (in getTracingIntegrations(), same as the other AI integrations). we never import @mastra/core, and if registerExporter is missing we just skip
|
|
||
| async function run() { | ||
| // Stub of `@mastra/sentry`: same `name: 'sentry'`, no brand. The real package calls `Sentry.init()`. | ||
| const communityExporter = { |
There was a problem hiding this comment.
The current @mastra/sentry exporter calls Sentry.init() and would replace the test client. The stub is only the name: 'sentry' signal so we hit the “community exporter already registered” path and still assert our spans go out
| const mastra = instance as MastraInstance; | ||
| if (typeof mastra.registerExporter !== 'function') { | ||
| DEBUG_BUILD && | ||
| debug.log('[instrumentation:mastra] `Mastra.registerExporter` is missing; needs @mastra/core >= 1.63.2'); |
There was a problem hiding this comment.
note: That maybe proofs https://github.com/getsentry/sentry-javascript/pull/23823/changes#r3913763644 is ok
| } catch { | ||
| // cwd is not the app, or `@mastra/core` is ESM-only and not in the CJS resolver. | ||
| } | ||
| parents.add(join(process.cwd(), 'noop.js')); |
There was a problem hiding this comment.
It's a dummy filename, nothing ever loads it, createRequire treats its argument as a filename and resolves from dirname(that), so passing cwd itself would look in cwd's parent. I'll add a comment so that's obvious.
Use the shared channel helper, document why createRequire needs a dummy filename, and drop redundant Sentry.flush from scenarios.
2 participants
Mastra agent traces now come from the SDK:
invoke_agent,chat, andexecute_toolspans, on by default for Node-family SDKs.Ref JS-3455 (dup JSSDK-21).
How it attaches
Mastraconstructor and registers our exporter viaregisterExporter().@mastra/observabilitywhen that package is already installed. We never add it ourselves.What we emit
workflow_step,memory_operation,processor_run,scorer_*) are dropped; children re-parent onto the nearest exported ancestor.model_inferenceis dropped too. Mastra nestsmodel_generation > model_step > model_inference, and the inference span repeats the generation’s model/usage — exporting both produced a duplicate nestedchatper step.model_generationis thegen_ai.chatspan.Note to reviewer: 🗯️
This diff is large mostly because of Node integration tests and the @mastra/* yarn.lock bump, not because the SDK surface is huge.
The product decisions are in three files:
If you are checking that auto-instrumentation actually hooks Mastra, also glance at packages/server-utils/src/orchestrion/config/mastra.ts