Node overview #4911
Node overview #4911
Conversation
openshift-ci-robot
added
do-not-merge/work-in-progress
openshift-ci-robot
added
component/core
|
@andybraren a few notes |
I feel like the ability to link to section headings within Details pages has come up before. I don't think we duplicate headings in any individual Details tabs, so using those to generate
I like that better, good call! |
rawagner
force-pushed
the
node-dashboard
branch
3 times, most recently
from
b9c4a38
to
742397b
Compare
|
@openshift/openshift-team-monitoring Still WIP, but this PR is adding a number of queries (see |
openshift-ci-robot
removed
the
needs-rebase
rawagner
force-pushed
the
node-dashboard
branch
5 times, most recently
from
24d6824
to
4928340
Compare
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
| `instance:node_filesystem_usage:sum{instance='<%= node %>'}`, | ||
| ), | ||
| [NodeQueries.FILESYSTEM_TOTAL]: _.template( | ||
| `node_filesystem_size_bytes{mountpoint="/", instance='<%= node %>'}`, |
There was a problem hiding this comment.
This is total filesystem size only for / mountpoint and not for all node filesystems. Is this intended?
There was a problem hiding this comment.
no, Im not sure why I added it there.. will be removed
|
Frankly I don't understand why we re-did this work when we already have the Grafana dashboards integrated into the console, and have a node details dashboard as part of that. A number of years have already gone into tuning details of the queries in those dashboards. |
|
@brancz I'll forward you a kinda long email thread from late last year on this topic when those integrated Monitoring Dashboards were being designed. TL;DR: "Overviews" like this one (this PR is mis-titled) are management-oriented jumping off points with functional and technical differences that distinguish them from the Prometheus-based Cluster/Node Dashboards within the Monitoring area of the Web Console. The Utilization card's queries do overlap, but the various other hyperlinks, statuses, messages, suggested actions, and operator integrations (like metal3) that appear in other cards and popovers address a different set of management-oriented use cases. FWIW the Console has included node metrics in Details pages for a while, and this new Overview de-emphasizes them somewhat. We'll continue to think about ways to better organize/integrate these views since we're not big fans of overlap either.
|
openshift-ci-robot
added
the
needs-rebase
rawagner
force-pushed
the
node-dashboard
branch
from
a815571
to
c55cbd9
Compare
openshift-ci-robot
removed
the
needs-rebase
|
The intend makes perfect sense to me, to me this is just about reusing queries that have been worked on and been tuned for several years as opposed to creating or maintaining the existence of other queries that are likely to repeat the same mistakes. If possible I'd like to see a future where we embed the dashboards that we recently integrated into console for this purpose. |
| import * as _ from 'lodash'; | ||
| import { QueryWithDescription } from '@console/shared/src/components/dashboard/utilization-card/UtilizationItem'; | ||
|
|
||
| export enum NodeQueries { |
There was a problem hiding this comment.
Since the Bare Metal Hosts use node exporter queries as well, we should eventually update Bare Metal Host dashboard to use these common queries defined here instead of redefining them specifically for BMHs. (see metal3-plugin/src/components/baremetal-hosts/dashboard/queries.ts).
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jtomasek, rawagner 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:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
resource limits/requests (threshold for error was adjusted)
This PR does not contain (will be a separate PR):