This is an automated email from the ASF dual-hosted git repository.
liuhongyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new 256302fdcf [fix] Fix antlr4 parsing of `or` and `unless` logical and
set binary operators (#3488)
256302fdcf is described below
commit 256302fdcf0b05ca5d9195e7b5f731c99e1501fb
Author: Duansg <[email protected]>
AuthorDate: Sun Jun 22 19:46:34 2025 +0800
[fix] Fix antlr4 parsing of `or` and `unless` logical and set binary
operators (#3488)
Co-authored-by: wordofwordof <[email protected]>
Co-authored-by: aias00 <[email protected]>
---
.../alert/expr/AlertExpressionEvalVisitor.java | 107 ++++-------
.../alert/expr/AlertExpressionEvalVisitorTest.java | 198 +++++++++++++++++++--
.../alert/service/DataSourceServiceTest.java | 17 +-
3 files changed, 227 insertions(+), 95 deletions(-)
diff --git
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitor.java
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitor.java
index 31eb63f592..2056d0343c 100644
---
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitor.java
+++
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitor.java
@@ -115,92 +115,51 @@ public class AlertExpressionEvalVisitor extends
AlertExpressionBaseVisitor<List<
List<Map<String, Object>> leftOperand = visit(ctx.left);
List<Map<String, Object>> rightOperand = visit(ctx.right);
- Map<String, Object> leftMap = null;
- boolean leftMatch = false;
- Map<String, Object> rightMap = null;
- boolean rightMatch = false;
- for (Map<String, Object> item : leftOperand) {
- if (leftMap == null) {
- leftMap = item;
- }
- if (item.get(VALUE) != null) {
- leftMap = item;
- leftMatch = true;
- break;
- }
- }
- for (Map<String, Object> item : rightOperand) {
- if (rightMap == null) {
- rightMap = item;
- }
- if (item.get(VALUE) != null) {
- rightMap = item;
- rightMatch = true;
- break;
+ // build a hashMap of the left-hand label collection
+ Map<String, Map<String, Object>> leftLabelMap = leftOperand.stream()
+ .filter(item -> item.get(VALUE) != null)
+ .collect(Collectors.toMap(this::labelKey, HashMap::new, (k1,
k2) -> k1));
+
+ // first add all the non-empty items on the left side
+ List<Map<String, Object>> results = new
ArrayList<>(leftLabelMap.values());
+
+ // add the term that has a value on the right side and not on the left
side
+ for (Map<String, Object> rightItem : rightOperand) {
+ Object rightVal = rightItem.get(VALUE);
+ if (rightVal == null) {
+ continue;
}
- }
- if (leftMatch && rightMatch) {
- rightMap.putAll(leftMap);
- return new LinkedList<>(List.of(rightMap));
- } else if (leftMatch) {
- return new LinkedList<>(List.of(leftMap));
- } else if (rightMatch) {
- return new LinkedList<>(List.of(rightMap));
- } else {
- if (leftMap != null && rightMap != null) {
- rightMap.putAll(leftMap);
- return new LinkedList<>(List.of(rightMap));
- } else if (leftMap != null) {
- return new LinkedList<>(List.of(leftMap));
- } else if (rightMap != null) {
- return new LinkedList<>(List.of(rightMap));
+ String key = labelKey(rightItem);
+ if (!leftLabelMap.containsKey(key)) {
+ results.add(new HashMap<>(rightItem));
}
}
- return new LinkedList<>();
+ return results;
}
@Override
public List<Map<String, Object>>
visitUnlessExpr(AlertExpressionParser.UnlessExprContext ctx) {
List<Map<String, Object>> leftOperand = visit(ctx.left);
List<Map<String, Object>> rightOperand = visit(ctx.right);
- Map<String, Object> leftMap = null;
- boolean leftMatch = false;
- Map<String, Object> rightMap = null;
- boolean rightMatch = false;
- for (Map<String, Object> item : leftOperand) {
- if (leftMap == null) {
- leftMap = item;
- }
- if (item.get(VALUE) != null) {
- leftMap = item;
- leftMatch = true;
- break;
- }
- }
- for (Map<String, Object> item : rightOperand) {
- if (rightMap == null) {
- rightMap = item;
- }
- if (item.get(VALUE) != null) {
- rightMap = item;
- rightMatch = true;
- break;
+ List<Map<String, Object>> results = new ArrayList<>();
+
+ // build a hash set of the right-side tag collection
+ Set<String> rightLabelSet = rightOperand.stream()
+ .filter(item -> item.get(VALUE) != null)
+ .map(this::labelKey)
+ .collect(Collectors.toSet());
+
+ // iterate over the left side, O(1) match
+ for (Map<String, Object> leftItem : leftOperand) {
+ Object leftVal = leftItem.get(VALUE);
+ if (leftVal == null) {
+ continue;
}
- }
- if (leftMatch && !rightMatch) {
- return new LinkedList<>(List.of(leftMap));
- } else {
- if (leftMap != null) {
- leftMap.put(VALUE, null);
- return new LinkedList<>(List.of(leftMap));
- } else {
- if (rightMap != null) {
- rightMap.put(VALUE, null);
- return new LinkedList<>(List.of(rightMap));
- }
+ if (!rightLabelSet.contains(labelKey(leftItem))) {
+ results.add(new HashMap<>(leftItem));
}
}
- return new LinkedList<>();
+ return results;
}
@Override
diff --git
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitorTest.java
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitorTest.java
index 7be155fcf6..2ee048e1d6 100644
---
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitorTest.java
+++
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/expr/AlertExpressionEvalVisitorTest.java
@@ -24,12 +24,14 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
+import java.util.ArrayList;
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.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.when;
/**
@@ -240,11 +242,187 @@ class AlertExpressionEvalVisitorTest {
assertEquals(10.0, result.get(0).get("__value__"));
}
+ @Test
+ void testUnlessOpPromql() {
+ String promql = "http_server_requests_seconds_count > 10 unless
http_server_requests_seconds_max > 0";
+
+ Map<String, Object> countValue1 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 1307);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_count");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/actuator/prometheus");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+ Map<String, Object> maxValue1 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 10.007799125);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_max");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/actuator/health");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+
when(mockExecutor.execute("http_server_requests_seconds_count")).thenReturn(List.of(countValue1));
+
when(mockExecutor.execute("http_server_requests_seconds_max")).thenReturn(List.of(maxValue1));
+ List<Map<String, Object>> result = evaluate(promql);
+ assertEquals(1, result.size());
+ assertEquals(1307, result.get(0).get("__value__"));
+
+ maxValue1.put("uri", "/actuator/prometheus");
+ result = evaluate(promql);
+ assertEquals(0, result.size());
+
+ Map<String, Object> sumValue1 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 20.018);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_sum");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/**");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+ maxValue1.put("uri", "/actuator/health");
+
when(mockExecutor.execute("http_server_requests_seconds_sum")).thenReturn(List.of(sumValue1));
+ promql = "(http_server_requests_seconds_count > 10 unless
http_server_requests_seconds_max > 0) unless http_server_requests_seconds_sum >
10";
+ result = evaluate(promql);
+ assertEquals(1, result.size());
+ assertEquals(1307, result.get(0).get("__value__"));
+ }
+
+ @Test
+ void testOrOpPromql() {
+ String promql = "http_server_requests_seconds_count > 10 or
jvm_threads_states_threads > 0";
+
+ Map<String, Object> countValue1 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 1307);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_count");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/actuator/prometheus");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+ Map<String, Object> countValue2 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 16);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_count");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/**");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+ Map<String, Object> countValue3 = new HashMap<>() {
+ {
+ put("exception", "none");
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 7);
+ put("method", "GET");
+ put("__name__", "http_server_requests_seconds_count");
+ put("__timestamp__", "1.750320922467E9");
+ put("error", "none");
+ put("job", "spring-boot-app");
+ put("uri", "/actuator/health");
+ put("outcome", "SUCCESS");
+ put("status", "200");
+ }
+ };
+
+ Map<String, Object> threadsValue1 = new HashMap<>() {
+ {
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 10.007799125);
+ put("__name__", "jvm_threads_states_threads");
+ put("__timestamp__", "1.750320922467E9");
+ put("job", "spring-boot-app");
+ put("state=", "runnable");
+ }
+ };
+
+ Map<String, Object> threadsValue2 = new HashMap<>() {
+ {
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 1);
+ put("__name__", "jvm_threads_states_threads");
+ put("__timestamp__", "1.750320922467E9");
+ put("job", "spring-boot-app");
+ put("state=", "timed-waiting");
+ }
+ };
+
+ Map<String, Object> threadsValue3 = new HashMap<>() {
+ {
+ put("instance", "host.docker.internal:8989");
+ put("__value__", 19.02);
+ put("__name__", "jvm_threads_states_threads");
+ put("__timestamp__", "1.750320922467E9");
+ put("job", "spring-boot-app");
+ put("state=", "waiting");
+ }
+ };
+
+
when(mockExecutor.execute("http_server_requests_seconds_count")).thenReturn(List.of(countValue1,
countValue2, countValue3));
+
when(mockExecutor.execute("jvm_threads_states_threads")).thenReturn(List.of(threadsValue1,
threadsValue2, threadsValue3));
+ List<Map<String, Object>> result = evaluate(promql);
+ assertEquals(5, result.size());
+ assertTrue(result.stream().allMatch(t -> null != t.get("__value__")));
+
+
+
when(mockExecutor.execute("jvm_threads_states_threads")).thenReturn(new
ArrayList<>());
+ result = evaluate(promql);
+ assertEquals(2, result.size());
+ assertTrue(result.stream().allMatch(t -> null != t.get("__value__")));
+ assertEquals(1307, result.get(0).get("__value__"));
+
+
when(mockExecutor.execute("http_server_requests_seconds_count")).thenReturn(new
ArrayList<>());
+
when(mockExecutor.execute("jvm_threads_states_threads")).thenReturn(List.of(threadsValue1,
threadsValue2, threadsValue3));
+ result = evaluate(promql);
+ assertEquals(3, result.size());
+ assertTrue(result.stream().allMatch(t -> null != t.get("__value__")));
+ assertEquals(10.007799125, result.get(0).get("__value__"));
+ }
+
@Test
void testMultipleUnlessConditions() {
- when(mockExecutor.execute("metric1")).thenReturn(List.of(new
HashMap<>(Map.of("__value__", 40.0))));
- when(mockExecutor.execute("metric2")).thenReturn(List.of(new
HashMap<>(Map.of("__value__", 50.0))));
- when(mockExecutor.execute("metric3")).thenReturn(List.of(new
HashMap<>(Map.of("__value__", 60.0))));
+ when(mockExecutor.execute("metric1")).thenReturn(List.of(new
HashMap<>(Map.of("job", "api", "__value__", 40.0))));
+ when(mockExecutor.execute("metric2")).thenReturn(List.of(new
HashMap<>(Map.of("job", "web", "__value__", 50.0))));
+ when(mockExecutor.execute("metric3")).thenReturn(List.of(new
HashMap<>(Map.of("job", "api", "__value__", 60.0))));
when(mockExecutor.execute("select cpu_usage from metrics where service
= 'web'")).thenReturn(
List.of(new HashMap<>(Map.of("__value__", 40.0))));
when(mockExecutor.execute("select memory_usage from metrics where
service = 'db'")).thenReturn(
@@ -252,15 +430,13 @@ class AlertExpressionEvalVisitorTest {
when(mockExecutor.execute("select disk_usage from metrics where
service = 'cache'")).thenReturn(
List.of(new HashMap<>(Map.of("__value__", 60.0))));
// promql
- List<Map<String, Object>> result = evaluate("metric1 > 30 unless
metric2 > 45 unless metric3 < 70");
- assertEquals(1, result.size());
- assertNull(result.get(0).get("__value__"));
+ List<Map<String, Object>> result = evaluate("(metric1 > 30 unless
metric2 > 45) unless metric3 > 50");
+ assertEquals(0, result.size());
// sql
- result = evaluate("(select cpu_usage from metrics where service =
'web') > 30"
- + " unless (select memory_usage from metrics where service =
'db') > 45"
- + " unless (select disk_usage from metrics where service =
'cache') < 70");
- assertEquals(1, result.size());
- assertNull(result.get(0).get("__value__"));
+ result = evaluate("((select cpu_usage from metrics where service =
'web') > 30"
+ + " unless (select memory_usage from metrics where service =
'db') > 55)"
+ + " unless (select disk_usage from metrics where service =
'cache') > 50");
+ assertEquals(0, result.size());
}
@Test
diff --git
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/DataSourceServiceTest.java
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/DataSourceServiceTest.java
index f4e35cf21f..25e5100ee4 100644
---
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/DataSourceServiceTest.java
+++
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/DataSourceServiceTest.java
@@ -392,8 +392,7 @@ class DataSourceServiceTest {
dataSourceService.setExecutors(List.of(mockExecutor));
List<Map<String, Object>> result =
dataSourceService.calculate("promql", "node_cpu_seconds_total{mode=\"user\"} >
250 or node_cpu_seconds_total{mode=\"idle\"} < 20");
- assertEquals(1, result.size());
- assertNull(result.get(0).get("__value__"));
+ assertEquals(0, result.size());
}
@Test
@@ -414,8 +413,7 @@ class DataSourceServiceTest {
dataSourceService.setExecutors(List.of(mockExecutor));
List<Map<String, Object>> result =
dataSourceService.calculate("promql", "node_cpu_seconds_total{mode=\"user\"} >
50 or node_cpu_seconds_total{mode=\"idle\"} < 320");
- assertEquals(1, result.size());
- assertNotNull(result.get(0).get("__value__"));
+ assertEquals(4, result.size());
}
@Test
@@ -436,8 +434,8 @@ class DataSourceServiceTest {
dataSourceService.setExecutors(List.of(mockExecutor));
List<Map<String, Object>> result =
dataSourceService.calculate("promql", "node_cpu_seconds_total{mode=\"user\"} >
50 unless node_cpu_seconds_total{mode=\"idle\"} < 320");
- assertEquals(1, result.size());
- assertNull(result.get(0).get("__value__"));
+ assertEquals(2, result.size());
+ assertEquals(100.0, result.get(0).get("__value__"));
}
@Test
@@ -458,8 +456,8 @@ class DataSourceServiceTest {
dataSourceService.setExecutors(List.of(mockExecutor));
List<Map<String, Object>> result =
dataSourceService.calculate("promql", "node_cpu_seconds_total{mode=\"user\"} >
50 unless node_cpu_seconds_total{mode=\"idle\"} < 20");
- assertEquals(1, result.size());
- assertNotNull(result.get(0).get("__value__"));
+ assertEquals(2, result.size());
+ assertEquals(100.0, result.get(0).get("__value__"));
}
@Test
@@ -480,8 +478,7 @@ class DataSourceServiceTest {
dataSourceService.setExecutors(List.of(mockExecutor));
List<Map<String, Object>> result =
dataSourceService.calculate("promql", "node_cpu_seconds_total{mode=\"user\"} >
250 unless node_cpu_seconds_total{mode=\"idle\"} < 20");
- assertEquals(1, result.size());
- assertNull(result.get(0).get("__value__"));
+ assertEquals(0, result.size());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
