Opentelemetry-js: Auto-instrumentation doesn't work when using Yarn 2 with PnP

[schickling]

What version of OpenTelemetry are you using?

    "@opentelemetry/api": "^0.10.2",
    "@opentelemetry/exporter-collector": "^0.10.2",
    "@opentelemetry/exporter-jaeger": "^0.10.2",
    "@opentelemetry/node": "^0.10.2",
    "@opentelemetry/plugin-grpc": "^0.10.2",
    "@opentelemetry/plugin-http": "^0.10.2",
    "@opentelemetry/plugin-https": "^0.10.2",
    "@opentelemetry/tracing": "^0.10.2",

What version of Node are you using?

v14.7.0

What did you do?

When using Yarn 2 with PnP enabled (which is on by default), the plugin-based auto-instrumentation doesn't seem to work.

What did you expect to see?

It should work as when not using PnP (i.e. node_modules being present).

What did you see instead?

PluginLoader#load: trying to load [email protected]
PluginLoader#load: could not load plugin @opentelemetry/plugin-http of module http. Error: @opentelemetry/node tried to access @opentelemetry/plugin-http, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @opentelemetry/plugin-http (via "@opentelemetry/plugin-http")
Required by: @opentelemetry/node@npm:0.10.2 (via /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/instrumentation/)

Require stack:
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/instrumentation/PluginLoader.js
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/NodeTracerProvider.js
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/index.js
- /home/schickling/code/my-project/packages/my-project-service/src/index.ts
PluginLoader#load: trying to load [email protected]
PluginLoader#load: could not load plugin @opentelemetry/plugin-https of module https. Error: @opentelemetry/node tried to access @opentelemetry/plugin-https, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @opentelemetry/plugin-https (via "@opentelemetry/plugin-https")
Required by: @opentelemetry/node@npm:0.10.2 (via /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/instrumentation/)

Require stack:
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/instrumentation/PluginLoader.js
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/NodeTracerProvider.js
- /home/schickling/code/my-project/.yarn/cache/@opentelemetry-node-npm-0.10.2-4554cb7ee1-d2e66e9307.zip/node_modules/@opentelemetry/node/build/src/index.js
- /home/schickling/code/my-project/packages/my-project-service/src/index.ts

[dyladan]

Removing the bug label as behavior with a non-standard dependency linker is not yet defined.

The issue here seems to be that @opentelemetry/node requires the plugins, but does not explicitly list them as dependencies. This is intentional because we can't know which plugins will be used in advance. They may even be plugins we didn't write and aren't aware of.

One potential solution to this might be to create an option to disable automatic detection of plugins, and force the user to construct plugin instances and enable them manually.

import { MyPlugin } from "some-plugin-module";
import { MyPlugin2 } from "some-other-plugin-module";

const provider = new NodeTracerProvider({ loadPlugins: false });

const myPlugin = new MyPlugin();
const myPlugin2 = new MyPlugin2();

enabling this type of pattern would require breaking changes to the plugin interface

[schickling]

Thanks for this additional context. Makes sense.

I've run into similar problems when using bundlers (e.g. esbuild or webpack). I think this is a common enough scenario/pattern in modern applications that it's worth considering the change you've outlined.

[dyladan]

If we're going to make this change, it needs to be done before GA in my opinion as changing the plugin interface to allow manual plugin installation would be quite the breaking change.

[arcanis]

Removing the bug label as behavior with a non-standard dependency linker is not yet defined

Just fyi, this isn't a PnP issue. It's a Node issue that PnP highlights more than node_modules can do. The problem is that because you're calling require on something that isn't listed as a dependency, you're subject to the hoisting whims. For example, take the following case. There's nothing special, just two incompatible telemetry versions used in two different deps:

root → dep-a → opentelemetry@1
             → opentelemetry-http@1
     → dep-b → opentelemetry@2
             → opentelemetry-http@2

