fix: align dependency versions #3847

llc1123 · 2023-06-01T15:15:07Z

Short description of the changes

Fixed several version mismatches of dependencies.

lerna WARN EHOIST_ROOT_VERSION The repository root depends on semver@7.3.5, which differs from the more common semver@^7.3.5.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/instrumentation-http" package depends on semver@^7.3.5, which differs from the hoisted semver@7.3.5.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/shim-opencensus" package depends on semver@^7.3.5, which differs from the hoisted semver@7.3.5.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/sdk-trace-node" package depends on semver@^7.3.5, which differs from the hoisted semver@7.3.5.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/instrumentation" package depends on semver@^7.3.2, which differs from the hoisted semver@7.3.5.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on typescript@^4.5.2, which differs from the hoisted typescript@4.4.4.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/opentelemetry-browser-detector" package depends on @types/mocha@9.1.1, which differs from the hoisted @types/mocha@10.0.0.
lerna WARN EHOIST_PKG_VERSION "backcompat-node14" package depends on @types/node@14.18.25, which differs from the hoisted @types/node@18.6.5.
lerna WARN EHOIST_PKG_VERSION "backcompat-node16" package depends on @types/node@16.11.52, which differs from the hoisted @types/node@18.6.5.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/exporter-logs-otlp-http" package depends on sinon@14.0.0, which differs from the hoisted sinon@15.0.0.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/sdk-logs" package depends on sinon@14.0.0, which differs from the hoisted sinon@15.0.0.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on ts-loader@^9.2.6, which differs from the hoisted ts-loader@8.4.0.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on webpack@^5.65.0, which differs from the hoisted webpack@4.46.0.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on @babel/core@^7.6.0, which differs from the hoisted @babel/core@7.16.0.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on babel-loader@^8.0.6, which differs from the hoisted babel-loader@8.2.3.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on webpack-cli@^4.10.0, which differs from the hoisted webpack-cli@4.9.1.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on webpack-dev-server@^4.5.0, which differs from the hoisted webpack-dev-server@4.5.0.
lerna WARN EHOIST_PKG_VERSION "web-opentelemetry-example" package depends on webpack-merge@^5.8.0, which differs from the hoisted webpack-merge@5.8.0.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/instrumentation" package depends on require-in-the-middle@^7.1.0, which differs from the hoisted require-in-the-middle@^7.0.0.
lerna WARN EHOIST_PKG_VERSION "@opentelemetry/otlp-grpc-exporter-base" package depends on protobufjs@^7.2.2, which differs from the hoisted protobufjs@^7.1.2.

I will try to refactor web-opentelemetry-example and get rid of webpack@4 (which is incompatible with node 17+) in another PR.

Type of change

Bug fix (non-breaking change which fixes an issue)

codecov · 2023-06-01T15:31:27Z

Codecov Report

Merging #3847 (e7aa2d0) into main (e8540c5) will decrease coverage by 1.16%.
The diff coverage is n/a.

❗ Current head e7aa2d0 differs from pull request most recent head 70a553d. Consider uploading reports for the commit 70a553d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3847      +/-   ##
==========================================
- Coverage   92.49%   91.34%   -1.16%     
==========================================
  Files         222      120     -102     
  Lines        5610     2587    -3023     
  Branches     1077      543     -534     
==========================================
- Hits         5189     2363    -2826     
+ Misses        421      224     -197

see 111 files with indirect coverage changes

dyladan

Did you intend to change the ^ dependencies to pinned versions?

experimental/packages/exporter-logs-otlp-http/package.json

llc1123 · 2023-06-02T02:27:03Z

Yes, just like most packages.

pichlermarc

Looks good, thanks. I'm not entirely sure why some versions were not pinned in the first place 🤔

Flarna · 2023-06-07T06:38:17Z

At least in the past the strategy was to pin only dependencies from the same lerna project as these dependencies are automatically updated on each release.

On the other hand dev-dependencies were pinned to reduce the risk of autobreaking the build and we were not able to use lock files.

After this PR we seem to have some pinned deps (e.g. protobufjs, semver, require-in-the-middle) and some not pinned (e.g. @grpc/grpc-js).

Pinning likely results in duplication of packages for end users because renovate bot tends to fail in this repo and we are slow in updating. in special for bigger packages like @grpc/grpc-js or protobufjs this is usually not nice.

I think we need a reasoning/guideline regarding this.

llc1123 · 2023-06-07T06:54:09Z

After this PR we seem to have some pinned deps (e.g. protobufjs, semver, require-in-the-middle) and some not pinned (e.g. @grpc/grpc-js).

You are right. I am not sure why some dependencies are pinned and others are not. But at least we should keep the versions of the same package the same. (we have semver@7.3.5 somewhere and semver@^7.3.5 elsewhere).

pichlermarc · 2023-06-07T12:27:05Z

At least in the past the strategy was to pin only dependencies from the same lerna project as these dependencies are automatically updated on each release.

On the other hand dev-dependencies were pinned to reduce the risk of autobreaking the build and we were not able to use lock files.

Ah, that makes sense.

After this PR we seem to have some pinned deps (e.g. protobufjs, semver, require-in-the-middle) and some not pinned (e.g. @grpc/grpc-js).

Pinning likely results in duplication of packages for end users because renovate bot tends to fail in this repo and we are slow in updating. in special for bigger packages like @grpc/grpc-js or protobufjs this is usually not nice.

