opentelemetry-js

opentelemetry-js copied to clipboard

What happened?

Warning output from esbuild bundler.

▲ [WARNING] Constructing "ImportInTheMiddle" will crash at run-time because it's an import namespace object, not a constructor [call-import-namespace]
    node_modules/@opentelemetry/instrumentation-mongoose/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:259:30:
      259 │ ...     var esmHook = new ImportInTheMiddle([module_2.name], { in...
          ╵                           ~~~~~~~~~~~~~~~~~
  Consider changing "ImportInTheMiddle" to a default import instead:
    node_modules/@opentelemetry/instrumentation-mongoose/node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js:48:7:
      48 │ import * as ImportInTheMiddle from 'import-in-the-middle';
         │        ~~~~~~~~~~~~~~~~~~~~~~
         ╵        ImportInTheMiddle
6 of 16 warnings shown (disable the message limit with --log-limit=0)
  ...310dc1affb2720b6af2dd368949f880c1da144faa5a6860bc328c2/index.mjs  3.8mb ⚠️
⚡ Done in 144ms

From the code, I would have to agree with the warning that this will crash if used.

~~I don't understand how you could import like this:~~ It is not valid to import like this: https://github.com/open-telemetry/opentelemetry-js/blob/ca027b5eed282b4e81e098ca885db9ce27fdd562/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts#L28

And then instantiate like this: https://github.com/open-telemetry/opentelemetry-js/blob/ca027b5eed282b4e81e098ca885db9ce27fdd562/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts#L308-L313

It will absolutely throw an exception.

For ESM, the instantiation would need to look like:

var esmHook = new ImportInTheMiddle.default.default( ... );

~~But there are lots of things I do not know, so would be interested to hear if this would actually work!~~

@davidmurdoch are you also seeing these warnings? (ps- sorry for randomly @'ing you on this repo)

FYI - For ESM this is absolutely a problem.

Using the import syntax as is, the code would need to look like:

import * as ImportInTheMiddle from 'import-in-the-middle';

var esmHook = new ImportInTheMiddle.default.default( ... );

Hey, we also have ran into this problem.

I went down a rabbit hole of fixing this across the codebase https://github.com/open-telemetry/opentelemetry-js/pull/4519

But I didn't account for this being a breaking change for some configs.

@dyladan had a proposal that would fix this without the breaking change https://github.com/open-telemetry/opentelemetry-js/pull/4546

I currently don't have the time to do any work on this