feat(node)!: Remove fine grained registerEsmLoaderHooks option and addOpenTelemetryInstrumentation() (#14606)

Luca Forstner · web-flow · commit e190793e71b9 · 2024-12-19T14:14:17.000+01:00
diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-loader/src/instrument.mjs b/dev-packages/e2e-tests/test-applications/node-express-esm-loader/src/instrument.mjs
@@ -5,5 +5,4 @@ Sentry.init({
   dsn: process.env.E2E_TEST_DSN,
   tunnel: `http://localhost:3031/`, // proxy server
   tracesSampleRate: 1,
-  registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
 });
diff --git a/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs b/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs
@@ -13,7 +13,6 @@ Sentry.init({
   release: '1.0',
   autoSessionTracking: false,
   transport: loggingTransport,
-  registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
 });
 
 await import('./sub-module.mjs');
diff --git a/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/test.ts b/dev-packages/node-integration-tests/suites/esm/import-in-the-middle/test.ts
@@ -8,7 +8,7 @@ afterAll(() => {
 });
 
 conditionalTest({ min: 18 })('import-in-the-middle', () => {
-  test('onlyIncludeInstrumentedModules', () => {
+  test('should only instrument modules that we have instrumentation for', () => {
     const result = spawnSync('node', [join(__dirname, 'app.mjs')], { encoding: 'utf-8' });
     expect(result.stderr).not.toMatch('should be the only hooked modules but we just hooked');
   });
diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts
@@ -12,8 +12,6 @@ export {
   addEventProcessor,
   addIntegration,
   // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
-  // eslint-disable-next-line deprecation/deprecation
   addRequestDataToEvent,
   amqplibIntegration,
   anrIntegration,
diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts
@@ -119,8 +119,6 @@ export {
   spanToTraceHeader,
   spanToBaggageHeader,
   trpcMiddleware,
-  // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
   zodErrorsIntegration,
   profiler,
   amqplibIntegration,
diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts
@@ -141,8 +141,6 @@ export {
   spanToTraceHeader,
   spanToBaggageHeader,
   trpcMiddleware,
-  // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
   zodErrorsIntegration,
   profiler,
   amqplibIntegration,
diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts
@@ -118,8 +118,6 @@ export {
   spanToTraceHeader,
   spanToBaggageHeader,
   trpcMiddleware,
-  // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
   zodErrorsIntegration,
   profiler,
   amqplibIntegration,
diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts
@@ -60,8 +60,6 @@ export type { NodeOptions } from './types';
 export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData } from '@sentry/core';
 
 export {
-  // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
   // These are custom variants that need to be used instead of the core one
   // As they have slightly different implementations
   continueTrace,
diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts
@@ -132,7 +132,7 @@ function _init(
   }
 
   if (!isCjs() && options.registerEsmLoaderHooks !== false) {
-    maybeInitializeEsmLoader(options.registerEsmLoaderHooks === true ? undefined : options.registerEsmLoaderHooks);
+    maybeInitializeEsmLoader();
   }
 
   setOpenTelemetryContextAsyncContextStrategy();
diff --git a/packages/node/src/sdk/initOtel.ts b/packages/node/src/sdk/initOtel.ts
@@ -13,7 +13,6 @@ import { createAddHookMessageChannel } from 'import-in-the-middle';
 import { DEBUG_BUILD } from '../debug-build';
 import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
 import { SentryContextManager } from '../otel/contextManager';
-import type { EsmLoaderHookOptions } from '../types';
 import { isCjs } from '../utils/commonjs';
 import type { NodeClient } from './client';
 
@@ -34,30 +33,8 @@ export function initOpenTelemetry(client: NodeClient): void {
   client.traceProvider = provider;
 }
 
-type ImportInTheMiddleInitData = Pick<EsmLoaderHookOptions, 'include' | 'exclude'> & {
-  addHookMessagePort?: unknown;
-};
-
-interface RegisterOptions {
-  data?: ImportInTheMiddleInitData;
-  transferList?: unknown[];
-}
-
-function getRegisterOptions(esmHookConfig?: EsmLoaderHookOptions): RegisterOptions {
-  // TODO(v9): Make onlyIncludeInstrumentedModules: true the default behavior.
-  if (esmHookConfig?.onlyIncludeInstrumentedModules) {
-    const { addHookMessagePort } = createAddHookMessageChannel();
-    // If the user supplied include, we need to use that as a starting point or use an empty array to ensure no modules
-    // are wrapped if they are not hooked
-    // eslint-disable-next-line deprecation/deprecation
-    return { data: { addHookMessagePort, include: esmHookConfig.include || [] }, transferList: [addHookMessagePort] };
-  }
-
-  return { data: esmHookConfig };
-}
-
 /** Initialize the ESM loader. */
-export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions): void {
+export function maybeInitializeEsmLoader(): void {
   const [nodeMajor = 0, nodeMinor = 0] = process.versions.node.split('.').map(Number);
 
   // Register hook was added in v20.6.0 and v18.19.0
@@ -68,8 +45,12 @@ export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions):
 
     if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered && importMetaUrl) {
       try {
+        const { addHookMessagePort } = createAddHookMessageChannel();
         // @ts-expect-error register is available in these versions
-        moduleModule.register('import-in-the-middle/hook.mjs', importMetaUrl, getRegisterOptions(esmHookConfig));
+        moduleModule.register('import-in-the-middle/hook.mjs', importMetaUrl, {
+          data: { addHookMessagePort, include: [] },
+          transferList: [addHookMessagePort],
+        });
         GLOBAL_OBJ._sentryEsmLoaderHookRegistered = true;
       } catch (error) {
         logger.warn('Failed to register ESM hook', error);
@@ -88,7 +69,6 @@ export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions):
 interface NodePreloadOptions {
   debug?: boolean;
   integrations?: string[];
-  registerEsmLoaderHooks?: EsmLoaderHookOptions;
 }
 
 /**
@@ -105,7 +85,7 @@ export function preloadOpenTelemetry(options: NodePreloadOptions = {}): void {
   }
 
   if (!isCjs()) {
-    maybeInitializeEsmLoader(options.registerEsmLoaderHooks);
+    maybeInitializeEsmLoader();
   }
 
   // These are all integrations that we need to pre-load to ensure they are set up before any other code runs
diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts
@@ -5,43 +5,6 @@ import type { ClientOptions, Options, SamplingContext, Scope, Span, TracePropaga
 
 import type { NodeTransportOptions } from './transports';
 
-/**
- * Note: In the next major version of the Sentry SDK this interface will be removed and the SDK will by default only wrap
- * ESM modules that are required to be wrapped by OpenTelemetry Instrumentation.
- */
-export interface EsmLoaderHookOptions {
-  /**
-   * Provide a list of modules to wrap with `import-in-the-middle`.
-   *
-   * @deprecated It is recommended to use `onlyIncludeInstrumentedModules: true` instead of manually defining modules to include and exclude.
-   */
-  include?: Array<string | RegExp>;
-
-  /**
-   * Provide a list of modules to prevent them from being wrapped with `import-in-the-middle`.
-   *
-   * @deprecated It is recommended to use `onlyIncludeInstrumentedModules: true` instead of manually defining modules to include and exclude.
-   */
-  exclude?: Array<string | RegExp>;
-
-  /**
-   * When set to `true`, `import-in-the-middle` will only wrap ESM modules that are specifically instrumented by
-   * OpenTelemetry plugins. This is useful to avoid issues where `import-in-the-middle` is not compatible with some of
-   * your dependencies.
-   *
-   * **Note**: This feature will only work if you `Sentry.init()` the SDK before the instrumented modules are loaded.
-   * This can be achieved via the Node `--import` CLI flag or by loading your app via async `import()` after calling
-   * `Sentry.init()`.
-   *
-   * Defaults to `false`.
-   *
-   * Note: In the next major version of the Sentry SDK this option will be removed and the SDK will by default only wrap
-   * ESM modules that are required to be wrapped by OpenTelemetry Instrumentation.
-   */
-  // TODO(v9): Make `onlyIncludeInstrumentedModules: true` the default behavior.
-  onlyIncludeInstrumentedModules?: boolean;
-}
-
 export interface BaseNodeOptions {
   /**
    * List of strings/regex controlling to which outgoing requests
@@ -138,22 +101,9 @@ export interface BaseNodeOptions {
    * with certain libraries. If you run into problems running your app with this enabled,
    * please raise an issue in https://github.com/getsentry/sentry-javascript.
    *
-   * You can optionally exclude specific modules or only include specific modules from being instrumented by providing
-   * an object with `include` or `exclude` properties.
-   *
-   * ```js
-   * registerEsmLoaderHooks: {
-   *   exclude: ['openai'],
-   * }
-   * ```
-   *
    * Defaults to `true`.
-   *
-   * Note: In the next major version of the SDK, the possibility to provide fine-grained control will be removed from this option.
-   * This means that it will only be possible to pass `true` or `false`. The default value will continue to be `true`.
    */
-  // TODO(v9): Only accept true | false | undefined.
-  registerEsmLoaderHooks?: boolean | EsmLoaderHookOptions;
+  registerEsmLoaderHooks?: boolean;
 
   /**
    * Configures in which interval client reports will be flushed. Defaults to `60_000` (milliseconds).
diff --git a/packages/nuxt/src/server/sdk.ts b/packages/nuxt/src/server/sdk.ts
@@ -18,7 +18,6 @@ import type { SentryNuxtServerOptions } from '../common/types';
 export function init(options: SentryNuxtServerOptions): Client | undefined {
   const sentryOptions = {
     ...options,
-    registerEsmLoaderHooks: mergeRegisterEsmLoaderHooks(options),
     defaultIntegrations: getNuxtDefaultIntegrations(options),
   };
 
@@ -92,28 +91,6 @@ function getNuxtDefaultIntegrations(options: NodeOptions): Integration[] {
   ];
 }
 
-/**
- * Adds /vue/ to the registerEsmLoaderHooks options and merges it with the old values in the array if one is defined.
- * If the registerEsmLoaderHooks option is already a boolean, nothing is changed.
- *
- * Only exported for Testing purposes.
- */
-export function mergeRegisterEsmLoaderHooks(
-  options: SentryNuxtServerOptions,
-): SentryNuxtServerOptions['registerEsmLoaderHooks'] {
-  if (typeof options.registerEsmLoaderHooks === 'object' && options.registerEsmLoaderHooks !== null) {
-    return {
-      // eslint-disable-next-line deprecation/deprecation
-      exclude: Array.isArray(options.registerEsmLoaderHooks.exclude)
-        ? // eslint-disable-next-line deprecation/deprecation
-          [...options.registerEsmLoaderHooks.exclude, /vue/]
-        : // eslint-disable-next-line deprecation/deprecation
-          options.registerEsmLoaderHooks.exclude ?? [/vue/],
-    };
-  }
-  return options.registerEsmLoaderHooks ?? { exclude: [/vue/] };
-}
-
 /**
  * Flushes pending Sentry events with a 2-second timeout and in a way that cannot create unhandled promise rejections.
  */
diff --git a/packages/nuxt/test/server/sdk.test.ts b/packages/nuxt/test/server/sdk.test.ts
@@ -5,9 +5,8 @@ import { Scope } from '@sentry/node';
 import { getGlobalScope } from '@sentry/node';
 import { SDK_VERSION } from '@sentry/node';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
-import type { SentryNuxtServerOptions } from '../../src/common/types';
 import { init } from '../../src/server';
-import { clientSourceMapErrorFilter, mergeRegisterEsmLoaderHooks } from '../../src/server/sdk';
+import { clientSourceMapErrorFilter } from '../../src/server/sdk';
 
 const nodeInit = vi.spyOn(SentryNode, 'init');
 
@@ -163,42 +162,4 @@ describe('Nuxt Server SDK', () => {
       });
     });
   });
-
-  describe('mergeRegisterEsmLoaderHooks', () => {
-    it('merges exclude array when registerEsmLoaderHooks is an object with an exclude array', () => {
-      const options: SentryNuxtServerOptions = {
-        registerEsmLoaderHooks: { exclude: [/test/] },
-      };
-      const result = mergeRegisterEsmLoaderHooks(options);
-      expect(result).toEqual({ exclude: [/test/, /vue/] });
-    });
-
-    it('sets exclude array when registerEsmLoaderHooks is an object without an exclude array', () => {
-      const options: SentryNuxtServerOptions = {
-        registerEsmLoaderHooks: {},
-      };
-      const result = mergeRegisterEsmLoaderHooks(options);
-      expect(result).toEqual({ exclude: [/vue/] });
-    });
-
-    it('returns boolean when registerEsmLoaderHooks is a boolean', () => {
-      const options1: SentryNuxtServerOptions = {
-        registerEsmLoaderHooks: true,
-      };
-      const result1 = mergeRegisterEsmLoaderHooks(options1);
-      expect(result1).toBe(true);
-
-      const options2: SentryNuxtServerOptions = {
-        registerEsmLoaderHooks: false,
-      };
-      const result2 = mergeRegisterEsmLoaderHooks(options2);
-      expect(result2).toBe(false);
-    });
-
-    it('sets exclude array when registerEsmLoaderHooks is undefined', () => {
-      const options: SentryNuxtServerOptions = {};
-      const result = mergeRegisterEsmLoaderHooks(options);
-      expect(result).toEqual({ exclude: [/vue/] });
-    });
-  });
 });
diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts
@@ -58,8 +58,5 @@ export {
 
 export { openTelemetrySetupCheck } from './utils/setupCheck';
 
-// eslint-disable-next-line deprecation/deprecation
-export { addOpenTelemetryInstrumentation } from './instrumentation';
-
 // Legacy
 export { getClient } from '@sentry/core';
diff --git a/packages/opentelemetry/src/instrumentation.ts b/packages/opentelemetry/src/instrumentation.ts
diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts
@@ -16,8 +16,6 @@ export {
   addEventProcessor,
   addIntegration,
   // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
-  // eslint-disable-next-line deprecation/deprecation
   addRequestDataToEvent,
   amqplibIntegration,
   anrIntegration,
diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts
@@ -8,8 +8,6 @@ export {
   addEventProcessor,
   addIntegration,
   // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
-  // eslint-disable-next-line deprecation/deprecation
   addRequestDataToEvent,
   amqplibIntegration,
   anrIntegration,
diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts
@@ -8,8 +8,6 @@ export {
   addEventProcessor,
   addIntegration,
   // eslint-disable-next-line deprecation/deprecation
-  addOpenTelemetryInstrumentation,
-  // eslint-disable-next-line deprecation/deprecation
   addRequestDataToEvent,
   amqplibIntegration,
   anrIntegration,

Original file line number	Diff line number	Diff line change
`@@ -132,7 +132,7 @@ function _init(`
`132`	`132`	`}`
`133`	`133`
`134`	`134`	`if (!isCjs() && options.registerEsmLoaderHooks !== false) {`
`135`		`- maybeInitializeEsmLoader(options.registerEsmLoaderHooks === true ? undefined : options.registerEsmLoaderHooks);`
	`135`	`+ maybeInitializeEsmLoader();`
`136`	`136`	`}`
`137`	`137`
`138`	`138`	`setOpenTelemetryContextAsyncContextStrategy();`