diff --git a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java index f7f62ab8b7d..dc4521aabbc 100644 --- a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java +++ b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java @@ -69,18 +69,18 @@ @Transactional(rollbackFor = Exception.class) @Slf4j public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLineRunner { - + private static final Map 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 List getReceiverFilterRule(GroupAlert alert) { } // 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 singleAlerts = alert.getAlerts(); + boolean labelMatch = singleAlerts != null && singleAlerts.stream().anyMatch(singleAlert -> { + Map 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(); diff --git a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java index 9d06aa26db5..81a9c2c24be 100644 --- a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java +++ b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java @@ -22,10 +22,13 @@ 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.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 @@ void sendTestMsg() { 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 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 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 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 matched = noticeConfigService.getReceiverFilterRule(buildGroupAlertWithPartialLabel()); + + assertEquals(1, matched.size()); + assertEquals(4L, matched.get(0).getId()); + } }