From 0e4ef7225a16b63a1b02715f03b63e186b9bb940 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 17 Jan 2025 15:01:35 +0100 Subject: [PATCH 1/3] fix(profiling): Make esbuild load node binaries --- dev-packages/rollup-utils/npmHelpers.mjs | 8 ++- packages/profiling-node/rollup.npm.config.mjs | 55 ++++++++++----- packages/profiling-node/src/cpu_profiler.ts | 70 ++++++++++--------- 3 files changed, 82 insertions(+), 51 deletions(-) diff --git a/dev-packages/rollup-utils/npmHelpers.mjs b/dev-packages/rollup-utils/npmHelpers.mjs index e34b515f0538..cf9b723599dd 100644 --- a/dev-packages/rollup-utils/npmHelpers.mjs +++ b/dev-packages/rollup-utils/npmHelpers.mjs @@ -110,9 +110,13 @@ export function makeBaseNPMConfig(options = {}) { } export function makeNPMConfigVariants(baseConfig, options = {}) { - const { emitEsm = true } = options; + const { emitEsm = true, emitCjs = true } = options; - const variantSpecificConfigs = [{ output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs') } }]; + const variantSpecificConfigs = []; + + if (emitCjs) { + variantSpecificConfigs.push({ output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs') } }); + } if (emitEsm) { variantSpecificConfigs.push({ diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index a9c148306709..df391addbec7 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -2,19 +2,42 @@ import commonjs from '@rollup/plugin-commonjs'; import replace from '@rollup/plugin-replace'; import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; -export default makeNPMConfigVariants( - makeBaseNPMConfig({ - packageSpecificConfig: { - output: { dir: 'lib', preserveModules: false }, - plugins: [ - commonjs(), - replace({ - preventAssignment: false, - values: { - __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', - }, - }), - ], - }, - }), -); +export default [ + ...makeNPMConfigVariants( + makeBaseNPMConfig({ + packageSpecificConfig: { + output: { dir: 'lib', preserveModules: false }, + plugins: [ + commonjs(), + replace({ + preventAssignment: false, + values: { + __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', + __CREATE_REQUIRE__: '', + __LOAD_MODULE_REPLACEMENT__: 'require', + }, + }), + ], + }, + }), + { emitEsm: false }, + ), + ...makeNPMConfigVariants( + makeBaseNPMConfig({ + packageSpecificConfig: { + output: { dir: 'lib', preserveModules: false }, + plugins: [ + replace({ + preventAssignment: false, + values: { + __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', + __CREATE_REQUIRE__: 'const require = createRequire(import.meta.url);', + __LOAD_MODULE_REPLACEMENT__: 'import', // TODO: this somehow only builds when using "require" (but it's ESM?) + }, + }), + ], + }, + }), + { emitCjs: false }, + ), +]; diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index a9a6d65ce191..a2b9dfcf38d6 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -1,4 +1,3 @@ -import { createRequire } from 'node:module'; import { arch as _arch, platform as _platform } from 'node:os'; import { join, resolve } from 'node:path'; import { dirname } from 'node:path'; @@ -19,6 +18,8 @@ import type { import type { ProfileFormat } from './types'; declare const __IMPORT_META_URL_REPLACEMENT__: string; +declare const __LOAD_MODULE_REPLACEMENT__: (path: string) => PrivateV8CpuProfilerBindings; +declare const __CREATE_REQUIRE__: string; const stdlib = familySync(); const platform = process.env['BUILD_PLATFORM'] || _platform(); @@ -39,19 +40,22 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { : // This case is hit when the tests are run pathToFileURL(__filename).href; - const createdRequire = createRequire(importMetaUrl); + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + // __CREATE_REQUIRE__; + + // const createdRequire = createRequire(importMetaUrl); const esmCompatibleDirname = dirname(fileURLToPath(importMetaUrl)); // If a binary path is specified, use that. if (env['SENTRY_PROFILER_BINARY_PATH']) { const envPath = env['SENTRY_PROFILER_BINARY_PATH']; - return createdRequire(envPath); + return __LOAD_MODULE_REPLACEMENT__(envPath); } // If a user specifies a different binary dir, they are in control of the binaries being moved there if (env['SENTRY_PROFILER_BINARY_DIR']) { const binaryPath = join(resolve(env['SENTRY_PROFILER_BINARY_DIR']), `sentry_cpu_profiler-${identifier}`); - return createdRequire(`${binaryPath}.node`); + return __LOAD_MODULE_REPLACEMENT__(`${binaryPath}.node`); } // We need the fallthrough so that in the end, we can fallback to the dynamic require. @@ -59,31 +63,31 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (platform === 'darwin') { if (arch === 'x64') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-darwin-x64-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-x64-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-darwin-x64-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-x64-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-darwin-x64-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-x64-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-darwin-x64-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-x64-127.node'); } } if (arch === 'arm64') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-darwin-arm64-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-arm64-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-darwin-arm64-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-arm64-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-darwin-arm64-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-arm64-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-darwin-arm64-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-darwin-arm64-127.node'); } } } @@ -91,16 +95,16 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (platform === 'win32') { if (arch === 'x64') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-win32-x64-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-win32-x64-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-win32-x64-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-win32-x64-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-win32-x64-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-win32-x64-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-win32-x64-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-win32-x64-127.node'); } } } @@ -109,68 +113,68 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { if (arch === 'x64') { if (stdlib === 'musl') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-linux-x64-musl-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-musl-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-linux-x64-musl-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-musl-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-linux-x64-musl-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-musl-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-linux-x64-musl-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-musl-127.node'); } } if (stdlib === 'glibc') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-glibc-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-glibc-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-glibc-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-x64-glibc-127.node'); } } } if (arch === 'arm64') { if (stdlib === 'musl') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-musl-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-musl-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-musl-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-musl-127.node'); } } if (stdlib === 'glibc') { if (abi === '93') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-93.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-glibc-93.node'); } if (abi === '108') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-108.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-glibc-108.node'); } if (abi === '115') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-115.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-glibc-115.node'); } if (abi === '127') { - return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-127.node'); + return __LOAD_MODULE_REPLACEMENT__('../sentry_cpu_profiler-linux-arm64-glibc-127.node'); } } } } const built_from_source_path = resolve(esmCompatibleDirname, '..', `sentry_cpu_profiler-${identifier}`); - return createdRequire(`${built_from_source_path}.node`); + return __LOAD_MODULE_REPLACEMENT__(`${built_from_source_path}.node`); } const PrivateCpuProfilerBindings: PrivateV8CpuProfilerBindings = importCppBindingsModule(); From 54cafe582972237be68eb94f6b136c2cf80171cd Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 17 Jan 2025 15:55:20 +0100 Subject: [PATCH 2/3] skip loading node binaries --- packages/profiling-node/rollup.npm.config.mjs | 19 ++++++++++++++++--- packages/profiling-node/src/cpu_profiler.ts | 4 ---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index df391addbec7..c84bc5a25ed5 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -2,6 +2,19 @@ import commonjs from '@rollup/plugin-commonjs'; import replace from '@rollup/plugin-replace'; import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; +/** + * Skip resolving dynamic imports of node binaries (effectively marking it as external) + */ +const nodeBinariesLoader = () => ({ + name: 'node-binaries-loader', + resolveDynamicImport(specifier) { + if (typeof specifier === 'string' && specifier.includes('sentry_cpu_profiler') && specifier.endsWith('.node')) { + return false; + } + return null; // defer to other resolvers + }, +}); + export default [ ...makeNPMConfigVariants( makeBaseNPMConfig({ @@ -13,7 +26,6 @@ export default [ preventAssignment: false, values: { __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', - __CREATE_REQUIRE__: '', __LOAD_MODULE_REPLACEMENT__: 'require', }, }), @@ -27,12 +39,13 @@ export default [ packageSpecificConfig: { output: { dir: 'lib', preserveModules: false }, plugins: [ + nodeBinariesLoader(), + commonjs(), replace({ preventAssignment: false, values: { __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', - __CREATE_REQUIRE__: 'const require = createRequire(import.meta.url);', - __LOAD_MODULE_REPLACEMENT__: 'import', // TODO: this somehow only builds when using "require" (but it's ESM?) + __LOAD_MODULE_REPLACEMENT__: 'import', }, }), ], diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index a2b9dfcf38d6..ff2eaf87fe11 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -40,10 +40,6 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { : // This case is hit when the tests are run pathToFileURL(__filename).href; - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - // __CREATE_REQUIRE__; - - // const createdRequire = createRequire(importMetaUrl); const esmCompatibleDirname = dirname(fileURLToPath(importMetaUrl)); // If a binary path is specified, use that. From 882d2a43e01317a5f13a19e11e73c7470a96e43b Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 22 Jan 2025 09:43:38 +0100 Subject: [PATCH 3/3] replace require variable --- packages/profiling-node/rollup.npm.config.mjs | 4 +++- packages/profiling-node/src/cpu_profiler.ts | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index c84bc5a25ed5..7a4705915fd9 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -26,6 +26,7 @@ export default [ preventAssignment: false, values: { __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', + __CREATE_REQUIRE__: '', // empty, so the native CJS `require` can be used without getting renamed during build (because of a new assignment) __LOAD_MODULE_REPLACEMENT__: 'require', }, }), @@ -45,7 +46,8 @@ export default [ preventAssignment: false, values: { __IMPORT_META_URL_REPLACEMENT__: 'import.meta.url', - __LOAD_MODULE_REPLACEMENT__: 'import', + __CREATE_REQUIRE__: 'const require = createRequire(importMetaUrl);', // Variable assignment for the require replacement below + __LOAD_MODULE_REPLACEMENT__: 'require', }, }), ], diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index ff2eaf87fe11..6c7350d6cd83 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -18,8 +18,8 @@ import type { import type { ProfileFormat } from './types'; declare const __IMPORT_META_URL_REPLACEMENT__: string; -declare const __LOAD_MODULE_REPLACEMENT__: (path: string) => PrivateV8CpuProfilerBindings; declare const __CREATE_REQUIRE__: string; +declare const __LOAD_MODULE_REPLACEMENT__: (path: string) => PrivateV8CpuProfilerBindings; const stdlib = familySync(); const platform = process.env['BUILD_PLATFORM'] || _platform(); @@ -40,6 +40,10 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { : // This case is hit when the tests are run pathToFileURL(__filename).href; + // During build-time in ESM, this is replaced with an assignment of `require`. In CJS, this line is empty, so we can use the native require. + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + __CREATE_REQUIRE__; + const esmCompatibleDirname = dirname(fileURLToPath(importMetaUrl)); // If a binary path is specified, use that.