Mirror ES status; set own status to red if user settings are not found #7454

ycombinator · 2016-06-14T18:26:04Z

If user settings are requested but are not available in the Kibana index, turn the Elasticsearch plugin yellow. When the plugin automatically recreates the index, it will go green.

Fixes #7424

… available If user settings are requested but are not available in the Kibana index, turn the Kibana plugin red. If they are requested and available, turn the plugin green again, if necessary.

bevacqua · 2016-06-14T18:30:16Z

LGTM!

Yellow is good in situations where the user doesn't need to take remedial actions to turn a plugin green. Red is good in situations where the user needs to do something to turn the plugin green.

ycombinator · 2016-06-14T19:08:43Z

Now that I'm looking at this PR, I think we can simplify things a bit further:

Instead of mucking with the Kibana plugin status, we only need to turn the Elasticsearch plugin status yellow if the settings are not found. And we don't need to worry about turning it green again in this code; the Elasticsearch plugin's code will do that when it (re-)creates the Kibana index and the config doc within it.

Thoughts @bevacqua, @LeeDr, @epixa?

bevacqua · 2016-06-14T19:10:32Z

I think that'd be a mistake. If the query times out ES will be marked yellow and never again be marked green.

ycombinator · 2016-06-14T19:11:35Z

Actually I believe the ES plugin regularly polls ES so it may eventually go green.

LeeDr · 2016-06-14T19:28:29Z

src/ui/settings/index.js

