-
Notifications
You must be signed in to change notification settings - Fork 806
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
fix: Add alerts_count, affected_services and sources fields to the Incident #1473
fix: Add alerts_count, affected_services and sources fields to the Incident #1473
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
84725df
to
8e52fa4
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.
I have a few comments, mostly around dialect specific SQL functions that could be problematic in the future (or even now).
Maybe the ideal case here is to have some worker doing those calcs in the background? just thinking out loud, idk.
fc36449
to
5d08733
Compare
…t and pre-population logic for them on adding alerts to the incident
…ricky, but optimized way to remove such info on alert removing
…ests for add/remove alerts to the incident
5d08733
to
2388a89
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1473 +/- ##
===========================================
- Coverage 68.38% 37.36% -31.03%
===========================================
Files 112 28 -84
Lines 9288 2671 -6617
===========================================
- Hits 6352 998 -5354
+ Misses 2936 1673 -1263 ☔ View full report in Codecov by Sentry. |
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.
LGTM!
…with-large-amount-of-alerts
…with-large-amount-of-alerts
Add alerts_count, affected_services and sources fields to the Incident and pre-population logic for them on adding alerts to the incident
Closes #1472
📑 Description
The incidents list page performed poorly when there were a large number of alerts in the incidents. That happened because alerts count, affected services and sources were calculated in runtime.
This PR changes this behavior to pre-calculated data for incidents
✅ Checks
ℹ Additional Information