feat(node): Add getInstrumentedModuleNames() - #24254
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1036b89. Configure here.
size-limit report 📦
|
2b5f1c9 to
3dd340f
Compare
…leNames, exclude eve from browser content - Make the standalone Eve guide extend the Node guide (fallbackGuide: javascript.node) with error-monitoring/tracing/profiling onboarding toggles. - Use getInstrumentedModuleNames() for orchestrion externalDependencies (getsentry/sentry-javascript#24254). - Add javascript.eve to every notSupported list in JS common content wherever javascript.node is excluded, so browser-only content is hidden from the Eve guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaYGdTstWVZKU9X4NjtYV1
| /** | ||
| * The package names the SDK instruments through the orchestrion module transform (its | ||
| * diagnostics-channel injection). Pass these to a server bundler's "keep external" option so the | ||
| * packages load through Node's module loader — the only path the transform can hook — instead of | ||
| * being inlined into the server bundle. A framework that has no Sentry bundler plugin (e.g. eve, via | ||
| * `build.externalDependencies`) is the main caller; a listed package the app doesn't use is simply | ||
| * ignored by the bundler. | ||
| * | ||
| * Unlike {@link INSTRUMENTED_MODULE_NAMES}, this is the plain instrumented set with no bundler-only | ||
| * additions — those force a helper package to be *bundled*, the opposite of keeping it external. | ||
| */ |
There was a problem hiding this comment.
If we expect users to actually use this in a setup I feel we should make this more comprehensible wdyt? Just an idea could we just add a wrapper for defineAgent where we set these things ourselves?
There was a problem hiding this comment.
I discussed this with @RulaKhaled , @andreiborza and @nicohrubec and the general consensus was that it was preferred to do an explicit thing like here - personally I have no strong opinion on this. Although, if we ship this here, nothing stops us from also shipping a define agent wrapper later?
There was a problem hiding this comment.
nothing stops us from also shipping a define agent wrapper later?
We just need to be careful that there are not 10 different setup's to achieve the same thing.
JPeer264
left a comment
There was a problem hiding this comment.
LGTM. That comment is the only point which must be decided by the AI squad. But as long as we have strong docs around it, it's fine I would assume
3dd340f to
59c2f57
Compare
…-eve orchestrion test Exposes `getInstrumentedModuleNames()` — the package names Sentry instruments through the orchestrion module transform — so an app on a framework with no Sentry bundler plugin can keep those packages external and let the transform hook them, instead of hardcoding the list. Lives in `@sentry/server-utils` (where the orchestrion config lives) and is re-exported from `@sentry/node`, so `export *` consumers (nitro — eve's base —, astro, …) surface it too. It returns the plain `module.name` set, deliberately without the bundler-only additions in `INSTRUMENTED_MODULE_NAMES` (those force a helper package to be bundled, the opposite of keeping it external). The node-eve app's orchestrion variant now passes `getInstrumentedModuleNames()` to `build.externalDependencies` instead of a hardcoded `['dataloader']`. The app only uses `dataloader` of that set, so the rest are no-ops; `ai` v7 stays fine externalized (native diagnostics channel + registration-only under the transform). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same footprint as eveConversationHook: added to the explicit re-export blocks of @sentry/bun, @sentry/aws-serverless and @sentry/google-cloud-serverless (from @sentry/node) and of @sentry/deno and @sentry/cloudflare (from @sentry/server-utils), so it is available wherever a server bundle is configured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Astro's runtime entry curates its `@sentry/node` re-exports (it can't `export *`), so it needs the explicit listing like the other SDKs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
59c2f57 to
f46e1f3
Compare

Stacked on #24247 (base branch
feat/eve-conversation-hook).Exposes
getInstrumentedModuleNames()— the package names Sentry instruments through the orchestrion module transform (its diagnostics-channel injection).Why:
build.externalDependencies) otherwise has to hardcode which packages to keep external. An inlined dependency never reaches the transform'sonLoad, so it's silently never instrumented — this hands the app the authoritative set.module.nameset, deliberately without the bundler-only additions in the internalINSTRUMENTED_MODULE_NAMES(e.g.@remix-run/node), which exist to force a helper package to be bundled — the opposite of keeping it external.Export surface: lives in
@sentry/server-utils(home of the orchestrion config). Re-exported from@sentry/node, soexport *consumers — nitro (eve's base), astro, nestjs, hono, effect — surface it automatically; and added explicitly to@sentry/bun,@sentry/aws-serverless,@sentry/google-cloud-serverless(from node) and@sentry/deno,@sentry/cloudflare(from server-utils), i.e. everywhere a server bundle is configured — the same footprint aseveConversationHook.node-eve e2e: the orchestrion variant's
build.externalDependenciesnow usesgetInstrumentedModuleNames()instead of a hardcoded['dataloader']. The app only usesdataloaderof that set, so the others are no-ops;aiv7 stays correct when externalized (nativeai:telemetrychannel, and registration-only under the transform, so no double instrumentation). The existingdataloaderassertion continues to prove the transform still runs.🤖 Generated with Claude Code