dependency version mismatch in instrumentation-express

[paulius-valiunas]

What version of OpenTelemetry are you using?

@opentelemetry/api: 1.1.0
@opentelemetry/sdk-node: 0.29.0
@opentelemetry/instrumentation-express: 0.29.0

What version of Node are you using?

16.15.1

What did you do?

import the three packages above and then add ExpressInstrumentation to the instrumentations list when initializing NodeSDK.

What did you expect to see?

No errors

What did you see instead?

Typescript failed to compile:

src/main.ts:37:5 - error TS2322: Type 'ExpressInstrumentation' is not assignable to type 'InstrumentationOption'.
  Type 'ExpressInstrumentation' is not assignable to type 'Instrumentation'.
    Types of property 'setMeterProvider' are incompatible.
      Type '(meterProvider: import("D:/code/node_modules/@opentelemetry/api-metrics/build/src/types/MeterProvider").MeterProvider) => void' is not assignable to type '(meterProvider: import("D:/code/node_modules/@opentelemetry/sdk-metrics-base/node_modules/@opentelemetry/api-metrics/build/src/types/MeterProvider").MeterProvider) => void'.
        Types of parameters 'meterProvider' and 'meterProvider' are incompatible.
          Type 'import("D:/code/node_modules/@opentelemetry/sdk-metrics-base/node_modules/@opentelemetry/api-metrics/build/src/types/MeterProvider").MeterProvider' is not assignable to type 'import("D:/code/node_modules/@opentelemetry/api-metrics/build/src/types/MeterProvider").MeterProvider'.
            The types of 'getMeter(...).createObservableGauge' are incompatible between these types.
              Type '(name: string, options?: MetricOptions | undefined) => Observable' is not assignable to type '(name: string, callback: ObservableCallback, options?: MetricOptions | undefined) => void'.
                Types of parameters 'options' and 'callback' are incompatible.
                  Type 'ObservableCallback' has no properties in common with type 'MetricOptions'.

Additional context

Upon investigating my package-lock.json I found that @opentelemetry/instrumentation-express@0.29.0 imports @opentelemetry/istrumentation@^0.28.0 which creates a version mismatch (0.29.0 vs 0.28.0). The compilation error comes from my dependency tree having two different versions of @opentelemetry/api-metrics: 0.28.0 and 0.29.0.

[Flarna]

You should use @opentelemetry/sdk-node 0.28.0 wiht @opentelemetry/instrumentation-express 0.29.0 until a new version of the contrib modules is done.

Work regarding this is ongoing (e.g. #1042).

[paulius-valiunas]

I'll risk stating the obvious here, but adding a caret won't help. ^0.28.0 does not resolve to 0.29.0, it resolves to 0.28.x.

Also, is there any documentation explaining which versions of each package are compatible? I just pulled the latest version of everything. How do I know which packages have to be 0.28.0 and which ones 0.29.0? I have other opentelemetry dependencies too, I just didn't list them all out. I just tried changing them all to ~0.28.0 and I'm still getting conflicting type definitions.

[paulius-valiunas]

OK, finally found a combination that worked... instrumentation-express is ~0.29.0, everything else is either ~1.2.0 or ~0.28.0.

[Flarna]

Unfortunately there is no such table.
The packages in this repo are released independent and therefore the version numbers are not in sync. Reason is that they are maintained by different persons and we don't want to slow down development.

As long as some packages are in the experimental phase (0.x version) it's even harder to get some sort of rule as NPM interprets a ^ on a 0.x version like a ~ on a 1.x version.

Besides that there are Otel components not hosted in the @open-telemetry github/npm namespace which use there own release timing/dependency updates.

I guess this is not the answer you would like to hear and I would be happy if there is a better solution. If you have any ideas how to improve this let us know.

[dyladan]

The spec for ^ is "Allows changes that do not modify the left-most non-zero digit in the [major, minor, patch] tuple."

I think the root of this is that some of our external interface is still concrete classes and everything should be an interface

[paulius-valiunas]

yes, the correct solution would be to reduce the coupling by introducing stable interfaces for everything. The short term solution would be releasing all these packages in lock step - even if some of them don't have any new commits, release a new version just so they're always in sync.

[Flarna]

stable interfaces and experimental version numbers are usually not the best friends :o)

[paulius-valiunas]

well, then releasing them in lock step is the only solution. Stable interfaces is more of a long-term goal for when these packages get near "stable" status.

[dyladan]

The instrumentation base class only uses API and metrics API types, no SDK types which means it should be OK once the interface is stable (which it is now)

[dyladan]

the problem is that metrics types were added to the instrumentation base class before they were stabilized. we won't make this mistake for future signals

[dyladan]

I believe this should be fixed by #1042

[dyladan]

Unlinking the PR actually because this isn't fixed until it is released. I'll link the release PR when #1042 is merged

[blumamir]

This seemed to be released on github but not in npm.

@dyladan - any idea why?

[rauno56]

well, then releasing them in lock step is the only solution

Lockstepped releases is a tricky solution because of the reasons @Flarna has also mentioned:

1. Different people are managing the libraries in different repos - we don't manage all instrumentation packages out there;
2. Even if it's released in lockstep, no one stops them being installed out-of-sync, unless you mean they also need to be versioned in lockstep in which case I'm sure there are other problems awaiting for us since we're talking about versions 50 packages in unison some of which can be experimental and new, etc...

There's no ideal solution. Best one I know is to convert the instrumentation away from using concrete classes.

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[ringerc]

This looks obsolete; AFAICS the express module is fine now.