opensearch-project / OpenSearch-Dashboards

πŸ“Š Open source visualization dashboards for OpenSearch.
https://opensearch.org/docs/latest/dashboards/index/
Apache License 2.0
1.67k stars 875 forks source link

Bump Node.js from 14 to 18 Proposal #3601

Closed ananzh closed 1 year ago

ananzh commented 1 year ago

Background

This is a proposal to bump Node.js from 14 to 18. Major concern is the security support. Based on the announcement of nodejs [1], here is a list of end of security support for different node.js package.

The best one we could do is version 18.

Pros and Cons of 18

Pros [2],[3]

Cons

Upgrade Propsal

There are some detail proposals to bump to node.js 18. The main difference is the sass package we use. Currently, we are using dart-sass (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2054). To improve dart sass performance, we brought in fibers package (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2319). However, fibers is no longer maintained and it doesn't support node.js 18. When bump to 18, you will see the following errors

np bld    log   [04:43:58.801] [warning][@osd/optimizer] worker stderr node: ../src/coroutine.cc:134: void* find_thread_id_key(void*): Assertion `thread_id_key != 0x7777' failed.

Therefore, I propose to either go back to node-sass or use sass-embedded.

Proposal-1 Bump node-sass to v8.0.0 which supports node.js 18. Then bump node.js to 18.

node-sass is deprecated but still maintained. It recently released v8.0.0 [8], about 4 months ago, to support node.js v18. sass-loader also patch the latest 10.4.1, 4 months ago, to support node-sass v8.0.0 [9].

pros

cons

Proposal-2 Use the latest sass-embedded and bump webpack from v4 to v5. Then bump node.js to 18.

This approach requires to bump sass-loader to v12.6.0+ which is the version that starts to support sass-embedded. Then we will observe the following errors during bootstrap:

       β”‚          ERROR in ./public/index.scss?v7light (/home/ubuntu/work/OpenSearch-Dashboards/node_modules/css-loader/dist/cjs.js??ref--6-oneOf-1-1!/home/ubuntu/work/OpenSearch-Dashboards/node_modules/postcss-loader/dist/cjs.js??ref--6-oneOf-1-2!/home/ubuntu/work/OpenSearch-Dashboards/node_modules/sass-loader/dist/cjs.js??ref--6-oneOf-1-3!./public/index.scss?v7light)
       β”‚          Module build failed (from /home/ubuntu/work/OpenSearch-Dashboards/node_modules/sass-loader/dist/cjs.js):
       β”‚          TypeError: this.getOptions is not a function
       β”‚              at Object.loader (/home/ubuntu/work/OpenSearch-Dashboards/node_modules/sass-loader/dist/index.js:27:24)
       β”‚           @ ./public/index.scss?v7light 2:26-262
       β”‚           @ ./public/index.scss
       β”‚           @ ./public/plugin.ts
       β”‚           @ ./public/index.ts
       β”‚           @ /home/ubuntu/work/OpenSearch-Dashboards/packages/osd-optimizer/target/worker/entry_point_creator.js

This error needs us to bump webpack to 5 [5].

pros

cons

Proposal-3 Bump to a sass-embedded that supports webpack v4. Then bump to node.js 18.

Well, this is not a working proposal. With my research and implementation, I found the bottleneck is not sass-embedded which does not have a request on webpack version, it is sass-loader. sass-loader supports sass-embedded since 12.5.0 [7] but version 11.0.0 is the last version that supports Webpack 4. If we need to use Webpack 4, you should install sass-loader version 11.0.0 or an earlier version.

Summary

Using sass-embedded can improve performance but it requires us to bump webpack to v5. We should discuss whether we want to put upgrading to webpack 5 in our long term roadmap. Right now, seems proposal-1 is our only option to upgrade node.js to version 18 before 4/30/2023.

Reference

[1] https://endoflife.date/nodejs [2] https://nodesource.com/blog/11-features-nodeJS-18-to-try [3] https://nodejs.org/de/blog/announcements/v18-release-announce/ [4] https://github.com/nodejs/node/issues/45662 [5] https://github.com/webpack-contrib/sass-loader/issues/924 [6] https://sass-lang.com/blog/node-fibers-discontinued [7] https://github.com/webpack-contrib/sass-loader/releases [8] https://www.npmjs.com/package/node-sass?activeTab=versions [9] https://github.com/webpack-contrib/sass-loader/releases

ananzh commented 1 year ago

How to bump to node.js 18 using node-sass

Steps

ubuntu@ip-172-31-55-237:~/work/OpenSearch-Dashboards$ yarn osd bootstrap
yarn run v1.22.19
$ node scripts/osd bootstrap
 info [opensearch-dashboards] running yarn

$ node ./preinstall_check
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/4] Resolving packages...
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.5.2"
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning "workspace-aggregator-98577cfb-2723-4fe4-b408-7a2473d70b73 > osd_tp_run_pipeline > @elastic/eui@1.0.0" has incorrect peer dependency "typescript@^4.0.5".
warning " > http-aws-es@6.0.0" has unmet peer dependency "aws-sdk@^2.138.0".
warning " > http-aws-es@6.0.0" has incorrect peer dependency "elasticsearch@^15.0.0".
[4/4] Building fresh packages...
[-/18] β’€ waiting...
[6/18] β’€ re2
[-/18] β’€ waiting...
[13/18] β’€ lmdb-store
error /home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store: Command failed.
Exit code: 1
Command: node-gyp-build
Arguments: 
Directory: /home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store
Output:
gyp info it worked if it ends with ok
gyp info using node-gyp@8.4.1
gyp info using node@18.15.0 | linux | x64
gyp info find Python using Python version 3.8.10 found at "/usr/bin/python3"
gyp info spawn /usr/bin/python3
gyp info spawn args [
gyp info spawn args   '/home/ubuntu/work/OpenSearch-Dashboards/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/ubuntu/work/OpenSearch-Dashboards/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/ubuntu/.cache/node-gyp/18.15.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/home/ubuntu/.cache/node-gyp/18.15.0',
gyp info spawn args   '-Dnode_gyp_dir=/home/ubuntu/work/OpenSearch-Dashboards/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/home/ubuntu/.cache/node-gyp/18.15.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
make: Entering directory '/home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store/build'
  CXX(target) Release/obj.target/lmdb/src/node-lmdb.o
In file included from ../src/node-lmdb.h:32,
                 from ../src/node-lmdb.cpp:1:
../../nan/nan.h: In function β€˜void Nan::SetAccessor(v8::Local<v8::ObjectTemplate>, v8::Local<v8::String>, Nan::GetterCallback, Nan::SetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, Nan::imp::Sig)’:
../../nan/nan.h:2551:16: warning: β€˜void v8::ObjectTemplate::SetAccessor(v8::Local<v8::Name>, v8::AccessorNameGetterCallback, v8::AccessorNameSetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, v8::Local<v8::AccessorSignature>, v8::SideEffectType, v8::SideEffectType)’ is deprecated: Do signature check in accessor [-Wdeprecated-declarations]
 2551 |     , signature);
      |                ^
In file included from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-function.h:15,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8.h:33,
                 from ../src/node-lmdb.h:29,
                 from ../src/node-lmdb.cpp:1:
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-template.h:838:8: note: declared here
  838 |   void SetAccessor(
      |        ^~~~~~~~~~~
  CC(target) Release/obj.target/lmdb/dependencies/lmdb/libraries/liblmdb/midl.o
  CC(target) Release/obj.target/lmdb/dependencies/lmdb/libraries/liblmdb/chacha8.o
  CC(target) Release/obj.target/lmdb/dependencies/lz4/lib/lz4.o
  CXX(target) Release/obj.target/lmdb/src/env.o
In file included from ../src/node-lmdb.h:32,
                 from ../src/env.cpp:25:
../../nan/nan.h: In function β€˜void Nan::SetAccessor(v8::Local<v8::ObjectTemplate>, v8::Local<v8::String>, Nan::GetterCallback, Nan::SetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, Nan::imp::Sig)’:
../../nan/nan.h:2551:16: warning: β€˜void v8::ObjectTemplate::SetAccessor(v8::Local<v8::Name>, v8::AccessorNameGetterCallback, v8::AccessorNameSetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, v8::Local<v8::AccessorSignature>, v8::SideEffectType, v8::SideEffectType)’ is deprecated: Do signature check in accessor [-Wdeprecated-declarations]
 2551 |     , signature);
      |                ^
In file included from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-function.h:15,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8.h:33,
                 from ../src/node-lmdb.h:29,
                 from ../src/env.cpp:25:
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-template.h:838:8: note: declared here
  838 |   void SetAccessor(
      |        ^~~~~~~~~~~
  CXX(target) Release/obj.target/lmdb/src/compression.o
In file included from ../src/node-lmdb.h:32,
                 from ../src/compression.cpp:26:
../../nan/nan.h: In function β€˜void Nan::SetAccessor(v8::Local<v8::ObjectTemplate>, v8::Local<v8::String>, Nan::GetterCallback, Nan::SetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, Nan::imp::Sig)’:
../../nan/nan.h:2551:16: warning: β€˜void v8::ObjectTemplate::SetAccessor(v8::Local<v8::Name>, v8::AccessorNameGetterCallback, v8::AccessorNameSetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, v8::Local<v8::AccessorSignature>, v8::SideEffectType, v8::SideEffectType)’ is deprecated: Do signature check in accessor [-Wdeprecated-declarations]
 2551 |     , signature);
      |                ^
In file included from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-function.h:15,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8.h:33,
                 from ../src/node-lmdb.h:29,
                 from ../src/compression.cpp:26:
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-template.h:838:8: note: declared here
  838 |   void SetAccessor(
      |        ^~~~~~~~~~~
  CXX(target) Release/obj.target/lmdb/src/ordered-binary.o
In file included from ../src/node-lmdb.h:32,
                 from ../src/ordered-binary.cpp:1:
../../nan/nan.h: In function β€˜void Nan::SetAccessor(v8::Local<v8::ObjectTemplate>, v8::Local<v8::String>, Nan::GetterCallback, Nan::SetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, Nan::imp::Sig)’:
../../nan/nan.h:2551:16: warning: β€˜void v8::ObjectTemplate::SetAccessor(v8::Local<v8::Name>, v8::AccessorNameGetterCallback, v8::AccessorNameSetterCallback, v8::Local<v8::Value>, v8::AccessControl, v8::PropertyAttribute, v8::Local<v8::AccessorSignature>, v8::SideEffectType, v8::SideEffectType)’ is deprecated: Do signature check in accessor [-Wdeprecated-declarations]
 2551 |     , signature);
      |                ^
In file included from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-function.h:15,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8.h:33,
                 from ../src/node-lmdb.h:29,
                 from ../src/ordered-binary.cpp:1:
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-template.h:838:8: note: declared here
  838 |   void SetAccessor(
      |        ^~~~~~~~~~~
../src/ordered-binary.cpp: In function β€˜size_t valueToKey(const v8::Local<v8::Value>&, uint8_t*, size_t, bool, bool)’:
../src/ordered-binary.cpp:138:92: error: no matching function for call to β€˜v8::Symbol::Description()’
  138 |         Local<String> string = Local<String>::Cast(Local<Symbol>::Cast(jsKey)->Description());
      |                                                                                            ^
In file included from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-object.h:11,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-array-buffer.h:13,
                 from /home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8.h:24,
                 from ../src/node-lmdb.h:29,
                 from ../src/ordered-binary.cpp:1:
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-primitive.h:588:16: note: candidate: β€˜v8::Local<v8::Value> v8::Symbol::Description(v8::Isolate*) const’
  588 |   Local<Value> Description(Isolate* isolate) const;
      |                ^~~~~~~~~~~
/home/ubuntu/.cache/node-gyp/18.15.0/include/node/v8-primitive.h:588:16: note:   candidate expects 1 argument, 0 provided
make: *** [lmdb.target.mk:153: Release/obj.target/lmdb/src/ordered-binary.o] Error 1
make: Leaving directory '/home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/ubuntu/work/OpenSearch-Dashboards/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (node:events:513:28)
gyp ERR! stack     at ChildProcess._handle.onexit (node:internal/child_process:291:12)
gyp ERR! System Linux 5.15.0-1023-aws
gyp ERR! command "/home/ubuntu/.nvm/versions/node/v18.15.0/bin/node" "/home/ubuntu/work/OpenSearch-Dashboards/node_modules/.bin/node-gyp" "rebuild"
gyp ERR! cwd /home/ubuntu/work/OpenSearch-Dashboards/node_modules/lmdb-store

ERROR [bootstrap] failed:
ERROR Error: Command failed with exit code 1: /usr/share/yarn/bin/yarn.js install --non-interactive
          at makeError (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:25150:11)
          at handlePromise (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:24085:26)
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async installInDir (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:23821:3)
          at async Project.installDependencies (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:14780:5)
          at async Object.run (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:9003:11)
          at async runCommand (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:57870:5)
          at async Module.run (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:263:3)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
yarn osd bootstrap
yarn run v1.22.19
$ node scripts/osd bootstrap
 info [opensearch-dashboards] running yarn

$ node ./preinstall_check
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/4] Resolving packages...
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.5.2"
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning "workspace-aggregator-19bd11ba-347d-4179-afc6-87b45fab54c2 > osd_tp_run_pipeline > @elastic/eui@1.0.0" has incorrect peer dependency "typescript@^4.0.5".
warning " > http-aws-es@6.0.0" has unmet peer dependency "aws-sdk@^2.138.0".
warning " > http-aws-es@6.0.0" has incorrect peer dependency "elasticsearch@^15.0.0".
[4/4] Building fresh packages...

 succ yarn.lock analysis completed without any issues
 info [@osd/cross-platform] running [osd:bootstrap] script
 info [@osd/test-subj-selector] running [osd:bootstrap] script
 info [@osd/utility-types] running [osd:bootstrap] script
ERROR [bootstrap] failed:
ERROR Error: Command failed with exit code 2: /usr/share/yarn/bin/yarn.js run osd:bootstrap
      error Command failed with exit code 2.
      error Command failed with exit code 2.
      $ yarn build
      $ tsc
      ../../node_modules/@types/node/ts4.8/test.d.ts(612,34): error TS1005: '?' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(613,17): error TS1005: ':' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(614,17): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(617,34): error TS1005: '?' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(618,17): error TS1005: ':' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(619,17): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(619,26): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(620,5): error TS1109: Expression expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(624,24): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(628,35): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(634,39): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(638,21): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,19): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,27): error TS1005: ':' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,36): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,55): error TS1005: '{' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,64): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(643,77): error TS1005: ',' expected.
      ../../node_modules/@types/node/ts4.8/test.d.ts(647,22): error TS1005: ',' expected.
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
          at makeError (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:25150:11)
          at handlePromise (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:24085:26)
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async /home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:9051:9
          at async scheduleItem (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:10938:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
yarn osd bootstrap
yarn run v1.22.19
$ node scripts/osd bootstrap
 info [opensearch-dashboards] running yarn

$ node ./preinstall_check
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/4] Resolving packages...
warning Resolution field "@types/node@18.11.18" is incompatible with requested version "@types/node@12.20.24"
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.5.2"
warning Resolution field "@types/node@18.11.18" is incompatible with requested version "@types/node@16.9.1"
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning "workspace-aggregator-f785b3e9-c172-441d-8175-4eb0791d33be > osd_tp_run_pipeline > @elastic/eui@1.0.0" has incorrect peer dependency "typescript@^4.0.5".
warning " > http-aws-es@6.0.0" has unmet peer dependency "aws-sdk@^2.138.0".
warning " > http-aws-es@6.0.0" has incorrect peer dependency "elasticsearch@^15.0.0".
[4/4] Building fresh packages...
success Saved lockfile.

 succ yarn.lock analysis completed without any issues
 info [@osd/cross-platform] running [osd:bootstrap] script
 info [@osd/test-subj-selector] running [osd:bootstrap] script
 info [@osd/utility-types] running [osd:bootstrap] script
 succ [@osd/test-subj-selector] bootstrap complete
 succ [@osd/utility-types] bootstrap complete
 succ [@osd/cross-platform] bootstrap complete
 info [@osd/config-schema] running [osd:bootstrap] script
 info [@osd/std] running [osd:bootstrap] script
 succ [@osd/std] bootstrap complete
 succ [@osd/config-schema] bootstrap complete
 info [@osd/logging] running [osd:bootstrap] script
 info [@osd/utils] running [osd:bootstrap] script
 succ [@osd/logging] bootstrap complete
 succ [@osd/utils] bootstrap complete
 info [@osd/apm-config-loader] running [osd:bootstrap] script
 info [@osd/dev-utils] running [osd:bootstrap] script
 succ [@osd/apm-config-loader] bootstrap complete
ERROR [bootstrap] failed:
ERROR Error: Command failed with exit code 2: /usr/share/yarn/bin/yarn.js run osd:bootstrap
      error Command failed with exit code 2.
      error Command failed with exit code 2.
      $ yarn build
      $ tsc
      src/proc_runner/proc.ts(156,29): error TS2345: Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
        Type 'undefined' is not assignable to type 'number'.
      src/proc_runner/proc.ts(164,29): error TS2345: Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
        Type 'undefined' is not assignable to type 'number'.
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
          at makeError (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:25150:11)
          at handlePromise (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:24085:26)
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async /home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:9051:9
          at async scheduleItem (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:10938:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR [bootstrap] failed:
ERROR Error: Command failed with exit code 1: /usr/share/yarn/bin/yarn.js run osd:bootstrap
      error Command failed with exit code 1.
      $ node scripts/build_ts_refs && node scripts/register_git_hook
      ERROR Error: Command failed with exit code 2: /home/ubuntu/work/OpenSearch-Dashboards/node_modules/typescript/bin/tsc -b tsconfig.refs.json --pretty
            src/core/server/status/plugins_status.ts:108:5 - error TS2322: Type 'Observable<unknown>' is not assignable to type 'Observable<Record<string, ServiceStatus<unknown>>>'.
              Type 'unknown' is not assignable to type 'Record<string, ServiceStatus<unknown>>'.

            108     return this.update$.pipe(
                    ~~~~~~~~~~~~~~~~~~~~~~~~~
            109       switchMap(() => {
                ~~~~~~~~~~~~~~~~~~~~~~~
            ... 
            126       })
                ~~~~~~~~
            127     );
                ~~~~~~

            src/core/server/status/status_service.ts:98:7 - error TS2345: Argument of type 'MonoTypeOperatorFunction<unknown>' is not assignable to parameter of type 'OperatorFunction<ServiceStatus<unknown>, ServiceStatus<unknown>>'.
              Type 'Observable<unknown>' is not assignable to type 'Observable<ServiceStatus<unknown>>'.
                Type 'unknown' is not assignable to type 'ServiceStatus<unknown>'.

            98       distinctUntilChanged(isDeepStrictEqual),
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

            src/core/server/status/status_service.ts:152:7 - error TS2345: Argument of type 'MonoTypeOperatorFunction<unknown>' is not assignable to parameter of type 'OperatorFunction<{ opensearch: ServiceStatus<OpenSearchStatusMeta>; savedObjects: ServiceStatus<SavedObjectStatusMeta>; }, CoreStatus>'.
              Type 'Observable<unknown>' is not assignable to type 'Observable<CoreStatus>'.
                Type 'unknown' is not assignable to type 'CoreStatus'.

            152       distinctUntilChanged(isDeepStrictEqual),
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

            src/core/server/bootstrap.ts:32:10 - error TS2614: Module '"cluster"' has no exported member 'isMaster'. Did you mean to use 'import isMaster from "cluster"' instead?

            32 import { isMaster as isClusterManager } from 'cluster';
                        ~~~~~~~~

            src/core/server/bootstrap.ts:100:17 - error TS2571: Object is of type 'unknown'.

            100     if (!msg || msg.reloadLoggingConfig !== true) {
                                ~~~

            Found 5 errors.

                at makeError (/home/ubuntu/work/OpenSearch-Dashboards/node_modules/execa/lib/error.js:59:11)
                at handlePromise (/home/ubuntu/work/OpenSearch-Dashboards/node_modules/execa/index.js:114:26)
                at processTicksAndRejections (node:internal/process/task_queues:95:5)
                at buildRefs (/home/ubuntu/work/OpenSearch-Dashboards/src/dev/typescript/build_refs.ts:41:5)
                at buildAllRefs (/home/ubuntu/work/OpenSearch-Dashboards/src/dev/typescript/build_refs.ts:35:3)
                at description (/home/ubuntu/work/OpenSearch-Dashboards/src/dev/typescript/build_refs.ts:51:7)
                at /home/ubuntu/work/OpenSearch-Dashboards/packages/osd-dev-utils/target/run/run.js:64:13
                at Object.withProcRunner (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-dev-utils/target/proc_runner/with_proc_runner.js:45:9)
                at run (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-dev-utils/target/run/run.js:63:9)
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
          at makeError (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:25150:11)
          at handlePromise (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:24085:26)
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async /home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:9051:9
          at async scheduleItem (/home/ubuntu/work/OpenSearch-Dashboards/packages/osd-pm/dist/index.js:10938:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Need to fix the type errors. The 3 errors in src/core/server/status/status_service.ts by specifying return type. For example:

src/core/server/status/status_service.ts:152:7 - error TS2345: Argument of type 'MonoTypeOperatorFunction<unknown>' is not assignable to parameter of type 'OperatorFunction<{ opensearch: ServiceStatus<OpenSearchStatusMeta>; savedObjects: ServiceStatus<SavedObjectStatusMeta>; }, CoreStatus>'.
              Type 'Observable<unknown>' is not assignable to type 'Observable<CoreStatus>'.
                Type 'unknown' is not assignable to type 'CoreStatus'.

            152       distinctUntilChanged(isDeepStrictEqual),
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This type error can be fixed by changing:
distinctUntilChanged(isDeepStrictEqual) --> distinctUntilChanged<CoreStatus>(isDeepStrictEqual)

Result

After step 10, OSD runs up successfully using v18.12.0. Note: The highest we could upgrade is v18.15.0 which needs to bump typescript version.

Run yarn build-platform --darwin --skip-archives --release

.......
   β”‚          limit: 27596,

   β”‚        {
   β”‚          group: 'page load bundle size',
   β”‚          id: 'visualizations',
   β”‚          value: 368928,
   β”‚          limit: 295169,
   β”‚          limitConfigPath: 'packages/osd-optimizer/limits.yml'
   β”‚        },
   β”‚        { group: 'async chunks size', id: 'visualizations', value: 0 },
   β”‚        {
   β”‚          group: 'miscellaneous assets size',
   β”‚          id: 'visualizations',
   β”‚          value: 0
   β”‚        }
   β”‚      ]
   β”‚ debg visualize metrics [
   β”‚        {
   β”‚          group: '@osd/optimizer bundle module count',
   β”‚          id: 'visualize',
   β”‚          value: 209
   β”‚        },
   β”‚        {
   β”‚          group: 'page load bundle size',
   β”‚          id: 'visualize',
   β”‚          value: 42132,
   β”‚          limit: 57433,
   β”‚          limitConfigPath: 'packages/osd-optimizer/limits.yml'
   β”‚        },
   β”‚        { group: 'async chunks size', id: 'visualize', value: 508564 },
   β”‚        { group: 'miscellaneous assets size', id: 'visualize', value: 0 }
   β”‚      ]
   β”‚ warn Metric [page load bundle size] for [core] of [1,189,835] over the limit of [692,646]
   β”‚ warn Metric [page load bundle size] for [console] of [107,797] over the limit of [46,235]
   β”‚ warn Metric [page load bundle size] for [dashboard] of [391,561] over the limit of [374,267]
   β”‚ warn Metric [page load bundle size] for [data] of [1,266,704] over the limit of [1,174,083]
   β”‚ warn Metric [page load bundle size] for [devTools] of [100,725] over the limit of [38,781]
   β”‚ warn Metric [page load bundle size] for [discover] of [111,419] over the limit of [105,147]
   β”‚ warn Metric [page load bundle size] for [embeddable] of [267,233] over the limit of [242,671]
   β”‚ warn Metric [page load bundle size] for [expressions] of [264,805] over the limit of [224,120]
   β”‚ warn Metric [page load bundle size] for [indexPatternManagement] of [229,103] over the limit of [154,366]
   β”‚ warn Metric [page load bundle size] for [inputControlVis] of [242,689] over the limit of [172,819]
   β”‚ warn Metric [page load bundle size] for [inspector] of [296,399] over the limit of [148,999]
   β”‚ warn Metric [page load bundle size] for [mapsLegacy] of [174,102] over the limit of [116,961]
   β”‚ warn Metric [page load bundle size] for [navigation] of [99,187] over the limit of [37,413]
   β”‚ warn Metric [page load bundle size] for [opensearchDashboardsLegacy] of [171,389] over the limit of [107,855]
   β”‚ warn Metric [page load bundle size] for [opensearchDashboardsOverview] of [120,028] over the limit of [56,426]
   β”‚ warn Metric [page load bundle size] for [opensearchDashboardsReact] of [368,591] over the limit of [162,353]
   β”‚ warn Metric [page load bundle size] for [opensearchUiShared] of [391,094] over the limit of [326,798]
   β”‚ warn Metric [page load bundle size] for [regionMap] of [156,642] over the limit of [66,098]
   β”‚ warn Metric [page load bundle size] for [savedObjects] of [172,727] over the limit of [108,662]
   β”‚ warn Metric [page load bundle size] for [share] of [160,056] over the limit of [99,205]
   β”‚ warn Metric [page load bundle size] for [visDefaultEditor] of [382,057] over the limit of [50,178]
   β”‚ warn Metric [page load bundle size] for [visTypeTable] of [182,275] over the limit of [95,078]
   β”‚ warn Metric [page load bundle size] for [visTypeTimeseries] of [248,950] over the limit of [155,347]
   β”‚ warn Metric [page load bundle size] for [visTypeVega] of [295,143] over the limit of [153,861]
   β”‚ warn Metric [page load bundle size] for [visTypeVislib] of [306,140] over the limit of [242,982]
   β”‚ warn Metric [page load bundle size] for [visualizations] of [368,928] over the limit of [295,169]
   β”‚ succ βœ“ 1 min 45 sec

 info [  opensearch-dashboards  ] Transpiling sources with babel
   β”‚ succ βœ“ 10 sec

 info [  opensearch-dashboards  ] Remove workspace artifacts
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Cleaning source for packages that are now installed in node_modules
   β”‚ debg Deleting patterns: [
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards/packages',
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards/yarn.lock'
   β”‚      ]
   β”‚ debg Deleted 2 files/directories
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Generating NOTICE.txt file
   β”‚ info Generating notice from source
       β”‚ debg vfs.src globs [ '**/*.{js,less,css,ts,tsx}' ]
       β”‚ debg vfs.src options {
       β”‚        cwd: '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards',
       β”‚        nodir: true,
       β”‚        ignore: [
       β”‚          '{node_modules,build,dist,data,built_assets}/**',
       β”‚          'packages/*/{node_modules,build,dist}/**',
       β”‚          'src/plugins/*/{node_modules,build,dist}/**',
       β”‚          '**/target/**'
       β”‚        ]
       β”‚      }
       β”‚ info Searching /home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards for multi-line comments starting with @notice
       β”‚ info Found @notice comment in src/core/server/core_app/assets/legacy_dark_theme.css
       β”‚ info Found @notice comment in src/core/server/core_app/assets/legacy_light_theme.css
       β”‚ debg notice text:
       β”‚
       β”‚      OpenSearch (https://opensearch.org/)
       β”‚      Copyright OpenSearch Contributors
       β”‚
       β”‚      This product includes software, including Kibana source code,
       β”‚      developed by Elasticsearch (http://www.elastic.co).
       β”‚      Copyright 2009-2021 Elasticsearch B.V.
       β”‚
       β”‚      This product includes software developed by The Apache Software
       β”‚      Foundation (http://www.apache.org/)
       β”‚
       β”‚      This product includes software developed by
       β”‚      Joda.org (http://www.joda.org/).
       β””      ---
       β”‚      This product bundles bootstrap@3.3.6 which is available under a
       β”‚      "MIT" license.
       β”‚
       β”‚      The MIT License (MIT)
       β”‚
       β”‚      Copyright (c) 2011-2015 Twitter, Inc
       β”‚
       β”‚      Permission is hereby granted, free of charge, to any person obtaining a copy
       β”‚      of this software and associated documentation files (the "Software"), to deal
       β”‚      in the Software without restriction, including without limitation the rights
       β”‚      to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
       β”‚      copies of the Software, and to permit persons to whom the Software is
       β”‚      furnished to do so, subject to the following conditions:
       β”‚
       β”‚      The above copyright notice and this permission notice shall be included in
       β”‚      all copies or substantial portions of the Software.
       β”‚
       β”‚      THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
       β”‚      IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
       β”‚      FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
       β”‚      AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
       β”‚      LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
       β”‚      OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
       β”‚      THE SOFTWARE.
       β”‚
       β”‚
   β”‚ info Discovering installed packages
   β”‚ info Generating build notice
   β”‚ info Writing notice to NOTICE.txt
   β”‚ succ βœ“ 2 sec

 info [  opensearch-dashboards  ] Updating LICENSE.txt file
   β”‚ info Copying Apache 2.0 license to LICENSE.txt
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Removing dependencies from package.json
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Cleaning typescript source files that have been transpiled to JS
   β”‚ info Deleted 7750 files
   β”‚ succ βœ“ 2 sec

 info [  opensearch-dashboards  ] Cleaning tests, examples, docs, etc. from node_modules
   β”‚ info Deleted 4732 files
   β”‚ succ βœ“ 2 sec

 info [  opensearch-dashboards  ] Cleaning all empty folders recursively
   β”‚ debg Deleting all empty folders and their children recursively starting on  /home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards
   β”‚ debg Deleted 912 empty folders
   β”‚ succ βœ“ 3 sec

 info [  opensearch-dashboards  ] Creating platform-specific archive source directories
   β”‚ debg Generic build source copied into darwin-x64 specific build directory
   β”‚ debg Node.js copied into darwin-x64 specific build directory
   β”‚ succ βœ“ 1 sec

 info [  opensearch-dashboards  ] Patching platform-specific native modules
   β”‚ debg Patching re2 binaries from https://github.com/uhop/node-re2/releases/download/1.17.4/darwin-x64-83.gz to /home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node_modules/re2/build/Release/re2.node
   β”‚ debg Deleting patterns: [
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node_modules/re2/build/Release/re2.node'
   β”‚      ]
   β”‚ debg Deleted 1 files/directories
   β”‚ debg Attempting download of https://github.com/uhop/node-re2/releases/download/1.17.4/darwin-x64-83.gz 9112ed93c1544ecc6397f7ff20bd2b28f3b04c7fbb54024e10f9a376a132a87d
   β”‚ debg Downloaded https://github.com/uhop/node-re2/releases/download/1.17.4/darwin-x64-83.gz and verified checksum
   β”‚ succ βœ“ 1 sec

 info [  opensearch-dashboards  ] Installing Chromium
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Cleaning extra bin/* scripts from platform-specific builds
   β”‚ debg Deleting patterns: [
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/bin/*.bat'
   β”‚      ]
   β”‚ debg Deleted 3 files/directories
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Cleaning npm from node
   β”‚ debg Deleting patterns: [
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node/**/node_modules',
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node/**/npm*',
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node/**/npx*',
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node/**/corepack*',
   β”‚        '/home/ubuntu/work/OpenSearch-Dashboards/build/opensearch-dashboards-3.0.0-darwin-x64/node/**/nodevars*'
   β”‚      ]
   β”‚ debg Deleted 266 files/directories
   β”‚ succ βœ“ 0 sec

 info [  opensearch-dashboards  ] Checking Windows for paths > 200 characters
   β”‚ succ βœ“ 1 sec

 info [  opensearch-dashboards  ] Verify that no UUID file is baked into the build
   β”‚ succ βœ“ 0 sec

 info [  global  ] Writing sha1sums of archives and packages in target directory
   β”‚ succ βœ“ 0 sec

Done in 273.19s.
Screenshot 2023-03-13 at 10 33 55

Reference:

[1] https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/64262

seanneumann commented 1 year ago

Thanks @ananzh! This is great.

Compatibility issues: Upgrading to Node.js 18 may require changes and upgrade some of the dependency packages.

Can you add the list of dependent packages you are aware of?

Performance concern: I found some complains about a slight V18 performance downgrade from V16. [4]

Looks like the 10-13% startup slow down is addressed here. We should confirm though. But even if not, the pros of the upgrade outweigh the con.

ananzh commented 1 year ago

Thanks @ananzh! This is great.

Compatibility issues: Upgrading to Node.js 18 may require changes and upgrade some of the dependency packages.

Can you add the list of dependent packages you are aware of?

Performance concern: I found some complains about a slight V18 performance downgrade from V16. [4]

Looks like the 10-13% startup slow down is addressed here. We should confirm though. But even if not, the pros of the upgrade outweigh the con.

Changed dependency packages

Performance compare

From the following compare, we could see node.js v14.20.1 does 49 bundle using 138.1 sec and v18.15.0 (latest v18 LTS) does 48 bundles with 53 sec (This has one less bundle because I removed myPluginName). So the performance concern seems not a problem. Since that performance concern is between 16 and 18. Not sure if 16 will give us a better performance but 16 is not our option.

server    log   [00:08:25.042] [info][savedobjects-service] Waiting until all OpenSearch nodes are compatible with OpenSearch Dashboards before starting saved objects migrations...
server    log   [00:08:25.063] [info][savedobjects-service] Starting saved objects migrations
server    log   [00:08:25.103] [info][plugins-system] Starting [38] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,visBuilder,tileMap,regionMap,inputControlVis,visualize,myPluginName,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:08:25.278] [info][listening] Server running at http://localhost:5603/xhg
server    log   [00:08:25.304] [info][server][OpenSearchDashboards][http] http server running at http://localhost:5603/xhg
np bld    log   [00:10:39.023] [success][@osd/optimizer] 49 bundles compiled successfully after 138.1 sec, watching for changes
server    log   [00:21:04.739] [info][savedobjects-service] Waiting until all OpenSearch nodes are compatible with OpenSearch Dashboards before starting saved objects migrations...
server    log   [00:21:04.818] [info][savedobjects-service] Starting saved objects migrations
server    log   [00:21:04.968] [info][plugins-system] Starting [37] plugins: [usageCollection,opensearchDashboardsUsageCollection,opensearchDashboardsLegacy,mapsLegacy,share,opensearchUiShared,legacyExport,embeddable,expressions,data,home,console,apmOss,management,indexPatternManagement,advancedSettings,savedObjects,dashboard,visualizations,visTypeVega,visTypeTimeline,timeline,visTypeTable,visTypeMarkdown,visBuilder,tileMap,regionMap,inputControlVis,visualize,charts,visTypeVislib,visTypeTimeseries,visTypeTagcloud,visTypeMetric,discover,savedObjectsManagement,bfetch]
server    log   [00:21:05.518] [info][listening] Server running at http://localhost:5603/yho
server    log   [00:21:05.612] [info][server][OpenSearchDashboards][http] http server running at http://localhost:5603/yho
np bld    log   [00:21:49.603] [success][@osd/optimizer] 48 bundles compiled successfully after 53 sec, watching for changes
ananzh commented 1 year ago

Dev error

Build and Verify on Linux error

error elastic-apm-node@3.31.0: The engine "node" is incompatible with this module. Expected version "^8.6.0 || 10 || 12 || 14 || 15 || 16 || 17". Got "18.15.0"

elastic-apm-node is a node-module which block us to bump to v18.

Monaco build error

Monaco uses a different build script and export NODE_OPTIONS=--openssl-legacy-provider && node ./scripts/build.js --dev throws error because windows doesn't recognize export

ERROR Error: Command failed with exit code 1: C:\npm\prefix\node_modules\yarn\bin\yarn.js run osd:bootstrap
      'export' is not recognized as an internal or external command,
      operable program or batch file.
      error Command failed with exit code 1.
      error Command failed with exit code 1.
      $ yarn build --dev
      $ export NODE_OPTIONS=--openssl-legacy-provider && node ./scripts/build.js --dev
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
      info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
          at makeError (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\packages\osd-pm\dist\index.js:25150:11)
          at handlePromise (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\packages\osd-pm\dist\index.js:24085:26)
          at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\packages\osd-pm\dist\index.js:9051:9
          at async scheduleItem (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\packages\osd-pm\dist\index.js:[109](https://github.com/opensearch-project/OpenSearch-Dashboards/actions/runs/4429617021/jobs/7771524465#step:11:110)38:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
yarn run v1.22.10
ananzh commented 1 year ago

[Unit test] async handle issue

FAIL  src/core/server/plugins/discovery/plugins_discovery.test.ts (91.864 s)
  ● plugins discovery system β€Ί discovers plugins in the search locations [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      145 |   });
      146 |
    > 147 |   it('discovers plugins in the search locations', async () => {
          |   ^
      148 |     const { plugin$ } = discover(new PluginsConfig(pluginConfig, env), coreContext, instanceInfo);
      149 |
      150 |     mockFs(

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:147:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)
  ● plugins discovery system β€Ί return errors when the manifest is invalid or incompatible [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      163 |   });
      164 |
    > 165 |   it('return errors when the manifest is invalid or incompatible', async () => {
          |   ^
      166 |     const { plugin$, error$ } = discover(
      167 |       new PluginsConfig(pluginConfig, env),
      168 |       coreContext,

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:165:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)
  ● plugins discovery system β€Ί discovers plugins in nested directories [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      287 |   });
      288 |
    > 289 |   it('discovers plugins in nested directories', async () => {
          |   ^
      290 |     const { plugin$, error$ } = discover(
      291 |       new PluginsConfig(pluginConfig, env),
      292 |       coreContext,

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:289:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)
  ● plugins discovery system β€Ί does not discover plugins nested inside another plugin [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      328 |   });
      329 |
    > 330 |   it('does not discover plugins nested inside another plugin', async () => {
          |   ^
      331 |     const { plugin$ } = discover(new PluginsConfig(pluginConfig, env), coreContext, instanceInfo);
      332 |
      333 |     mockFs(

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:330:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)
  ● plugins discovery system β€Ί stops scanning when reaching `maxDepth` [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      347 |   });
      348 |
    > 349 |   it('stops scanning when reaching `maxDepth`', async () => {
          |   ^
      350 |     const { plugin$ } = discover(new PluginsConfig(pluginConfig, env), coreContext, instanceInfo);
      351 |
      352 |     mockFs(

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:349:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)
  ● plugins discovery system β€Ί works with symlinks [Attempt #3]
    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      375 |   });
      376 |
    > 377 |   it('works with symlinks', async () => {
          |   ^
      378 |     const { plugin$ } = discover(new PluginsConfig(pluginConfig, env), coreContext, instanceInfo);
      379 |
      380 |     const pluginFolder = resolve(PROCESS_WORKING_DIR, '..', 'ext-plugins');

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:120:22)
      at Suite.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:377:3)
      at Object.<anonymous> (src/core/server/plugins/discovery/plugins_discovery.test.ts:90:1)

After fixing, see errors on Windows:

FAIL  src/core/server/plugins/discovery/plugins_discovery.test.ts
  ● plugins discovery system β€Ί return an error when the manifest file is not accessible

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 1

      Array [
    -   "Error: EACCES: permission denied, open '\\\\?\\D:\\a\\OpenSearch-Dashboards\\OpenSearch-Dashboards\\src\\plugins\\plugin_a\\opensearch_dashboards.json' (missing-manifest, D:\\a\\OpenSearch-Dashboards\\OpenSearch-Dashboards\\src\\plugins\\plugin_a\\opensearch_dashboards.json)",
    +   "Error: EACCES: permission denied, open 'D:\\a\\OpenSearch-Dashboards\\OpenSearch-Dashboards\\src\\plugins\\plugin_a\\opensearch_dashboards.json' (missing-manifest, D:\\a\\OpenSearch-Dashboards\\OpenSearch-Dashboards\\src\\plugins\\plugin_a\\opensearch_dashboards.json)",
      ]

      293 |
      294 |     const errorPath = manifestPath('plugin_a');
    > 295 |     expect(errors).toEqual([
          |                    ^
      296 |       `Error: EACCES: permission denied, open '${standardize(
      297 |         errorPath,
      298 |         false,

