From c3511d67a665d727852e6b152b7cd8f39370093d Mon Sep 17 00:00:00 2001 From: JonasBa Date: Wed, 7 Aug 2024 11:49:32 -0400 Subject: [PATCH 01/10] ref(profiling) conditionally shim cjs globals --- packages/profiling-node/rollup.npm.config.mjs | 47 +++++++++++++++++-- packages/profiling-node/src/cpu_profiler.ts | 6 +++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index 1cc4f0936954..c4fa78e8b00e 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -1,12 +1,53 @@ import commonjs from '@rollup/plugin-commonjs'; -import esmshim from '@rollup/plugin-esm-shim'; import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; -export default makeNPMConfigVariants( +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/; + + console.log('Matched', SHIM_REGEXP.test(code)); + + return code.replace(SHIM_REGEXP, shim); + }, + }; +} + +const variants = makeNPMConfigVariants( makeBaseNPMConfig({ packageSpecificConfig: { output: { dir: 'lib', preserveModules: false }, - plugins: [commonjs(), esmshim()], + plugins: [commonjs()], }, }), ); + +for (const variant of variants) { + if (variant.output.format === 'esm') { + console.log('injected plugin'); + variant.plugins.push(makeESMShimPlugin(ESMShim)); + } else { + // Remove the ESM shim comment + variant.plugins.push(makeESMShimPlugin('')); + } +} + +export default variants; diff --git a/packages/profiling-node/src/cpu_profiler.ts b/packages/profiling-node/src/cpu_profiler.ts index 9ab470e2ca70..fb739a939e77 100644 --- a/packages/profiling-node/src/cpu_profiler.ts +++ b/packages/profiling-node/src/cpu_profiler.ts @@ -15,6 +15,12 @@ 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 + const stdlib = familySync(); const platform = process.env['BUILD_PLATFORM'] || _platform(); const arch = process.env['BUILD_ARCH'] || _arch(); From 4498049a2e8243031687545bcef42401d85d086b Mon Sep 17 00:00:00 2001 From: JonasBa Date: Wed, 7 Aug 2024 12:32:51 -0400 Subject: [PATCH 02/10] ref(profiling) remove logs and add separate build with shims --- .github/workflows/build.yml | 73 +++++++++++++------ .../node-profiling/build.shimmed.mjs | 28 +++++++ .../node-profiling/package.json | 8 +- packages/profiling-node/rollup.npm.config.mjs | 4 - 4 files changed, 83 insertions(+), 30 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6eea92c884ef..31c7e5fc0761 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -97,7 +97,6 @@ jobs: - 'packages/profiling-node/**' - 'dev-packages/e2e-tests/test-applications/node-profiling/**' - - name: Get PR labels id: pr-labels uses: mydea/pr-labels-action@fn/bump-node20 @@ -196,12 +195,24 @@ jobs: outputs: dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }} - changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }} - changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }} - changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }} - changed_deno: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/deno') }} - changed_bun: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/bun') }} - changed_browser_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/browser-integration-tests') }} + changed_node_integration: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry-internal/node-integration-tests') }} + changed_remix: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/remix') }} + changed_node: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/node') }} + changed_deno: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/deno') }} + changed_bun: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/bun') }} + changed_browser_integration: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry-internal/browser-integration-tests') }} # If you are looking for changed_profiling_node, this is defined in job_get_metadata job_check_branches: @@ -502,7 +513,9 @@ jobs: job_profiling_node_unit_tests: name: Node Profiling Unit Tests needs: [job_get_metadata, job_build] - if: needs.job_build.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' || github.event_name != 'pull_request' + if: + needs.job_build.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' + || github.event_name != 'pull_request' runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -528,7 +541,8 @@ jobs: run: yarn lerna run test --scope @sentry/profiling-node job_browser_playwright_tests: - name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests + name: + Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04-large-js @@ -597,7 +611,9 @@ jobs: env: PW_BUNDLE: ${{ matrix.bundle }} working-directory: dev-packages/browser-integration-tests - run: yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} + run: + yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && + format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} - name: Upload Playwright Traces uses: actions/upload-artifact@v3 if: always() @@ -897,7 +913,7 @@ jobs: 'nuxt-3', 'vue-3', 'webpack-4', - 'webpack-5' + 'webpack-5', ] build-command: - false @@ -984,9 +1000,8 @@ jobs: # We need to add the `always()` check here because the previous step has this as well :( # See: https://github.com/actions/runner/issues/2205 if: - always() && needs.job_e2e_prepare.result == 'success' && - (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && - github.actor != 'dependabot[bot]' + always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1093,13 +1108,10 @@ jobs: # See: https://github.com/actions/runner/issues/2205 if: # Only run profiling e2e tests if profiling node bindings have changed - always() && needs.job_e2e_prepare.result == 'success' && - (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && - ( - (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || - (needs.job_get_metadata.outputs.is_release == 'true') || - (github.event_name != 'pull_request') - ) + always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && ( + (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || (needs.job_get_metadata.outputs.is_release == + 'true') || (github.event_name != 'pull_request') ) needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1145,7 +1157,7 @@ jobs: with: path: ${{ github.workspace }}/packages/*/*.tgz key: ${{ env.BUILD_CACHE_TARBALL_KEY }} - fail-on-cache-miss : true + fail-on-cache-miss: true - name: Install Playwright uses: ./.github/actions/install-playwright @@ -1175,6 +1187,16 @@ jobs: timeout-minutes: 5 run: yarn test:assert + - name: Build E2E app with shimmed ESM globals + working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} + timeout-minutes: 5 + run: yarn ${{ matrix.build-command || 'test:build:shimmed' }} + + - name: Run E2E test with shimmed ESM globals + working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} + timeout-minutes: 5 + run: yarn test:assert:shimmed + job_required_jobs_passed: name: All required jobs passed or were skipped needs: @@ -1248,7 +1270,10 @@ jobs: path: ${{ steps.process.outputs.artifactPath }} job_compile_bindings_profiling_node: - name: Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, 'alpine') && 'musl' || 'glibc' }} + name: + Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ + matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, + 'alpine') && 'musl' || 'glibc' }} needs: [job_get_metadata, job_build] # Compiling bindings can be very slow (especially on windows), so only run precompile # Skip precompile unless we are on a release branch as precompile slows down CI times. @@ -1263,7 +1288,7 @@ jobs: fail-fast: false matrix: include: - # x64 glibc + # x64 glibc - os: ubuntu-20.04 node: 16 binary: linux-x64-glibc-93 diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs new file mode 100644 index 000000000000..9cba90d6db4b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs @@ -0,0 +1,28 @@ +// Because bundlers can now predetermine a static set of binaries we need to ensure those binaries +// actually exists, else we risk a compile time error when bundling the package. This could happen +// if we added a new binary in cpu_profiler.ts, but forgot to prebuild binaries for it. Because CI +// only runs integration and unit tests, this change would be missed and could end up in a release. +// Therefor, once all binaries are precompiled in CI and tests pass, run esbuild with bundle:true +// which will copy all binaries to the outfile folder and throw if any of them are missing. +import esbuild from 'esbuild'; + +console.log('Running build using esbuild version', esbuild.version); + +esbuild.buildSync({ + platform: 'node', + entryPoints: ['./index.ts'], + outdir: './dist', + target: 'esnext', + format: 'esm', + bundle: true, + loader: { '.node': 'copy' }, + banner: { + js: ` + import { fileURLToPath } from 'node:url'; + import { createRequire } from 'node:module'; + const require = createRequire(import.meta.url); + const __filename = fileURLToPath(import.meta.url); + const __dirname = path.dirname(__filename); + `, + }, +}); 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 94ec4926f2f6..152e21165920 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -5,10 +5,14 @@ "scripts": { "typecheck": "tsc --noEmit", "build": "node build.mjs", + "build:shimmed": "node build.shimmed.mjs", "test": "npm run build && node dist/index.js", - "clean": "npx rimraf node_modules", + "test:shimmed": "npm run build:shimmed && node dist/index.js", + "clean": "npx rimraf node_modules dist", "test:build": "npm run typecheck && npm run build", - "test:assert": "npm run test" + "test:build:shimmed": "npm run typecheck && npm run build:shimmed", + "test:assert": "npm run test", + "test:assert:shimmed": "npm run test:shimmed" }, "dependencies": { "@sentry/node": "latest || *", diff --git a/packages/profiling-node/rollup.npm.config.mjs b/packages/profiling-node/rollup.npm.config.mjs index c4fa78e8b00e..12492b7c83e8 100644 --- a/packages/profiling-node/rollup.npm.config.mjs +++ b/packages/profiling-node/rollup.npm.config.mjs @@ -23,9 +23,6 @@ function makeESMShimPlugin(shim) { return { transform(code) { const SHIM_REGEXP = /\/\/ #START_SENTRY_ESM_SHIM[\s\S]*?\/\/ #END_SENTRY_ESM_SHIM/; - - console.log('Matched', SHIM_REGEXP.test(code)); - return code.replace(SHIM_REGEXP, shim); }, }; @@ -42,7 +39,6 @@ const variants = makeNPMConfigVariants( for (const variant of variants) { if (variant.output.format === 'esm') { - console.log('injected plugin'); variant.plugins.push(makeESMShimPlugin(ESMShim)); } else { // Remove the ESM shim comment From 17c41221535a4bb200727cda59698dff350666ff Mon Sep 17 00:00:00 2001 From: JonasBa Date: Wed, 7 Aug 2024 12:40:04 -0400 Subject: [PATCH 03/10] ref(profiling) run test --- .github/workflows/build.yml | 63 ++++++++++++++----------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 31c7e5fc0761..317238cc84fa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -97,6 +97,7 @@ jobs: - 'packages/profiling-node/**' - 'dev-packages/e2e-tests/test-applications/node-profiling/**' + - name: Get PR labels id: pr-labels uses: mydea/pr-labels-action@fn/bump-node20 @@ -195,24 +196,12 @@ jobs: outputs: dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }} - changed_node_integration: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry-internal/node-integration-tests') }} - changed_remix: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/remix') }} - changed_node: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/node') }} - changed_deno: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/deno') }} - changed_bun: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/bun') }} - changed_browser_integration: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry-internal/browser-integration-tests') }} + changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }} + changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }} + changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }} + changed_deno: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/deno') }} + changed_bun: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/bun') }} + changed_browser_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/browser-integration-tests') }} # If you are looking for changed_profiling_node, this is defined in job_get_metadata job_check_branches: @@ -513,9 +502,7 @@ jobs: job_profiling_node_unit_tests: name: Node Profiling Unit Tests needs: [job_get_metadata, job_build] - if: - needs.job_build.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' - || github.event_name != 'pull_request' + if: needs.job_build.outputs.changed_node == 'true' || needs.job_get_metadata.outputs.changed_profiling_node == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -541,8 +528,7 @@ jobs: run: yarn lerna run test --scope @sentry/profiling-node job_browser_playwright_tests: - name: - Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests + name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04-large-js @@ -611,9 +597,7 @@ jobs: env: PW_BUNDLE: ${{ matrix.bundle }} working-directory: dev-packages/browser-integration-tests - run: - yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && - format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} + run: yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} - name: Upload Playwright Traces uses: actions/upload-artifact@v3 if: always() @@ -913,7 +897,7 @@ jobs: 'nuxt-3', 'vue-3', 'webpack-4', - 'webpack-5', + 'webpack-5' ] build-command: - false @@ -1000,8 +984,9 @@ jobs: # We need to add the `always()` check here because the previous step has this as well :( # See: https://github.com/actions/runner/issues/2205 if: - always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || - github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' + always() && needs.job_e2e_prepare.result == 'success' && + (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && + github.actor != 'dependabot[bot]' needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1108,10 +1093,13 @@ jobs: # See: https://github.com/actions/runner/issues/2205 if: # Only run profiling e2e tests if profiling node bindings have changed - always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || - github.event.pull_request.head.repo.full_name == github.repository) && ( - (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || (needs.job_get_metadata.outputs.is_release == - 'true') || (github.event_name != 'pull_request') ) + always() && needs.job_e2e_prepare.result == 'success' && + (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && + ( + (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || + (needs.job_get_metadata.outputs.is_release == 'true') || + (github.event_name != 'pull_request') + ) needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1157,7 +1145,7 @@ jobs: with: path: ${{ github.workspace }}/packages/*/*.tgz key: ${{ env.BUILD_CACHE_TARBALL_KEY }} - fail-on-cache-miss: true + fail-on-cache-miss : true - name: Install Playwright uses: ./.github/actions/install-playwright @@ -1270,10 +1258,7 @@ jobs: path: ${{ steps.process.outputs.artifactPath }} job_compile_bindings_profiling_node: - name: - Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ - matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, - 'alpine') && 'musl' || 'glibc' }} + name: Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, 'alpine') && 'musl' || 'glibc' }} needs: [job_get_metadata, job_build] # Compiling bindings can be very slow (especially on windows), so only run precompile # Skip precompile unless we are on a release branch as precompile slows down CI times. @@ -1288,7 +1273,7 @@ jobs: fail-fast: false matrix: include: - # x64 glibc + # x64 glibc - os: ubuntu-20.04 node: 16 binary: linux-x64-glibc-93 From 8d801ebda1a6b1d1ab92ed409122ae8807be8854 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Wed, 7 Aug 2024 14:04:21 -0400 Subject: [PATCH 04/10] ref(profiling) output mjs --- .../test-applications/node-profiling/build.shimmed.mjs | 2 +- .../e2e-tests/test-applications/node-profiling/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs index 9cba90d6db4b..a54b3d7c9215 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs @@ -11,7 +11,7 @@ console.log('Running build using esbuild version', esbuild.version); esbuild.buildSync({ platform: 'node', entryPoints: ['./index.ts'], - outdir: './dist', + outfile: './dist/index.mjs', target: 'esnext', format: 'esm', bundle: true, 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 152e21165920..12db7562d9ee 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -7,7 +7,7 @@ "build": "node build.mjs", "build:shimmed": "node build.shimmed.mjs", "test": "npm run build && node dist/index.js", - "test:shimmed": "npm run build:shimmed && node dist/index.js", + "test:shimmed": "npm run build:shimmed && node dist/index.mjs", "clean": "npx rimraf node_modules dist", "test:build": "npm run typecheck && npm run build", "test:build:shimmed": "npm run typecheck && npm run build:shimmed", From fc1a9f82284a3acbb556ac0a0655f4d70be0bb35 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Wed, 7 Aug 2024 14:41:20 -0400 Subject: [PATCH 05/10] ref(profiling) add missing dirname import --- .../test-applications/node-profiling/build.shimmed.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs index a54b3d7c9215..cfb1fd2a7db0 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs @@ -18,11 +18,12 @@ esbuild.buildSync({ 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 = path.dirname(__filename); + const __dirname = dirname(__filename); `, }, }); From cf53b909cb959bcddb643793125ca66ab0dba2f7 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 9 Sep 2024 10:18:58 -0400 Subject: [PATCH 06/10] ref: build and test as a single command --- .github/workflows/build.yml | 73 ++++++++++--------- .../node-profiling/build.shimmed.mjs | 2 +- .../node-profiling/package.json | 10 +-- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index aa922a10ab48..d54c77ec4e97 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -98,7 +98,6 @@ jobs: any_code: - '!**/*.md' - - name: Get PR labels id: pr-labels uses: mydea/pr-labels-action@fn/bump-node20 @@ -191,12 +190,24 @@ jobs: outputs: dependency_cache_key: ${{ steps.install_dependencies.outputs.cache_key }} - changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }} - changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }} - changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }} - changed_deno: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/deno') }} - changed_bun: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/bun') }} - changed_browser_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/browser-integration-tests') }} + changed_node_integration: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry-internal/node-integration-tests') }} + changed_remix: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/remix') }} + changed_node: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/node') }} + changed_deno: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/deno') }} + changed_bun: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry/bun') }} + changed_browser_integration: + ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, + '@sentry-internal/browser-integration-tests') }} # If you are looking for changed_profiling_node, this is defined in job_get_metadata job_check_branches: @@ -529,7 +540,9 @@ jobs: run: yarn lerna run test --scope @sentry/profiling-node job_browser_playwright_tests: - name: Playwright ${{ matrix.bundle }}${{ matrix.project && matrix.project != 'chromium' && format(' {0}', matrix.project) || ''}}${{ matrix.shard && format(' ({0}/{1})', matrix.shard, matrix.shards) || ''}} Tests + name: + Playwright ${{ matrix.bundle }}${{ matrix.project && matrix.project != 'chromium' && format(' {0}', + matrix.project) || ''}}${{ matrix.shard && format(' ({0}/{1})', matrix.shard, matrix.shards) || ''}} Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04-large-js @@ -599,13 +612,17 @@ jobs: env: PW_BUNDLE: ${{ matrix.bundle }} working-directory: dev-packages/browser-integration-tests - run: yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} + run: + yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && + format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} - name: Upload Playwright Traces uses: actions/upload-artifact@v4 if: failure() with: - name: playwright-traces-job_browser_playwright_tests-${{ matrix.bundle}}-${{matrix.project}}-${{matrix.shard || '0'}} + name: + playwright-traces-job_browser_playwright_tests-${{ matrix.bundle}}-${{matrix.project}}-${{matrix.shard || + '0'}} path: dev-packages/browser-integration-tests/test-results overwrite: true retention-days: 7 @@ -916,7 +933,7 @@ jobs: 'nuxt-3', 'vue-3', 'webpack-4', - 'webpack-5' + 'webpack-5', ] build-command: - false @@ -1019,9 +1036,8 @@ jobs: # We need to add the `always()` check here because the previous step has this as well :( # See: https://github.com/actions/runner/issues/2205 if: - always() && needs.job_e2e_prepare.result == 'success' && - (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && - github.actor != 'dependabot[bot]' + always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1135,12 +1151,10 @@ jobs: # See: https://github.com/actions/runner/issues/2205 if: # Only run profiling e2e tests if profiling node bindings have changed - always() && needs.job_e2e_prepare.result == 'success' && - (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && - ( - (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || - (needs.job_get_metadata.outputs.is_release == 'true') - ) + always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) && ( + (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || (needs.job_get_metadata.outputs.is_release == + 'true') ) needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1223,16 +1237,6 @@ jobs: timeout-minutes: 5 run: yarn test:assert - - name: Build E2E app with shimmed ESM globals - working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} - timeout-minutes: 5 - run: yarn ${{ matrix.build-command || 'test:build:shimmed' }} - - - name: Run E2E test with shimmed ESM globals - working-directory: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} - timeout-minutes: 5 - run: yarn test:assert:shimmed - job_required_jobs_passed: name: All required jobs passed or were skipped needs: @@ -1307,7 +1311,10 @@ jobs: retention-days: 7 job_compile_bindings_profiling_node: - name: Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, 'alpine') && 'musl' || 'glibc' }} + name: + Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ + matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, + 'alpine') && 'musl' || 'glibc' }} needs: [job_get_metadata, job_build] # Compiling bindings can be very slow (especially on windows), so only run precompile # Skip precompile unless we are on a release branch as precompile slows down CI times. @@ -1321,7 +1328,7 @@ jobs: fail-fast: false matrix: include: - # x64 glibc + # x64 glibc - os: ubuntu-20.04 node: 16 binary: linux-x64-glibc-93 @@ -1478,7 +1485,7 @@ jobs: - name: Install dependencies env: - SKIP_PLAYWRIGHT_BROWSER_INSTALL: "1" + SKIP_PLAYWRIGHT_BROWSER_INSTALL: '1' if: steps.restore-dependencies.outputs.cache-hit != 'true' run: yarn install --ignore-engines --frozen-lockfile diff --git a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs index cfb1fd2a7db0..c45e30539bc0 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs +++ b/dev-packages/e2e-tests/test-applications/node-profiling/build.shimmed.mjs @@ -11,7 +11,7 @@ console.log('Running build using esbuild version', esbuild.version); esbuild.buildSync({ platform: 'node', entryPoints: ['./index.ts'], - outfile: './dist/index.mjs', + outfile: './dist/index.shimmed.mjs', target: 'esnext', format: 'esm', bundle: true, 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 12db7562d9ee..e0f79461af07 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -4,15 +4,11 @@ "private": true, "scripts": { "typecheck": "tsc --noEmit", - "build": "node build.mjs", - "build:shimmed": "node build.shimmed.mjs", - "test": "npm run build && node dist/index.js", - "test:shimmed": "npm run build:shimmed && node dist/index.mjs", + "build": "node build.mjs && npm run build:shimmed", + "test": "node dist/index.js && node dist/index.shimmed.mjs", "clean": "npx rimraf node_modules dist", "test:build": "npm run typecheck && npm run build", - "test:build:shimmed": "npm run typecheck && npm run build:shimmed", - "test:assert": "npm run test", - "test:assert:shimmed": "npm run test:shimmed" + "test:assert": "npm run test" }, "dependencies": { "@sentry/node": "latest || *", From f1f9bd472d4e006447f4a038889f222dfb5cd975 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 9 Sep 2024 10:20:59 -0400 Subject: [PATCH 07/10] build: revert changes to build.yml --- .github/workflows/build.yml | 63 ++++++++++++++----------------------- 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d54c77ec4e97..626955ac2718 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -98,6 +98,7 @@ jobs: any_code: - '!**/*.md' + - name: Get PR labels id: pr-labels uses: mydea/pr-labels-action@fn/bump-node20 @@ -190,24 +191,12 @@ jobs: outputs: dependency_cache_key: ${{ steps.install_dependencies.outputs.cache_key }} - changed_node_integration: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry-internal/node-integration-tests') }} - changed_remix: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/remix') }} - changed_node: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/node') }} - changed_deno: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/deno') }} - changed_bun: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry/bun') }} - changed_browser_integration: - ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, - '@sentry-internal/browser-integration-tests') }} + changed_node_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/node-integration-tests') }} + changed_remix: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/remix') }} + changed_node: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/node') }} + changed_deno: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/deno') }} + changed_bun: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry/bun') }} + changed_browser_integration: ${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected, '@sentry-internal/browser-integration-tests') }} # If you are looking for changed_profiling_node, this is defined in job_get_metadata job_check_branches: @@ -540,9 +529,7 @@ jobs: run: yarn lerna run test --scope @sentry/profiling-node job_browser_playwright_tests: - name: - Playwright ${{ matrix.bundle }}${{ matrix.project && matrix.project != 'chromium' && format(' {0}', - matrix.project) || ''}}${{ matrix.shard && format(' ({0}/{1})', matrix.shard, matrix.shards) || ''}} Tests + name: Playwright ${{ matrix.bundle }}${{ matrix.project && matrix.project != 'chromium' && format(' {0}', matrix.project) || ''}}${{ matrix.shard && format(' ({0}/{1})', matrix.shard, matrix.shards) || ''}} Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04-large-js @@ -612,17 +599,13 @@ jobs: env: PW_BUNDLE: ${{ matrix.bundle }} working-directory: dev-packages/browser-integration-tests - run: - yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && - format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} + run: yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} - name: Upload Playwright Traces uses: actions/upload-artifact@v4 if: failure() with: - name: - playwright-traces-job_browser_playwright_tests-${{ matrix.bundle}}-${{matrix.project}}-${{matrix.shard || - '0'}} + name: playwright-traces-job_browser_playwright_tests-${{ matrix.bundle}}-${{matrix.project}}-${{matrix.shard || '0'}} path: dev-packages/browser-integration-tests/test-results overwrite: true retention-days: 7 @@ -933,7 +916,7 @@ jobs: 'nuxt-3', 'vue-3', 'webpack-4', - 'webpack-5', + 'webpack-5' ] build-command: - false @@ -1036,8 +1019,9 @@ jobs: # We need to add the `always()` check here because the previous step has this as well :( # See: https://github.com/actions/runner/issues/2205 if: - always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || - github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' + always() && needs.job_e2e_prepare.result == 'success' && + (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && + github.actor != 'dependabot[bot]' needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1151,10 +1135,12 @@ jobs: # See: https://github.com/actions/runner/issues/2205 if: # Only run profiling e2e tests if profiling node bindings have changed - always() && needs.job_e2e_prepare.result == 'success' && (github.event_name != 'pull_request' || - github.event.pull_request.head.repo.full_name == github.repository) && ( - (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || (needs.job_get_metadata.outputs.is_release == - 'true') ) + always() && needs.job_e2e_prepare.result == 'success' && + (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && + ( + (needs.job_get_metadata.outputs.changed_profiling_node == 'true') || + (needs.job_get_metadata.outputs.is_release == 'true') + ) needs: [job_get_metadata, job_build, job_e2e_prepare] runs-on: ubuntu-20.04 timeout-minutes: 10 @@ -1311,10 +1297,7 @@ jobs: retention-days: 7 job_compile_bindings_profiling_node: - name: - Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ - matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, - 'alpine') && 'musl' || 'glibc' }} + name: Compile & Test Profiling Bindings (v${{ matrix.node }}) ${{ matrix.target_platform || matrix.os }}, ${{ matrix.node || matrix.container }}, ${{ matrix.arch || matrix.container }}, ${{ contains(matrix.container, 'alpine') && 'musl' || 'glibc' }} needs: [job_get_metadata, job_build] # Compiling bindings can be very slow (especially on windows), so only run precompile # Skip precompile unless we are on a release branch as precompile slows down CI times. @@ -1328,7 +1311,7 @@ jobs: fail-fast: false matrix: include: - # x64 glibc + # x64 glibc - os: ubuntu-20.04 node: 16 binary: linux-x64-glibc-93 @@ -1485,7 +1468,7 @@ jobs: - name: Install dependencies env: - SKIP_PLAYWRIGHT_BROWSER_INSTALL: '1' + SKIP_PLAYWRIGHT_BROWSER_INSTALL: "1" if: steps.restore-dependencies.outputs.cache-hit != 'true' run: yarn install --ignore-engines --frozen-lockfile From 1b22002c3277ffbf31908f77c78c43a6a1a98554 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 9 Sep 2024 10:24:26 -0400 Subject: [PATCH 08/10] ref: correct build command --- .../e2e-tests/test-applications/node-profiling/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e0f79461af07..a81acefc8002 100644 --- a/dev-packages/e2e-tests/test-applications/node-profiling/package.json +++ b/dev-packages/e2e-tests/test-applications/node-profiling/package.json @@ -4,7 +4,7 @@ "private": true, "scripts": { "typecheck": "tsc --noEmit", - "build": "node build.mjs && npm run build:shimmed", + "build": "node build.mjs && node build.shimmed.mjs", "test": "node dist/index.js && node dist/index.shimmed.mjs", "clean": "npx rimraf node_modules dist", "test:build": "npm run typecheck && npm run build", From d6adbfa196762956dfa1adbf5946c6b4f7c959a9 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 9 Sep 2024 11:19:12 -0400 Subject: [PATCH 09/10] ref: add --experimental-require-module to test --- .../e2e-tests/test-applications/node-profiling/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a81acefc8002..a4c4bf1284fe 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 dist/index.shimmed.mjs", + "test": "node dist/index.js && node --experimental-require-module dist/index.js && node dist/index.shimmed.mjs", "clean": "npx rimraf node_modules dist", "test:build": "npm run typecheck && npm run build", "test:assert": "npm run test" From 7aecbb5ecfd9cd496a42e7ba29bbd89de327e271 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Mon, 9 Sep 2024 11:23:38 -0400 Subject: [PATCH 10/10] ref: bump to node 22 and run using the experimental flag --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 626955ac2718..38102ff204c7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1168,7 +1168,7 @@ jobs: - name: Set up Node uses: actions/setup-node@v4 with: - node-version-file: 'dev-packages/e2e-tests/package.json' + node-version: 22 - name: Restore caches uses: ./.github/actions/restore-cache with: