-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(node): Register ESM patching hooks in init
for supported Node.js versions
#11933
Conversation
packages/node/src/sdk/init.ts
Outdated
// Register hook was added in v20.6.0 and v18.19.0 | ||
if (nodeMajor > 22 || (nodeMajor === 20 && nodeMinor >= 6) || (nodeMajor === 18 && nodeMinor >= 19)) { | ||
// @ts-expect-error register is available in these versions & import.meta is allowed | ||
moduleModule.register('@opentelemetry/instrumentation/hook.mjs', import.meta.url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be sure, does it matter if this is registered twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I tested this and registering twice is a problem. We get around this for ourselves by only registering once and tracking with a global whether we have already registered.
I can see this becoming a problem if people have their own hooks registered already. I currently don't see a straight forward way of detecting this so for now we will just have to document this behaviour.
We should keep an eye open though for automatic detection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, LGTM!
size-limit report 📦
|
To avoid needing to have two
--require
flags when running a Node process (one for the ESM loader hooks, one forSentry.init()
), we can useSentry.init()
itself to register the ESM patching hooks.We
register
when we are on a compatible Node.js versionloader
flag on older versions if the the hook wasn't registered yet