feat(nextjs): Add opt-in for orchestrion instrumentation#22043
Draft
chargome wants to merge 8 commits into
Draft
feat(nextjs): Add opt-in for orchestrion instrumentation#22043chargome wants to merge 8 commits into
chargome wants to merge 8 commits into
Conversation
Two fixes for diagnostics-channel injection under Turbopack:
Externalize @apm-js-collab/tracing-hooks and @apm-js-collab/code-transformer
when the flag is on. Bundled, the code transformer's parser breaks
('a.parse is not a function'), so the runtime module hook silently returned
untransformed sources and externalized packages never produced spans.
Keep mysql in serverExternalPackages instead of force-bundling it. Turbopack
cannot bundle mysql 2.x correctly (the wire-protocol handshake fails with
'Received packet in the wrong sequence' even untransformed). External, it is
now instrumented by the working runtime hook instead of the build-time loader.
Also drop the bundler marker from the orchestrion webpack plugin: it made
registerDiagnosticsChannelInjection() skip the runtime hook, but the hybrid
setup (loader for bundled deps, runtime hook for external ones) needs both.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nto cg/nextjs-orchestrion-support
Instead of un-externalizing every instrumented package and forcing the Next-default-external ones through transpilePackages, only packages on an explicit bundle-safe allowlist (currently ioredis) are removed from Sentry's own serverExternalPackages defaults. Everything else — Next's defaults, the user's externals, the rest of Sentry's defaults — stays external and is instrumented by the runtime module hook on require, which works since the orchestrion machinery is externalized. This is safer: bundling a server package changes real behavior (mysql 2.x corrupts its wire protocol when bundled by Turbopack), and new upstream instrumentations (e.g. hapi) now default to the external/runtime-hook path instead of silently becoming bundled. It also removes the Next server-external-packages list parsing and the pg-native webpack workaround, both only needed to support force-bundling. Verified in the nextjs-16-orchestrion e2e: ioredis via the build-time loader, pg and mysql via the runtime hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t@13 Use createRequire(__filename) instead of aliasing the CJS require in the orchestrion webpack bundler module. Next.js 13 bundles @sentry/nextjs into the server build, and webpack flags the aliased require with 'Critical dependency: require function is used in a way in which dependencies cannot be statically extracted', which the nextjs-app-dir e2e treats as a failure. Add isDiagnosticsChannelInjectionEnabled to the consistent-exports ignore list: like its companions experimentalUseDiagnosticsChannelInjection and diagnosticsChannelInjectionIntegrations, the Node-runtime-only opt-in is not surfaced through the framework / serverless SDKs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The plugin previously ran for all server compilations, including the edge runtime, which diagnostics-channel injection does not target. Gate it on the already-derived runtime instead of isServer and add unit tests covering the server/edge/client/flag-off cases. Also trim down the orchestrion-related comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a3a3595. Configure here.
| '[@sentry/nextjs] `useDiagnosticsChannelInjection` is enabled in `withSentryConfig`, but ' + | ||
| '`Sentry.experimentalUseDiagnosticsChannelInjection()` was not called before `Sentry.init()`. ' + | ||
| 'Server DB spans will not be recorded.', | ||
| ); |
There was a problem hiding this comment.
Init warning hidden in production
Medium Severity
The new check for a missing experimentalUseDiagnosticsChannelInjection() call uses DEBUG_BUILD && debug.warn, so production builds with only useDiagnosticsChannelInjection in withSentryConfig get no visible warning while server DB spans stay disabled.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit a3a3595. Configure here.
| expect(externals).toContain('mysql'); | ||
| expect(externals).not.toContain('@apm-js-collab/tracing-hooks'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Feat lacks integration or E2E
Medium Severity
This feature PR adds unit and webpack config tests but no integration or E2E test in the diff, though the description references separate e2e verification. Review guidelines expect at least one integration or E2E test for feat changes.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit a3a3595. Configure here.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adds an experimental option for opting in to orchestrion instrumentation.
transpilePackagesfor the ones that Next externalizes by default (e.g.pg) — removing them from serverExternalPackages isn't enough for Next's own defaults.Verified in e2e, which I will stack on top of this pr
closes #22009