[Stack monitoring] No-data page #110432

matschaffer · 2021-08-30T07:11:53Z

Summary

A no-data react page that uses the NoData react component.

There is still one caveat:

The "Set up monitoring with Metricbeat" button relies on what seems to be some "empty state handling", I opened up [Stack Monitoring][Angular removal] Elasticsearch views for no data case #112215 for this since we'll need a nodes page first

But I think there's still benefit to merging this to help provide a better "no data" dev experience.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

Fixes #112217

matschaffer · 2021-08-30T07:13:07Z

x-pack/plugins/monitoring/public/application/pages/no_data.tsx

+      <EuiFlexGroup gutterSize="l" justifyContent="spaceBetween" responsive>
+        <EuiFlexItem />
+        <EuiFlexItem>
+          <div>HERE GOES THE TIMEPICKER</div>


PageTemplate seemed like too much for this tiny page, so I just copied over this timepicker placeholder. We'll have to update it once that's sorted for the main pages.

I don't think it's a tiny page if you look at the controller 😅
But we can also add it later if it's needed. Since the angular page for no data has the MonitoringMain directive and the PageTemplate has some of the logic of that directive I would think that maybe there is something needed from it.

Yeah maybe "tiny" is overstating it. What I meant was that since it doesn't ever have a title header or tabs, PageTemplate seemed like it might be too much.

I'll revisit once we have setup mode and $http sorted, since it looks like NoData can't do much without those dependencies.

matschaffer · 2021-08-30T07:14:04Z

x-pack/plugins/monitoring/public/application/pages/no_data.tsx

+
+import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
+// @ts-ignore
+import { NoData as NoDataComponent } from '../../components/no_data';


Let me know your thoughts on this naming. We could potentially call this NoDataPage as an alternative, or rename the NoData component but I figured we should avoid trying to alter the angular UI.

I prefer calling it NoDataPage as it's how I would name it if I was implementing it from scratch. I don't think we alter the angular UI if we call it NoDataPage since in Angular this is not a component it's a route definition.

matschaffer · 2021-08-30T07:14:40Z

Posting as draft for now since I'm sure there's work to do here before merging. Just opening it early for feedback.

matschaffer · 2021-08-31T06:25:38Z

@elasticmachine update branch

…ation

estermv

