Skip to content

[v9] Change default of onlyIncludeInstrumentedModules in Node SDK #14332

Open

Description

Description

Today, by default we instrument all packages with import-in-the-middle in ESM.

For v9, we may flip the default of onlyIncludeInstrumentedModules from false to true, so that by default only the things you are adding instrumentation for in the --import hook are wrapped. This will work for everybody using the SDK to handle OTEL for them; only for users with a custom OTEL setup this may have problems.

To clarify, having this flag enabled can lead to issues in rare cases, e.g. if users run node --import instrument.mjs app.mjs with code like this:

// instrument.mjs 
import * as Sentry from '@sentry/node';

Sentry.init({
  skipOpenTelemetrySetup: true,
});

customOtelSetup();
// app.mjs
startApp();

// Now add more otel instrumentation, outside of --import
registerInstrumentations({ instrumentations });
// this will only affect stuff that is imported _after_ this code, so it will not work for many/most cases anyhow,
// but it is technically possible...

So we have a few possible permutations here, which are unfortunate:

  1. Default case: We register the loader hook & our default OTEL setup: ✅ onlyIncludeInstrumentedModules: true will always work and makes sense
  2. Users set registerEsmLoaderHooks: false: Users have to set up their own OTEL loader hook, in which case they have to configure things like what onlyIncludeInstrumentedModules themselves. ✅
    onlyIncludeInstrumentedModules is ignored anyhow, so default does not matter.
  3. Users use the Sentry ESM loader hook, but their own OTEL setup (so registerEsmLoaderHooks: true (default) and skipOpenTelemetrySetup: true): This is the tricky scenario.

There are a few possible ways we could go there:

  1. If skipOpenTelemetrySetup, registerEsmLoaderHooks is automatically false (so we never set up the loader for users that do skipOpenTelemetrySetup. While this kind of makes sense, it is definitely breaking for users of this setup (instrumentation will just stop). While setting this up yourself technically makes sense if you are shipping your own OTEL SDK, some users probably rely on us doing this for them right now.
  2. Just always default onlyIncludeInstrumentedModules to true, and if users rely on a scenario where they register instrumentation after --import, they need to flip this themselves. --> it's worth noting that I suspect even for custom otel setups this is a rather rare (but not impossible) scenario.
  3. Use a different default for onlyIncludeInstrumentedModules depending on if skipOpenTelemetrySetup is enabled

I think I would tend to solution 2 🤔 this is breaking but unlikely to affect a lot of users - it would only affect

a) People using a custom otel setup
b) ... who ALSO register instrumentation outside of --import

And those people only have to flip the setting back to false to get stuff working again 🤔

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions