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

Alerting: Display correct results when using different filters on alerting panels #70482

Merged
merged 7 commits into from Jun 23, 2023

Conversation

VikaCep
Copy link
Contributor

@VikaCep VikaCep commented Jun 21, 2023

What is this feature?

Fixes an issue where showing several alerting panels within a dashboard don't respect each panel's alert state filter settings.

Why do we need this feature?

To be able to respect alert state filters within a dashboard with several alerting panels.

Who is this feature for?

All users

Which issue(s) does this PR fix?:

Fixes https://github.com/grafana/support-escalations/issues/6423

Special notes for your reviewer:

When loading several alert list panels where each panel uses a different state filter, the request called to obtain the rules information remains the same. It doesn't trigger a new request if the param changes (in the example state=pending and state=firing are different filters that should trigger a separate request each).

After some investigation we concluded the reason behind this is the Redux store keeps the data from the latest loaded panel. This started happening after the filtering logic was moved to the backend. In order to fix this, a potential solution (that is implemented in this PR) is to fetch the grafana alert rules using RTK query instead of getting it from the store for prom rules, which are the ones that are filtered in the backend.

We are only getting promrules from RTKQ instead of the Redux Store for grafana managed alerts because so far, are the only ones that the filter is done in the BE.

Before - Only one request with state=firing when there is one missing with state=pending.
Screenshot 2023-06-21 at 19 06 12

Before - Two requests with state=firing and state=pending after forcing the request calls. The problem here is that the firing panel is not showing all instances due an incorrect result from the redux store.
Screenshot 2023-06-21 at 19 06 39

After - Using RTK Query the panels show the correct instances as we skip reading it from the store.
2023-06-22 16 20 24

@VikaCep VikaCep self-assigned this Jun 21, 2023
Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron left a comment

Choose a reason for hiding this comment

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

Great job! 🚀
I added some non-blocking comments

Copy link
Member

@gillesdemey gillesdemey left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, and I can't repro the bug on this branch – awesome work! 🚀

@VikaCep VikaCep marked this pull request as ready for review June 23, 2023 14:18
@VikaCep VikaCep requested a review from a team as a code owner June 23, 2023 14:18
@VikaCep VikaCep modified the milestone: 10.0.x Jun 23, 2023
@soniaAguilarPeiron
Copy link
Member

I would add in the PR description that we are only getting promrules from RTKQ instead of the Redux Store for grafana managed alerts because so far, are the only ones that the filter is done in the BE. @VikaCep

@gillesdemey gillesdemey changed the title Alerting: display correct results when using different filters on alerting panels Alerting: Display correct results when using different filters on alerting panels Jun 23, 2023
@VikaCep VikaCep merged commit f17c49e into main Jun 23, 2023
14 checks passed
@VikaCep VikaCep deleted the alerting/6423-escalation branch June 23, 2023 17:15
@grafana-delivery-bot
Copy link
Contributor

The backport to v10.0.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-70482-to-v10.0.x origin/v10.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x f17c49e632775a6c96d6c9361d2a3c9c80ca9e67
# When the conflicts are resolved, stage and commit the changes
git add . && git cherry-pick --continue
# If you have the GitHub CLI installed: Push the branch to GitHub and a PR:
gh pr create --title "[v10.0.x] Alerting: Display correct results when using different filters on alerting panels" --body "Backport f17c49e632775a6c96d6c9361d2a3c9c80ca9e67 from #70482" --label backport --base v10.0.x --milestone 10.0.x --web
# If you don't have the GitHub CLI installed: Push the branch to GitHub and manually create a PR:
git push --set-upstream origin backport-70482-to-v10.0.x
# Remove the local backport branch
git switch main
git branch -D backport-70482-to-v10.0.x

Unless you've used the GitHub CLI above, now create a pull request where the base branch is v10.0.x and the compare/head branch is backport-70482-to-v10.0.x.

@grafana-delivery-bot grafana-delivery-bot bot added the backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. label Jun 23, 2023
VikaCep added a commit that referenced this pull request Jun 23, 2023
…rting panels (#70482)

* Trigger separate rules request for each alerting panel in a dashboard

* Add RTK method to fetch prom rules

* Use RTKQuery to get prom rules in UnifiedAlertList

* Fix lint

* Mock promRules call

* Address PR comments

* Fix tests

(cherry picked from commit f17c49e)
VikaCep added a commit that referenced this pull request Jun 23, 2023
…ers on alerting panels (#70639)

Alerting: Display correct results when using different filters on alerting panels (#70482)

* Trigger separate rules request for each alerting panel in a dashboard

* Add RTK method to fetch prom rules

* Use RTKQuery to get prom rules in UnifiedAlertList

* Fix lint

* Mock promRules call

* Address PR comments

* Fix tests

(cherry picked from commit f17c49e)
LudoVio pushed a commit that referenced this pull request Jun 26, 2023
…rting panels (#70482)

* Trigger separate rules request for each alerting panel in a dashboard

* Add RTK method to fetch prom rules

* Use RTKQuery to get prom rules in UnifiedAlertList

* Fix lint

* Mock promRules call

* Address PR comments

* Fix tests
harisrozajac pushed a commit that referenced this pull request Jun 29, 2023
…rting panels (#70482)

* Trigger separate rules request for each alerting panel in a dashboard

* Add RTK method to fetch prom rules

* Use RTKQuery to get prom rules in UnifiedAlertList

* Fix lint

* Mock promRules call

* Address PR comments

* Fix tests
@zerok zerok modified the milestones: 10.0.x, 10.0.2 Jun 30, 2023
@zerok zerok modified the milestones: 10.0.2, 10.1.x Jun 30, 2023
harisrozajac pushed a commit that referenced this pull request Jun 30, 2023
…rting panels (#70482)

* Trigger separate rules request for each alerting panel in a dashboard

* Add RTK method to fetch prom rules

* Use RTKQuery to get prom rules in UnifiedAlertList

* Fix lint

* Mock promRules call

* Address PR comments

* Fix tests
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/alerting Grafana Alerting area/frontend backport v10.0.x backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. type/bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Dashboards: Only one alerts list panel on dashboard shows alerts even with mutually exclusive filters
6 participants