-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(nextjs): Add opt-in for orchestrion instrumentation #22043
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
Changes from all commits
e908fcb
49fe3ad
8e64adb
608f593
5ae940d
f068a03
dcf5b0b
a3a3595
5820b35
027bac6
7982e11
0a6439d
48a8e7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /** | ||
| * Instrumented packages verified (via e2e) to bundle correctly, removed from Sentry's own | ||
| * `serverExternalPackages` defaults so the build-time loader can transform them. Everything else | ||
| * stays external and is instrumented by the runtime module hook instead. Deliberately an | ||
| * allowlist — bundling can break packages outright (e.g. `mysql` 2.x under Turbopack). | ||
| */ | ||
| export const BUNDLE_SAFE_INSTRUMENTED_PACKAGES = ['ioredis']; | ||
|
|
||
| /** | ||
| * The orchestrion runtime machinery must stay external — its parser breaks when bundled, which | ||
| * silently disables the runtime module hook. | ||
| */ | ||
| export const ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES = [ | ||
| '@apm-js-collab/tracing-hooks', | ||
| '@apm-js-collab/code-transformer', | ||
| ]; | ||
|
|
||
| /** Remove the given packages from a `serverExternalPackages` list. */ | ||
| export function filterInstrumentedExternals(externals: string[], packagesToBundle: string[]): string[] { | ||
| const set = new Set(packagesToBundle); | ||
| return externals.filter(name => !set.has(name)); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| BUNDLE_SAFE_INSTRUMENTED_PACKAGES, | ||
| filterInstrumentedExternals, | ||
| } from '../../src/config/diagnosticsChannelInjection'; | ||
| import { setUpBuildTimeVariables } from '../../src/config/withSentryConfig/buildTime'; | ||
| import { getServerExternalPackagesPatch } from '../../src/config/withSentryConfig/getFinalConfigObjectBundlerUtils'; | ||
| import type { NextConfigObject } from '../../src/config/types'; | ||
|
|
||
| describe('filterInstrumentedExternals', () => { | ||
| it('removes the given packages, keeps the rest', () => { | ||
| expect( | ||
| filterInstrumentedExternals(['express', 'pg', 'pg-pool', 'ioredis', 'mongodb'], ['pg', 'pg-pool', 'ioredis']), | ||
| ).toEqual(['express', 'mongodb']); | ||
| }); | ||
|
|
||
| it('is a no-op with an empty bundle list', () => { | ||
| expect(filterInstrumentedExternals(['express', 'pg'], [])).toEqual(['express', 'pg']); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getServerExternalPackagesPatch (diagnostics-channel injection)', () => { | ||
| it('keeps everything external except the bundle-safe allowlist, and adds the runtime machinery', () => { | ||
| const patch = getServerExternalPackagesPatch({}, 16, true); | ||
| const externals = patch.serverExternalPackages ?? []; | ||
|
|
||
| // Only the verified bundle-safe packages leave the external list (→ build-time loader). | ||
| for (const name of BUNDLE_SAFE_INSTRUMENTED_PACKAGES) { | ||
| expect(externals).not.toContain(name); | ||
| } | ||
| // Other instrumented packages stay external (→ runtime module hook). | ||
| expect(externals).toContain('mysql'); | ||
| expect(externals).toContain('pg'); | ||
| expect(externals).toContain('pg-pool'); | ||
| // The orchestrion machinery must be external for the runtime hook to work. | ||
| expect(externals).toContain('@apm-js-collab/tracing-hooks'); | ||
| expect(externals).toContain('@apm-js-collab/code-transformer'); | ||
| }); | ||
|
|
||
| it('respects user-provided externals even for bundle-safe packages', () => { | ||
| const patch = getServerExternalPackagesPatch({ serverExternalPackages: ['ioredis'] }, 16, true); | ||
| expect(patch.serverExternalPackages).toContain('ioredis'); | ||
| }); | ||
|
|
||
| it('is unchanged with the flag off', () => { | ||
| const patch = getServerExternalPackagesPatch({}, 16, false); | ||
| const externals = patch.serverExternalPackages ?? []; | ||
| expect(externals).toContain('ioredis'); | ||
| expect(externals).toContain('mysql'); | ||
| expect(externals).not.toContain('@apm-js-collab/tracing-hooks'); | ||
| }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feat lacks integration or E2EMedium 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 Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit a3a3595. Configure here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handled in #22080 |
||
|
|
||
| describe('setUpBuildTimeVariables (diagnostics-channel injection)', () => { | ||
| it('injects the flag marker and the tracing-hooks location', () => { | ||
| const nextConfig: NextConfigObject = {}; | ||
| setUpBuildTimeVariables(nextConfig, { _experimental: { useDiagnosticsChannelInjection: true } }, undefined); | ||
|
|
||
| expect(nextConfig.env).toMatchObject({ | ||
| _sentryUseDiagnosticsChannelInjection: 'true', | ||
| // The runtime module hook joins subpaths onto this, so it must be an absolute directory. | ||
| _sentryOrchestrionTracingHooksDir: expect.stringMatching(/@apm-js-collab[/+]tracing-hooks/), | ||
| }); | ||
| }); | ||
|
|
||
| it('injects neither with the flag off', () => { | ||
| const nextConfig: NextConfigObject = {}; | ||
| setUpBuildTimeVariables(nextConfig, {}, undefined); | ||
|
|
||
| expect(nextConfig.env).not.toHaveProperty('_sentryUseDiagnosticsChannelInjection'); | ||
| expect(nextConfig.env).not.toHaveProperty('_sentryOrchestrionTracingHooksDir'); | ||
| }); | ||
| }); | ||


Uh oh!
There was an error while loading. Please reload this page.