opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Semver type definition can't be resolved from @opentelemetry/instrumentation when use rollup bundler

Open tsubomii opened this issue 1 year ago • 10 comments
trafficstars

What happened?

Steps to Reproduce

The following is the rollup.config.js I use to create a browser esm bundle. I also tried to add "@types/semver": "^7.5.6" in my package.json, still doesn't stop the error in Actual Result section

import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import { getBabelOutputPlugin } from '@rollup/plugin-babel';
import sourcemaps from 'rollup-plugin-sourcemaps';
import json from '@rollup/plugin-json';

function getBabelPlugin() {
  return getBabelOutputPlugin({
    // default target to es5
    presets: ['@babel/preset-env'],
    allowAllFormats: true
  });
}

const plugins = [
  sourcemaps(),
  json(),
  resolve({
    browser: true
  }),
  commonjs({
 /** I have to manually create type definition inside my code, because the following error from RequireInTheMiddleSingleton.js from otel
[!] Error: 'Hook' is not exported by node_modules/require-in-the-middle/index.js, imported by node_modules/@opentelemetry/instrumentation/build/esm/platform/node/RequireInTheMiddleSingleton.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
node_modules/@opentelemetry/instrumentation/build/esm/platform/node/RequireInTheMiddleSingleton.js (27:9)
25:     throw new TypeError(s ? "Object is not iterable." : "Symbol.iterator is not defined.");
26: };
27: import { Hook } from 'require-in-the-middle';
**/
    include: ['node_modules/uuid/**/*','node_modules/shimmer/index.js', 'node_modules/semver/**/*', 'node_modules/require-in-the-middle/index.js']
  }),
  getBabelPlugin(),
];

export default {
  onwarn(warning, warn) {
  // I have to suppress a lot undefined warning from Otel plugins
    if (warning.code === 'THIS_IS_UNDEFINED') return;
    warn(warning);
  },
  input: 'dist/index.js',
  output: [
    {
      format: 'cjs',
      sourcemap: true,
      file: 'dist/bundle.js'
    },
    {
      format: 'esm',
      sourcemap: true,
      file: 'dist/bundle.esm.mjs'
    }
  ],
  plugins
};

Following are relevant typescript compiler options from tsconfig.json

 "module": "es2015",
    "moduleResolution": "node",
    "target": "ESNext",
    "noImplicitAny": true,
    "strictNullChecks": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "removeComments": false,
    "preserveConstEnums": true,
    "sourceMap": true,
    "esModuleInterop": false,
    "types": [],
    "outDir": "dist",
    "lib": ["dom", "es5", "es2015.promise", "es2015.collection", "es2015.iterable", "es2019", "es2020.promise"],
    "typeRoots": [],
    "baseUrl": ".",
    "paths": {
      "uuid/v4": ["types/uuid/v4.d.ts"],
      "semver": ["node_modules/@types/semver/index.d.ts"],
      "shimmer": ["types/shimmer/index.d.ts"],
      "require-in-the-middle": ["types/require-in-the-middle/index.d.ts"],
    }

Expected Result

Generate dist/bundle.esm.mjs

Actual Result

[!] Error: 'satisfies' is not exported by node_modules/@opentelemetry/instrumentation/node_modules/semver/index.js, imported by node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js (44:9)
42: import * as path from 'path';
43: import { types as utilTypes } from 'util';
44: import { satisfies } from 'semver';
             ^
45: import { wrap, unwrap } from 'shimmer';
46: import { InstrumentationAbstract } from '../../instrumentation';
Error: 'satisfies' is not exported by node_modules/@opentelemetry/instrumentation/node_modules/semver/index.js, imported by node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js
    at error (/myrepo/node_modules/rollup/dist/shared/rollup.js:198:30)

Additional Details

I tried setup using auto-Instrumentations-web plugin or the individual plugins, see below otel setup code. Both have the same compilation error. I got the " 'Hook' is not exported by node_modules/require-in-the-middle/index.js" error first. Then I work around it by copying the following type definition from otel to my code base. However, this shouldn't be the way how down stream consume otel library

