improve agent docs for use with TypeScript, bundlers, and perhaps ESM

[trentm]

There are a number of surprises/gotchas/not obvious hurdles to getting APM working with TypeScript, bundlers (like webpack), and ESM. The agent docs could be improved a lot for some of these cases. Comments below are collecting some Qs, gotchas, notes for this. (I realize that makes this issue a little fuzzy for "completion.")

initial description

See #1962 (comment) about it being hard to find the suggestion to use the npm -r elastic-apm-node/start app.js technique for those using bundlers (like webpack) or transpilers (like TypeScript) that might remove or re-order the "elastic-apm-node" import.

#608 was a similar issue a while back, for which #638 added a note to https://www.elastic.co/guide/en/apm/agent/nodejs/current/agent-api.html#apm-start

[trentm]

Note #2318 and this comment explaining my understanding: #2318 (comment)

[trentm]

• TypeScript will elide the following import:

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

  One can avoid that elision with:

  import 'elastic-apm-node/start'

  which is subtle enough in my experience to surprise users, because it means that commenting out the second line (which may be 100s of lines into the file) has a huge impact:

  import apm from 'elastic-apm-node/start'
  ...
  console.log('curr trans is:', apm.currentTransaction)

  Any use of the apm import will do. So we could suggest this usage to be more explicit:

  import apm from 'elastic-apm-node/start'; apm // Ensure import is not elided by tsc.

  The canonical TypeScript reference is https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-imports-being-elided-in-my-emit

• I am not yet positive if this at the top of a TypeScript file, or used in ESM (e.g. with node v16) suffices to have that Agent#start() setup hooks before subsequent imports:

  import apm from 'elastic-apm-node/start'
  import {Client} from 'pg'

  Or if the sometimes-suggestion to have a separate "apm.{js,mjs,ts}" file is preferred:

  // app.ts
  import apm from './apm'
  
  // apm.ts
  import apm from 'elastic-apm-node'
  apm.start({
      // ... any options wanted here
  })
  export default apm

  • Q: Figure out the answer for the above for TypeScript and native ESM.
  • Q: Can we have a single 'apm.js' that works for all cases, or do need/want to document 'apm.{js,mjs,ts}' for the various use cases? For example, if just 'apm.js', the TypeScript users may be hampered without types?

• Q: Wassup with there being a need to give the ".js" suffix on start.js in:

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

  I'd had a note about that for native ESM usage in node v16 (or with --experimental-modules). If that is required and having the suffix works for TypeScript use cases, then let's have the docs use that (and state why) to have one less way.

• Q: Do we need the esModuleInterop case to use:

  import * as apm from 'elastic-apm-node/start'

  If so, can we get a single form that works with/without esModuleInterop and with TypeScript and native ESM?

• Using 'externals' in webpack.config.js like this:

  ...
  externals: {
      'elastic-apm-node': 'commonjs elastic-apm-node'
  },
  ...

  Naively works to get instrumentation of core node modules only, e.g. the http module. It may work for adding other modules as well:

  ...
  externals: {
      'elastic-apm-node': 'commonjs elastic-apm-node',
      'pg': 'commonjs pg'
  },
  ...

  However, I does not work for log correlation via:

  ...
  externals: {
      'elastic-apm-node': 'commonjs elastic-apm-node',
      '@elastic/ecs-winston-format': 'commonjs @elastic/ecs-winston-format'
  },
  ...

  because I found that the built "dist/app.js" replaces the require('elastic-apm-node') in ecs-winston-format with the __webpack_require__, breaking the sniffing. Solutions:

  1. Something about the more complete 'webpack-node-externals' module works (and for all packages). So this is probably our best current workaround answer. However it may defeat the user's use case for webpack, perhaps not.

     // webpack.config.js
     const nodeExternals = require('webpack-node-externals')
     ...
     externals: [nodeExternals()],
     ...

  2. If we switch to the global var, this might just work. We should probably do that anyway, to handle the "weird, hard" case.

  3. Perhaps, however, it would be better for log correlation here to be from the APM side. It shouldn't just be Elastic APM here. The circular dep issue might be different here then.

[trentm]

The "weird, hard" case I'm thinking about is: a future Fleet-based deployment that:

1. sets NODE_OPTIONS='-r /opt/fleet/integrations/.../elastic-apm-node' to automatically instrument an application,
2. the application bundles in its own agent because it wants to do some manual instrumentation as well, and
3. the application uses webpack, so there are two separate elastic-apm-node modules in play.

