Skip to content

Commit a69da1b

Browse files
authored
feat(node): Option to only wrap instrumented modules (#13139)
Likely closes many issues but I don't want to auto-close anything specific here. We should probably confirm the issues are closed individually. `import-in-the-middle` by default wraps every ES module with a wrapping module that later allows it exports to be modified. This has issues though because the wrapping output of `import-in-the-middle` is not compatible with all modules. To help work around this I [added a feature](nodejs/import-in-the-middle#146) to `import-in-the-middle` that allows us to only wrap modules that we specifically need to instrument. ```ts import * as Sentry from '@sentry/node'; Sentry.init({ dsn: '__DSN__', registerEsmLoaderHooks: { onlyHookedModules: true }, }); ``` So far I've only changed the express integration test to use this new mode.
1 parent 80ec41a commit a69da1b

File tree

6 files changed

+70
-2
lines changed

6 files changed

+70
-2
lines changed

dev-packages/e2e-tests/test-applications/node-express-esm-loader/src/instrument.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ Sentry.init({
55
dsn: process.env.E2E_TEST_DSN,
66
tunnel: `http://localhost:3031/`, // proxy server
77
tracesSampleRate: 1,
8+
registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
89
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
import * as iitm from 'import-in-the-middle';
4+
5+
new iitm.Hook((_, name) => {
6+
if (name !== 'http') {
7+
throw new Error(`'http' should be the only hooked modules but we just hooked '${name}'`);
8+
}
9+
});
10+
11+
Sentry.init({
12+
dsn: 'https://[email protected]/1337',
13+
release: '1.0',
14+
autoSessionTracking: false,
15+
transport: loggingTransport,
16+
registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
17+
});
18+
19+
await import('./sub-module.mjs');
20+
await import('http');
21+
await import('os');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// eslint-disable-next-line no-console
2+
console.assert(true);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { conditionalTest } from '../../../utils';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
conditionalTest({ min: 18 })('import-in-the-middle', () => {
9+
test('onlyIncludeInstrumentedModules', done => {
10+
createRunner(__dirname, 'app.mjs').ensureNoErrorOutput().start(done);
11+
});
12+
});

packages/node/src/sdk/initOtel.ts

+22-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { SDK_VERSION } from '@sentry/core';
1111
import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
1212
import { GLOBAL_OBJ, consoleSandbox, logger } from '@sentry/utils';
13+
import { createAddHookMessageChannel } from 'import-in-the-middle';
1314

1415
import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
1516
import { SentryContextManager } from '../otel/contextManager';
@@ -31,6 +32,26 @@ export function initOpenTelemetry(client: NodeClient): void {
3132
client.traceProvider = provider;
3233
}
3334

35+
type ImportInTheMiddleInitData = Pick<EsmLoaderHookOptions, 'include' | 'exclude'> & {
36+
addHookMessagePort?: unknown;
37+
};
38+
39+
interface RegisterOptions {
40+
data?: ImportInTheMiddleInitData;
41+
transferList?: unknown[];
42+
}
43+
44+
function getRegisterOptions(esmHookConfig?: EsmLoaderHookOptions): RegisterOptions {
45+
if (esmHookConfig?.onlyIncludeInstrumentedModules) {
46+
const { addHookMessagePort } = createAddHookMessageChannel();
47+
// If the user supplied include, we need to use that as a starting point or use an empty array to ensure no modules
48+
// are wrapped if they are not hooked
49+
return { data: { addHookMessagePort, include: esmHookConfig.include || [] }, transferList: [addHookMessagePort] };
50+
}
51+
52+
return { data: esmHookConfig };
53+
}
54+
3455
/** Initialize the ESM loader. */
3556
export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions): void {
3657
const [nodeMajor = 0, nodeMinor = 0] = process.versions.node.split('.').map(Number);
@@ -44,7 +65,7 @@ export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions):
4465
if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered && importMetaUrl) {
4566
try {
4667
// @ts-expect-error register is available in these versions
47-
moduleModule.register('import-in-the-middle/hook.mjs', importMetaUrl, { data: esmHookConfig });
68+
moduleModule.register('import-in-the-middle/hook.mjs', importMetaUrl, getRegisterOptions(esmHookConfig));
4869
GLOBAL_OBJ._sentryEsmLoaderHookRegistered = true;
4970
} catch (error) {
5071
logger.warn('Failed to register ESM hook', error);

packages/node/src/types.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,18 @@ import type { NodeTransportOptions } from './transports';
66

77
export interface EsmLoaderHookOptions {
88
include?: Array<string | RegExp>;
9-
exclude?: Array<string | RegExp>;
9+
exclude?: Array<string | RegExp> /**
10+
* When set to `true`, `import-in-the-middle` will only wrap ESM modules that are specifically instrumented by
11+
* OpenTelemetry plugins. This is useful to avoid issues where `import-in-the-middle` is not compatible with some of
12+
* your dependencies.
13+
*
14+
* **Note**: This feature will only work if you `Sentry.init()` the SDK before the instrumented modules are loaded.
15+
* This can be achieved via the Node `--import` CLI flag or by loading your app via async `import()` after calling
16+
* `Sentry.init()`.
17+
*
18+
* Defaults to `false`.
19+
*/;
20+
onlyIncludeInstrumentedModules?: boolean;
1021
}
1122

1223
export interface BaseNodeOptions {

0 commit comments

Comments
 (0)