Skip to content

Commit

Permalink
feat: Expose exclude and include options for ESM loader (#12910)
Browse files Browse the repository at this point in the history
Closes #12878

I added this feature to `import-in-the-middle` which was released in
v1.9.0:
- nodejs/import-in-the-middle#124

This PR changes the hook from `@opentelemetry/instrumentation/hook.mjs`
to `import-in-the-middle/hook.mjs` as it was only pasing though anyway
and the otel hook doesn't pass the `initialize` export.
  • Loading branch information
timfish authored Jul 15, 2024
1 parent 05b2754 commit 1d3e208
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
2 changes: 1 addition & 1 deletion packages/node/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function _init(
}

if (!isCjs() && options.registerEsmLoaderHooks !== false) {
maybeInitializeEsmLoader();
maybeInitializeEsmLoader(options.registerEsmLoaderHooks === true ? undefined : options.registerEsmLoaderHooks);
}

setOpenTelemetryContextAsyncContextStrategy();
Expand Down
5 changes: 3 additions & 2 deletions packages/node/src/sdk/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { GLOBAL_OBJ, consoleSandbox, logger } from '@sentry/utils';

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';

Expand All @@ -31,7 +32,7 @@ export function initOpenTelemetry(client: NodeClient): void {
}

/** Initialize the ESM loader. */
export function maybeInitializeEsmLoader(): void {
export function maybeInitializeEsmLoader(esmHookConfig?: EsmLoaderHookOptions): void {
const [nodeMajor = 0, nodeMinor = 0] = process.versions.node.split('.').map(Number);

// Register hook was added in v20.6.0 and v18.19.0
Expand All @@ -43,7 +44,7 @@ export function maybeInitializeEsmLoader(): void {
if (!GLOBAL_OBJ._sentryEsmLoaderHookRegistered && importMetaUrl) {
try {
// @ts-expect-error register is available in these versions
moduleModule.register('@opentelemetry/instrumentation/hook.mjs', importMetaUrl);
moduleModule.register('import-in-the-middle/hook.mjs', importMetaUrl, { data: esmHookConfig });
GLOBAL_OBJ._sentryEsmLoaderHookRegistered = true;
} catch (error) {
logger.warn('Failed to register ESM hook', error);
Expand Down
18 changes: 16 additions & 2 deletions packages/node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import type { ClientOptions, Options, SamplingContext, Scope, Span, TracePropaga

import type { NodeTransportOptions } from './transports';

export interface EsmLoaderHookOptions {
include?: string[];
exclude?: string[];
}

export interface BaseNodeOptions {
/**
* List of strings/regex controlling to which outgoing requests
Expand Down Expand Up @@ -87,13 +92,22 @@ export interface BaseNodeOptions {

/**
* Whether to register ESM loader hooks to automatically instrument libraries.
* This is necessary to auto instrument libraries that are loaded via ESM imports, but might it can cause issues
* This is necessary to auto instrument libraries that are loaded via ESM imports, but it can cause issues
* 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`.
*/
registerEsmLoaderHooks?: boolean;
registerEsmLoaderHooks?: boolean | EsmLoaderHookOptions;

/** Callback that is executed when a fatal global error occurs. */
onFatalError?(this: void, error: Error): void;
Expand Down

0 comments on commit 1d3e208

Please sign in to comment.