From 3cbef45851bdb90f9f40820b560b2bd89ed0ba11 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Mon, 13 Oct 2025 15:24:46 +0200 Subject: [PATCH] Alerting: Fix instances matching in notification policies (#112326) Fix instances matching algorithm --- .../alerting/unified/routeGroupsMatcher.ts | 2 +- .../utils/notification-policies.test.ts | 311 +++++++++++++++++- .../unified/utils/notification-policies.ts | 22 +- 3 files changed, 324 insertions(+), 11 deletions(-) diff --git a/public/app/features/alerting/unified/routeGroupsMatcher.ts b/public/app/features/alerting/unified/routeGroupsMatcher.ts index 61b74cea8ff..1a75c5be57c 100644 --- a/public/app/features/alerting/unified/routeGroupsMatcher.ts +++ b/public/app/features/alerting/unified/routeGroupsMatcher.ts @@ -16,7 +16,7 @@ export const routeGroupsMatcher = { groups: AlertmanagerGroup[], options?: MatchOptions ): Map { - const normalizedRootRoute = getNormalizedRoute(rootRoute, options); + const normalizedRootRoute: RouteWithID = getNormalizedRoute(rootRoute, options); function addRouteGroups(route: RouteWithID, acc: Map) { const routeGroups = findMatchingAlertGroups(normalizedRootRoute, route, groups); 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 05e19092012..5aea0ed67b6 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,13 @@ -import { MatcherOperator, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { + AlertState, + AlertmanagerAlert, + AlertmanagerGroup, + MatcherOperator, + ObjectMatcher, + RouteWithID, +} from 'app/plugins/datasource/alertmanager/types'; -import { normalizeRoute, unquoteRouteMatchers } from './notification-policies'; +import { findMatchingAlertGroups, normalizeRoute, unquoteRouteMatchers } from './notification-policies'; describe('normalizeRoute', () => { it('should map matchers property to object_matchers', function () { @@ -74,3 +81,303 @@ describe('unquoteRouteMatchers', () => { expect(unwrapped.object_matchers).toContainEqual(['f\\oo with slash', MatcherOperator.equal, 'bar']); }); }); + +describe('findMatchingAlertGroups', () => { + // Helper functions to create minimal test data + const createAlert = (labels: Record): AlertmanagerAlert => ({ + labels, + annotations: {}, + startsAt: '2024-01-01T00:00:00Z', + endsAt: '2024-01-01T01:00:00Z', + updatedAt: '2024-01-01T00:00:00Z', + fingerprint: 'test-fingerprint', + receivers: [{ name: 'default' }], + status: { + state: AlertState.Active, + silencedBy: [], + inhibitedBy: [], + }, + }); + + const createGroup = (alerts: AlertmanagerAlert[]): AlertmanagerGroup => ({ + labels: {}, + receiver: { name: 'default' }, + alerts, + }); + + const createRoute = ( + id: string, + matchers?: ObjectMatcher[], + routes?: RouteWithID[], + continueMatching?: boolean + ): RouteWithID => ({ + id, + receiver: 'default', + object_matchers: matchers, + routes, + continue: continueMatching, + }); + + it('should match alerts to the correct route by ID', () => { + // Create a route tree with 2 child routes + const teamFrontendRoute = createRoute('route-1', [['team', MatcherOperator.equal, 'frontend']]); + const teamBackendRoute = createRoute('route-2', [['team', MatcherOperator.equal, 'backend']]); + const rootRoute = createRoute('root', [], [teamFrontendRoute, teamBackendRoute]); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const backendWarningAlert = createAlert({ team: 'backend', severity: 'warning' }); + + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendCriticalAlert, backendWarningAlert])]; + + // Test matching alerts for teamFrontendRoute + const result = findMatchingAlertGroups(rootRoute, teamFrontendRoute, alertGroups); + + expect(result).toHaveLength(1); + expect(result[0].alerts).toHaveLength(1); + expect(result[0].alerts).toContainEqual(frontendCriticalAlert); + }); + + it('should return empty array when no alerts match the route', () => { + const teamFrontendRoute = createRoute('route-1', [['team', MatcherOperator.equal, 'frontend']]); + const rootRoute = createRoute('root', [], [teamFrontendRoute]); + + const backendAlert = createAlert({ team: 'backend' }); + const opsAlert = createAlert({ team: 'ops' }); + + const alertGroups: AlertmanagerGroup[] = [createGroup([backendAlert, opsAlert])]; + + const result = findMatchingAlertGroups(rootRoute, teamFrontendRoute, alertGroups); + + expect(result).toHaveLength(0); + }); + + it('should handle alerts matching the root route', () => { + // Root route with no matchers (catch-all) + const rootRoute = createRoute('root', []); + + const frontendAlert = createAlert({ team: 'frontend' }); + const backendAlert = createAlert({ team: 'backend' }); + + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendAlert, backendAlert])]; + + const result = findMatchingAlertGroups(rootRoute, rootRoute, alertGroups); + + expect(result).toHaveLength(1); + expect(result[0].alerts).toHaveLength(2); + expect(result[0].alerts).toContainEqual(frontendAlert); + expect(result[0].alerts).toContainEqual(backendAlert); + }); + + it('should correctly filter alerts within groups', () => { + const severityCritialRoute = createRoute('route-1', [['severity', MatcherOperator.equal, 'critical']]); + const rootRoute = createRoute('root', [], [severityCritialRoute]); + + const criticalFrontendAlert = createAlert({ severity: 'critical', team: 'frontend' }); + const warningFrontendAlert = createAlert({ severity: 'warning', team: 'frontend' }); + const criticalBackendAlert = createAlert({ severity: 'critical', team: 'backend' }); + + const alertGroups: AlertmanagerGroup[] = [ + createGroup([criticalFrontendAlert, warningFrontendAlert, criticalBackendAlert]), + ]; + + const result = findMatchingAlertGroups(rootRoute, severityCritialRoute, alertGroups); + + expect(result).toHaveLength(1); + expect(result[0].alerts).toHaveLength(2); + + // Verify only critical alerts are returned + expect(result[0].alerts).toContainEqual(criticalFrontendAlert); + expect(result[0].alerts).toContainEqual(criticalBackendAlert); + expect(result[0].alerts).not.toContainEqual(warningFrontendAlert); + }); + + it('should handle multiple alert groups', () => { + const teamFrontendRoute = createRoute('route-1', [['team', MatcherOperator.equal, 'frontend']]); + const rootRoute = createRoute('root', [], [teamFrontendRoute]); + + const frontendProdAlert = createAlert({ team: 'frontend', env: 'prod' }); + const backendProdAlert = createAlert({ team: 'backend', env: 'prod' }); + const frontendDevAlert = createAlert({ team: 'frontend', env: 'dev' }); + + const alertGroups: AlertmanagerGroup[] = [ + createGroup([frontendProdAlert]), + createGroup([backendProdAlert]), + createGroup([frontendDevAlert]), + ]; + + const result = findMatchingAlertGroups(rootRoute, teamFrontendRoute, alertGroups); + + expect(result).toHaveLength(2); + + // Verify we got the correct alerts (frontend only) + const allAlerts = result.flatMap((group) => group.alerts); + expect(allAlerts).toHaveLength(2); + expect(allAlerts).toContainEqual(frontendProdAlert); + expect(allAlerts).toContainEqual(frontendDevAlert); + expect(allAlerts).not.toContainEqual(backendProdAlert); + }); + + it('should match alerts using regex matchers', () => { + const route: RouteWithID = createRoute('route-1', [['service', MatcherOperator.regex, 'api-.*']]); + const rootRoute: RouteWithID = createRoute('root', [], [route]); + + const apiFrontendAlert = createAlert({ service: 'api-frontend' }); + const apiBackendAlert = createAlert({ service: 'api-backend' }); + const workerAlert = createAlert({ service: 'worker' }); + + const alertGroups: AlertmanagerGroup[] = [createGroup([apiFrontendAlert, apiBackendAlert, workerAlert])]; + + const result = findMatchingAlertGroups(rootRoute, route, alertGroups); + + expect(result).toHaveLength(1); + expect(result[0].alerts).toHaveLength(2); + + // Verify we got the api-* alerts and not the worker alert + expect(result[0].alerts).toContainEqual(apiFrontendAlert); + expect(result[0].alerts).toContainEqual(apiBackendAlert); + expect(result[0].alerts).not.toContainEqual(workerAlert); + }); + + it('should match alerts with multiple matchers (AND logic)', () => { + const route: RouteWithID = createRoute('route-1', [ + ['team', MatcherOperator.equal, 'frontend'], + ['severity', MatcherOperator.equal, 'critical'], + ]); + const rootRoute: RouteWithID = createRoute('root', [], [route]); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const frontendWarningAlert = createAlert({ team: 'frontend', severity: 'warning' }); + const backendCriticalAlert = createAlert({ team: 'backend', severity: 'critical' }); + + const alertGroups: AlertmanagerGroup[] = [ + createGroup([frontendCriticalAlert, frontendWarningAlert, backendCriticalAlert]), + ]; + + const result = findMatchingAlertGroups(rootRoute, route, alertGroups); + + expect(result).toHaveLength(1); + expect(result[0].alerts).toHaveLength(1); + + // Only the alert matching both conditions should be returned + expect(result[0].alerts).toContainEqual(frontendCriticalAlert); + expect(result[0].alerts).not.toContainEqual(frontendWarningAlert); + expect(result[0].alerts).not.toContainEqual(backendCriticalAlert); + }); + + it('should match only the first route when multiple routes have identical matchers', () => { + // Create two routes with identical matchers - only the first should match (depth-first left-to-right) + const firstTeamFrontendRoute: RouteWithID = createRoute('route-1', [['team', MatcherOperator.equal, 'frontend']]); + const secondTeamFrontendRoute: RouteWithID = createRoute('route-2', [['team', MatcherOperator.equal, 'frontend']]); + const rootRoute: RouteWithID = createRoute('root', [], [firstTeamFrontendRoute, secondTeamFrontendRoute]); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendCriticalAlert])]; + + // Check that firstTeamFrontendRoute matches + const resultFirst = findMatchingAlertGroups(rootRoute, firstTeamFrontendRoute, alertGroups); + expect(resultFirst).toHaveLength(1); + expect(resultFirst[0].alerts).toContainEqual(frontendCriticalAlert); + + // Check that secondTeamFrontendRoute does NOT match (alert stops at first route) + const resultSecond = findMatchingAlertGroups(rootRoute, secondTeamFrontendRoute, alertGroups); + expect(resultSecond).toHaveLength(0); + }); + + it('should match multiple routes when continue flag is set to true', () => { + // Create routes with continue=true on the first one + const teamFrontendRouteWithContinue: RouteWithID = createRoute( + 'route-1', + [['team', MatcherOperator.equal, 'frontend']], + [], + true // continue=true + ); + const teamFrontendSiblingRoute: RouteWithID = createRoute('route-2', [['team', MatcherOperator.equal, 'frontend']]); + const rootRoute: RouteWithID = createRoute('root', [], [teamFrontendRouteWithContinue, teamFrontendSiblingRoute]); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendCriticalAlert])]; + + // With continue=true, both routes should match the same alert + const resultWithContinue = findMatchingAlertGroups(rootRoute, teamFrontendRouteWithContinue, alertGroups); + expect(resultWithContinue).toHaveLength(1); + expect(resultWithContinue[0].alerts).toContainEqual(frontendCriticalAlert); + + const resultSibling = findMatchingAlertGroups(rootRoute, teamFrontendSiblingRoute, alertGroups); + expect(resultSibling).toHaveLength(1); + expect(resultSibling[0].alerts).toContainEqual(frontendCriticalAlert); + }); + + it('should handle nested routes with continue flag', () => { + // Create a more complex tree: + // root + // ├─ teamFrontendRouteWithContinue (team=frontend, continue=true) + // │ └─ severityCriticalSubRoute (severity=critical) + // └─ teamFrontendSiblingRoute (team=frontend) + const severityCriticalSubRoute: RouteWithID = createRoute('severity-critical-sub', [ + ['severity', MatcherOperator.equal, 'critical'], + ]); + const teamFrontendRouteWithContinue: RouteWithID = createRoute( + 'team-frontend-continue', + [['team', MatcherOperator.equal, 'frontend']], + [severityCriticalSubRoute], + true + ); + const teamFrontendSiblingRoute: RouteWithID = createRoute('team-frontend-sibling', [ + ['team', MatcherOperator.equal, 'frontend'], + ]); + const rootRoute: RouteWithID = createRoute('root', [], [teamFrontendRouteWithContinue, teamFrontendSiblingRoute]); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const frontendWarningAlert = createAlert({ team: 'frontend', severity: 'warning' }); + + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendCriticalAlert, frontendWarningAlert])]; + + // severityCriticalSubRoute should match only the critical alert (most specific match) + const resultSubRoute = findMatchingAlertGroups(rootRoute, severityCriticalSubRoute, alertGroups); + expect(resultSubRoute).toHaveLength(1); + expect(resultSubRoute[0].alerts).toHaveLength(1); + expect(resultSubRoute[0].alerts).toContainEqual(frontendCriticalAlert); + + // teamFrontendSiblingRoute should match both alerts because teamFrontendRouteWithContinue has continue=true + const resultSibling = findMatchingAlertGroups(rootRoute, teamFrontendSiblingRoute, alertGroups); + expect(resultSibling).toHaveLength(1); + expect(resultSibling[0].alerts).toHaveLength(2); + expect(resultSibling[0].alerts).toContainEqual(frontendCriticalAlert); + expect(resultSibling[0].alerts).toContainEqual(frontendWarningAlert); + }); + + it('should stop at first match when continue flag is false (default)', () => { + // Test that without continue flag, matching stops at first route + const firstSeverityCriticalRoute: RouteWithID = createRoute( + 'first-severity-critical', + [['severity', MatcherOperator.equal, 'critical']], + [], + false + ); + const secondSeverityCriticalRoute: RouteWithID = createRoute('second-severity-critical', [ + ['severity', MatcherOperator.equal, 'critical'], + ]); + const teamFrontendRoute: RouteWithID = createRoute('team-frontend', [['team', MatcherOperator.equal, 'frontend']]); + const rootRoute: RouteWithID = createRoute( + 'root', + [], + [firstSeverityCriticalRoute, secondSeverityCriticalRoute, teamFrontendRoute] + ); + + const frontendCriticalAlert = createAlert({ team: 'frontend', severity: 'critical' }); + const alertGroups: AlertmanagerGroup[] = [createGroup([frontendCriticalAlert])]; + + // firstSeverityCriticalRoute should match (first matching route) + const resultFirst = findMatchingAlertGroups(rootRoute, firstSeverityCriticalRoute, alertGroups); + expect(resultFirst).toHaveLength(1); + expect(resultFirst[0].alerts).toContainEqual(frontendCriticalAlert); + + // secondSeverityCriticalRoute should NOT match (stopped at first route) + const resultSecond = findMatchingAlertGroups(rootRoute, secondSeverityCriticalRoute, alertGroups); + expect(resultSecond).toHaveLength(0); + + // teamFrontendRoute should NOT match (stopped at first route) + const resultTeam = findMatchingAlertGroups(rootRoute, teamFrontendRoute, alertGroups); + expect(resultTeam).toHaveLength(0); + }); +}); diff --git a/public/app/features/alerting/unified/utils/notification-policies.ts b/public/app/features/alerting/unified/utils/notification-policies.ts index 7f2e0437b3b..28f5dc8d9c0 100644 --- a/public/app/features/alerting/unified/utils/notification-policies.ts +++ b/public/app/features/alerting/unified/utils/notification-policies.ts @@ -1,5 +1,5 @@ import { findMatchingRoutes } from '@grafana/alerting/unstable'; -import { AlertmanagerGroup, Route } from 'app/plugins/datasource/alertmanager/types'; +import { AlertmanagerGroup, Route, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; import { normalizeMatchers, unquoteWithUnescape } from './matchers'; import { routeAdapter } from './routeAdapter'; @@ -39,21 +39,27 @@ export function unquoteRouteMatchers(route: T): T { * (and their grouping) for the given route */ function findMatchingAlertGroups( - routeTree: Route, - route: Route, + routeTree: RouteWithID, + route: RouteWithID, alertGroups: AlertmanagerGroup[] ): AlertmanagerGroup[] { const matchingGroups: AlertmanagerGroup[] = []; + // Convert routes once outside the loop for efficiency + // findMatchingRoutes expects the alerting package Route type, so we need to convert + const alertingRouteTree = routeAdapter.toPackage(routeTree); + const alertingRoute = routeAdapter.toPackage(route); + return alertGroups.reduce((acc, group) => { // find matching alerts in the current group const matchingAlerts = group.alerts.filter((alert) => { const labels = Object.entries(alert.labels); - const alertingRouteTree = routeAdapter.toPackage(routeTree); - const alertingRoute = routeAdapter.toPackage(route); - return findMatchingRoutes(alertingRouteTree, labels).some( - (matchingRoute) => matchingRoute.route === alertingRoute - ); + const matchingRoutes = findMatchingRoutes(alertingRouteTree, labels); + + // Compare routes by id - we must use ID comparison because routeAdapter.toPackage() + // creates new objects, so reference equality would always be false. + // The ID is preserved during conversion and uniquely identifies each route. + return matchingRoutes.some((matchingRoute) => matchingRoute.route.id === alertingRoute.id); }); // if the groups has any alerts left after matching, add it to the results