+  function resetKibanaPluginStatusIfNecessary(user) {
+    const isElasticsearchPluginGreen = server.plugins.elasticsearch.status.state === 'green';
+    const isKibanaPluginCurrentlyYellow = server.plugins.kibana.status.state === 'yellow';
+    if (isElasticsearchPluginGreen && isKibanaPluginCurrentlyYellow) {


What if server.plugins.kibana.status.state === 'red' ? Does code somewhere else change it from red to yellow, and then this changes it yellow to green?

Related: https://elastic.slack.com/archives/kibana/p1465930751000513

Yeah, good point Lee. I really think all we ought to do when user settings are not found is set the Elastiscearch plugin status to yellow, because ultimately that's the plugin responsible for (re)creating the .kibana index and going green again when the index is ready. I think we shouldn't muck with the Kibana plugin status at all. Thoughts?

LeeDr · 2016-06-14T22:39:10Z

Your current code is working for me. And our tests are really suffering right now. So maybe we can go with what you have here now, and open another issue to reconsider it?

Or revert whatever change put this in place until we have a better solution?

ycombinator · 2016-06-14T23:09:28Z

@LeeDr Alright, lets try to get this PR in today. I'm going to do one more thing: I'm going to update this PR to do the simplification in #7454 (comment). If the tests continue to work well with that, lets use that. Otherwise I'll revert to the current state of this PR and we can use that.

The Elasticsearch plugin is what is responsible for (re)creating the Kibana index. So it makes sense that if we couldn't find the user settings in that index, we set the Elasticsearch plugin to yellow. Once it recreates the Kibana index, it will set itself to green. No need to toggle the states of the Kibana plugin at all.

LeeDr · 2016-06-15T02:23:18Z

jenkins, test it

spalger · 2016-06-15T02:24:45Z

I don't like how this is setting the status of the Kibana plugin, so I made #7459 so that this service could have it's own status.

I implemented it in this pr and sent a pr to @ycombinator in ycombinator#1

Here is what the flow looks like with these changes:

server starts up normally:
request for user settings fails but the health check has not run yet:
healthcheck runs:
elasticsearch comes back online and healthcheck runs:

ycombinator · 2016-06-15T02:48:05Z

@spalger I completely agree that non-plugin code setting plugin status is hacky. Thanks for building a more generic notion of status's that could affect the overall server status!

bevacqua · 2016-06-15T02:52:38Z

@spalger This is awesome

spalger · 2016-06-15T05:04:50Z

If you merge master here the changed files will be a lot cleaner

ycombinator · 2016-06-15T05:06:45Z

Merged master; had to force push.

…status

[ui/settings] go red when settings are not found, otherwise mimic es …

spalger · 2016-06-15T05:43:58Z

jenkins, test it

ycombinator · 2016-06-15T07:19:16Z

@spalger @bevacqua Mind taking a look at this PR again? I had to amend a line in the tests (7b61123 and 658d5f5) to make them pass again.

spalger · 2016-06-15T07:20:29Z

LGTM

bevacqua · 2016-06-15T15:24:48Z

src/ui/settings/index.js

+
+    function copyStatus() {
+      const { state } = esStatus;
+      status[state](state === 'green' ? 'Ready' : `Elasticsearch plugin is ${state}`);


I feel like this line should change into a proper conditional, or maybe just extract the ternary into a const variable.

I felt wrong typing it

bevacqua · 2016-06-15T15:25:10Z

Just a minor nit but LGTM, you don't have to heed my comment.

…bility

bevacqua · 2016-06-15T15:38:21Z

LGTM

ycombinator · 2016-06-15T15:39:16Z

Thanks folks. Will merge as soon as build passes.

Mirror ES status; set own status to red if user settings are not found Former-commit-id: 9714484

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([#7435](elastic/eui#7435))

`v91.3.1`⏩`v92.0.0-backport.0` --- ## [`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0) **This is a backport release only intended for use by Kibana.** **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) ## [`v92.0.0`](https://github.com/elastic/eui/releases/v92.0.0) - Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and `EuiSearchBar.Query.execute` to add `extends object` constraint ([elastic#7340](elastic/eui#7340)) - This change should have no impact on your applications since the updated types only affect properties that exclusively accept object values. - Added a new `EuiFlyoutResizable` component ([elastic#7439](elastic/eui#7439)) - Updated `EuiTextArea` to accept `isClearable` and `icon` as props ([elastic#7449](elastic/eui#7449)) **Bug fixes** - `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their positions on resize ([elastic#7442](elastic/eui#7442)) **Deprecations** - Updated `EuiFilterButton` to remove the second `.euiFilterButton__textShift` span wrapper. Target `.euiFilterButton__text` instead ([elastic#7444](elastic/eui#7444)) **Breaking changes** - Removed deprecated `EuiNotificationEvent`. We recommend copying the component to your application if necessary ([elastic#7434](elastic/eui#7434)) - Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar` instead ([elastic#7435](elastic/eui#7435))

`v92.0.0-backport.0`⏩ `v92.1.1` --- ## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1) **Bug fixes** - Minor `EuiDataGrid` cell performance fixes ([#7465](elastic/eui#7465)) ## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0) - Updated `EuiResizableButton` to allow customizing the `indicator` style with either `handle` (default) or `border` ([#7455](elastic/eui#7455)) - Enhanced `EuiResizableContainer` to preserve the drag/resize event when the user's mouse leaves the parent container and re-enters ([#7456](elastic/eui#7456)) **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([#7454](elastic/eui#7454)) **Accessibility** - `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to be more consistent for varying complex data: ([#7448](elastic/eui#7448)) - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress - Expandable cells continue to be toggled via Enter or F2 keypress - `EuiDataGrid` now provides a direct screen reader hint for Enter key behavior for expandable & interactive cells ([#7448](elastic/eui#7448))

`v92.0.0-backport.0`⏩ `v92.1.1` --- ## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1) **Bug fixes** - Minor `EuiDataGrid` cell performance fixes ([elastic#7465](elastic/eui#7465)) ## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0) - Updated `EuiResizableButton` to allow customizing the `indicator` style with either `handle` (default) or `border` ([elastic#7455](elastic/eui#7455)) - Enhanced `EuiResizableContainer` to preserve the drag/resize event when the user's mouse leaves the parent container and re-enters ([elastic#7456](elastic/eui#7456)) **Bug fixes** - Fixed an `EuiTreeView` JSX Typescript error ([elastic#7452](elastic/eui#7452)) - Fixed a color console warning being generated by disabled `EuiStep`s ([elastic#7454](elastic/eui#7454)) **Accessibility** - `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to be more consistent for varying complex data: ([elastic#7448](elastic/eui#7448)) - Headers are now always navigable by arrow key, regardless of whether the header cells contain interactive content - Non-expandable cells containing any amount of interactive content now must be entered via Enter or F2 keypress - Expandable cells continue to be toggled via Enter or F2 keypress - `EuiDataGrid` now provides a direct screen reader hint for Enter key behavior for expandable & interactive cells ([elastic#7448](elastic/eui#7448))

Toggle Kibana plugin red/green depending on whether user settings are…

9c302e2

… available If user settings are requested but are not available in the Kibana index, turn the Kibana plugin red. If they are requested and available, turn the plugin green again, if necessary.

ycombinator added review v5.0.0 v5.0.0-alpha4 labels Jun 14, 2016

ycombinator assigned bevacqua and LeeDr Jun 14, 2016

bevacqua removed their assignment Jun 14, 2016

Toggle Kibana to yellow instead of red

a81d759

Yellow is good in situations where the user doesn't need to take remedial actions to turn a plugin green. Red is good in situations where the user needs to do something to turn the plugin green.

ycombinator changed the title ~~Toggle Kibana plugin red/green depending on whether user settings are available~~ Toggle Kibana plugin yellow/green depending on whether user settings are available Jun 14, 2016

LeeDr reviewed Jun 14, 2016
View reviewed changes

ycombinator changed the title ~~Toggle Kibana plugin yellow/green depending on whether user settings are available~~ Set Elasticsearch plugin to yellow when user settings are not available Jun 14, 2016

spalger mentioned this pull request Jun 15, 2016

Pr/7454 ycombinator/kibana#1

Merged

Merge branch 'master' into elasticgh-7424

273c349

ycombinator force-pushed the gh-7424 branch from 3215dbc to 273c349 Compare June 15, 2016 05:06

spalger and others added 2 commits June 14, 2016 22:21

[ui/settings] go red when settings are not found, otherwise mimic es …

ced2a21

…status

Merge pull request #2 from spalger/pr/7454

5fe4a6b

[ui/settings] go red when settings are not found, otherwise mimic es …

ycombinator changed the title ~~Set Elasticsearch plugin to yellow when user settings are not available~~ Mirror ES status; set own status to red if user settings are not found Jun 15, 2016

ycombinator added 2 commits June 14, 2016 23:26

Ignore the extra log line about the settings status

7b61123

Improving comment

658d5f5

ycombinator assigned bevacqua and unassigned LeeDr Jun 15, 2016

bevacqua reviewed Jun 15, 2016
View reviewed changes

bevacqua assigned ycombinator and unassigned bevacqua Jun 15, 2016

Extracting conditional message into its own constant for better reada…

6222729

…bility

ycombinator merged commit 9714484 into elastic:master Jun 15, 2016

ycombinator deleted the gh-7424 branch June 15, 2016 16:28

epixa added v5.0.0 and removed v5.0.0 labels Jun 28, 2016

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Merge pull request elastic#7454 from ycombinator/elasticgh-7424

10dbce1

Mirror ES status; set own status to red if user settings are not found Former-commit-id: 9714484

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mirror ES status; set own status to red if user settings are not found #7454

Mirror ES status; set own status to red if user settings are not found #7454

ycombinator commented Jun 14, 2016 •

edited

Loading

bevacqua commented Jun 14, 2016

ycombinator commented Jun 14, 2016 •

edited

Loading

bevacqua commented Jun 14, 2016

ycombinator commented Jun 14, 2016

LeeDr Jun 14, 2016

bevacqua Jun 14, 2016 •

edited

Loading

ycombinator Jun 14, 2016

LeeDr commented Jun 14, 2016

ycombinator commented Jun 14, 2016

LeeDr commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

bevacqua commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

spalger commented Jun 15, 2016

bevacqua Jun 15, 2016

spalger Jun 15, 2016

bevacqua commented Jun 15, 2016

bevacqua commented Jun 15, 2016

ycombinator commented Jun 15, 2016

Mirror ES status; set own status to red if user settings are not found #7454

Mirror ES status; set own status to red if user settings are not found #7454

Conversation

ycombinator commented Jun 14, 2016 • edited Loading

bevacqua commented Jun 14, 2016

ycombinator commented Jun 14, 2016 • edited Loading

bevacqua commented Jun 14, 2016

ycombinator commented Jun 14, 2016

LeeDr Jun 14, 2016

Choose a reason for hiding this comment

bevacqua Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

ycombinator Jun 14, 2016

Choose a reason for hiding this comment

LeeDr commented Jun 14, 2016

ycombinator commented Jun 14, 2016

LeeDr commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

bevacqua commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

spalger commented Jun 15, 2016

ycombinator commented Jun 15, 2016

spalger commented Jun 15, 2016

bevacqua Jun 15, 2016

Choose a reason for hiding this comment

spalger Jun 15, 2016

Choose a reason for hiding this comment

bevacqua commented Jun 15, 2016

bevacqua commented Jun 15, 2016

ycombinator commented Jun 15, 2016

ycombinator commented Jun 14, 2016 •

edited

Loading

ycombinator commented Jun 14, 2016 •

edited

Loading

bevacqua Jun 14, 2016 •

edited

Loading