[BUG]: `tsx` and auto-instrumentation of ESM modules does not work.

[stathis-alexander]

Tracer Version(s)

3.58.0

Node.js Version(s)

20.10

Bug Report

I have filed this against 3.58.0 but I believe it affects all versions.

Our auto instrumentation of graphql-yoga is not working for execute spans, despite it working for other packages and for validate and parse spans.

We use tsx to run out express app like so:

NODE_ENV=production tsx --import dd-trace/register.js src/main.ts

Only some of our auto-instrumentation works. At first, I believed this to be an issue with the specific file dd-trace registers against:

addHook({ name: '@graphql-tools/executor', file: 'cjs/execution/execute.js', versions: ['>=0.0.14'] }, execute => {
  shimmer.wrap(execute, 'execute', wrapExecute(execute))
  return execute
})

since we are using tsx and ESM directly, we are importing from esm/execution/execute.js and not cjs/execution/execute.js. I assumed that adding a second addHook would suffice, but it did not.

Upon further digging, it appears that addHook does not work for any imported modules, only required modules, which explains why some of our instrumentation was working on not others. While I was not able to ascertain the specific issue, I tracked it down to the import-in-the-middle dependency which just is not working with tsx. This is not well documented anywhere that I can find.

I worked around this issue by patching graphql-yoga to use a require instead,

+import Module from 'module';
+const require = Module.createRequire(import.meta.url);
+
+// There is a bug in `tsx` and `dd-trace`'s `import-in-the-middle` dependency that makes using `import` here not work
+// their `import` hook strategy. Monkey patch this to use `require` instead.
+const { normalizedExecutor } = require('@graphql-tools/executor');
+
 import { parse, specifiedRules, validate } from 'graphql';
 import { envelop, useEngine, useExtendContext, useMaskedErrors, } from '@envelop/core';
-import { normalizedExecutor } from '@graphql-tools/executor';

which is a temporary workaround, but it would be great if we could resolve the issue in dd-trace upstream somehow, so that auto-instrumentation works. I did not verify, but I suspect this may afflict ts-node also.

Reproduction Code

No response

Error Logs

No response

Tracer Config

import tracer from "dd-trace"

tracer.init({
  logInjection: true,
  sampleRate: 1,
})
tracer.use("express", { enabled: true })
tracer.use("pino", { enabled: true })
tracer.use("pg", {
  enabled: true,
  dbmPropagationMode: "service",
})
tracer.use("graphql", {
  enabled: true,
  depth: -1,
})

export default tracer

Operating System

No response

Bundling

Unsure

[BridgeAR]

A related issue in import-in-the-middle is https://github.com/nodejs/import-in-the-middle/issues/58.

That was resolved in a newer version than the one used in dd-trace 3.58.0. So updating could fix the issue. Could you please check if that works?

[stathis-alexander]

Hello @BridgeAR , thanks for the response.

I've recently updated dd-trace to 5.56.0 which relies on import-in-the-middle version 1.14.0, and the issue still persists. Additionally, if I pin import-in-the-middle to the latest version 1.14.2, the issue continues to persist.

The app has and continues to boot successfully and work just fine, the imported modules just do not get hooked correctly due to import-in-the-middle silently not working.

To confirm, I'm doing the following:

1. modify dd-trace/packages/datadog-instrumentation/src/graphql.js to include the following:

// copy of the existing, just `esm/`
addHook(
	{
		name: "@graphql-tools/executor",
		file: "esm/execution/execute.js",
		versions: [">=0.0.14"],
	},
	(execute) => {
		shimmer.wrap(execute, "execute", wrapExecute(execute));
		return execute;
	},
);

2. modify dd-trace/packages/datadog-plugin-graphql/src/execute.js to console.log in the start method of the plugin.

If I remove my require patch, the log statements from datadog-instrumentation/src/graphql.js do not appear. If I add the patch back, they do.

[BridgeAR]

We just added support for that and it should be included in the upcoming release :)

[joeyhotz]

Hi @BridgeAR, any way we can keep track of the fix here? 🙏 thank you