nitrojs
Upgrade `@vercel/nft` to 0.27.4 to include ESM `module.register` hooks
Environment
ESM
Reproduction
Link: https://github.com/s1gr1d/nitro-sentry-iitm
This is how import-in-the-middle (IITM) looks like in .output/server/node_modules:
| Without resolution (uses @vercel/nft 0.26.x) | With @vercel/nft package resolution to 0.27.4 |
|---|---|
Describe the bug
Previous versions of @vercel/nft do not include ESM module.register hooks so the @opentelemetry/instrumentation/hook.mjs is not included, which leads to errors when starting the server if opentelemetry or packages that depend on it (like Sentry) are used.
@vercel/nft released a new version with the fix, but as it is still a 0 major version, the caret (^) does not work like usual. With 0 major versions, only the patch is automatically updated, but not the minor (npm source).
The current release of nitro depends on ^0.26.5 and the fix is included in 0.27.4 (another minor version).
It would be awesome to release a new nitro version (minor or patch), which includes this @vercel/nft minor version upgrade 🙌
Additional context
Issue Reference:
- https://github.com/vercel/nft/issues/428
- https://github.com/getsentry/sentry-javascript/issues/12603 (netlify-related, but this uses nft as well) - includes a reproduction
Logs
No response
We have already upgraded to 0.27 range in v2/main branches but since v2 branch has some risky changes that require more ecosystem testing, unfortunately, there is no immediate release planned.
In the meantime, it is possible to use nightly release channel to receive the update.
Thanks for this information :) Would it be possible to create a patch release (nitro 2.9.8) with just this nft change? The current 0.27 range is fine as well, but the current version 2.9.7 only includes 0.26. I understand that this might be a big request but with this update people would be able to use OTel instrumentation with nitro (OTel uses import-in-the-middle).
We can... Can you please help to make a minimal nitro reproduction for the issue with open telemetry dependency? I remember last time there were more issues specially on nodeless (edge) platforms this way at least we can make sure this releases fully solves the problem.
I updated the issue and created a minimal reproduction with Sentry as Sentry uses module.register to add import-in-the-middle to make it work with OTel: https://github.com/getsentry/sentry-javascript/blob/a8e5f593b3341f90cf9aafaaa1f6eb1e0da7545e/packages/node/src/sdk/initOtel.ts#L68
Hi @pi0, any decisions on this? This also holds up otel instrumentation for vinxi for example.
Thanks for reproduction @s1gr1d it helped me to understand the better of usage.
@andreiborza We are working on the v2 branch to be stable (we also already asked both Nuxt and solid/vinxi teams since last week to review the nightly channel) as soon as we are sure of stability will issue a new release, I hope (but don't promise) by early next week at latest for it to happen.
My understanding is that import-in-the-middle is an experimental feature (and it is for Node.js targets only). In the meantime (while I fully understand the inconvenience) you can offer users to opt into the nightly channel or manually add the resolution of vercel/nft.
On a relevant topic, if you are interested I would love to have better support of Sentry for Nitro (both Node.js and Edge) (long-time user!), i will contact you if you are intrested.
Hy, yeah adding a resolution is the only current workaround. And it is good to hear that another release is coming soon! Currently, the Nuxt and SolidStart SDKs are in alpha/beta and it would be nice for developers to use it without the resolution once the proper release of the SDK comes out.
And we are about to start working on a Nitro instrumentation: https://github.com/getsentry/sentry-javascript/issues/13670 It's great to hear that we can reach out to you!
Sounds exciting. I could not find any contact in X. Would you mind to drop me a DM in discord (@pi0) or X (@_pi0_)? 🙏🏼
We've been using a manual OpenTelemetry instrumentation for our projects and the idea of having an officially supported version is exciting!
Have you given any thought to supporting Native Instrumentation within the Nitro package itself? (Similar to NextJS) With an external package it can be difficult to inject the necessary code at the right points. For example, in our instrumentation package we have to use a nitro plugin where we get a handle on the h3 app and patch the nitro router to wrap each request in a handle. It works, but as with all code that depends on internals there is the risk of future breakage.
https://opentelemetry.io/docs/concepts/instrumentation/libraries/
@cjpearson Actually yes one of the reasons I am thinking of nitro-native support is to have a cross-platform instrument system. I don't prefer the complexity of open telemetry Node.js SDK either.
Hi, I'm using Nuxt 3 and already add resolutions for @vercel/nft but still didn't work. Here's my package version:
"nuxt": "^3.9.0",
2.10 is released with vercel/nft dependency update.
otel support requires more works.
@pi0 any updates on otel
We are working with sentry team to leverage Node.js built-in tracing channel in Nitro v3 (already landed in srvx and h3 v2). This means there will be no need of custom instruments and tracing natively working.