From 74eeb5beeeedd7f91a820979f31b367b46927c8b Mon Sep 17 00:00:00 2001 From: Wan Kai Date: Thu, 26 Sep 2024 15:59:03 +0800 Subject: [PATCH] PromQL service: fix operators result missing `rangeExpression` flag. (#12649) --- docs/en/api/promql-service.md | 7 +++++ docs/en/changes/changes.md | 1 + .../expr/rt/AlarmMQEVerifyVisitor.java | 4 +-- .../provider/expr/rt/AlarmMQEVisitor.java | 2 +- .../core/alarm/provider/AlarmRuleTest.java | 4 +-- .../oap/query/promql/rt/PromOpUtils.java | 5 +++ .../promql/rt/PromQLExprQueryVisitor.java | 2 +- .../oap/query/graphql/mqe/rt/MQEVisitor.java | 4 +-- .../expected/service-metric-matrix-add.yml | 31 +++++++++++++++++++ .../service-metric-matrix-compare.yml | 31 +++++++++++++++++++ .../expected/service-metric-vector-add.yml | 29 +++++++++++++++++ test/e2e-v2/cases/promql/promql-cases.yaml | 10 ++++-- 12 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 test/e2e-v2/cases/promql/expected/service-metric-matrix-add.yml create mode 100644 test/e2e-v2/cases/promql/expected/service-metric-matrix-compare.yml create mode 100644 test/e2e-v2/cases/promql/expected/service-metric-vector-add.yml diff --git a/docs/en/api/promql-service.md b/docs/en/api/promql-service.md index 9b7b3f4874..4088ff52d7 100644 --- a/docs/en/api/promql-service.md +++ b/docs/en/api/promql-service.md @@ -180,6 +180,13 @@ Result: } ``` +We can also use [Range Vector Selectors](#range-vector-selectors) in the instant query. +``` +/api/v1/query?query=service_cpm{service='agent::songs', layer='GENERAL'}[5m] +``` + +the result is the same as the [Range queries](#range-queries). + ##### Range queries [Prometheus Docs Reference](https://prometheus.io/docs/prometheus/latest/querying/api/#range-queries) diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md index ec0abdc5f5..e94c6cf8da 100644 --- a/docs/en/changes/changes.md +++ b/docs/en/changes/changes.md @@ -78,6 +78,7 @@ * BanyanDB: support TLS connection and configuration. * PromQL service: query API support RFC3399 time format. * Improve the performance of OTEL metrics handler. +* PromQL service: fix operators result missing `rangeExpression` flag. #### UI diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java index 6b73c2cd3d..df4989949a 100644 --- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java +++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java @@ -54,7 +54,7 @@ public class AlarmMQEVerifyVisitor extends MQEVisitorBase { metricName); if (valueColumn.isEmpty()) { result.setType(ExpressionResultType.UNKNOWN); - result.setError("Metric: [" + metricName + "] dose not exist."); + result.setError("Metric: [" + metricName + "] does not exist."); return result; } @@ -80,7 +80,7 @@ public class AlarmMQEVerifyVisitor extends MQEVisitorBase { return result; } else { result.setType(ExpressionResultType.UNKNOWN); - result.setError("Metric dose not supported in alarm, metric: [" + metricName + "] is not a common or labeled metric."); + result.setError("Metric does not supported in alarm, metric: [" + metricName + "] is not a common or labeled metric."); return result; } } diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java index 0d27a6743f..ed8c82b3b6 100644 --- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java +++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java @@ -80,7 +80,7 @@ public class AlarmMQEVisitor extends MQEVisitorBase { metricName); if (valueColumn.isEmpty()) { result.setType(ExpressionResultType.UNKNOWN); - result.setError("Metric: [" + metricName + "] dose not exist."); + result.setError("Metric: [" + metricName + "] does not exist."); return result; } Column.ValueDataType dataType = valueColumn.get().getDataType(); diff --git a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java index 1ab3446d76..7bbda290a8 100644 --- a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java +++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java @@ -77,7 +77,7 @@ public class AlarmRuleTest { //not exist metric Assertions.assertEquals( - "Expression: sum(service_percent111 < 85) >= 3 error: Metric: [service_percent111] dose not exist.", + "Expression: sum(service_percent111 < 85) >= 3 error: Metric: [service_percent111] does not exist.", Assertions.assertThrows(IllegalExpressionException.class, () -> { rule.setExpression("sum(service_percent111 < 85) >= 3"); }).getMessage() @@ -101,7 +101,7 @@ public class AlarmRuleTest { //not a common or labeled metric Assertions.assertEquals( - "Expression: sum(record < 85) > 1 error: Metric dose not supported in alarm, metric: [record] is not a common or labeled metric.", + "Expression: sum(record < 85) > 1 error: Metric does not supported in alarm, metric: [record] is not a common or labeled metric.", Assertions.assertThrows(IllegalExpressionException.class, () -> { rule.setExpression("sum(record < 85) > 1"); }).getMessage() diff --git a/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java b/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java index c29ba62e08..9bfa818a6b 100644 --- a/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java +++ b/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java @@ -56,6 +56,7 @@ public class PromOpUtils { static MetricsRangeResult matrixScalarBinaryOp(MetricsRangeResult matrix, ScalarResult scalar, int opType) { MetricsRangeResult result = new MetricsRangeResult(); result.setResultType(ParseResultType.METRICS_RANGE); + result.setRangeExpression(matrix.isRangeExpression()); matrix.getMetricDataList().forEach(metricData -> { MetricRangeData newData = new MetricRangeData(); result.getMetricDataList().add(newData); @@ -78,6 +79,7 @@ public class PromOpUtils { int opType) throws IllegalExpressionException { MetricsRangeResult result = new MetricsRangeResult(); result.setResultType(ParseResultType.METRICS_RANGE); + result.setRangeExpression(matrixLeft.isRangeExpression()); for (int i = 0; i < matrixLeft.getMetricDataList().size(); i++) { MetricRangeData dataLeft = matrixLeft.getMetricDataList().get(i); MetricRangeData dataRight = matrixRight.getMetricDataList().get(i); @@ -113,6 +115,7 @@ public class PromOpUtils { MetricsRangeResult rangeResult = new MetricsRangeResult(); rangeResult.setResultType(ParseResultType.METRICS_RANGE); + rangeResult.setRangeExpression(result.isRangeExpression()); AggregateLabelsFuncFactory factory = getAggregateFuncFactory(funcType); groupedResult.forEach((labels, dataList) -> { if (dataList.isEmpty()) { @@ -224,6 +227,7 @@ public class PromOpUtils { static MetricsRangeResult matrixScalarCompareOp(MetricsRangeResult matrix, ScalarResult scalar, int opType) { MetricsRangeResult result = new MetricsRangeResult(); result.setResultType(ParseResultType.METRICS_RANGE); + result.setRangeExpression(matrix.isRangeExpression()); matrix.getMetricDataList().forEach(metricData -> { MetricRangeData newData = new MetricRangeData(); result.getMetricDataList().add(newData); @@ -246,6 +250,7 @@ public class PromOpUtils { int opType) throws IllegalExpressionException { MetricsRangeResult result = new MetricsRangeResult(); result.setResultType(ParseResultType.METRICS_RANGE); + result.setRangeExpression(matrixLeft.isRangeExpression()); for (int i = 0; i < matrixLeft.getMetricDataList().size(); i++) { MetricRangeData dataLeft = matrixLeft.getMetricDataList().get(i); MetricRangeData dataRight = matrixRight.getMetricDataList().get(i); diff --git a/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java b/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java index e96dd72b0b..f7795b875b 100644 --- a/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java +++ b/oap-server/server-query-plugin/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java @@ -237,7 +237,7 @@ public class PromQLExprQueryVisitor extends PromQLParserBaseVisitor Optional valueColumn = getValueColumn(metricName); if (valueColumn.isEmpty()) { result.setErrorType(ErrorType.BAD_DATA); - result.setErrorInfo("Metric: [" + metricName + "] dose not exist."); + result.setErrorInfo("Metric: [" + metricName + "] does not exist."); return result; } if (ctx.labelList() == null) { diff --git a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java index 834b9b5191..ae7933e2f0 100644 --- a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java +++ b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java @@ -115,7 +115,7 @@ public class MQEVisitor extends MQEVisitorBase { metricName); if (valueColumn.isEmpty()) { result.setType(ExpressionResultType.UNKNOWN); - result.setError("Metric: [" + metricName + "] dose not exist."); + result.setError("Metric: [" + metricName + "] does not exist."); return result; } @@ -141,7 +141,7 @@ public class MQEVisitor extends MQEVisitorBase { } else if (Column.ValueDataType.LABELED_VALUE == dataType) { if (ctx.parent instanceof MQEParser.TopNOPContext) { throw new IllegalExpressionException( - "Metric: [" + metricName + "] is labeled value, dose not support top_n query."); + "Metric: [" + metricName + "] is labeled value, does not support top_n query."); } List queryLabels = super.buildLabels(ctx.labelList()); if (ctx.parent instanceof MQEParser.TrendOPContext) { diff --git a/test/e2e-v2/cases/promql/expected/service-metric-matrix-add.yml b/test/e2e-v2/cases/promql/expected/service-metric-matrix-add.yml new file mode 100644 index 0000000000..2bfba16ae8 --- /dev/null +++ b/test/e2e-v2/cases/promql/expected/service-metric-matrix-add.yml @@ -0,0 +1,31 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +status: success +data: + resultType: matrix + result: + {{- contains .data.result }} + - metric: + __name__: service_sla + layer: GENERAL + scope: Service + service: e2e-service-consumer + values: + {{- contains .values }} + - - "{{ index . 0 }}" + - '20000' + {{- end}} + {{- end}} diff --git a/test/e2e-v2/cases/promql/expected/service-metric-matrix-compare.yml b/test/e2e-v2/cases/promql/expected/service-metric-matrix-compare.yml new file mode 100644 index 0000000000..69bb99add0 --- /dev/null +++ b/test/e2e-v2/cases/promql/expected/service-metric-matrix-compare.yml @@ -0,0 +1,31 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +status: success +data: + resultType: matrix + result: + {{- contains .data.result }} + - metric: + __name__: service_sla + layer: GENERAL + scope: Service + service: e2e-service-consumer + values: + {{- contains .values }} + - - "{{ index . 0 }}" + - '10000' + {{- end}} + {{- end}} diff --git a/test/e2e-v2/cases/promql/expected/service-metric-vector-add.yml b/test/e2e-v2/cases/promql/expected/service-metric-vector-add.yml new file mode 100644 index 0000000000..ac8bc773d2 --- /dev/null +++ b/test/e2e-v2/cases/promql/expected/service-metric-vector-add.yml @@ -0,0 +1,29 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +status: success +data: + resultType: vector + result: + {{- contains .data.result }} + - metric: + __name__: service_sla + layer: GENERAL + scope: Service + service: e2e-service-consumer + value: + - "{{ index .value 0 }}" + - '20000' + {{- end}} diff --git a/test/e2e-v2/cases/promql/promql-cases.yaml b/test/e2e-v2/cases/promql/promql-cases.yaml index 3548d54e50..e53eb38a36 100644 --- a/test/e2e-v2/cases/promql/promql-cases.yaml +++ b/test/e2e-v2/cases/promql/promql-cases.yaml @@ -75,8 +75,12 @@ cases: - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_cpm{layer="GENERAL",top_n="10", order="DES"}[30m]' expected: expected/service-metric-sort-matrix.yml ## multiple metrics operation query - - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_sla{service="e2e-service-consumer", layer="GENERAL"} + service_cpm{service="e2e-service-consumer", layer="GENERAL"}' - expected: expected/service-metric-vector.yml + - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_sla{service="e2e-service-consumer", layer="GENERAL"} %2B service_sla{service="e2e-service-consumer", layer="GENERAL"}' + expected: expected/service-metric-vector-add.yml + - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_sla{service="e2e-service-consumer", layer="GENERAL"}[30m] %2B service_sla{service="e2e-service-consumer", layer="GENERAL"}[30m]' + expected: expected/service-metric-matrix-add.yml + - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_sla{service="e2e-service-consumer", layer="GENERAL"}[30m] == service_sla{service="e2e-service-consumer", layer="GENERAL"}[30m]' + expected: expected/service-metric-matrix-compare.yml - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=service_percentile{service="e2e-service-consumer", layer="GENERAL", p="99"} - service_percentile{service="e2e-service-consumer", layer="GENERAL", p="99"}' expected: expected/service-metric-labeled-compare-matrix.yml ## query_range @@ -89,6 +93,8 @@ cases: expected: expected/service-metric-labeled-matrix.yml - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query_range -d 'query=sum by (p) (service_percentile{service="e2e-service-consumer", layer="GENERAL", p="50,75,90"})&start='$(($(date +%s)-1800))'&end='$(date +%s) expected: expected/service-metric-labeled-matrix-aggregate-by-p.yml + - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query -d 'query=sum by (p) (service_percentile{service="e2e-service-consumer", layer="GENERAL", p="50,75,90"}[30m])' + expected: expected/service-metric-labeled-matrix-aggregate-by-p.yml - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query_range -d 'query=sum without (p) (service_percentile{service="e2e-service-consumer", layer="GENERAL", p="50,75,90"})&start='$(($(date +%s)-1800))'&end='$(date +%s) expected: expected/service-metric-labeled-matrix-aggregate-without-p.yml - query: curl -X GET http://${oap_host}:${oap_9090}/api/v1/query_range -d 'query=service_cpm{layer="GENERAL",top_n="10", order="DES"}&start='$(($(date +%s)-1800))'&end='$(date +%s)