Analysis and Reproduction

To debug in RxJs observables, two things are useful:

const discoveryResults$ = merge(
  from(config.additionalPluginPaths),
  processPluginSearchPaths$(config.pluginSearchPaths, log)
).pipe(
  mergeMap((pluginPathOrError) => {
    return typeof pluginPathOrError === 'string'
      ? createPlugin$(pluginPathOrError, log, coreContext, instanceInfo)
      : [pluginPathOrError];
  }),
  tap({
    next: value => console.log('Value in discoveryResults$:', value),
    complete: () => console.log('discoveryResults$ completed')
  }),
  shareReplay()
);

After debug, found out the stuck place is this parseManifest function and when it calls fsReadFileAsync:

const fsReadFileAsync = promisify(readFile);

This parseManifest function is used by createPlugin$ in src/core/server/plugins/discovery/plugins_discovery.ts. createPlugin$ is called in discover, which is the actual function we are testing.

export function discover(
  config: PluginsConfig,
  coreContext: CoreContext,
  instanceInfo: InstanceInfo
) {
  const log = coreContext.logger.get('plugins-discovery');
  log.debug('Discovering plugins...');

  if (config.additionalPluginPaths.length && coreContext.env.mode.dev) {
    log.warn(
      `Explicit plugin paths [${config.additionalPluginPaths}] should only be used in development. Relative imports may not work properly in production.`
    );
  }

  const discoveryResults$ = merge(
    from(config.additionalPluginPaths),
    processPluginSearchPaths$(config.pluginSearchPaths, log)
  ).pipe(
    mergeMap((pluginPathOrError) => {
      return typeof pluginPathOrError === 'string'
        ? createPlugin$(pluginPathOrError, log, coreContext, instanceInfo)
        : [pluginPathOrError];
    }),
    shareReplay()
  );

  return {
    plugin$: discoveryResults$.pipe(
      filter((entry): entry is PluginWrapper => entry instanceof PluginWrapper)
    ),
    error$: discoveryResults$.pipe(
      filter((entry): entry is PluginDiscoveryError => !(entry instanceof PluginWrapper))
    ),
  };
}

