Monitoring dashboards: Initial commit #3771

kyoto · 2019-12-12T11:16:51Z

Initial commit of the monitoring dashboards. Including laying out the
dashboard cards dynamically and rendering single stat, bar graph and
line graph cards.

Follow-up PRs will add

Table cards
Stacked line graphs
Chart legends
Dynamic loading for the list of available dashboards
Proper handling of dashboard variables
Apply "Time Range" and "Refresh Interval" options to all cards
Lots of styling

spadgett · 2019-12-12T14:07:50Z

/test analyze

spadgett · 2019-12-12T18:49:35Z

@cshinn @andybraren Will it be confusing for users that there are two different sets of dashboards, Home -> Dashboards and Monitoring -> Dashboards? Have we considered showing these under the existing Home -> Dashboards page?

spadgett · 2019-12-12T18:49:40Z

cc @alimobrem

cshinn · 2019-12-12T18:53:31Z

@cshinn @andybraren Will it be confusing for users that there are two different sets of dashboards, Home -> Dashboards and Monitoring -> Dashboards? Have we considered showing these under the existing Home -> Dashboards page?

I've heard tell of changing the current cluster dashboard to "Overview" or somesuch. This monitoring section could also be called "metrics dashboards". Either way, we've had this situation for a couple releases now, It's just where the link goes that's different

benjaminapetersen · 2019-12-12T19:23:50Z

frontend/public/components/monitoring/dashboards/index.tsx

+  const [error, setError] = React.useState();
+
+  React.useEffect(() => {
+    const path = `${k8sBasePath}/api/v1/namespaces/openshift-monitoring/configmaps/grafana-dashboard-${board}`;


Ok great, can take a look at the configmaps from here.

This is where we need to have the discussion about ConfigMap vs CRD and the RBAC ramifications, as users may not be able to access these resources.

andybraren · 2019-12-12T19:23:59Z

@spadgett I do think it'll be confusing.

We discussed this briefly back in October, but I think it's time to start calling what we know as "dashboards" today "overviews" instead (since that's what really what they are, philosophically) and free up that word to be used by these more traditional Metric Dashboards instead.

To get there, we would:

Rename the "Dashboards" nav item "Overview" with a page title of "Overview"
- Rename the first "Overview" tab to "Cluster" instead
Rename the "Overview" tabs of every resource page "Details" instead
- (Including the slideovers that appear within the Workloads tab of a Project page)
Add a "View all" link to the existing small Details card of the Project/Node/Host/VM overviews that would take the user to the full Details tab

I think this is worth POC'ing to see how it feels and see if we come across any major conflicts, but calling the existing things "overviews" seems better aligned with their purpose and our (prescriptive) extensibility plans for them, compared to these new traditional "dashboards".

FYI @rawagner

spadgett

Great to see this progress 👍

spadgett · 2019-12-12T17:13:43Z

frontend/public/components/nav/admin-nav.tsx

        <HrefLink
          href="/monitoring/query-browser?query0="
          name="Metrics"
          startsWith={['monitoring/query-browser']}
        />
      )}
-      {showGrafana && <ExternalLink href={grafanaURL} name="Dashboards" />}
+      {canAccessPrometheus && <HrefLink href="/monitoring/dashboards" name="Dashboards" />}


Let's hide the nav item until we're ready to expose the dashboards

OK, I reverted the changes in admin-nav.tsx for now

spadgett · 2019-12-12T19:04:05Z

frontend/public/components/monitoring/dashboards/error.tsx