Package managers will tend to hoist them to the top. Now, there's no rule around hoisting, except that the dependency contract must be respected (ie a package doing require on one of its listed dependency must obtain a version compatible with the range it requested). Because you didn't list explicit relationships, the following is possible:

root → opentelemetry@2
     → opentelemetry-http@1
     → dep-a → opentelemetry@1
     → dep-b
             → opentelemetry-http@2

When opentelemetry@2 will make the require call to retrieve its http plugin, due to the hoisting, it will get the v1 from this plugin, not the v2. The hoisting is invalid, but because of the lack of explicit relationship, package managers don't have the information needed to prevent it.

---

In non-plugin scenarios, you'd list those dependencies as (optional?) peer dependencies. This would be enough to fix the problem, as the additional constraint would prevent package managers from generating hoisting like the above. In your case however, you likely don't know what plugins will be used - after all, maybe your users will develop their own!

In this case, I'd suggest to do the same thing as Webpack, Babel, ESLint, ...: load your plugins on behalf of someone else. For example, if your require call is like this:

const http = require(pluginName)

Instead require it using this:

const {createRequire} = require('module');

const req = createRequire(pathToConfigFile);
const http = req(pluginName);

Or, another option:

const httpPath = require.resolve(pluginName, {paths: [pathToConfigFile]});
const http = require(pluginName);

In both cases, the resolution is done on behalf of the package that actually lists the plugin as dependencies. How you know its path is up to you, but it can come from anything: the configuration file path (Babel, ESLint), a basedir parameter in the initializer (Webpack), ... it's the most flexible option.

[dyladan]

I am working on a plugins v2 right now that will solve this issue by requiring any non-default plugins to be manually constructed by the end-user application. Does this sound like a reasonable solution to you? Thanks for your patience.

[arcanis]

Yep, dependency injection is a good solution too 👍

[dyladan]

I prefer to build the lower layers as explicitly as possible, then build the "magic" (like auto-detecting/auto-installing plugins) into wrappers around it. That way you can always drop down to the explicit version in the case that you have some unexpected failure.

[ekremney]

Hi @dyladan, any update on this one?

We're having the exact same problem (with webpack), auto-instrumentation doesn't work.

[dobesv]

By the way you can workaround this by adding overrides to .yarnrc.yml

packageExtensions:
  "@opentelemetry/node@^0.14.0":
    dependencies:
      "@opentelemetry/plugin-http": "^0.14.0"
      "@opentelemetry/plugin-https": "^0.14.0"
      "@opentelemetry/plugin-pg-pool": "^0.12.0"
      "@opentelemetry/plugin-pg": "^0.12.0"
      "@opentelemetry/plugin-ioredis": "^0.12.0"
      "@opentelemetry/plugin-mongodb": "^0.12.0"
  "@opentelemetry/plugin-ioredis@^0.12.1":
    dependencies:
      "@opentelemetry/semantic-conventions": "^0.14.0"
  "@opentelemetry/plugin-mongodb@^0.12.1":
    dependencies:
      "@opentelemetry/semantic-conventions": "^0.14.0"
pnpMode: strict

[dobesv]

Can't you declare these as peer dependencies? Then they won't be installed by default but it can still load them using pnp. Seems like a simple fix.

[oliversalzburg]

It seems like dependencies should be declared in the packages that have the dependencies. If consumers of a package are left to figure out which packages (and versions) to puzzle together, you're kind of missing the target.

[dyladan]

It seems like dependencies should be declared in the packages that have the dependencies. If consumers of a package are left to figure out which packages (and versions) to puzzle together, you're kind of missing the target.

It's not that simple. If we declare all plugins as dependencies the deployment size will be unnecessarily bloated. The whole point is that we only require them if the user has chosen to include them.

[dobesv]

If we declare all plugins as dependencies the deployment size will be unnecessarily bloated.

When you use peer dependencies, they are not installed by default, but they are still declared for Yarn PnP purposes.