It seems that mock-fs which is used in the test to mock fs system does not properly handle the asynchronous readFile function, causing the tests to get stuck. It's possible that the issue is related to changes in the Node.js APIs or the underlying file system implementation in Node.js 18 compared to Node.js 14. The mock-fs library might not have been updated to handle these changes properly, which could lead to unexpected behavior when using the library with newer Node.js versions.

To confirm this hypothesis, we could try to reproduce the issue in a simple standalone script using Node.js 14 and Node.js 18. If the issue only occurs with Node.js 18, it would be reasonable to assume that the problem is related to changes in the newer version of Node.js.

Let's create a test:

const { readFile, readFileSync } = require('fs');
const mockFs = require('mock-fs');

mockFs({
  '/test/plugins/plugin_a': {
    'opensearch_dashboards.json': mockFs.file({
      mode: 0o666,
      content: JSON.stringify({ id: 'plugin', version: '1' }),
    }),
  },
});

// Using readFile (asynchronous)
readFile('/test/plugins/plugin_a/opensearch_dashboards.json', (err, data) => {
  if (err) {
    console.error('[readFile] Error:', err);
  } else {
    console.log('[readFile] Data:', data.toString());
  }
});

// Using readFileSync (synchronous)
try {
  const data = readFileSync('/test/plugins/plugin_a/opensearch_dashboards.json');
  console.log('[readFileSync] Data:', data.toString());
} catch (err) {
  console.error('[readFileSync] Error:', err);
}

