Skip to content

Commit 0a1c214

Browse files
authored
Fix range matrix and scalar binary operation in PromQL (#13616)
1 parent 1b579d2 commit 0a1c214

File tree

4 files changed

+24
-9
lines changed

4 files changed

+24
-9
lines changed

docs/en/changes/changes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* Fix BrowserWebVitalsPerfData `clsTime` to `cls` and make it double type.
1111
* Init `log-mal-rules` at module provider start stage to avoid re-init for every LAL.
1212
* Fail fast if SampleFamily is empty after MAL filter expression.
13+
* Fix range matrix and scalar binary operation in PromQL.
1314

1415
#### UI
1516
* Fix the missing icon in new native trace view.

oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.LinkedHashMap;
2424
import java.util.List;
2525
import java.util.Map;
26+
import java.util.function.Function;
2627
import java.util.stream.Collectors;
2728
import org.apache.skywalking.mqe.rt.operation.aggregatelabels.AggregateLabelsFunc;
2829
import org.apache.skywalking.mqe.rt.operation.aggregatelabels.AggregateLabelsFuncFactory;
@@ -53,7 +54,7 @@
5354

5455
public class PromOpUtils {
5556

56-
static MetricsRangeResult matrixScalarBinaryOp(MetricsRangeResult matrix, ScalarResult scalar, int opType) {
57+
static MetricsRangeResult matrixScalarBinaryOp(MetricsRangeResult matrix, Function<Double, Double> rangeValueOp) {
5758
MetricsRangeResult result = new MetricsRangeResult();
5859
result.setResultType(ParseResultType.METRICS_RANGE);
5960
result.setRangeExpression(matrix.isRangeExpression());
@@ -64,8 +65,7 @@ static MetricsRangeResult matrixScalarBinaryOp(MetricsRangeResult matrix, Scalar
6465
List<TimeValuePair> newValues = metricData.getValues().stream().map(value -> {
6566
double v = Double.parseDouble(value.getValue());
6667
return new TimeValuePair(
67-
value.getTime(),
68-
formatDoubleValue(scalarBinaryOp(v, scalar.getValue(), opType))
68+
value.getTime(), formatDoubleValue(rangeValueOp.apply(v))
6969
);
7070

7171
}).collect(Collectors.toList());

oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,9 +460,15 @@ private ParseResult binaryOp(ParseResult left, ParseResult right, int opType) {
460460
scalarResult.setResultType(ParseResultType.SCALAR);
461461
return scalarResult;
462462
} else if (left.getResultType() == ParseResultType.METRICS_RANGE && right.getResultType() == ParseResultType.SCALAR) {
463-
return matrixScalarBinaryOp((MetricsRangeResult) left, (ScalarResult) right, opType);
463+
return matrixScalarBinaryOp(
464+
(MetricsRangeResult) left,
465+
rangeValue -> scalarBinaryOp(rangeValue, ((ScalarResult) right).getValue(), opType)
466+
);
464467
} else if (left.getResultType() == ParseResultType.SCALAR && right.getResultType() == ParseResultType.METRICS_RANGE) {
465-
return matrixScalarBinaryOp((MetricsRangeResult) right, (ScalarResult) left, opType);
468+
return matrixScalarBinaryOp(
469+
(MetricsRangeResult) right,
470+
rangeValue -> scalarBinaryOp(((ScalarResult) left).getValue(), rangeValue, opType)
471+
);
466472
} else if (left.getResultType() == ParseResultType.METRICS_RANGE && right.getResultType() == ParseResultType.METRICS_RANGE) {
467473
try {
468474
return matrixBinaryOp((MetricsRangeResult) left, (MetricsRangeResult) right, opType);

oap-server/server-query-plugin/promql-plugin/src/test/java/org/apache/skywalking/promql/rt/parser/PromQLExprQueryVisitorTest.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,14 @@ public static Collection<Object[]> data() {
111111
List.of(new TimeValuePair(TIME_2023022010, "100"), new TimeValuePair(TIME_2023022011, "101"),
112112
new TimeValuePair(TIME_2023022012, "102"))
113113
},
114+
{
115+
"ScalarMetricsBinaryOp",
116+
PromQLApiHandler.QueryType.RANGE,
117+
"100 - service_cpm{service='serviceA', layer='GENERAL'}",
118+
ParseResultType.METRICS_RANGE,
119+
List.of(new TimeValuePair(TIME_2023022010, "100"), new TimeValuePair(TIME_2023022011, "99"),
120+
new TimeValuePair(TIME_2023022012, "98"))
121+
},
114122
{
115123
"MetricsBinaryOp",
116124
PromQLApiHandler.QueryType.RANGE,
@@ -359,7 +367,7 @@ public void test(String name,
359367
break;
360368
case METRICS_RANGE:
361369
MetricsRangeResult metricsRangeResult = (MetricsRangeResult) parseResult;
362-
Assertions.assertEquals(metricsRangeResult.getMetricDataList().get(0).getValues(), wantResultValues);
370+
Assertions.assertEquals(wantResultValues, metricsRangeResult.getMetricDataList().get(0).getValues());
363371
break;
364372
default:
365373
Assertions.fail();
@@ -383,10 +391,10 @@ public void testAggregate(String name,
383391
Assertions.assertEquals(ParseResultType.METRICS_RANGE, parseResult.getResultType());
384392

385393
MetricsRangeResult result = (MetricsRangeResult) parseResult;
386-
Assertions.assertEquals(result.getMetricDataList().size(), wantResultValues.size());
394+
Assertions.assertEquals(wantResultValues.size(), result.getMetricDataList().size());
387395
for (int i = 0; i < result.getMetricDataList().size(); i++) {
388-
Assertions.assertEquals(result.getMetricDataList().get(i).getValues(), wantResultValues.get(i));
389-
Assertions.assertEquals(result.getMetricDataList().get(i).getMetric().getLabels(), wantResultLabels.get(i));
396+
Assertions.assertEquals(wantResultValues.get(i), result.getMetricDataList().get(i).getValues());
397+
Assertions.assertEquals(wantResultLabels.get(i), result.getMetricDataList().get(i).getMetric().getLabels());
390398
}
391399
}
392400
}

0 commit comments

Comments
 (0)