Unified pattern for instrumentation dependencies

[blumamir]

This issue is about dependencies in instrumentation package.jsons, discussed here

The current code base includes many instrumentations that have different dependencies patterns on @opentelemetry/* packages.

I would like to get to a consensus about the correct way to do it, and then apply the decision on all pacakges

Peer dependency on @opentelemetry/api

Should be set to minimum version that satisfies the instrumentation, with a caret. This will usually be:

  "peerDependencies": {
    "@opentelemetry/api": "^1.0.0"
  },

This change is covered in #984 by @Flarna

Dev depenency on @opentelemetry/api

Currently, each instrumentation uses a different version (probably the latest one available when it was created).
This dependency is used for build and test.
I guess we want to keep it pinned to enforce consistency, but the question is if it should be pinned to least supported version (1.0.0), or latest version (1.1.0 and bump on new releases).
A suggestion is to use test-all-versions to verify tests are passing with various api versions, but we will still need a single version in package.json dependencies for all other tasks.

Dependencies on @opentelemetry/* packages

Should instrumentation state the minimum supported version (e.g. ^1.0.0), or the latest version (^1.2.0). I guess both are OK, even if user is using old SDK version, instrumentation will just pull it's own copy version into another branch in node_modules and will function correctly.
Currently, there is inconsistency in these dependencies which I support in straightening and documenting the "right" template.

@Flarna - you always have good insights on these topics. What do you recommend doing?

[Flarna]

If we state compatibility with API 1.0.0 we should test it somewhere. Not sure if using test-all-versions is really needed, maybe some smaller setup which just installs deps and transpiles is already enough.
Same applies more or less on SDK.

I guess both are OK, even if user is using old SDK version, instrumentation will just pull it's own copy version into another branch in node_modules and will function correctly.

It will most likely work correct but typescript will likely complain - see #983 Long term this could be fixed by replacing all classes in exported APIs by interfaces but I doubt this will happen soon. Not sure if this is even possible because I think we even export abstract classes at a few places.

Also duplication of API can be problematic. If an instrumentation has a copy of API 1.1.0 in it's subtree and app uses 1.0.x the instrumentation won't create traces because the global installed TracerProvider of 1.0.0 gets a request from 1.1.0 API it can't fulfill. The other way around (instrumentation has 1.0.0 pinned, app uses 1.1.0) should be ok.

[blumamir]

If we state compatibility with API 1.0.0 we should test it somewhere. Not sure if using test-all-versions is really needed, maybe some smaller setup which just installs deps and transpiles is already enough.
Same applies more or less on SDK.

We can configure tav to run only the compile script and not tests. I think we can also define a matrix to test multiple combinations of api and SDKs.

It will most likely work correct but typescript will likely complain - see #983 Long term this could be fixed by replacing all classes in exported APIs by interfaces but I doubt this will happen soon. Not sure if this is even possible because I think we even export abstract classes at a few places.

I think this also depends on the exact combination of usages. Instrumentation mostly depends on the API and @opentelemetry/instrumentation libraries, so as long as they don't have conflicts, the situation should be relatively ok I assume. But did not check it in depth.

Also duplication of API can be problematic. If an instrumentation has a copy of API 1.1.0 in it's subtree and app uses 1.0.x the instrumentation won't create traces because the global installed TracerProvider of 1.0.0 gets a request from 1.1.0 API it can't fulfill. The other way around (instrumentation has 1.0.0 pinned, app uses 1.1.0) should be ok.

We talked about it a few weeks ago at the SIG, and understood this is unaviodable.
For instrumentations, this is simpler as one can just use a compatible version of the instrumentation package until the SDK is upgraded to support a newer API. For packages that embed OTEL, this can be an issue as users will want to upgrade the package version for it's new functionality or bug fixes and it could bump the supported version of @opentelemetry/api which will break their app. For these cases, we talked about recommending always using the minimum supported version in dependency, e.g. @opentelemetry/api@^1.0.0, unless some new feature is required. That will increase flexability of npm in choosing the right version and less chance for issues.

[Flarna]

I think this also depends on the exact combination of usages. Instrumentation mostly depends on the API and @opentelemetry/instrumentation libraries, so as long as they don't have conflicts, the situation should be relatively ok I assume. But did not check it in depth.

Even if the same version is in both folders it wont work. And I think this has a good reason. Even if the class has the same source and therefore the same functionality an instanceof check would fail as the class with the same name exists twice and this is what instanceof compares.

[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.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.