Specify instrument modules instead of automatic detection

[ebuildy (Thomas Decaux)]

Using nodeJS 12 / ES6.

Some libraries are not detected because: import * as IORedis from 'ioredis';

We had to change it to require("ioredis"); to have ioredis lib detected.

I see there is something called "require-in-the-middle" that spy all nodejs require to detect automatically libraries used.

What about adding an option to specify this list instead of detecting it?

[alanstorm]

Thanks for reporting @ebuildy -- we've recently opened an issue to track our support for ECMAScript/import modules. You can find that issue on GitHub -- if you have additional thoughts we'd welcome your feedback there as well.

For broader context -- the require-in-the-middle module is how the Node.js knows you've called a certain method in your application, and how we add out method/span timings. A standard module for doing similar things with ECMAScript modules hasn't emerged yet.

If you have a moment to answer a question -- how are you using ECMAScript import statements in your Node 12 project? Are you compiling/transpiling your program, or are you using the experimental flags with Node.js to enable this feature? Or some third thing?

Thanks again for your feedback -- it helps us make the product better for everyone.

[ebuildy (Thomas Decaux)]

Thanks you, we use TypeScript , with target: es2017.

Is it possible to manually set instrumentation modules instead of relying on detection (which seem broken and also cost performance)?

[trentm (Trent Mick)]

@ebuildy Hi. Currently, no, there isn't a config variable to specify modules to instrument. There is this issue that considers a requireInstrumentations option that would support that.

I'll add a note on that ticket about this discussion/use case -- which helps with prioritization.

Note that there are still potential surprises when using ES modules (import ...). Because imports fully parse and execute all imports in a file before executing that file, that means that something like:

import apm from 'elastic-apm-node'
apm.start(/* my config ... */)

can result in instrumentation of modules happening after user code has references to those modules. Some instrumentation still works in this case, some doesn't.

As well, it is possible that some 3rd party packages will provide separate ES module and CommonJS module files. User usage of import 'foo', plus APM instrumentation of require('foo') will then lead to two separate loads of the same package, and hence broken instrumentation.

[trentm (Trent Mick)]

@ebuildy I was playing around a little bit, and here is a possible workaround. I'm not sure on how well this would integrate with your TypeScript usage.

Instead of starting your initial script with:

import apm from 'elastic-apm-node/start.js'

write a CommonJS apmsetup.cjs file like this:

'use strict'
module.exports = (function () {
    const apm = require('elastic-apm-node')
    apm.start({
        // ...
    })
    // Pre-require modules for instrumentation as a workaround for
    // `import 'foo'` not being detected.
    require('http')
    require('ioredis')
    return apm
})()

and replace the import apm ... above with:

import apm from './apmsetup.cjs'

In my small test that resulted in successfully getting http and ioredis instrumentation. I'm curious if this works for you.

[alanstorm]

Ah ha, that's interesting @ebuildy. I had not realized you were using typescript, and presumed you were using node 12's experimental support for ECMAScript modules. The Node.js Agent does support (certain configurations of) typescript. Now that we know you're using typescript I have a few additional questions/comments that might help/inform us here.

You mentioned you're using typescript with

target: es2017

First -- what is your module configuration set to? If it's set to "module": "commonjs" (TypeScript's default) you should end up with code that uses native require statements. They, in turn, should work with the agent. If it's set to something else, what is it set to?

Second -- how are you compiling your typescript? Is there a bundler (webpack, bablel, etc.) involved? If so, bundlers can often move/disrupt the requirement that the Node.js Agent be the first thing that's loaded in the program. If that's the case, loading the agent with a --require option

node --require elastic-apm-node [...other flags if you have them...] path/to/start/file.js

and using env variables for configuration can solve a whole host of issues related to a bundler/compilers that can't comply with "the agent must be first".

Thanks again for taking the time to report/answer these question. Getting these discussions out in the open almost always helps other folks who may be having similar issues, and helps make our product better.

[ebuildy (Thomas Decaux)]

Hello,

Sure I am always happy to help elastic guys, I follow elasticsearch from the beginning and love the products and also the philosophy (especially this last time!).

I am not the nodejs dev, so I try to give you best answer as possible:

File tsconfig.json:

{
  "compilerOptions": {
    "module": "commonjs",
    "declaration": true,
    "removeComments": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "target": "es2017",
    "sourceMap": true,
    "outDir": "./dist",
    "baseUrl": "./",
    "incremental": true,
    "paths": {
      "@app/*": ["src/*"],
      "@test/*": ["test/*"]
    }
  },
  "exclude": ["node_modules", "dist"]
}

This is a nestJS application, we setup elasticAPM in the main.ts file.

Actually I have found the issue, import * as IORedis from 'ioredis'; was ignored! I have added a dummy new IORedis(); then its ok!

So I am sorry, the root cause was include ioredis before apm.start();. But this do not work if we do this after, I am investigating.