Skip to content
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

admin/alert-counts: add admin alert counts #2509

Merged
merged 34 commits into from
Aug 3, 2022
Merged

Conversation

KatieMSB
Copy link
Collaborator

@KatieMSB KatieMSB commented Jul 13, 2022

  • Identified the issue which this PR solves.
  • Read the CONTRIBUTING document.
  • Code builds clean without any errors or warnings.
  • Added appropriate tests for any new functionality.
  • All new and existing tests passed.
  • Added comments in the code, where necessary.
  • Ran make check to catch common errors. Fixed any that came up.

Description:
This pr adds new alert count page under the admin section to display all service alert counts over time. Total, max, and average values for alerts for all services are show in the data table. All services in the current page of the data table are shown in the graph.

Which issue(s) this PR fixes:
Closes #2475

Screenshots:
Screen Shot 2022-07-26 at 1 32 11 PM
Screen Shot 2022-07-26 at 1 32 24 PM

@KatieMSB KatieMSB requested a review from Forfold July 26, 2022 21:44
@mastercactapus
Copy link
Member

This has come along well, I like the colors, and it keeps the page rendering smoothly. When I tested it a couple of things I noticed:

  • Seems like there should be some sort of loading state when the data is stale/still loading
  • Can we have the X-axis labels account for the selected duration (e.g., daily show the day but not hour:minute)
  • Is it possible to limit labels with the graph we have selected (e.g., show tick marks but only label every x values)

The last two are probably nice to have to reduce noise

@Forfold
Copy link
Contributor

Forfold commented Jul 27, 2022

@mastercactapus I created the issue for the graphs loading here #2519 since ill need to solve that in the message logs graph pr as well

Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 🎉 🎉

@KatieMSB KatieMSB merged commit e2d378d into master Aug 3, 2022
@KatieMSB KatieMSB deleted the admin-alert-counts branch August 3, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

admin/alert-counts: Add alert count graph to admin panel
3 participants