// Clean up mock-fs after tests
mockFs.restore();

The result:

ubuntu@***:~/test-fs$ nvm use 18.15.0
Now using node v18.15.0 (npm v9.5.0)
ubuntu@***:~/test-fs$ node test.js
[readFileSync] Data: {"id":"plugin","version":"1"}
[readFile] Error: [Error: EINVAL: invalid argument, read] {
  errno: -22,
  code: 'EINVAL',
  syscall: 'read'
}
Aborted (core dumped)
ubuntu@***:~/test-fs$ nvm use 14.21.3
Now using node v14.21.3 (npm v6.14.18)
ubuntu@***:~/test-fs$ node --version
v14.21.3
ubuntu@***:~/test-fs$ node test.js
[readFileSync] Data: {"id":"plugin","version":"1"}
[readFile] Error: [Error: EINVAL: invalid argument, read] {
  errno: -22,
  code: 'EINVAL',
  syscall: 'read'
}
Aborted (core dumped)

Well, it seems the issue is not with readFile. It is possible that the combination of mock-fs, asynchronous file reading with readFile, and the way the tests are structured could be causing issues in Node.js 18. The exact reason for this behavior is not clear for now.

Solutions

Change to synchronous method

To resolve this issue, we used the synchronous version of readFile, called readFileSync. This function reads the file content in a synchronous manner, meaning it blocks the execution of the code until the read operation is completed. Since this function doesn't rely on callbacks or Promises, it is not affected by the same issues as the asynchronous version when used with mock-fs.

