This is an automated email from the ASF dual-hosted git repository.

lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git


The following commit(s) were added to refs/heads/rocketmq-studio by this push:
     new f04dea2be fix(alert): suppression window, metric-key trimming and 
whole-scope proxy failure (#4119)
f04dea2be is described below

commit f04dea2be6c49fffde360e9af26291b22eee7374
Author: Zhao Jianing <[email protected]>
AuthorDate: Wed Sep 9 20:54:45 2026 +0800

    fix(alert): suppression window, metric-key trimming and whole-scope proxy 
failure (#4119)
    
    Four independent alerting defects, grouped because they all sit on the 
native alert evaluation path and each one alone leaves the others still 
misfiring.
    
    **Suppression dropped out while an incident was still firing.** 
`AlertNotificationSuppressionService.findSuppressingClusterAlert` only accepted 
a latest in-window event whose transition was `FIRING`. But `AlertStateMachine` 
emits `REMINDER` *while the state remains FIRING*, and the 30-minute 
association window is the same length as the default `reminderInterval` — so 
once a reminder was emitted, the original FIRING event had usually slid out of 
the window and the incident stopped supp [...]
    
    **Stored metric keys were compared untrimmed in two of the three places 
that read them.** `AlertRuleEvaluator` already matched samples with 
`rule.getMetric().trim()`, but `MybatisPlusAlertStateRepository.findActive` 
filtered with the raw value, so a rule whose stored metric had surrounding 
whitespace found no active states — `scopedRules` came back empty, 
`reconcileMissingActiveStates` concluded there was nothing active, and a stale 
FIRING alert was never resolved. `AlertRuleSemanticF [...]
    
    **A proxy discovery outage falsely resolved every firing proxy alert.** 
When `ApacheRocketMqProxyMetricsCollector` cannot discover proxies for an 
instance it emitted one unavailable sample labelled `proxyAddr=unknown`. 
Non-empty labels defeat `NativeAlertProcessor.containsWholeScopeFailure`, so 
reconciliation still ran, the placeholder fingerprint matched none of the real 
proxies' active states, and each of them was resolved — with resolution 
notifications — while the collector actual [...]
    
    Test note: the collector change is pinned by 
`ApacheRocketMqProxyMetricsCollectorTest.recordsWholeScopeFailureSampleWhenProxyDiscoveryFailsTest`,
 which drives the real collector and asserts the emitted labels are empty. A 
companion processor-level case was dropped during integration because it 
constructed the empty-label sample inline and therefore still passed with the 
collector fix reverted; the processor side of that path is already covered by 
the existing `doesNotResolveMissingAct [...]
    
    Folded in from #4124, #4126 and #4133, all by the same author and all on 
this path; those PRs are closed as superseded.
---
 .../ApacheRocketMqProxyMetricsCollector.java       |  4 ++-
 .../alert/AlertNotificationSuppressionService.java |  5 +++-
 .../ops/alert/AlertNotificationTemplate.java       |  3 ++-
 .../ops/alert/AlertRuleSemanticFingerprint.java    |  2 +-
 .../ops/alert/MybatisPlusAlertStateRepository.java |  2 +-
 .../ApacheRocketMqProxyMetricsCollectorTest.java   |  6 +++--
 .../AlertNotificationSuppressionServiceTest.java   | 16 ++++++++++++
 .../ops/alert/AlertNotificationTemplateTest.java   | 13 ++++++++++
 .../studio/ops/alert/AlertRuleEvaluatorTest.java   | 14 ++++++++++
 .../alert/MybatisPlusAlertStateRepositoryTest.java | 30 ++++++++++++++++++++++
 10 files changed, 88 insertions(+), 7 deletions(-)

diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollector.java
 
b/server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollector.java
index 2a384ab74..a79c4e138 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollector.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollector.java
@@ -79,7 +79,9 @@ public class ApacheRocketMqProxyMetricsCollector implements 
ClusterMetricsCollec
             return samples;
         } catch (RuntimeException error) {
             log.warn("Failed to discover proxies for instance {}: {}", 
instance.getName(), error.getMessage());
-            return List.of(unavailable(instance, null, Map.of("proxyAddr", 
"unknown"), collectedAt));
+            // no proxy is known at this point: emit the whole-scope failure 
marker (empty labels)
+            // so NativeAlertProcessor skips reconciliation instead of 
resolving active proxy alerts
+            return List.of(unavailable(instance, null, Map.of(), collectedAt));
         }
     }
 
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionService.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionService.java
index 9609af9ec..d8a1da304 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionService.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionService.java
@@ -67,7 +67,10 @@ public class AlertNotificationSuppressionService {
             page++;
         }
         return latestByIncident.values().stream()
