You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2023/08/28 04:04:13 UTC
[skywalking] branch master updated: optimize and throws IllegalExpressionException in AlarmRule expression verify. (#11270)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new 23e7b88f2e optimize and throws IllegalExpressionException in AlarmRule expression verify. (#11270)
23e7b88f2e is described below
commit 23e7b88f2e97d92d3ea2776da6a27cad7ed5a628
Author: Wan Kai <wa...@foxmail.com>
AuthorDate: Mon Aug 28 12:04:07 2023 +0800
optimize and throws IllegalExpressionException in AlarmRule expression verify. (#11270)
---
.../oap/server/core/alarm/provider/AlarmRule.java | 21 +++++++++---------
.../server/core/alarm/provider/RulesReader.java | 7 +++++-
.../core/alarm/provider/AlarmRulesWatcherTest.java | 9 ++++----
.../core/alarm/provider/RunningRuleTest.java | 25 +++++++++++-----------
4 files changed, 35 insertions(+), 27 deletions(-)
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java
index 02c8c78e49..5c85bcbfd2 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRule.java
@@ -31,6 +31,7 @@ import org.antlr.v4.runtime.CharStreams;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.misc.ParseCancellationException;
import org.antlr.v4.runtime.tree.ParseTree;
+import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
import org.apache.skywalking.mqe.rt.exception.ParseErrorListener;
import org.apache.skywalking.mqe.rt.grammar.MQELexer;
import org.apache.skywalking.mqe.rt.grammar.MQEParser;
@@ -63,7 +64,7 @@ public class AlarmRule {
* Init includeMetrics and verify the expression.
* ValueColumnMetadata need init metrics info, don't invoke before the module finishes start.
*/
- public void setExpression(final String expression) {
+ public void setExpression(final String expression) throws IllegalExpressionException {
MQELexer lexer = new MQELexer(CharStreams.fromString(expression));
lexer.addErrorListener(new ParseErrorListener());
MQEParser parser = new MQEParser(new CommonTokenStream(lexer));
@@ -71,18 +72,18 @@ public class AlarmRule {
try {
tree = parser.expression();
} catch (ParseCancellationException e) {
- throw new IllegalArgumentException("expression:" + expression + " error: " + e.getMessage());
+ throw new IllegalExpressionException("expression:" + expression + " error: " + e.getMessage());
}
AlarmMQEVerifyVisitor visitor = new AlarmMQEVerifyVisitor();
ExpressionResult parseResult = visitor.visit(tree);
if (StringUtil.isNotBlank(parseResult.getError())) {
- throw new IllegalArgumentException("expression:" + expression + " error: " + parseResult.getError());
+ throw new IllegalExpressionException("expression:" + expression + " error: " + parseResult.getError());
}
if (!parseResult.isBoolResult()) {
- throw new IllegalArgumentException("expression:" + expression + " root operation is not a Compare Operation.");
+ throw new IllegalExpressionException("expression:" + expression + " root operation is not a Compare Operation.");
}
if (ExpressionResultType.SINGLE_VALUE != parseResult.getType()) {
- throw new IllegalArgumentException("expression:" + expression + " is not a SINGLE_VALUE result expression.");
+ throw new IllegalExpressionException("expression:" + expression + " is not a SINGLE_VALUE result expression.");
}
verifyIncludeMetrics(visitor.getIncludeMetrics());
@@ -90,12 +91,12 @@ public class AlarmRule {
this.includeMetrics = visitor.getIncludeMetrics();
}
- private void verifyIncludeMetrics(Set<String> includeMetrics) {
- includeMetrics.forEach(metricName -> {
+ private void verifyIncludeMetrics(Set<String> includeMetrics) throws IllegalExpressionException {
+ for (String metricName : includeMetrics) {
Optional<ValueColumnMetadata.ValueColumn> valueColumn = ValueColumnMetadata.INSTANCE.readValueColumnDefinition(
metricName);
if (valueColumn.isEmpty()) {
- throw new IllegalArgumentException("Metric: [" + metricName + "] dose not exist.");
+ throw new IllegalExpressionException("Metric: [" + metricName + "] dose not exist.");
}
Column.ValueDataType dataType = valueColumn.get().getDataType();
switch (dataType) {
@@ -103,9 +104,9 @@ public class AlarmRule {
case LABELED_VALUE:
return;
default:
- throw new IllegalArgumentException(
+ throw new IllegalExpressionException(
"Metric dose not supported in alarm, metric: [" + metricName + "] is not a common or labeled metric.");
}
- });
+ }
}
}
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
index df288c6cf1..66d5832afb 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
@@ -29,6 +29,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
+import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
import org.apache.skywalking.oap.server.core.alarm.provider.discord.DiscordSettings;
import org.apache.skywalking.oap.server.core.alarm.provider.pagerduty.PagerDutySettings;
import org.apache.skywalking.oap.server.core.alarm.provider.webhook.WebhookSettings;
@@ -94,7 +95,11 @@ public class RulesReader {
if (StringUtil.isEmpty((String) expression)) {
throw new IllegalArgumentException("expression can't be empty");
}
- alarmRule.setExpression(expression.toString());
+ try {
+ alarmRule.setExpression(expression.toString());
+ } catch (IllegalExpressionException e) {
+ throw new IllegalArgumentException(e);
+ }
alarmRule.setIncludeNames((ArrayList) settings.getOrDefault("include-names", new ArrayList(0)));
alarmRule.setExcludeNames((ArrayList) settings.getOrDefault("exclude-names", new ArrayList(0)));
alarmRule.setIncludeNamesRegex((String) settings.getOrDefault("include-names-regex", ""));
diff --git a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcherTest.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcherTest.java
index ab9d6c8a1c..da12da8dad 100644
--- a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcherTest.java
+++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRulesWatcherTest.java
@@ -18,6 +18,7 @@
package org.apache.skywalking.oap.server.core.alarm.provider;
+import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher;
import org.apache.skywalking.oap.server.core.query.enumeration.Scope;
import org.apache.skywalking.oap.server.core.query.sql.Function;
@@ -93,7 +94,7 @@ public class AlarmRulesWatcherTest {
}
@Test
- public void shouldKeepExistedRunningRuleIfAlarmRuleExists() {
+ public void shouldKeepExistedRunningRuleIfAlarmRuleExists() throws IllegalExpressionException {
AlarmRule rule = newAlarmRule("name1", "avg(service_percent) < 80");
Rules rules = new Rules();
rules.getRules().add(rule);
@@ -117,7 +118,7 @@ public class AlarmRulesWatcherTest {
}
@Test
- public void shouldRemoveRunningRuleIfAlarmRuleIsRemoved() {
+ public void shouldRemoveRunningRuleIfAlarmRuleIsRemoved() throws IllegalExpressionException {
AlarmRule rule = newAlarmRule("name1", "avg(service_percent) < 80");
Rules rules = new Rules();
rules.getRules().add(rule);
@@ -141,7 +142,7 @@ public class AlarmRulesWatcherTest {
}
@Test
- public void shouldReplaceRunningRuleIfAlarmRulesAreReplaced() {
+ public void shouldReplaceRunningRuleIfAlarmRulesAreReplaced() throws IllegalExpressionException {
AlarmRule rule = newAlarmRule("name1", "avg(service_percent) < 80");
Rules rules = new Rules();
rules.getRules().add(rule);
@@ -166,7 +167,7 @@ public class AlarmRulesWatcherTest {
assertEquals(1, alarmRulesWatcher.getRunningContext().get("avg(service_percent) < 99").size());
}
- private AlarmRule newAlarmRule(String name, String expression) {
+ private AlarmRule newAlarmRule(String name, String expression) throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName(name);
alarmRule.setIncludeNames(new ArrayList<String>() {
diff --git a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRuleTest.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRuleTest.java
index 43dbe96fc7..dc50eefc52 100644
--- a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRuleTest.java
+++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RunningRuleTest.java
@@ -21,6 +21,7 @@ package org.apache.skywalking.oap.server.core.alarm.provider;
import com.google.common.collect.Lists;
import lombok.Getter;
import lombok.Setter;
+import org.apache.skywalking.mqe.rt.exception.IllegalExpressionException;
import org.apache.skywalking.oap.server.core.Const;
import org.apache.skywalking.oap.server.core.alarm.AlarmCallback;
import org.apache.skywalking.oap.server.core.alarm.AlarmMessage;
@@ -70,7 +71,7 @@ public class RunningRuleTest {
}
@Test
- public void testInitAndStart() {
+ public void testInitAndStart() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 75) >= 3");
@@ -97,7 +98,7 @@ public class RunningRuleTest {
}
@Test
- public void testAlarm() {
+ public void testAlarm() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 75) >= 3");
@@ -129,7 +130,7 @@ public class RunningRuleTest {
}
@Test
- public void testMultipleValuesAlarm() {
+ public void testMultipleValuesAlarm() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_multiple_values_rule");
alarmRule.setExpression("sum(endpoint_multiple_values > 50) >= 3");
@@ -163,7 +164,7 @@ public class RunningRuleTest {
}
@Test
- public void testLabeledAlarm() {
+ public void testLabeledAlarm() throws IllegalExpressionException {
ValueColumnMetadata.INSTANCE.putIfAbsent(
"endpoint_labeled", "testColumn", Column.ValueDataType.LABELED_VALUE, Function.Avg, 0, Scope.Endpoint.getScopeId());
AlarmRule alarmRule = new AlarmRule();
@@ -177,16 +178,16 @@ public class RunningRuleTest {
}
@Test
- public void testMultipleMetricsAlarm() {
+ public void testMultipleMetricsAlarm() throws IllegalExpressionException {
multipleMetricsAlarm("sum((endpoint_percent < 75) * (endpoint_cpm < 100)) >= 3", 1);
}
@Test
- public void testMultipleMetricsNoAlarm() {
+ public void testMultipleMetricsNoAlarm() throws IllegalExpressionException {
multipleMetricsAlarm("sum((endpoint_percent < 75) * (endpoint_cpm < 99)) >= 3", 0);
}
- private void multipleMetricsAlarm(String expression, int alarmMsgSize) {
+ private void multipleMetricsAlarm(String expression, int alarmMsgSize) throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression(expression);
@@ -220,7 +221,7 @@ public class RunningRuleTest {
}
@Test
- public void testNoAlarm() {
+ public void testNoAlarm() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent > 75) >= 3");
@@ -265,7 +266,7 @@ public class RunningRuleTest {
}
@Test
- public void testSilence() {
+ public void testSilence() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 75) >= 3");
@@ -303,7 +304,7 @@ public class RunningRuleTest {
}
@Test
- public void testExclude() {
+ public void testExclude() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 75) >= 3");
@@ -335,7 +336,7 @@ public class RunningRuleTest {
}
@Test
- public void testIncludeNamesRegex() {
+ public void testIncludeNamesRegex() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 1000) >= 1");
@@ -368,7 +369,7 @@ public class RunningRuleTest {
}
@Test
- public void testExcludeNamesRegex() {
+ public void testExcludeNamesRegex() throws IllegalExpressionException {
AlarmRule alarmRule = new AlarmRule();
alarmRule.setAlarmRuleName("endpoint_percent_rule");
alarmRule.setExpression("sum(endpoint_percent < 1000) >= 1");