import { readFileSync } from 'fs'; 
const fsReadFileAsync = (path) => Promise.resolve(readFileSync(path));

Pass a default fsReadFileAsyncFn

src/core/server/plugins/discovery/plugin_manifest_parser.ts

export async function parseManifest(
  pluginPath: string,
  packageInfo: PackageInfo,
  log: Logger,
  fsReadFileAsyncFn: (path: string) => Promise<Buffer> = fsReadFileAsync // Default to fsReadFileAsync
): Promise<PluginManifest> {
  // ...
}

Same in discover and createPlugin$

src/core/server/plugins/discovery/plugins_discovery.ts

export function discover(
  config: PluginsConfig,
  coreContext: CoreContext,
  instanceInfo: InstanceInfo,
  fsReadFileAsyncFn: (path: string) => Promise<Buffer> = fsReadFileAsync // Default to fsReadFileAsync
) {
  // ...
}

function createPlugin$(
  pluginPath: string,
  log: Logger,
  coreContext: CoreContext,
  instanceInfo: InstanceInfo,
  fsReadFileAsyncFn: (path: string) => Promise<Buffer> = fsReadFileAsync // Default to fsReadFileAsync
) {
  // ...
}

In the test cases, provide a custom implementation as needed:

const fsReadFileAsyncWrapper = async (path: string) => {
  // custom implementation for reading files in tests
};