-                .filter(candidate -> 
"FIRING".equalsIgnoreCase(candidate.getTransition()))
+                // REMINDER is emitted only while the state stays FIRING, so 
an incident whose
+                // latest in-window event is a REMINDER is still active; only 
RESOLVED ends it.
+                .filter(candidate -> 
"FIRING".equalsIgnoreCase(candidate.getTransition())
+                        || 
"REMINDER".equalsIgnoreCase(candidate.getTransition()))
                 .max(Comparator.comparing(SystemAlertVO::getTime, 
Comparator.nullsLast(Comparator.naturalOrder())));
     }
 
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplate.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplate.java
index 4c8d63247..2930d659f 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplate.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplate.java
@@ -62,7 +62,8 @@ final class AlertNotificationTemplate {
         if (alert.getCurrentValue() == null) {
             return "";
         }
-        if (rule != null && "%".equals(rule.getThresholdUnit()) && 
RATIO_METRICS.contains(rule.getMetric())) {
+        if (rule != null && "%".equals(rule.getThresholdUnit())
+                && RATIO_METRICS.contains(rule.getMetric() == null ? "" : 
rule.getMetric().trim())) {
             return String.valueOf(alert.getCurrentValue() * 100);
         }
         return String.valueOf(alert.getCurrentValue());
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleSemanticFingerprint.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleSemanticFingerprint.java
index fa2c06a27..a0748ed04 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleSemanticFingerprint.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleSemanticFingerprint.java
@@ -62,7 +62,7 @@ final class AlertRuleSemanticFingerprint {
     }
 
     static double normalizedThreshold(AlertRuleVO rule) {
-        if ("%".equals(rule.getThresholdUnit()) && 
RATIO_METRICS.contains(rule.getMetric())) {
+        if ("%".equals(rule.getThresholdUnit()) && 
RATIO_METRICS.contains(normalize(rule.getMetric()))) {
             return rule.getThreshold() / 100D;
         }
         return rule.getThreshold();
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepository.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepository.java
index b71d44628..0e2f94d01 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepository.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepository.java
@@ -103,7 +103,7 @@ public class MybatisPlusAlertStateRepository implements 
AlertStateRepository {
                 .filter(rule -> rule.getId() != null)
                 .filter(AlertRuleVO::isEnabled)
                 .filter(rule -> ruleDomain(rule) == scope.domain())
-                .filter(rule -> metricKeys.contains(rule.getMetric()))
+                .filter(rule -> 
metricKeys.contains(StringUtils.trimWhitespace(rule.getMetric())))
                 .filter(rule -> !StringUtils.hasText(rule.getInstanceId())
                         || scope.instanceId().equals(rule.getInstanceId()))
                 .collect(Collectors.toMap(AlertRuleVO::getId, rule -> rule, 
(left, right) -> left));
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollectorTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollectorTest.java
index b3fc26a8e..54501f474 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollectorTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/cluster/metrics/collectors/ApacheRocketMqProxyMetricsCollectorTest.java
@@ -82,7 +82,7 @@ class ApacheRocketMqProxyMetricsCollectorTest {
     }
 
     @Test
-    void recordsUnavailableSampleWhenProxyDiscoveryFailsTest() {
+    void recordsWholeScopeFailureSampleWhenProxyDiscoveryFailsTest() {
         ClusterService clusterService = mock(ClusterService.class);
         InstanceVO instance = 
InstanceVO.builder().name("local").endpoint("localhost:9876").build();
         doThrow(new IllegalStateException("nameserver 
unavailable")).when(clusterService).listClusters("local");
@@ -93,7 +93,9 @@ class ApacheRocketMqProxyMetricsCollectorTest {
         assertThat(samples).singleElement().satisfies(sample -> {
             assertThat(sample.metricKey()).isEqualTo("proxy.availability");
             
assertThat(sample.availability()).isEqualTo(MetricAvailability.UNAVAILABLE);
-            assertThat(sample.labels()).containsEntry("proxyAddr", "unknown");
+            // empty labels mark the whole-scope failure, keeping 
NativeAlertProcessor
+            // from reconciling (and falsely resolving) active proxy alerts
+            assertThat(sample.labels()).isEmpty();
         });
     }
 }
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionServiceTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionServiceTest.java
index e945a51a7..faf5a8e5c 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionServiceTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationSuppressionServiceTest.java
@@ -130,6 +130,22 @@ class AlertNotificationSuppressionServiceTest {
         
verify(repository).findAlertsPage(org.mockito.ArgumentMatchers.argThat(query -> 
query.page() == 2));
     }
 
+    @Test
+    void suppressesWhileTheSameClusterIncidentOnlyReminderRemainsTest() {
+        AlertRepository repository = mock(AlertRepository.class);
+        LocalDateTime now = LocalDateTime.now();
+        // The original FIRING fell out of the 30-minute correlation window; 
only the
+        // REMINDER the state machine emits while the incident stays FIRING is 
visible.
+        SystemAlertVO reminder = event(2L, AlertDomain.CLUSTER, "REMINDER", 
"broker-1", now.minusMinutes(5));
+        reminder.setFingerprint("broker-1-availability");
+        
when(repository.findAlertsPage(any())).thenReturn(PageResult.of(List.of(reminder),
 1, 1, 100));
+
+        Optional<SystemAlertVO> result = new 
AlertNotificationSuppressionService(repository)
+                .findSuppressingClusterAlert(event(3L, AlertDomain.BUSINESS, 
"FIRING", "broker-1", now));
+
+        assertThat(result).contains(reminder);
+    }
+
     private static SystemAlertVO event(Long id, AlertDomain domain, String 
transition, String brokerName,
             LocalDateTime time) {
         return 
SystemAlertVO.builder().id(id).domain(domain).transition(transition).instanceId("local")
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplateTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplateTest.java
index 08897ad8f..ed1a15110 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplateTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertNotificationTemplateTest.java
@@ -32,6 +32,19 @@ class AlertNotificationTemplateTest {
                 + "brokerAddr=127.0.0.1:10911, 
brokerName=broker-a|${missing}");
     }
 
+    @Test
+    void rendersPercentageValuesForPaddedStoredMetricsTest() {
+        AlertRuleVO rule = AlertRuleVO.builder().name("Disk 
threshold").metric(" broker.disk.usage_ratio ")
+                .threshold(85).thresholdUnit("%").build();
+        SystemAlertVO alert = 
SystemAlertVO.builder().level(AlertLevel.warning).title("Disk threshold")
+                
.description("FIRING").transition("FIRING").instanceId("local").currentValue(0.865)
+                .time(LocalDateTime.of(2026, 8, 23, 12, 
0)).labels(Map.of()).build();
+
+        String rendered = 
AlertNotificationTemplate.render("${value}${thresholdUnit}", alert, rule);
+
+        assertThat(rendered).isEqualTo("86.5%");
+    }
+
     @Test
     void usesTheExistingNotificationFormatWhenNoTemplateWasConfiguredTest() {
         SystemAlertVO alert = 
SystemAlertVO.builder().level(AlertLevel.info).title("Test")
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java
index b55fa7c1c..89bab3f94 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleEvaluatorTest.java
@@ -50,6 +50,20 @@ class AlertRuleEvaluatorTest {
         assertThat(result.conditionMet()).isTrue();
     }
 
+    @Test
+    void appliesPercentageThresholdsToPaddedStoredMetricsTest() {
+        // The evaluator matches padded stored metrics via 
rule.getMetric().trim()
+        // (legacy rows); the % normalization must see the same trimmed value 
or a
+        // " broker.disk.usage_ratio > 85%" rule compares 0.9 >= 85 and never 
fires.
+        AlertRuleVO rule = 
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric(" 
broker.disk.usage_ratio ")
+                
.operator(">=").threshold(85).thresholdUnit("%").enabled(true).build();
+
+        AlertEvaluationResult result = evaluator.evaluate(rule, 
sample(MetricAvailability.AVAILABLE, 0.9));
+
+        assertThat(result.matches()).isTrue();
+        assertThat(result.conditionMet()).isTrue();
+    }
+
     @Test
     void unavailableMetricDoesNotBehaveAsZeroTest() {
         AlertRuleVO rule = 
AlertRuleVO.builder().domain(AlertDomain.CLUSTER).metric("broker.disk.usage_ratio")
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepositoryTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepositoryTest.java
index 4598d69b8..36dbd7f60 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepositoryTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/MybatisPlusAlertStateRepositoryTest.java
@@ -96,6 +96,36 @@ class MybatisPlusAlertStateRepositoryTest {
         verify(alertMapper, never()).selectOne(any());
     }
 
+    @Test
+    void findsActiveStatesForRulesWithPaddedStoredMetricsTest() {
+        // NativeAlertProcessor keeps rules whose stored metric has 
surrounding whitespace
+        // (legacy rows) by matching trimWhitespace(rule.getMetric()) against 
the scope keys;
+        // findActive must apply the same normalization or reconcile sees no 
active states
+        // and the stale FIRING/ACKED state is never resolved.
+        RmqAlertStateMapper mapper = mock(RmqAlertStateMapper.class);
+        RmqAlertState state = new RmqAlertState();
+        state.setRuleId(4L);
+        state.setFingerprint("fingerprint");
+        state.setStatus(AlertStateStatus.FIRING.name());
+        state.setConsecutiveHits(3);
+        state.setCurrentValue(30D);
+        when(mapper.selectList(any())).thenReturn(List.of(state));
+        RmqSystemAlertMapper alertMapper = mock(RmqSystemAlertMapper.class);
+        when(alertMapper.selectList(any())).thenReturn(List.of(
+                alert(4L, "fingerprint", "local", 
"{\"consumerGroup\":\"orders\"}")));
+        MybatisPlusAlertStateRepository repository = new 
MybatisPlusAlertStateRepository(mapper, alertMapper);
+        AlertRuleVO rule = 
AlertRuleVO.builder().id(4L).domain(AlertDomain.BUSINESS).enabled(true)
+                .instanceId("local").metric(" consumer.lag.total ").build();
+
+        List<ActiveAlertState> active = repository.findActive(new 
MetricCollectionScope(AlertDomain.BUSINESS,
+                "local", Set.of("consumer.lag.total")), List.of(rule));
+
+        assertThat(active).singleElement().satisfies(item -> {
+            assertThat(item.key()).isEqualTo(new AlertStateKey(4L, 
"fingerprint"));
+            
assertThat(item.state().status()).isEqualTo(AlertStateStatus.FIRING);
+        });
+    }
+
     @Test
     void findsActiveStatesWithOneLatestAlertMetadataQueryTest() {
         RmqAlertStateMapper mapper = mock(RmqAlertStateMapper.class);

Reply via email to