diff --git a/public/app/features/alerting/unified/NotificationPolicies.test.tsx b/public/app/features/alerting/unified/NotificationPolicies.test.tsx index fc80a85b6da..3698e5abc9d 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, within } from '@testing-library/react'; +import { render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { TestProvider } from 'test/helpers/TestProvider'; @@ -20,7 +20,6 @@ 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'; @@ -33,7 +32,6 @@ 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), @@ -390,7 +388,6 @@ describe('NotificationPolicies', () => { renderNotificationPolicies(); await waitFor(() => expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(1)); - expect(ui.newPolicyButton.query()).not.toBeInTheDocument(); expect(ui.editButton.query()).not.toBeInTheDocument(); }); @@ -402,12 +399,6 @@ 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(); @@ -639,18 +630,8 @@ 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 e80701b8227..d213921829d 100644 --- a/public/app/features/alerting/unified/NotificationPolicies.tsx +++ b/public/app/features/alerting/unified/NotificationPolicies.tsx @@ -1,7 +1,6 @@ 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'; @@ -12,7 +11,6 @@ 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'; @@ -35,12 +33,10 @@ import { Policy } from './components/notification-policies/Policy'; import { useAlertManagerSourceName } from './hooks/useAlertManagerSourceName'; import { useAlertManagersByPermission } from './hooks/useAlertManagerSources'; import { useUnifiedAlertingSelector } from './hooks/useUnifiedAlertingSelector'; -import { fetchAlertManagerConfigAction, updateAlertManagerConfigAction } from './state/actions'; +import { fetchAlertGroupsAction, fetchAlertManagerConfigAction, updateAlertManagerConfigAction } from './state/actions'; import { FormAmRoute } from './types/amroutes'; -import { useRouteGroupsMatcher } from './useRouteGroupsMatcher'; -import { addUniqueIdentifierToRoute } from './utils/amroutes'; +import { addUniqueIdentifierToRoute, normalizeMatchers } 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'; @@ -52,7 +48,6 @@ enum ActiveTab { const AmRoutes = () => { const dispatch = useDispatch(); const styles = useStyles2(getStyles); - const { useGetAlertmanagerAlertGroupsQuery } = alertmanagerApi; const [queryParams, setQueryParams] = useQueryParams(); const { tab } = getActiveTabFromUrl(queryParams); @@ -62,8 +57,6 @@ const AmRoutes = () => { const [contactPointFilter, setContactPointFilter] = useState(); const [labelMatchersFilter, setLabelMatchersFilter] = useState([]); - const { getRouteGroupsMap } = useRouteGroupsMatcher(); - const alertManagers = useAlertManagersByPermission('notification'); const [alertManagerSourceName, setAlertManagerSourceName] = useAlertManagerSourceName(alertManagers); @@ -76,11 +69,6 @@ const AmRoutes = () => { } }, [alertManagerSourceName, dispatch]); - const { currentData: alertGroups, refetch: refetchAlertGroups } = useGetAlertmanagerAlertGroupsQuery( - { amSourceName: alertManagerSourceName ?? '' }, - { skip: !alertManagerSourceName } - ); - const { result, loading: resultLoading, @@ -94,19 +82,10 @@ 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) { @@ -117,6 +96,9 @@ 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; @@ -167,7 +149,7 @@ const AmRoutes = () => { .unwrap() .then(() => { if (alertManagerSourceName) { - refetchAlertGroups(); + dispatch(fetchAlertGroupsAction(alertManagerSourceName)); } closeEditModal(); closeAddModal(); @@ -191,6 +173,13 @@ const AmRoutes = () => { useCleanup((state) => (state.unifiedAlerting.saveAMConfig = initialAsyncRequestState)); + // fetch AM instances grouping + useEffect(() => { + if (alertManagerSourceName) { + dispatch(fetchAlertGroupsAction(alertManagerSourceName)); + } + }, [alertManagerSourceName, dispatch]); + if (!alertManagerSourceName) { return ( @@ -263,7 +252,7 @@ const AmRoutes = () => { receivers={receivers} routeTree={rootRoute} currentRoute={rootRoute} - alertGroups={alertGroups ?? []} + alertGroups={fetchAlertGroups.result} contactPointsState={contactPointsState.receivers} readOnly={readOnlyPolicies} alertManagerSourceName={alertManagerSourceName} @@ -272,7 +261,6 @@ 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 deleted file mode 100644 index 94ac4ea7a2d..00000000000 --- a/public/app/features/alerting/unified/__mocks__/useRouteGroupsMatcher.ts +++ /dev/null @@ -1,20 +0,0 @@ -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 bd83e5071bd..a761134eaf9 100644 --- a/public/app/features/alerting/unified/api/alertmanagerApi.ts +++ b/public/app/features/alerting/unified/api/alertmanagerApi.ts @@ -2,7 +2,6 @@ import { AlertmanagerAlert, AlertmanagerChoice, AlertManagerCortexConfig, - AlertmanagerGroup, ExternalAlertmanagerConfig, ExternalAlertmanagers, ExternalAlertmanagersResponse, @@ -62,12 +61,6 @@ 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 330ab24c1aa..6fbeba09612 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Policy.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Policy.tsx @@ -1,7 +1,8 @@ import { css } from '@emotion/css'; import { uniqueId, pick, groupBy, upperFirst, merge, reduce, sumBy } from 'lodash'; import pluralize from 'pluralize'; -import React, { FC, Fragment, ReactNode } from 'react'; +import React, { FC, Fragment, ReactNode, useMemo } from 'react'; +import { useEnabled } from 'react-enable'; import { Link } from 'react-router-dom'; import { GrafanaTheme2, IconName } from '@grafana/data'; @@ -17,9 +18,11 @@ 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/matchers'; +import { normalizeMatchers } from '../../utils/amroutes'; import { createContactPointLink, createMuteTimingLink } from '../../utils/misc'; +import { findMatchingAlertGroups } from '../../utils/notification-policies'; import { HoverCard } from '../HoverCard'; import { Label } from '../Label'; import { MetaText } from '../MetaText'; @@ -41,7 +44,6 @@ interface PolicyComponentProps { readOnly?: boolean; inheritedProperties?: InhertitableProperties; routesMatchingFilters?: RouteWithID[]; - routeAlertGroupsMap?: Map; routeTree: RouteWithID; currentRoute: RouteWithID; @@ -62,7 +64,6 @@ const Policy: FC = ({ routeTree, inheritedProperties, routesMatchingFilters = [], - routeAlertGroupsMap, onEditPolicy, onAddPolicy, onDeletePolicy, @@ -70,6 +71,7 @@ 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); @@ -112,12 +114,12 @@ const Policy: FC = ({ const isEditable = canEditRoutes; const isDeletable = canDeleteRoutes && !isDefaultPolicy; - const matchingAlertGroups = routeAlertGroupsMap?.get(currentRoute.id); + const matchingAlertGroups = useMemo(() => { + return showMatchingInstances ? findMatchingAlertGroups(routeTree, currentRoute, alertGroups) : []; + }, [alertGroups, currentRoute, routeTree, showMatchingInstances]); // sum all alert instances for all groups we're handling - const numberOfAlertInstances = matchingAlertGroups - ? sumBy(matchingAlertGroups, (group) => group.alerts.length) - : undefined; + const numberOfAlertInstances = sumBy(matchingAlertGroups, (group) => group.alerts.length); // TODO dead branch detection, warnings for all sort of configs that won't work or will never be activated return ( @@ -194,16 +196,18 @@ const Policy: FC = ({ {/* Metadata row */}
- { - matchingAlertGroups && onShowAlertInstances(matchingAlertGroups, matchers); - }} - data-testid="matching-instances" - > - {numberOfAlertInstances ?? '-'} - {pluralize('instance', numberOfAlertInstances)} - + {showMatchingInstances && ( + { + onShowAlertInstances(matchingAlertGroups, matchers); + }} + data-testid="matching-instances" + > + {numberOfAlertInstances} + {pluralize('instance', numberOfAlertInstances)} + + )} {contactPoint && ( Delivered to @@ -294,7 +298,6 @@ 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 b0493878fc0..d94f1848ee2 100644 --- a/public/app/features/alerting/unified/features.ts +++ b/public/app/features/alerting/unified/features.ts @@ -1,7 +1,14 @@ import { FeatureDescription } from 'react-enable/dist/FeatureState'; -export enum AlertingFeature {} +export enum AlertingFeature { + NotificationPoliciesV2MatchingInstances = 'notification-policies.v2.matching-instances', +} -const FEATURES: FeatureDescription[] = []; +const FEATURES: FeatureDescription[] = [ + { + name: AlertingFeature.NotificationPoliciesV2MatchingInstances, + defaultValue: false, + }, +]; export default FEATURES; diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.ts index 228eea3dc78..aea4ba65c3e 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.ts @@ -9,9 +9,8 @@ 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, parseMatchers } from '../utils/alertmanager'; +import { labelsMatchMatchers, matcherToMatcherField, parseMatcher, 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 deleted file mode 100644 index f4eac245b3a..00000000000 --- a/public/app/features/alerting/unified/routeGroupsMatcher.worker.ts +++ /dev/null @@ -1,27 +0,0 @@ -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 deleted file mode 100644 index 23e11707421..00000000000 --- a/public/app/features/alerting/unified/useRouteGroupsMatcher.ts +++ /dev/null @@ -1,31 +0,0 @@ -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 30954e9b35c..2fd657a6a1b 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.test.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.test.ts @@ -1,8 +1,13 @@ import { Matcher, MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; import { Labels } from 'app/types/unified-alerting-dto'; -import { parseMatchers, labelsMatchMatchers, removeMuteTimingFromRoute, matchersToString } from './alertmanager'; -import { parseMatcher } from './matchers'; +import { + parseMatcher, + parseMatchers, + labelsMatchMatchers, + removeMuteTimingFromRoute, + matchersToString, +} from './alertmanager'; 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 b75671506fd..96a12752b91 100644 --- a/public/app/features/alerting/unified/utils/alertmanager.ts +++ b/public/app/features/alerting/unified/utils/alertmanager.ts @@ -126,6 +126,41 @@ 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 13538a666db..8895125b8b1 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 { Route } from 'app/plugins/datasource/alertmanager/types'; +import { MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; import { FormAmRoute } from '../types/amroutes'; -import { amRouteToFormAmRoute, emptyRoute, formAmRouteToAmRoute } from './amroutes'; +import { amRouteToFormAmRoute, emptyRoute, formAmRouteToAmRoute, normalizeMatchers } from './amroutes'; const emptyAmRoute: Route = { receiver: '', @@ -89,3 +89,28 @@ 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 7fbb401e007..0c39d0d2139 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -6,9 +6,8 @@ import { MatcherOperator, ObjectMatcher, Route, RouteWithID } from 'app/plugins/ import { FormAmRoute } from '../types/amroutes'; import { MatcherFieldValue } from '../types/silence-form'; -import { matcherToMatcherField } from './alertmanager'; +import { matcherToMatcherField, parseMatcher } from './alertmanager'; import { GRAFANA_RULES_SOURCE_NAME } from './datasource'; -import { normalizeMatchers, parseMatcher } from './matchers'; import { findExistingRoute } from './routeTree'; import { isValidPrometheusDuration } from './time'; @@ -64,6 +63,54 @@ 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 43b3cbc2320..ae5b64b105e 100644 --- a/public/app/features/alerting/unified/utils/matchers.test.ts +++ b/public/app/features/alerting/unified/utils/matchers.test.ts @@ -1,6 +1,4 @@ -import { MatcherOperator, Route } from '../../../../plugins/datasource/alertmanager/types'; - -import { getMatcherQueryParams, normalizeMatchers, parseQueryParamMatchers } from './matchers'; +import { getMatcherQueryParams, parseQueryParamMatchers } from './matchers'; describe('Unified Alerting matchers', () => { describe('getMatcherQueryParams tests', () => { @@ -35,29 +33,4 @@ 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 76a98bc6eab..7ca37819719 100644 --- a/public/app/features/alerting/unified/utils/matchers.ts +++ b/public/app/features/alerting/unified/utils/matchers.ts @@ -1,43 +1,9 @@ import { uniqBy } from 'lodash'; -import { Matcher, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; +import { Labels } from '@grafana/data'; +import { Matcher } from 'app/plugins/datasource/alertmanager/types'; -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, - }; -} +import { parseMatcher } from './alertmanager'; // Parses a list of entries like like "['foo=bar', 'baz=~bad*']" into SilenceMatcher[] export function parseQueryParamMatchers(matcherPairs: string[]): Matcher[] { @@ -60,84 +26,3 @@ 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 03c48de027c..0029d3af795 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.test.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.test.ts @@ -1,8 +1,6 @@ -import { MatcherOperator, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { MatcherOperator, Route } from 'app/plugins/datasource/alertmanager/types'; -import { findMatchingRoutes, normalizeRoute } from './notification-policies'; - -import 'core-js/stable/structured-clone'; +import { findMatchingRoutes } from './notification-policies'; const CATCH_ALL_ROUTE: Route = { receiver: 'ALL', @@ -13,7 +11,6 @@ describe('findMatchingRoutes', () => { const policies: Route = { receiver: 'ROOT', group_by: ['grafana_folder'], - object_matchers: [], routes: [ { receiver: 'A', @@ -120,40 +117,3 @@ 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 ff87829736b..a2f9a35d535 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -1,25 +1,61 @@ -import { AlertmanagerGroup, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { AlertmanagerGroup, MatcherOperator, ObjectMatcher, Route } from 'app/plugins/datasource/alertmanager/types'; -import { Label, normalizeMatchers, labelsMatchObjectMatchers } from './matchers'; +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)); + }); +} // Match does a depth-first left-to-right search through the route tree // and returns the matching routing nodes. -function findMatchingRoutes(root: Route, labels: Label[]): Route[] { - const matches: Route[] = []; +function findMatchingRoutes(root: T, labels: Label[]): T[] { + let matches: T[] = []; // If the current node is not a match, return nothing - // const normalizedMatchers = normalizeMatchers(root); - // Normalization should have happened earlier in the code - if (!root.object_matchers || !labelsMatchObjectMatchers(root.object_matchers, labels)) { + const normalizedMatchers = normalizeMatchers(root); + if (!matchLabels(normalizedMatchers, labels)) { return []; } // If the current node matches, recurse through child nodes if (root.routes) { - for (const child of root.routes) { - const matchingChildren = findMatchingRoutes(child, labels); + for (let index = 0; index < root.routes.length; index++) { + let child = root.routes[index]; + let matchingChildren = findMatchingRoutes(child, labels); - matches.push(...matchingChildren); + // TODO how do I solve this typescript thingy? It looks correct to me /shrug + // @ts-ignore + matches = matches.concat(matchingChildren); // we have matching children and we don't want to continue, so break here if (matchingChildren.length && !child.continue) { @@ -36,22 +72,6 @@ function findMatchingRoutes(root: Route, labels: Label[]): Route[] { 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 @@ -82,4 +102,4 @@ function findMatchingAlertGroups( }, matchingGroups); } -export { findMatchingAlertGroups, findMatchingRoutes }; +export { findMatchingAlertGroups, findMatchingRoutes, matchLabels };