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

feat: add supported node versions for all packages #973

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

rauno56
Copy link
Member

@rauno56 rauno56 commented Apr 13, 2022

Which problem is this PR solving?

In accordance with open-telemetry/opentelemetry-js#2834 we've changed the support range to >=8.12.
open-telemetry/opentelemetry-js@da22673 changed that in Core.

Short description of the changes

Updated every package.json according to the issue above and out CI configuration, which skips tests for some of the packaged that have dropped the support for older node versions.

@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #973 (0e2e0f5) into main (e99d682) will decrease coverage by 3.75%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #973      +/-   ##
==========================================
- Coverage   95.91%   92.15%   -3.76%     
==========================================
  Files          13      129     +116     
  Lines         856     5925    +5069     
  Branches      178     1144     +966     
==========================================
+ Hits          821     5460    +4639     
- Misses         35      465     +430     
Impacted Files Coverage Δ
plugins/node/instrumentation-amqplib/src/utils.ts 95.38% <0.00%> (ø)
plugins/node/instrumentation-tedious/src/utils.ts 88.23% <0.00%> (ø)
...metry-resource-detector-aws/src/detectors/index.ts 100.00% <0.00%> (ø)
...lugins/node/instrumentation-amqplib/src/amqplib.ts 90.40% <0.00%> (ø)
...trumentation-express/src/enums/ExpressLayerType.ts 100.00% <0.00%> (ø)
...etry-instrumentation-aws-sdk/src/services/index.ts 100.00% <0.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts 97.91% <0.00%> (ø)
...opentelemetry-instrumentation-ioredis/src/utils.ts 100.00% <0.00%> (ø)
...etry-instrumentation-router/src/enums/LayerType.ts 100.00% <0.00%> (ø)
...urce-detector-alibaba-cloud/src/detectors/index.ts 100.00% <0.00%> (ø)
... and 106 more

@Flarna
Copy link
Member

Flarna commented Apr 13, 2022

strictly speaking only modules using the performance times require 8.12 others work fine with lower versions. But not sure if there is any benefit of having e.g. a resource detector working with 8.0 but there is no SDK to use it with.

Copy link
Member

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

LGTM.
This will break users if they are running on node >=8.5.0 && <=8.12.0
Not sure if there are any of these out there 🤷‍♂️

Could be nice to at least bump the minor version for publishing this change (e.g. label it as "feat"?)

@rauno56 rauno56 changed the title chore: add supported node versions for all packages feat: add supported node versions for all packages Apr 14, 2022
@rauno56
Copy link
Member Author

rauno56 commented Apr 14, 2022

strictly speaking only modules using the performance times require 8.12 others work fine with lower versions. But not sure if there is any benefit of having e.g. a resource detector working with 8.0 but there is no SDK to use it with.

That's right. In that sense we don't absolutely need to narrow down propagators and detectors. I think I'd approached it differently if we had SDK compatible with 8.5 and took it from module<>plugin direction making the version minimum of what either can support. Would you prefer it solved differently?

@Flarna
Copy link
Member

Flarna commented Apr 14, 2022

I think it's fine. I doubt that it has much impact at all as people using that old node versions are most likely used to ignore warnings anyway :o).

@rauno56 rauno56 merged commit baaacbd into open-telemetry:main Apr 14, 2022
@rauno56 rauno56 deleted the chore/pkg-engine-node branch April 14, 2022 09:04
@dyladan dyladan mentioned this pull request Apr 14, 2022
@dyladan dyladan mentioned this pull request May 25, 2022
@dyladan dyladan mentioned this pull request Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants