[fix] Fix notice rule label matching against grouped alerts (#4191)

This commit is contained in:
Duansg
2026-07-12 08:34:45 -07:00
committed by GitHub
parent 9bd2796119
commit 4cf968a3f4
2 changed files with 111 additions and 10 deletions
@@ -69,18 +69,18 @@ import java.util.stream.Collectors;
@Transactional(rollbackFor = Exception.class)
@Slf4j
public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLineRunner {
private static final Map<Byte, NoticeTemplate> PRESET_TEMPLATE = new HashMap<>(16);
@Autowired
private NoticeReceiverDao noticeReceiverDao;
@Autowired
private NoticeRuleDao noticeRuleDao;
@Autowired
private NoticeTemplateDao noticeTemplateDao;
@Autowired
@Lazy
private AlertNoticeDispatch dispatcherAlarm;
@@ -212,24 +212,30 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
}
// The temporary rule is to forward all, and then implement more matching rules: alarm status selection, monitoring type selection, etc.
// TODO: This matches an already-grouped alert against notice rules (group-then-route). It cannot fully
// separate alerts that were grouped together but should reach different receivers, so a rule matched by
// one alert still notifies the whole group. The ideal design is route-then-group (like Alertmanager):
// route each single alert by its labels first, then group per receiver. Tracked as a follow-up to #3852.
return rules.stream()
.filter(rule -> {
if (!rule.isFilterAll()) {
// filter labels
// filter labels: a rule matches when ANY single alert in the group carries
if (rule.getLabels() != null && !rule.getLabels().isEmpty()) {
boolean labelMatch = rule.getLabels().entrySet().stream().allMatch(labelItem -> {
if (!alert.getCommonLabels().containsKey(labelItem.getKey())) {
List<SingleAlert> singleAlerts = alert.getAlerts();
boolean labelMatch = singleAlerts != null && singleAlerts.stream().anyMatch(singleAlert -> {
Map<String, String> alertLabels = singleAlert.getLabels();
if (alertLabels == null) {
return false;
}
String alertLabelValue = alert.getCommonLabels().get(labelItem.getKey());
return Objects.equals(labelItem.getValue(), alertLabelValue);
return rule.getLabels().entrySet().stream().allMatch(labelItem ->
Objects.equals(labelItem.getValue(), alertLabels.get(labelItem.getKey())));
});
if (!labelMatch) {
return false;
}
}
}
LocalDateTime nowDate = LocalDateTime.now();
// filter day
int currentDayOfWeek = nowDate.toLocalDate().getDayOfWeek().getValue();
@@ -22,10 +22,13 @@ import org.apache.hertzbeat.alert.dao.NoticeRuleDao;
import org.apache.hertzbeat.alert.dao.NoticeTemplateDao;
import org.apache.hertzbeat.alert.notice.AlertNoticeDispatch;
import org.apache.hertzbeat.alert.service.impl.NoticeConfigServiceImpl;
import org.apache.hertzbeat.common.cache.CacheFactory;
import org.apache.hertzbeat.common.entity.alerter.GroupAlert;
import org.apache.hertzbeat.common.entity.alerter.NoticeReceiver;
import org.apache.hertzbeat.common.entity.alerter.NoticeRule;
import org.apache.hertzbeat.common.entity.alerter.NoticeTemplate;
import org.apache.hertzbeat.common.entity.alerter.SingleAlert;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -39,9 +42,12 @@ import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.domain.Specification;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
@@ -284,4 +290,93 @@ class NoticeConfigServiceTest {
noticeConfigService.sendTestMsg(noticeReceiver);
verify(dispatcherAlarm, times(1)).sendNoticeMsg(eq(noticeReceiver), eq(noticeTemplate), any(GroupAlert.class));
}
@AfterEach
void tearDown() {
CacheFactory.clearNoticeCache();
}
private GroupAlert buildGroupAlertWithPartialLabel() {
SingleAlert alertA = SingleAlert.builder()
.labels(new HashMap<>(Map.of("alertname", "cpu", "instance", "s1", "department", "algorithm")))
.build();
SingleAlert alertB = SingleAlert.builder()
.labels(new HashMap<>(Map.of("alertname", "cpu", "instance", "s2")))
.build();
return GroupAlert.builder()
.commonLabels(new HashMap<>(Map.of("alertname", "cpu")))
.alerts(Arrays.asList(alertA, alertB))
.build();
}
/**
* A rule scoped to a label carried by only part of the group's alerts must still match,
* even though that label is absent from commonLabels. Regression test for issue #3852.
*/
@Test
void getReceiverFilterRuleMatchesLabelOnPartialAlert() {
NoticeRule rule = new NoticeRule();
rule.setId(1L);
rule.setName("algorithm-team");
rule.setFilterAll(false);
rule.setLabels(new HashMap<>(Map.of("department", "algorithm")));
CacheFactory.setNoticeCache(List.of(rule));
List<NoticeRule> matched = noticeConfigService.getReceiverFilterRule(buildGroupAlertWithPartialLabel());
assertEquals(1, matched.size());
assertEquals(1L, matched.get(0).getId());
}
/**
* A rule whose label value is not present on any alert in the group must not match.
*/
@Test
void getReceiverFilterRuleSkipsRuleWithUnmatchedLabel() {
NoticeRule rule = new NoticeRule();
rule.setId(2L);
rule.setName("infra-team");
rule.setFilterAll(false);
rule.setLabels(new HashMap<>(Map.of("department", "infra")));
CacheFactory.setNoticeCache(List.of(rule));
List<NoticeRule> matched = noticeConfigService.getReceiverFilterRule(buildGroupAlertWithPartialLabel());
assertTrue(matched.isEmpty());
}
/**
* A rule requiring several labels matches only when a single alert carries all of them,
* preventing false matches assembled across different alerts in the group.
*/
@Test
void getReceiverFilterRuleRequiresAllLabelsOnSameAlert() {
NoticeRule rule = new NoticeRule();
rule.setId(3L);
rule.setName("algorithm-on-s2");
rule.setFilterAll(false);
rule.setLabels(new HashMap<>(Map.of("department", "algorithm", "instance", "s2")));
CacheFactory.setNoticeCache(List.of(rule));
List<NoticeRule> matched = noticeConfigService.getReceiverFilterRule(buildGroupAlertWithPartialLabel());
assertTrue(matched.isEmpty());
}
/**
* A forward-all rule keeps matching regardless of labels.
*/
@Test
void getReceiverFilterRuleForwardAllAlwaysMatches() {
NoticeRule rule = new NoticeRule();
rule.setId(4L);
rule.setName("forward-all");
rule.setFilterAll(true);
CacheFactory.setNoticeCache(List.of(rule));
List<NoticeRule> matched = noticeConfigService.getReceiverFilterRule(buildGroupAlertWithPartialLabel());
assertEquals(1, matched.size());
assertEquals(4L, matched.get(0).getId());
}
}