Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MQE: check the metrics value before do binary operation to improve robustness #12483

Merged
merged 2 commits into from
Jul 26, 2024
Merged

MQE: check the metrics value before do binary operation to improve robustness #12483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd49368
MQE: check the metrics value before do binary operation to improve ro…
wankai123 Jul 26, 2024
88887a2
update BanyanDB e2e version
wankai123 Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/en/changes/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/BinaryOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static ExpressionResult doBinaryOp(ExpressionResult left,
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());
}
}

Expand Down
2 changes: 1 addition & 1 deletion oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/CompareOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static ExpressionResult doCompareOP(ExpressionResult left,
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());
}
}

Expand Down
65 changes: 55 additions & 10 deletions oap-server/mqe-rt/src/main/java/org/apache/skywalking/mqe/rt/operation/LROp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,17 @@ private static ExpressionResult single2SingleBinaryOp(ExpressionResult singleLef

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();
Expand Down Expand Up @@ -119,12 +129,18 @@ private static ExpressionResult single2SingleLabeled(ExpressionResult singleLeft
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);
Expand All @@ -141,7 +157,11 @@ private static ExpressionResult single2SingleLabeled(ExpressionResult singleLeft
//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()) {
Expand All @@ -161,7 +181,11 @@ private static ExpressionResult many2OneBinaryOp(ExpressionResult manyResult,
//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()) {
Expand All @@ -182,9 +206,15 @@ private static ExpressionResult one2ManyBinaryOp(ExpressionResult singleResult,

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);
Expand All @@ -203,9 +233,15 @@ private static ExpressionResult seriesNoLabeled(ExpressionResult seriesLeft,

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);
Expand All @@ -217,16 +253,22 @@ private static ExpressionResult seriesLabeledWithNoLabeled(ExpressionResult seri
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);
Expand All @@ -238,7 +280,7 @@ private static ExpressionResult seriesNoLabeledWithLabeled(ExpressionResult seri
double newValue = calculate.apply(valueL.getDoubleValue(), valueR.getDoubleValue(), opType);
mqeValuesR.getValues().get(i).setDoubleValue(newValue);
}
});
}

return seriesRight;
}
Expand All @@ -258,6 +300,9 @@ private static ExpressionResult seriesLabeledWithLabeled(ExpressionResult series
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);
Expand Down
38 changes: 38 additions & 0 deletions oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/BinaryOpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@
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();

//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());
Expand Down Expand Up @@ -76,6 +81,10 @@ public void seriesNoLabeledTest() throws Exception {

@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());
Expand Down Expand Up @@ -133,6 +142,18 @@ public void seriesLabeledTest() throws Exception {

@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);
Expand Down Expand Up @@ -171,6 +192,14 @@ public void many2OneTest() throws Exception {

@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);
Expand Down Expand Up @@ -209,6 +238,15 @@ public void one2ManyTest() throws Exception {

@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);
Expand Down
7 changes: 7 additions & 0 deletions oap-server/mqe-rt/src/test/java/org/apache/skywalking/mqe/rt/MockData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ public ExpressionResult newSingleLabeledResult(double id1001, double id1002) {
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);
Expand Down
2 changes: 1 addition & 1 deletion test/e2e-v2/script/env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading