Skip to content
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

Update esm-loader to work with Node 20 #1720

Closed
bizob2828 opened this issue Jul 12, 2023 · 4 comments · Fixed by #1760
Closed

Update esm-loader to work with Node 20 #1720

bizob2828 opened this issue Jul 12, 2023 · 4 comments · Fixed by #1760
Assignees

Comments

@bizob2828
Copy link
Member

Description

During our investigation of Node.js 20 it was discovered the runtime has changed how loaders work. They now run in their own threads: nodejs/node#44710. They provided a temporary hook globalPreload that shares context with the main runtime.

Acceptance Criteria

What tasks need to be accomplished to achieve the goal?

As a New Relic Node.js user I want the ability to run the node agent with ESM on Node.js 20 so that I can see my telemetry data in NR1.

Additional context

This will take some time to solve but I think we if we load the agent, shimmer, etc in the globalPreload hook we should be good.

@workato-integration
Copy link

@bizob2828 bizob2828 self-assigned this Jul 17, 2023
@bizob2828
Copy link
Member Author

I'm capturing my findings to date. We wrote the esm loader to really do 3 things:

  • Include fully resolved paths of CJS modules into shimmer so they could be instrumented properly
  • Rewrite ESM source to proxy a ES module to register instrumentation on first load.
  • Provide an ability to load custom instrumentation for ESM.

As described in the description above nodejs/node#44710 moved loaders to their own threads. This caused a few issues:

  • Needed a way to load agent in main thread for people using ESM loader
  • Needed a way to update the list of known instrumentations in main thread for CJS
  • Needed a way to register and execute ESM instrumentation.

I was able to load the agent in the main thread by implementing the globalPreload hook which you can see here. This wasn't loading custom ESM instrumentation because I couldn't seem to figure out how to import, I kept getting:

TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.
    at new NodeError (node:internal/errors:405:5)
    at importModuleDynamicallyCallback (node:internal/modules/esm/utils:91:9)
    at loadCustomEntryPoint (<preload>:17:9)
    at <preload>:21:5
    at #executePreloadScripts (node:internal/modules/esm/hooks:619:7)
    at #waitForWorker (node:internal/modules/esm/hooks:514:36)
    at HooksProxy.makeAsyncRequest (node:internal/modules/esm/hooks:522:24)
    at #getModuleJob (node:internal/modules/esm/loader:312:44)
    at CustomizedModuleLoader.getModuleJob (node:internal/modules/esm/loader:317:42)
    at CustomizedModuleLoader.import (node:internal/modules/esm/loader:227:28) {
  code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING'
}

With that being said, I moved on to figure out how to get the load hook to work to proxy ESM exports. It is worth noting the work mentioned above is going away as globalPreload was a near term solution. It's getting phased out by a combination of --import register, and initialize ESM hooks. We should wait for this work to land before trying to update our loader.
You can read more about it here, and this slack thread.

We will still need to figure out how to properly get the ESM exports so they can be instrumented. import-in-the-middle went the parse the ESM source into AST and get the exports that way. There was a long discussion about all of this and want to capture it in here. However it got closed because IITM just extract exports via AST parsing. We will have to wait if there is a native solution to this but sounds like there probably won't be.

With all that being said I propose we move this to the backlog and wait for Node. 20 to release a stable loader API to tackle. The reason I propose this is because we only have 1 custom using custom ESM instrumentation. We can get CJS working in ESM by implementing a more permissive matching of packages to our instrumentation. This is being done in #1646.

We can then release support for Node 20 and drop 14 and make sure we include the known issues with ESM custom instrumentation.

@bizob2828
Copy link
Member Author

Moving this back until the loader API has the new stable hook points

@bizob2828
Copy link
Member Author

This is being done is two parts which the PRs are #1758 and #1760

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
1 participant