@opentelemetry/instrumentation crashes when used with typescript esModuleInterop: true

[matthias-pichler]

What happened?

Steps to Reproduce

Create a Node application in TypeScript setting esModuleInterop: true in tsconfig.json. Add otel instrumentations using the @opentelemetry/instrumentation package. Transpile the application using esbuild (with target format CommonJS). Try to run the application.

Expected Result

Since the final application is a CommonJS file (not ESM) it should run without crashing.

Actual Result

Build warnings are produced and the application crashes.

Additional Details

This StackOverflow answer suggests that the culprit is indeed esModuleInterop: true. It seems to me that all that is needed is the following change here:

- import * as RequireInTheMiddle from 'require-in-the-middle'
+ import RequireInTheMiddle from 'require-in-the-middle'

OpenTelemetry Setup Code

import os from "node:os";
import { Logger } from "@warrify/json-logger";
import {
  diag,
  DiagLogLevel,
  trace,
} from "@opentelemetry/api";
import { BatchSpanProcessor } from "@opentelemetry/sdk-trace-base";
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http";
import { NodeTracerProvider } from "@opentelemetry/sdk-trace-node";
import { registerInstrumentations } from "@opentelemetry/instrumentation";
import { AwsLambdaInstrumentation } from "@opentelemetry/instrumentation-aws-lambda";
import { DnsInstrumentation } from "@opentelemetry/instrumentation-dns";
import { AwsInstrumentation } from "@opentelemetry/instrumentation-aws-sdk";
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
import {
  SemanticAttributes,
  SemanticResourceAttributes,
  CloudProviderValues,
  CloudPlatformValues,
} from "@opentelemetry/semantic-conventions";

const logger = new Logger("tracing");

diag.setLogger(logger as any, DiagLogLevel.ALL);

try {
  const provider = new NodeTracerProvider();
  const spanProcessor = new BatchSpanProcessor(
    new OTLPTraceExporter({
      url: process.env.OTEL_EXPORTER_OTLP_ENDPOINT,
    })
  );
  
  provider.addSpanProcessor(spanProcessor);
  provider.register();
  trace.setGlobalTracerProvider(provider);

  registerInstrumentations({
    instrumentations: [
      new AwsInstrumentation({}),
      new AwsLambdaInstrumentation({
        requestHook: (span, { event, context }) => {
          span.setAttributes({
            [SemanticAttributes.AWS_LAMBDA_INVOKED_ARN]:
              context.invokedFunctionArn,
          });
        },
        responseHook: (span, { err, res }) => {
          if (err instanceof Error) {
            span.recordException(err);
          }
          if (res) {
            span.setAttribute("faas.res", res);
          }
        },
      }),
      new DnsInstrumentation({}),
      new HttpInstrumentation(),
    ],
  });
} catch (err: any) {
  logger.error(err);
}

package.json

{
  "dependencies": {
    "@opentelemetry/auto-instrumentations-node": "^0.36.4",
    "@opentelemetry/exporter-trace-otlp-http": "^0.36.1",
    "@opentelemetry/instrumentation": "^0.36.1",
    "@opentelemetry/instrumentation-aws-lambda": "^0.35.0",
    "@opentelemetry/instrumentation-aws-sdk": "^0.34.0",
    "@opentelemetry/instrumentation-dns": "^0.31.2",
    "@opentelemetry/instrumentation-http": "^0.36.1",
    "@opentelemetry/resources": "^1.10.1",
    "@opentelemetry/sdk-node": "^0.36.1",
    "@opentelemetry/sdk-trace-base": "^1.10.1",
    "@opentelemetry/sdk-trace-node": "^1.10.1",
    "@opentelemetry/semantic-conventions": "^1.10.1"
  }
}

Relevant log output

## during build 

▲ [WARNING] Calling "RequireInTheMiddle" will crash at run-time because it's an import namespace object, not a function [call-import-namespace]

    ../../.yarn/__virtual__/@opentelemetry-instrumentation-virtual-4be7f7a678/0/cache/@opentelemetry-instrumentation-npm-0.36.1-606efe6525-07401e23e0.zip/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/RequireInTheMiddleSingleton.js:63:8:
      63 │         RequireInTheMiddle(
         ╵         ~~~~~~~~~~~~~~~~~~

  Consider changing "RequireInTheMiddle" to a default import instead:

    ../../.yarn/__virtual__/@opentelemetry-instrumentation-virtual-4be7f7a678/0/cache/@opentelemetry-instrumentation-npm-0.36.1-606efe6525-07401e23e0.zip/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/RequireInTheMiddleSingleton.js:27:7:
      27 │ import * as RequireInTheMiddle from 'require-in-the-middle';
         │        ~~~~~~~~~~~~~~~~~~~~~~~
         ╵        RequireInTheMiddle


## during runtime


2023-03-27T16:46:22.383Z	undefined	ERROR	Uncaught Exception 	
{
    "errorType": "TypeError",
    "errorMessage": "RequireInTheMiddle3 is not a function",
    "stack": [
        "TypeError: RequireInTheMiddle3 is not a function",
        "    at RequireInTheMiddleSingleton3._initialize (/var/task/index.js:53274:9)",
        "    at new RequireInTheMiddleSingleton3 (/var/task/index.js:53270:14)",
        "    at RequireInTheMiddleSingleton3.getInstance (/var/task/index.js:53317:91)",
        "    at new InstrumentationBase3 (/var/task/index.js:53386:75)",
        "    at new AwsInstrumentation2 (/var/task/index.js:55209:9)",
        "    at Object.<anonymous> (/var/task/index.js:82015:5)",
        "    at Module._compile (node:internal/modules/cjs/loader:1218:14)",
        "    at Module._extensions..js (node:internal/modules/cjs/loader:1272:10)",
        "    at Module.load (node:internal/modules/cjs/loader:1081:32)",
        "    at Module._load (node:internal/modules/cjs/loader:922:12)"
    ]
}

[dyladan]

The suggested fix would require us to enable es module interop, which would in turn require our downstream consumers to enable it which we want to avoid. The real fix here is to add a named export to require in the middle so it can be used from Typescript more easily. The maintainer of that module was on the SIG call and is going to look into this for us.

[trentm]

I'll get a new release of require-in-the-middle out that does:

module.exports.Hook = Hook

My understanding from the call is that this will allow @opentelemetry/instrumentation to continue to use import * as RequireInTheMiddle from 'require-in-the-middle'.

[trentm]

@matthias-pichler-warrify I need some help reproducing what you see. I'd like to understand it and confirm that the proposed change in 'require-in-the-middle' will help.

I have an attempted repro here: https://github.com/trentm/repro-otel-js-issue-3701
The README shows my current status. There must be some difference between that and what you have in your project (code or tsconfig.json or esbuild options or "type": "module"). Can you either show a full repro that I could use or help me update my attempt to reproduce the same esbuild warnings?

[doronkopit5]

Any estimation when this will be released?

[pichlermarc]

Any estimation when this will be released?

It will be part of 1.13.0/0.39.0, next week most likely.