I think we need a reasoning/guideline regarding this.

I agree, we should have a guideline for this, I'll look into drafting one up. I think it makes sense to unpin larger packages for the reason you stated.

After this PR we seem to have some pinned deps (e.g. protobufjs, semver, require-in-the-middle) and some not pinned (e.g. @grpc/grpc-js).

You are right. I am not sure why some dependencies are pinned and others are not. But at least we should keep the versions of the same package the same. (we have semver@7.3.5 somewhere and semver@^7.3.5 elsewhere).

Agreed. @llc1123 would you mind unpinning protobufjs, require-in-the-middle, @types/shimmer and shimmer again while keeping it at caret versions? Looking into it semver is pinned already in most packages in this repo so I think we can pin this one. This way we'd still align but keep most of what's unpinned still unpinned for now.

pichlermarc · 2023-06-07T13:07:32Z

It looks I made a mistake while searching for dependencies onsemver. Looks like it's actually unpinned for dependencies pretty much everywhere but only pinned in devDependencies, which is seems in-line with what Flarna mentioned.

Intuitively I'd say to

keep semver dependencies unpinned
pin semver devDependencies to latest and then see if the situation improves with the new renovate bot config (chore(renovate): improve PR grouping and add @pichlermarc to assignees #3868) that should allow us to merge dependency PRs quicker, which would get rid of the warning should it come back.

llc1123 · 2023-06-07T13:09:01Z

I've updated the code. I unpinned all dependencies except ones in the lerna project.

experimental/packages/opentelemetry-instrumentation/package.json

llc1123 · 2023-06-09T17:47:11Z

@Flarna What do you think of this version?

Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>

llc1123 requested a review from a team June 1, 2023 15:15

dyladan reviewed Jun 1, 2023

View reviewed changes

experimental/packages/exporter-logs-otlp-http/package.json Outdated Show resolved Hide resolved

fix: align dependency versions

a2272e4

llc1123 force-pushed the fix/version-mismatch branch from 31a6b5a to a2272e4 Compare June 2, 2023 04:41

llc1123 added 2 commits June 5, 2023 02:19

Merge branch 'main' into fix/version-mismatch

6e99a1b

Merge branch 'main' into fix/version-mismatch

4577250

pichlermarc approved these changes Jun 7, 2023

View reviewed changes

pichlermarc and others added 2 commits June 7, 2023 14:27

Merge branch 'main' into fix/version-mismatch

6eb2443

fix: unpin dependencies only

2d680b5

pichlermarc reviewed Jun 7, 2023

View reviewed changes

experimental/packages/opentelemetry-instrumentation/package.json Outdated Show resolved Hide resolved

pichlermarc self-requested a review June 7, 2023 15:13

llc1123 added 3 commits June 7, 2023 23:58

Revert unpinned import-in-the-middle

8824f19

Merge branch 'main' into fix/version-mismatch

5c6747f

Merge branch 'main' into fix/version-mismatch

be48adf

llc1123 force-pushed the fix/version-mismatch branch from fb3b520 to be48adf Compare June 8, 2023 15:11

Merge branch 'main' into fix/version-mismatch

f49d436

Merge branch 'main' into fix/version-mismatch

44e2850

Flarna approved these changes Jun 12, 2023

View reviewed changes

pichlermarc added the Skip Changelog label Jun 12, 2023

Merge branch 'main' into fix/version-mismatch

70a553d

pichlermarc approved these changes Jun 12, 2023

View reviewed changes

pichlermarc merged commit 3b387d8 into open-telemetry:main Jun 12, 2023

llc1123 deleted the fix/version-mismatch branch June 12, 2023 12:21

Nico385412 pushed a commit to Nico385412/opentelemetry-js that referenced this pull request Jun 13, 2023

fix: align dependency versions (open-telemetry#3847)

bb42171

Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>

haddasbronfman mentioned this pull request Aug 5, 2023

docs(guidelines): add dependencies guidelines #4040

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: align dependency versions #3847

fix: align dependency versions #3847

llc1123 commented Jun 1, 2023 •

edited

Loading

codecov bot commented Jun 1, 2023 •

edited

Loading

dyladan left a comment •

edited

Loading

llc1123 commented Jun 2, 2023

pichlermarc left a comment

Flarna commented Jun 7, 2023

llc1123 commented Jun 7, 2023 •

edited

Loading

pichlermarc commented Jun 7, 2023

pichlermarc commented Jun 7, 2023

llc1123 commented Jun 7, 2023

llc1123 commented Jun 9, 2023

fix: align dependency versions #3847

fix: align dependency versions #3847

Conversation

llc1123 commented Jun 1, 2023 • edited Loading

Short description of the changes

Type of change

codecov bot commented Jun 1, 2023 • edited Loading

Codecov Report

dyladan left a comment • edited Loading

Choose a reason for hiding this comment

llc1123 commented Jun 2, 2023

pichlermarc left a comment

Choose a reason for hiding this comment

Flarna commented Jun 7, 2023

llc1123 commented Jun 7, 2023 • edited Loading

pichlermarc commented Jun 7, 2023

pichlermarc commented Jun 7, 2023

llc1123 commented Jun 7, 2023

llc1123 commented Jun 9, 2023

llc1123 commented Jun 1, 2023 •

edited

Loading

codecov bot commented Jun 1, 2023 •

edited

Loading

dyladan left a comment •

edited

Loading

llc1123 commented Jun 7, 2023 •

edited

Loading