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 714eda566 fix(alert): consolidate rule authoring correctness (#2965)
714eda566 is described below

commit 714eda5664eaf6c28bed3a52e44562066d39e99b
Author: aias00 <[email protected]>
AuthorDate: Wed Sep 2 15:08:56 2026 +0800

    fix(alert): consolidate rule authoring correctness (#2965)
    
    * Make producer connection drop alert triggerable
    
    Use a signed gauge delta for the bundled producer connection-drop rule so a 
drop of more than five producers can satisfy the PromQL expression. Keep the 
generated YAML and frontend mock aligned with the generator and lock the 
expression in backend and frontend regression tests.
    
    Constraint: changes() reports transition count and is non-negative, so 
changes(...) < -5 can never fire.
    
    Rejected: keep changes() and invert threshold | PromQL changes() cannot 
represent signed drops.
    
    Directive: keep server/scripts/gen_alert_rule_yaml.py, generated alert 
YAML, and web mock expression synchronized for bundled rules.
    
    Confidence: high
    
    Scope-risk: narrow
    
    Tested: 
JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home
 mvn -Dtest=AlertRuleAssetServiceTest,AlertRuleRequestDTOTest,AlertServiceTest 
test; npm test -- alertRuleAssetService.test.ts alerts.test.ts 
AlertsPage.test.tsx; npm run build; python3 -m py_compile 
server/scripts/gen_alert_rule_yaml.py; git diff --check
    Signed-off-by: liuhy <[email protected]>
    
    * Normalize alert rule form payload values
    
    Normalize legacy display metric and duration values before submitting alert 
rules so old saved rules and copied rules keep the backend API contract. Keep 
threshold unit derivation keyed by canonical metric names and cover legacy 
conversion in the page tests.
    
    Constraint: existing stored rules may still contain older display labels, 
but new submissions should use canonical metric and Prometheus duration values.
    
    Rejected: only changing select option values | edit and duplicate flows can 
still replay legacy stored labels without explicit normalization.
    
    Confidence: high
    
    Scope-risk: narrow
    
    Tested: 
JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home
 mvn -Dtest=AlertRuleAssetServiceTest,AlertRuleRequestDTOTest,AlertServiceTest 
test; npm test -- alertRuleAssetService.test.ts alerts.test.ts 
AlertsPage.test.tsx; npm run build; git diff --check
    Signed-off-by: liuhy <[email protected]>
    
    * Preserve composite alert durations in exports
    
    Share the alert rule request Prometheus duration grammar with export 
validation so composite durations such as 1h30m round-trip into generated YAML 
instead of falling back to 5m.
    
    Constraint: keep existing fallback behavior for invalid or missing 
durations.
    
    Rejected: duplicating a wider regex in AlertService | would let DTO 
validation and export validation drift again.
    
    Confidence: high
    
    Scope-risk: narrow
    
    Tested: 
JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home
 mvn -Dtest=AlertRuleAssetServiceTest,AlertRuleRequestDTOTest,AlertServiceTest 
test; 
JAVA_HOME=/Users/aias/Library/Java/JavaVirtualMachines/openjdk-21.0.2/Contents/Home
 mvn -DskipTests package; git diff --check
    Signed-off-by: liuhy <[email protected]>
    
    ---------
    
    Signed-off-by: liuhy <[email protected]>
---
 server/scripts/gen_alert_rule_yaml.py              |  2 +-
 .../studio/ops/alert/AlertRuleRequestDTO.java      |  4 +-
 .../rocketmq/studio/ops/alert/AlertService.java    |  3 +-
 .../alerts/rocketmq-client-connection-drop.yaml    |  2 +-
 .../ops/alert/AlertRuleAssetServiceTest.java       | 24 +++++++++++
 .../studio/ops/alert/AlertRuleRequestDTOTest.java  |  9 +++++
 .../studio/ops/alert/AlertServiceTest.java         | 18 +++++++++
 web/src/mock/alertRuleAssets.ts                    |  2 +-
 web/src/pages/ops/__tests__/alerts.test.ts         | 19 +++++++--
 web/src/pages/ops/alertRulePayload.ts              | 46 ++++++++++++++++++----
 web/src/pages/ops/alerts.tsx                       | 22 ++++++++---
 web/src/services/alertRuleAssetService.test.ts     |  8 ++++
 12 files changed, 136 insertions(+), 23 deletions(-)

diff --git a/server/scripts/gen_alert_rule_yaml.py 
b/server/scripts/gen_alert_rule_yaml.py
index cf1b0d3ac..17073f0a8 100644
--- a/server/scripts/gen_alert_rule_yaml.py
+++ b/server/scripts/gen_alert_rule_yaml.py
@@ -58,7 +58,7 @@ RULES = [
      'histogram_quantile(0.99, rate(rocketmq_dispatch_latency_bucket[5m])) > 
1', "5m", "warning", "topic",
      "Dispatch latency high", "99th percentile dispatch latency is above 1 
second."),
     ("rocketmq-client-connection-drop", "RocketMQClientConnectionDrop", 
"rocketmq-client.rules",
-     'changes(rocketmq_producer_count[5m]) < -5', "5m", "warning", "client",
+     'delta(rocketmq_producer_count[5m]) < -5', "5m", "warning", "client",
      "Client connections dropped", "More than 5 producer connections dropped 
in 5 minutes."),
     ("rocketmq-client-timeout", "RocketMQClientTimeout", 
"rocketmq-client.rules",
      'rocketmq_send_to_client_latency > 3000', "5m", "warning", "client",
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTO.java
 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTO.java
index 9fcffdab2..6d8f5a997 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTO.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTO.java
@@ -26,6 +26,8 @@ import java.util.List;
 
 @Data
 public class AlertRuleRequestDTO {
+    static final String PROMETHEUS_DURATION_REGEXP = 
"(?:[0-9]+(?:ms|s|m|h|d|w|y))+";
+
     private Long id;
     @NotBlank(message = "name is required")
     private String name;
@@ -34,7 +36,7 @@ public class AlertRuleRequestDTO {
     private String operator;
     private double threshold;
     private String thresholdUnit;
-    @Pattern(regexp = "(?:[0-9]+(?:ms|s|m|h|d|w|y))+", message = "duration is 
invalid")
+    @Pattern(regexp = PROMETHEUS_DURATION_REGEXP, message = "duration is 
invalid")
     private String duration;
     @Pattern(regexp = "LAST|MAX|MIN|AVG|SUM", flags = 
Pattern.Flag.CASE_INSENSITIVE,
             message = "aggregation is invalid")
diff --git 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
index 6b69fbe21..ab9f1e2e8 100644
--- 
a/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
+++ 
b/server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java
@@ -46,7 +46,8 @@ public class AlertService {
 
     private static final Set<String> VALID_OPERATORS = Set.of(">", ">=", "<", 
"<=", "==", "!=", "UNAVAILABLE");
     private static final Pattern METRIC_NAME_PATTERN = 
Pattern.compile("^[a-zA-Z_:][a-zA-Z0-9_:]*$");
-    private static final Pattern DURATION_PATTERN = 
Pattern.compile("^\\d+(ms|s|m|h|d|w|y)$");
+    private static final Pattern DURATION_PATTERN = Pattern.compile(
+            "^" + AlertRuleRequestDTO.PROMETHEUS_DURATION_REGEXP + "$");
 
     private final AlertRepository alertRepository;
     private final AlertStateRepository alertStateRepository;
diff --git 
a/server/src/main/resources/alerts/rocketmq-client-connection-drop.yaml 
b/server/src/main/resources/alerts/rocketmq-client-connection-drop.yaml
index 9857c9c22..c5e0a8992 100644
--- a/server/src/main/resources/alerts/rocketmq-client-connection-drop.yaml
+++ b/server/src/main/resources/alerts/rocketmq-client-connection-drop.yaml
@@ -6,7 +6,7 @@ groups:
   - name: rocketmq-client.rules
     rules:
       - alert: RocketMQClientConnectionDrop
-        expr: changes(rocketmq_producer_count[5m]) < -5
+        expr: delta(rocketmq_producer_count[5m]) < -5
         for: 5m
         labels:
           severity: warning
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleAssetServiceTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleAssetServiceTest.java
index 664bbe800..74bb73430 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleAssetServiceTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleAssetServiceTest.java
@@ -24,6 +24,8 @@ import 
org.springframework.core.io.support.ResourcePatternResolver;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -101,6 +103,28 @@ class AlertRuleAssetServiceTest {
         assertTrue(hasCritical, "expected at least one critical rule");
         assertTrue(hasBroker, "expected at least one broker rule");
     }
+
+    @Test
+    void clientConnectionDropRuleShouldUseSignedGaugeDeltaTest() {
+        PrometheusAlertRule rule = service.loadDefaultRules().stream()
+                .filter(r -> "RocketMQClientConnectionDrop".equals(r.alert()))
+                .findFirst()
+                .orElseThrow(() -> new AssertionError("expected bundled client 
connection drop rule"));
+
+        assertEquals("delta(rocketmq_producer_count[5m]) < -5", rule.expr());
+        assertFalse(rule.expr().contains("changes(rocketmq_producer_count[5m]) 
< -5"),
+                "changes() counts value transitions and cannot produce a 
negative drop");
+    }
+
+    @Test
+    void generatorShouldUseSameTriggerableClientConnectionDropExpressionTest() 
throws IOException {
+        String generator = Files.readString(Path.of("scripts", 
"gen_alert_rule_yaml.py"));
+
+        assertTrue(generator.contains("'delta(rocketmq_producer_count[5m]) < 
-5'"));
+        assertFalse(generator.contains("'changes(rocketmq_producer_count[5m]) 
< -5'"),
+                "generator must not recreate a non-triggerable changes() drop 
rule");
+    }
+
     @Test
     void assetLoadingShouldSkipEmptyAndNonObjectYaml() {
         AlertRuleAssetService service = serviceWithResources(
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTOTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTOTest.java
index de1a6f6e6..8dc37df09 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTOTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertRuleRequestDTOTest.java
@@ -41,6 +41,15 @@ class AlertRuleRequestDTOTest {
                         "channel is unsupported");
     }
 
+    @Test
+    void durationShouldAcceptCompositePrometheusDurationTest() {
+        AlertRuleRequestDTO request = new AlertRuleRequestDTO();
+        request.setName("High Lag");
+        request.setDuration("1h30m");
+
+        assertThat(validator.validate(request)).isEmpty();
+    }
+
     @Test
     void toAlertRuleVOShouldTrimAndDeduplicateChannelsInInputOrderTest() {
         AlertRuleRequestDTO request = new AlertRuleRequestDTO();
diff --git 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
index b137d57a1..ef0e5a270 100644
--- 
a/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
+++ 
b/server/src/test/java/org/apache/rocketmq/studio/ops/alert/AlertServiceTest.java
@@ -241,6 +241,24 @@ class AlertServiceTest {
                 .contains("description: \"Lag too high\"");
     }
 
+    @Test
+    void 
exportPrometheusRulesYamlShouldPreserveCompositePrometheusDurationTest() {
+        AlertRuleVO rule = AlertRuleVO.builder()
+                .name("High Lag Alert")
+                .metric("rocketmq_consumer_lag_messages")
+                .operator(">")
+                .threshold(5000)
+                .duration("1h30m")
+                .description("Lag too high")
+                .enabled(true)
+                .build();
+        when(alertRepository.findAllRules()).thenReturn(List.of(rule));
+
+        String result = alertService.exportPrometheusRulesYaml();
+
+        assertThat(result).contains("for: 1h30m").doesNotContain("for: 5m");
+    }
+
     @Test
     void exportPrometheusRulesYamlShouldExcludeNativeClusterRulesTest() {
         AlertRuleVO legacy = AlertRuleVO.builder()
diff --git a/web/src/mock/alertRuleAssets.ts b/web/src/mock/alertRuleAssets.ts
index 5ea804704..abd6cbbc5 100644
--- a/web/src/mock/alertRuleAssets.ts
+++ b/web/src/mock/alertRuleAssets.ts
@@ -138,7 +138,7 @@ export const mockAlertRuleAssets: MockAlertRuleAsset[] = [
       'rocketmq-client-connection-drop',
       'rocketmq-client.rules',
       'RocketMQClientConnectionDrop',
-      'changes(rocketmq_producer_count[5m]) < -5',
+      'delta(rocketmq_producer_count[5m]) < -5',
       'warning',
     ),
   },
diff --git a/web/src/pages/ops/__tests__/alerts.test.ts 
b/web/src/pages/ops/__tests__/alerts.test.ts
index 1da7b0e9f..8be4dee74 100644
--- a/web/src/pages/ops/__tests__/alerts.test.ts
+++ b/web/src/pages/ops/__tests__/alerts.test.ts
@@ -20,8 +20,8 @@ import { attachThresholdUnit } from '../alertRulePayload';
 
 describe('attachThresholdUnit', () => {
   it('derives the threshold unit from the selected metric', () => {
-    expect(attachThresholdUnit({ metric: 'Broker 离线', threshold: 1 
})).toEqual({
-      metric: 'Broker 离线',
+    expect(attachThresholdUnit({ metric: 'rocketmq_broker_offline', threshold: 
1 })).toEqual({
+      metric: 'rocketmq_broker_offline',
       threshold: 1,
       thresholdUnit: '个',
     });
@@ -30,14 +30,25 @@ describe('attachThresholdUnit', () => {
   it('overwrites stale units when a metric changes', () => {
     expect(
       attachThresholdUnit({
-        metric: '消费堆积量',
+        metric: 'rocketmq_consumer_lag_messages',
         threshold: 100,
         thresholdUnit: '%',
       }),
     ).toEqual({
-      metric: '消费堆积量',
+      metric: 'rocketmq_consumer_lag_messages',
       threshold: 100,
       thresholdUnit: '条',
     });
   });
+
+  it('normalizes legacy display values before submitting them to the backend', 
() => {
+    expect(attachThresholdUnit({ metric: '磁盘使用率', duration: '5分钟', threshold: 
85 })).toEqual(
+      {
+        metric: 'rocketmq_disk_use_ratio',
+        duration: '5m',
+        threshold: 85,
+        thresholdUnit: '%',
+      },
+    );
+  });
 });
diff --git a/web/src/pages/ops/alertRulePayload.ts 
b/web/src/pages/ops/alertRulePayload.ts
index 366fd1826..b3114a446 100644
--- a/web/src/pages/ops/alertRulePayload.ts
+++ b/web/src/pages/ops/alertRulePayload.ts
@@ -15,19 +15,49 @@
  * limitations under the License.
  */
 
+export const legacyMetricValues: Record<string, string> = {
+  磁盘使用率: 'rocketmq_disk_use_ratio',
+  消费堆积量: 'rocketmq_consumer_lag_messages',
+  'TPS 异常': 'rocketmq_tps',
+  'Broker 离线': 'rocketmq_broker_offline',
+  'Proxy 连接数': 'rocketmq_proxy_connections',
+};
+
+const legacyDurationValues: Record<string, string> = {
+  '1分钟': '1m',
+  '5分钟': '5m',
+  '15分钟': '15m',
+  '30分钟': '30m',
+};
+
 export const thresholdUnits: Record<string, string> = {
-  磁盘使用率: '%',
-  消费堆积量: '条',
-  'TPS 异常': 'TPS',
-  'Broker 离线': '个',
-  'Proxy 连接数': '个',
+  rocketmq_disk_use_ratio: '%',
+  rocketmq_consumer_lag_messages: '条',
+  rocketmq_tps: 'TPS',
+  rocketmq_broker_offline: '个',
+  rocketmq_proxy_connections: '个',
 };
 
-export function attachThresholdUnit<T extends { metric: string }>(
+export function normalizeMetric(metric: string): string {
+  return legacyMetricValues[metric] ?? metric;
+}
+
+export function normalizeDuration(duration: string): string {
+  return legacyDurationValues[duration] ?? duration;
+}
+
+export function attachThresholdUnit<T extends { metric: string; duration?: 
string }>(
   values: T,
-): T & { thresholdUnit: string } {
+): Omit<T, 'metric' | 'duration'> & {
+  metric: string;
+  duration?: string;
+  thresholdUnit: string;
+} {
+  const metric = normalizeMetric(values.metric);
   return {
     ...values,
-    thresholdUnit: thresholdUnits[values.metric] ?? '',
+    metric,
+    ...(values.duration === undefined ? {} : { duration: 
normalizeDuration(values.duration) }),
+    thresholdUnit: thresholdUnits[metric] ?? '',
   };
 }
diff --git a/web/src/pages/ops/alerts.tsx b/web/src/pages/ops/alerts.tsx
index 6bc364ea7..26013d3b2 100644
--- a/web/src/pages/ops/alerts.tsx
+++ b/web/src/pages/ops/alerts.tsx
@@ -61,6 +61,7 @@ import {
   testAlertRule,
   updateAlertRule,
 } from '../../services/opsService';
+import { attachThresholdUnit, normalizeDuration, normalizeMetric } from 
'./alertRulePayload';
 import { tableScrollX } from '../../utils/table';
 import { formatDateTime } from '../../utils/format';
 import { listInstances } from '../../services/instanceService';
@@ -340,7 +341,11 @@ const AlertsPage = ({ domain = 'CLUSTER' }: 
AlertsPageProps) => {
 
   const openEditModal = (rule: AlertRule) => {
     setEditingRule(rule);
-    form.setFieldsValue(rule);
+    form.setFieldsValue({
+      ...rule,
+      metric: normalizeMetric(rule.metric),
+      duration: normalizeDuration(rule.duration),
+    });
     setSelectedInstanceId(rule.instanceId);
     if (rule.instanceId?.trim()) {
       void loadMetricCapabilities(rule.instanceId, false);
@@ -355,7 +360,12 @@ const AlertsPage = ({ domain = 'CLUSTER' }: 
AlertsPageProps) => {
     setTestResult(null);
     setSelectedInstanceId(rule.instanceId);
     const { id: _id, lastTriggered: _lastTriggered, ...copy } = rule;
-    form.setFieldsValue({ ...copy, name: t('alerts.duplicateName', { name: 
rule.name }) });
+    form.setFieldsValue({
+      ...copy,
+      name: t('alerts.duplicateName', { name: rule.name }),
+      metric: normalizeMetric(copy.metric),
+      duration: normalizeDuration(copy.duration),
+    });
     if (rule.instanceId?.trim()) {
       void loadMetricCapabilities(rule.instanceId, false);
     } else {
@@ -615,8 +625,8 @@ const AlertsPage = ({ domain = 'CLUSTER' }: 
AlertsPageProps) => {
     try {
       const values = await form.validateFields();
       const payload = {
-        ...values,
-        ...(nativeRatioMetrics.has(values.metric) ? { thresholdUnit: '%' } : 
{}),
+        ...attachThresholdUnit(values),
+        ...(nativeRatioMetrics.has(normalizeMetric(values.metric)) ? { 
thresholdUnit: '%' } : {}),
       } as Partial<AlertRule>;
       setSubmitting(true);
       if (editingRule) {
@@ -649,8 +659,8 @@ const AlertsPage = ({ domain = 'CLUSTER' }: 
AlertsPageProps) => {
     try {
       const values = await form.validateFields();
       const payload = {
-        ...values,
-        ...(nativeRatioMetrics.has(values.metric) ? { thresholdUnit: '%' } : 
{}),
+        ...attachThresholdUnit(values),
+        ...(nativeRatioMetrics.has(normalizeMetric(values.metric)) ? { 
thresholdUnit: '%' } : {}),
       } as Partial<AlertRule>;
       setTesting(true);
       const result = await (domain === 'CLUSTER'
diff --git a/web/src/services/alertRuleAssetService.test.ts 
b/web/src/services/alertRuleAssetService.test.ts
index 281a014bc..2c545f713 100644
--- a/web/src/services/alertRuleAssetService.test.ts
+++ b/web/src/services/alertRuleAssetService.test.ts
@@ -54,6 +54,14 @@ describe('alertRuleAssetService (mock mode)', () => {
     expect(yaml).toContain('RocketMQBrokerDown');
   });
 
+  it('uses a triggerable producer connection drop expression in mock assets', 
async () => {
+    mode.mock = true;
+    const yaml = await getAlertRuleAsset('rocketmq-client-connection-drop');
+
+    expect(yaml).toContain('expr: delta(rocketmq_producer_count[5m]) < -5');
+    expect(yaml).not.toContain('changes(rocketmq_producer_count[5m]) < -5');
+  });
+
   it('throws for an unknown asset', async () => {
     mode.mock = true;
     await expect(getAlertRuleAsset('does-not-exist')).rejects.toThrow();

Reply via email to