[instrumentation] Consider IITM's include list (createAddHookMessageChannel) as the default for ESM, since wrapping every module breaks bundled apps

[segevfiner]

Note

This issue was researched and written by Claude (an AI agent) on behalf of @segevfiner, who reviewed it before filing. Findings below were verified against the installed packages on a real project — nothing is inferred from documentation alone.

Important

Overlap disclosed up front. This touches #4933 (which already shows createAddHookMessageChannel in a proposed bootstrap), shares a root cause with #6489 (IITM proxy holding stale snapshots), and is adjacent to #4845. I believe the ask here is distinct — default behaviour rather than documented bootstrap — but please close as a duplicate or fold it into #4933 if you disagree.

The problem

Registering @opentelemetry/instrumentation/hook.mjs rewrites every module in the graph. For bundled ESM applications that is not merely wasteful, it is a hard crash — and it happens with the hook alone, before any SDK is started.

import-in-the-middle replaces ESM live bindings with a snapshot taken when its wrapper evaluates. Bundlers emit lazily initialized bindings (esbuild's __esm helper, which rolldown also emits): var X; declared undefined and assigned later inside an init_*() function, relying on the live binding across a chunk boundary. The wrapper snapshots undefined, and a synchronous consumer reads that:

TypeError: Class extends value undefined is not a constructor or null

IITM does retry still-undefined exports, but only on a microtask and then at 0/10/50 ms, so a class X extends Y evaluated during module evaluation always loses that race. Filed upstream with a three-file reproduction that involves no OpenTelemetry and no bundler: nodejs/import-in-the-middle#280.

That crash is arguably IITM's to fix. What is in scope here is that OpenTelemetry chooses the configuration that triggers it, and doesn't have to.

Why the include list is a reasonable default for OTel specifically

In the common path, OpenTelemetry already knows the complete module set at sdk.start() — it is the code calling Hook() for each instrumentation. That is precisely the precondition IITM's include list requires. So the information needed to scope the hook is already in hand and simply isn't used:

• @opentelemetry/auto-instrumentations-node/build/src/register.js (0.79.0) constructs NodeSDK and calls sdk.start() with no include channel.
• The documented ESM bootstrap registers the hook bare, so everything is wrapped.

Concretely, on a bundled Fastify service this was the difference between crash-on-boot and working:

const { registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel();
module.register('@opentelemetry/instrumentation/hook.mjs', import.meta.url, registerOptions);

// … construct NodeSDK …
sdk.start();
await waitForAllMessagesAcknowledged();

With that change the app boots and instrumentation is fully live — not merely non-crashing:

@opentelemetry/instrumentation-http -> ['GET /health']
@fastify/otel                      -> ['handler - health', 'request']

Dropping the hook instead is not an alternative: the service then boots but emits only dns.lookup spans, losing every request span.

Proposal

Default to the include list where OTel controls the bootstrap — @opentelemetry/auto-instrumentations-node/register first, and the recommended ESM bootstrap in the docs — with an opt-out for the dynamic case.

The tradeoff, stated honestly

Wrapping everything is not merely a default-by-accident; it buys something real. addHook replays IITM's toHook list, so an instrumentation enabled after a module was imported still patches it retroactively. With an include list, the loader must know a module before it is imported, so registerInstrumentations() called after startup would silently stop instrumenting already-loaded modules.

That argues for an escape hatch, not against the default:

• The NodeSDK + fixed-instrumentation-list path — which is what register.js and the docs describe — has no late registration, so nothing is lost there.
• Late registration is the exception and can keep wrap-everything behaviour behind an explicit option.
• waitForAllMessagesAcknowledged() cannot be made implicit by IITM, since it can't know when an app has finished registering hooks — but OTel can know, because it owns sdk.start(). This asymmetry is the core of the argument: it is a default OTel can implement safely and IITM cannot.

Two smaller benefits come along with it: less transform work at startup, and no rewriting of application modules that no instrumentation will ever touch.

Environment

@opentelemetry/sdk-node	0.221.0
@opentelemetry/instrumentation	0.221.0
@opentelemetry/auto-instrumentations-node	0.79.0
import-in-the-middle	3.3.3
Node.js	v22.23.2
Bundler	rolldown via tsdown 0.22.14

Related

• Retry for delayed exports cannot cover synchronous consumers (class X extends Y) — follow-up to #227 nodejs/import-in-the-middle#280 — the underlying live-binding defect, with a minimal reproduction
• use module.register(...) in recommended bootstrap code for ESM support #4933 — module.register(...) in recommended ESM bootstrap (shows the same API)
• _httpPatched guard breaks ESM instrumentation when IITM proxy holds stale snapshots #6489 — _httpPatched guard breaks ESM instrumentation when the IITM proxy holds stale snapshots (same snapshot mechanism, different symptom)
• [instrumentation] update "Instrumentation for ES Modules" documentation #4845 — updating the "Instrumentation for ES Modules" documentation

[segevfiner]

Note

As with the issue body, this comment was researched and written by Claude (an AI agent) on behalf of @segevfiner, who reviewed it before posting. The measurements below are from real runs on Node v22.23.2 with import-in-the-middle@3.3.3.

Synchronous loader hooks change the cost of this proposal, not the underlying bug

Two things worth adding, since #6922 is now proposing to expose module.registerHooks()-based registration.

They do not fix the crash on their own

I checked whether running the loader synchronously avoids the live-binding problem from nodejs/import-in-the-middle#280 without any scoping. It does not:

$ node main.mjs                                # baseline, no hook
ok: Sub

$ node --import ./hook-async.mjs main.mjs      # module.register('.../hook.mjs')
TypeError: Class extends value undefined is not a constructor or null

$ node --import ./hook-sync.mjs main.mjs       # register() from register-hooks.mjs
supportsSyncHooks(): true
TypeError: Class extends value undefined is not a constructor or null

(main.mjs/dep.mjs are the three-file reproduction in nodejs/import-in-the-middle#280.)

That is expected once you look at where the defect lives: register-hooks.mjs calls the same createHook(import.meta) from create-hook.mjs, so it emits a byte-identical wrapper, and the wrapper is what severs the live binding — an exported let seeded once from the real namespace can never track it, regardless of which thread performed the rewrite.

The subtler part: the asynchrony that causes the failure is not the loader-thread round-trip that sync hooks remove. It is ModuleBinder.flush() (queueMicrotask, then RETRY_DELAYS = [0, 10, 50]), which lives in lib/register.js and is invoked from the generated wrapper — i.e. on the application thread in both modes. So sync hooks eliminate the one asynchrony that was never the problem here.

But they remove the main objection to scoping by default

The awkward part of the include-list approach in the issue body is the createAddHookMessageChannel / waitForAllMessagesAcknowledged() handshake: it is marked experimental, and it forces callers to interleave an await with sdk.start(). Under module.registerHooks() that handshake disappears — per import-in-the-middle's README, "Hook() registrations are visible to the loader directly, so the createAddHookMessageChannel / waitForAllMessagesAcknowledged step shown above is unnecessary."

All the scoping variants still rescue the case, in both modes:

SYNC  + include: [...]                -> ok: Sub
SYNC  + exclude: [/dep\.mjs$/]        -> ok: Sub
ASYNC + data.exclude: [/dep\.mjs$/]   -> ok: Sub

So the trade-off in the issue body stands unchanged — scoping still gives up toHook replay for instrumentations registered after startup — but the implementation cost of making it the default drops a lot once registration is in-thread.

The open question is the API, not the mechanism

Both mechanisms work; what needs deciding upstream is the shape:

• Is scoping the default, with an opt-out for late registerInstrumentations() calls, or opt-in?
• How do include / exclude get surfaced? Worth noting exclude is not marked experimental (unlike the hooked-modules-only channel), accepts regexes, and matches against URL, specifier, or resolved path — so exclude: [/\/dist\//] is a one-line escape hatch for a bundled app, and arguably the most useful thing to document for that audience.
• Sync hooks need >= 22.22.3 / 24.11.1 / 25.1.0 / 26.0.0 for the synchronous loader (supportsSyncHooks()), so any default has to degrade to the async loader on older supported versions — which means the message channel cannot be dropped outright, only avoided on newer runtimes.

Happy to test whatever shape you land on against the bundled-app case that motivated this — it reproduces reliably.