Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add missing @opentelemetry/core dependency #2473

Merged

Conversation

pichlermarc
Copy link
Member

Which problem is this PR solving?

See #2472, we're using @opentelemetry/core but don't have it listed as a dependency in some packages.

Short description of the changes

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

@@ -73,8 +73,9 @@
},
"dependencies": {
"@opentelemetry/instrumentation": "^0.53.0",
"@opentelemetry/semantic-conventions": "^1.27.0",
"@opentelemetry/semantic-conventions": "1.27.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by-fix: we're using the @opentelemetry/semantic-conventions/incubating entrypoint which may break in minor versions - pinning this to avoid any surprises when we release the 1.28.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want the same at

We should have a lint check for this perhaps.

Copy link

codecov bot commented Oct 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.74%. Comparing base (645ac2e) to head (5b41b6e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2473   +/-   ##
=======================================
  Coverage   90.74%   90.74%           
=======================================
  Files         156      156           
  Lines        7723     7723           
  Branches     1588     1588           
=======================================
  Hits         7008     7008           
  Misses        715      715           

@pichlermarc pichlermarc merged commit 4d66431 into open-telemetry:main Oct 10, 2024
23 checks passed
@pichlermarc pichlermarc deleted the fix/missing-core-deps branch October 10, 2024 17:49
@dyladan dyladan mentioned this pull request Oct 10, 2024
trentm added a commit to trentm/opentelemetry-js-contrib that referenced this pull request Oct 10, 2024
This uses knip (https://knip.dev) to check dependencies, in hopes of avoiding
issues like open-telemetry#2473. This adds 'npm run lint:deps' and adds that to the
existing 'npm run lint'.

This change includes fixes for a handful of unused and unlisted deps.
For now knip is configured to only check 'production' deps. Checking non-prod
deps results in way too many false positives.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants