Alerting: Reduce number of unnecessary request in the alert list pane…

…l in case … (#70583) * Reduce number of unnecessary request in the alert list panel in case we have data source filter selected * use fetchPromAndRulerRulesAction instead of fetchAllPromAndRulerRulesAction in case of having datasource selected in options * remove unnecessary useEffect * Reduce number of requests in GroupBy component if data source is selected * use redux utils for asyncmapslice * lint * Address review comments * Skip fetching grafana rules if datasource is not Grafana rules source name --------- Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
grafana · Jun 30, 2023 · 80140c7 · 80140c7
1 parent 4d7ca80
commit 80140c7
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 72 deletions.
diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts
@@ -246,15 +246,23 @@ export const fetchRulerRulesAction = createAsyncThunk(
 export function fetchPromAndRulerRulesAction({
   rulesSourceName,
   identifier,
+  filter,
+  limitAlerts,
+  matcher,
+  state,
 }: {
   rulesSourceName: string;
   identifier?: RuleIdentifier;
+  filter?: FetchPromRulesFilter;
+  limitAlerts?: number;
+  matcher?: Matcher[];
+  state?: string[];
 }): ThunkResult<void> {
   return async (dispatch, getState) => {
     await dispatch(fetchRulesSourceBuildInfoAction({ rulesSourceName }));
     const dsConfig = getDataSourceConfig(getState, rulesSourceName);
 
-    await dispatch(fetchPromRulesAction({ rulesSourceName, identifier }));
+    await dispatch(fetchPromRulesAction({ rulesSourceName, identifier, filter, limitAlerts, matcher, state }));
     if (dsConfig.rulerConfig) {
       await dispatch(fetchRulerRulesAction({ rulesSourceName }));
     }

diff --git a/public/app/features/alerting/unified/utils/redux.ts b/public/app/features/alerting/unified/utils/redux.ts
@@ -188,6 +188,14 @@ export function isAsyncRequestMapSlicePending<T>(slice: AsyncRequestMapSlice<T>)
   return Object.values(slice).some(isAsyncRequestStatePending);
 }
 
+export function isAsyncRequestMapSlicePartiallyDispatched<T>(slice: AsyncRequestMapSlice<T>): boolean {
+  return Object.values(slice).some((state) => state.dispatched);
+}
+
+export function isAsyncRequestMapSlicePartiallyFulfilled<T>(slice: AsyncRequestMapSlice<T>): boolean {
+  return Object.values(slice).some(isAsyncRequestStateFulfilled);
+}
+
 export function isAsyncRequestStatePending<T>(state?: AsyncRequestState<T>): boolean {
   if (!state) {
     return false;

diff --git a/public/app/plugins/panel/alertlist/GroupByWithLoading.tsx b/public/app/plugins/panel/alertlist/GroupByWithLoading.tsx
@@ -13,21 +13,28 @@ import { useDispatch } from 'app/types';
 import { AlertingRule } from 'app/types/unified-alerting';
 import { PromRuleType } from 'app/types/unified-alerting-dto';
 
+import { fetchPromRulesAction } from '../../../features/alerting/unified/state/actions';
+
 import { isPrivateLabel } from './util';
 
 interface Props {
   id: string;
   defaultValue: SelectableValue<string>;
   onChange: (keys: string[]) => void;
+  dataSource?: string;
 }
 
 export const GroupBy = (props: Props) => {
-  const { onChange, id, defaultValue } = props;
+  const { onChange, id, defaultValue, dataSource } = props;
   const dispatch = useDispatch();
 
   useEffect(() => {
-    dispatch(fetchAllPromRulesAction());
-  }, [dispatch]);
+    if (dataSource) {
+      dataSource && dispatch(fetchPromRulesAction({ rulesSourceName: dataSource }));
+    } else {
+      dispatch(fetchAllPromRulesAction());
+    }
+  }, [dispatch, dataSource]);
 
   const promRulesByDatasource = useUnifiedAlertingSelector((state) => state.promRules);
 

diff --git a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx
@@ -22,19 +22,23 @@ import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi';
 import { INSTANCES_DISPLAY_LIMIT } from 'app/features/alerting/unified/components/rules/RuleDetails';
 import { useCombinedRuleNamespaces } from 'app/features/alerting/unified/hooks/useCombinedRuleNamespaces';
 import { useUnifiedAlertingSelector } from 'app/features/alerting/unified/hooks/useUnifiedAlertingSelector';
-import { fetchAllPromAndRulerRulesAction } from 'app/features/alerting/unified/state/actions';
+import {
+  fetchAllPromAndRulerRulesAction,
+  fetchPromAndRulerRulesAction,
+} from 'app/features/alerting/unified/state/actions';
 import { parseMatchers } from 'app/features/alerting/unified/utils/alertmanager';
 import { Annotation } from 'app/features/alerting/unified/utils/constants';
+import { GRAFANA_DATASOURCE_NAME, GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource';
 import {
-  getAllRulesSourceNames,
-  GRAFANA_DATASOURCE_NAME,
-  GRAFANA_RULES_SOURCE_NAME,
-} from 'app/features/alerting/unified/utils/datasource';
-import { initialAsyncRequestState } from 'app/features/alerting/unified/utils/redux';
+  isAsyncRequestMapSlicePartiallyDispatched,
+  isAsyncRequestMapSlicePartiallyFulfilled,
+  isAsyncRequestMapSlicePending,
+} from 'app/features/alerting/unified/utils/redux';
 import { flattenCombinedRules, getFirstActiveAt } from 'app/features/alerting/unified/utils/rules';
 import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
 import { DashboardModel } from 'app/features/dashboard/state';
-import { AccessControlAction, useDispatch } from 'app/types';
+import { Matcher } from 'app/plugins/datasource/alertmanager/types';
+import { AccessControlAction, ThunkDispatch, useDispatch } from 'app/types';
 import { PromAlertingRuleState } from 'app/types/unified-alerting-dto';
 
 import { getAlertingRule } from '../../../features/alerting/unified/utils/rules';
@@ -56,13 +60,50 @@ function getStateList(state: StateFilter) {
   return Object.entries(state).reduce(reducer, []);
 }
 
+const fetchPromAndRuler = ({
+  dispatch,
+  limitInstances,
+  matcherList,
+  dataSourceName,
+  stateList,
+}: {
+  dispatch: ThunkDispatch;
+  limitInstances: boolean;
+  matcherList?: Matcher[] | undefined;
+  dataSourceName?: string;
+  stateList: string[];
+}) => {
+  if (dataSourceName) {
+    dispatch(
+      fetchPromAndRulerRulesAction({
+        rulesSourceName: dataSourceName,
+        limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
+        matcher: matcherList,
+        state: stateList,
+      })
+    );
+  } else {
+    dispatch(
+      fetchAllPromAndRulerRulesAction(false, {
+        limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
+        matcher: matcherList,
+        state: stateList,
+      })
+    );
+  }
+};
+
 export function UnifiedAlertList(props: PanelProps<UnifiedAlertListOptions>) {
   const dispatch = useDispatch();
-  const rulesDataSourceNames = useMemo(getAllRulesSourceNames, []);
   const [limitInstances, toggleLimit] = useToggle(true);
 
   const { usePrometheusRulesByNamespaceQuery } = alertRuleApi;
 
+  const promRulesRequests = useUnifiedAlertingSelector((state) => state.promRules);
+  const rulerRulesRequests = useUnifiedAlertingSelector((state) => state.rulerRules);
+
+  const somePromRulesDispatched = isAsyncRequestMapSlicePartiallyDispatched(promRulesRequests);
+
   // backwards compat for "Inactive" state filter
   useEffect(() => {
     if (props.options.stateFilter.inactive === true) {
@@ -79,6 +120,8 @@ export function UnifiedAlertList(props: PanelProps<UnifiedAlertListOptions>) {
 
   const stateList = useMemo(() => getStateList(props.options.stateFilter), [props.options.stateFilter]);
   const { options, replaceVariables } = props;
+  const dataSourceName =
+    options.datasource === GRAFANA_DATASOURCE_NAME ? GRAFANA_RULES_SOURCE_NAME : options.datasource;
   const parsedOptions: UnifiedAlertListOptions = {
     ...props.options,
     alertName: replaceVariables(options.alertName),
@@ -90,87 +133,47 @@ export function UnifiedAlertList(props: PanelProps<UnifiedAlertListOptions>) {
     [parsedOptions.alertInstanceLabelFilter]
   );
 
-  useEffect(() => {
-    if (props.options.groupMode === GroupMode.Default) {
-      dispatch(
-        fetchAllPromAndRulerRulesAction(false, {
-          limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
-          matcher: matcherList,
-          state: stateList,
-        })
-      );
-    }
-  }, [props.options.groupMode, limitInstances, dispatch, matcherList, stateList]);
-
   useEffect(() => {
     //we need promRules and rulerRules for getting the uid when creating the alert link in panel in case of being a rulerRule.
-    dispatch(
-      fetchAllPromAndRulerRulesAction(false, {
-        limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
-        matcher: matcherList,
-        state: stateList,
-      })
-    );
+    if (!promRulesRequests.loading) {
+      fetchPromAndRuler({ dispatch, limitInstances, matcherList, dataSourceName, stateList });
+    }
     const sub = dashboard?.events.subscribe(TimeRangeUpdatedEvent, () =>
-      dispatch(
-        fetchAllPromAndRulerRulesAction(false, {
-          limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
-          matcher: matcherList,
-          state: stateList,
-        })
-      )
+      fetchPromAndRuler({ dispatch, limitInstances, matcherList, dataSourceName, stateList })
     );
     return () => {
       sub?.unsubscribe();
     };
-  }, [dispatch, dashboard, matcherList, stateList, toggleLimit, limitInstances]);
+  }, [dispatch, dashboard, matcherList, stateList, limitInstances, dataSourceName, promRulesRequests.loading]);
 
   const handleInstancesLimit = (limit: boolean) => {
     if (limit) {
-      dispatch(
-        fetchAllPromAndRulerRulesAction(false, {
-          limitAlerts: INSTANCES_DISPLAY_LIMIT,
-          matcher: matcherList,
-          state: stateList,
-        })
-      );
+      fetchPromAndRuler({ dispatch, limitInstances, matcherList, dataSourceName, stateList });
       toggleLimit(true);
     } else {
-      dispatch(
-        fetchAllPromAndRulerRulesAction(false, {
-          matcher: matcherList,
-          state: stateList,
-        })
-      );
+      fetchPromAndRuler({ dispatch, limitInstances: false, matcherList, dataSourceName, stateList });
       toggleLimit(false);
     }
   };
 
-  const { prom, ruler } = useUnifiedAlertingSelector((state) => ({
-    prom: state.promRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState,
-    ruler: state.rulerRules[GRAFANA_RULES_SOURCE_NAME] || initialAsyncRequestState,
-  }));
-
-  const loading = prom.loading || ruler.loading;
-  const haveResults = !!prom.result || !!ruler.result;
-
-  const promRulesRequests = useUnifiedAlertingSelector((state) => state.promRules);
-  const rulerRulesRequests = useUnifiedAlertingSelector((state) => state.rulerRules);
-
-  const somePromRulesDispatched = rulesDataSourceNames.some((name) => promRulesRequests[name]?.dispatched);
-
   //For grafana managed rules, get the result using RTK Query to avoid the need of using the redux store
   //See https://github.com/grafana/grafana/pull/70482
-  const { currentData: promRules = [], isLoading: grafanaRulesLoading } = usePrometheusRulesByNamespaceQuery({
-    limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
-    matcher: matcherList,
-    state: stateList,
-  });
+  const { currentData: promRules = [], isLoading: grafanaRulesLoading } = usePrometheusRulesByNamespaceQuery(
+    {
+      limitAlerts: limitInstances ? INSTANCES_DISPLAY_LIMIT : undefined,
+      matcher: matcherList,
+      state: stateList,
+    },
+    { skip: dataSourceName !== GRAFANA_RULES_SOURCE_NAME }
+  );
 
   const combinedRules = useCombinedRuleNamespaces(undefined, promRules);
 
-  const someRulerRulesDispatched = rulesDataSourceNames.some((name) => rulerRulesRequests[name]?.dispatched);
+  const someRulerRulesDispatched = isAsyncRequestMapSlicePartiallyDispatched(rulerRulesRequests);
+  const haveResults = isAsyncRequestMapSlicePartiallyFulfilled(promRulesRequests);
+
   const dispatched = somePromRulesDispatched || someRulerRulesDispatched;
+  const loading = isAsyncRequestMapSlicePending(promRulesRequests);
 
   const styles = useStyles2(getStyles);
 

diff --git a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx
@@ -89,7 +89,7 @@ const defaultOptions: UnifiedAlertListOptions = {
   folder: { id: 1, title: 'test folder' },
   stateFilter: { firing: true, pending: false, noData: false, normal: true, error: false },
   alertInstanceLabelFilter: '',
-  datasource: 'Alertmanager',
+  datasource: 'grafana',
   viewMode: ViewMode.List,
 };
 

diff --git a/public/app/plugins/panel/alertlist/module.tsx b/public/app/plugins/panel/alertlist/module.tsx
@@ -197,6 +197,7 @@ const unifiedAlertList = new PanelPlugin<UnifiedAlertListOptions>(UnifiedAlertLi
             id={props.id ?? 'groupBy'}
             defaultValue={props.value.map((value: string) => ({ label: value, value }))}
             onChange={props.onChange}
+            dataSource={props.context.options.datasource}
           />
         );
       },