Opentelemetry-js: Question: Plans for Node.js ESM Module Support?

[astorm]

• [X] This only affects the JavaScript OpenTelemetry library

Opening this issue based on a conversation with @dyladan and @obecny in order to start a wider discussion.

Has there been any talk about how the Node side of the project is going to address to coming ESM/ES6 module migration that (parts of) the node community is planning once Node 10 reaches end of life?

Specifically -- Node.js has added native support (no bundler needed) for the import statment and ESM/ES6 modules. These modules can't be monkey-patched in the same way that CommonJS can via require-in-the-middle. This means a lot of auto instrumentation is going to stop working as packages begin to adopt ESM modules and end users begin to use them.

Node's ESM modules have a loader API which _might_ offer a way to monkey-patch modules in the same way, but I don't know if anyone's done any public research on that yet. (waves in the direction of @michaelgoin for reasons)

In addition to the question of instrumentation, there's the question of what sort of modules this project will publish to NPM (CommonJS, ESM, both) and whether the project's individual modules will ship with with type attribute set or not.

Finally, FWIW, these questions come out of a discussion with @delvedor and @mcollina about fastify's plans for ESM support, and that @mcollina pulled together a list of the relevant Node.js core working groups and issues for folks interested in getting involved on that side of things.

[Flarna]

The main problem with the loader API is that it is still experimental and it doesn't seem that this changes soon.

The current limitations are severe, e.g. only one loader per process is allowed. Monitoring tools like OTel should not eat the unique resource.

I think the Otel project can't do much about this as it has to rely on the infrastructure available. Unfortunatelly this is only monkey patching for now - which seems to be "broken" by ESM. I remember some node PRs to improve loader APIs but some of them abandoned because endless discussions. But I think this is more a topic for node repo this one.

In short I fear that APM tools (incl Otel) and ESM will not fall in love soon.

[Flarna]

Another option to avoid the need for monkey patching / loader hooks would be to avoid creating speparate instrumentation packages.
Instead integrate OpenTelementry API directly into the frameworks in question. If they reject such a dependency at least hooks/events could be added to allow an instrumentation to get the needed data via a public, documented interface.

[dyladan]

Another option to avoid the need for monkey patching / loader hooks would be to avoid creating speparate instrumentation packages.
Instead integrate OpenTelementry API directly into the frameworks in question. If they reject such a dependency at least hooks/events could be added to allow an instrumentation to get the needed data via a public, documented interface.

This is the best case for us, but it will most likely be a slow process. It is already beginning in some places though https://github.com/typeorm/typeorm/issues/7406

[mcollina]

FYI, Fastify offers integration points to gather all sort of information. There are a few modules that provide open telemetry integration:

https://www.npmjs.com/package/fastify-opentelemetry

https://www.npmjs.com/package/@autotelic/fastify-opentelemetry

[weyert]

Another option to avoid the need for monkey patching / loader hooks would be to avoid creating speparate instrumentation packages.
Instead integrate OpenTelementry API directly into the frameworks in question. If they reject such a dependency at least hooks/events could be added to allow an instrumentation to get the needed data via a public, documented interface.

This is the best case for us, but it will most likely be a slow process. It is already beginning in some places though typeorm/typeorm#7406

Yeah, I created that ticket. We are actively using TypeORM at work and trying to find out if TypeORM would be interested in me making a pull request to built-in OpenTelemetry support

[michaelgoin]

Think I got pinged cause I'd done some early experimental module loader investigation and had proven out some concepts.

I was also going to mention I thought an early goal of these efforts (even before the merge into Open Telemetry) was to encourage framework authors to integrate Open Telemetry, given being a vendor-agnostic and hopefully future go-to standard. So perhaps focusing there is a better use of time, as others have beat me to. (Along w/ the integration points approach, although that then requires further instrumentation writing).

If auto-instrumentation was going to be a long-term goal, then I do think a project like this is perfect for helping Node evolve its story for providing good support for instrumenting ESM. Really, all APM/Observability vendors for their various needs but certainly this as an open standard.