const { plugin$ } = discover(
  new PluginsConfig(pluginConfig, env),
  coreContext,
  instanceInfo,
  fsReadFileAsyncWrapper
);
ananzh commented 1 year ago

Deprecations

See some deprecation msg when running tests. fs.rmdir should be changed to fs.rm.

(node:1772) [DEP0147] DeprecationWarning: In future versions of Node.js, fs.rmdir(path, { recursive: true }) will be removed. Use fs.rm(path, { recursive: true }) instead
(Use `node --trace-deprecation ...` to show where the warning was created)
Node.js process-warning detected:

DeprecationWarning: In future versions of Node.js, fs.rmdir(path, { recursive: true }) will be removed. Use fs.rm(path, { recursive: true }) instead
    at emitRecursiveRmdirWarning (node:internal/fs/utils:844:13)
    at rmdir (node:internal/fs/promises:689:5)
    at Object.<anonymous> (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\packages\osd-cross-platform\src\path.test.ts:63:19)
    at Object.asyncJestLifecycle (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\jest-jasmine2\build\jasmineAsyncInstall.js:113:37)
    at D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\jest-jasmine2\build\queueRunner.js:89:12
    at new Promise (<anonymous>)
    at mapper (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\jest-jasmine2\build\queueRunner.js:72:19)
    at D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\jest-jasmine2\build\queueRunner.js:119:41
    at Object.fn (D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\jest-jasmine2\build\treeProcessor.js:38:7)
ananzh commented 1 year ago

[Unit test] memory leak

<--- Last few GCs --->
[1428](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1429)

[1429](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1430)
[4130:0x6862480]  1415411 ms: Scavenge 2008.4 (2074.5) -> 2004.0 (2076.3) MB, 13.3 / 0.0 ms  (average mu = 0.690, current mu = 0.444) allocation failure; 
[1430](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1431)
[4130:0x6862480]  1415499 ms: Scavenge 2010.1 (2076.3) -> 2005.9 (2078.5) MB, 11.6 / 0.0 ms  (average mu = 0.690, current mu = 0.444) allocation failure; 
[1431](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1432)
[4130:0x6862480]  1415582 ms: Scavenge 2012.7 (2078.8) -> 2008.3 (2096.5) MB, 14.7 / 0.0 ms  (average mu = 0.690, current mu = 0.444) allocation failure; 
[1432](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1433)

[1433](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1434)

[1434](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1435)
<--- JS stacktrace --->
[1435](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1436)

[1436](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1437)
FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
[1437](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1438)
 1: 0xb7b3e0 node::Abort() [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1438](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1439)
 2: 0xa8c8aa  [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1439](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1440)
 3: 0xd69100 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1440](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1441)
 4: 0xd694a7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1441](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1442)
 5: 0xf46ba5  [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1442](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1443)
 6: 0xf5908d v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1443](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1444)
 7: 0xf3378e v8::internal::HeapAllocator::AllocateRawWithLightRetrySlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1444](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1445)
 8: 0xf34b57 v8::internal::HeapAllocator::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1445](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1446)
 9: 0xf150a0 v8::internal::Factory::AllocateRaw(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1446](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1447)
10: 0xf0cb14 v8::internal::FactoryBase<v8::internal::Factory>::AllocateRawWithImmortalMap(int, v8::internal::AllocationType, v8::internal::Map, v8::internal::AllocationAlignment) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1447](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1448)
11: 0xf2340a v8::internal::Factory::NewCallSiteInfo(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::HeapObject>, int, int, v8::internal::Handle<v8::internal::FixedArray>) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1448](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1449)
12: 0xec8f3a  [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1449](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1450)
13: 0xec930a  [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1450](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1451)
14: 0xec9abe v8::internal::Isolate::CaptureAndSetErrorStack(v8::internal::Handle<v8::internal::JSObject>, v8::internal::FrameSkipMode, v8::internal::Handle<v8::internal::Object>) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1451](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1452)
15: 0xde4cef v8::internal::Builtin_ErrorCaptureStackTrace(int, unsigned long*, v8::internal::Isolate*) [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1452](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1453)
16: 0x1707c79  [/opt/hostedtoolcache/node/18.15.0/x64/bin/node]
[1453](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1454)
Aborted (core dumped)
[1454](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1455)
error Command failed with exit code 134.
[1455](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1456)
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[1456](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4473198876/jobs/7860289879#step:14:1457)
Error: Process completed with exit code 134.

First of all, fix any memory leak.

I see some memory leak on componentDidMount from OptInExampleFlyout when setState is called.

export class OptInExampleFlyout extends React.PureComponent<Props, State> {
  public readonly state: State = {
    data: null,
    isLoading: true,
    hasPrivilegeToRead: false,
  };

  async componentDidMount() {
    try {
      const { fetchExample } = this.props;
      const clusters = await fetchExample();
          this.setState({
          data: Array.isArray(clusters) ? clusters : null,
          isLoading: false,
          hasPrivilegeToRead: true,
        });
    } catch (err) {
        this.setState({
          isLoading: false,
          hasPrivilegeToRead: err.status !== 403,
        });
    }
  }
  ...
  }
 ...
}

The memory leak issue might be caused by updating the component's state after the component has been unmounted. When a component gets unmounted, it's removed from the DOM, but if there's a pending async operation that tries to update the component's state, it can cause a memory leak. Here the componentDidMount method has an async operation. If the component is unmounted before the async operation is resolved, it will try to call setState on an unmounted component, which can lead to the memory leak.

Solution is to add isMounted = false and add componentWillUnmount() to update this flag to false when the component is unmounted. Then make componentDidMount() check this property to setState only when the component is mounted.

export class OptInExampleFlyout extends React.PureComponent<Props, State> {
  private isMounted: boolean = false;
  public readonly state: State = {
    data: null,
    isLoading: true,
    hasPrivilegeToRead: false,
  };

  async componentDidMount() {
    this.isMounted = true;
    try {
      const { fetchExample } = this.props;
      const clusters = await fetchExample();
      if (this.isMounted) {
        this.setState({
          data: Array.isArray(clusters) ? clusters : null,
          isLoading: false,
          hasPrivilegeToRead: true,
        });
      }
    } catch (err) {
      if (this.isMounted) {
        this.setState({
          isLoading: false,
          hasPrivilegeToRead: err.status !== 403,
        });
      }
    }
  }

  componentWillUnmount() {
    this.isMounted = false;
  }
...

Second, increase memory allocation

"test:jest:ci:coverage": "scripts/use_node scripts/jest --ci --colors --runInBand --coverage --max-old-space-size=8192",
ananzh commented 1 year ago

[Unit test] lmdb can't be recognized

src/cli/cluster/cluster_manager.test.ts
  ● Test suite failed to run

    TypeError: The URL must be of scheme file

      33 |
      34 | import chalk from 'chalk';
    > 35 | import * as LmdbStore from 'lmdb';
         | ^
      36 | import { getMatchingRoot } from '@osd/cross-platform';
      37 |
      38 | const GLOBAL_ATIME = `${Date.now()}`;

      at Object.<anonymous> (node_modules/lmdb/native.js:6:23)
      at Object.<anonymous> (packages/osd-optimizer/src/node/cache.ts:35:1)

 FAIL  src/core/server/legacy/legacy_service.test.ts
  ● Test suite failed to run

    TypeError: The URL must be of scheme file

      33 |
      34 | import chalk from 'chalk';
    > 35 | import * as LmdbStore from 'lmdb';
         | ^
      36 | import { getMatchingRoot } from '@osd/cross-platform';
      37 |
      38 | const GLOBAL_ATIME = `${Date.now()}`;

      at Object.<anonymous> (node_modules/lmdb/native.js:6:23)
      at Object.<anonymous> (packages/osd-optimizer/src/node/cache.ts:35:1)
ananzh commented 1 year ago

[Mocha test] a flaky test

Run yarn test:mocha:coverage
yarn run v[1](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4483980643/jobs/7883934789#step:15:1).22.10
$ yarn nyc --reporter=text-summary --reporter=lcov --report-dir=./target/opensearch-dashboards-coverage/mocha node scripts/mocha
$ D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\node_modules\.bin\nyc --reporter=text-summary --reporter=lcov --report-dir=./target/opensearch-dashboards-coverage/mocha node scripts/mocha

  dev/mocha/junit report generation
    1) reports on failed setup hooks

