diff --git a/packages/cloudflare/src/vite/flueRuntime.ts b/packages/cloudflare/src/vite/flueRuntime.ts deleted file mode 100644 index 004012a5f09f..000000000000 --- a/packages/cloudflare/src/vite/flueRuntime.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { ProvidedModulePlugin } from './providedModulePlugin'; -import { createProvidedModulePlugin } from './providedModulePlugin'; - -/** - * Splices a static `import * as … from '@flue/runtime'` into Sentry's own Flue integration module - * and exposes the namespace on the global orchestrion marker. - * - * Flue is registered rather than patched — `instrument()` writes into module-scope state — so - * instrumenting it needs that module's own binding, and no channel payload carries one. On Node the - * user passes it by calling `instrument()` themselves; a bundled worker has no `node_modules` to - * resolve from, so it is supplied at build time instead. - */ -export function sentryFlueRuntimeProviderPlugin(): ProvidedModulePlugin { - return createProvidedModulePlugin({ - name: 'sentry-cloudflare-flue-runtime-provider', - moduleName: '@flue/runtime', - identifier: '__SENTRY_FLUE_RUNTIME__', - integrationModule: 'flue', - }); -} diff --git a/packages/cloudflare/src/vite/index.ts b/packages/cloudflare/src/vite/index.ts index a0a4b91fecdc..9566f15aa85b 100644 --- a/packages/cloudflare/src/vite/index.ts +++ b/packages/cloudflare/src/vite/index.ts @@ -5,7 +5,6 @@ // expose it — same setup as `@sentry/server-utils/orchestrion/vite` itself. import { sentryOrchestrionPlugin } from '@sentry/server-utils/orchestrion/vite'; import { sentryCloudflareAutoInstrumentPlugin } from './autoInstrument'; -import { sentryFlueRuntimeProviderPlugin } from './flueRuntime'; import { sentryMastraObservabilityProviderPlugin } from './mastraObservability'; /** @@ -91,9 +90,10 @@ export function sentryCloudflareVitePlugin(options: SentryCloudflareVitePluginOp // dependency wrapped at module scope (`const app = new Hono()`) doesn't throw on workerd. dcModule: '@sentry/cloudflare/orchestrion-diagnostics-channel', transformOptimizedDeps: true, + // `flueIntegration` needs Flue's own `instrument()`, which a bundled worker can't resolve. + selfProvidedModules: ['@flue/runtime'], }), sentryMastraObservabilityProviderPlugin(), - sentryFlueRuntimeProviderPlugin(), ...(options.autoInstrumentation !== false ? [sentryCloudflareAutoInstrumentPlugin({ wranglerConfigPath: options.wranglerConfigPath })] : []), diff --git a/packages/cloudflare/src/vite/providedModulePlugin.ts b/packages/cloudflare/src/vite/providedModulePlugin.ts index 1c63bd226741..fcd6b1ba1811 100644 --- a/packages/cloudflare/src/vite/providedModulePlugin.ts +++ b/packages/cloudflare/src/vite/providedModulePlugin.ts @@ -24,13 +24,13 @@ export interface ProvidedModulePlugin { } export interface ProvidedModulePluginOptions { - /** Vite plugin name, e.g. `sentry-cloudflare-flue-runtime-provider`. */ + /** Vite plugin name, e.g. `sentry-cloudflare-mastra-observability-provider`. */ name: string; - /** Bare specifier of the package to provide, e.g. `@flue/runtime`. */ + /** Bare specifier of the package to provide, e.g. `@mastra/observability`. */ moduleName: string; - /** Namespace binding the injected import uses, e.g. `__SENTRY_FLUE_RUNTIME__`. */ + /** Namespace binding the injected import uses, e.g. `__SENTRY_MASTRA_OBSERVABILITY__`. */ identifier: string; - /** Basename of the `@sentry/server-utils` integration module to inject into, e.g. `flue`. */ + /** Basename of the `@sentry/server-utils` integration module to inject into, e.g. `mastra`. */ integrationModule: string; } @@ -55,9 +55,7 @@ export function createIntegrationModuleMatcher(integrationModule: string): (id: function buildProviderSnippet({ moduleName, identifier }: ProvidedModulePluginOptions): string { const marker = 'globalThis.__SENTRY_ORCHESTRION__'; - // A getter, not an assignment: assigning reads the binding at injection time, so it stores - // `undefined` whenever the bundler evaluates Sentry's module before the provided package - // finished initializing. Enumerable so the entry shows up in `Object.keys` and a spread. + // A getter: Sentry's module can evaluate before the provided package has initialized. return ( `import * as ${identifier} from '${moduleName}';\n` + `(${marker} = ${marker} || {});\n` + @@ -68,14 +66,18 @@ function buildProviderSnippet({ moduleName, identifier }: ProvidedModulePluginOp } /** - * Build a Vite plugin that splices a static `import * as … from ''` into one of - * Sentry's own integration modules and exposes the namespace on the global orchestrion marker. + * Build a Vite plugin that hands a package to one of Sentry's integration modules in a bundled + * worker, which has no `node_modules` to resolve it from. For Mastra, this prepends roughly the + * following to `@sentry/server-utils/build/esm/integrations/mastra.js`: * - * Some packages are instrumented by registration rather than by patching, so instrumenting them - * needs a reference to that module's own binding and no channel payload carries one. On Node the - * integration resolves it itself; a bundled worker has no `node_modules` to resolve from, so the - * binding is supplied at build time instead. The import is static, lands in Sentry's module rather - * than the user's code, and is only emitted when the package actually resolves. + * ```js + * import * as __SENTRY_MASTRA_OBSERVABILITY__ from '@mastra/observability'; + * globalThis.__SENTRY_ORCHESTRION__.providedModules['@mastra/observability'] = __SENTRY_MASTRA_OBSERVABILITY__; // as a getter + * ``` + * + * Only for packages the app doesn't import itself: this import is what brings them into the bundle. + * A package the app loads anyway publishes itself instead (`selfProvidedModules`), which adds no + * edge to Sentry's module and so can't close a cycle. */ export function createProvidedModulePlugin(options: ProvidedModulePluginOptions): ProvidedModulePlugin { const isIntegrationModuleId = createIntegrationModuleMatcher(options.integrationModule); diff --git a/packages/cloudflare/test/vite/flueRuntime.test.ts b/packages/cloudflare/test/vite/flueRuntime.test.ts deleted file mode 100644 index 5b9d1d36d429..000000000000 --- a/packages/cloudflare/test/vite/flueRuntime.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; -import { sentryFlueRuntimeProviderPlugin } from '../../src/vite/flueRuntime'; -import { sentryCloudflareVitePlugin } from '../../src/vite/index'; - -const PROVIDER_PLUGIN = 'sentry-cloudflare-flue-runtime-provider'; -const FLUE_INTEGRATION_MODULE = '/app/node_modules/@sentry/server-utils/build/esm/integrations/flue.js'; - -describe('sentryFlueRuntimeProviderPlugin', () => { - it('injects `@flue/runtime` behind a getter', async () => { - // A getter, not an assignment: the bundler may evaluate Sentry's module before - // `@flue/runtime` is initialized, and assigning there would store `undefined`. - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root: '/app' }); - const resolve = vi.fn(async () => ({ id: '/app/node_modules/@flue/runtime/dist/index.mjs' })); - await plugin.buildStart.call({ resolve, warn: vi.fn() }); - - const code = plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)?.code; - - expect(resolve).toHaveBeenCalledWith('@flue/runtime', '/app/noop.js'); - expect(code).toContain("import * as __SENTRY_FLUE_RUNTIME__ from '@flue/runtime';"); - expect(code).toContain('get() { return __SENTRY_FLUE_RUNTIME__; }'); - }); - - it('injects nothing when the app has no @flue/runtime', async () => { - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root: '/app' }); - await plugin.buildStart.call({ resolve: vi.fn(async () => null), warn: vi.fn() }); - - expect(plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)).toBeUndefined(); - }); -}); - -describe('sentryCloudflareVitePlugin', () => { - it('always includes the Flue runtime provider plugin', () => { - expect(sentryCloudflareVitePlugin().map(plugin => plugin.name)).toContain(PROVIDER_PLUGIN); - // Not gated by auto-instrumentation: it injects into Sentry's own module, not the entry. - expect(sentryCloudflareVitePlugin({ autoInstrumentation: false }).map(plugin => plugin.name)).toContain( - PROVIDER_PLUGIN, - ); - }); -}); diff --git a/packages/cloudflare/test/vite/providedModulePlugin.test.ts b/packages/cloudflare/test/vite/providedModulePlugin.test.ts index 5a138cde1a19..a36b07dd79fb 100644 --- a/packages/cloudflare/test/vite/providedModulePlugin.test.ts +++ b/packages/cloudflare/test/vite/providedModulePlugin.test.ts @@ -2,13 +2,13 @@ import { describe, expect, it, vi } from 'vitest'; import type { ProvidedModulePluginOptions } from '../../src/vite/providedModulePlugin'; import { createIntegrationModuleMatcher, createProvidedModulePlugin } from '../../src/vite/providedModulePlugin'; -const TARGET = '/app/node_modules/@sentry/server-utils/build/esm/integrations/flue.js'; +const TARGET = '/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js'; const OPTIONS: ProvidedModulePluginOptions = { name: 'sentry-test-provider', moduleName: '@scope/pkg', identifier: '__SENTRY_TEST_PKG__', - integrationModule: 'flue', + integrationModule: 'mastra', }; /** A Rollup plugin context whose `resolve` answers however the test wants. */ @@ -39,37 +39,36 @@ async function start( } describe('createIntegrationModuleMatcher', () => { - const isFlueIntegrationModuleId = createIntegrationModuleMatcher('flue'); + const isMastraIntegrationModuleId = createIntegrationModuleMatcher('mastra'); it('matches the ESM integration module', () => { - expect(isFlueIntegrationModuleId(TARGET)).toBe(true); + expect(isMastraIntegrationModuleId(TARGET)).toBe(true); }); it('ignores a trailing query/hash Vite may append', () => { - expect(isFlueIntegrationModuleId(`${TARGET}?v=abc`)).toBe(true); + expect(isMastraIntegrationModuleId(`${TARGET}?v=abc`)).toBe(true); }); it('normalizes Windows separators', () => { expect( - isFlueIntegrationModuleId('C:\\app\\node_modules\\@sentry\\server-utils\\build\\esm\\integrations\\flue.js'), + isMastraIntegrationModuleId('C:\\app\\node_modules\\@sentry\\server-utils\\build\\esm\\integrations\\mastra.js'), ).toBe(true); }); it('does not match the CJS build (workers load ESM)', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/cjs/integrations/flue.js')).toBe( + expect(isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/cjs/integrations/mastra.js')).toBe( false, ); }); it('does not match another integration module', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js')).toBe( + expect(isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/flue.js')).toBe( false, ); }); it('does not match the instrumented package itself', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@flue/runtime/dist/index.mjs')).toBe(false); - expect(createIntegrationModuleMatcher('mastra')('/app/node_modules/@mastra/core/dist/index.js')).toBe(false); + expect(isMastraIntegrationModuleId('/app/node_modules/@mastra/core/dist/index.js')).toBe(false); }); }); diff --git a/packages/server-utils/src/integrations/flue.ts b/packages/server-utils/src/integrations/flue.ts index dacf328d72ce..92fe234894a5 100644 --- a/packages/server-utils/src/integrations/flue.ts +++ b/packages/server-utils/src/integrations/flue.ts @@ -21,12 +21,9 @@ let registeredBinding: FlueInstrumentFn | undefined; /** * Register the instrumentation with Flue on the user's behalf, when the runtime binding is available. * - * Flue is registered rather than patched — `instrument()` writes into module-scope state — so this - * needs a reference to that module's own binding. In a bundled worker there is no `node_modules` to - * resolve one from, so `@sentry/cloudflare/vite` splices a static `@flue/runtime` import into this - * module at build time and stashes the namespace on the global marker. Outside that setup the marker - * is empty and this no-ops, leaving the user's own `instrument(Sentry.createFlueInstrumentation())` - * as the way in. + * Flue is registered rather than patched, so this needs `@flue/runtime`'s own `instrument()`. In a + * bundled worker, `@sentry/cloudflare/vite` has the package publish it on the global marker. + * Elsewhere this no-ops and users call `instrument(Sentry.createFlueInstrumentation())` themselves. */ const _flueIntegration = ((options: FlueOptions = {}) => { return { diff --git a/packages/server-utils/src/orchestrion/bundler/moduleInjectedTransform.ts b/packages/server-utils/src/orchestrion/bundler/moduleInjectedTransform.ts index 737d9be98f40..b668b859ea14 100644 --- a/packages/server-utils/src/orchestrion/bundler/moduleInjectedTransform.ts +++ b/packages/server-utils/src/orchestrion/bundler/moduleInjectedTransform.ts @@ -76,17 +76,41 @@ function moduleInjectedSnippet( ? `import { ${bindings} } from ${JSON.stringify(importSpecifier)};` : `const { ${bindings} } = require(${JSON.stringify(importSpecifier)});`; - // `exportName` is itself an integration factory, invoked inside the arrow (`() => exportName()`), so - // `orchestrionModuleInjected`'s consumer still gets an Integration back from calling the stored - // thunk. The point of the arrow is to defer the read of the `exportName` binding to when that thunk - // runs (at `init()`), instead of reading it by reference the moment this snippet evaluates. A - // provided-module integration (e.g. `flueIntegration`) is imported back into its own instrumented - // package by `@sentry/*/vite`, so a direct reference here would close an import cycle and touch the - // binding in its TDZ ("Cannot access '…' before initialization"); deferring the read breaks that. + // The arrow defers reading `exportName` to `init()`. `@sentry/*/vite` imports some packages into + // their own integration module (e.g. Mastra's), and in that cycle a direct read would hit the TDZ. const args = exportName ? `${JSON.stringify(moduleName)}, () => ${exportName}()` : JSON.stringify(moduleName); return `${importStmt}\n${MODULE_INJECTED_SINK} = orchestrionModuleInjected(${args});`; } +/** + * Publishes the module's own namespace on `providedModules`. For `@flue/runtime`'s `dist/index.mjs`, + * this prepends roughly: + * + * ```js + * import * as __SENTRY_SELF_PROVIDED__ from "./index.mjs"; + * globalThis.__SENTRY_ORCHESTRION__.providedModules["@flue/runtime"] = __SENTRY_SELF_PROVIDED__; // as a getter + * ``` + * + * A self-import adds no edge to the module graph; importing the package into a Sentry module would + * close a cycle through every instrumented dependency, since those all import `@sentry/server-utils`. + */ +function selfProvidedSnippet(moduleName: string, filePath: string): string { + const marker = 'globalThis.__SENTRY_ORCHESTRION__'; + const self = `./${filePath.split('/').pop()}`; + return ( + `import * as __SENTRY_SELF_PROVIDED__ from ${JSON.stringify(self)};\n` + + `(${marker} = ${marker} || {});\n` + + `(${marker}.providedModules = ${marker}.providedModules || {});\n` + + `Object.defineProperty(${marker}.providedModules, ${JSON.stringify(moduleName)}, ` + + '{ configurable: true, enumerable: true, get() { return __SENTRY_SELF_PROVIDED__; } });' + ); +} + +export interface ModuleInjectedTransformsOptions { + /** Packages whose ESM entry publishes its own namespace on `providedModules`. */ + selfProvidedModules?: readonly string[]; +} + /** * The unified `customTransforms` every orchestrion bundler plugin (and the * webpack/Turbopack loader) applies: an override for orchestrion's built-in @@ -112,11 +136,12 @@ export function moduleInjectedTransforms( // absolute-path imports nor bare specifiers that don't resolve from the // importing file's location). importSpecifier?: string | (() => string | undefined), + { selfProvidedModules = [] }: ModuleInjectedTransformsOptions = {}, ): Record { const spliceModuleInjected = (state: unknown, program: unknown): void => { const { moduleType, module } = state as { moduleType?: string; - module?: { name?: string }; + module?: { name?: string; filePath?: string }; }; const node = program as ProgramNode; @@ -134,8 +159,13 @@ export function moduleInjectedTransforms( const specifier = (typeof importSpecifier === 'function' ? importSpecifier() : importSpecifier) ?? DEFAULT_IMPORT_SPECIFIER; const exportName = subscriberExportForModule(moduleName); - const statements = parse(moduleInjectedSnippet(moduleName, exportName, moduleType === 'esm', specifier), { - module: moduleType === 'esm', + const esm = moduleType === 'esm'; + const selfProvided = + esm && module?.filePath && selfProvidedModules.includes(moduleName) + ? `${selfProvidedSnippet(moduleName, module.filePath)}\n` + : ''; + const statements = parse(selfProvided + moduleInjectedSnippet(moduleName, exportName, esm, specifier), { + module: esm, next: true, }).body as ProgramNode['body']; diff --git a/packages/server-utils/src/orchestrion/bundler/options.ts b/packages/server-utils/src/orchestrion/bundler/options.ts index 0d9aec844d59..c28860e7bea9 100644 --- a/packages/server-utils/src/orchestrion/bundler/options.ts +++ b/packages/server-utils/src/orchestrion/bundler/options.ts @@ -43,6 +43,11 @@ export type PluginOptions = { * @default false */ transformOptimizedDeps?: boolean; + /** + * Packages that publish their own namespace on `providedModules`, for integrations that register + * with a package instead of patching it. Opt-in, as apps that register it themselves would clash. + */ + selfProvidedModules?: string[]; }; /** @@ -89,7 +94,10 @@ export function orchestrionTransformOptions( ): CodeTransformerPluginOptions { return { instrumentations: [...SENTRY_INSTRUMENTATIONS, ...(options.instrumentations || [])], - customTransforms: { ...options.customTransforms, ...moduleInjectedTransforms() }, + customTransforms: { + ...options.customTransforms, + ...moduleInjectedTransforms(undefined, { selfProvidedModules: options.selfProvidedModules }), + }, ...(options.dcModule && { dcModule: options.dcModule }), ...(injectDiagnostics && { injectDiagnostics: () => ORCHESTRION_BUNDLER_MARKER_BANNER }), }; diff --git a/packages/server-utils/src/orchestrion/config/flue.ts b/packages/server-utils/src/orchestrion/config/flue.ts index da6841a1ae9b..edf1f6d58cae 100644 --- a/packages/server-utils/src/orchestrion/config/flue.ts +++ b/packages/server-utils/src/orchestrion/config/flue.ts @@ -5,7 +5,7 @@ import { registrationOnly } from './registration-only'; * Flue publishes no diagnostics channels and needs none: it is instrumented by registering with * `instrument()`, not by patching call sites. Transforming the entry is only how the module's * integration gets registered at evaluation time, which is what installs it on a bundler-only SDK - * like `@sentry/cloudflare`. + * like `@sentry/cloudflare`. With `selfProvidedModules`, the entry also publishes its namespace. */ export const flueConfig = [ registrationOnly({ name: '@flue/runtime', versionRange: '>=2.0.0 <3.0.0', filePath: 'dist/index.mjs' }), diff --git a/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts b/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts index 5dac489035d1..73b7e14a9297 100644 --- a/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts +++ b/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts @@ -199,3 +199,64 @@ describe('module-injected transform', () => { expect(result!.code).not.toContain('require("@sentry/server-utils")'); }); }); + +describe('self-provided modules', () => { + let root: string; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'orch-self-provided-')); + makePackage(root, '@flue/runtime', '2.0.3', 'module', 'dist'); + makePackage(root, 'pg', '8.11.0', 'module'); + makePackage(root, 'ioredis', '5.11.0', 'commonjs', 'built'); + }); + + afterAll(() => { + rmSync(root, { recursive: true, force: true }); + }); + + const FLUE_ENTRY = 'import { r as instrument } from "./instrumentation.mjs";\nexport { instrument };\n'; + + it('publishes the namespace through a self-import, before registering the integration', () => { + const t = createCodeTransformer(orchestrionTransformOptions({ selfProvidedModules: ['@flue/runtime'] })); + const code = t.transform(FLUE_ENTRY, join(root, 'node_modules/@flue/runtime/dist/index.mjs'))!.code; + + expect(code).toContain('import * as __SENTRY_SELF_PROVIDED__ from "./index.mjs";'); + expect(code).toContain('Object.defineProperty(globalThis.__SENTRY_ORCHESTRION__.providedModules, "@flue/runtime"'); + expect(code).toMatch(/get\(\)\s*\{\s*return __SENTRY_SELF_PROVIDED__;\s*\}/); + // Registering can run `setup()` right away, which reads the namespace. + expect(code.indexOf('providedModules, "@flue/runtime"')).toBeLessThan( + code.indexOf('orchestrionModuleInjected("@flue/runtime"'), + ); + }); + + it('publishes nothing for a module that is not listed', () => { + const t = createCodeTransformer(orchestrionTransformOptions({})); + const code = t.transform(FLUE_ENTRY, join(root, 'node_modules/@flue/runtime/dist/index.mjs'))!.code; + + expect(code).toContain('orchestrionModuleInjected("@flue/runtime"'); + expect(code).not.toContain('__SENTRY_SELF_PROVIDED__'); + expect(code).not.toContain('providedModules'); + }); + + it('publishes nothing from a CJS module', () => { + const t = createCodeTransformer(orchestrionTransformOptions({ selfProvidedModules: ['ioredis'] })); + const code = t.transform( + "'use strict';\nclass Redis {}\nmodule.exports = Redis;\n", + join(root, 'node_modules/ioredis/built/Redis.js'), + )!.code; + + expect(code).toContain('orchestrionModuleInjected("ioredis"'); + expect(code).not.toContain('__SENTRY_SELF_PROVIDED__'); + }); + + it('publishes once per file', () => { + const t = createCodeTransformer(orchestrionTransformOptions({ selfProvidedModules: ['pg'] })); + // `pg`'s `lib/client.js` is matched by both the `query` and `connect` configs. + const code = t.transform( + 'export class Client { query(){} connect(){} }\n', + join(root, 'node_modules/pg/lib/client.js'), + )!.code; + + expect(code.match(/import \* as __SENTRY_SELF_PROVIDED__ from "\.\/client\.js";/g)).toHaveLength(1); + }); +});