[astorm]

Thanks for the background and the thoughts @Flarna -- they're appreciated and useful. I agree that an optimal end goal for the project is to have enough influence and sway that library and framework authors welcome adding instrumentation directly to their libraries and frameworks. I also agree that ESM loaders aren't quite there yet and aren't something oTel can do by itself with the current ESM implementation in Node.js.

That said -- right now there aren't many libraries with oTel instrumentation built in and monkey patching allows _anyone_ to write instrumentation for a framework. It seems like this patching of require is something that, at least in the medium term, is going to be needed if this project wants to see wider adoption. My assumption is there's a need for a similar monkey patching of modules loaded via the native import statment in Node.js, and that the loader mechanism is the what the Node.js core team has proposed as a replacement for directly patching require.

It's fair to say that it's ultimately up to Node's core team to provide a replacement for this lost functionality -- but it's equally fair to say that Node's core can't do this alone. They need both feedback and direct help from APM/Observability vendors. It seems like OpenTelemetry/CNCF are ideally situated to be a conduit for this.

This is just me thinking out loud but I wonder if the next best steps would be for someone on the OpenTelemetry side of the fence to develop an ESM loader prototype that could be used in place of require-in-the-middle? With a prototype written we'd have a piece of code to bring to Node's core team that would highlight the issues with the current loader implementation, and provide focus for the future work on ESM loaders.

Also -- speaking of future work on ESM loaders, based on these comments it sounds like Node.js core contributor @GeoffreyBooth is up for reviving a working group and mentoring folks who might be interested in working on these features and bringing some of the debates @Flarna mentioned to a close. This sounds like a great opportunity for anyone that's wanted to get involved and influence Node.js core development.

[astorm]

Think I got pinged cause I'd done some early experimental module loader investigation and had proven out some concepts.
No passive voice needed @michaelgoin, that's exactly why I pinged you. :)

Do you think that work is something that New Relic would be willing/able to share? It seems like it might be a good basis for the prototype I mentioned above (if OpenTelemetry decides an ESM loader is needed/appropriate).

[Flarna]

@astorm A while ago I ran across this which seems to be what you are looking for. But not sure if this fits current loader hooks or is based on an older variant.

And I agree that something like monkey patching will be needed. My main point was that once OTel API is GA we have now something in place which is not experimental _and_ open source to approach libraries. We should at least try to get monitoring support into the libraries instead of blind continue the error-prone monkey patching approach.

[michaelgoin]

@astorm I don't think I'd have issues getting NR to share but it looks like that article @Flarna shared is pretty similar to what we'd experimented with (leveraging params, etc.). If we do find a need, I can dig back through the old notes and see if there's anything additionally useful to surface up. We also experimented pretty early on so unsure how out of date things are. Or I could likely carve out time to help prototype (and document), as desired.

One thing that was the case back when I first researched, which may or may not still be the case, is CJS modules imported into ESM had full resolve paths so our module/instrumentation name matching was off for auto-instrumentation even with the common loader. We have not gotten to bridging that gap yet and unsure if that will be a problem for this project or not (can't remember how the auto-instrumentation is hooked up anymore). But that may be a small stop-gap-ish thing that may want to get solved regardless, if an issue.

On a further note, I'm confident NR would be willing to help instrument willing frameworks out there to help with Open Telemetry's success.

[chuanqisun]

Might be relevant: I was using a downstream library built with opentelemetry and it requires excessive configuration to make rollup bundler happy with opentelemetry. But I was still stuck because I was using vite, which is built on top of rollup and it conflicts with the vanilla rollup config. Then I cracked open the Azure sdk and realized it has written a money patch fix for opentelemetry's export issue.

So I wonder if opentelemetry can do something at the upstream so more bundlers can directly consume it. I also wonder if the folks from Azure SDK would be able to provide some help as they have already spent so much effort hacking around it. Here is their PR