Description
openedon Nov 18, 2024
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:
- Default case: We register the loader hook & our default OTEL setup: ✅
onlyIncludeInstrumentedModules: true
will always work and makes sense - Users set
registerEsmLoaderHooks: false
: Users have to set up their own OTEL loader hook, in which case they have to configure things like whatonlyIncludeInstrumentedModules
themselves. ✅
onlyIncludeInstrumentedModules
is ignored anyhow, so default does not matter. - Users use the Sentry ESM loader hook, but their own OTEL setup (so
registerEsmLoaderHooks: true
(default) andskipOpenTelemetrySetup: true
): This is the tricky scenario.
There are a few possible ways we could go there:
- If
skipOpenTelemetrySetup
,registerEsmLoaderHooks
is automaticallyfalse
(so we never set up the loader for users that doskipOpenTelemetrySetup
. 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. - 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. - Use a different default for
onlyIncludeInstrumentedModules
depending on ifskipOpenTelemetrySetup
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