Alerting: Add notification policies preview in alert creation #68839

soniaAguilarPeiron · 2023-05-22T15:14:33Z

What is this feature?

This feature is about adding the notification policies preview in the alert rule form. That means, showing all the potential instances we could have taking in account:

labels autogenerated by the expression defined, alert name and selected folder.
custom labels

For getting the the potential instances, we need an endpoint to get this potential labels => we created this issue for solving this problem.

This feature can only be done only for Grafana managed alerts.

Once we have this list of potential alerts, we have to traverse the notification policy tree to apply the algorithm that matches these labels from the list with the notification policies labels. => for this part we need to move some logic to a webworker.

We also use Suspense and lazy loading for rendering the NotificationPreviewByAlertManager.

Why do we need this feature?

Having this potential alerts from the rule creation context, could be a great help for users understanding which instances will be generated and where will be sent depending on the labels we have.

Who is this feature for?

All users
Which issue(s) does this PR fix?:

Fixes #68098

Special notes for your reviewer:

notification-preview-2.webm

Last three commits make some changes to the details preview:

We removed the labels section as is redundant:

Please check that:

It works as expected from a user's perspective.
If this is a pre-GA feature, it is behind a feature toggle.
The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>

… changes

…the code more clean

… from parent as we do with undefined

… policy matchers to match potential instances" This reverts commit ee73ae9.

VikaCep · 2023-06-15T19:59:35Z

I think the text on top shouldn't be shown until the Preview routing button is clicked. The text on the bottom seems more appropriate for when the list isn't shown yet so I would only show that part.

soniaAguilarPeiron · 2023-06-16T11:41:40Z

I think the text on top shouldn't be shown until the Preview routing button is clicked. The text on the bottom seems more appropriate for when the list isn't shown yet so I would only show that part.

💯 good catch @VikaCep !

Commit pushed!

prev-mess.mp4

public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap

gillesdemey · 2023-06-14T15:32:20Z

...fied/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts

