Skip to content
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

fix(profiling/v8): Don't put require, __filename and __dirname on global object #14952

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,8 @@ jobs:
- name: Run E2E test
working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }}
timeout-minutes: 10
run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert
run: |
xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" -- yarn test:assert

job_required_jobs_passed:
name: All required jobs passed or were skipped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ console.log('Running build using esbuild version', esbuild.version);
esbuild.buildSync({
platform: 'node',
entryPoints: ['./index.ts'],
outdir: './dist',
outfile: './dist/cjs/index.js',
target: 'esnext',
format: 'cjs',
bundle: true,
loader: { '.node': 'copy' },
external: ['@sentry/node', '@sentry/profiling-node'],
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,10 @@ console.log('Running build using esbuild version', esbuild.version);
esbuild.buildSync({
platform: 'node',
entryPoints: ['./index.ts'],
outfile: './dist/index.shimmed.mjs',
outfile: './dist/esm/index.mjs',
target: 'esnext',
format: 'esm',
bundle: true,
loader: { '.node': 'copy' },
banner: {
js: `
import { dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
`,
},
external: ['@sentry/node', '@sentry/profiling-node'],
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const Sentry = require('@sentry/node');
const { nodeProfilingIntegration } = require('@sentry/profiling-node');
import * as Sentry from '@sentry/node';
import { nodeProfilingIntegration } from '@sentry/profiling-node';

const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"private": true,
"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",
"build": "node build-cjs.mjs && node build-esm.mjs",
"test": "node dist/cjs/index.js && node --experimental-require-module dist/cjs/index.js && node dist/esm/index.mjs",
"clean": "npx rimraf node_modules dist",
"test:electron": "$(pnpm bin)/electron-rebuild && playwright test",
"test:build": "pnpm run typecheck && pnpm run build",
Expand All @@ -17,9 +17,9 @@
"@sentry/electron": "latest || *",
"@sentry/node": "latest || *",
"@sentry/profiling-node": "latest || *",
"electron": "^33.2.0"
"electron": "^33.2.0",
"esbuild": "0.20.0"
},
"devDependencies": {},
"volta": {
"extends": "../../package.json"
},
Expand Down
1 change: 1 addition & 0 deletions packages/profiling-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"author": "Sentry",
"license": "MIT",
"main": "lib/cjs/index.js",
"module": "lib/esm/index.js",
"types": "lib/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
Expand Down
51 changes: 11 additions & 40 deletions packages/profiling-node/rollup.npm.config.mjs
Original file line number Diff line number Diff line change
@@ -1,49 +1,20 @@
import commonjs from '@rollup/plugin-commonjs';
import replace from '@rollup/plugin-replace';
import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';

export const ESMShim = `
import cjsUrl from 'node:url';
import cjsPath from 'node:path';
import cjsModule from 'node:module';

if(typeof __filename === 'undefined'){
globalThis.__filename = cjsUrl.fileURLToPath(import.meta.url);
}

if(typeof __dirname === 'undefined'){
globalThis.__dirname = cjsPath.dirname(__filename);
}

if(typeof require === 'undefined'){
globalThis.require = cjsModule.createRequire(import.meta.url);
}
`;

function makeESMShimPlugin(shim) {
return {
transform(code) {
const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/;
return code.replace(SHIM_REGEXP, shim);
},
};
}

const variants = makeNPMConfigVariants(
export default makeNPMConfigVariants(
makeBaseNPMConfig({
packageSpecificConfig: {
output: { dir: 'lib', preserveModules: false },
plugins: [commonjs()],
plugins: [
commonjs(),
replace({
preventAssignment: false,
values: {
__IMPORT_META_URL_REPLACEMENT__: 'import.meta.url',
},
}),
],
},
}),
);

for (const variant of variants) {
if (variant.output.format === 'esm') {
variant.plugins.push(makeESMShimPlugin(ESMShim));
} else {
// Remove the ESM shim comment
variant.plugins.push(makeESMShimPlugin(''));
}
}

export default variants;
86 changes: 48 additions & 38 deletions packages/profiling-node/src/cpu_profiler.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
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';
import { env, versions } from 'node:process';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { threadId } from 'node:worker_threads';
import { familySync } from 'detect-libc';
import { getAbi } from 'node-abi';
Expand All @@ -15,84 +18,89 @@ import type {
} from './types';
import type { ProfileFormat } from './types';

// #START_SENTRY_ESM_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
declare const __IMPORT_META_URL_REPLACEMENT__: string;

const stdlib = familySync();
const platform = process.env['BUILD_PLATFORM'] || _platform();
const arch = process.env['BUILD_ARCH'] || _arch();
const abi = getAbi(versions.node, 'node');
const identifier = [platform, arch, stdlib, abi].filter(c => c !== undefined && c !== null).join('-');

const built_from_source_path = resolve(__dirname, '..', `./sentry_cpu_profiler-${identifier}`);

/**
* Imports cpp bindings based on the current platform and architecture.
*/
// eslint-disable-next-line complexity
export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
// We need to work around using import.meta.url directly with __IMPORT_META_URL_REPLACEMENT__ because jest complains about it.
const importMetaUrl =
typeof __IMPORT_META_URL_REPLACEMENT__ !== 'undefined'
? // This case is always hit when the SDK is built
__IMPORT_META_URL_REPLACEMENT__
: // This case is hit when the tests are run
pathToFileURL(__filename).href;

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 require(envPath);
return createdRequire(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 require(`${binaryPath}.node`);
return createdRequire(`${binaryPath}.node`);
}

// We need the fallthrough so that in the end, we can fallback to the dynamic require.
// This is for cases where precompiled binaries were not provided, but may have been compiled from source.
if (platform === 'darwin') {
if (arch === 'x64') {
if (abi === '93') {
return require('../sentry_cpu_profiler-darwin-x64-93.node');
return createdRequire('../sentry_cpu_profiler-darwin-x64-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-darwin-x64-108.node');
return createdRequire('../sentry_cpu_profiler-darwin-x64-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-darwin-x64-115.node');
return createdRequire('../sentry_cpu_profiler-darwin-x64-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-darwin-x64-127.node');
return createdRequire('../sentry_cpu_profiler-darwin-x64-127.node');
}
}

if (arch === 'arm64') {
if (abi === '93') {
return require('../sentry_cpu_profiler-darwin-arm64-93.node');
return createdRequire('../sentry_cpu_profiler-darwin-arm64-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-darwin-arm64-108.node');
return createdRequire('../sentry_cpu_profiler-darwin-arm64-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-darwin-arm64-115.node');
return createdRequire('../sentry_cpu_profiler-darwin-arm64-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-darwin-arm64-127.node');
return createdRequire('../sentry_cpu_profiler-darwin-arm64-127.node');
}
}
}

if (platform === 'win32') {
if (arch === 'x64') {
if (abi === '93') {
return require('../sentry_cpu_profiler-win32-x64-93.node');
return createdRequire('../sentry_cpu_profiler-win32-x64-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-win32-x64-108.node');
return createdRequire('../sentry_cpu_profiler-win32-x64-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-win32-x64-115.node');
return createdRequire('../sentry_cpu_profiler-win32-x64-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-win32-x64-127.node');
return createdRequire('../sentry_cpu_profiler-win32-x64-127.node');
}
}
}
Expand All @@ -101,66 +109,68 @@ export function importCppBindingsModule(): PrivateV8CpuProfilerBindings {
if (arch === 'x64') {
if (stdlib === 'musl') {
if (abi === '93') {
return require('../sentry_cpu_profiler-linux-x64-musl-93.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-linux-x64-musl-108.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-linux-x64-musl-115.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-linux-x64-musl-127.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-musl-127.node');
}
}
if (stdlib === 'glibc') {
if (abi === '93') {
return require('../sentry_cpu_profiler-linux-x64-glibc-93.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-linux-x64-glibc-108.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-linux-x64-glibc-115.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-linux-x64-glibc-127.node');
return createdRequire('../sentry_cpu_profiler-linux-x64-glibc-127.node');
}
}
}
if (arch === 'arm64') {
if (stdlib === 'musl') {
if (abi === '93') {
return require('../sentry_cpu_profiler-linux-arm64-musl-93.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-linux-arm64-musl-108.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-linux-arm64-musl-115.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-linux-arm64-musl-127.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-musl-127.node');
}
}

if (stdlib === 'glibc') {
if (abi === '93') {
return require('../sentry_cpu_profiler-linux-arm64-glibc-93.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-93.node');
}
if (abi === '108') {
return require('../sentry_cpu_profiler-linux-arm64-glibc-108.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-108.node');
}
if (abi === '115') {
return require('../sentry_cpu_profiler-linux-arm64-glibc-115.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-115.node');
}
if (abi === '127') {
return require('../sentry_cpu_profiler-linux-arm64-glibc-127.node');
return createdRequire('../sentry_cpu_profiler-linux-arm64-glibc-127.node');
}
}
}
}
return require(`${built_from_source_path}.node`);

const built_from_source_path = resolve(esmCompatibleDirname, '..', `sentry_cpu_profiler-${identifier}`);
return createdRequire(`${built_from_source_path}.node`);
}

const PrivateCpuProfilerBindings: PrivateV8CpuProfilerBindings = importCppBindingsModule();
Expand Down
1 change: 0 additions & 1 deletion packages/profiling-node/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,3 @@
},
"include": ["src/**/*"]
}

Loading