elastic / elastic/apm-agent-nodejs

use IITM wrap-only-hooked support for ESM

Open
#4,176 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
594
Forks
244
Avg merge
1d 8h
Merged PRs (30d)
16

Description

IITM v1.11.0 includes support for only wrapping modules that are to be Hooked: https://github.com/nodejs/import-in-the-middle?tab=readme-ov-file#only-intercepting-hooked-modules
This avoid possible (even inevitable) issues with the best-effort wrapping that IITM does. If we only wrap the modules that we are going to hook for instrumentation (a known set that we test against), then we can be sure IITM isn't going to cause issues with other modules loaded in the user's app.

A possible benefit is a perf benefit as well, mentioned above. "Possible" because I haven't measured to see if this is at all significant overhead.

- Changing to wrap-only-hooked requires using `module.register(...)` so the `registerOptions` can be passed through. This means moving away from recommending `--experimental-loader=.../hook.mjs` at all for ESM support.
- Full usage of this IITM support means using top-level await, which means we now should recommend using `--import ...` over `--require ...` for starting the agent. (The race that motivated the `await waitForAllMessagesAcknowledged()` is, IIUC, theoretical, so using `--require ...` will probably still work FWIW.)
- This basically also means that we say ESM support is unsupported in earlier versions of Node.js that don'e have `module.register(...)` and `--import ...`. The required min Node.js versions for ESM support would then be: `^18.19.0 || >=20.6.0`.

See equiv issue for the OTel distro here: https://github.com/elastic/elastic-otel-node/issues/288

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.