[fix] Fixed matching issues caused by date components and outdated time zone offsets (#4192)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-09-17 09:40:58 +00:00 · 2026-07-13 07:23:02 -07:00
parent 8b6d0d0056
commit cc8206062f
4 changed files with 146 additions and 77 deletions
@@ -19,9 +19,11 @@ package org.apache.hertzbeat.alert.reduce;

 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.ZoneId;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+
 import lombok.RequiredArgsConstructor;
 import org.apache.hertzbeat.alert.dao.AlertSilenceDao;
 import org.apache.hertzbeat.alert.notice.AlertNoticeDispatch;
@@ -44,6 +46,7 @@ public class AlarmSilenceReduce {
    /**
     * Process alert with silence rules
     * If alert matches any active silence rule, it will be silenced
+     *
     * @param groupAlert The alert to be processed
     */
    public void silenceAlarm(GroupAlert groupAlert) {
@@ -52,7 +55,7 @@ public class AlarmSilenceReduce {
            alertSilenceList = alertSilenceDao.findAlertSilencesByEnableTrue();
            CacheFactory.setAlertSilenceCache(alertSilenceList);
        }
-        
+
        // Check each silence rule
        for (AlertSilence alertSilence : alertSilenceList) {
            // Check if alert matches silence rule
@@ -60,10 +63,10 @@ public class AlarmSilenceReduce {
            if (!match && groupAlert.getGroupLabels() != null) {
                Map<String, String> labels = alertSilence.getLabels();
                Map<String, String> alertLabels = groupAlert.getGroupLabels();
-                match = labels.entrySet().stream().anyMatch(item -> 
+                match = labels.entrySet().stream().anyMatch(item ->
                    alertLabels.containsKey(item.getKey()) && item.getValue().equals(alertLabels.get(item.getKey())));
            }
-            
+
            if (match) {
                LocalDateTime now = LocalDateTime.now();
                if (alertSilence.getType() == 0) {
@@ -76,22 +79,23 @@ public class AlarmSilenceReduce {
                } else if (alertSilence.getType() == 1) {
                    // Cyclic silence rule
                    int currentDayOfWeek = now.getDayOfWeek().getValue();
-                    if (alertSilence.getDays() != null && alertSilence.getDays().contains((byte) currentDayOfWeek) 
-                            && !checkAndSave(now, alertSilence)) {
+                    if (alertSilence.getDays() != null && alertSilence.getDays().contains((byte) currentDayOfWeek)
+                        && !checkAndSave(now, alertSilence)) {
                        // Alert is silenced
                        return;
                    }
                }
            }
        }
-        
+
        // No matching silence rule, forward the alert
        dispatcherAlarm.dispatchAlarm(groupAlert);
    }

    /**
     * Check if alert time is within silence period and update silence rule counter
-     * @param now Current time
+     *
+     * @param now          Current time
     * @param alertSilence Silence rule to check
     * @return true if alert should not be silenced, false if alert should be silenced
     */
@@ -100,8 +104,11 @@ public class AlarmSilenceReduce {
        boolean endMatch;
        if (alertSilence.getType() == 1) {
            LocalTime nowTime = now.toLocalTime();
-            LocalTime startTime = alertSilence.getPeriodStart() == null ? null : alertSilence.getPeriodStart().toLocalTime();
-            LocalTime endTime = alertSilence.getPeriodEnd() == null ? null : alertSilence.getPeriodEnd().toLocalTime();
+            // compare wall-clock times in the server time zone, the stored offset may differ from it
+            LocalTime startTime = alertSilence.getPeriodStart() == null
+                ? null : alertSilence.getPeriodStart().withZoneSameInstant(ZoneId.systemDefault()).toLocalTime();
+            LocalTime endTime = alertSilence.getPeriodEnd() == null
+                ? null : alertSilence.getPeriodEnd().withZoneSameInstant(ZoneId.systemDefault()).toLocalTime();
            if (startTime == null && endTime == null) {
                startMatch = true;
                endMatch = true;
@@ -121,9 +128,9 @@ public class AlarmSilenceReduce {
            }
        } else {
            startMatch = alertSilence.getPeriodStart() == null
-                    || now.isAfter(alertSilence.getPeriodStart().toLocalDateTime());
+                || now.isAfter(alertSilence.getPeriodStart().withZoneSameInstant(ZoneId.systemDefault()).toLocalDateTime());
            endMatch = alertSilence.getPeriodEnd() == null
-                    || now.isBefore(alertSilence.getPeriodEnd().toLocalDateTime());
+                || now.isBefore(alertSilence.getPeriodEnd().withZoneSameInstant(ZoneId.systemDefault()).toLocalDateTime());
        }

        if (startMatch && endMatch) {
@@ -52,6 +52,7 @@ import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.ZoneId;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -91,7 +92,7 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
            Predicate predicate = criteriaBuilder.conjunction();
            if (StringUtils.isNotBlank(name)) {
                Predicate predicateName = criteriaBuilder.like(
-                        criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
+                    criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
                );
                predicate = criteriaBuilder.and(predicateName);
            }
@@ -113,9 +114,9 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine

            // Filter by name (case-insensitive)
            List<NoticeTemplate> filteredDefaultTemplates = defaultTemplates.stream()
-                    .filter(template -> StringUtils.isBlank(name)
-                            || template.getName().toLowerCase().contains(name.toLowerCase()))
-                    .collect(Collectors.toList());
+                .filter(template -> StringUtils.isBlank(name)
+                    || template.getName().toLowerCase().contains(name.toLowerCase()))
+                .collect(Collectors.toList());

            // Pagination logic
            int totalItems = filteredDefaultTemplates.size();
@@ -134,7 +135,7 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
                Predicate predicate = criteriaBuilder.conjunction();
                if (StringUtils.isNotBlank(name)) {
                    Predicate predicateName = criteriaBuilder.like(
-                            criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
+                        criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
                    );
                    predicate = criteriaBuilder.and(predicateName);
                }
@@ -146,7 +147,6 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
    }


-
    @Override
    public List<NoticeTemplate> getAllNoticeTemplates() {
        List<NoticeTemplate> defaultTemplates = new LinkedList<>(PRESET_TEMPLATE.values());
@@ -160,7 +160,7 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
            Predicate predicate = criteriaBuilder.conjunction();
            if (StringUtils.isNotBlank(name)) {
                Predicate predicateName = criteriaBuilder.like(
-                        criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
+                    criteriaBuilder.lower(root.get("name")), "%" + name.toLowerCase() + "%"
                );
                predicate = criteriaBuilder.and(predicateName);
            }
@@ -217,45 +217,56 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
        //  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: a rule matches when ANY single alert in the group carries
-                        if (rule.getLabels() != null && !rule.getLabels().isEmpty()) {
-                            List<SingleAlert> singleAlerts = alert.getAlerts();
-                            boolean labelMatch = singleAlerts != null && singleAlerts.stream().anyMatch(singleAlert -> {
-                                Map<String, String> alertLabels = singleAlert.getLabels();
-                                if (alertLabels == null) {
-                                    return false;
-                                }
-                                return rule.getLabels().entrySet().stream().allMatch(labelItem ->
-                                        Objects.equals(labelItem.getValue(), alertLabels.get(labelItem.getKey())));
-                            });
-                            if (!labelMatch) {
+            .filter(rule -> {
+                if (!rule.isFilterAll()) {
+                    // filter labels: a rule matches when ANY single alert in the group carries
+                    if (rule.getLabels() != null && !rule.getLabels().isEmpty()) {
+                        List<SingleAlert> singleAlerts = alert.getAlerts();
+                        boolean labelMatch = singleAlerts != null && singleAlerts.stream().anyMatch(singleAlert -> {
+                            Map<String, String> alertLabels = singleAlert.getLabels();
+                            if (alertLabels == null) {
                                return false;
                            }
-                        }
-                    }
-
-                    LocalDateTime nowDate = LocalDateTime.now();
-                    // filter day
-                    int currentDayOfWeek = nowDate.toLocalDate().getDayOfWeek().getValue();
-                    if (rule.getDays() != null && !rule.getDays().isEmpty()) {
-                        boolean dayMatch = rule.getDays().stream().anyMatch(item -> item == currentDayOfWeek);
-                        if (!dayMatch) {
+                            return rule.getLabels().entrySet().stream().allMatch(labelItem ->
+                             Objects.equals(labelItem.getValue(), alertLabels.get(labelItem.getKey())));
+                        });
+                        if (!labelMatch) {
                            return false;
                        }
                    }
-                    // filter time
-                    LocalTime nowTime = nowDate.toLocalTime();
-                    boolean startMatch = rule.getPeriodStart() == null
-                            || nowTime.isAfter(rule.getPeriodStart().toLocalTime())
-                            || (rule.getPeriodEnd() != null && rule.getPeriodStart().isAfter(rule.getPeriodEnd())
-                                    && nowTime.isBefore(rule.getPeriodStart().toLocalTime()));
-                    boolean endMatch = rule.getPeriodEnd() == null
-                            || nowTime.isBefore(rule.getPeriodEnd().toLocalTime());
-                    return startMatch && endMatch;
-                })
-                .collect(Collectors.toList());
+                }
+
+                LocalDateTime nowDate = LocalDateTime.now();
+                // filter day
+                int currentDayOfWeek = nowDate.toLocalDate().getDayOfWeek().getValue();
+                if (rule.getDays() != null && !rule.getDays().isEmpty()) {
+                    boolean dayMatch = rule.getDays().stream().anyMatch(item -> item == currentDayOfWeek);
+                    if (!dayMatch) {
+                        return false;
+                    }
+                }
+                // filter time, compare wall-clock times in the server time zone,
+                // the stored date part is meaningless (it is the day the user picked the time on the ui)
+                LocalTime nowTime = nowDate.toLocalTime();
+                LocalTime startTime = rule.getPeriodStart() == null
+                    ? null : rule.getPeriodStart().withZoneSameInstant(ZoneId.systemDefault()).toLocalTime();
+                LocalTime endTime = rule.getPeriodEnd() == null
+                    ? null : rule.getPeriodEnd().withZoneSameInstant(ZoneId.systemDefault()).toLocalTime();
+                if (startTime == null && endTime == null) {
+                    return true;
+                }
+                if (startTime == null) {
+                    return !nowTime.isAfter(endTime);
+                }
+                if (endTime == null) {
+                    return !nowTime.isBefore(startTime);
+                }
+                if (!startTime.isAfter(endTime)) {
+                    return !nowTime.isBefore(startTime) && !nowTime.isAfter(endTime);
+                }
+                // cross-midnight window, e.g. 22:00-06:00
+                return !nowTime.isBefore(startTime) || !nowTime.isAfter(endTime);
+            }).collect(Collectors.toList());
    }

    @Override
@@ -314,31 +325,31 @@ public class NoticeConfigServiceImpl implements NoticeConfigService, CommandLine
        Map<String, String> annotations = new HashMap<>(8);
        annotations.put("suggest", "Please check the CPU usage of the server");
        SingleAlert singleAlert1 = SingleAlert.builder()
-                .labels(labels)
-                .content("test send msg! \\n This is the test data. It is proved that it can be received successfully")
-                .startAt(System.currentTimeMillis())
-                .activeAt(System.currentTimeMillis())
-                .endAt(System.currentTimeMillis())
-                .triggerTimes(2)
-                .annotations(annotations)
-                .status("firing")
-                .build();
+            .labels(labels)
+            .content("test send msg! \\n This is the test data. It is proved that it can be received successfully")
+            .startAt(System.currentTimeMillis())
+            .activeAt(System.currentTimeMillis())
+            .endAt(System.currentTimeMillis())
+            .triggerTimes(2)
+            .annotations(annotations)
+            .status("firing")
+            .build();
        SingleAlert singleAlert2 = SingleAlert.builder()
-                .labels(labels)
-                .content("test send msg! \\n This is the test data. It is proved that it can be received successfully")
-                .startAt(System.currentTimeMillis())
-                .activeAt(System.currentTimeMillis())
-                .endAt(System.currentTimeMillis())
-                .triggerTimes(4)
-                .annotations(annotations)
-                .status("firing")
-                .build();
+            .labels(labels)
+            .content("test send msg! \\n This is the test data. It is proved that it can be received successfully")
+            .startAt(System.currentTimeMillis())
+            .activeAt(System.currentTimeMillis())
+            .endAt(System.currentTimeMillis())
+            .triggerTimes(4)
+            .annotations(annotations)
+            .status("firing")
+            .build();
        GroupAlert groupAlert = GroupAlert.builder()
-                .commonLabels(Map.of(CommonConstants.LABEL_ALERT_NAME, "CPU Usage Alert"))
-                .commonAnnotations(annotations)
-                .alerts(List.of(singleAlert1, singleAlert2))
-                .status("firing")
-                .build();
+            .commonLabels(Map.of(CommonConstants.LABEL_ALERT_NAME, "CPU Usage Alert"))
+            .commonAnnotations(annotations)
+            .alerts(List.of(singleAlert1, singleAlert2))
+            .status("firing")
+            .build();
        return dispatcherAlarm.sendNoticeMsg(noticeReceiver, null, groupAlert);
    }

@@ -41,7 +41,10 @@ import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.jpa.domain.Specification;

+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -379,4 +382,46 @@ class NoticeConfigServiceTest {
        assertEquals(1, matched.size());
        assertEquals(4L, matched.get(0).getId());
    }
+
+    @Test
+    void getReceiverFilterRuleMatchesPeriodContainingNow() {
+        ZonedDateTime now = ZonedDateTime.now();
+        List<NoticeRule> matched = filterWithPeriod(now.minusHours(6), now.plusHours(6));
+        assertEquals(1, matched.size());
+    }
+
+    @Test
+    void getReceiverFilterRuleFiltersPeriodExcludingNow() {
+        ZonedDateTime now = ZonedDateTime.now();
+        List<NoticeRule> matched = filterWithPeriod(now.plusHours(1), now.plusHours(2));
+        assertEquals(0, matched.size());
+    }
+
+    @Test
+    void getReceiverFilterRuleMatchesCrossMidnightPeriod() {
+        ZonedDateTime now = ZonedDateTime.now();
+        List<NoticeRule> matched = filterWithPeriod(now.minusHours(1), now.minusHours(2).plusDays(1));
+        assertEquals(1, matched.size());
+    }
+
+    @Test
+    void getReceiverFilterRuleNormalizesStoredOffsetToServerZone() {
+        ZonedDateTime now = ZonedDateTime.now();
+        List<NoticeRule> matched = filterWithPeriod(
+                now.minusHours(6).withZoneSameInstant(ZoneOffset.ofHours(-7)),
+                now.plusHours(6).withZoneSameInstant(ZoneOffset.ofHours(9)));
+        assertEquals(1, matched.size());
+    }
+
+    private List<NoticeRule> filterWithPeriod(ZonedDateTime periodStart, ZonedDateTime periodEnd) {
+        NoticeRule rule = new NoticeRule();
+        rule.setId(10L);
+        rule.setName("PeriodRule");
+        rule.setFilterAll(true);
+        rule.setPeriodStart(periodStart);
+        rule.setPeriodEnd(periodEnd);
+        CacheFactory.clearNoticeCache();
+        when(noticeRuleDao.findNoticeRulesByEnableTrue()).thenReturn(Collections.singletonList(rule));
+        return noticeConfigService.getReceiverFilterRule(new GroupAlert());
+    }
 }
@@ -40,6 +40,7 @@ class BackoffUtilsTest {
        ExponentialBackoff backoff = new ExponentialBackoff(10L, 100L);
        boolean shouldContinue = BackoffUtils.shouldContinueAfterBackoff(backoff);
        assertFalse(shouldContinue);
+        assertTrue(Thread.interrupted());
    }

    @Test
@@ -60,9 +61,14 @@ class BackoffUtilsTest {

        boolean shouldContinue = BackoffUtils.shouldContinueAfterBackoff(backoff);

+        // read and clear the restored interrupt status before join(),
+        // otherwise join() throws InterruptedException when the
+        // interrupting thread is still alive at this point
+        boolean interrupted = Thread.interrupted();
+
        interruptingThread.join();

        assertFalse(shouldContinue);
-        assertTrue(Thread.currentThread().isInterrupted());
+        assertTrue(interrupted);
    }
-}
+}