From f7b8a666f4ce78e6f44fe78148e605dcc515ab14 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Mon, 15 May 2023 16:53:03 +0200 Subject: [PATCH] Alerting: Enable alerts preview on notification policies page (#68291) * Basic implementation in web worker * Move instances discovery to the worker * Remove filtering from the worker * Use normalized routes, use rtk query for alert groups fetching * Reorganize matchers utilities to be available for web workers * Move object matchers to the machers util file, rename worker * Move worker code to a separate hook, add perf logging * Add a mock for the web worker code, fix tests * Fix tests warnings * Remove notification policy feature flag * Add normalizeRoute tests, change the regex match to test for label matching * Move worker init to the file scope * Simplify useAsyncFn hook --- .../unified/NotificationPolicies.test.tsx | 21 ++- .../alerting/unified/NotificationPolicies.tsx | 42 +++--- .../__mocks__/useRouteGroupsMatcher.ts | 20 +++ .../alerting/unified/api/alertmanagerApi.ts | 7 + .../notification-policies/Policy.tsx | 41 +++--- .../app/features/alerting/unified/features.ts | 11 +- .../unified/hooks/useFilteredRules.ts | 3 +- .../unified/routeGroupsMatcher.worker.ts | 27 ++++ .../alerting/unified/useRouteGroupsMatcher.ts | 31 +++++ .../unified/utils/alertmanager.test.ts | 9 +- .../alerting/unified/utils/alertmanager.ts | 35 ----- .../alerting/unified/utils/amroutes.test.ts | 29 +---- .../alerting/unified/utils/amroutes.ts | 51 +------- .../alerting/unified/utils/matchers.test.ts | 29 ++++- .../alerting/unified/utils/matchers.ts | 121 +++++++++++++++++- .../utils/notification-policies.test.ts | 44 ++++++- .../unified/utils/notification-policies.ts | 74 ++++------- 17 files changed, 376 insertions(+), 219 deletions(-) create mode 100644 public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts create mode 100644 public/app/features/alerting/unified/routeGroupsMatcher.worker.ts create mode 100644 public/app/features/alerting/unified/useRouteGroupsMatcher.ts diff --git a/public/app/features/alerting/unified/NotificationPolicies.test.tsx b/public/app/features/alerting/unified/NotificationPolicies.test.tsx index 3698e5abc9d..fc80a85b6da 100644 --- a/public/app/features/alerting/unified/NotificationPolicies.test.tsx +++ b/public/app/features/alerting/unified/NotificationPolicies.test.tsx @@ -1,4 +1,4 @@ -import { render, waitFor } from '@testing-library/react'; +import { render, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { TestProvider } from 'test/helpers/TestProvider'; @@ -20,6 +20,7 @@ import { AccessControlAction } from 'app/types'; import NotificationPolicies, { findRoutesMatchingFilters } from './NotificationPolicies'; import { fetchAlertManagerConfig, fetchStatus, updateAlertManagerConfig } from './api/alertmanager'; +import { alertmanagerApi } from './api/alertmanagerApi'; import { discoverAlertmanagerFeatures } from './api/buildInfo'; import * as grafanaApp from './components/receivers/grafanaAppReceivers/grafanaApp'; import { mockDataSource, MockDataSourceSrv, someCloudAlertManagerConfig, someCloudAlertManagerStatus } from './mocks'; @@ -32,6 +33,7 @@ jest.mock('./api/alertmanager'); jest.mock('./utils/config'); jest.mock('app/core/services/context_srv'); jest.mock('./api/buildInfo'); +jest.mock('./useRouteGroupsMatcher'); const mocks = { getAllDataSourcesMock: jest.mocked(getAllDataSources), @@ -388,6 +390,7 @@ describe('NotificationPolicies', () => { renderNotificationPolicies(); await waitFor(() => expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(1)); + expect(ui.newPolicyButton.query()).not.toBeInTheDocument(); expect(ui.editButton.query()).not.toBeInTheDocument(); }); @@ -399,6 +402,12 @@ describe('NotificationPolicies', () => { message: "Alertmanager has exploded. it's gone. Forget about it.", }, }); + + jest.spyOn(alertmanagerApi, 'useGetAlertmanagerAlertGroupsQuery').mockImplementation(() => ({ + currentData: [], + refetch: jest.fn(), + })); + await renderNotificationPolicies(); await waitFor(() => expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(1)); expect(await byText("Alertmanager has exploded. it's gone. Forget about it.").find()).toBeInTheDocument(); @@ -630,8 +639,18 @@ describe('NotificationPolicies', () => { }, }, }); + + jest.spyOn(alertmanagerApi, 'useGetAlertmanagerAlertGroupsQuery').mockImplementation(() => ({ + currentData: [], + refetch: jest.fn(), + })); + await renderNotificationPolicies(dataSources.promAlertManager.name); const rootRouteContainer = await ui.rootRouteContainer.find(); + await waitFor(() => + expect(within(rootRouteContainer).getByTestId('matching-instances')).toHaveTextContent('0instances') + ); + expect(ui.editButton.query(rootRouteContainer)).not.toBeInTheDocument(); expect(ui.newPolicyCTAButton.query()).not.toBeInTheDocument(); expect(mocks.api.fetchAlertManagerConfig).not.toHaveBeenCalled(); diff --git a/public/app/features/alerting/unified/NotificationPolicies.tsx b/public/app/features/alerting/unified/NotificationPolicies.tsx index d213921829d..e80701b8227 100644 --- a/public/app/features/alerting/unified/NotificationPolicies.tsx +++ b/public/app/features/alerting/unified/NotificationPolicies.tsx @@ -1,6 +1,7 @@ import { css } from '@emotion/css'; import { intersectionBy, isEqual } from 'lodash'; import React, { useEffect, useMemo, useState } from 'react'; +import { useAsyncFn } from 'react-use'; import { GrafanaTheme2, UrlQueryMap } from '@grafana/data'; import { Stack } from '@grafana/experimental'; @@ -11,6 +12,7 @@ import { useDispatch } from 'app/types'; import { useCleanup } from '../../../core/hooks/useCleanup'; +import { alertmanagerApi } from './api/alertmanagerApi'; import { useGetContactPointsState } from './api/receiversApi'; import { AlertManagerPicker } from './components/AlertManagerPicker'; import { AlertingPageWrapper } from './components/AlertingPageWrapper'; @@ -33,10 +35,12 @@ import { Policy } from './components/notification-policies/Policy'; import { useAlertManagerSourceName } from './hooks/useAlertManagerSourceName'; import { useAlertManagersByPermission } from './hooks/useAlertManagerSources'; import { useUnifiedAlertingSelector } from './hooks/useUnifiedAlertingSelector'; -import { fetchAlertGroupsAction, fetchAlertManagerConfigAction, updateAlertManagerConfigAction } from './state/actions'; +import { fetchAlertManagerConfigAction, updateAlertManagerConfigAction } from './state/actions'; import { FormAmRoute } from './types/amroutes'; -import { addUniqueIdentifierToRoute, normalizeMatchers } from './utils/amroutes'; +import { useRouteGroupsMatcher } from './useRouteGroupsMatcher'; +import { addUniqueIdentifierToRoute } from './utils/amroutes'; import { isVanillaPrometheusAlertManagerDataSource } from './utils/datasource'; +import { normalizeMatchers } from './utils/matchers'; import { initialAsyncRequestState } from './utils/redux'; import { addRouteToParentRoute, mergePartialAmRouteWithRouteTree, omitRouteFromRouteTree } from './utils/routeTree'; @@ -48,6 +52,7 @@ enum ActiveTab { const AmRoutes = () => { const dispatch = useDispatch(); const styles = useStyles2(getStyles); + const { useGetAlertmanagerAlertGroupsQuery } = alertmanagerApi; const [queryParams, setQueryParams] = useQueryParams(); const { tab } = getActiveTabFromUrl(queryParams); @@ -57,6 +62,8 @@ const AmRoutes = () => { const [contactPointFilter, setContactPointFilter] = useState(); const [labelMatchersFilter, setLabelMatchersFilter] = useState([]); + const { getRouteGroupsMap } = useRouteGroupsMatcher(); + const alertManagers = useAlertManagersByPermission('notification'); const [alertManagerSourceName, setAlertManagerSourceName] = useAlertManagerSourceName(alertManagers); @@ -69,6 +76,11 @@ const AmRoutes = () => { } }, [alertManagerSourceName, dispatch]); + const { currentData: alertGroups, refetch: refetchAlertGroups } = useGetAlertmanagerAlertGroupsQuery( + { amSourceName: alertManagerSourceName ?? '' }, + { skip: !alertManagerSourceName } + ); + const { result, loading: resultLoading, @@ -82,10 +94,19 @@ const AmRoutes = () => { if (config?.route) { return addUniqueIdentifierToRoute(config.route); } - return; }, [config?.route]); + // useAsync could also work but it's hard to wait until it's done in the tests + // Combining with useEffect gives more predictable results because the condition is in useEffect + const [{ value: routeAlertGroupsMap }, triggerGetRouteGroupsMap] = useAsyncFn(getRouteGroupsMap, []); + + useEffect(() => { + if (rootRoute && alertGroups) { + triggerGetRouteGroupsMap(rootRoute, alertGroups); + } + }, [rootRoute, alertGroups, triggerGetRouteGroupsMap]); + // these are computed from the contactPoint and labels matchers filter const routesMatchingFilters = useMemo(() => { if (!rootRoute) { @@ -96,9 +117,6 @@ const AmRoutes = () => { const isProvisioned = Boolean(config?.route?.provenance); - const alertGroups = useUnifiedAlertingSelector((state) => state.amAlertGroups); - const fetchAlertGroups = alertGroups[alertManagerSourceName || ''] ?? initialAsyncRequestState; - function handleSave(partialRoute: Partial) { if (!rootRoute) { return; @@ -149,7 +167,7 @@ const AmRoutes = () => { .unwrap() .then(() => { if (alertManagerSourceName) { - dispatch(fetchAlertGroupsAction(alertManagerSourceName)); + refetchAlertGroups(); } closeEditModal(); closeAddModal(); @@ -173,13 +191,6 @@ const AmRoutes = () => { useCleanup((state) => (state.unifiedAlerting.saveAMConfig = initialAsyncRequestState)); - // fetch AM instances grouping - useEffect(() => { - if (alertManagerSourceName) { - dispatch(fetchAlertGroupsAction(alertManagerSourceName)); - } - }, [alertManagerSourceName, dispatch]); - if (!alertManagerSourceName) { return ( @@ -252,7 +263,7 @@ const AmRoutes = () => { receivers={receivers} routeTree={rootRoute} currentRoute={rootRoute} - alertGroups={fetchAlertGroups.result} + alertGroups={alertGroups ?? []} contactPointsState={contactPointsState.receivers} readOnly={readOnlyPolicies} alertManagerSourceName={alertManagerSourceName} @@ -261,6 +272,7 @@ const AmRoutes = () => { onDeletePolicy={openDeleteModal} onShowAlertInstances={showAlertGroupsModal} routesMatchingFilters={routesMatchingFilters} + routeAlertGroupsMap={routeAlertGroupsMap} /> )} diff --git a/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts b/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts new file mode 100644 index 00000000000..94ac4ea7a2d --- /dev/null +++ b/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts @@ -0,0 +1,20 @@ +import { useCallback } from 'react'; + +import { AlertmanagerGroup, RouteWithID } from '../../../../plugins/datasource/alertmanager/types'; + +export function useRouteGroupsMatcher() { + const getRouteGroupsMap = useCallback(async (route: RouteWithID, __: AlertmanagerGroup[]) => { + const groupsMap = new Map(); + function addRoutes(route: RouteWithID) { + groupsMap.set(route.id, []); + + route.routes?.forEach((r) => addRoutes(r)); + } + + addRoutes(route); + + return groupsMap; + }, []); + + return { getRouteGroupsMap }; +} diff --git a/public/app/features/alerting/unified/api/alertmanagerApi.ts b/public/app/features/alerting/unified/api/alertmanagerApi.ts index a761134eaf9..bd83e5071bd 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -2,6 +2,7 @@ import { AlertmanagerAlert, AlertmanagerChoice, AlertManagerCortexConfig, + AlertmanagerGroup, ExternalAlertmanagerConfig, ExternalAlertmanagers, ExternalAlertmanagersResponse, @@ -61,6 +62,12 @@ export const alertmanagerApi = alertingApi.injectEndpoints({ }, }), + getAlertmanagerAlertGroups: build.query({ + query: ({ amSourceName }) => ({ + url: `/api/alertmanager/${getDatasourceAPIUid(amSourceName)}/api/v2/alerts/groups`, + }), + }), + getAlertmanagerChoiceStatus: build.query({ query: () => ({ url: '/api/v1/ngalert' }), providesTags: ['AlertmanagerChoice'], diff --git a/public/app/features/alerting/unified/components/notification-policies/Policy.tsx b/public/app/features/alerting/unified/components/notification-policies/Policy.tsx index 6fbeba09612..330ab24c1aa 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Policy.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Policy.tsx @@ -1,8 +1,7 @@ import { css } from '@emotion/css'; import { uniqueId, pick, groupBy, upperFirst, merge, reduce, sumBy } from 'lodash'; import pluralize from 'pluralize'; -import React, { FC, Fragment, ReactNode, useMemo } from 'react'; -import { useEnabled } from 'react-enable'; +import React, { FC, Fragment, ReactNode } from 'react'; import { Link } from 'react-router-dom'; import { GrafanaTheme2, IconName } from '@grafana/data'; @@ -18,11 +17,9 @@ import { } from 'app/plugins/datasource/alertmanager/types'; import { ReceiversState } from 'app/types'; -import { AlertingFeature } from '../../features'; import { getNotificationsPermissions } from '../../utils/access-control'; -import { normalizeMatchers } from '../../utils/amroutes'; +import { normalizeMatchers } from '../../utils/matchers'; import { createContactPointLink, createMuteTimingLink } from '../../utils/misc'; -import { findMatchingAlertGroups } from '../../utils/notification-policies'; import { HoverCard } from '../HoverCard'; import { Label } from '../Label'; import { MetaText } from '../MetaText'; @@ -44,6 +41,7 @@ interface PolicyComponentProps { readOnly?: boolean; inheritedProperties?: InhertitableProperties; routesMatchingFilters?: RouteWithID[]; + routeAlertGroupsMap?: Map; routeTree: RouteWithID; currentRoute: RouteWithID; @@ -64,6 +62,7 @@ const Policy: FC = ({ routeTree, inheritedProperties, routesMatchingFilters = [], + routeAlertGroupsMap, onEditPolicy, onAddPolicy, onDeletePolicy, @@ -71,7 +70,6 @@ const Policy: FC = ({ }) => { const styles = useStyles2(getStyles); const isDefaultPolicy = currentRoute === routeTree; - const showMatchingInstances = useEnabled(AlertingFeature.NotificationPoliciesV2MatchingInstances); const permissions = getNotificationsPermissions(alertManagerSourceName); const canEditRoutes = contextSrv.hasPermission(permissions.update); @@ -114,12 +112,12 @@ const Policy: FC = ({ const isEditable = canEditRoutes; const isDeletable = canDeleteRoutes && !isDefaultPolicy; - const matchingAlertGroups = useMemo(() => { - return showMatchingInstances ? findMatchingAlertGroups(routeTree, currentRoute, alertGroups) : []; - }, [alertGroups, currentRoute, routeTree, showMatchingInstances]); + const matchingAlertGroups = routeAlertGroupsMap?.get(currentRoute.id); // sum all alert instances for all groups we're handling - const numberOfAlertInstances = sumBy(matchingAlertGroups, (group) => group.alerts.length); + const numberOfAlertInstances = matchingAlertGroups + ? sumBy(matchingAlertGroups, (group) => group.alerts.length) + : undefined; // TODO dead branch detection, warnings for all sort of configs that won't work or will never be activated return ( @@ -196,18 +194,16 @@ const Policy: FC = ({ {/* Metadata row */}
- {showMatchingInstances && ( - { - onShowAlertInstances(matchingAlertGroups, matchers); - }} - data-testid="matching-instances" - > - {numberOfAlertInstances} - {pluralize('instance', numberOfAlertInstances)} - - )} + { + matchingAlertGroups && onShowAlertInstances(matchingAlertGroups, matchers); + }} + data-testid="matching-instances" + > + {numberOfAlertInstances ?? '-'} + {pluralize('instance', numberOfAlertInstances)} + {contactPoint && ( Delivered to @@ -298,6 +294,7 @@ const Policy: FC = ({ alertManagerSourceName={alertManagerSourceName} alertGroups={alertGroups} routesMatchingFilters={routesMatchingFilters} + routeAlertGroupsMap={routeAlertGroupsMap} /> ); })} diff --git a/public/app/features/alerting/unified/features.ts b/public/app/features/alerting/unified/features.ts index d94f1848ee2..b0493878fc0 100644 --- a/public/app/features/alerting/unified/features.ts +++ b/public/app/features/alerting/unified/features.ts @@ -1,14 +1,7 @@ import { FeatureDescription } from 'react-enable/dist/FeatureState'; -export enum AlertingFeature { - NotificationPoliciesV2MatchingInstances = 'notification-policies.v2.matching-instances', -} +export enum AlertingFeature {} -const FEATURES: FeatureDescription[] = [ - { - name: AlertingFeature.NotificationPoliciesV2MatchingInstances, - defaultValue: false, - }, -]; +const FEATURES: FeatureDescription[] = []; export default FEATURES; diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.ts index aea4ba65c3e..228eea3dc78 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.ts @@ -9,8 +9,9 @@ import { CombinedRuleGroup, CombinedRuleNamespace, Rule } from 'app/types/unifie import { isPromAlertingRuleState, PromRuleType, RulerGrafanaRuleDTO } from 'app/types/unified-alerting-dto'; import { applySearchFilterToQuery, getSearchFilterFromQuery, RulesFilter } from '../search/rulesSearchParser'; -import { labelsMatchMatchers, matcherToMatcherField, parseMatcher, parseMatchers } from '../utils/alertmanager'; +import { labelsMatchMatchers, matcherToMatcherField, parseMatchers } from '../utils/alertmanager'; import { isCloudRulesSource } from '../utils/datasource'; +import { parseMatcher } from '../utils/matchers'; import { getRuleHealth, isAlertingRule, isGrafanaRulerRule, isPromRuleType } from '../utils/rules'; import { calculateGroupTotals, calculateRuleFilteredTotals, calculateRuleTotals } from './useCombinedRuleNamespaces'; diff --git a/public/app/features/alerting/unified/routeGroupsMatcher.worker.ts b/public/app/features/alerting/unified/routeGroupsMatcher.worker.ts new file mode 100644 index 00000000000..f4eac245b3a --- /dev/null +++ b/public/app/features/alerting/unified/routeGroupsMatcher.worker.ts @@ -0,0 +1,27 @@ +import * as comlink from 'comlink'; + +import type { AlertmanagerGroup, RouteWithID } from '../../../plugins/datasource/alertmanager/types'; + +import { findMatchingAlertGroups, normalizeRoute } from './utils/notification-policies'; + +const routeGroupsMatcher = { + getRouteGroupsMap(rootRoute: RouteWithID, groups: AlertmanagerGroup[]): Map { + const normalizedRootRoute = normalizeRoute(rootRoute); + + function addRouteGroups(route: RouteWithID, acc: Map) { + const routeGroups = findMatchingAlertGroups(normalizedRootRoute, route, groups); + acc.set(route.id, routeGroups); + + route.routes?.forEach((r) => addRouteGroups(r, acc)); + } + + const routeGroupsMap = new Map(); + addRouteGroups(normalizedRootRoute, routeGroupsMap); + + return routeGroupsMap; + }, +}; + +export type RouteGroupsMatcher = typeof routeGroupsMatcher; + +comlink.expose(routeGroupsMatcher); diff --git a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts new file mode 100644 index 00000000000..23e11707421 --- /dev/null +++ b/public/app/features/alerting/unified/useRouteGroupsMatcher.ts @@ -0,0 +1,31 @@ +import * as comlink from 'comlink'; +import { useCallback } from 'react'; + +import { AlertmanagerGroup, RouteWithID } from '../../../plugins/datasource/alertmanager/types'; + +import { logInfo } from './Analytics'; +import type { RouteGroupsMatcher } from './routeGroupsMatcher.worker'; + +const worker = new Worker(new URL('./routeGroupsMatcher.worker.ts', import.meta.url), { type: 'module' }); +const routeMatcher = comlink.wrap(worker); + +export function useRouteGroupsMatcher() { + const getRouteGroupsMap = useCallback(async (rootRoute: RouteWithID, alertGroups: AlertmanagerGroup[]) => { + const startTime = performance.now(); + + const result = await routeMatcher.getRouteGroupsMap(rootRoute, alertGroups); + + const timeSpent = performance.now() - startTime; + + logInfo(`Route Groups Matched in ${timeSpent} ms`, { + matchingTime: timeSpent.toString(), + alertGroupsCount: alertGroups.length.toString(), + // Counting all nested routes might be too time-consuming, so we only count the first level + topLevelRoutesCount: rootRoute.routes?.length.toString() ?? '0', + }); + + return result; + }, []); + + return { getRouteGroupsMap }; +} diff --git a/public/app/features/alerting/unified/utils/alertmanager.test.ts b/public/app/features/alerting/unified/utils/alertmanager.test.ts index 2fd657a6a1b..30954e9b35c 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.test.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.test.ts @@ -1,13 +1,8 @@ import { Matcher, MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from 'app/types/unified-alerting-dto'; -import { - parseMatcher, - parseMatchers, - labelsMatchMatchers, - removeMuteTimingFromRoute, - matchersToString, -} from './alertmanager'; +import { parseMatchers, labelsMatchMatchers, removeMuteTimingFromRoute, matchersToString } from './alertmanager'; +import { parseMatcher } from './matchers'; describe('Alertmanager utils', () => { describe('parseMatcher', () => { diff --git a/public/app/features/alerting/unified/utils/alertmanager.ts b/public/app/features/alerting/unified/utils/alertmanager.ts index 96a12752b91..b75671506fd 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.ts @@ -126,41 +126,6 @@ export const matcherFieldOptions: SelectableValue[] = [ { label: MatcherOperator.notRegex, description: 'Does not match regex', value: MatcherOperator.notRegex }, ]; -const matcherOperators = [ - MatcherOperator.regex, - MatcherOperator.notRegex, - MatcherOperator.notEqual, - MatcherOperator.equal, -]; - -export function parseMatcher(matcher: string): Matcher { - const trimmed = matcher.trim(); - if (trimmed.startsWith('{') && trimmed.endsWith('}')) { - throw new Error(`PromQL matchers not supported yet, sorry! PromQL matcher found: ${trimmed}`); - } - const operatorsFound = matcherOperators - .map((op): [MatcherOperator, number] => [op, trimmed.indexOf(op)]) - .filter(([_, idx]) => idx > -1) - .sort((a, b) => a[1] - b[1]); - - if (!operatorsFound.length) { - throw new Error(`Invalid matcher: ${trimmed}`); - } - const [operator, idx] = operatorsFound[0]; - const name = trimmed.slice(0, idx).trim(); - const value = trimmed.slice(idx + operator.length).trim(); - if (!name) { - throw new Error(`Invalid matcher: ${trimmed}`); - } - - return { - name, - value, - isRegex: operator === MatcherOperator.regex || operator === MatcherOperator.notRegex, - isEqual: operator === MatcherOperator.equal || operator === MatcherOperator.regex, - }; -} - export function matcherToObjectMatcher(matcher: Matcher): ObjectMatcher { const operator = matcherToOperator(matcher); return [matcher.name, operator, matcher.value]; diff --git a/public/app/features/alerting/unified/utils/amroutes.test.ts b/public/app/features/alerting/unified/utils/amroutes.test.ts index 8895125b8b1..13538a666db 100644 --- a/public/app/features/alerting/unified/utils/amroutes.test.ts +++ b/public/app/features/alerting/unified/utils/amroutes.test.ts @@ -1,8 +1,8 @@ -import { MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; +import { Route } from 'app/plugins/datasource/alertmanager/types'; import { FormAmRoute } from '../types/amroutes'; -import { amRouteToFormAmRoute, emptyRoute, formAmRouteToAmRoute, normalizeMatchers } from './amroutes'; +import { amRouteToFormAmRoute, emptyRoute, formAmRouteToAmRoute } from './amroutes'; const emptyAmRoute: Route = { receiver: '', @@ -89,28 +89,3 @@ describe('amRouteToFormAmRoute', () => { }); }); }); - -describe('normalizeMatchers', () => { - const eq = MatcherOperator.equal; - - it('should work for object_matchers', () => { - const route: Route = { object_matchers: [['foo', eq, 'bar']] }; - expect(normalizeMatchers(route)).toEqual([['foo', eq, 'bar']]); - }); - it('should work for matchers', () => { - const route: Route = { matchers: ['foo=bar', 'foo!=bar', 'foo=~bar', 'foo!~bar'] }; - expect(normalizeMatchers(route)).toEqual([ - ['foo', MatcherOperator.equal, 'bar'], - ['foo', MatcherOperator.notEqual, 'bar'], - ['foo', MatcherOperator.regex, 'bar'], - ['foo', MatcherOperator.notRegex, 'bar'], - ]); - }); - it('should work for match and match_re', () => { - const route: Route = { match: { foo: 'bar' }, match_re: { foo: 'bar' } }; - expect(normalizeMatchers(route)).toEqual([ - ['foo', MatcherOperator.regex, 'bar'], - ['foo', MatcherOperator.equal, 'bar'], - ]); - }); -}); diff --git a/public/app/features/alerting/unified/utils/amroutes.ts b/public/app/features/alerting/unified/utils/amroutes.ts index 0c39d0d2139..7fbb401e007 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -6,8 +6,9 @@ import { MatcherOperator, ObjectMatcher, Route, RouteWithID } from 'app/plugins/ import { FormAmRoute } from '../types/amroutes'; import { MatcherFieldValue } from '../types/silence-form'; -import { matcherToMatcherField, parseMatcher } from './alertmanager'; +import { matcherToMatcherField } from './alertmanager'; import { GRAFANA_RULES_SOURCE_NAME } from './datasource'; +import { normalizeMatchers, parseMatcher } from './matchers'; import { findExistingRoute } from './routeTree'; import { isValidPrometheusDuration } from './time'; @@ -63,54 +64,6 @@ export const emptyRoute: FormAmRoute = { muteTimeIntervals: [], }; -/** - * We need to deal with multiple (deprecated) properties such as "match" and "match_re" - * this function will normalize all of the different ways to define matchers in to a single one. - */ -export const normalizeMatchers = (route: Route): ObjectMatcher[] => { - const matchers: ObjectMatcher[] = []; - - if (route.matchers) { - route.matchers.forEach((matcher) => { - const { name, value, isEqual, isRegex } = parseMatcher(matcher); - let operator = MatcherOperator.equal; - - if (isEqual && isRegex) { - operator = MatcherOperator.regex; - } - if (!isEqual && isRegex) { - operator = MatcherOperator.notRegex; - } - if (isEqual && !isRegex) { - operator = MatcherOperator.equal; - } - if (!isEqual && !isRegex) { - operator = MatcherOperator.notEqual; - } - - matchers.push([name, operator, value]); - }); - } - - if (route.object_matchers) { - matchers.push(...route.object_matchers); - } - - if (route.match_re) { - Object.entries(route.match_re).forEach(([label, value]) => { - matchers.push([label, MatcherOperator.regex, value]); - }); - } - - if (route.match) { - Object.entries(route.match).forEach(([label, value]) => { - matchers.push([label, MatcherOperator.equal, value]); - }); - } - - return matchers; -}; - // add unique identifiers to each route in the route tree, that way we can figure out what route we've edited / deleted export function addUniqueIdentifierToRoute(route: Route): RouteWithID { return { diff --git a/public/app/features/alerting/unified/utils/matchers.test.ts b/public/app/features/alerting/unified/utils/matchers.test.ts index ae5b64b105e..43b3cbc2320 100644 --- a/public/app/features/alerting/unified/utils/matchers.test.ts +++ b/public/app/features/alerting/unified/utils/matchers.test.ts @@ -1,4 +1,6 @@ -import { getMatcherQueryParams, parseQueryParamMatchers } from './matchers'; +import { MatcherOperator, Route } from '../../../../plugins/datasource/alertmanager/types'; + +import { getMatcherQueryParams, normalizeMatchers, parseQueryParamMatchers } from './matchers'; describe('Unified Alerting matchers', () => { describe('getMatcherQueryParams tests', () => { @@ -33,4 +35,29 @@ describe('Unified Alerting matchers', () => { expect(matchers[0].value).toBe('TestData 1'); }); }); + + describe('normalizeMatchers', () => { + const eq = MatcherOperator.equal; + + it('should work for object_matchers', () => { + const route: Route = { object_matchers: [['foo', eq, 'bar']] }; + expect(normalizeMatchers(route)).toEqual([['foo', eq, 'bar']]); + }); + it('should work for matchers', () => { + const route: Route = { matchers: ['foo=bar', 'foo!=bar', 'foo=~bar', 'foo!~bar'] }; + expect(normalizeMatchers(route)).toEqual([ + ['foo', MatcherOperator.equal, 'bar'], + ['foo', MatcherOperator.notEqual, 'bar'], + ['foo', MatcherOperator.regex, 'bar'], + ['foo', MatcherOperator.notRegex, 'bar'], + ]); + }); + it('should work for match and match_re', () => { + const route: Route = { match: { foo: 'bar' }, match_re: { foo: 'bar' } }; + expect(normalizeMatchers(route)).toEqual([ + ['foo', MatcherOperator.regex, 'bar'], + ['foo', MatcherOperator.equal, 'bar'], + ]); + }); + }); }); diff --git a/public/app/features/alerting/unified/utils/matchers.ts b/public/app/features/alerting/unified/utils/matchers.ts index 7ca37819719..76a98bc6eab 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -1,9 +1,43 @@ import { uniqBy } from 'lodash'; -import { Labels } from '@grafana/data'; -import { Matcher } from 'app/plugins/datasource/alertmanager/types'; +import { Matcher, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; -import { parseMatcher } from './alertmanager'; +import { Labels } from '../../../../types/unified-alerting-dto'; + +const matcherOperators = [ + MatcherOperator.regex, + MatcherOperator.notRegex, + MatcherOperator.notEqual, + MatcherOperator.equal, +]; + +export function parseMatcher(matcher: string): Matcher { + const trimmed = matcher.trim(); + if (trimmed.startsWith('{') && trimmed.endsWith('}')) { + throw new Error(`PromQL matchers not supported yet, sorry! PromQL matcher found: ${trimmed}`); + } + const operatorsFound = matcherOperators + .map((op): [MatcherOperator, number] => [op, trimmed.indexOf(op)]) + .filter(([_, idx]) => idx > -1) + .sort((a, b) => a[1] - b[1]); + + if (!operatorsFound.length) { + throw new Error(`Invalid matcher: ${trimmed}`); + } + const [operator, idx] = operatorsFound[0]; + const name = trimmed.slice(0, idx).trim(); + const value = trimmed.slice(idx + operator.length).trim(); + if (!name) { + throw new Error(`Invalid matcher: ${trimmed}`); + } + + return { + name, + value, + isRegex: operator === MatcherOperator.regex || operator === MatcherOperator.notRegex, + isEqual: operator === MatcherOperator.equal || operator === MatcherOperator.regex, + }; +} // Parses a list of entries like like "['foo=bar', 'baz=~bad*']" into SilenceMatcher[] export function parseQueryParamMatchers(matcherPairs: string[]): Matcher[] { @@ -26,3 +60,84 @@ export const getMatcherQueryParams = (labels: Labels) => { return matcherUrlParams; }; + +/** + * We need to deal with multiple (deprecated) properties such as "match" and "match_re" + * this function will normalize all of the different ways to define matchers in to a single one. + */ +export const normalizeMatchers = (route: Route): ObjectMatcher[] => { + const matchers: ObjectMatcher[] = []; + + if (route.matchers) { + route.matchers.forEach((matcher) => { + const { name, value, isEqual, isRegex } = parseMatcher(matcher); + let operator = MatcherOperator.equal; + + if (isEqual && isRegex) { + operator = MatcherOperator.regex; + } + if (!isEqual && isRegex) { + operator = MatcherOperator.notRegex; + } + if (isEqual && !isRegex) { + operator = MatcherOperator.equal; + } + if (!isEqual && !isRegex) { + operator = MatcherOperator.notEqual; + } + + matchers.push([name, operator, value]); + }); + } + + if (route.object_matchers) { + matchers.push(...route.object_matchers); + } + + if (route.match_re) { + Object.entries(route.match_re).forEach(([label, value]) => { + matchers.push([label, MatcherOperator.regex, value]); + }); + } + + if (route.match) { + Object.entries(route.match).forEach(([label, value]) => { + matchers.push([label, MatcherOperator.equal, value]); + }); + } + + return matchers; +}; + +export type Label = [string, string]; +type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean; +const OperatorFunctions: Record = { + [MatcherOperator.equal]: (lv, mv) => lv === mv, + [MatcherOperator.notEqual]: (lv, mv) => lv !== mv, + [MatcherOperator.regex]: (lv, mv) => new RegExp(mv).test(lv), + [MatcherOperator.notRegex]: (lv, mv) => !new RegExp(mv).test(lv), +}; + +function isLabelMatch(matcher: ObjectMatcher, label: Label) { + const [labelKey, labelValue] = label; + const [matcherKey, operator, matcherValue] = matcher; + + // not interested, keys don't match + if (labelKey !== matcherKey) { + return false; + } + + const matchFunction = OperatorFunctions[operator]; + if (!matchFunction) { + throw new Error(`no such operator: ${operator}`); + } + + return matchFunction(labelValue, matcherValue); +} + +// check if every matcher returns "true" for the set of labels +export function labelsMatchObjectMatchers(matchers: ObjectMatcher[], labels: Label[]) { + return matchers.every((matcher) => { + return labels.some((label) => isLabelMatch(matcher, label)); + }); +} diff --git a/public/app/features/alerting/unified/utils/notification-policies.test.ts b/public/app/features/alerting/unified/utils/notification-policies.test.ts index 0029d3af795..03c48de027c 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.test.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.test.ts @@ -1,6 +1,8 @@ -import { MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; +import { MatcherOperator, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; -import { findMatchingRoutes } from './notification-policies'; +import { findMatchingRoutes, normalizeRoute } from './notification-policies'; + +import 'core-js/stable/structured-clone'; const CATCH_ALL_ROUTE: Route = { receiver: 'ALL', @@ -11,6 +13,7 @@ describe('findMatchingRoutes', () => { const policies: Route = { receiver: 'ROOT', group_by: ['grafana_folder'], + object_matchers: [], routes: [ { receiver: 'A', @@ -117,3 +120,40 @@ describe('findMatchingRoutes', () => { expect(matches[0]).toHaveProperty('receiver', 'PARENT'); }); }); + +describe('normalizeRoute', () => { + it('should map matchers property to object_matchers', function () { + const route: RouteWithID = { + id: '1', + matchers: ['foo=bar', 'foo=~ba.*'], + }; + + const normalized = normalizeRoute(route); + + expect(normalized.object_matchers).toHaveLength(2); + expect(normalized.object_matchers).toContainEqual(['foo', MatcherOperator.equal, 'bar']); + expect(normalized.object_matchers).toContainEqual(['foo', MatcherOperator.regex, 'ba.*']); + expect(normalized).not.toHaveProperty('matchers'); + }); + + it('should map match and match_re properties to object_matchers', function () { + const route: RouteWithID = { + id: '1', + match: { + foo: 'bar', + }, + match_re: { + team: 'op.*', + }, + }; + + const normalized = normalizeRoute(route); + + expect(normalized.object_matchers).toHaveLength(2); + expect(normalized.object_matchers).toContainEqual(['foo', MatcherOperator.equal, 'bar']); + expect(normalized.object_matchers).toContainEqual(['team', MatcherOperator.regex, 'op.*']); + + expect(normalized).not.toHaveProperty('match'); + expect(normalized).not.toHaveProperty('match_re'); + }); +}); diff --git a/public/app/features/alerting/unified/utils/notification-policies.ts b/public/app/features/alerting/unified/utils/notification-policies.ts index a2f9a35d535..ff87829736b 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -1,61 +1,25 @@ -import { AlertmanagerGroup, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; +import { AlertmanagerGroup, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; -import { normalizeMatchers } from './amroutes'; - -export type Label = [string, string]; -type OperatorPredicate = (labelValue: string, matcherValue: string) => boolean; - -const OperatorFunctions: Record = { - [MatcherOperator.equal]: (lv, mv) => lv === mv, - [MatcherOperator.notEqual]: (lv, mv) => lv !== mv, - [MatcherOperator.regex]: (lv, mv) => Boolean(lv.match(new RegExp(mv))), - [MatcherOperator.notRegex]: (lv, mv) => !Boolean(lv.match(new RegExp(mv))), -}; - -function isLabelMatch(matcher: ObjectMatcher, label: Label) { - const [labelKey, labelValue] = label; - const [matcherKey, operator, matcherValue] = matcher; - - // not interested, keys don't match - if (labelKey !== matcherKey) { - return false; - } - - const matchFunction = OperatorFunctions[operator]; - if (!matchFunction) { - throw new Error(`no such operator: ${operator}`); - } - - return matchFunction(labelValue, matcherValue); -} - -// check if every matcher returns "true" for the set of labels -function matchLabels(matchers: ObjectMatcher[], labels: Label[]) { - return matchers.every((matcher) => { - return labels.some((label) => isLabelMatch(matcher, label)); - }); -} +import { Label, normalizeMatchers, labelsMatchObjectMatchers } from './matchers'; // Match does a depth-first left-to-right search through the route tree // and returns the matching routing nodes. -function findMatchingRoutes(root: T, labels: Label[]): T[] { - let matches: T[] = []; +function findMatchingRoutes(root: Route, labels: Label[]): Route[] { + const matches: Route[] = []; // If the current node is not a match, return nothing - const normalizedMatchers = normalizeMatchers(root); - if (!matchLabels(normalizedMatchers, labels)) { + // const normalizedMatchers = normalizeMatchers(root); + // Normalization should have happened earlier in the code + if (!root.object_matchers || !labelsMatchObjectMatchers(root.object_matchers, labels)) { return []; } // If the current node matches, recurse through child nodes if (root.routes) { - for (let index = 0; index < root.routes.length; index++) { - let child = root.routes[index]; - let matchingChildren = findMatchingRoutes(child, labels); + for (const child of root.routes) { + const matchingChildren = findMatchingRoutes(child, labels); - // TODO how do I solve this typescript thingy? It looks correct to me /shrug - // @ts-ignore - matches = matches.concat(matchingChildren); + matches.push(...matchingChildren); // we have matching children and we don't want to continue, so break here if (matchingChildren.length && !child.continue) { @@ -72,6 +36,22 @@ function findMatchingRoutes(root: T, labels: Label[]): T[] { return matches; } +// This is a performance improvement to normalize matchers only once and use the normalized version later on +export function normalizeRoute(rootRoute: RouteWithID): RouteWithID { + function normalizeRoute(route: RouteWithID) { + route.object_matchers = normalizeMatchers(route); + delete route.matchers; + delete route.match; + delete route.match_re; + route.routes?.forEach(normalizeRoute); + } + + const normalizedRootRoute = structuredClone(rootRoute); + normalizeRoute(normalizedRootRoute); + + return normalizedRootRoute; +} + /** * find all of the groups that have instances that match the route, thay way we can find all instances * (and their grouping) for the given route @@ -102,4 +82,4 @@ function findMatchingAlertGroups( }, matchingGroups); } -export { findMatchingAlertGroups, findMatchingRoutes, matchLabels }; +export { findMatchingAlertGroups, findMatchingRoutes };