-
Notifications
You must be signed in to change notification settings - Fork 824
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
Minimum NodeJS version is 8.5.0 instead 8.0.0 #711
Comments
Do we bump to support only |
is there some statistics about what is running currently ? You can still push serverless functions to Google cloud with node 8 and node 10 is in beta ^^. |
I think we can support 8 since the tests run on it and it is not so much of a burden. I think we should update the node 8 tests to run using the minimum version, instead of the latest version as they do right now. |
Some numbers from our side: |
I'm for keeping node 8 and set 8.5.0. |
@Flarna can you provide more detailed statistics on how many of the node8 users are using versions older than 8.5? ~40% is a large number of users |
Around 5% of the NodeJs 8 processes seen use <8.5 |
@mayurkale22 @open-telemetry/javascript-approvers given that over 35% of @Flarna's users are using |
also packages (package.json file) should have |
It may be EOL but it is still the used by a very large number of users, including being the default node runtime on some serverless platforms. |
@mayurkale22 @dyladan I would like to pick this up if its alright with yall |
@Flarna have you seen any drop in node 8 customers since EOL? |
Current state: I would not care much about node <8.5. First of all the number is low and besides that I'm quite sure that the at last most processes using old versions have not been changed/restarted for a long time. |
30% is still a large portion of users. I think 8.5.0 is still a decent minimum. The tests are still running against 8 latest. I'll update circle to run against 8.5 |
Yes, node 8 seems to be longer alive then expected... But as mentioned the concrete minor version is maybe not that important. |
Hmm so maybe we're ok with only testing on 8 latest since it is unsupported anyways. No need to overcomplicate |
Co-authored-by: Daniel Dyla <[email protected]>
What version of OpenTelemetry are you using?
0.3.2
What version of Node are you using?
<8.5.0
What did you do?
create a
Span
(e.g. via example basic-tracer-node)What did you expect to see?
it should work
What did you see instead?
crash because of
Error: Cannot find module 'perf_hooks'
Additional context
perf_hooks module was added in 8.5.0 - see https://nodejs.org/docs/latest-v13.x/api/perf_hooks.html
Maybe minimum NodeJS version should be bumped to 8.5.0 - or even 10.0.0.
I don't expect it's really worth to write a polyfill as Node.JS 8 has reached EOL.
The text was updated successfully, but these errors were encountered: