-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(server-runtime-injection): Split runtime orchestrion injection into a dedicated package #23685
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
1797686
77b5375
34084c7
14c8a99
a3e6fe2
f94b60f
4140c0a
e1f15a7
da0de0d
7bd9c46
2d039c7
39428c8
8f2f48b
fe6c45b
2a2a5e3
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 |
|---|---|---|
| @@ -1 +1 @@ | ||
| import '@sentry/server-utils/orchestrion/import-hook'; | ||
| import '@sentry/server-runtime-injection/import-hook'; |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,28 +24,28 @@ fs.readdirSync(esmTemplateDir).forEach(templateFile => | |||||||
| ); | ||||||||
|
|
||||||||
| // Generate the orchestrion runtime forwarders (see `src/config/diagnosticsChannelInjection.ts`) | ||||||||
| // from `@sentry/server-utils`' own exports map, so a new subpath there is forwarded automatically. | ||||||||
| // Only `require`-able entries get one, since the emitted external is a `require()`. Written as | ||||||||
| // plain CJS, not built by rollup: they are loaded by specifier, never bundled. | ||||||||
| const SERVER_UTILS = '@sentry/server-utils'; | ||||||||
| // from `@sentry/server-runtime-injection`' own exports map, so a new subpath there is forwarded | ||||||||
| // automatically. Only `require`-able entries get one, since the emitted external is a `require()`. | ||||||||
| // Written as plain CJS, not built by rollup: they are loaded by specifier, never bundled. | ||||||||
| const RUNTIME_INJECTION = '@sentry/server-runtime-injection'; | ||||||||
| const orchestrionRuntimeBuildDir = 'build/orchestrion-runtime'; | ||||||||
|
|
||||||||
| const serverUtilsExports = ( | ||||||||
| JSON.parse(fs.readFileSync(require.resolve(`${SERVER_UTILS}/package.json`), 'utf8')) as { | ||||||||
| const runtimeInjectionExports = ( | ||||||||
| JSON.parse(fs.readFileSync(require.resolve(`${RUNTIME_INJECTION}/package.json`), 'utf8')) as { | ||||||||
| exports: Record<string, { require?: string } | string>; | ||||||||
| } | ||||||||
| ).exports; | ||||||||
|
|
||||||||
| for (const [key, conditions] of Object.entries(serverUtilsExports)) { | ||||||||
| for (const [key, conditions] of Object.entries(runtimeInjectionExports)) { | ||||||||
|
Member
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. This loop writes to the folder, but doesn't clean up anything. It's a low-probability issue, but if the set ever changes, stale build artifacts could creep in. Consider adding removing the folder before starting the loop:
Suggested change
|
||||||||
| if (key === './package.json' || typeof conditions === 'string' || !conditions.require) { | ||||||||
| continue; | ||||||||
| } | ||||||||
|
|
||||||||
| // '.' → 'index', './orchestrion/register' → 'orchestrion/register' | ||||||||
| // '.' → 'index', './register' → 'register' | ||||||||
| const forwarderPath = path.join(orchestrionRuntimeBuildDir, `${key === '.' ? 'index' : key.slice(2)}.js`); | ||||||||
| fs.mkdirSync(path.dirname(forwarderPath), { recursive: true }); | ||||||||
| fs.writeFileSync( | ||||||||
| forwarderPath, | ||||||||
| `// Generated by scripts/buildRollup.ts — do not edit.\nmodule.exports = require('${SERVER_UTILS}${key.slice(1)}');\n`, | ||||||||
| `// Generated by scripts/buildRollup.ts — do not edit.\nmodule.exports = require('${RUNTIME_INJECTION}${key.slice(1)}');\n`, | ||||||||
| ); | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { resolveOrchestrionRuntimeRequest } from '@sentry/server-utils/orchestrion/webpack'; | ||
| import { createRequire } from 'node:module'; | ||
|
|
||
| /** | ||
| * Instrumented packages verified (via e2e) to bundle correctly, removed from Sentry's own | ||
|
|
@@ -9,17 +9,38 @@ import { resolveOrchestrionRuntimeRequest } from '@sentry/server-utils/orchestri | |
| export const BUNDLE_SAFE_INSTRUMENTED_PACKAGES = ['ioredis']; | ||
|
|
||
| /** | ||
| * `@sentry/server-utils` (where `register.ts` and the bundled orchestrion runtime ship) must stay | ||
| * external: `register.ts` passes its own `__filename`/`import.meta.url` as the `parentURL` for | ||
| * `Module.register('@sentry/server-utils/orchestrion/hook.mjs', …)`, so that self-reference only | ||
| * `@sentry/server-runtime-injection` (where `register.ts` and the bundled orchestrion runtime ship) | ||
| * must stay external: `register.ts` passes its own `__filename`/`import.meta.url` as the `parentURL` | ||
| * for `Module.register('@sentry/server-runtime-injection/hook', …)`, so that self-reference only | ||
| * resolves while the code still lives at its real `node_modules` location. Bundled into an app | ||
| * server chunk instead, the specifier would have to resolve from the chunk's output location, | ||
| * which fails under isolated installs (pnpm) where the package is a transitive dependency. | ||
| * | ||
| * (The `@apm-js-collab/*` packages no longer appear here: they are bundled into | ||
| * `@sentry/server-utils`' build, so no import of them exists at runtime.) | ||
| * `@sentry/server-utils` (the barrel + bundler plugins) is NOT here — it is meant to be bundled; the | ||
| * build-time snippet's `@sentry/server-utils` import is handled separately by the code-transform. | ||
| */ | ||
| export const ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES = ['@sentry/server-utils']; | ||
| export const ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES = ['@sentry/server-runtime-injection']; | ||
|
Member
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. I understand why this is changing, but it does have one interesting ramification. The orchestrion config now gets loaded twice; once from the server chunk, and once from |
||
|
|
||
| // `require` anchored at THIS package (`@sentry/nextjs`), which depends on | ||
| // `@sentry/server-runtime-injection` — so the resolvability check below works even under isolated | ||
| // installs (pnpm), where a resolver anchored at `@sentry/server-utils` could not see it. | ||
| let nextjsRequire: NodeJS.Require; | ||
| /*! rollup-include-cjs-only */ | ||
| nextjsRequire = createRequire(__filename); | ||
| /*! rollup-include-cjs-only-end */ | ||
| /*! rollup-include-esm-only */ | ||
| nextjsRequire = createRequire(import.meta.url); | ||
| /*! rollup-include-esm-only-end */ | ||
|
|
||
| /** Whether `request` resolves as a `require`-able module (skips ESM-only subpaths like `/hook`). */ | ||
| function isRequireResolvable(request: string): boolean { | ||
| try { | ||
| nextjsRequire.resolve(request); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** Remove the given packages from a `serverExternalPackages` list. */ | ||
| export function filterInstrumentedExternals(externals: string[], packagesToBundle: string[]): string[] { | ||
|
|
@@ -28,7 +49,7 @@ export function filterInstrumentedExternals(externals: string[], packagesToBundl | |
| } | ||
|
|
||
| /** | ||
| * Where the generated forwarders live — one CJS one-liner per `@sentry/server-utils` entrypoint | ||
| * Where the generated forwarders live — one CJS one-liner per `@sentry/server-runtime-injection` entrypoint | ||
| * (see `scripts/buildRollup.ts`). Forwarding through `@sentry/nextjs`, always a direct dependency, | ||
| * is what makes the emitted specifier both resolvable from `.next/server/**` and relocation-safe. | ||
| */ | ||
|
|
@@ -67,7 +88,7 @@ export async function externalizeOrchestrionRuntimePackages({ | |
| } | ||
|
|
||
| // Not `require`-able (ESM-only subpath, or a typo): webpack reports it better than we can. | ||
| if (!resolveOrchestrionRuntimeRequest(request)) { | ||
| if (!isRequireResolvable(request)) { | ||
| return undefined; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.