elastic / apm-agent-nodejs

https://www.elastic.co/guide/en/apm/agent/nodejs/current/index.html
BSD 2-Clause "Simplified" License
581 stars 224 forks source link

elastic-apm-node/start cannot load config with type 'module' #1612

Open caritasverein opened 4 years ago

caritasverein commented 4 years ago

Describe the bug

When using --experimental-modules with the corresponding "type": "module", inside the package.json, the elastic-apm-node/start module is unable to load the config file:

Elastic APM initialization error: Can't read config file /home/luca/routeplanner-service/elastic-apm-node.js
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/luca/routeplanner-service/elastic-apm-node.js
require() of ES modules is not supported.
require() of /home/luca/routeplanner-service/elastic-apm-node.js from /home/luca/routeplanner-service/node_modules/elastic-apm-node/lib/config.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename elastic-apm-node.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/luca/routeplanner-service/package.json.

To Reproduce

Steps to reproduce the behavior: 1) Create a project that imports elastic-apm-node/start 2) Create the config file elastic-apm-node.js 2) Set the type: "module" setting in package.json 3) Launch project using --experimental-modules

Expected behavior

The elastic-apm-node/start is expected to load the configuration correctly using either the .cjs extension or making it available as an imported module itself.

Environment (please complete the following information)

How are you starting the agent? (please tick one of the boxes)

Additional context

delvedor commented 3 years ago

I have a similar issue when using native ESM in node v14 with "type":"module" and Fastify, I was able to solve it with this workaround:

// app.js
export default async function (fastify, opts) { ... }
// index.cjs
const apm = require('elastic-apm-node')
apm.start({ ... })

const fastify = require('fastify')({ logger: true })
fastify.register(import('./app.js'))
fastify.listen(3000)

And run the code with:

node index.cjs

@trentm let me know if I can help debug this one!

smoke commented 2 years ago

When forcing ECMAScript modules using "type": "module" in package.json or in other way as per https://nodejs.org/api/esm.html#enabling, I have managed to workaround this by forcing CommonJS modules for Elastic APM only in the following way:

// package.json
...
  "type": "module",
  "main": "app.js",
...
// app.js
import apm from './elastic-apm-node-start.cjs';
// elastic-apm-node-start.cjs
module.exports = require('elastic-apm-node').start(require('./elastic-apm-node.cjs'));
// elastic-apm-node.cjs
// @see https://www.elastic.co/guide/en/apm/agent/nodejs/current/configuration.html
module.exports = {
  serviceName: 'my-service'
}
trentm commented 1 year ago

My intent is to take this after early ESM work is in (https://github.com/elastic/apm-agent-nodejs/pull/3381). That work isn't required to improve here, but it is loosely related.

My plan is to:

  1. Improve the error message to make it clearer that the issue is that "elastic-apm-node.js" for config doesn't work from a type:"module" package.
  2. Then add support for "elastic-apm-node.cjs" that will be looked for first. In a type:"module" package we will be able to require("path/to/elastic-apm-node.cjs").

I notice that eslint config files do the same thing regarding a .cjs file and not supporting an ESM config file: https://eslint.org/docs/latest/use/configure/configuration-files

JavaScript (ESM) - use .eslintrc.cjs when running ESLint in JavaScript packages that specify "type":"module" in their package.json. Note that ESLint does not support ESM configuration at this time.