mgr/dashboard: Improve monitoring tabs content #46811

aaSharma14 · 2022-06-22T11:51:05Z

This PR intends to fix the column spacing in the alert list in the alerts tab

Fixes:https://tracker.ceph.com/issues/56165
Signed-off-by: Aashish Sharma aasharma@redhat.com

Active Alerts -

Alerts -

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

epuertat · 2022-06-22T11:57:21Z

Some ideas:

For columns whose value is a predefined set (enum-like) we should use labels/badges. That applies to Severity (critical, warning, etc.) for which we should definitely use the prescribed colors (danger, warning, info, etc). Same for Group, but in that case we can simply use the primary color for all of them (until the auto-coloring label from @Pegonzal arrives).
For numeric columns I think those should be right-aligned (but not 100% sure, given that works great when the values are purely numeric, without units, which is not the case here: as we display seconds/minutes/etc.).
Regarding the columns width we can be even more kind to the Description field (flexGrow 4 versus 2-1-1, or even 8-2-1-1)?

aaSharma14 · 2022-06-24T06:48:19Z

Some ideas:

For columns whose value is a predefined set (enum-like) we should use labels/badges. That applies to Severity (critical, warning, etc.) for which we should definitely use the prescribed colors (danger, warning, info, etc). Same for Group, but in that case we can simply use the primary color for all of them (until the auto-coloring label from @Pegonzal arrives).

For numeric columns I think those should be right-aligned (but not 100% sure, given that works great when the values are purely numeric, without units, which is not the case here: as we display seconds/minutes/etc.).

Regarding the columns width we can be even more kind to the Description field (flexGrow 4 versus 2-1-1, or even 8-2-1-1)?

Thanks @epuertat for the suggestions. Updated the PR accordingly. PTAL.

epuertat

Looks much better now! Just some comments about the implementation and another suggestion (although feel free to drop it for a follow-up PR).

.../mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/rules-list/rules-list.component.html

...nd/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/rules-list/rules-list.component.ts

aaSharma14 · 2022-06-27T08:39:48Z

@epuertat , I have collaborated the changes of your PR #46836, in this PR itself so we don't have to track two different PRs.

aaSharma14 · 2022-06-27T09:10:02Z

jenkins test dashboard cephadm

aaSharma14 · 2022-06-27T09:10:11Z

jenkins test api

pereman2 · 2022-06-27T10:50:04Z

...rd/frontend/src/app/ceph/cluster/prometheus/active-alert-list/active-alert-list.component.ts

