PromQL service: fix operators result missing rangeExpression flag. (#…

…12649)
apache · Sep 26, 2024 · 74eeb5b · 74eeb5b
1 parent c5a95ba
commit 74eeb5b
Show file tree

Hide file tree

Showing 12 changed files with 120 additions and 10 deletions.
diff --git 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
@@ -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/...a/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java b/...a/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVerifyVisitor.java
@@ -54,7 +54,7 @@ public ExpressionResult visitMetric(MQEParser.MetricContext ctx) {
             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 ExpressionResult visitMetric(MQEParser.MetricContext ctx) {
             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/...in/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java b/...in/java/org/apache/skywalking/oap/server/core/alarm/provider/expr/rt/AlarmMQEVisitor.java
@@ -80,7 +80,7 @@ public ExpressionResult visitMetric(MQEParser.MetricContext ctx) {
             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/...gin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java b/...gin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/AlarmRuleTest.java
@@ -77,7 +77,7 @@ public void testExpressionVerify() throws IllegalExpressionException {
 
         //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 void testExpressionVerify() throws IllegalExpressionException {
 
         //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/...in/promql-plugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromOpUtils.java b/...in/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 @@ static MetricsRangeResult matrixBinaryOp(MetricsRangeResult matrixLeft,
                                              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 @@ static MetricsRangeResult matrixAggregateOp(MetricsRangeResult result, int funcT
 
         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 @@ private static int boolToInt(boolean v) {
     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 @@ static MetricsRangeResult matrixCompareOp(MetricsRangeResult matrixLeft,
                                               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/...lugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java b/...lugin/src/main/java/org/apache/skywalking/oap/query/promql/rt/PromQLExprQueryVisitor.java
@@ -237,7 +237,7 @@ public ParseResult visitMetricInstant(PromQLParser.MetricInstantContext ctx) {
             Optional<ValueColumnMetadata.ValueColumn> 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/...aphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java b/...aphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java
@@ -115,7 +115,7 @@ public ExpressionResult visitMetric(MQEParser.MetricContext ctx) {
                 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 ExpressionResult visitMetric(MQEParser.MetricContext ctx) {
                 } 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<KeyValue> 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
@@ -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
@@ -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
@@ -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
@@ -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)