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();