From 7b07ab970f44103f86fabbe6aca6ea0edc913ac9 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 25 Nov 2024 12:44:13 -0500 Subject: [PATCH] ref(profiling) test local require --- .../node-profiling/index.mjs | 21 +++++++++++ .../node-profiling/package.json | 2 +- packages/profiling-node/rollup.npm.config.mjs | 35 ++++++++++--------- packages/profiling-node/src/cpu_profiler.ts | 10 ++++-- 4 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-profiling/index.mjs diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/index.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/index.mjs new file mode 100644 index 000000000000..ca155bd9d670 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-profiling/index.mjs @@ -0,0 +1,21 @@ +import * as Sentry from '@sentry/node'; +import { nodeProfilingIntegration } from '@sentry/profiling-node'; + +const wait = ms => new Promise(resolve => setTimeout(resolve, ms)); + +Sentry.init({ + dsn: 'https://7fa19397baaf433f919fbe02228d5470@o1137848.ingest.sentry.io/6625302', + integrations: [nodeProfilingIntegration()], + tracesSampleRate: 1.0, + profilesSampleRate: 1.0, +}); + +Sentry.startSpan({ name: 'Precompile test' }, async () => { + await wait(500); +}); + +// Test that globalThis.require is not defined by any side effects of the profiling +// https://github.com/getsentry/sentry-javascript/issues/13662 +if (globalThis.require !== undefined) { + throw new Error('globalThis.require should not be defined, check that profiling integration is not defining it, received: ' + typeof globalThis.require); +} diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/package.json b/dev-packages/e2e-tests/test-applications/node-profiling/package.json index 8aede827a1f3..d61b9cad3202 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -5,7 +5,7 @@ "scripts": { "typecheck": "tsc --noEmit", "build": "node build.mjs && node build.shimmed.mjs", - "test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs", + "test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs && node index.mjs", "clean": "npx rimraf node_modules dist", "test:electron": "$(pnpm bin)/electron-rebuild && playwright test", "test:build": "pnpm run typecheck && pnpm run build", diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index 12492b7c83e8..f82eff8a8493 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -1,28 +1,27 @@ import commonjs from '@rollup/plugin-commonjs'; import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; -export const ESMShim = ` -import cjsUrl from 'node:url'; -import cjsPath from 'node:path'; +export const ESMImportShim = ` import cjsModule from 'node:module'; +`; -if(typeof __filename === 'undefined'){ - globalThis.__filename = cjsUrl.fileURLToPath(import.meta.url); -} +const ESMRequireShim = ` +const require = cjsModule.createRequire(import.meta.url); +` -if(typeof __dirname === 'undefined'){ - globalThis.__dirname = cjsPath.dirname(__filename); -} - -if(typeof require === 'undefined'){ - globalThis.require = cjsModule.createRequire(import.meta.url); +function makeESMImportShimPlugin(shim) { + return { + transform(code) { + const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_IMPORT_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_IMPORT_SHIM/; + return code.replace(SHIM_REGEXP, shim); + }, + }; } -`; -function makeESMShimPlugin(shim) { +function makeESMRequireShimPlugin(shim){ return { transform(code) { - const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/; + const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_REQUIRE_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_REQUIRE_SHIM/; return code.replace(SHIM_REGEXP, shim); }, }; @@ -39,10 +38,12 @@ const variants = makeNPMConfigVariants( for (const variant of variants) { if (variant.output.format === 'esm') { - variant.plugins.push(makeESMShimPlugin(ESMShim)); + variant.plugins.push(makeESMImportShimPlugin(ESMImportShim)); + variant.plugins.push(makeESMRequireShimPlugin(ESMRequireShim)) } else { // Remove the ESM shim comment - variant.plugins.push(makeESMShimPlugin('')); + variant.plugins.push(makeESMImportShimPlugin('')); + variant.plugins.push(makeESMRequireShimPlugin('')); } } diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index 76fad9a286a6..6104b54ab5bb 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -15,11 +15,11 @@ import type { } from './types'; import type { ProfileFormat } from './types'; -// #START_SENTRY_ESM_SHIM +// #START_SENTRY_ESM_IMPORT_SHIM // When building for ESM, we shim require to use createRequire and __dirname. // We need to do this because .node extensions in esm are not supported. // The comment below this line exists as a placeholder for where to insert the shim. -// #END_SENTRY_ESM_SHIM +// #END_SENTRY_ESM_IMPORT_SHIM const stdlib = familySync(); const platform = process.env['BUILD_PLATFORM'] || _platform(); @@ -34,6 +34,12 @@ const built_from_source_path = resolve(__dirname, '..', `./sentry_cpu_profiler-$ */ // eslint-disable-next-line complexity export function importCppBindingsModule(): PrivateV8CpuProfilerBindings { + // #START_SENTRY_ESM_REQUIRE_SHIM + // When building for ESM, we shim require to use createRequire and __dirname. + // We need to do this because .node extensions in esm are not supported. + // The comment below this line exists as a placeholder for where to insert the shim. + // #END_SENTRY_ESM_REQUIRE_SHIM + // If a binary path is specified, use that. if (env['SENTRY_PROFILER_BINARY_PATH']) { const envPath = env['SENTRY_PROFILER_BINARY_PATH'];