What changes (docs and agent support) do we need for this to work? One likely agent change needed here is for the Agent singleton handling to move from depending on require.cache to using a property on global.

[trentm]

• Note that a dep on pkg-up (which I think we transitively use) might create a surprise for webpack users that don't have "*.json" in their extensions:

    resolve: {
        extensions: [".tsx", ".ts", ".js", ".json"]
    },

A possible webpack-using test case should perhaps have a case for ".json" not being in that set to see if things still work. If not, and if ".json" is not in the default webpack.config.js (via docs or whatever generator), then see if we can drop those deps.

Update 2022-07-26: This transitive dep on pkg-up has since been removed.

[trentm]

Note the excellent Fastify TypeScript intro docs that might be worth using as inspiration: https://github.com/fastify/fastify/blob/main/docs/TypeScript.md#getting-started

Update: Now here: https://github.com/fastify/fastify/blob/main/docs/Reference/TypeScript.md

[trentm]

Note bundler "fun" with pino@7 should we upgrade to pino@7 (or if a user passes in a pino@7 logger in config): pinojs/pino#1209

[trentm]

A related discuss topic: https://discuss.elastic.co/t/apm-agent-not-working-with-nodejs/295638

[trentm]

Perhaps related is support for (or at least setup and troubleshooting docs for) Next.js support. See #1611 and other enhancement requests.

[trentm]

• Q: Wassup with there being a need to give the ".js" suffix on start.js in:
  js import apm from 'elastic-apm-node/start.js'

  I'd had a note about that for native ESM usage in node v16 (or with `--experimental-modules`). If that _is_ required _and_ having the suffix works for TypeScript use cases, then let's have the docs use that (and state why) to have one less way.
  

Answering my own question. Attempting to import apm from 'elastic-apm-node/start' with pure ESM (i.e. not TypeScript translating to CommonJS) currently fails with:

% node foo2.mjs
node:internal/process/esm_loader:91
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../tmp/asdf/node_modules/elastic-apm-node/start' imported from .../tmp/asdf/foo2.mjs
Did you mean to import elastic-apm-node/start.js?
    at new NodeError (node:internal/errors:377:5)
    at finalizeResolution (node:internal/modules/esm/resolve:405:11)
    at moduleResolve (node:internal/modules/esm/resolve:966:10)
    at defaultResolve (node:internal/modules/esm/resolve:1174:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:605:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:318:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Node.js v18.3.0

This is because we are using a "Bare specifier" to the import statement that doesn't have an "export" in elastic-apm-node/package.json.

So if we add this to our package.json:

  "exports": {
    ".": "./index.js",
    "./start": "./start.js"
  },

then it "works". Where "it" here is:

// foo2.mjs
import 'elastic-apm-node/start';

import http from 'http';
const server = http.createServer((req, res) => {
    req.resume()
    res.statusCode = 200;
    res.end('pong');
});
server.listen(3000, () => {
    http.get('http://localhost:3000', res => {
        console.log('CLIENT: res.headers:', res.headers)
        res.resume()
        res.on('end', () => {
            console.log('CLIENT: res "end"')
            server.close()
        })
    })
});

And "works" is in quotes because this would only be a start to true ESM support, because we don't yet support auto-instrumenting import ...s of other modules.

This section of the docs has advice on whether to go "with extension" or "without extension": https://nodejs.org/api/packages.html#extensions-in-subpaths The first suggestion is to be consistently all one way. Later in that section it tends to imply that with the extension would be preferred. However, given that we already have -r elastic-apm-node/start in documentation, I think our choice has been made to go without extension.

tl;dr: The answer is to not have the ".js" in our docs for using import ... 'elastic-apm-node/start' and to eventually add that "exports" section to our package.json (perhaps as part of this change). One thing to watch out for is whether adding that "exports" section has impact on other usage: regular commonjs usage? Does it break some user attempting to require some subfile/module in our install tree? If so, we should look at those use cases to see if we need to support them.

Update 2022-07-26: My opinion on whether we should prefer "with extension" or "without extension" is wavering.

[trentm]

• Q: Do we need the esModuleInterop case to use:
  js import * as apm from 'elastic-apm-node/start'

  If so, can we get a single form that works with/without `esModuleInterop` and with TypeScript and native ESM?
  

Per some history and info from https://esbuild.github.io/content-types/#es-module-interop I think we should only bother worrying about support with esModuleInterop: true. Perhaps have a troubleshooting reference if there is breakage without it for those using TypeScript with the default config that doesn't have esModuleInterop set.