Bug 1873192: show empty state in monitoring when no datapoint available #6463

nemesis09 · 2020-08-27T15:22:07Z

Fixes:
https://issues.redhat.com/browse/ODC-4469
https://issues.redhat.com/browse/ODC-3297

Analysis / Root cause:
the monitoring tab shows empty charts when there are no datapoints available
Solution Description:
show empty state when there are no datapoints

Screens:
Before

After

Test Coverage:
NA

Browser Conformance:

Firefox
Chrome
Safari
Edge

openshift-ci-robot · 2020-08-27T15:22:14Z

@nemesis09: This pull request references Bugzilla bug 1873192, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1873192: show empty state in monitoring when no datapoint available

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-08-27T15:23:57Z

@nemesis09: This pull request references Bugzilla bug 1873192, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1873192: show empty state in monitoring when no datapoint available

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nemesis09 · 2020-08-27T19:35:11Z

cc @beaumorley @parvathyvr

kyoto · 2020-08-28T01:17:06Z

frontend/public/components/monitoring/query-browser.tsx

@@ -673,11 +676,12 @@ const QueryBrowser_: React.FC<QueryBrowserProps> = ({
    setXDomain([from, to]);
    setSpan(to - from);
  };
+  const isGraphDataEmpty = _.isEmpty(_.flattenDeep(graphData));


I think we should avoid using _.flattenDeep because there can be a large amount of data in graphData and we already have performance concerns.

Maybe

const isGraphDataEmpty = _.isEmpty(graphData) || _.every(graphData, (d) => d.length === 0);

or

const isGraphDataEmpty = !_.some(graphData, (d) => d.length > 0);

we could use const isGraphDataEmpty = !graphData || graphData.every((d) => d.length === 0)
since every() always returns true for any condition when array is empty, we could drop the check _.isEmpty
updated the checks. please take a look.

👍 Thanks

kyoto · 2020-08-28T01:48:00Z

frontend/public/components/monitoring/query-browser.tsx

@@ -501,6 +501,9 @@ const QueryBrowser_: React.FC<QueryBrowserProps> = ({
  tickInterval,
  timespan,
 }) => {
+  // Min height for GraphEmpty(empty state)
+  const EMPTY_GRAPH_MIN_HEIGHT = 310;


Do we need this? I think the height should still be reasonable without it. We already have a state where an error message is displayed without a graph and I think the height is OK in that case without forcing a height.

Is there a page that looks bad without EMPTY_GRAPH_MIN_HEIGHT?

Also, I notice that 310 is too much for some places, like the Admin perspective > Monitoring > Metrics page.

Ideally, we wouldn't use EMPTY_GRAPH_MIN_HEIGHT and just fallback to CSS to give a reasonable height.

QueryBrowser already has a CSS class graph-empty-state which sets min height 310px for empty state
but the GraphEmpty defaults to 180px which made it seem floating towards the top.

Topology Sidebar

Dev perspective -> Monitoring

One problem with this is that depending on the page, the graph-empty-state wrapper may contain other content in addition to the empty state message (like the graph controls on the Metrics page), so the 310px actually makes the graph-empty-state taller than 310 pixels.

Not sure if there is an easy CSS fix here.

If we don't want to set the height then we can do it by using display: table CSS property. WDYT @kyoto?

^ @christianvogt What will be the better way to fix this?

Unfortunately this introduces a bug where the graph controls are now vertically centered while the graph is loading.

Looks like the styling needs to be applied more conditionally since it's only necessary when the empty state is shown

added conditional styling @kyoto

Unfortunately, I think this is introducing a different bug on the Metrics page. When the graph is loading, the graph wrapper loses its height, which I think is not what we want since it causes everything below the graph to jump up / down as the graph is loading.

I've pushed a fix addressing the css issue. Please take a look

nemesis09 · 2020-08-28T13:29:20Z

/retest

openshift-ci-robot · 2020-09-02T20:34:15Z

@nemesis09: This pull request references Bugzilla bug 1873192, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1873192: show empty state in monitoring when no datapoint available

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nemesis09 · 2020-09-14T10:44:27Z

/retest

kyoto · 2020-09-18T10:10:27Z

I tested and didn't see any layout issues with this now.

Thanks @nemesis09!

/lgtm

openshift-ci-robot · 2020-09-18T10:10:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyoto, nemesis09

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [kyoto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-09-18T12:10:55Z

@nemesis09: All pull requests linked via external trackers have merged:

openshift/console#6463

Bugzilla bug 1873192 has been moved to the MODIFIED state.

In response to this:

Bug 1873192: show empty state in monitoring when no datapoint available

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Aug 27, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 27, 2020

openshift-ci-robot requested review from kyoto and TheRealJon August 27, 2020 15:22

openshift-ci-robot added component/core Related to console core functionality component/monitoring Related to monitoring labels Aug 27, 2020

kyoto reviewed Aug 28, 2020

View reviewed changes

kyoto suggested changes Aug 28, 2020

View reviewed changes

nemesis09 force-pushed the fix-monitoring-empty-state branch from a6d69ef to 992e670 Compare August 28, 2020 12:00

nemesis09 force-pushed the fix-monitoring-empty-state branch 2 times, most recently from f176c8b to cc32b9b Compare September 13, 2020 20:15

show empty state in monitoring when no datapoint available

e4fa3f9

nemesis09 force-pushed the fix-monitoring-empty-state branch from cc32b9b to e4fa3f9 Compare September 15, 2020 22:06

openshift-ci-robot assigned kyoto Sep 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 18, 2020

openshift-merge-robot merged commit fecc422 into openshift:master Sep 18, 2020

spadgett added this to the v4.6 milestone Oct 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1873192: show empty state in monitoring when no datapoint available #6463

Bug 1873192: show empty state in monitoring when no datapoint available #6463

nemesis09 commented Aug 27, 2020 •

edited

openshift-ci-robot commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

nemesis09 commented Aug 27, 2020

kyoto Aug 28, 2020

kyoto Aug 28, 2020

nemesis09 Aug 28, 2020

kyoto Aug 30, 2020

kyoto Aug 28, 2020

nemesis09 Aug 28, 2020

kyoto Aug 31, 2020

vikram-raj Sep 2, 2020

vikram-raj Sep 9, 2020

kyoto Sep 10, 2020

christianvogt Sep 10, 2020

nemesis09 Sep 14, 2020 •

edited

kyoto Sep 15, 2020

nemesis09 Sep 16, 2020

nemesis09 commented Aug 28, 2020

openshift-ci-robot commented Sep 2, 2020

nemesis09 commented Sep 14, 2020

kyoto commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

Bug 1873192: show empty state in monitoring when no datapoint available #6463

Bug 1873192: show empty state in monitoring when no datapoint available #6463

Conversation

nemesis09 commented Aug 27, 2020 • edited

openshift-ci-robot commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

nemesis09 commented Aug 27, 2020

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

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

nemesis09 Sep 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemesis09 commented Aug 28, 2020

openshift-ci-robot commented Sep 2, 2020

nemesis09 commented Sep 14, 2020

kyoto commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

nemesis09 commented Aug 27, 2020 •

edited

nemesis09 Sep 14, 2020 •

edited