I left a couple of comments but I didn't test the PR, so I'm not sure of close you are to a working solution (If it's already working you can ignore the comments!).

Basically, I think if we make the model part of the state and we and we update it from the Enabler class, child components should be updated.
Most of the weird things in the Angular controller seem to be to make Angular aware that the model changed inside React components and to re-render them when this happened.

estermv · 2021-09-08T09:48:02Z

x-pack/plugins/monitoring/public/application/pages/no_data_page.tsx

+  const model = {
+    errors: [], // errors can happen from trying to check or set ES settings
+    checkMessage: null, // message to show while waiting for api response
+    isLoading: isLoading, // flag for in-progress state of checking for no data reason
+    isCollectionEnabledUpdating: false, // flags to indicate whether to show a spinner while waiting for ajax
+    isCollectionEnabledUpdated: false,
+    isCollectionIntervalUpdating: false,
+    isCollectionIntervalUpdated: false,
+  };


I think this should be part of the state

Cool, thanks! I was thinking the same but not sure if it should be one state object or discrete state variables for each (like I did with isLoading).

estermv · 2021-09-08T09:51:12Z

x-pack/plugins/monitoring/public/application/pages/no_data_page.tsx

+  const { updateModel } = new ModelUpdater(model);
+  const enabler = new Enabler(updateModel);


We don't need the ModelUpdater class, if we make the model part of the state of the component, we could do something like this:

Suggested change

const { updateModel } = new ModelUpdater(model);

const enabler = new Enabler(updateModel);

const updateModel = (properties) => {

setModel({

...model,

...properties

})

}

const enabler = new Enabler(updateModel);

EDIT: setModel is not that simple... I didn't see that errors is an array

NIce, thanks! I'll try out model as a state object and see show that goes.

matschaffer · 2021-09-10T01:03:48Z

@elasticmachine merge upstream

matschaffer · 2021-09-15T05:54:05Z

@elasticmachine merge upstream

elasticmachine · 2021-09-15T05:54:17Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

…ation

phillipb

Seems to work!

phillipb · 2021-09-16T12:18:40Z

x-pack/plugins/monitoring/public/application/pages/no_data/no_data_page.tsx

+      // TODO not sure if we can add this conditional, but might be required for smooth internal collection enablement
+      // if (!this.isCollectionEnabledUpdating && !this.isCollectionIntervalUpdating) {


Were you able to figure out if this was necessary?

It's not really clear yet. I sometimes get a refresh of "we couldn't enable monitoring" that might be rooted in these conditionals missing. But I haven't been able to cleanly add them without leading to weird dependency cycles.

phillipb · 2021-09-16T12:20:29Z

x-pack/plugins/monitoring/public/application/pages/no_data/no_data_page.tsx

+  const [model, setModel] = useState({
+    errors: [], // errors can happen from trying to check or set ES settings
+    checkMessage: null, // message to show while waiting for api response
+    isLoading: true, // flag for in-progress state of checking for no data reason
+    isCollectionEnabledUpdating: false, // flags to indicate whether to show a spinner while waiting for ajax
+    isCollectionEnabledUpdated: false,
+    isCollectionIntervalUpdating: false,
+    isCollectionIntervalUpdated: false,
+  } as any);


Don't think you need to change it for this PR, but usually you would want these to all be separate variables. For example: const [isLoading, setIsLoading] = useState(true). You usually want to group things in objects if they are updated at the same time.

Yeah, I was wondering about that. Thanks for the cross check.

The no data flow in angular seemed to contain a lot of updateModel type code so I thought this approach might be better to keep the diff smaller even it's not great practice.

phillipb · 2021-09-16T12:21:55Z

x-pack/plugins/monitoring/public/application/pages/no_data/no_data_page.tsx

+
+const clusterCheckers: SettingsChecker[] = [
+  {
+    message: 'Checking cluster settings API on production cluster',


Need to i18n.translate this string

phillipb · 2021-09-16T12:22:16Z

x-pack/plugins/monitoring/public/application/pages/no_data/no_data_page.tsx

+    api: '../api/monitoring/v1/elasticsearch_settings/check/cluster',
+  },
+  {
+    message: 'Checking nodes settings API on production cluster',


Need to i18n.translate this string

matschaffer · 2021-09-21T06:21:19Z

@elasticmachine merge upstream

This replicates how the angular app did selective re-rendering of the react component, but while still being able to render the component.

matschaffer · 2021-09-21T14:14:19Z

Fixed this caveat in the latest push.

After enabling internal collection, there's a "we couldn't find your data" that appears until the next getPageData happens, it seems like we may want to address [Stack Monitoring] [Angular Removal] Avoid multiple requests simulataneously [Stack Monitoring] [Angular Removal] Avoid multiple requests simulataneously #111536 before fixing this

phillipb

LGTM!

kibanamachine · 2021-09-21T16:30:59Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`monitoring`	579	582	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`monitoring`	782.6KB	788.4KB	+5.8KB

History

💚 Build #154469 succeeded 917bf43
💔 Build #154467 failed 93f866c
💚 Build #153724 succeeded ad924d8
💚 Build #153432 succeeded 8a62652
💔 Build #153107 failed c371ed2

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

cc @matschaffer

* Basic no-data page * Rename to NoDataPage * Add getData property to pass into EuiSuperDatePicker from pages * Wire getData and redirect for no data page * Draft port of isLoading & model updating * Add todo on handling checkers * Switch to model as state object * Add checkers * Porting enabler * Fix build checks * Attempting to smooth out enablement * Clean up CI errors * Fix breadcrumbs * Fix linter warning * Fix checkers dependency (I hope) * Hook up catchReason * Add a stub for react setup mode * Clean warnings * Fix toggleSetupMode by calling initSetupModeState first * Translating checker strings * typo on "xpack" * Move isCollection/reason check in NoData This replicates how the angular app did selective re-rendering of the react component, but while still being able to render the component. Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-09-21T16:34:18Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* Basic no-data page * Rename to NoDataPage * Add getData property to pass into EuiSuperDatePicker from pages * Wire getData and redirect for no data page * Draft port of isLoading & model updating * Add todo on handling checkers * Switch to model as state object * Add checkers * Porting enabler * Fix build checks * Attempting to smooth out enablement * Clean up CI errors * Fix breadcrumbs * Fix linter warning * Fix checkers dependency (I hope) * Hook up catchReason * Add a stub for react setup mode * Clean warnings * Fix toggleSetupMode by calling initSetupModeState first * Translating checker strings * typo on "xpack" * Move isCollection/reason check in NoData This replicates how the angular app did selective re-rendering of the react component, but while still being able to render the component. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Mat Schaffer <[email protected]>

Basic no-data page

3436b35

matschaffer self-assigned this Aug 30, 2021

matschaffer requested review from estermv and phillipb August 30, 2021 07:12

matschaffer commented Aug 30, 2021

View reviewed changes

Rename to NoDataPage

9d8b938

kibanamachine and others added 5 commits August 31, 2021 02:25

Merge branch 'master' into no-data-page-migration

59f04bd

Merge remote-tracking branch 'upstream/master' into no-data-page-migr…

0d1963a

…ation

Add getData property to pass into EuiSuperDatePicker from pages

fe56aaf

Wire getData and redirect for no data page

bcd68ad

Merge remote-tracking branch 'upstream/master' into no-data-page-migr…

a63c210

…ation

matschaffer mentioned this pull request Sep 7, 2021

[Stack monitoring] Convert Angular views to React #111309

Closed

19 tasks

matschaffer added 3 commits September 8, 2021 12:07

Merge remote-tracking branch 'upstream/master' into no-data-page-migr…

b13375c

…ation

Draft port of isLoading & model updating

4cf97bf

Add todo on handling checkers

14cfc03

estermv reviewed Sep 8, 2021

View reviewed changes

kibanamachine and others added 4 commits September 9, 2021 21:03

Merge branch 'master' into no-data-page-migration

aa5a9ba

Switch to model as state object

01938a9

Add checkers

cfac638

Porting enabler

bac9264

matschaffer added 3 commits September 14, 2021 06:56

Add a stub for react setup mode

c8f01b7

Clean warnings

c371ed2

Fix toggleSetupMode by calling initSetupModeState first

6b7908c

matschaffer changed the title ~~[Stack monitoring] Basic no-data page~~ [Stack monitoring] No-data page Sep 15, 2021

Merge branch 'master' into no-data-page-migration

8a62652

matschaffer marked this pull request as ready for review September 15, 2021 05:54

matschaffer requested a review from a team as a code owner September 15, 2021 05:54

matschaffer requested review from a team September 15, 2021 05:54

matschaffer added the release_note:skip Skip the PR/issue when compiling release notes label Sep 15, 2021

matschaffer mentioned this pull request Sep 16, 2021

[es] disable wildcards in destructive actions #88986

Merged

Merge remote-tracking branch 'upstream/master' into no-data-page-migr…

ad924d8

…ation

phillipb suggested changes Sep 16, 2021

View reviewed changes

kibanamachine and others added 3 commits September 21, 2021 02:21

Merge branch 'master' into no-data-page-migration

ee89f73

Translating checker strings

93f866c

typo on "xpack"

917bf43

matschaffer requested a review from phillipb September 21, 2021 07:25

Move isCollection/reason check in NoData

3987326

This replicates how the angular app did selective re-rendering of the react component, but while still being able to render the component.

phillipb approved these changes Sep 21, 2021

View reviewed changes

matschaffer enabled auto-merge (squash) September 21, 2021 15:31

matschaffer merged commit d37ad90 into elastic:master Sep 21, 2021

kibanamachine mentioned this pull request Sep 21, 2021

[7.x] [Stack Monitoring][Angular removal] No-data page (#110432) #112692

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Stack monitoring] No-data page #110432

[Stack monitoring] No-data page #110432

matschaffer commented Aug 30, 2021 •

edited

Loading

matschaffer Aug 30, 2021

estermv Aug 30, 2021

matschaffer Aug 31, 2021

matschaffer Aug 30, 2021

estermv Aug 30, 2021

matschaffer commented Aug 30, 2021 •

edited

Loading

matschaffer commented Aug 31, 2021

estermv left a comment

estermv Sep 8, 2021

matschaffer Sep 9, 2021

estermv Sep 8, 2021 •

edited

Loading

matschaffer Sep 9, 2021

matschaffer commented Sep 10, 2021

matschaffer commented Sep 15, 2021

elasticmachine commented Sep 15, 2021

phillipb left a comment

phillipb Sep 16, 2021

matschaffer Sep 21, 2021

phillipb Sep 16, 2021

matschaffer Sep 21, 2021

phillipb Sep 16, 2021

phillipb Sep 16, 2021

matschaffer commented Sep 21, 2021

matschaffer commented Sep 21, 2021

phillipb left a comment

kibanamachine commented Sep 21, 2021

kibanamachine commented Sep 21, 2021

		const { updateModel } = new ModelUpdater(model);
		const enabler = new Enabler(updateModel);

-  const { updateModel } = new ModelUpdater(model);
-  const enabler = new Enabler(updateModel);
+  const updateModel = (properties) => {
+     setModel({
+        ...model,
+        ...properties
+     })
+  }
+  const enabler = new Enabler(updateModel);

		// TODO not sure if we can add this conditional, but might be required for smooth internal collection enablement
		// if (!this.isCollectionEnabledUpdating && !this.isCollectionIntervalUpdating) {

[Stack monitoring] No-data page #110432

[Stack monitoring] No-data page #110432

Conversation

matschaffer commented Aug 30, 2021 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matschaffer commented Aug 30, 2021 • edited Loading

matschaffer commented Aug 31, 2021

estermv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estermv Sep 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matschaffer commented Sep 10, 2021

matschaffer commented Sep 15, 2021

elasticmachine commented Sep 15, 2021

phillipb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matschaffer commented Sep 21, 2021

matschaffer commented Sep 21, 2021

phillipb left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 21, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Sep 21, 2021

💚 Backport successful

matschaffer commented Aug 30, 2021 •

edited

Loading

matschaffer commented Aug 30, 2021 •

edited

Loading

estermv Sep 8, 2021 •

edited

Loading