+  const { matchInstancesToRoute } = useRouteGroupsMatcher();
+
+  // to create the list of matching contact points we need to first get the rootRoute
+  const { rootRoute, receivers } = useMemo((): { rootRoute?: RouteWithID; receivers: Receiver[] } => {


I think we can remove the necessity to type the return value here by simply explicitly setting rootRoute to undefined when we check for the AMConfig

gillesdemey · 2023-06-16T11:59:07Z

public/app/features/alerting/unified/NotificationPolicies.test.tsx

@@ -29,6 +29,8 @@ import { getAllDataSources } from './utils/config';
 import { ALERTMANAGER_NAME_QUERY_KEY } from './utils/constants';
 import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource';

+import 'core-js/stable/structured-clone';


Can we not use cloneDeep from lodash?

It's mainly a matter of preference for me to use built-in functions instead of external ones. Unfortunately, we use a quite old Node version, so a polyfill is needed in tests

public/app/features/alerting/unified/ExistingRuleEditor.tsx

public/app/features/alerting/unified/NotificationPolicies.tsx

...p/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx

public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx

gillesdemey · 2023-06-16T12:56:40Z

...p/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx

+}: NotificationRouteProps) {
+  const styles = useStyles2(getStyles);
+  const [expandRoute, setExpandRoute] = useToggle(false);
+  const GREY_COLOR_INDEX = 9;


non-blocking: not a big fan of this approach since the color index might be updated at some point in the future, but not blocking for now.

Maybe we should roll our own tag component, one that supports a custom function to define the color and allow manual color overrides?

Added a TODO comment for this one

...p/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationRoute.tsx

VikaCep

First round of review, looks awesome! Just a few comments/questions 💬

public/app/features/alerting/unified/components/rule-editor/NotificationsStep.tsx

...features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.tsx

...fied/components/rule-editor/notificaton-preview/useAlertmanagerNotificationRoutingPreview.ts

brendamuir · 2023-06-18T08:23:17Z

Hey there! am seeing the UI text not exactly matching this doc - can you cross-check and update? thanks! https://docs.google.com/document/d/1drJhkJ0O7fvQeOO8_c6V0FAu3_3mBtkFZuJJbbOHJ-o/edit#heading=h.neepxy20wlig

soniaAguilarPeiron · 2023-06-19T08:49:46Z

Hey there! am seeing the UI text not exactly matching this doc - can you cross-check and update? thanks! https://docs.google.com/document/d/1drJhkJ0O7fvQeOO8_c6V0FAu3_3mBtkFZuJJbbOHJ-o/edit#heading=h.neepxy20wlig

Thank you @brendamuir ! I've just updated the text following the doc:

gillesdemey

LGTM no blockers

* Add notification policies preview in alert rule form Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com> * Refactor using new useGetPotentialInstances hook and apply some style changes * Add notification policy detail modal * Use backtesting api for simulating potential alert instances * Fix logic to travserse all the children from the root route * Split notification preview by alert manager * Add instance count to matching policy header and fix some styles * Move some logic to a new hook useGetAlertManagersSourceNames to make the code more clean * Fix some tests * Add initial test for NotificationPreview * Use button to preview potential instances * Add link to contact point details * Add route matching result details * Show AlertManager image in the routing preview list * Add tests setup, add single AM preview test * Handle no matchers and no labels use case * Update some style in collapse component and fix policy path in modal * Update modal styles * Update styles * Update collapse header styling * Normalize tree nodes should happen before findMatchingRoutes call * Fix findMatchingRoutes and findMatchingAlertGroups methods after reabasing * Move instances matching to the web worker code * Fix config fetching for vanilla prometheus AMs * Add tests * Add tests mocks * Fix tests after adding web worker * Display matching labels for each matching alert instance * Add minor css improvements * Revert changes added in Collapse component as we don't use it anymore * Move the route details modal to a separate file * Move NotificationRoute and preview hook into separate files * Fix Alertmanager preview tests * Fix tests * Move matcher code to a separate file, improve matcher mock * Add permissions control for contact point edit view link * Fix from and to for the temporal use of backtesting api * Fix tests, add lazy loading of the preview component Co-authored-by: Sonia Aguilar <soniaaguilarpeiron@gmail.com> * Fix preview test * Add onclick on the header div so it collapse and expands when clicking on it, and update styles to be consistent with the rest of tables * Adapt the code to the new rule testing endpoint definition * Fix tests * small changes after reviewing the final code * compute entire inherited tree before computing the routes map * Throw error in case of not having receiver in routesByIdMap and add test for the use case of inheriting receiver from parent to check UI throws no errors * Add list of labels in the policy route path that produces the policy matchers to match potential instances * Use color determined by the key, in label tags when hovering matchers in the policy tree * Remove labels in modal and handle empty string as receiver to inherit from parent as we do with undefined * Revert "Add list of labels in the policy route path that produces the policy matchers to match potential instances" This reverts commit ee73ae9. * fix inheritance for computeInheritedTree * Fix message shown when preview has not been executed yet * First round for adressing PR review comments * Adress the rest of PR review commments * Update texts and rename id prop in NotificaitonStep to alertUid --------- Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com> Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>

soniaAguilarPeiron added area/alerting Grafana Alerting add to changelog no-backport Skip backport of PR add to what's new kata:alert-rule-creation labels May 22, 2023

soniaAguilarPeiron added this to the 10.1.x milestone May 22, 2023

soniaAguilarPeiron assigned konrad147 May 22, 2023

soniaAguilarPeiron requested a review from a team as a code owner May 22, 2023 15:14

soniaAguilarPeiron requested a review from a team May 22, 2023 15:14

soniaAguilarPeiron self-assigned this May 22, 2023

soniaAguilarPeiron requested review from tskarhed and L-M-K-B and removed request for a team May 22, 2023 15:14

soniaAguilarPeiron marked this pull request as draft May 22, 2023 15:14

grafanabot added the area/frontend label May 22, 2023

soniaAguilarPeiron changed the title ~~Alerting: notification policies preview in alert creation~~ Alerting: Add notification policies preview in alert creation May 22, 2023

soniaAguilarPeiron force-pushed the alerting/notification-policies-preview-in-alert-creation branch 5 times, most recently from eb336c9 to c7d7bc0 Compare May 29, 2023 14:52

soniaAguilarPeiron added 9 commits May 31, 2023 09:29

Add notification policies preview in alert rule form

b9106f6

Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com>

Refactor using new useGetPotentialInstances hook and apply some style…

e4cb594

… changes

Add notification policy detail modal

99e2a3e

Use backtesting api for simulating potential alert instances

16949c1

Fix logic to travserse all the children from the root route

e18c986

Split notification preview by alert manager

77690fd

Add instance count to matching policy header and fix some styles

dd85bd4

Move some logic to a new hook useGetAlertManagersSourceNames to make …

3625292

…the code more clean

Fix some tests

a77ae70

soniaAguilarPeiron added 2 commits June 14, 2023 17:30

Remove labels in modal and handle empty string as receiver to inherit…

cb48b48

… from parent as we do with undefined

Revert "Add list of labels in the policy route path that produces the…

8c0b6c3

… policy matchers to match potential instances" This reverts commit ee73ae9.

soniaAguilarPeiron force-pushed the alerting/notification-policies-preview-in-alert-creation branch from 8e88d0e to 8c0b6c3 Compare June 14, 2023 16:08

fix inheritance for computeInheritedTree

9d32c2e

soniaAguilarPeiron requested a review from konrad147 June 16, 2023 07:19

Fix message shown when preview has not been executed yet

f14ee91

gillesdemey reviewed Jun 16, 2023

View reviewed changes

First round for adressing PR review comments

261b12f

soniaAguilarPeiron force-pushed the alerting/notification-policies-preview-in-alert-creation branch from 1187e38 to 261b12f Compare June 16, 2023 15:49

VikaCep reviewed Jun 16, 2023

View reviewed changes

Adress the rest of PR review commments

ba150ab

soniaAguilarPeiron force-pushed the alerting/notification-policies-preview-in-alert-creation branch from c74d226 to ba150ab Compare June 19, 2023 07:40

soniaAguilarPeiron requested review from gillesdemey, VikaCep and brendamuir June 19, 2023 07:46

Update texts and rename id prop in NotificaitonStep to alertUid

b008cac

soniaAguilarPeiron force-pushed the alerting/notification-policies-preview-in-alert-creation branch from 9734fe7 to b008cac Compare June 19, 2023 08:58

gillesdemey approved these changes Jun 19, 2023

View reviewed changes

brendamuir approved these changes Jun 19, 2023

View reviewed changes

soniaAguilarPeiron merged commit 9a252c7 into main Jun 19, 2023
16 checks passed

soniaAguilarPeiron deleted the alerting/notification-policies-preview-in-alert-creation branch June 19, 2023 11:32

ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023

chenrui333 mentioned this pull request Aug 24, 2023

grafana 10.1.0 Homebrew/homebrew-core#140321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerting: Add notification policies preview in alert creation #68839

Alerting: Add notification policies preview in alert creation #68839

soniaAguilarPeiron commented May 22, 2023 •

edited

Loading

VikaCep commented Jun 15, 2023

soniaAguilarPeiron commented Jun 16, 2023 •

edited

Loading

gillesdemey Jun 14, 2023

gillesdemey Jun 16, 2023

konrad147 Jun 19, 2023

gillesdemey Jun 16, 2023

soniaAguilarPeiron Jun 19, 2023 •

edited

Loading

VikaCep left a comment

brendamuir commented Jun 18, 2023

soniaAguilarPeiron commented Jun 19, 2023 •

edited

Loading

gillesdemey left a comment

Alerting: Add notification policies preview in alert creation #68839

Alerting: Add notification policies preview in alert creation #68839

Conversation

soniaAguilarPeiron commented May 22, 2023 • edited Loading

VikaCep commented Jun 15, 2023

soniaAguilarPeiron commented Jun 16, 2023 • edited Loading

gillesdemey Jun 14, 2023

Choose a reason for hiding this comment

gillesdemey Jun 16, 2023

Choose a reason for hiding this comment

konrad147 Jun 19, 2023

Choose a reason for hiding this comment

gillesdemey Jun 16, 2023

Choose a reason for hiding this comment

soniaAguilarPeiron Jun 19, 2023 • edited Loading

Choose a reason for hiding this comment

VikaCep left a comment

Choose a reason for hiding this comment

brendamuir commented Jun 18, 2023

soniaAguilarPeiron commented Jun 19, 2023 • edited Loading

gillesdemey left a comment

Choose a reason for hiding this comment

soniaAguilarPeiron commented May 22, 2023 •

edited

Loading

soniaAguilarPeiron commented Jun 16, 2023 •

edited

Loading

soniaAguilarPeiron Jun 19, 2023 •

edited

Loading

soniaAguilarPeiron commented Jun 19, 2023 •

edited

Loading