+
+import { Alert } from '@patternfly/react-core';
+
+const ErrorAlert: React.FC<Props> = ({ message, title = 'An error occurred' }) => (


I'd move this to shared components so it's a general utility we can use elsewhere.

👍 Moved to frontend/packages/console-shared/src/components/alerts/error.tsx

frontend/public/components/monitoring/dashboards/error.tsx

spadgett · 2019-12-12T19:07:47Z

frontend/public/components/monitoring/dashboards/index.tsx

+  let result = s;
+  _.each(variables, (v, k) => {
+    result = result.replace(new RegExp(`\\$${k}`, 'g'), v);
+  });
+  return result;


I'd suggest reduce

return _.reduce(variables, (result: string, v: string, k: string): string => { return result.replace(new RegExp(`\\$${k}`, 'g'), v); }, s);

spadgett · 2019-12-12T19:12:08Z

frontend/public/components/monitoring/_monitoring.scss

+  justify-content: flex-start;
+}
+
+.monitoring-dashboards-dropdown {


We should follow BEM naming for these classes

frontend/public/components/monitoring/dashboards/index.tsx

frontend/public/components/monitoring/dashboards/single-stat.tsx

spadgett · 2019-12-12T19:20:59Z

frontend/public/components/monitoring/dashboards/graph.tsx

+import { QueryBrowser } from '../query-browser';
+
+const Graph: React.FC<Props> = ({ pollInterval, queries, timespan }) => (
+  <QueryBrowser


Happy to see we can reuse this 👍

spadgett · 2019-12-12T19:46:21Z

@andybraren I guess I don't understand why Home -> Dashboards is an overview and not a dashboard. It looks like a dashboard to me :)

The way these two pages work under the hood is very different. That doesn't really matter to a user, though. They seem to fulfill the same fundamental purpose, and they look almost the same. Why not just add these as additional dashboards under the existing Home -> Dashboards page?

cshinn · 2019-12-12T21:05:47Z

@spadgett something similar was proposed back before the current dashboard existed. I think it's now made tricky because lots of different "metrics dashboards" can exist as well as several different "overview dashboards" for different subsystems. @sichvoge what are your thoughts on the relationship between these?

andybraren · 2019-12-12T22:53:49Z

@spadgett Our reasoning is somewhat forward-looking, but IIRC @cshinn and I lean against placing these within Home -> Dashboards because:

Everything in these dashboards is powered by Prometheus data (conceptually, Monitoring)
Eventually, users might be able to add queries from within the Metrics page to new/existing Metric dashboards via the CR mechanism, or go the opposite direction and edit a query from a Metric dashboard within the Metrics page. Keeping them next to each other makes that connection more natural.
When that day comes, it'll be difficult to explain why some "dashboards" are create-able & customizable by users and operators while others on the same page aren't.

@andybraren I guess I don't understand why Home -> Dashboards is an overview and not a dashboard. It looks like a dashboard to me :) .... They seem to fulfill the same fundamental purpose, and they look almost the same.

Maybe I've just been haunted by "dashboards" for so long that I feel the need to rename them. 😄

Although they do look visually similar-ish, I'd argue that they fulfill fundamentally different purposes and will only continue to functionally diverge in the future due to the architectural differences between them. Both are valid and useful, but as the differences grow, calling them the same thing may confuse more than it helps.

Overviews should continue to be prescriptive "windows" that highlight what's important 90% of the time (determined by our research & domain expertise) and help users quickly jump away to other areas of the Console that need human attention. The 5-6 cards we have today seem to do a decent job at that, and their deep integration into the Console and the API allows them to do things these new dashboards can't.

Dashboards have a clear path forward for deep customization, and their insight is scoped to what Prometheus collects (which is a lot). They have a different but similarly important focus, and they'll allow users to get deeper data-driven insights when necessary.

People will probably still call both of them "dashboards" anyway 🤷‍♂️ but it seems like our UI should at least attempt to make the functional & philosophical distinction clearer by not calling them the same thing and placing them next to one another.

FYI @sichvoge @jelkosz @rawagner @yuvalgalanti @serenamarie125 @beanh66 @lizsurette (related to an email discussion from October)

Initial commit of the monitoring dashboards. Including laying out the dashboard cards dynamically and rendering single stat, bar graph and line graph cards.

kyoto · 2019-12-13T06:13:58Z

Addressed feedback and also made a couple of other small improvements.

Pass panel data to <Table /> so that all the panel fields are available to it
Fix React warning caused by the Dropdown's onChange passing a second parameter that was being passed through to setBoard
Create types.ts for shared types

kyoto · 2019-12-13T06:44:32Z

/test analyze

spadgett

/lgtm

Thanks @kyoto. Let's get the initial work integrated so our teams can better collaborate, then we can expose the dashboards in the nav when ready. This is a fantastic start 👍

openshift-ci-robot · 2019-12-13T14:01:30Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,spadgett]

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

spadgett · 2019-12-13T14:26:26Z

@andybraren The way it's architected, the out-of-the-box dashboards won't be customizable. They're managed by operators, which will overwrite any changes you make. You would only be able to create and edit the dashboards you add. So there shouldn't be any confusion there.

It's not clear when or if we'd add that capability. Most likely it'd be driven by a cluster admin editing a custom resource and not through an editor in the UI.

I think we're still getting hung up on the implementation, and not what the user sees. Fundamentally, these pages are about the same thing: monitoring the state of your cluster. The look and behave basically the same regardless of what we call them.

benjaminapetersen · 2019-12-13T15:22:23Z

I'll chime in along the same lines of @spadgett, Dashboard makes sense. I've honestly never liked the term Overview. Is it a summary page, or is it details? The term isn't clear, I don't think it really communicates not-dashboard.

For an administrator, these views seem to be essential to understanding how the cluster is doing. I'd be in favor of moving this up to Home -> Dashboards as well.

openshift-ci-robot requested review from TheRealJon and zherman0 December 12, 2019 11:17

benjaminapetersen reviewed Dec 12, 2019

View reviewed changes

spadgett reviewed Dec 12, 2019

View reviewed changes

Monitoring dashboards: Initial commit

9bc68f0

Initial commit of the monitoring dashboards. Including laying out the dashboard cards dynamically and rendering single stat, bar graph and line graph cards.

kyoto force-pushed the monitoring-dashboards branch from bb281b3 to 9bc68f0 Compare December 13, 2019 06:05

openshift-ci-robot added the component/shared Related to console-shared label Dec 13, 2019

spadgett approved these changes Dec 13, 2019

View reviewed changes

openshift-ci-robot assigned spadgett Dec 13, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2019

openshift-merge-robot merged commit dbd4684 into openshift:master Dec 13, 2019

kyoto deleted the monitoring-dashboards branch December 16, 2019 00:52

spadgett added this to the v4.4 milestone Dec 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monitoring dashboards: Initial commit #3771

Monitoring dashboards: Initial commit #3771

kyoto commented Dec 12, 2019

spadgett commented Dec 12, 2019

spadgett commented Dec 12, 2019

spadgett commented Dec 12, 2019

cshinn commented Dec 12, 2019

benjaminapetersen Dec 12, 2019

andybraren commented Dec 12, 2019 •

edited

spadgett left a comment

spadgett Dec 12, 2019 •

edited

kyoto Dec 13, 2019

spadgett Dec 12, 2019

kyoto Dec 13, 2019

spadgett Dec 12, 2019

kyoto Dec 13, 2019

spadgett Dec 12, 2019

kyoto Dec 13, 2019

spadgett Dec 12, 2019

spadgett commented Dec 12, 2019

cshinn commented Dec 12, 2019 •

edited

andybraren commented Dec 12, 2019

kyoto commented Dec 13, 2019

kyoto commented Dec 13, 2019

spadgett left a comment

openshift-ci-robot commented Dec 13, 2019

spadgett commented Dec 13, 2019

benjaminapetersen commented Dec 13, 2019


		import { Alert } from '@patternfly/react-core';

		const ErrorAlert: React.FC<Props> = ({ message, title = 'An error occurred' }) => (

Monitoring dashboards: Initial commit #3771

Monitoring dashboards: Initial commit #3771

Conversation

kyoto commented Dec 12, 2019

spadgett commented Dec 12, 2019

spadgett commented Dec 12, 2019

spadgett commented Dec 12, 2019

cshinn commented Dec 12, 2019

Choose a reason for hiding this comment

andybraren commented Dec 12, 2019 • edited

spadgett left a comment

Choose a reason for hiding this comment

spadgett Dec 12, 2019 • edited

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

spadgett commented Dec 12, 2019

cshinn commented Dec 12, 2019 • edited

andybraren commented Dec 12, 2019

kyoto commented Dec 13, 2019

kyoto commented Dec 13, 2019

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Dec 13, 2019

spadgett commented Dec 13, 2019

benjaminapetersen commented Dec 13, 2019

andybraren commented Dec 12, 2019 •

edited

spadgett Dec 12, 2019 •

edited

cshinn commented Dec 12, 2019 •

edited