http & https module instrumentation breaks "stripe"

[mikecann]

It appears that the instrumentation of the "http" or "https" modules breaks the "stripe" (https://www.npmjs.com/package/stripe) package.

[web] - C:\dev\markd\battletabs\packages\server\dist\web.js
[web]     at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
[web]     at Module.Hook._require.Module.require (C:\dev\markd\battletabs\packages\server\dist\web.js:839624:33)
[web]     at require (node:internal/modules/cjs/helpers:102:18)
[web]     at C:\dev\markd\battletabs\packages\server\dist\web.js:842838:18
[web]     at Instrumentation._patchModule (C:\dev\markd\battletabs\packages\server\dist\web.js:842975:22)
[web]     at C:\dev\markd\battletabs\packages\server\dist\web.js:842955:22
[web]     at Module.Hook._require.Module.require (C:\dev\markd\battletabs\packages\server\dist\web.js:839688:37)
[web]     at require (node:internal/modules/cjs/helpers:102:18)
[web]     at ../../node_modules/stripe/lib/net/NodeHttpClient.js (C:\dev\markd\battletabs\packages\server\dist\web.js:151846:16)
[web]     at __require (C:\dev\markd\battletabs\packages\server\dist\web.js:38:45)
[web]     at Function.Stripe2.createNodeHttpClient (C:\dev\markd\battletabs\packages\server\dist\web.js:152134:34)
[web]     at new Stripe2 (C:\dev\markd\battletabs\packages\server\dist\web.js:152112:49)
[web]     at getStripeClient (C:\dev\markd\battletabs\packages\server\dist\web.js:1028946:21)
[web]     at Object.Stripe (C:\dev\markd\battletabs\packages\server\dist\web.js:1029253:38)
[web]     at matchLiteral10 (C:\dev\markd\battletabs\packages\server\dist\web.js:9023:87)
[web]     at getSKUs (C:\dev\markd\battletabs\packages\server\dist\web.js:1029252:53)
[web]     at getSKUsHandler (C:\dev\markd\battletabs\packages\server\dist\web.js:1031113:22)
[web]     at onSocketEndpointRequest (C:\dev\markd\battletabs\packages\server\dist\web.js:1036982:26)
[web]     at Socket5.<anonymous> (C:\dev\markd\battletabs\packages\server\dist\web.js:1037027:71)
[web]     at Socket5.emit (node:events:390:28)
[web]     at Socket5.emit (node:domain:475:12)
[web]     at Socket5.emitUntyped (C:\dev\markd\battletabs\packages\server\dist\web.js:992036:22)
[web]     at C:\dev\markd\battletabs\packages\server\dist\web.js:992440:33
[web]     at processTicksAndRejections (node:internal/process/task_queues:78:11) {
[web]   code: 'MODULE_NOT_FOUND',
[web]   requireStack: [ 'C:\\dev\\markd\\battletabs\\packages\\server\\dist\\web.js' ]
[web] }

The above error happens when trying to create the Slack client after instrumentation.

https://github.com/elastic/apm-agent-nodejs/blob/master/lib/instrumentation/index.js#L236 line outputs that its patching "http" version 17.3.0.

If I jump out of that function but returning "exports" at the top then I dont get the error so its definately to do with the instrumentation of it.

I have tried disableInstrumentations: ['http', 'https'], in the config and instrument: false, and neither seem to work, it seems like instrumentation is still applied?!

I am using node 17.3.0 and esbuild to bundle my node application.

[mikecann]

I actually think the issue is to do with the way esbuild bundles node modules around this line: https://github.com/mikecann/apm-agent-nodejs/blob/master/lib/instrumentation/index.js#L246 I think you cant do dynamic imports when using a bundler like esbuild ..

By adding this hack then the issue goes away: https://github.com/mikecann/apm-agent-nodejs/blob/master/lib/instrumentation/index.js#L238

[mikecann]

I dont want to rely on my fork indefinately, would be be interested in that "hack" making it into the lib?

[astorm]

Hey @mikecann, thanks for reporting. Do you happen to have a program and esbuild configuration that reproduces the problem you've reported? If you can provide that we can take a look at the underlying reason for this issue and determine if there's a way to solve it that preserves the instrumentation (vs. your solution which changes the definition of "disabled" to include not hooking into the module at all).

[mikecann]

Hi @astorm thanks so much for your kindly worded reply.

I was worried putting together an example might take a long time but it turn out it wasnt too bad.

This repo replicates the issue: https://github.com/mikecann/example-elastic-stripe-issue

[mikecann]

Okay a bit more info on this.

On a whim I decided to see if tsup would have any more luck. It uses esbuild behind the scenes so should have the same issue.

https://github.com/mikecann/example-elastic-stripe-issue/tree/tsup

Well to my surprise it worked!

After some digging I realise that they externalise ALL modules. So I experimented with my own esbuild setup a little more exernalising a few things and huzzah!

https://github.com/mikecann/example-elastic-stripe-issue/tree/manual-esbuild

It turns out you need to externalise just "elastic-apm-node" to get it to work :)

[trentm]

@mikecann Thanks for the issue. I just merged #2837 which improves the APM agent's docs to discuss issues with bundlers, including esbuild. Basically, as you found, the APM agent doesn't support being bundled, so 'elastic-apm-node' must be made external. In addition, any module that you want the APM agent to be able to instrument (for example 'pg', 'express', etc.) must also be made external.

Bundlers are discussed here in the APM agent docs: https://www.elastic.co/guide/en/apm/agent/nodejs/master/starting-the-agent.html#start-bundlers (scroll down for the esbuild-specific section). There is also a small example showing using esbuild and the agent here: https://github.com/elastic/apm-agent-nodejs/tree/main/examples/esbuild

This is all just a workaround. Having to externalize "elastic-apm-node" (and possibly other modules) means that for deployment, one needs to deploy parts or all of "node_modules/...". It would be nice if the APM agent could be made to work in a bundle. However, we don't have current plans to dig into that yet -- it would be a significant amount of work.

Thanks very much for reporting this. Please re-open if I've missed something.