declare module 'require-in-the-middle' {
  namespace hook {
    type Options = {
      internals?: boolean;
    };
    type OnRequireFn = <T>(exports: T, name: string, basedir?: string) => T;
    type Hooked = { unhook(): void };
  }
  function hook(
    modules: string[] | null,
    options: hook.Options | null,
    onRequire: hook.OnRequireFn
  ): hook.Hooked;
  function hook(
    modules: string[] | null,
    onRequire: hook.OnRequireFn
  ): hook.Hooked;
  function hook(onRequire: hook.OnRequireFn): hook.Hooked;
  export = hook;
}

After I work around the require-in-the-middle type error, then I got the error from semver. Then I tried to add "@types/semver": "^7.5.6" in my package.json. This doesn't help. Also, I don't think it's the right way to get otel library ESM build in general. Please provide some insight. Thank you!!!

OpenTelemetry Setup Code

const contextManager = new ZoneContextManager()
  const webProvider = new WebTracerProvider({
      resource: new Resource({
        [SemanticResourceAttributes.SERVICE_NAME]: 'my-service',
        [SemanticResourceAttributes.SERVICE_VERSION]: '0.0.1',
      })
    });
    webProvider.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter()));
    webProvider.register({
      contextManager: contextManager
    });
    contextManager.enable();

    registerInstrumentations({
      instrumentations: [
        getWebAutoInstrumentations({
          // load custom configuration for xml-http-request instrumentation
          '@opentelemetry/instrumentation-xml-http-request': {
            propagateTraceHeaderCorsUrls: [
              /.+/g,
            ],
          },
          // load custom configuration for fetch instrumentation
          '@opentelemetry/instrumentation-fetch': {
            propagateTraceHeaderCorsUrls: [
              /.+/g,
            ],
          },
        }),
        //new FetchInstrumentation(),
        // new DocumentLoadInstrumentation(),
        // new XMLHttpRequestInstrumentation()
      ],
      tracerProvider: webProvider
    });

package.json

"dependencies": {
    .....
    "@opentelemetry/api": "^1.7.0",
    "@opentelemetry/auto-instrumentations-web": "^0.35.0",
    "@opentelemetry/context-zone": "^1.20.0",
/** I have to ping to 1.19.0 because between @opentelemetry/sdk-trace-base version 1.19.0 and 1.20.0 there is backwards incompatible definition of Span and Tracer **/
    "@opentelemetry/sdk-trace-web": "1.19.0",
    "@opentelemetry/semantic-conventions": "1.19.0",
  ...
"devDependencies": {
....
"@types/semver": "^7.5.6",
....
},
"resolutions": {
    "@opentelemetry/sdk-trace-base": "1.19.0"
  },
}

Relevant log output

No response

tsubomii avatar Jan 31 '24 01:01 tsubomii

Can you have a look at the opentelemetry-sandbox-web-js as we use rollup to construct bundles for web instrumentations.

Here is the rollup configuration that is used their, where we include nodeResolve with the browser:true as well https://github.com/open-telemetry/opentelemetry-sandbox-web-js/blob/d8fab86386dd3cc207b845a3ea3ea31ed74967ea/rollup.base.config.js#L110-L134

MSNev avatar Jan 31 '24 17:01 MSNev

Thanks a lot for the quick response @MSNev. I just realize the error message is from the node bundle build, not from the browser one. Sorry about the wrong description in the issue. My app does server side rendering. I have two rollup.config, one for browser, the other is for node. The one I pasted in the description is for browser. The following is for Node.

import nodeResolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import { getBabelOutputPlugin } from '@rollup/plugin-babel';
import sourcemaps from 'rollup-plugin-sourcemaps';
import json from '@rollup/plugin-json';

function getBabelPlugin() {
  return getBabelOutputPlugin({
    presets: [['@babel/preset-env']]
  });
}

const plugins = [
  sourcemaps(),
  json(),
  nodeResolve({
    browser: false,
    exportConditions: ['node']
  }),
  commonjs({
    include: ['node_modules/uuid/**/*', 'node_modules/semver/**/*', 'node_modules/shimmer/index.js', 'node_modules/require-in-the-middle/index.js']
  }),
  getBabelPlugin()
];

