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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Node.js 20 #162696

Merged
merged 29 commits into from
Nov 20, 2023
Merged

Upgrade to Node.js 20 #162696

merged 29 commits into from
Nov 20, 2023

Conversation

watson
Copy link
Contributor

@watson watson commented Jul 28, 2023 •
edited by jbudz
Loading

jbudz edits:

  • Removing WIP notice
  • Add link to second mock-fs sync issue in tasklist
  • Reviewers: mock-fs is currently not working with synchronous fs methods. All of our uses of mock-fs were already wrapped in asynchronous methods and so promises were used in place in several areas.

Changes

Platforms and toolchains

Node.js changes

Blockers
Beta Give feedback
  1. Upgrade re2 from v1.17.7 to v1.20.1 #162880
    2 of 2
    Team:Operations backported release_note:skip v8.10.0
    watson
  2. Consolidate Elastic APM dependencies #162686
    Team:APM Team:Operations backport:skip release_note:skip v8.10.0
    watson
  3. Upgrade to Node 20 ems-client#208
    v7.17.1 v8.4.1 v8.5.1
  4. readFileSync with 'utf-8' option broken by node v20.5.0 tschaub/mock-fs#377
  5. fs.statSync no longer mocked from NodeJS 20.8.0 tschaub/mock-fs#380

@watson watson added release_note:skip Skip the PR/issue when compiling release notes backport:all-open Backport to all branches that could still receive a release labels Jul 28, 2023
@watson watson self-assigned this Jul 28, 2023
@github-actions
Copy link
Contributor

Documentation preview:

@watson watson force-pushed the nodejs20 branch 6 times, most recently from 1b15ba9 to 4fd7bc4 Compare August 4, 2023 20:20
@watson watson added backport:skip This commit does not require backporting and removed backport:all-open Backport to all branches that could still receive a release labels Aug 4, 2023
@watson watson force-pushed the nodejs20 branch 3 times, most recently from ae29e67 to 3bd9e85 Compare August 7, 2023 19:58
Thomas Watson added 4 commits August 9, 2023 11:22
Upgrade to Node.js 20
22269e4
Fix invalid JSON error message jest tests
65a9dcc
Get around mock-fs incompatibility with Node.js 20
460fba9
Increase server close timeout
08f9411 
Prior to Node.js 19.0.0, calling close while an idle client was connected to a
server would not actually close the server before the client disconnected. This
behavior was modified in Node.js 19.0.0:

nodejs/node#43522

This resulted in our mock server being closed before we expected and as a
result, some of the tests in `cluster.test.js` would fail. This is because the
1 second timeout is lower than the exponential backoff logic in the
`_autoRetry` function inside the `native_realm.js` file.

To get around this issue, the timeout have simply been incresed to 5 seconds
instead of 1. This is not pretty, but it gets the job done.
@watson watson added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 9, 2023
@dokmic dokmic mentioned this pull request Nov 7, 2023
Merged
1 task
davismcphee
davismcphee approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code-only review. Data Discovery esdsl.test.ts change LGTM 👍

watson
watson commented Nov 16, 2023
View reviewed changes
Copy link
Contributor Author

@watson watson left a comment

Choose a reason for hiding this comment

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

@jbudz your changes look awesome! Thanks 💯

sphilipse
sphilipse approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

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

ent-search changes LGTM

@kibana-ci
Copy link
Collaborator

kibana-ci commented Nov 20, 2023 •
edited
Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #37 / endpoint When on the Endpoint Policy Details Page when on Ingest Policy Edit Package Policy page "after each" hook for "should include updated endpoint data when saved"
  • [job] [logs] FTR Configs #37 / endpoint When on the Endpoint Policy Details Page when on Ingest Policy Edit Package Policy page "before each" hook for "should include updated endpoint data when saved"

Metrics [docs]

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5656 +5656
total size - 5.9MB +5.9MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @watson

Ikuni17
Ikuni17 approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@Ikuni17 Ikuni17 left a comment

Choose a reason for hiding this comment

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

Ops changes LGTM. Checked some deployments and ran locally.

@watson watson merged commit 029b3ba into elastic:main Nov 20, 2023
33 of 34 checks passed
@watson watson deleted the nodejs20 branch November 20, 2023 20:47
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
❌ 7.17 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 162696

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@jbudz jbudz added the reverted label Nov 21, 2023
@jbudz
Copy link
Member

jbudz commented Nov 21, 2023

This was reverted with f51e6cd until prebuilds are sorted. Compile times are causing issues for contributors.

charlie-pichette pushed a commit to charlie-pichette/kibana that referenced this pull request Nov 21, 2023
@jbudz
Revert "Upgrade to Node.js 20 (elastic#162696)"
f51e6cd 
This reverts commit 029b3ba.
@kibanamachine kibanamachine mentioned this pull request Nov 21, 2023
Merged
@jbudz jbudz mentioned this pull request Dec 13, 2023
Closed
@mistic mistic mentioned this pull request Dec 19, 2023
Merged
jbudz pushed a commit to jbudz/kibana that referenced this pull request Dec 26, 2023
@mistic
Revert "Revert "Upgrade to Node.js 20 (elastic#162696)""
c8856b6 
This reverts commit f51e6cd.
jbudz added a commit that referenced this pull request Jan 2, 2024
@mistic @jbudz
chore(NA): bump node into v20 (#173461)
c6f9d98 
Closes #173334

This is a reattempt of doing what was planned on
#162696 after solving the
bottlenecks we discovered previously.

---------

Co-authored-by: Jonathan Budzenski <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 2, 2024
@mistic
chore(NA): bump node into v20 (elastic#173461)
7a1554c 
Closes elastic#173334

This is a reattempt of doing what was planned on
elastic#162696 after solving the
bottlenecks we discovered previously.

---------

Co-authored-by: Jonathan Budzenski <[email protected]>
(cherry picked from commit c6f9d98)
This was referenced May 15, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbudz jbudz jbudz left review comments

@sphilipse sphilipse sphilipse approved these changes

@davismcphee davismcphee davismcphee approved these changes

@dokmic dokmic dokmic approved these changes

@Ikuni17 Ikuni17 Ikuni17 approved these changes

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

@mbondyra mbondyra mbondyra approved these changes

@maximpn maximpn maximpn approved these changes

@sebelga sebelga sebelga approved these changes

@pheyos pheyos pheyos approved these changes

@pmuellr pmuellr pmuellr approved these changes

Assignees

@watson watson

Labels
auto-backport Deprecated - use backport:version if exact versions are needed ci:build-all-platforms ci:build-canvas-shareable-runtime ci:build-storybooks ci:cloud-deploy Create or update a Cloud deployment ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project ci:project-deploy-observability Create an Observability project ci:project-deploy-security Create a Security Serverless Project Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:skip Skip the PR/issue when compiling release notes reverted Team:Operations Team label for Operations Team v7.17.16 v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.