[dev/cli] detect worker type using env, not cluster module #83977

spalger · 2020-11-20T20:17:16Z

I'm working on moving us off of the cluster module, and wanted to start by removing our usage of the cluster module's isMaster and isWorker flags, and instead rely only on the process.env to determine if we're running as a child of the dev cli or not. Additionally, the dev/cli used to manage two types of workers: server and optimizer, but now we only have a single server worker (maybe more soon) so I switched to a simpler flag: process.env.isDevCliChild.

…e-cluster-detection

elasticmachine · 2020-11-23T16:21:41Z

Pinging @elastic/kibana-operations (Team:Operations)

spalger · 2020-11-23T16:22:27Z

packages/kbn-legacy-logging/src/rotate/index.ts

@@ -30,12 +29,6 @@ export async function setupLoggingRotate(server: Server, config: LegacyLoggingCo
    return;
  }

-  // We just want to start the logging rotate service once
-  // and we choose to use the master (prod) or the worker server (dev)
-  if (!isMaster && isWorker && process.env.kbnWorkerType !== 'server') {


In master all workers are of type server, so this condition will always evaluate to false

…e-cluster-detection

spalger · 2020-11-24T17:43:19Z

@elasticmachine merge upstream

packages/kbn-config/src/env.ts

joshdover · 2020-11-24T19:54:09Z

packages/kbn-legacy-logging/src/rotate/log_rotator.ts

-    if (isMaster) {
-      (process as NodeJS.EventEmitter).emit('SIGHUP');
+    if (!process.env.isDevCliChild || !process.send) {
+      process.emit('SIGHUP', 'SIGHUP');


Why are there two arguments to this function? It appears the second argument is supposed to be a socket or server?

That's what @types/node calls for, signal listeners get the signal as their first argument and the emit type reflects that.

…e-cluster-detection

kibanamachine · 2020-11-24T22:33:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 6f72a36

Metrics [docs]

✅ unchanged

History

💚 Build #90046 succeeded 32f9e7c
💚 Build #89752 succeeded 6b67a1d
💚 Build #89696 succeeded 60b1a19
💚 Build #89396 succeeded 2d034f5
💔 Build #89390 failed a883ede

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

vigneshshanmugam · 2020-11-25T06:32:20Z

src/apm.js

@@ -30,10 +30,6 @@ let apmConfig;
 const isKibanaDistributable = Boolean(build && build.distributable === true);

 module.exports = function (serviceName = name) {
-  if (process.env.kbnWorkerType === 'optmzr') {


Do we want to start APM for both parent and child CLI process? Do they behave differently?

I don't know if we want to run APM in both processes, but they do behave differently. The parent process is basically just loading enough of Kibana to understand the config files and proxy requests to the child process which runs the full server and is restarted by the parent process when a server file is changed.

Based on the way things are setup it looks like APM is intentionally setup in the parent process, and it uses a different service name to identify itself as the parent.

Thanks for the info, yeah makes sense. I am fine with keeping it as it is for now.

…3977) * [dev/cli] detect worker type using env, not cluster module * remove unused property * assume that if process.send is undefined we are not a child * update comment Co-authored-by: spalger <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

) (#84349) * [dev/cli] detect worker type using env, not cluster module * remove unused property * assume that if process.send is undefined we are not a child * update comment Co-authored-by: spalger <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: spalger <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

* master: (70 commits) [Uptime] Fix headers io-ts type (elastic#84089) [fleet] Add config options to accepted docker env vars (elastic#84338) [Fleet] Support URL query state in agent logs UI (elastic#84298) [basePathProxy] include query in redirect (elastic#84356) [Security Solution] Add Endpoint policy feature checks (elastic#83972) Fix issues with show_license_expiration (elastic#84361) [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103) [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354) [Fleet] Support input-level vars & templates (elastic#83878) [APM] Elastic chart issues (elastic#84238) [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873) redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287) add live region for field search (elastic#84310) [ML] Persisted URL state for Anomalies table (elastic#84314) [dev/cli] detect worker type using env, not cluster module (elastic#83977) [Workplace Search] Migrate DisplaySettings tree (elastic#84283) Deprecate `xpack.task_manager.index` setting (elastic#84155) [Search] Search batching using bfetch (again) (elastic#84043) Use .kibana instead of .kibana_current to mark migration completion (elastic#83373) [Monitoring] Only look at ES for the missing data alert for now (elastic#83839) ...

* master: (119 commits) [Uptime] Fix headers io-ts type (elastic#84089) [fleet] Add config options to accepted docker env vars (elastic#84338) [Fleet] Support URL query state in agent logs UI (elastic#84298) [basePathProxy] include query in redirect (elastic#84356) [Security Solution] Add Endpoint policy feature checks (elastic#83972) Fix issues with show_license_expiration (elastic#84361) [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103) [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354) [Fleet] Support input-level vars & templates (elastic#83878) [APM] Elastic chart issues (elastic#84238) [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873) redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287) add live region for field search (elastic#84310) [ML] Persisted URL state for Anomalies table (elastic#84314) [dev/cli] detect worker type using env, not cluster module (elastic#83977) [Workplace Search] Migrate DisplaySettings tree (elastic#84283) Deprecate `xpack.task_manager.index` setting (elastic#84155) [Search] Search batching using bfetch (again) (elastic#84043) Use .kibana instead of .kibana_current to mark migration completion (elastic#83373) [Monitoring] Only look at ES for the missing data alert for now (elastic#83839) ...

spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 20, 2020

spalger force-pushed the dev-cli/remove-cluster-detection branch from b61306f to 31e43f2 Compare November 20, 2020 20:23

[dev/cli] detect worker type using env, not cluster module

a883ede

spalger force-pushed the dev-cli/remove-cluster-detection branch from 497b7cc to a883ede Compare November 20, 2020 20:46

remove unused property

2d034f5

This comment has been minimized.

Sign in to view

Merge branch 'master' of github.com:elastic/kibana into dev-cli/remov…

60b1a19

…e-cluster-detection

spalger marked this pull request as ready for review November 23, 2020 16:21

spalger requested review from vigneshshanmugam, watson and a team as code owners November 23, 2020 16:21

spalger commented Nov 23, 2020

View reviewed changes

watson requested review from trentm and astorm November 23, 2020 16:52

trentm mentioned this pull request Nov 23, 2020

Update CODEOWNERS with new APM people #84120

Merged

spalger added 2 commits November 23, 2020 11:20

assume that if process.send is undefined we are not a child

6347688

Merge branch 'master' of github.com:elastic/kibana into dev-cli/remov…

6b67a1d

…e-cluster-detection

watson removed request for watson, trentm and astorm November 24, 2020 09:40

Merge branch 'master' into dev-cli/remove-cluster-detection

32f9e7c

joshdover reviewed Nov 24, 2020

View reviewed changes

spalger added 2 commits November 24, 2020 13:19

Merge branch 'master' of github.com:elastic/kibana into dev-cli/remov…

b5446b7

…e-cluster-detection

update comment

6f72a36

joshdover approved these changes Nov 24, 2020

View reviewed changes

vigneshshanmugam reviewed Nov 25, 2020

View reviewed changes

vigneshshanmugam approved these changes Nov 25, 2020

View reviewed changes

spalger merged commit ea8ea4e into elastic:master Nov 25, 2020

spalger deleted the dev-cli/remove-cluster-detection branch November 25, 2020 15:03

spalger mentioned this pull request Nov 25, 2020

[7.x] [dev/cli] detect worker type using env, not cluster module (#83977) #84349

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dev/cli] detect worker type using env, not cluster module #83977

[dev/cli] detect worker type using env, not cluster module #83977

spalger commented Nov 20, 2020 •

edited

Loading

This comment has been minimized.

elasticmachine commented Nov 23, 2020

spalger Nov 23, 2020

spalger commented Nov 24, 2020

joshdover Nov 24, 2020

spalger Nov 24, 2020

kibanamachine commented Nov 24, 2020

vigneshshanmugam Nov 25, 2020

spalger Nov 25, 2020

spalger Nov 25, 2020

vigneshshanmugam Nov 25, 2020

[dev/cli] detect worker type using env, not cluster module #83977

[dev/cli] detect worker type using env, not cluster module #83977

Conversation

spalger commented Nov 20, 2020 • edited Loading

This comment has been minimized.

elasticmachine commented Nov 23, 2020

spalger Nov 23, 2020

Choose a reason for hiding this comment

spalger commented Nov 24, 2020

joshdover Nov 24, 2020

Choose a reason for hiding this comment

spalger Nov 24, 2020

Choose a reason for hiding this comment

kibanamachine commented Nov 24, 2020

💚 Build Succeeded

Metrics [docs]

History

vigneshshanmugam Nov 25, 2020

Choose a reason for hiding this comment

spalger Nov 25, 2020

Choose a reason for hiding this comment

spalger Nov 25, 2020

Choose a reason for hiding this comment

vigneshshanmugam Nov 25, 2020

Choose a reason for hiding this comment

spalger commented Nov 20, 2020 •

edited

Loading