  0 passing (70ms)
  1 failing

  1) dev/mocha/junit report generation
       reports on failed setup hooks:
     Error: expected 'Error: FORCE_TEST_FAIL\n    at Context.<anonymous> (src\\dev\\mocha\\__tests__\\fixtures\\project\\/test.js:34:11)\n    at processImmediate (node:internal/timers:476:[21](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4483980643/jobs/7883934789#step:15:22))' to match /Error: FORCE_TEST_FAIL\n.+fixtures.project.test.js/
      at Assertion.assert (packages\osd-expect\expect.js:[27](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4483980643/jobs/7883934789#step:15:28):447)
      at Assertion.match (packages\osd-expect\expect.js:82:[29](https://github.com/ananzh/OpenSearch-Dashboards/actions/runs/4483980643/jobs/7883934789#step:15:30)3)
      at Context.<anonymous> (src\dev\mocha\__tests__\/junit_report_generation.js:108:36)
ananzh commented 1 year ago

[Integration test] receive 500 response (local+remote build)

$ node scripts/jest_integration
 FAIL  src/core/server/http/integration_tests/request.test.ts (63.291 s)
  ● OpenSearchDashboardsRequest β€Ί events β€Ί aborted$ β€Ί does not complete before response has been sent

    expected 200 "OK", got 500 "Internal Server Error"

      291 |         await server.start();
      292 |
    > 293 |         await supertest(innerServer.listener).post('/').send({ data: 'test' }).expect(200);
          |                                                                                ^
      294 |
      295 |         expect(nextSpy).toHaveBeenCalledTimes(0);
      296 |         expect(completeSpy).toHaveBeenCalledTimes(1);

      at Object.<anonymous> (src/core/server/http/integration_tests/request.test.ts:293:80)
      ----
      at Test._assertStatus (node_modules/supertest/lib/test.js:252:14)
      at node_modules/supertest/lib/test.js:308:13
      at Test._assertFunction (node_modules/supertest/lib/test.js:285:13)
      at Test.assert (node_modules/supertest/lib/test.js:164:23)
      at localAssert (node_modules/supertest/lib/test.js:120:14)
      at fn (node_modules/supertest/lib/test.js:125:7)
      at Test.callback (node_modules/superagent/src/node/index.js:924:3)
      at fn (node_modules/superagent/src/node/index.js:1153:18)
      at IncomingMessage.<anonymous> (node_modules/superagent/src/node/parsers/json.js:19:7)

The issue occurs because in getEvents method finish$ Observable listens to both finish and close events which emits a value when either of these events is emitted. The close event can be emitted before the response is fully sent, while the finish event is emitted when the response is sent successfully.

The completed$ Observable might emit a value as soon as the close event is triggered, even if the response has not been fully sent yet. This causes the test to fail because aborted$ complete called is logged before the response is sent.

By removing either the finish or close event from the merge function, we could ensure that the completed$ Observable only emits a value when the remaining event is triggered. If we remove the close event and keep the finish event, the completed$ Observable will emit a value only when the response is sent successfully, which is the desired behavior for this test.

In conclusion, it's better to keep the finish event and remove the 'close' event, as the finish event is more appropriate for detecting when the response has been sent successfully. The updated getEvents function should look like this:

private getEvents(request: Request): OpenSearchDashboardsRequestEvents {
  const finish$ = fromEvent(request.raw.res, 'finish').pipe(shareReplay(1), first());
  const aborted$ = fromEvent<void>(request.raw.req, 'aborted').pipe(first(), takeUntil(finish$));
  const completed$ = merge<void, void>(finish$, aborted$).pipe(shareReplay(1), first());

  return {
    aborted$,
    completed$,
  } as const;
}
ananzh commented 1 year ago

[Integration test] lmdb path fail (local+remote build)

 FAIL  packages/osd-optimizer/src/integration_tests/basic_optimization.test.ts
  ● Test suite failed to run

    TypeError: The argument 'filename' must be a file URL object, file URL string, or absolute path string. Received 'http://localhost/index.cjs'

      33 |
      34 | import chalk from 'chalk';
    > 35 | import * as LmdbStore from 'lmdb';
         | ^
      36 | import { getMatchingRoot } from '@osd/cross-platform';
      37 |
      38 | const GLOBAL_ATIME = `${Date.now()}`;

      at Function.createRequire (node_modules/jest-runtime/build/index.js:1813:23)
      at Object.<anonymous> (node_modules/lmdb/node-index.js:6:12)
      at Object.<anonymous> (packages/osd-optimizer/src/node/cache.ts:35:1)
      at Object.<anonymous> (packages/osd-optimizer/src/node/node_auto_tranpilation.ts:56:1)

The issue is only for test import. Guess is something with jest config. Currently, we will implement a temp fix to mock lmdb.

joshuali925 commented 1 year ago

given v14 will be EOL on 4/30/23, is there a targeted release or date for this version bump in OSD?

ananzh commented 1 year ago

given v14 will be EOL on 4/30/23, is there a targeted release or date for this version bump in OSD?

@joshuali925 Currently we have a draft PR and other testing branches. I am not sure about the release bc it might be considered as a breaking change. But the target date to bump nodejs 18 on main is 4/21/2023.

ananzh commented 1 year ago

[Integration test] src/core/server/ui_settings/integration_tests/index.test.ts fail for multiple tests (only fail at remote dev env)

 FAIL  src/core/server/ui_settings/integration_tests/index.test.ts (6.333 s)
  ● uiSettings/routes β€Ί doc missing β€Ί get route β€Ί creates doc, returns a 200 with settings
    EBUSY: resource busy or locked, rmdir 'D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\.opensearch\opensearch-test-cluster'

  ● uiSettings/routes β€Ί doc missing β€Ί get route β€Ί creates doc, returns a 200 with settings

    OpenSearch exited with code 1

      31 |  */
      32 | exports.createCliError = function (message) {
    > 33 |   const error = new Error(message);
         |                 ^
      34 |   error.isCliError = true;
      35 |   return error;
      36 | };

      at createCliError (packages/osd-opensearch/target/errors.js:33:17)
      at packages/osd-opensearch/target/cluster.js:432:15
      at Cluster.start (packages/osd-opensearch/target/cluster.js:288:5)
      at OpenSearchTestCluster.start (packages/osd-test/src/legacy_opensearch/legacy_opensearch_test_cluster.js:105:7)
      at Object.startOpenSearch (src/core/test_helpers/osd_server.ts:265:7)
      at Object.startServers (src/core/server/ui_settings/integration_tests/lib/servers.ts:70:22)

  ● uiSettings/routes β€Ί doc missing β€Ί get route β€Ί creates doc, returns a 200 with settings

    TypeError: Cannot read properties of undefined (reading 'opensearch')

      78 |   }
      79 |
    > 80 |   const callCluster = opensearchServer.opensearch.getCallCluster();
         |                                        ^
      81 |
      82 |   const savedObjectsClient = osd.coreStart.savedObjects.getScopedClient(
      83 |     httpServerMock.createOpenSearchDashboardsRequest()

      at getServices (src/core/server/ui_settings/integration_tests/lib/servers.ts:80:40)
      at Object.<anonymous> (src/core/server/ui_settings/integration_tests/doc_missing.ts:36:40)
ananzh commented 1 year ago

[Integration test] sconnect ECONNREFUSED 127.0.0.1 for multiple tests (only fail at remote dev env)

Found multiple test suites failed due to OpenSearch connection refuse. Here is the list of failed tests:

FAIL  src/core/server/core_app/integration_tests/default_route_provider_config.test.ts
FAIL  src/core/server/legacy/integration_tests/legacy_service.test.ts (6.224 s)
FAIL  src/core/server/ui_settings/integration_tests/routes.test.ts (6.371 s)
FAIL  src/core/server/saved_objects/routes/integration_tests/migrate.test.ts (7.424 s)
FAIL  src/core/server/core_app/integration_tests/static_assets.test.ts (7.998 s)
FAIL src/core/server/http/integration_tests/core_services.test.ts (29.747 s)
FAIL src/core/server/http_resources/integration_tests/http_resources_service.test.ts (9.589 s)
FAIL  src/legacy/server/http/integration_tests/max_payload_size.test.js (6.892 s)

Example fail test

(node:6972) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
(node:6972) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 warning listeners added to [process]. Use emitter.setMaxListeners() to increase limit

 FAIL  src/core/server/http/integration_tests/core_services.test.ts (29.747 s)

  ● http service β€Ί auth β€Ί #isAuthenticated() β€Ί returns true if has been authenticated

    connect ECONNREFUSED 127.0.0.1:51113

  ● http service β€Ί auth β€Ί #isAuthenticated() β€Ί returns false if has not been authenticated

    connect ECONNREFUSED 127.0.0.1:51116
AMoo-Miki commented 1 year ago

Step 10

globby@8 had an issue with paths that didn't exist; to avoid us having to check if a folder exists, we should bump to globby@11 which solves this issue. This could be as a result of blobby dropping some deps or this change.

AMoo-Miki commented 1 year ago

To debug integration tests, I modified https://github.com/opensearch-project/OpenSearch-Dashboards/blob/712a42fee9f8606b5e3b4fceb3164f948a87dce0/src/core/server/logging/logging_system.ts#L150-L151

to

const loggerLevel = LogLevel.fromId('all' || level); 
 const loggerAppenders = (['console'] || appenders).map((appenderKey) => this.appenders.get(appenderKey)!); 

The unnecessary || are just to avoid linting errors.

AMoo-Miki commented 1 year ago

[Integration test] receive 500 response (local+remote build)

A change in @hapi/hapi@20.2.1 could explain this.

Previously in lib/transmit.js,

    for (const func of response.request._route._marshalCycle) {
        if (response._state !== 'close') {
            await func(response);
        }
    }

   ...

    if (request._closed) {
        // No more events will be fired, so we proactively close-up shop
        request.raw.res.end(); // Ensure res is finished so internals.end() doesn't think we're responding
        internals.end(env, 'close');
        return team.work;
    }

and now:

    for (const func of response.request._route._marshalCycle) {
        await func(response);
    }

   ...

    if (request._closed) {
        // The request has already been aborted - no need to wait or attempt to write.
        internals.end(env, 'aborted');
        return team.work;
    }

The first code block deals with headers and could be the problem but the comment in the second code block interests me more:

Ensure res is finished so internals.end() doesn't think we're responding

joshuali925 commented 1 year ago

node 18 depends on glibc 2.28, is this a breaking change?

peterzhuamazon commented 1 year ago

This will also break the CI build as we are using CentOS7 which older version of glibc as well, just like AL2. This also means KNN needs to be built on higher version of glibc and might crash as well. (Note: might not seem related to OSD for KNN but if we need to change build image we need to change both OS and OSD in pairs to keep the consistency.)

Thanks.

ananzh commented 1 year ago

[Fuctional Test] 404 error for osd-ui-shared-deps bundles

       β””-: console app
         β””-> "before all" hook in "console app"
         β””-> "before all" hook in "console app"
         β””-: console app
           β””-> "before all" hook for "should show the default request"
           β””-> "before all" hook for "should show the default request"
             β”‚ERROR browser[SEVERE] http://localhost:6610/9007199254740991/bundles/osd-ui-shared-deps/osd-ui-shared-deps.@elastic.js - Failed to load resource: the server responded with a status of 404 (Not Found)
             β”‚ERROR browser[SEVERE] http://localhost:6610/9007199254740991/bundles/osd-ui-shared-deps/osd-ui-shared-deps.js - Failed to load resource: the server responded with a status of 404 (Not Found)
             β”‚ERROR browser[SEVERE] http://localhost:6610/bootstrap.js 58:68 Uncaught TypeError: Cannot read properties of null (reading 'dataset')
             β”‚ERROR browser[SEVERE] http://localhost:6610/9007199254740991/bundles/osd-ui-shared-deps/osd-ui-shared-deps.@elastic.js - Failed to load resource: the server responded with a status of 404 (Not Found)
             β”‚ERROR browser[SEVERE] http://localhost:6610/9007199254740991/bundles/osd-ui-shared-deps/osd-ui-shared-deps.js - Failed to load resource: the server responded with a status of 404 (Not Found)
ananzh commented 1 year ago

Update: We are working on three options for bumping node 18 1) legacy webpack: webpack 4 + node sass 8.0 + node 18.15.0 2) patched webpack: patched webpack + dart sass + node 18.15.0 3) SWC: SWC + dart sass + node 18.15.0

We will make a decision early next week from the above three options. We will also hook up plugins and open issues in each plugin.

ananzh commented 1 year ago

[BWC Test] Fail due to set-value not built in the artifact

set-value is not built in the artifact after node.js 18 and the current fix is to add it as a dependency in package.json. I have made an initial analysis and opened an issue because we might need more research and re-visit it after bumping to Node.js 18: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/4064

ananzh commented 1 year ago

[Integration Test] invalid_config.test.ts hangs in the GitHub environment

This test has spawns a child process to run a script. https://github.com/opensearch-project/OpenSearch-Dashboards/blob/c5058a3ff2d46f52059d0d06a32bc93f300dd13d/src/cli/serve/integration_tests/invalid_config.test.ts#L49

By adding console logs in the test

describe('cli invalid config support', function () {
  it(
    'exits with statusCode 64 and logs a single line when config is invalid',
    function () {
      // Unused keys only throw once LegacyService starts, so disable migrations so that Core
      // will finish the start lifecycle without a running OpenSearch instance.

      // eslint-disable-next-line no-console
      console.log('REPO_ROOT is:', REPO_ROOT); // Added log
      // eslint-disable-next-line no-console
      console.log('INVALID_CONFIG_PATH is:', INVALID_CONFIG_PATH); // Added log
      const { error, status, stdout, stderr } = spawnSync(
        process.execPath,
        [
          'scripts/opensearch_dashboards',
          '--config',
          INVALID_CONFIG_PATH,
          '--migrations.skip=true',
        ],
        {
          cwd: REPO_ROOT,
        }
      );
      // eslint-disable-next-line no-console
      console.log('spawnSync error is:', error); // Added log
      // eslint-disable-next-line no-console
      console.log('spawnSync stdout is:', stdout.toString('utf8')); // Added log
      // eslint-disable-next-line no-console
      console.log('spawnSync stderr is:', stderr.toString('utf8')); // Added log

      const [fatalLogLine] = stdout
        .toString('utf8')
        .split('\n')
        .filter(Boolean)
        .map((line) => JSON.parse(line) as LogEntry)
        .filter((line) => line.tags.includes('fatal'))
        .map((obj) => ({
          ...obj,
          pid: '## PID ##',
          '@timestamp': '## @timestamp ##',
          error: '## Error with stack trace ##',
        }));

      expect(error).toBe(undefined);

      if (!fatalLogLine) {
        throw new Error(
          `cli did not log the expected fatal error message:\n\nstdout: \n${stdout}\n\nstderr:\n${stderr}`
        );
      }

      expect(fatalLogLine.message).toContain(
        'Error: Unknown configuration key(s): "unknown.key", "other.unknown.key", "other.third", "some.flat.key", ' +
          '"some.array". Check for spelling errors and ensure that expected plugins are installed.'
      );
      expect(fatalLogLine.tags).toEqual(['fatal', 'root']);
      expect(fatalLogLine.type).toEqual('log');

      expect(status).toBe(64);
    },
    20 * 1000
  );
});

And see the logs in the Github env:

yarn run v1.22.10
$ scripts/use_node scripts/jest_integration --ci --colors
  console.log
    REPO_ROOT is: D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards

      at Object.<anonymous> (src/cli/serve/integration_tests/invalid_config.test.ts:51:15)

  console.log
    INVALID_CONFIG_PATH is: D:\a\OpenSearch-Dashboards\OpenSearch-Dashboards\src\cli\serve\integration_tests\__fixtures__\invalid_config.yml

      at Object.<anonymous> (src/cli/serve/integration_tests/invalid_config.test.ts:53:15)

Error: The operation was canceled.

It shows clearly that this script doesn’t terminate correctly or hangs which causes the child process never finishes in the GitHub environment.

joshuali925 commented 1 year ago

@peterzhuamazon i found that node18 can be built from source on centos 7, it runs ok and doesn't require glibc > 2.17. but centos 7 build tools need to be updated

❯ objdump -T ./node | awk '{print $5}' | grep -i glibc | sort | uniq
GLIBC_2.10
GLIBC_2.14
GLIBC_2.16
GLIBC_2.17
GLIBC_2.2.5
GLIBC_2.3
GLIBC_2.3.2
GLIBC_2.3.3
GLIBC_2.3.4
GLIBC_2.4
GLIBC_2.6
GLIBC_2.7
GLIBC_2.9
GLIBCXX_3.4
GLIBCXX_3.4.11
GLIBCXX_3.4.14
GLIBCXX_3.4.15
GLIBCXX_3.4.18
GLIBCXX_3.4.9

build script used

cat << 'EOF' | docker build -t nodejs - && docker create --name nodejs nodejs && docker cp nodejs:/build/nodejs . && docker rm nodejs && docker rmi nodejs
FROM centos:7

WORKDIR /build
ENV NODE_VERSION=v18.15.0

RUN yum install -y centos-release-scl-rh centos-release-scl && \
          yum install -y devtoolset-11-gcc devtoolset-11-gcc-c++ devtoolset-11-make python3 python3-pip && \
          source /opt/rh/devtoolset-11/enable && \
          curl -sL -o- https://nodejs.org/dist/$NODE_VERSION/node-$NODE_VERSION.tar.xz | tar -xJv && cd node-$NODE_VERSION && \
          ./configure --prefix=$PWD/../node-$NODE_VERSION-linux-$(arch) && make -j $(nproc) && make install && cd .. && \
          strip ./node-$NODE_VERSION-linux-$(arch)/bin/node && \
          mkdir -p nodejs && tar cvzf nodejs/node-$NODE_VERSION-linux-$(arch).tar.gz ./node-$NODE_VERSION-linux-$(arch)
EOF

If we need to run this on older systems probably need to build a custom version of node. i'm not sure if there will be significant performance differences or other issues due to lower glibc version used to compile