Skip to content

Commit 093e404

Browse files
weixiang1862weixiang1862
authored
Fix metrics comparison in promql with bool modifier (#13475)
1 parent aaf7a6a commit 093e404

File tree

4 files changed

+64
-16
lines changed

4 files changed

+64
-16
lines changed

‎docs/en/changes/changes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
* Remove method `appendMutant` from StorageID.
9494
* Fix otlp log handler reponse error and otlp span convert error.
9595
* Fix service_relation source layer in mq entry span analyse.
96+
* Fix metrics comparison in promql with bool modifier.
9697

9798
#### UI
9899

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

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -224,30 +224,49 @@ private static int boolToInt(boolean v) {
224224
return v ? 1 : 0;
225225
}
226226

227-
static MetricsRangeResult matrixScalarCompareOp(MetricsRangeResult matrix, ScalarResult scalar, int opType) {
227+
static MetricsRangeResult matrixScalarCompareOp(MetricsRangeResult matrix, ScalarResult scalar,
228+
int opType, boolean boolModifier) {
228229
MetricsRangeResult result = new MetricsRangeResult();
229230
result.setResultType(ParseResultType.METRICS_RANGE);
230231
result.setRangeExpression(matrix.isRangeExpression());
231232
matrix.getMetricDataList().forEach(metricData -> {
232233
MetricRangeData newData = new MetricRangeData();
233234
result.getMetricDataList().add(newData);
234235
newData.setMetric(metricData.getMetric());
235-
List<TimeValuePair> newValues = metricData.getValues()
236-
.stream()
237-
.filter(timeValuePair -> scalarCompareOp(
238-
Double.parseDouble(timeValuePair.getValue()),
239-
scalar.getValue(), opType
240-
) == 1)
241-
.collect(
242-
Collectors.toList());
236+
237+
List<TimeValuePair> newValues;
238+
if (boolModifier) {
239+
newValues = metricData.getValues()
240+
.stream()
241+
.map(timeValuePair -> {
242+
return new TimeValuePair(
243+
timeValuePair.getTime(),
244+
Integer.toString(scalarCompareOp(
245+
Double.parseDouble(timeValuePair.getValue()),
246+
scalar.getValue(), opType
247+
))
248+
);
249+
})
250+
.collect(Collectors.toList());
251+
} else {
252+
newValues = metricData.getValues()
253+
.stream()
254+
.filter(timeValuePair ->
255+
scalarCompareOp(
256+
Double.parseDouble(timeValuePair.getValue()),
257+
scalar.getValue(), opType
258+
) == 1
259+
)
260+
.collect(Collectors.toList());
261+
}
243262
newData.setValues(newValues);
244263
});
245264
return result;
246265
}
247266

248267
static MetricsRangeResult matrixCompareOp(MetricsRangeResult matrixLeft,
249268
MetricsRangeResult matrixRight,
250-
int opType) throws IllegalExpressionException {
269+
int opType, boolean boolModifier) throws IllegalExpressionException {
251270
MetricsRangeResult result = new MetricsRangeResult();
252271
result.setResultType(ParseResultType.METRICS_RANGE);
253272
result.setRangeExpression(matrixLeft.isRangeExpression());
@@ -264,14 +283,26 @@ static MetricsRangeResult matrixCompareOp(MetricsRangeResult matrixLeft,
264283
}
265284
MetricRangeData newData = new MetricRangeData();
266285
result.getMetricDataList().add(newData);
267-
newData.setMetric(dataLeft.getMetric());
286+
if (boolModifier) {
287+
// metric name should be removed between two matrix bool compare
288+
MetricInfo metricInfo = new MetricInfo(null);
289+
metricInfo.setLabels(dataLeft.getMetric().getLabels());
290+
newData.setMetric(metricInfo);
291+
} else {
292+
newData.setMetric(dataLeft.getMetric());
293+
}
268294
List<TimeValuePair> newValues = new ArrayList<>();
269295
newData.setValues(newValues);
270296
for (int j = 0; j < dataLeft.getValues().size(); j++) {
271297
double lv = Double.parseDouble(dataLeft.getValues().get(j).getValue());
272298
double rv = Double.parseDouble(dataRight.getValues().get(j).getValue());
273-
if (scalarCompareOp(lv, rv, opType) == 1) {
274-
newValues.add(dataLeft.getValues().get(j));
299+
if (boolModifier) {
300+
long time = dataLeft.getValues().get(j).getTime();
301+
newValues.add(new TimeValuePair(time, Integer.toString(scalarCompareOp(lv, rv, opType))));
302+
} else {
303+
if (scalarCompareOp(lv, rv, opType) == 1) {
304+
newValues.add(dataLeft.getValues().get(j));
305+
}
275306
}
276307
}
277308
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,12 @@ private ParseResult compareOp(ParseResult left, ParseResult right, int opType, b
203203
return scalarResult;
204204
}
205205
} else if (left.getResultType() == ParseResultType.METRICS_RANGE && right.getResultType() == ParseResultType.SCALAR) {
206-
return matrixScalarCompareOp((MetricsRangeResult) left, (ScalarResult) right, opType);
206+
return matrixScalarCompareOp((MetricsRangeResult) left, (ScalarResult) right, opType, boolModifier);
207207
} else if (left.getResultType() == ParseResultType.SCALAR && right.getResultType() == ParseResultType.METRICS_RANGE) {
208-
return matrixScalarCompareOp((MetricsRangeResult) right, (ScalarResult) left, opType);
208+
return matrixScalarCompareOp((MetricsRangeResult) right, (ScalarResult) left, opType, boolModifier);
209209
} else if (left.getResultType() == ParseResultType.METRICS_RANGE && right.getResultType() == ParseResultType.METRICS_RANGE) {
210210
try {
211-
return matrixCompareOp((MetricsRangeResult) left, (MetricsRangeResult) right, opType);
211+
return matrixCompareOp((MetricsRangeResult) left, (MetricsRangeResult) right, opType, boolModifier);
212212
} catch (IllegalExpressionException e) {
213213
MetricsRangeResult result = new MetricsRangeResult();
214214
result.setErrorType(ErrorType.BAD_DATA);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,22 @@ public static Collection<Object[]> data() {
133133
"service_cpm{service='serviceA', layer='GENERAL'} > 1",
134134
ParseResultType.METRICS_RANGE,
135135
List.of(new TimeValuePair(TIME_2023022012, "2"))
136+
},
137+
{
138+
"MetricsScalarCompareOpWithBoolModifier",
139+
PromQLApiHandler.QueryType.RANGE,
140+
"service_cpm{service='serviceA', layer='GENERAL'} > bool 1",
141+
ParseResultType.METRICS_RANGE,
142+
List.of(new TimeValuePair(TIME_2023022010, "0"), new TimeValuePair(TIME_2023022011, "0"),
143+
new TimeValuePair(TIME_2023022012, "1"))
144+
},
145+
{
146+
"MetricsMetricsCompareOpWithBoolModifier",
147+
PromQLApiHandler.QueryType.RANGE,
148+
"service_cpm{service='serviceA', layer='GENERAL'} > bool service_cpm{service='serviceA', layer='GENERAL'}",
149+
ParseResultType.METRICS_RANGE,
150+
List.of(new TimeValuePair(TIME_2023022010, "0"), new TimeValuePair(TIME_2023022011, "0"),
151+
new TimeValuePair(TIME_2023022012, "0"))
136152
}
137153
});
138154
}

0 commit comments

Comments
 (0)