From 5e42db4703a7132a83b4523a68a06973d7e9bfb4 Mon Sep 17 00:00:00 2001 From: Virginia Cepeda Date: Fri, 23 Jun 2023 17:03:19 -0300 Subject: [PATCH] [v10.0.x] Alerting: Display correct results when using different filters 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 f17c49e632775a6c96d6c9361d2a3c9c80ca9e67) --- .../alerting/unified/api/alertRuleApi.ts | 58 +++++++ .../alerting/unified/api/prometheus.ts | 40 ++--- .../hooks/useCombinedRuleNamespaces.ts | 14 +- .../app/features/alerting/unified/mockApi.ts | 141 ++++++++++++++++++ .../alerting/unified/mocks/alertRuleApi.ts | 10 ++ .../panel/alertlist/UnifiedAlertList.tsx | 17 ++- .../panel/alertlist/UnifiedalertList.test.tsx | 108 ++++++++++---- 7 files changed, 332 insertions(+), 56 deletions(-) create mode 100644 public/app/features/alerting/unified/api/alertRuleApi.ts create mode 100644 public/app/features/alerting/unified/mockApi.ts create mode 100644 public/app/features/alerting/unified/mocks/alertRuleApi.ts diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts new file mode 100644 index 000000000000..c35dc6cc8dda --- /dev/null +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -0,0 +1,58 @@ +import { Matcher } from 'app/plugins/datasource/alertmanager/types'; +import { RuleIdentifier, RuleNamespace } from 'app/types/unified-alerting'; +import { PromRulesResponse } from 'app/types/unified-alerting-dto'; + +import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; +import { isCloudRuleIdentifier, isPrometheusRuleIdentifier } from '../utils/rules'; + +import { alertingApi } from './alertingApi'; +import { + FetchPromRulesFilter, + groupRulesByFileName, + paramsWithMatcherAndState, + prepareRulesFilterQueryParams, +} from './prometheus'; +export interface Datasource { + type: string; + uid: string; +} + +export const PREVIEW_URL = '/api/v1/rule/test/grafana'; +export const PROM_RULES_URL = 'api/prometheus/grafana/api/v1/rules'; + +export const alertRuleApi = alertingApi.injectEndpoints({ + endpoints: (build) => ({ + prometheusRulesByNamespace: build.query< + RuleNamespace[], + { + limitAlerts?: number; + identifier?: RuleIdentifier; + filter?: FetchPromRulesFilter; + state?: string[]; + matcher?: Matcher[]; + } + >({ + query: ({ limitAlerts, identifier, filter, state, matcher }) => { + const searchParams = new URLSearchParams(); + + // if we're fetching for Grafana managed rules, we should add a limit to the number of alert instances + // we do this because the response is large otherwise and we don't show all of them in the UI anyway. + if (limitAlerts) { + searchParams.set('limit_alerts', String(limitAlerts)); + } + + if (identifier && (isPrometheusRuleIdentifier(identifier) || isCloudRuleIdentifier(identifier))) { + searchParams.set('file', identifier.namespace); + searchParams.set('rule_group', identifier.groupName); + } + + const params = prepareRulesFilterQueryParams(searchParams, filter); + + return { url: PROM_RULES_URL, params: paramsWithMatcherAndState(params, state, matcher) }; + }, + transformResponse: (response: PromRulesResponse): RuleNamespace[] => { + return groupRulesByFileName(response.data.groups, GRAFANA_RULES_SOURCE_NAME); + }, + }), + }), +}); diff --git a/public/app/features/alerting/unified/api/prometheus.ts b/public/app/features/alerting/unified/api/prometheus.ts index 2f7b8d05dbe8..75cd01f68c24 100644 --- a/public/app/features/alerting/unified/api/prometheus.ts +++ b/public/app/features/alerting/unified/api/prometheus.ts @@ -3,7 +3,7 @@ import { lastValueFrom } from 'rxjs'; import { getBackendSrv } from '@grafana/runtime'; import { Matcher } from 'app/plugins/datasource/alertmanager/types'; import { RuleIdentifier, RuleNamespace } from 'app/types/unified-alerting'; -import { PromRulesResponse } from 'app/types/unified-alerting-dto'; +import { PromRuleGroupDTO, PromRulesResponse } from 'app/types/unified-alerting-dto'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import { isCloudRuleIdentifier, isPrometheusRuleIdentifier } from '../utils/rules'; @@ -83,6 +83,26 @@ export function paramsWithMatcherAndState( return paramsResult; } +export const groupRulesByFileName = (groups: PromRuleGroupDTO[], dataSourceName: string) => { + const nsMap: { [key: string]: RuleNamespace } = {}; + groups.forEach((group) => { + group.rules.forEach((rule) => { + rule.query = rule.query || ''; + }); + if (!nsMap[group.file]) { + nsMap[group.file] = { + dataSourceName, + name: group.file, + groups: [group], + }; + } else { + nsMap[group.file].groups.push(group); + } + }); + + return Object.values(nsMap); +}; + export async function fetchRules( dataSourceName: string, filter?: FetchPromRulesFilter, @@ -116,21 +136,5 @@ export async function fetchRules( throw e; }); - const nsMap: { [key: string]: RuleNamespace } = {}; - response.data.data.groups.forEach((group) => { - group.rules.forEach((rule) => { - rule.query = rule.query || ''; - }); - if (!nsMap[group.file]) { - nsMap[group.file] = { - dataSourceName, - name: group.file, - groups: [group], - }; - } else { - nsMap[group.file].groups.push(group); - } - }); - - return Object.values(nsMap); + return groupRulesByFileName(response.data.data.groups, dataSourceName); } diff --git a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts index 9a903261b265..f0663ffdf14c 100644 --- a/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts +++ b/public/app/features/alerting/unified/hooks/useCombinedRuleNamespaces.ts @@ -24,6 +24,7 @@ import { import { getAllRulesSources, getRulesSourceByName, + GRAFANA_RULES_SOURCE_NAME, isCloudRulesSource, isGrafanaRulesSource, } from '../utils/datasource'; @@ -45,7 +46,10 @@ interface CacheValue { // this little monster combines prometheus rules and ruler rules to produce a unified data structure // can limit to a single rules source -export function useCombinedRuleNamespaces(rulesSourceName?: string): CombinedRuleNamespace[] { +export function useCombinedRuleNamespaces( + rulesSourceName?: string, + grafanaPromRuleNamespaces?: RuleNamespace[] +): CombinedRuleNamespace[] { const promRulesResponses = useUnifiedAlertingSelector((state) => state.promRules); const rulerRulesResponses = useUnifiedAlertingSelector((state) => state.rulerRules); @@ -67,9 +71,13 @@ export function useCombinedRuleNamespaces(rulesSourceName?: string): CombinedRul return rulesSources .map((rulesSource): CombinedRuleNamespace[] => { const rulesSourceName = isCloudRulesSource(rulesSource) ? rulesSource.name : rulesSource; - const promRules = promRulesResponses[rulesSourceName]?.result; const rulerRules = rulerRulesResponses[rulesSourceName]?.result; + let promRules = promRulesResponses[rulesSourceName]?.result; + if (rulesSourceName === GRAFANA_RULES_SOURCE_NAME && grafanaPromRuleNamespaces) { + promRules = grafanaPromRuleNamespaces; + } + const cached = cache.current[rulesSourceName]; if (cached && cached.promRules === promRules && cached.rulerRules === rulerRules) { return cached.result; @@ -104,7 +112,7 @@ export function useCombinedRuleNamespaces(rulesSourceName?: string): CombinedRul return result; }) .flat(); - }, [promRulesResponses, rulerRulesResponses, rulesSources]); + }, [promRulesResponses, rulerRulesResponses, rulesSources, grafanaPromRuleNamespaces]); } // merge all groups in case of grafana managed, essentially treating namespaces (folders) as groups diff --git a/public/app/features/alerting/unified/mockApi.ts b/public/app/features/alerting/unified/mockApi.ts new file mode 100644 index 000000000000..7ce3e754d485 --- /dev/null +++ b/public/app/features/alerting/unified/mockApi.ts @@ -0,0 +1,141 @@ +import { rest } from 'msw'; +import { setupServer, SetupServer } from 'msw/node'; +import 'whatwg-fetch'; + +import { setBackendSrv } from '@grafana/runtime'; + +import { backendSrv } from '../../../core/services/backend_srv'; +import { + AlertmanagerConfig, + AlertManagerCortexConfig, + EmailConfig, + MatcherOperator, + Receiver, + Route, +} from '../../../plugins/datasource/alertmanager/types'; + +class AlertmanagerConfigBuilder { + private alertmanagerConfig: AlertmanagerConfig = { receivers: [] }; + + addReceivers(configure: (builder: AlertmanagerReceiverBuilder) => void): AlertmanagerConfigBuilder { + const receiverBuilder = new AlertmanagerReceiverBuilder(); + configure(receiverBuilder); + this.alertmanagerConfig.receivers?.push(receiverBuilder.build()); + return this; + } + + withRoute(configure: (routeBuilder: AlertmanagerRouteBuilder) => void): AlertmanagerConfigBuilder { + const routeBuilder = new AlertmanagerRouteBuilder(); + configure(routeBuilder); + + this.alertmanagerConfig.route = routeBuilder.build(); + + return this; + } + + build() { + return this.alertmanagerConfig; + } +} + +class AlertmanagerRouteBuilder { + private route: Route = { routes: [], object_matchers: [] }; + + withReceiver(receiver: string): AlertmanagerRouteBuilder { + this.route.receiver = receiver; + return this; + } + withoutReceiver(): AlertmanagerRouteBuilder { + return this; + } + withEmptyReceiver(): AlertmanagerRouteBuilder { + this.route.receiver = ''; + return this; + } + + addRoute(configure: (builder: AlertmanagerRouteBuilder) => void): AlertmanagerRouteBuilder { + const routeBuilder = new AlertmanagerRouteBuilder(); + configure(routeBuilder); + this.route.routes?.push(routeBuilder.build()); + return this; + } + + addMatcher(key: string, operator: MatcherOperator, value: string): AlertmanagerRouteBuilder { + this.route.object_matchers?.push([key, operator, value]); + return this; + } + + build() { + return this.route; + } +} + +class EmailConfigBuilder { + private emailConfig: EmailConfig = { to: '' }; + + withTo(to: string): EmailConfigBuilder { + this.emailConfig.to = to; + return this; + } + + build() { + return this.emailConfig; + } +} + +class AlertmanagerReceiverBuilder { + private receiver: Receiver = { name: '', email_configs: [] }; + + withName(name: string): AlertmanagerReceiverBuilder { + this.receiver.name = name; + return this; + } + + addEmailConfig(configure: (builder: EmailConfigBuilder) => void): AlertmanagerReceiverBuilder { + const builder = new EmailConfigBuilder(); + configure(builder); + this.receiver.email_configs?.push(builder.build()); + return this; + } + + build() { + return this.receiver; + } +} + +export function mockApi(server: SetupServer) { + return { + getAlertmanagerConfig: (amName: string, configure: (builder: AlertmanagerConfigBuilder) => void) => { + const builder = new AlertmanagerConfigBuilder(); + configure(builder); + + server.use( + rest.get(`api/alertmanager/${amName}/config/api/v1/alerts`, (req, res, ctx) => + res( + ctx.status(200), + ctx.json({ + alertmanager_config: builder.build(), + template_files: {}, + }) + ) + ) + ); + }, + }; +} + +// Creates a MSW server and sets up beforeAll and afterAll handlers for it +export function setupMswServer() { + const server = setupServer(); + + beforeAll(() => { + setBackendSrv(backendSrv); + server.listen({ onUnhandledRequest: 'error' }); + }); + + afterAll(() => { + server.close(); + }); + + return server; +} diff --git a/public/app/features/alerting/unified/mocks/alertRuleApi.ts b/public/app/features/alerting/unified/mocks/alertRuleApi.ts new file mode 100644 index 000000000000..17eac42895d4 --- /dev/null +++ b/public/app/features/alerting/unified/mocks/alertRuleApi.ts @@ -0,0 +1,10 @@ +import { rest } from 'msw'; +import { SetupServer } from 'msw/node'; + +import { PromRulesResponse } from 'app/types/unified-alerting-dto'; + +import { PROM_RULES_URL } from '../api/alertRuleApi'; + +export function mockPromRulesApiResponse(server: SetupServer, result: PromRulesResponse) { + server.use(rest.get(PROM_RULES_URL, (req, res, ctx) => res(ctx.json(result)))); +} diff --git a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx index da19f2c8352e..93486f5be17f 100644 --- a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx @@ -18,6 +18,7 @@ import { import { config } from 'app/core/config'; import { contextSrv } from 'app/core/services/context_srv'; import alertDef from 'app/features/alerting/state/alertDef'; +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'; @@ -60,6 +61,8 @@ export function UnifiedAlertList(props: PanelProps) { const rulesDataSourceNames = useMemo(getAllRulesSourceNames, []); const [limitInstances, toggleLimit] = useToggle(true); + const { usePrometheusRulesByNamespaceQuery } = alertRuleApi; + // backwards compat for "Inactive" state filter useEffect(() => { if (props.options.stateFilter.inactive === true) { @@ -153,9 +156,19 @@ export function UnifiedAlertList(props: PanelProps) { const promRulesRequests = useUnifiedAlertingSelector((state) => state.promRules); const rulerRulesRequests = useUnifiedAlertingSelector((state) => state.rulerRules); - const combinedRules = useCombinedRuleNamespaces(); 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 combinedRules = useCombinedRuleNamespaces(undefined, promRules); + const someRulerRulesDispatched = rulesDataSourceNames.some((name) => rulerRulesRequests[name]?.dispatched); const dispatched = somePromRulesDispatched || someRulerRulesDispatched; @@ -183,7 +196,7 @@ export function UnifiedAlertList(props: PanelProps) { return (
- {dispatched && loading && !haveResults && } + {(grafanaRulesLoading || (dispatched && loading && !haveResults)) && } {noAlertsMessage &&
{noAlertsMessage}
}
{props.options.viewMode === ViewMode.Stat && haveResults && ( diff --git a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx index 1ff3972f11c7..b0c9cb8242cc 100644 --- a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx @@ -2,11 +2,17 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { Provider } from 'react-redux'; +import { act } from 'react-test-renderer'; import { byRole, byText } from 'testing-library-selector'; import { FieldConfigSource, getDefaultTimeRange, LoadingState, PanelProps } from '@grafana/data'; import { TimeRangeUpdatedEvent } from '@grafana/runtime'; +import { setupMswServer } from 'app/features/alerting/unified/mockApi'; +import { mockPromRulesApiResponse } from 'app/features/alerting/unified/mocks/alertRuleApi'; +import { mockRulerRulesApiResponse } from 'app/features/alerting/unified/mocks/rulerApi'; +import { Annotation } from 'app/features/alerting/unified/utils/constants'; import { DashboardSrv, setDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; +import { PromRuleGroupDTO, PromRulesResponse, RulerGrafanaRuleDTO } from 'app/types/unified-alerting-dto'; import { contextSrv } from '../../../core/services/context_srv'; import { @@ -14,6 +20,7 @@ import { mockPromAlertingRule, mockPromRuleGroup, mockPromRuleNamespace, + mockRulerGrafanaRule, mockUnifiedAlertingStore, } from '../../../features/alerting/unified/mocks'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../features/alerting/unified/utils/datasource'; @@ -22,8 +29,55 @@ import { UnifiedAlertList } from './UnifiedAlertList'; import { GroupMode, SortOrder, UnifiedAlertListOptions, ViewMode } from './types'; import * as utils from './util'; +const grafanaRuleMock = { + promRules: { + grafana: { + loading: false, + dispatched: true, + result: [ + mockPromRuleNamespace({ + name: 'ns1', + groups: [ + mockPromRuleGroup({ + name: 'group1', + rules: [ + mockPromAlertingRule({ + name: 'rule1', + alerts: [mockPromAlert({ labels: { severity: 'critical' } })], + totals: { alerting: 1 }, + totalsFiltered: { alerting: 1 }, + }), + ], + }), + ], + }), + ], + }, + }, +}; + jest.mock('app/features/alerting/unified/api/alertmanager'); +const fakeResponse: PromRulesResponse = { + data: { groups: grafanaRuleMock.promRules.grafana.result[0].groups as PromRuleGroupDTO[] }, + status: 'success', +}; + +const server = setupMswServer(); + +mockPromRulesApiResponse(server, fakeResponse); +const originRule: RulerGrafanaRuleDTO = mockRulerGrafanaRule( + { + for: '1m', + labels: { severity: 'critical', region: 'nasa' }, + annotations: { [Annotation.summary]: 'This is a very important alert rule' }, + }, + { uid: 'grafana-rule-1', title: 'First Grafana Rule', data: [] } +); +mockRulerRulesApiResponse(server, 'grafana', { + 'folder-one': [{ name: 'group1', interval: '20s', rules: [originRule] }], +}); + const defaultOptions: UnifiedAlertListOptions = { maxItems: 2, sortOrder: SortOrder.AlphaAsc, @@ -73,32 +127,7 @@ const dashboard = { }; const renderPanel = (options: Partial = defaultOptions) => { - const store = mockUnifiedAlertingStore({ - promRules: { - grafana: { - loading: false, - dispatched: true, - result: [ - mockPromRuleNamespace({ - name: 'ns1', - groups: [ - mockPromRuleGroup({ - name: 'group1', - rules: [ - mockPromAlertingRule({ - name: 'rule1', - alerts: [mockPromAlert({ labels: { severity: 'critical' } })], - totals: { alerting: 1 }, - totalsFiltered: { alerting: 1 }, - }), - ], - }), - ], - }), - ], - }, - }, - }); + const store = mockUnifiedAlertingStore(grafanaRuleMock); const dashSrv: unknown = { getCurrent: () => dashboard }; setDashboardSrv(dashSrv as DashboardSrv); @@ -115,12 +144,19 @@ const renderPanel = (options: Partial = defaultOptions) describe('UnifiedAlertList', () => { it('subscribes to the dashboard refresh interval', async () => { jest.spyOn(defaultProps, 'replaceVariables').mockReturnValue('severity=critical'); - await renderPanel(); + + await act(async () => { + renderPanel(); + }); + expect(dashboard.events.subscribe).toHaveBeenCalledTimes(1); expect(dashboard.events.subscribe.mock.calls[0][0]).toEqual(TimeRangeUpdatedEvent); }); it('should replace option variables before filtering', async () => { + await waitFor(() => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + }); jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true); const filterAlertsSpy = jest.spyOn(utils, 'filterAlerts'); @@ -128,12 +164,18 @@ describe('UnifiedAlertList', () => { const user = userEvent.setup(); - await renderPanel({ - alertInstanceLabelFilter: '$label', - dashboardAlerts: false, - alertName: '', - datasource: GRAFANA_RULES_SOURCE_NAME, - folder: undefined, + await act(async () => { + renderPanel({ + alertInstanceLabelFilter: '$label', + dashboardAlerts: false, + alertName: '', + datasource: GRAFANA_RULES_SOURCE_NAME, + folder: undefined, + }); + }); + + await waitFor(() => { + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); }); expect(byText('rule1').get()).toBeInTheDocument();