export default {
  onwarn(warning, warn) {
    if (warning.code === 'THIS_IS_UNDEFINED') return;
    warn(warning);
  },
  input: 'dist/index.js',
  output: [
    {
      format: 'cjs',
      sourcemap: true,
      file: 'dist/node-bundle.js'
    },
    {
      format: 'esm',
      sourcemap: true,
      file: 'dist/node-bundle.esm.mjs'
    }
  ],
  plugins
};

I tried change the nodeResolve plugin config from browser: false to true. The build got generated. But that's not going to work for the Node environment. Do you have example how to create a Node build? Thanks a lot!!!

tsubomii avatar Jan 31 '24 22:01 tsubomii

Yeah, the sandbox only builds the "browser" bundles (as it's web focused), so we effectively don't use rollup for the node build.

Looking at the semver package versions 7.5.2 and 7.5.4 the satisifies is being "exported" in the index.js, I don't think the @types/semver would help in any way as these are really just the definitions and not the "resolving" of the bundles.

I'm wondering if it's got something to do with the hack required for the import-in-the-middle where rollup is not "finding" the satisifies... So you would need to "pre-declare" that this really does exist like the global node operations, this way it would just add in the usage without validating it -- it's been a long time since I last did something like that so I don't immediately recall the configurations.

MSNev avatar Jan 31 '24 22:01 MSNev

Maybe just adding something like the following to the rollup config to "declare" that it exists??? -- guessing

external: ['satisifies'] // <-- suppresses the warning

MSNev avatar Jan 31 '24 22:01 MSNev

@MSNev I tried the external option, it didn't work. However, I did the following hack in the build output of node_modules/@opentelemetry/instrumentation/build/esm/platform/node/instrumentation.js. Then the build works.

//import { satisfies } from 'semver';
const satisfies = require('semver');

This indicates that the module import can't resolve the semver package in the parent node_modules under node_modules/@opentelemetry/instrumentation. But the node require can.

Btw, I also tried to add moduleDirectories option in for my rollup nodeResolve plugin like below. That doesn't help. Only changing the statement instrumentation.js from import to require works.

resolve({
    browser: false,
    exportConditions: ['node'],
    moduleDirectories: ['node_modules', 'node_modules/@opentelemetry/instrumentation/node_modules']
  }),

How does your node build work? I wonder how your build tooling resolve the dependencies from the parent node_modules. Is there any config you can add in the tsconfig.esm.json or the package.json to get node module resolution as require for the downstream consumer?

tsubomii avatar Feb 01 '24 01:02 tsubomii

How does your node build work?

With rollup it doesn't as we only build the browser packages with it, the main repo uses webpack (for testing) and doesn't actually create bundles.

There are however esm definition issues

@dyladan This looks like it might be related to the esm export issues??

MSNev avatar Feb 01 '24 02:02 MSNev

I don't think it's related to the esm publishing issues because that only affects node to my knowledge. I think rollup and webpack both support the way we publish our esm modules. In any case, it's being updated anyway.

dyladan avatar Feb 07 '24 17:02 dyladan

@dyladan I encounter this error when I try to export a Node format bundle. My application has server side rendering, the build system requires downstream library to publish a bundled Node esm package. I am working on a library. Therefore, this is a corner case. The rollup esm bundle for Browser works, but not Node.

Did you publish a new version to fix the Node publishing problem? Which library should I update?

tsubomii avatar Feb 08 '24 22:02 tsubomii

No new library has been published for the esm fix yet. I'll try to work on that this week

dyladan avatar Feb 14 '24 16:02 dyladan

This looks like a duplicate of #3989. @tsubomii could you have a look at the issue and let us know if this is the same problem? :thinking:

pichlermarc avatar Jun 19 '24 16:06 pichlermarc

Seems right to consider this a duplicate of #3989 , will close this as it's being tracked elsewhere.

JamieDanielson avatar Jul 24 '24 16:07 JamieDanielson