Alerting: Alert list performance improvements (#56247)

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>

.pa11yci-pr.conf.js

@@ -101,7 +101,7 @@ var config = {
       rootElement: '.main-view',
       // the unified alerting promotion alert's content contrast is too low
       // see https://github.com/grafana/grafana/pull/41829
-      threshold: 4,
+      threshold: 5,
     },
     {
       url: '${HOST}/datasources',

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

@@ -1,6 +1,7 @@
 import { css } from '@emotion/css';
 import React, { useEffect, useMemo, useState } from 'react';
 import { useLocation } from 'react-router-dom';
+import { useAsyncFn, useInterval } from 'react-use';
 
 import { GrafanaTheme2, urlUtil } from '@grafana/data';
 import { logInfo } from '@grafana/runtime';
@@ -8,6 +9,8 @@ import { Button, LinkButton, useStyles2, withErrorBoundary } from '@grafana/ui';
 import { useQueryParams } from 'app/core/hooks/useQueryParams';
 import { useDispatch } from 'app/types';
 
+import { CombinedRuleNamespace } from '../../../types/unified-alerting';
+
 import { LogMessages } from './Analytics';
 import { AlertingPageWrapper } from './components/AlertingPageWrapper';
 import { NoRulesSplash } from './components/rules/NoRulesCTA';
@@ -50,40 +53,45 @@ const RuleList = withErrorBoundary(
 
     const ViewComponent = VIEWS[view];
 
-    // fetch rules, then poll every RULE_LIST_POLL_INTERVAL_MS
-    useEffect(() => {
-      dispatch(fetchAllPromAndRulerRulesAction());
-      const interval = setInterval(() => dispatch(fetchAllPromAndRulerRulesAction()), RULE_LIST_POLL_INTERVAL_MS);
-      return () => {
-        clearInterval(interval);
-      };
-    }, [dispatch]);
-
     const promRuleRequests = useUnifiedAlertingSelector((state) => state.promRules);
     const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules);
 
-    const dispatched = rulesDataSourceNames.some(
-      (name) => promRuleRequests[name]?.dispatched || rulerRuleRequests[name]?.dispatched
-    );
     const loading = rulesDataSourceNames.some(
       (name) => promRuleRequests[name]?.loading || rulerRuleRequests[name]?.loading
     );
-    const haveResults = rulesDataSourceNames.some(
+
-      (name) =>
+    const promRequests = Object.entries(promRuleRequests);
-        (promRuleRequests[name]?.result?.length && !promRuleRequests[name]?.error) ||
+    const allPromLoaded = promRequests.every(
-        (Object.keys(rulerRuleRequests[name]?.result || {}).length && !rulerRuleRequests[name]?.error)
+      ([_, state]) => state.dispatched && (state?.result !== undefined || state?.error !== undefined)
     );
+    const allPromEmpty = promRequests.every(([_, state]) => state.dispatched && state?.result?.length === 0);
 
-    const showNewAlertSplash = dispatched && !loading && !haveResults;
+    // Trigger data refresh only when the RULE_LIST_POLL_INTERVAL_MS elapsed since the previous load FINISHED
+    const [_, fetchRules] = useAsyncFn(async () => {
+      if (!loading) {
+        await dispatch(fetchAllPromAndRulerRulesAction());
+      }
+    }, [loading]);
 
-    const combinedNamespaces = useCombinedRuleNamespaces();
+    // fetch rules, then poll every RULE_LIST_POLL_INTERVAL_MS
+    useEffect(() => {
+      dispatch(fetchAllPromAndRulerRulesAction());
+    }, [dispatch]);
+    useInterval(fetchRules, RULE_LIST_POLL_INTERVAL_MS);
+
+    // Show splash only when we loaded all of the data sources and none of them has alerts
+    const hasNoAlertRulesCreatedYet = allPromLoaded && allPromEmpty && promRequests.length > 0;
+
+    const combinedNamespaces: CombinedRuleNamespace[] = useCombinedRuleNamespaces();
     const filteredNamespaces = useFilteredRules(combinedNamespaces);
     return (
-      <AlertingPageWrapper pageId="alert-list" isLoading={loading && !haveResults}>
+      // We don't want to show the Loading... indicator for the whole page.
+      // We show separate indicators for Grafana-managed and Cloud rules
+      <AlertingPageWrapper pageId="alert-list" isLoading={false}>
         <RuleListErrors />
-        {!showNewAlertSplash && (
+        <RulesFilter />
+        {!hasNoAlertRulesCreatedYet && (
           <>
-            <RulesFilter />
             <div className={styles.break} />
             <div className={styles.buttonsContainer}>
               <div className={styles.statsContainer}>
@@ -111,8 +119,8 @@ const RuleList = withErrorBoundary(
             </div>
           </>
         )}
-        {showNewAlertSplash && <NoRulesSplash />}
+        {hasNoAlertRulesCreatedYet && <NoRulesSplash />}
-        {haveResults && <ViewComponent expandAll={expandAll} namespaces={filteredNamespaces} />}
+        {!hasNoAlertRulesCreatedYet && <ViewComponent expandAll={expandAll} namespaces={filteredNamespaces} />}
       </AlertingPageWrapper>
     );
   },

public/app/features/alerting/unified/components/rules/CloudRules.tsx

@@ -3,7 +3,7 @@ import pluralize from 'pluralize';
 import React, { FC, useMemo } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { LoadingPlaceholder, Pagination, useStyles2 } from '@grafana/ui';
+import { LoadingPlaceholder, Pagination, Spinner, useStyles2 } from '@grafana/ui';
 import { CombinedRuleNamespace } from 'app/types/unified-alerting';
 
 import { DEFAULT_PER_PAGE_PAGINATION } from '../../../../../core/constants';
@@ -25,18 +25,20 @@ export const CloudRules: FC<Props> = ({ namespaces, expandAll }) => {
   const styles = useStyles2(getStyles);
 
   const dsConfigs = useUnifiedAlertingSelector((state) => state.dataSources);
-  const rules = useUnifiedAlertingSelector((state) => state.promRules);
+  const promRules = useUnifiedAlertingSelector((state) => state.promRules);
   const rulesDataSources = useMemo(getRulesDataSources, []);
   const groupsWithNamespaces = useCombinedGroupNamespace(namespaces);
 
   const dataSourcesLoading = useMemo(
     () =>
       rulesDataSources.filter(
-        (ds) => isAsyncRequestStatePending(rules[ds.name]) || isAsyncRequestStatePending(dsConfigs[ds.name])
+        (ds) => isAsyncRequestStatePending(promRules[ds.name]) || isAsyncRequestStatePending(dsConfigs[ds.name])
       ),
-    [rules, dsConfigs, rulesDataSources]
+    [promRules, dsConfigs, rulesDataSources]
   );
 
+  const hasSomeResults = rulesDataSources.some((ds) => promRules[ds.name]?.result?.length ?? 0 > 0);
+
   const hasDataSourcesConfigured = rulesDataSources.length > 0;
   const hasDataSourcesLoading = dataSourcesLoading.length > 0;
   const hasNamespaces = namespaces.length > 0;
@@ -75,6 +77,7 @@ export const CloudRules: FC<Props> = ({ namespaces, expandAll }) => {
 
       {!hasDataSourcesConfigured && <p>There are no Prometheus or Loki data sources configured.</p>}
       {hasDataSourcesConfigured && !hasDataSourcesLoading && !hasNamespaces && <p>No rules found.</p>}
+      {!hasSomeResults && hasDataSourcesLoading && <Spinner size={24} className={styles.spinner} />}
 
       <Pagination
         className={styles.pagination}
@@ -98,5 +101,9 @@ const getStyles = (theme: GrafanaTheme2) => ({
   wrapper: css`
     margin-bottom: ${theme.spacing(4)};
   `,
+  spinner: css`
+    text-align: center;
+    padding: ${theme.spacing(2)};
+  `,
   pagination: getPaginationStyles(theme),
 });

public/app/features/alerting/unified/components/rules/GrafanaRules.tsx

@@ -2,7 +2,7 @@ import { css } from '@emotion/css';
 import React, { FC } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { LoadingPlaceholder, Pagination, useStyles2 } from '@grafana/ui';
+import { LoadingPlaceholder, Pagination, Spinner, useStyles2 } from '@grafana/ui';
 import { useQueryParams } from 'app/core/hooks/useQueryParams';
 import { CombinedRuleNamespace } from 'app/types/unified-alerting';
 
@@ -26,9 +26,13 @@ export const GrafanaRules: FC<Props> = ({ namespaces, expandAll }) => {
   const styles = useStyles2(getStyles);
   const [queryParams] = useQueryParams();
 
-  const { loading } = useUnifiedAlertingSelector(
+  const { prom, ruler } = useUnifiedAlertingSelector((state) => ({
-    (state) => state.promRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState
+    prom: state.promRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState,
-  );
+    ruler: state.rulerRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState,
+  }));
+
+  const loading = prom.loading || ruler.loading;
+  const hasResult = !!prom.result || !!ruler.result;
 
   const wantsGroupedView = queryParams['view'] === 'grouped';
   const namespacesFormat = wantsGroupedView ? namespaces : flattenGrafanaManagedRules(namespaces);
@@ -57,7 +61,8 @@ export const GrafanaRules: FC<Props> = ({ namespaces, expandAll }) => {
           viewMode={wantsGroupedView ? 'grouped' : 'list'}
         />
       ))}
-      {namespacesFormat?.length === 0 && <p>No rules found.</p>}
+      {hasResult && namespacesFormat?.length === 0 && <p>No rules found.</p>}
+      {!hasResult && loading && <Spinner size={24} className={styles.spinner} />}
       <Pagination
         className={styles.pagination}
         currentPage={page}
@@ -80,5 +85,9 @@ const getStyles = (theme: GrafanaTheme2) => ({
   wrapper: css`
     margin-bottom: ${theme.spacing(4)};
   `,
+  spinner: css`
+    text-align: center;
+    padding: ${theme.spacing(2)};
+  `,
   pagination: getPaginationStyles(theme),
 });

public/app/features/alerting/unified/components/rules/RuleListErrors.tsx

@@ -53,7 +53,10 @@ export function RuleListErrors(): ReactElement {
       result.push(
         <>
           Failed to load the data source configuration for{' '}
-          <a href={makeDataSourceLink(dataSource)}>{dataSource.name}</a>: {error.message || 'Unknown error.'}
+          <a href={makeDataSourceLink(dataSource)} className={styles.dsLink}>
+            {dataSource.name}
+          </a>
+          : {error.message || 'Unknown error.'}
         </>
       );
     });
@@ -61,8 +64,11 @@ export function RuleListErrors(): ReactElement {
     promRequestErrors.forEach(({ dataSource, error }) =>
       result.push(
         <>
-          Failed to load rules state from <a href={makeDataSourceLink(dataSource)}>{dataSource.name}</a>:{' '}
+          Failed to load rules state from{' '}
-          {error.message || 'Unknown error.'}
+          <a href={makeDataSourceLink(dataSource)} className={styles.dsLink}>
+            {dataSource.name}
+          </a>
+          : {error.message || 'Unknown error.'}
         </>
       )
     );
@@ -70,14 +76,17 @@ export function RuleListErrors(): ReactElement {
     rulerRequestErrors.forEach(({ dataSource, error }) =>
       result.push(
         <>
-          Failed to load rules config from <a href={makeDataSourceLink(dataSource)}>{dataSource.name}</a>:{' '}
+          Failed to load rules config from{' '}
-          {error.message || 'Unknown error.'}
+          <a href={makeDataSourceLink(dataSource)} className={styles.dsLink}>
+            {dataSource.name}
+          </a>
+          : {error.message || 'Unknown error.'}
         </>
       )
     );
 
     return result;
-  }, [dataSourceConfigRequests, promRuleRequests, rulerRuleRequests]);
+  }, [dataSourceConfigRequests, promRuleRequests, rulerRuleRequests, styles.dsLink]);
 
   return (
     <>
@@ -141,4 +150,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: flex;
     justify-content: flex-end;
   `,
+  dsLink: css`
+    font-weight: ${theme.typography.fontWeightBold};
+  `,
 });

public/app/features/alerting/unified/components/rules/RulesTable.tsx

@@ -45,15 +45,9 @@ export const RulesTable: FC<Props> = ({
   const wrapperClass = cx(styles.wrapper, className, { [styles.wrapperMargin]: showGuidelines });
 
   const items = useMemo((): RuleTableItemProps[] => {
-    const seenKeys: string[] = [];
     return rules.map((rule, ruleIdx) => {
-      let key = JSON.stringify([rule.promRule?.type, rule.labels, rule.query, rule.name, rule.annotations]);
-      if (seenKeys.includes(key)) {
-        key += `-${ruleIdx}`;
-      }
-      seenKeys.push(key);
       return {
-        id: key,
+        id: `${rule.namespace.name}-${rule.group.name}-${rule.name}-${ruleIdx}`,
         data: rule,
       };
     });

public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts

@@ -1,3 +1,4 @@
+import { isEqual } from 'lodash';
 import { useMemo, useRef } from 'react';
 
 import {
@@ -129,18 +130,29 @@ function addRulerGroupsToCombinedNamespace(namespace: CombinedRuleNamespace, gro
 }
 
 function addPromGroupsToCombinedNamespace(namespace: CombinedRuleNamespace, groups: RuleGroup[]): void {
+  const existingGroupsByName = new Map<string, CombinedRuleGroup>();
+  namespace.groups.forEach((group) => existingGroupsByName.set(group.name, group));
+
   groups.forEach((group) => {
-    let combinedGroup = namespace.groups.find((g) => g.name === group.name);
+    let combinedGroup = existingGroupsByName.get(group.name);
     if (!combinedGroup) {
       combinedGroup = {
         name: group.name,
         rules: [],
       };
       namespace.groups.push(combinedGroup);
+      existingGroupsByName.set(group.name, combinedGroup);
     }
 
+    const combinedRulesByName = new Map<string, CombinedRule[]>();
+    combinedGroup!.rules.forEach((r) => {
+      // Prometheus rules do not have to be unique by name
+      const existingRule = combinedRulesByName.get(r.name);
+      existingRule ? existingRule.push(r) : combinedRulesByName.set(r.name, [r]);
+    });
+
     (group.rules ?? []).forEach((rule) => {
-      const existingRule = getExistingRuleInGroup(rule, combinedGroup!, namespace.rulesSource);
+      const existingRule = getExistingRuleInGroup(rule, combinedRulesByName, namespace.rulesSource);
       if (existingRule) {
         existingRule.promRule = rule;
       } else {
@@ -201,39 +213,47 @@ function rulerRuleToCombinedRule(
 // find existing rule in group that matches the given prom rule
 function getExistingRuleInGroup(
   rule: Rule,
-  group: CombinedRuleGroup,
+  existingCombinedRulesMap: Map<string, CombinedRule[]>,
   rulesSource: RulesSource
 ): CombinedRule | undefined {
+  // Using Map of name-based rules is important performance optimization for the code below
+  // Otherwise we would perform find method multiple times on (possibly) thousands of rules
+
+  const nameMatchingRules = existingCombinedRulesMap.get(rule.name);
+  if (!nameMatchingRules) {
+    return undefined;
+  }
+
   if (isGrafanaRulesSource(rulesSource)) {
     // assume grafana groups have only the one rule. check name anyway because paranoid
-    return group!.rules.find((existingRule) => existingRule.name === rule.name);
+    return nameMatchingRules[0];
   }
-  return (
+
-    // try finding a rule that matches name, labels, annotations and query
+  // try finding a rule that matches name, labels, annotations and query
-    group!.rules.find(
+  const strictlyMatchingRule = nameMatchingRules.find(
-      (existingRule) => !existingRule.promRule && isCombinedRuleEqualToPromRule(existingRule, rule, true)
+    (combinedRule) => !combinedRule.promRule && isCombinedRuleEqualToPromRule(combinedRule, rule, true)
-    ) ??
-    // if that fails, try finding a rule that only matches name, labels and annotations.
-    // loki & prom can sometimes modify the query so it doesnt match, eg `2 > 1` becomes `1`
-    group!.rules.find(
-      (existingRule) => !existingRule.promRule && isCombinedRuleEqualToPromRule(existingRule, rule, false)
-    )
   );
+  if (strictlyMatchingRule) {
+    return strictlyMatchingRule;
+  }
+
+  // if that fails, try finding a rule that only matches name, labels and annotations.
+  // loki & prom can sometimes modify the query so it doesnt match, eg `2 > 1` becomes `1`
+  const looselyMatchingRule = nameMatchingRules.find(
+    (combinedRule) => !combinedRule.promRule && isCombinedRuleEqualToPromRule(combinedRule, rule, false)
+  );
+  if (looselyMatchingRule) {
+    return looselyMatchingRule;
+  }
+
+  return undefined;
 }
 
 function isCombinedRuleEqualToPromRule(combinedRule: CombinedRule, rule: Rule, checkQuery = true): boolean {
   if (combinedRule.name === rule.name) {
-    return (
+    return isEqual(
-      JSON.stringify([
+      [checkQuery ? hashQuery(combinedRule.query) : '', combinedRule.labels, combinedRule.annotations],
-        checkQuery ? hashQuery(combinedRule.query) : '',
+      [checkQuery ? hashQuery(rule.query) : '', rule.labels || {}, isAlertingRule(rule) ? rule.annotations || {} : {}]
-        combinedRule.labels,
-        combinedRule.annotations,
-      ]) ===
-      JSON.stringify([
-        checkQuery ? hashQuery(rule.query) : '',
-        rule.labels || {},
-        isAlertingRule(rule) ? rule.annotations || {} : {},
-      ])
     );
   }
   return false;

public/app/features/alerting/unified/state/actions.ts

@@ -290,9 +290,9 @@ export const fetchRulesSourceBuildInfoAction = createAsyncThunk(
   }
 );
 
-export function fetchAllPromAndRulerRulesAction(force = false): ThunkResult<void> {
+export function fetchAllPromAndRulerRulesAction(force = false): ThunkResult<Promise<void>> {
   return async (dispatch, getStore) => {
-    return Promise.all(
+    await Promise.allSettled(
       getAllRulesSourceNames().map(async (rulesSourceName) => {
         await dispatch(fetchRulesSourceBuildInfoAction({ rulesSourceName }));
 
@@ -303,12 +303,13 @@ export function fetchAllPromAndRulerRulesAction(force = false): ThunkResult<void
           return;
         }
 
-        if (force || !promRules[rulesSourceName]?.loading) {
+        const shouldLoadProm = force || !promRules[rulesSourceName]?.loading;
-          dispatch(fetchPromRulesAction({ rulesSourceName }));
+        const shouldLoadRuler = (force || !rulerRules[rulesSourceName]?.loading) && dataSourceConfig.rulerConfig;
-        }
+
-        if ((force || !rulerRules[rulesSourceName]?.loading) && dataSourceConfig.rulerConfig) {
+        await Promise.allSettled([
-          dispatch(fetchRulerRulesAction({ rulesSourceName }));
+          shouldLoadProm && dispatch(fetchPromRulesAction({ rulesSourceName })),
-        }
+          shouldLoadRuler && dispatch(fetchRulerRulesAction({ rulesSourceName })),
+        ]);
       })
     );
   };