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 5441f58f4c MQE: check the metrics value before do binary operation to
improve robustness (#12483)
5441f58f4c is described below
commit 5441f58f4cf2a3404a5035ffd594d54a32800c23
Author: Wan Kai <[email protected]>
AuthorDate: Fri Jul 26 18:49:33 2024 +0800
MQE: check the metrics value before do binary operation to improve
robustness (#12483)
---
docs/en/changes/changes.md | 1 +
.../skywalking/mqe/rt/operation/BinaryOp.java | 2 +-
.../skywalking/mqe/rt/operation/CompareOp.java | 2 +-
.../apache/skywalking/mqe/rt/operation/LROp.java | 65 ++++++++++++++++++----
.../org/apache/skywalking/mqe/rt/BinaryOpTest.java | 38 +++++++++++++
.../org/apache/skywalking/mqe/rt/MockData.java | 7 +++
test/e2e-v2/script/env | 2 +-
7 files changed, 104 insertions(+), 13 deletions(-)
diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
index 12559cc48b..7a6d526e80 100644
--- a/docs/en/changes/changes.md
+++ b/docs/en/changes/changes.md
@@ -38,6 +38,7 @@
* Support BanyanDB internal stream query execution tracing.
* Fix Elasticsearch, MySQL, RabbitMQ dashboards typos and missing expressions.
* BanyanDB: Zipkin Module set service as Entity for improving the query
performance.
+* MQE: check the metrics value before do binary operation to improve
robustness.
#### UI
diff --git
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/BinaryOp.java
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/BinaryOp.java
index 00bfec134f..0c84aaf983 100644
---
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/BinaryOp.java
+++
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/BinaryOp.java
@@ -29,7 +29,7 @@ public class BinaryOp {
try {
return LROp.doLROp(left, right, opType, BinaryOp::scalarBinaryOp);
} catch (IllegalExpressionException e) {
- throw new IllegalExpressionException("Unsupported binary
operation.");
+ throw new IllegalExpressionException("Unsupported binary
operation: " + e.getMessage());
}
}
diff --git
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/CompareOp.java
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/CompareOp.java
index 499e7ca79e..030f3174fd 100644
---
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/CompareOp.java
+++
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/CompareOp.java
@@ -29,7 +29,7 @@ public class CompareOp {
try {
return LROp.doLROp(left, right, opType,
CompareOp::scalarCompareOp);
} catch (IllegalExpressionException e) {
- throw new IllegalExpressionException("Unsupported compare
operation.");
+ throw new IllegalExpressionException("Unsupported compare
operation: " + e.getMessage());
}
}
diff --git
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/LROp.java
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/LROp.java
index c86ae61893..4254b59898 100644
---
a/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/LROp.java
+++
b/oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/LROp.java
@@ -91,7 +91,17 @@ public interface LROp {
private static ExpressionResult single2SingleNoLabeled(ExpressionResult
singleLeft,
ExpressionResult
singleRight,
- int opType, LROp
calculate) {
+ int opType, LROp
calculate) throws IllegalExpressionException {
+ if (singleLeft.getResults().isEmpty() ||
+ singleLeft.getResults().get(0).getValues().size() != 1) {
+ throw new IllegalExpressionException("Single to Single, left
result is empty or has more than one value.");
+ }
+
+ if (singleRight.getResults().isEmpty() ||
+ singleRight.getResults().get(0).getValues().size() != 1) {
+ throw new IllegalExpressionException("Single to Single, right
result is empty or has more than one value.");
+ }
+
ExpressionResult result = new ExpressionResult();
MQEValue mqeValue = new MQEValue();
MQEValues mqeValues = new MQEValues();
@@ -119,12 +129,18 @@ public interface LROp {
labelMapR.put(new HashSet<>(mqeValuesR.getMetric().getLabels()),
mqeValuesR.getValues());
});
for (MQEValues mqeValuesL : singleLeft.getResults()) {
+ if (mqeValuesL.getValues().size() != 1) {
+ throw new IllegalExpressionException("Single Labeled to Single
Labeled, left labeled result is empty or has more than one value.");
+ }
//reserve left metric info
MQEValue valueL = mqeValuesL.getValues().get(0);
List<MQEValue> mqeValuesR = labelMapR.get(new
HashSet<>(mqeValuesL.getMetric().getLabels()));
if (mqeValuesR == null) {
valueL.setEmptyValue(true);
} else {
+ if (mqeValuesR.size() != 1) {
+ throw new IllegalExpressionException("Single Labeled to
Single Labeled, right labeled result has more than one value.");
+ }
MQEValue valueR = mqeValuesR.get(0);
if (valueL.isEmptyValue() || valueR.isEmptyValue()) {
valueL.setEmptyValue(true);
@@ -141,7 +157,11 @@ public interface LROp {
//series or list or labeled single value with scalar
private static ExpressionResult many2OneBinaryOp(ExpressionResult
manyResult,
ExpressionResult
singleResult,
- int opType, LROp
calculate) {
+ int opType, LROp
calculate) throws IllegalExpressionException {
+ if (singleResult.getResults().isEmpty() ||
+ singleResult.getResults().get(0).getValues().size() != 1) {
+ throw new IllegalExpressionException("Many to One, single result
is empty or has more than one value.");
+ }
manyResult.getResults().forEach(mqeValues -> {
mqeValues.getValues().forEach(mqeValue -> {
if (!mqeValue.isEmptyValue()) {
@@ -161,7 +181,11 @@ public interface LROp {
//scalar with series or list or labeled single value
private static ExpressionResult one2ManyBinaryOp(ExpressionResult
singleResult,
ExpressionResult
manyResult,
- int opType, LROp
calculate) {
+ int opType, LROp
calculate) throws IllegalExpressionException {
+ if (singleResult.getResults().isEmpty() ||
+ singleResult.getResults().get(0).getValues().size() != 1) {
+ throw new IllegalExpressionException("One to Many, single result
is empty or has more than one value.");
+ }
manyResult.getResults().forEach(mqeValues -> {
mqeValues.getValues().forEach(mqeValue -> {
if (!mqeValue.isEmptyValue()) {
@@ -182,9 +206,15 @@ public interface LROp {
private static ExpressionResult seriesNoLabeled(ExpressionResult
seriesLeft,
ExpressionResult
seriesRight,
- int opType, LROp
calculate) {
+ int opType, LROp
calculate) throws IllegalExpressionException {
+ if (seriesLeft.getResults().isEmpty() ||
seriesRight.getResults().isEmpty()) {
+ throw new IllegalExpressionException("Series No Labeled, left or
right result is empty.");
+ }
MQEValues mqeValuesL = seriesLeft.getResults().get(0);
MQEValues mqeValuesR = seriesRight.getResults().get(0);
+ if (mqeValuesL.getValues().size() != mqeValuesR.getValues().size()) {
+ throw new IllegalExpressionException("Series No Labeled, left and
right series value size not equal.");
+ }
for (int i = 0; i < mqeValuesL.getValues().size(); i++) {
//clean metric info
MQEValue valueL = mqeValuesL.getValues().get(i);
@@ -203,9 +233,15 @@ public interface LROp {
private static ExpressionResult
seriesLabeledWithNoLabeled(ExpressionResult seriesLeft,
ExpressionResult seriesRight,
- int opType,
LROp calculate) {
+ int opType,
LROp calculate) throws IllegalExpressionException {
+ if (seriesRight.getResults().isEmpty()) {
+ throw new IllegalExpressionException("Series Labeled with No
Labeled, no labeled result is empty.");
+ }
MQEValues mqeValuesR = seriesRight.getResults().get(0);
- seriesLeft.getResults().forEach(mqeValuesL -> {
+ for (MQEValues mqeValuesL : seriesLeft.getResults()) {
+ if (mqeValuesL.getValues().size() !=
mqeValuesR.getValues().size()) {
+ throw new IllegalExpressionException("Series Labeled with No
Labeled, left and right series value size not equal.");
+ }
for (int i = 0; i < mqeValuesL.getValues().size(); i++) {
//reserve left metric info
MQEValue valueL = mqeValuesL.getValues().get(i);
@@ -217,16 +253,22 @@ public interface LROp {
double newValue = calculate.apply(valueL.getDoubleValue(),
valueR.getDoubleValue(), opType);
mqeValuesL.getValues().get(i).setDoubleValue(newValue);
}
- });
+ }
return seriesLeft;
}
private static ExpressionResult
seriesNoLabeledWithLabeled(ExpressionResult seriesLeft,
ExpressionResult seriesRight,
- int opType,
LROp calculate) {
+ int opType,
LROp calculate) throws IllegalExpressionException {
+ if (seriesLeft.getResults().isEmpty()) {
+ throw new IllegalExpressionException("Series No Labeled with
Labeled, no labeled result is empty.");
+ }
MQEValues mqeValuesL = seriesLeft.getResults().get(0);
- seriesRight.getResults().forEach(mqeValuesR -> {
+ for (MQEValues mqeValuesR : seriesRight.getResults()) {
+ if (mqeValuesL.getValues().size() !=
mqeValuesR.getValues().size()) {
+ throw new IllegalExpressionException("Series No Labeled with
Labeled, left and right series value size not equal.");
+ }
for (int i = 0; i < mqeValuesL.getValues().size(); i++) {
//reserve left metric info
MQEValue valueL = mqeValuesL.getValues().get(i);
@@ -238,7 +280,7 @@ public interface LROp {
double newValue = calculate.apply(valueL.getDoubleValue(),
valueR.getDoubleValue(), opType);
mqeValuesR.getValues().get(i).setDoubleValue(newValue);
}
- });
+ }
return seriesRight;
}
@@ -258,6 +300,9 @@ public interface LROp {
if (mqeValuesR == null) {
valueL.setEmptyValue(true);
} else {
+ if (mqeValuesR.size() != mqeValuesL.getValues().size()) {
+ throw new IllegalExpressionException("Series Labeled
with Labeled, left and right series value size not equal.");
+ }
MQEValue valueR = mqeValuesR.get(i);
if (valueL.isEmptyValue() || valueR.isEmptyValue()) {
valueL.setEmptyValue(true);
diff --git
a/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/BinaryOpTest.java
b/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/BinaryOpTest.java
index 74b458c106..ea5e3d56f3 100644
---
a/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/BinaryOpTest.java
+++
b/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/BinaryOpTest.java
@@ -26,6 +26,7 @@ import org.apache.skywalking.mqe.rt.type.ExpressionResultType;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class BinaryOpTest {
private final MockData mockData = new MockData();
@@ -33,6 +34,10 @@ public class BinaryOpTest {
//DIV/MUL/MOD/SUB... are the same logic and tested in here, the others
only test ADD is enough.
@Test
public void seriesNoLabeledTest() throws Exception {
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false),
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false), MQEParser.ADD
+ ));
ExpressionResult add = BinaryOp.doBinaryOp(
mockData.newSeriesNoLabeledResult(),
mockData.newSeriesNoLabeledResult(), MQEParser.ADD);
assertEquals(ExpressionResultType.TIME_SERIES_VALUES, add.getType());
@@ -76,6 +81,10 @@ public class BinaryOpTest {
@Test
public void seriesLabeledTest() throws Exception {
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newSeriesLabeledResult(),
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false), MQEParser.ADD
+ ));
//seriesLabeled + seriesNoLabeled
ExpressionResult add =
BinaryOp.doBinaryOp(mockData.newSeriesLabeledResult(),
mockData.newSeriesNoLabeledResult(), MQEParser.ADD);
assertEquals(ExpressionResultType.TIME_SERIES_VALUES, add.getType());
@@ -133,6 +142,18 @@ public class BinaryOpTest {
@Test
public void many2OneTest() throws Exception {
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false),
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
MQEParser.ADD
+ ));
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
true),
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
MQEParser.ADD
+ ));
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false),
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, true),
MQEParser.ADD
+ ));
//sort_list + single
ExpressionResult add = BinaryOp.doBinaryOp(
mockData.newListResult(), mockData.newSingleResult(1000),
MQEParser.ADD);
@@ -171,6 +192,14 @@ public class BinaryOpTest {
@Test
public void one2ManyTest() throws Exception {
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
false), MQEParser.ADD
+ ));
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
+ mockData.newEmptyResult(ExpressionResultType.TIME_SERIES_VALUES,
true), MQEParser.ADD
+ ));
// single - sort_list
ExpressionResult sub = BinaryOp.doBinaryOp(
mockData.newSingleResult(1000), mockData.newListResult(),
MQEParser.SUB);
@@ -209,6 +238,15 @@ public class BinaryOpTest {
@Test
public void single2SingleTest() throws IllegalExpressionException {
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
MQEParser.ADD
+ ));
+ assertThrows(IllegalExpressionException.class, () ->
BinaryOp.doBinaryOp(
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, false),
+ mockData.newEmptyResult(ExpressionResultType.SINGLE_VALUE, true),
MQEParser.ADD
+ ));
+
//noLabeled + noLabeled
ExpressionResult add = BinaryOp.doBinaryOp(
mockData.newSingleResult(100), mockData.newSingleResult(200),
MQEParser.ADD);
diff --git
a/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/MockData.java
b/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/MockData.java
index 2a53df8f0d..ff25c1e997 100644
--- a/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/MockData.java
+++ b/oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/MockData.java
@@ -185,6 +185,13 @@ public class MockData {
return result;
}
+ public ExpressionResult newEmptyResult(ExpressionResultType type, boolean
labeled) {
+ ExpressionResult result = new ExpressionResult();
+ result.setType(type);
+ result.setLabeledResult(labeled);
+ return result;
+ }
+
public MQEValue newMQEValue(String id, double value) {
MQEValue mqeValue = new MQEValue();
mqeValue.setId(id);
diff --git a/test/e2e-v2/script/env b/test/e2e-v2/script/env
index 3d61b338bd..b495437dde 100644
--- a/test/e2e-v2/script/env
+++ b/test/e2e-v2/script/env
@@ -23,7 +23,7 @@
SW_AGENT_CLIENT_JS_COMMIT=af0565a67d382b683c1dbd94c379b7080db61449
SW_AGENT_CLIENT_JS_TEST_COMMIT=4f1eb1dcdbde3ec4a38534bf01dded4ab5d2f016
SW_KUBERNETES_COMMIT_SHA=1335f15bf821a40a7cd71448fa805f0be265afcc
SW_ROVER_COMMIT=6bbd39aa701984482330d9dfb4dbaaff0527d55c
-SW_BANYANDB_COMMIT=285db188e633c6b95b8c5c354e043db79658c147
+SW_BANYANDB_COMMIT=6f938ccf812e2183b4bd891fb90b2124aa65c170
SW_AGENT_PHP_COMMIT=3192c553002707d344bd6774cfab5bc61f67a1d3
SW_CTL_COMMIT=d5f3597733aa5217373986d776a3ee5ee8b3c468
