-
Notifications
You must be signed in to change notification settings - Fork 667
Add events and alerts to monitoring tab on overview page #4036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add events and alerts to monitoring tab on overview page #4036
Conversation
|
/kind feature |
serenamarie125
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change the "Events" accordion to say "All Events"
Please change the "Alerts" section to "Events (Warning)", and only show the warning level events
29c6b97 to
50dcd81
Compare
bc973e5 to
701037c
Compare
701037c to
f85bb20
Compare
@serenamarie125 I rename the accordion and keep metrics accordion expanded by default as per your requested changes. |
f85bb20 to
2fb8c94
Compare
2fb8c94 to
7ab90b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Better if renamed as MonitoringMetrics & moved to separate file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better if we move each AccordionItem into specific section components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already added separate component for events (Warning), Metrics, Events. I added AccordianItem here so that state can be maintained for the accordion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MonitoringMetricsSection has been renamed. Use MonitoringOverview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import appropriate module name & also rename the spec file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
34483cc to
89b96d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: odc-monitoring-event should odc-monitoring-events based on filename
89b96d8 to
af8a0e6
Compare
af8a0e6 to
15c2e1f
Compare
Thanks @invincibleJai Fixed this. |
|
/lgtm |
|
tested this locally and it works fine |
serenamarie125
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for doing the rework! This looks great
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, debsmita1, invincibleJai, serenamarie125, vikram-raj The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds 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. |


Stories - https://issues.redhat.com/browse/ODC-2596 and https://issues.redhat.com/browse/ODC-2600
This PR adds the Alerts and Events section under the monitoring tab of the overview panel.