+        prop: 'labels.severity',
+        flexGrow: 1,
+        cellTransformation: CellTemplate.badge,
+        customTemplateConfig: {
+          map: {
+            critical: { class: 'badge-danger' },
+            warning: { class: 'badge-warning' }


Would it make sense to sort by severity as default? I feel people would prefer seeing the critical first then warnings.

aaSharma14 · 2022-06-28T06:35:46Z

jenkins test dashboard cephadm

aaSharma14 · 2022-06-29T11:02:08Z

jenkins test api

aaSharma14 · 2022-06-29T11:02:19Z

jenkins test dashboard cephadm

epuertat · 2022-06-29T13:12:35Z

Discussed in today's daily meeting:

Add badges to the tabs: instead of getting the information in the parent/tab component, trying to send the alerts from the child component to the parent one ~~[1]~~, ~~[2]~~, [parent-child communication using service].
Add badges to the menu item: with a similar approach.

aaSharma14 · 2022-06-30T10:58:05Z

Discussed in today's daily meeting:

Add badges to the tabs: instead of getting the information in the parent/tab component, trying to send the alerts from the child component to the parent one ~~[1]~~, ~~[2]~~, [parent-child communication using service].

Add badges to the menu item: with a similar approach.

Thankyou for the suggestion @epuertat , Updated the PR accordingly.

epuertat

LGTM @aaSharma14 ! Thanks for addressing my suggestions.

Once the following issues are fixed I'm ok with it:

Padding between badges:
And while switching from tab Active Alerts to Alerts I got the following dump:

polyfills.js:214 Uncaught TypeError: ctx_r5.updateSelection is not a function
    at RulesListComponent_cd_table_2_Template_cd_table_updateSelection_0_listener (main.js:11918:634)
    at executeListenerWithErrorHandling (vendor.js:29760:16)
    at Object.wrapListenerIn_markDirtyAndPreventDefault [as next] (vendor.js:29798:22)
    at SafeSubscriber.__tryOrUnsub (vendor.js:155859:16)
    at SafeSubscriber.next (vendor.js:155798:22)
    at Subscriber._next (vendor.js:155748:26)
    at Subscriber.next (vendor.js:155725:18)
    at EventEmitter_.next (vendor.js:155509:25)
    at EventEmitter_.emit (vendor.js:40388:15)
    at TableComponent.ngOnInit (main.js:36088:30)

pereman2

177 - mgr-dashboard-frontend-unittests (Failed)
I still think sorting by critical could be a nice touch. nevertheless, looks good.

...oard/frontend/src/app/ceph/cluster/prometheus/prometheus-tabs/prometheus-tabs.component.html

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.html

This PR intends to fix the column spacing in the alert list in the alerts tab Fixes:https://tracker.ceph.com/issues/56165 Signed-off-by: Aashish Sharma <aasharma@redhat.com>

aaSharma14 · 2022-07-01T05:59:38Z

LGTM @aaSharma14 ! Thanks for addressing my suggestions.

Once the following issues are fixed I'm ok with it:

Padding between badges:
And while switching from tab Active Alerts to Alerts I got the following dump:

polyfills.js:214 Uncaught TypeError: ctx_r5.updateSelection is not a function
    at RulesListComponent_cd_table_2_Template_cd_table_updateSelection_0_listener (main.js:11918:634)
    at executeListenerWithErrorHandling (vendor.js:29760:16)
    at Object.wrapListenerIn_markDirtyAndPreventDefault [as next] (vendor.js:29798:22)
    at SafeSubscriber.__tryOrUnsub (vendor.js:155859:16)
    at SafeSubscriber.next (vendor.js:155798:22)
    at Subscriber._next (vendor.js:155748:26)
    at Subscriber.next (vendor.js:155725:18)
    at EventEmitter_.next (vendor.js:155509:25)
    at EventEmitter_.emit (vendor.js:40388:15)
    at TableComponent.ngOnInit (main.js:36088:30)

Done

aaSharma14 · 2022-07-04T07:40:52Z

jenkins test make check

aaSharma14 · 2022-07-04T07:41:05Z

jenkins test dashboard cephadm

avanthakkar · 2022-07-06T09:46:58Z

jenkins test dashboard cephadm

aaSharma14 · 2022-07-06T10:00:27Z

jenkins test dashboard cephadm

nizamial09 · 2022-07-06T11:03:53Z

cephadm e2e failures are tracked here : https://tracker.ceph.com/issues/55609

aaSharma14 added cleanup dashboard labels Jun 22, 2022

aaSharma14 requested a review from a team as a code owner June 22, 2022 11:51

aaSharma14 added this to In progress in Dashboard via automation Jun 22, 2022

aaSharma14 requested review from bryanmontalvan and nizamial09 and removed request for a team June 22, 2022 11:51

github-actions bot added the pybind label Jun 22, 2022

aaSharma14 requested review from pereman2 and avanthakkar June 22, 2022 11:51

aaSharma14 force-pushed the fix-56165-main branch from ee740fb to b15935c Compare June 24, 2022 06:46

aaSharma14 force-pushed the fix-56165-main branch from b15935c to 13f7e28 Compare June 24, 2022 09:13

epuertat mentioned this pull request Jun 24, 2022

mgr/dashboard: improve active alerts table #46836

Closed

14 tasks

epuertat reviewed Jun 24, 2022

View reviewed changes

.../mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/rules-list/rules-list.component.html Outdated Show resolved Hide resolved

...nd/mgr/dashboard/frontend/src/app/ceph/cluster/prometheus/rules-list/rules-list.component.ts Outdated Show resolved Hide resolved

aaSharma14 force-pushed the fix-56165-main branch from 13f7e28 to e122838 Compare June 27, 2022 08:34

aaSharma14 requested a review from epuertat June 27, 2022 08:38

aaSharma14 changed the title ~~mgr/dashboard: Improve column spacing in alerts tab~~ mgr/dashboard: Improve monitoring tabs content Jun 27, 2022

aaSharma14 force-pushed the fix-56165-main branch from e122838 to 6888bd8 Compare June 27, 2022 10:44

pereman2 reviewed Jun 27, 2022

View reviewed changes

aaSharma14 force-pushed the fix-56165-main branch from 6888bd8 to cee1ad8 Compare June 30, 2022 10:56

aaSharma14 force-pushed the fix-56165-main branch from cee1ad8 to a12702c Compare June 30, 2022 13:29

epuertat approved these changes Jun 30, 2022

View reviewed changes

Dashboard automation moved this from In progress to Reviewer approved Jun 30, 2022

pereman2 approved these changes Jun 30, 2022

View reviewed changes

nizamial09 reviewed Jun 30, 2022

View reviewed changes

...oard/frontend/src/app/ceph/cluster/prometheus/prometheus-tabs/prometheus-tabs.component.html Outdated Show resolved Hide resolved

src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.html Outdated Show resolved Hide resolved

mgr/dashboard: Improve column spacing in alerts tab

d60ca04

This PR intends to fix the column spacing in the alert list in the alerts tab Fixes:https://tracker.ceph.com/issues/56165 Signed-off-by: Aashish Sharma <aasharma@redhat.com>

aaSharma14 force-pushed the fix-56165-main branch from a12702c to d60ca04 Compare July 1, 2022 05:57

aaSharma14 assigned avanthakkar Jul 6, 2022

avanthakkar approved these changes Jul 6, 2022

View reviewed changes

nizamial09 merged commit d05b5e2 into ceph:main Jul 6, 2022

Dashboard automation moved this from Reviewer approved to Done Jul 6, 2022

nizamial09 deleted the fix-56165-main branch July 6, 2022 11:04

aaSharma14 mentioned this pull request Jul 6, 2022

quincy: mgr/dashboard: Improve monitoring tabs content #46990

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/dashboard: Improve monitoring tabs content #46811

mgr/dashboard: Improve monitoring tabs content #46811

aaSharma14 commented Jun 22, 2022 •

edited

epuertat commented Jun 22, 2022 •

edited

aaSharma14 commented Jun 24, 2022

epuertat left a comment

aaSharma14 commented Jun 27, 2022 •

edited

aaSharma14 commented Jun 27, 2022

aaSharma14 commented Jun 27, 2022

pereman2 Jun 27, 2022

aaSharma14 commented Jun 28, 2022

aaSharma14 commented Jun 29, 2022

aaSharma14 commented Jun 29, 2022

epuertat commented Jun 29, 2022 •

edited

aaSharma14 commented Jun 30, 2022

epuertat left a comment

pereman2 left a comment

aaSharma14 commented Jul 1, 2022

aaSharma14 commented Jul 4, 2022

aaSharma14 commented Jul 4, 2022

avanthakkar commented Jul 6, 2022

aaSharma14 commented Jul 6, 2022

nizamial09 commented Jul 6, 2022

mgr/dashboard: Improve monitoring tabs content #46811

mgr/dashboard: Improve monitoring tabs content #46811

Conversation

aaSharma14 commented Jun 22, 2022 • edited

Contribution Guidelines

Checklist

epuertat commented Jun 22, 2022 • edited

aaSharma14 commented Jun 24, 2022

epuertat left a comment

Choose a reason for hiding this comment

aaSharma14 commented Jun 27, 2022 • edited

aaSharma14 commented Jun 27, 2022

aaSharma14 commented Jun 27, 2022

pereman2 Jun 27, 2022

Choose a reason for hiding this comment

aaSharma14 commented Jun 28, 2022

aaSharma14 commented Jun 29, 2022

aaSharma14 commented Jun 29, 2022

epuertat commented Jun 29, 2022 • edited

aaSharma14 commented Jun 30, 2022

epuertat left a comment

Choose a reason for hiding this comment

pereman2 left a comment

Choose a reason for hiding this comment

aaSharma14 commented Jul 1, 2022

aaSharma14 commented Jul 4, 2022

aaSharma14 commented Jul 4, 2022

avanthakkar commented Jul 6, 2022

aaSharma14 commented Jul 6, 2022

nizamial09 commented Jul 6, 2022

aaSharma14 commented Jun 22, 2022 •

edited

epuertat commented Jun 22, 2022 •

edited

aaSharma14 commented Jun 27, 2022 •

edited

epuertat commented Jun 29, 2022 •

edited