chore: depend on API ^1.0.0 also in devDependencies

[Flarna]

Which problem is this PR solving?

Not sure if this actually solves a problem or creates new ones. But it seems pinning API results in duplicates installed by at least some NPM versions.

Short description of the changes

Avoid API duplication during npm install by allowing a range similar as in other modules.

Checklist

• Ran npm run test-all-versions for the edited package(s) on the latest commit if applicable.

[codecov]

Codecov Report

Merging #1012 (4a245d6) into main (d39c642) will decrease coverage by 3.66%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1012      +/-   ##
==========================================
- Coverage   95.91%   92.24%   -3.67%     
==========================================
  Files          13      129     +116     
  Lines         856     5933    +5077     
  Branches      178     1148     +970     
==========================================
+ Hits          821     5473    +4652     
- Misses         35      460     +425     

Impacted Files	Coverage Δ
...ry-instrumentation-dns/src/enums/AttributeNames.ts	100.00% <0.00%> (ø)
...ntelemetry-instrumentation-router/src/constants.ts	100.00% <0.00%> (ø)
...ode/opentelemetry-instrumentation-dns/src/utils.ts	95.91% <0.00%> (ø)
...emetry-id-generator-aws-xray/src/platform/index.ts	100.00% <0.00%> (ø)
...aba-cloud/src/detectors/AlibabaCloudEcsDetector.ts	97.67% <0.00%> (ø)
...ode/opentelemetry-instrumentation-net/src/utils.ts	90.47% <0.00%> (ø)
...try-instrumentation-graphql/src/instrumentation.ts	91.66% <0.00%> (ø)
...ce-detector-aws/src/detectors/AwsLambdaDetector.ts	100.00% <0.00%> (ø)
...metry-instrumentation-mysql/src/instrumentation.ts	93.27% <0.00%> (ø)
...entelemetry-instrumentation-aws-sdk/src/aws-sdk.ts	97.44% <0.00%> (ø)
... and 106 more

[dyladan]

I see no reason not to make this consistent

[rauno56]

Could you please comment on the changes other than making api ^1.0.0 in dev deps. Not that they must not be here, but I'd just like to understand the premise.

[Flarna]

pin dependencies within the same learna monorep, forgot this one in a previous PR #984

[rauno56]

Please help me understand why are we supposed to pin "monorepo-local" packages / why should we make those follow an different rule than external pacakges.

There are more:

• @opentelemetry/propagator-aws-xray@^1.0.1 in @opentelemetry/instrumentation-aws-lambda
• @opentelemetry/instrumentation-express@^0.28.0 in express-example
• @opentelemetry/instrumentation-document-load@^0.27.1 in @opentelemetry/browser-extension-autoinjection
• all instrumentations in @opentelemetry/auto-instrumentations-*

Are these also supposed to be pinned? Why not?

[Flarna]

We do it in core repo and I see no reason to do it different here. They are released in one shot and user should update all of them or nothing. So it's always consistent as during tests here.
At least this is my understanding based on the setup in core repo.

Whenever I had a mixup of versions it ended up in a nightmare.

And yes, you are right, there are a few dependencies I haven't seen yet. In general I have the feeling that dependency management in this (and core) repo should be more automated but till now I found no time to get familiar with renovate bot.

[Flarna]

I'm still not sure what is the best way to setup the dependencies. It seems that every release made has it's own followup problems regarding dependencies. Not sure if we are able to get to a setup which just works.

[rauno56]

We do it in core repo and I see no reason to do it different here. They are released in one shot and user should update all of them or nothing. So it's always consistent as during tests here.

I'm all for consistency, just trying to understand the reasoning behind it. So the main reason is to mimic the repository setup since they are linked in CI. That makes sense.

And yes, you are right, there are a few dependencies I haven't seen yet. In general I have the feeling that dependency management in this (and core) repo should be more automated but till now I found no time to get familiar with renovate bot.

I'm also totally for automation, but currently I personally don't even always understand the rules to automate them. Hence me bombarding you with questions. :)

[rauno56]

What about the examples I listed above tho of other places that do not follow the same rule?

[Flarna]

I haven't touched the examples (except that on part of the lerna project) as they have no tests and I don't want to run all of them manually to verify that they still work.

[rauno56]

By "examples I listed above" I meant:

There are more:

• @opentelemetry/propagator-aws-xray@^1.0.1 in @opentelemetry/instrumentation-aws-lambda
• @opentelemetry/instrumentation-express@^0.28.0 in express-example
• @opentelemetry/instrumentation-document-load@^0.27.1 in @opentelemetry/browser-extension-autoinjection
• all instrumentations in @opentelemetry/auto-instrumentations-*

Which by the rule of "monorepo local deps should be pinned" are currently inconsistent.

What comes to @opentelemetry/instrumentation package, I'd leave that to ^0.27 for now and specify http and grpc instrumentations accordingly to a version that uses ^0.27 - we can then update that separately. What do you think?

[blumamir]

I was involved in pinning the dependencies in core repo and support doing this here as well.

At aspecto, we craft our own distribution of OpenTelemetry and publish it to our users. To guarantee stability, we pin every otel package dependency to exact version. When a new otel version is released, we first test it with our distro and backend and when we verify there are no issues, we release a new version of our distro to clients.

Now, imagine that someone accidentally introduces a breaking change or bug into a patch version. If the dependencies are defined with a caret, then the new patch version will be picked up automatically, break the installation, and there is nothing the user can do to resolve this issue.

This is the original issue that was followed with pinning transitive same-mono-repo dependencies

For example:

1. User pinned "@opentelemetry/[email protected]", and it depends on "@opentelemetry/propagation-utils@^0.27.0". Now if "0.27.1" is published and it's broken - user's app is broken until a patch fix is published :(

2. User pinned "@opentelemetry/[email protected]", and it depends on "@opentelemetry/[email protected]". Now if "0.27.1" is published, it will go along with "@opentelemetry/[email protected]" and user has the freedom to test 0.6.1 and upgrade when he wants :)

3. User does not pin - it uses "@opentelemetry/instrumentation-aws-sdk@^0.6.0". Now if "@opentelemetry/[email protected]" is published lerna will also publish "@opentelemetry/[email protected]" which means the user that asked to depend on caret will automatically get the new transitive version as he asked :)

If I am not missing anything, I believe this strategy covers both needs and makes sense

[Flarna]

What comes to @opentelemetry/instrumentation package, I'd leave that to ^0.27 for now and specify http and grpc instrumentations accordingly to a version that uses ^0.27 - we can then update that separately. What do you think?

If we depend on old http instrumentation this will pull in old SDK components and likely to new problems.

[Flarna]

correct SDK/core deps here - forgot this one in #984
As this example is part of the lerna project it needs to be kept in sync as it participates in hoisting. Other examples can be updated independent.

[rauno56]

#984 has some of the same dependencies pinned for the browser extension package but not others, afaict. Why is that?
The extension pacakge also participates in hoisting.

[Flarna]

I pinned dev-dependencies from other repos as this seems to be the standard setup here and in core.

Not pinning dependencies to local repos was not my focus that time but I think it should be done. But I guess for dev-deps it's anyway don't care as they have no influence to end user and lerna links pinned and non pinned dependencies.

[rauno56]

So should we achieve consistency between this here and browser extension package?

[Flarna]

Yes, I think we should be consistent.

[Flarna]

created a followup to harmonize more then API: #1020