diff --git a/packages/server-utils/src/orchestrion/bundler/esbuild.ts b/packages/server-utils/src/orchestrion/bundler/esbuild.ts index be6f7bfca8ad..307e5e78a033 100644 --- a/packages/server-utils/src/orchestrion/bundler/esbuild.ts +++ b/packages/server-utils/src/orchestrion/bundler/esbuild.ts @@ -1,6 +1,16 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/esbuild'; +import { escapeStringForRegex } from '@sentry/core'; +import { instrumentedModuleNames } from '../config'; import type { PluginOptions } from './options'; -import { orchestrionTransformOptions } from './options'; +import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; + +// esbuild `external` entries may contain `*` wildcards. +function matchesEsbuildExternal(entry: string, moduleName: string): boolean { + if (entry.includes('*')) { + return new RegExp(`^${entry.split('*').map(escapeStringForRegex).join('.*')}$`).test(moduleName); + } + return externalEntryMatchesModule(entry, moduleName); +} /** * esbuild plugin that runs the orchestrion code transform on the bundled output. @@ -8,9 +18,8 @@ import { orchestrionTransformOptions } from './options'; * Use when bundling a Node app with esbuild. For unbundled Node processes use the * runtime hook instead (`node --import @sentry/node/orchestrion app.js`). * - * esbuild does not flatten nested `plugins` arrays, so this returns a single - * plugin that strips instrumented packages from an `external` denylist before - * delegating to the upstream transform. + * Instrumented packages marked as `external` never pass through the code + * transform, so a build warning is emitted for them. * * @example * ```ts @@ -20,5 +29,21 @@ import { orchestrionTransformOptions } from './options'; * ``` */ export function sentryOrchestrionPlugin(options: PluginOptions = {}): ReturnType { - return codeTransformer(orchestrionTransformOptions(options)); + const plugin = codeTransformer(orchestrionTransformOptions(options)); + const moduleNames = instrumentedModuleNames(options.instrumentations); + const setup = plugin.setup; + + return { + ...plugin, + setup(build): ReturnType { + const external = build.initialOptions.external || []; + const externalizedModules = moduleNames.filter(name => + external.some(entry => matchesEsbuildExternal(entry, name)), + ); + if (externalizedModules.length > 0) { + build.onStart(() => ({ warnings: [{ text: externalizedModulesWarning(externalizedModules) }] })); + } + return setup(build); + }, + }; } diff --git a/packages/server-utils/src/orchestrion/bundler/options.ts b/packages/server-utils/src/orchestrion/bundler/options.ts index c536482df346..a3c194aae165 100644 --- a/packages/server-utils/src/orchestrion/bundler/options.ts +++ b/packages/server-utils/src/orchestrion/bundler/options.ts @@ -19,6 +19,30 @@ export type PluginOptions = { shouldInjectDiagnostics?: boolean; }; +/** + * Whether an "external" config entry covers an instrumented module: an exact + * package name (`'mysql'`) or a subpath (`'mysql/lib/...'`) — the transform may + * target exactly the file a subpath entry externalizes. Mirrors the matching in + * `withoutInstrumentedExternals`. + */ +export function externalEntryMatchesModule(entry: string, moduleName: string): boolean { + return entry === moduleName || entry.startsWith(`${moduleName}/`); +} + +/** + * Warning emitted when a bundler config externalizes packages that orchestrion + * needs to transform. An externalized dependency is resolved from + * `node_modules` at runtime and never passes through the code transform, so + * its diagnostics_channel calls are silently never injected. + */ +export function externalizedModulesWarning(externalizedModules: string[]): string { + return ( + `The following packages are marked as external in your bundler configuration but need to be bundled for Sentry ` + + `instrumentation to work: ${externalizedModules.join(', ')}. Remove them from your bundler's "external" ` + + `configuration, or use the Sentry Node SDK's runtime instrumentation instead.` + ); +} + /** * The `@apm-js-collab/code-transformer-bundler-plugins` options shared by every * orchestrion bundler plugin. diff --git a/packages/server-utils/src/orchestrion/bundler/rollup.ts b/packages/server-utils/src/orchestrion/bundler/rollup.ts index 28b080802fee..d42abede972c 100644 --- a/packages/server-utils/src/orchestrion/bundler/rollup.ts +++ b/packages/server-utils/src/orchestrion/bundler/rollup.ts @@ -1,6 +1,8 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/rollup'; +import type { NormalizedInputOptions, PluginContext } from 'rollup'; +import { instrumentedModuleNames } from '../config'; import type { PluginOptions } from './options'; -import { orchestrionTransformOptions } from './options'; +import { externalizedModulesWarning, orchestrionTransformOptions } from './options'; /** * Rollup plugin that runs the orchestrion code transform on the bundled output. @@ -16,5 +18,19 @@ import { orchestrionTransformOptions } from './options'; * ``` */ export function sentryOrchestrionPlugin(options: PluginOptions = {}): ReturnType { - return codeTransformer(orchestrionTransformOptions(options)); + const moduleNames = instrumentedModuleNames(options.instrumentations); + + return { + ...codeTransformer(orchestrionTransformOptions(options)), + buildStart(this: PluginContext, rollupOptions: NormalizedInputOptions): void { + // An externalized dependency never passes through the code transform, so + // its diagnostics_channel calls are silently never injected. By the time + // buildStart runs, Rollup has normalized `external` (string arrays, + // RegExps or user functions) into a single predicate we can probe. + const externalizedModules = moduleNames.filter(name => rollupOptions.external(name, undefined, false)); + if (externalizedModules.length > 0) { + this.warn(externalizedModulesWarning(externalizedModules)); + } + }, + }; } diff --git a/packages/server-utils/src/orchestrion/bundler/vite.ts b/packages/server-utils/src/orchestrion/bundler/vite.ts index 75ad1ab49855..0b586f0b682c 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -1,7 +1,8 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/vite'; +import type { ResolvedConfig } from 'vite'; import { instrumentedModuleNames } from '../config'; import type { PluginOptions } from './options'; -import { orchestrionTransformOptions } from './options'; +import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; /** * Vite plugin that runs the orchestrion code transform on the bundled output. @@ -30,5 +31,21 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): ReturnType // their additions. return { ssr: { noExternal: instrumentedModuleNames(options.instrumentations) } }; }, + configResolved(config: ResolvedConfig): void { + // Explicit `ssr.external` string entries take priority over `noExternal` + // in Vite, so they defeat the force-bundling above. (`ssr.external: true` + // does not — `noExternal` entries still win there.) + const external = config.ssr?.external; + if (!Array.isArray(external)) { + return; + } + const moduleNames = instrumentedModuleNames(options.instrumentations); + const externalizedModules = moduleNames.filter(name => + external.some(entry => externalEntryMatchesModule(entry, name)), + ); + if (externalizedModules.length > 0) { + config.logger.warn(`[Sentry] ${externalizedModulesWarning(externalizedModules)}`); + } + }, }; } diff --git a/packages/server-utils/src/orchestrion/bundler/webpack.ts b/packages/server-utils/src/orchestrion/bundler/webpack.ts index 7e2f584b1819..a20daf49b7b9 100644 --- a/packages/server-utils/src/orchestrion/bundler/webpack.ts +++ b/packages/server-utils/src/orchestrion/bundler/webpack.ts @@ -3,11 +3,12 @@ import { createRequire } from 'node:module'; import { dirname } from 'node:path'; +import type { Compiler } from 'webpack'; import type { InstrumentationConfig } from '..'; -import { SENTRY_INSTRUMENTATIONS } from '../config'; +import { instrumentedModuleNames, SENTRY_INSTRUMENTATIONS } from '../config'; import codeTransformerWebpack from '@apm-js-collab/code-transformer-bundler-plugins/webpack'; import type { PluginOptions } from './options'; -import { orchestrionTransformOptions } from './options'; +import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; // Both branches use `createRequire` (never alias the CJS `require`) so bundlers consuming this // module don't emit a "Critical dependency" warning. @@ -44,9 +45,47 @@ export function getSentryInstrumentations(): InstrumentationConfig[] { return SENTRY_INSTRUMENTATIONS; } +// Handles the declarative `externals` shapes (string, RegExp, object, arrays +// thereof). Function externals (e.g. webpack-node-externals) are skipped: they +// may resolve asynchronously, so they can't be probed reliably here. +function externalizedWebpackModules(externals: unknown, moduleNames: string[]): string[] { + const entries = Array.isArray(externals) ? externals : [externals]; + return moduleNames.filter(name => + entries.some(entry => { + if (typeof entry === 'string') { + return externalEntryMatchesModule(entry, name); + } + if (entry instanceof RegExp) { + return entry.test(name); + } + if (entry && typeof entry === 'object') { + return name in entry; + } + return false; + }), + ); +} + /** - * The code-transform webpack plugin, pre-fed the instrumentation config + * The code-transform webpack plugin, pre-fed the instrumentation config. + * + * Instrumented packages marked as `externals` never pass through the code + * transform, so a compilation warning is emitted for them. */ export function sentryOrchestrionWebpackPlugin(options: PluginOptions = {}): ReturnType { - return codeTransformerWebpack(orchestrionTransformOptions(options)); + const plugin = codeTransformerWebpack(orchestrionTransformOptions(options)); + const moduleNames = instrumentedModuleNames(options.instrumentations); + // The upstream plugin is a class instance, so `apply` is overridden in place + // rather than spread into a new object (which would lose prototype methods). + const apply = plugin.apply.bind(plugin); + plugin.apply = (compiler: Compiler): void => { + const externalizedModules = externalizedWebpackModules(compiler.options.externals, moduleNames); + if (externalizedModules.length > 0) { + compiler.hooks.thisCompilation.tap('SentryOrchestrionExternalsCheck', compilation => { + compilation.warnings.push(new compiler.webpack.WebpackError(externalizedModulesWarning(externalizedModules))); + }); + } + apply(compiler); + }; + return plugin; } diff --git a/packages/server-utils/test/orchestrion/bundler.test.ts b/packages/server-utils/test/orchestrion/bundler.test.ts index b4c309b9152c..f13209fba323 100644 --- a/packages/server-utils/test/orchestrion/bundler.test.ts +++ b/packages/server-utils/test/orchestrion/bundler.test.ts @@ -1,7 +1,140 @@ import { existsSync } from 'node:fs'; import { join } from 'node:path'; -import { describe, expect, it } from 'vitest'; -import { getTracingHooksDirectory } from '../../src/orchestrion/bundler/webpack'; +import type { OnStartResult, PluginBuild } from 'esbuild'; +import type { NormalizedInputOptions, PluginContext } from 'rollup'; +import type { ResolvedConfig } from 'vite'; +import type { Compiler } from 'webpack'; +import { describe, expect, it, vi } from 'vitest'; +import { sentryOrchestrionPlugin as esbuildPlugin } from '../../src/orchestrion/bundler/esbuild'; +import { sentryOrchestrionPlugin as rollupPlugin } from '../../src/orchestrion/bundler/rollup'; +import { sentryOrchestrionPlugin as vitePlugin } from '../../src/orchestrion/bundler/vite'; +import { getTracingHooksDirectory, sentryOrchestrionWebpackPlugin } from '../../src/orchestrion/bundler/webpack'; + +// The upstream transform plugins are mocked so tests exercise only the hooks +// added on top of them (the externalized-modules warnings). +vi.mock('@apm-js-collab/code-transformer-bundler-plugins/esbuild', () => ({ + default: () => ({ name: 'code-transformer', setup: vi.fn() }), +})); +vi.mock('@apm-js-collab/code-transformer-bundler-plugins/vite', () => ({ + default: () => ({ name: 'code-transformer' }), +})); +vi.mock('@apm-js-collab/code-transformer-bundler-plugins/webpack', () => ({ + default: () => ({ apply: vi.fn() }), +})); + +describe('sentryOrchestrionPlugin (rollup)', () => { + // Mirrors what Rollup passes to buildStart: `external` is already normalized + // into a predicate function, regardless of how the user configured it. + function runBuildStart(external: (source: string) => boolean): ReturnType { + const warn = vi.fn(); + const plugin = rollupPlugin(); + (plugin.buildStart as (this: unknown, options: unknown) => void).call( + { warn } as unknown as PluginContext, + { external } as unknown as NormalizedInputOptions, + ); + return warn; + } + + it('warns when instrumented modules are externalized', () => { + const warn = runBuildStart(source => source === 'mysql' || source === 'pg'); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('mysql, pg')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('need to be bundled')); + }); + + it('does not warn when no instrumented modules are externalized', () => { + const warn = runBuildStart(source => source === 'some-other-package'); + + expect(warn).not.toHaveBeenCalled(); + }); +}); + +describe('sentryOrchestrionPlugin (esbuild)', () => { + function runSetup(external: string[] | undefined): OnStartResult[] { + const onStartCallbacks: Array<() => OnStartResult> = []; + const build = { + initialOptions: { external }, + onStart: (callback: () => OnStartResult) => onStartCallbacks.push(callback), + } as unknown as PluginBuild; + void esbuildPlugin().setup(build); + return onStartCallbacks.map(callback => callback()); + } + + it('warns when instrumented modules are externalized', () => { + const results = runSetup(['mysql', 'pg/lib/client']); + + expect(results).toHaveLength(1); + expect(results[0]?.warnings?.[0]?.text).toContain('mysql, pg'); + }); + + it('matches wildcard external patterns', () => { + const results = runSetup(['mysql*']); + + expect(results).toHaveLength(1); + expect(results[0]?.warnings?.[0]?.text).toContain('mysql'); + expect(results[0]?.warnings?.[0]?.text).toContain('mysql2'); + }); + + it('does not warn when no instrumented modules are externalized', () => { + expect(runSetup(['lodash'])).toHaveLength(0); + expect(runSetup(undefined)).toHaveLength(0); + }); +}); + +describe('sentryOrchestrionWebpackPlugin', () => { + function runApply(externals: unknown): Error[] { + const compilation = { warnings: [] as Error[] }; + const compiler = { + options: { externals }, + hooks: { + thisCompilation: { tap: (_name: string, callback: (compilation: unknown) => void) => callback(compilation) }, + }, + webpack: { WebpackError: Error }, + } as unknown as Compiler; + sentryOrchestrionWebpackPlugin().apply(compiler); + return compilation.warnings; + } + + it('warns for string, RegExp and object externals', () => { + expect(runApply(['mysql'])[0]?.message).toContain('mysql'); + expect(runApply('mysql')[0]?.message).toContain('mysql'); + expect(runApply([/^pg$/])[0]?.message).toContain('pg'); + expect(runApply({ mysql: 'commonjs mysql' })[0]?.message).toContain('mysql'); + }); + + it('does not warn for unrelated or function externals', () => { + expect(runApply(['lodash'])).toHaveLength(0); + expect(runApply(() => undefined)).toHaveLength(0); + expect(runApply(undefined)).toHaveLength(0); + }); +}); + +describe('sentryOrchestrionPlugin (vite)', () => { + function runConfigResolved(ssrExternal: string[] | true | undefined): ReturnType { + const warn = vi.fn(); + const plugin = vitePlugin(); + (plugin.configResolved as (config: unknown) => void)({ + ssr: { external: ssrExternal }, + logger: { warn }, + } as unknown as ResolvedConfig); + return warn; + } + + it('warns when instrumented modules are listed in ssr.external', () => { + const warn = runConfigResolved(['mysql', 'lodash']); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('mysql')); + }); + + it('does not warn for ssr.external: true or unrelated entries', () => { + // `ssr.external: true` does not override the plugin's noExternal entries. + expect(runConfigResolved(true)).not.toHaveBeenCalled(); + expect(runConfigResolved(['lodash'])).not.toHaveBeenCalled(); + expect(runConfigResolved(undefined)).not.toHaveBeenCalled(); + }); +}); describe('getTracingHooksDirectory', () => { it('returns the tracing-hooks package directory with the runtime hook entry points', () => {