TopN query attrs filter support not equal. (#12730)

docs/en/api/metrics-query-expression.md

@@ -237,7 +237,12 @@ top_n(<metric_name>, <top_number>, <order>, <attrs>)
 - `attrs` optional, attrs is the attributes of the metrics, could be used to filter the topN results. 
    SkyWalking supports 6 attrs: `attr0`, `attr1`, `attr2`, `attr3`, `attr4`, `attr5`. 
    The format is `attr0='value', attr1='value'...attr5='value5'`, could use one or multiple attrs to filter the topN results.
-  **Notice**: The `attrs` only support Service metrics for now and should be added in the metrics first, see [Metrics Additional Attributes](../concepts-and-designs/metrics-additional-attributes.md).
+   The attrs filter also supports not-equal filter `!=`, the format is `attr0 != 'value'`.
+
+**Notice**: 
+- The `attrs` only support Service metrics for now and should be added in the metrics first, see [Metrics Additional Attributes](../concepts-and-designs/metrics-additional-attributes.md).
+- When use not-equal filter, for example `attr1 != 'value'`, if the storage is using `MySQL` or other JDBC storage and `attr1 value is NULL` in the metrics, 
+the result of `attr1 != 'value'` will always `false` and would NOT include this metric in the result due to SQL can't compare `NULL` with the `value`.
 
 For example:
 1. If we want to query the top 10 services with the highest `service_cpm` metric value, we can use the following expression and make sure the `entity` is empty:

oap-server/mqe-grammar/src/main/antlr4/org/apache/skywalking/mqe/rt/grammar/MQEParser.g4

@@ -102,5 +102,5 @@ sort_label_values:
 
 attributeName:
     ATTR0 | ATTR1 | ATTR2 | ATTR3 | ATTR4 | ATTR5;
-attribute: attributeName EQ VALUE_STRING;
+attribute: attributeName (EQ | NEQ) VALUE_STRING;
 attributeList: attribute (COMMA attribute)*;

oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/metrics/Metrics.java

@@ -38,7 +38,6 @@ import org.apache.skywalking.oap.server.core.storage.annotation.Column;
 public abstract class Metrics extends StreamData implements StorageData {
     public static final String ENTITY_ID = "entity_id";
     public static final String ID = "id";
-    public static final String ATTR_NAME_PREFIX = "attr";
 
     /**
      * Time attribute

oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/input/AttrCondition.java

@@ -0,0 +1,30 @@
+/*
+ * 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.
+ *
+ */
+
+package org.apache.skywalking.oap.server.core.query.input;
+
+import lombok.Data;
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+@Data
+public class AttrCondition {
+    private final String key;
+    private final String value;
+    private final boolean isEquals;
+}

oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/input/TopNCondition.java

@@ -18,6 +18,7 @@
 
 package org.apache.skywalking.oap.server.core.query.input;
 
+import java.util.List;
 import lombok.Getter;
 import lombok.Setter;
 import lombok.ToString;
@@ -62,7 +63,7 @@ public class TopNCondition {
      * Attributes for query condition, if the metrics support attributes from
      * {@link org.apache.skywalking.oap.server.core.analysis.ISourceDecorator}.
      */
-    private String[] attributes;
+    private List<AttrCondition> attributes;
 
     /**
      * Sense Scope through metric name.

oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/mqe/rt/MQEVisitor.java

@@ -37,6 +37,7 @@ import org.apache.skywalking.oap.server.core.query.MetricsQueryService;
 import org.apache.skywalking.oap.server.core.query.PointOfTime;
 import org.apache.skywalking.oap.server.core.query.RecordQueryService;
 import org.apache.skywalking.oap.server.core.query.enumeration.Order;
+import org.apache.skywalking.oap.server.core.query.input.AttrCondition;
 import org.apache.skywalking.oap.server.core.query.input.Duration;
 import org.apache.skywalking.oap.server.core.query.input.Entity;
 import org.apache.skywalking.oap.server.core.query.input.MetricsCondition;
@@ -129,21 +130,24 @@ public class MQEVisitor extends MQEVisitorBase {
                         if (topN <= 0) {
                             throw new IllegalExpressionException("TopN value must be > 0.");
                         }
-                        String[] attrsCondition = new String[6];
+                        List<AttrCondition> attrConditions = new ArrayList<>();
                         if (parent.attributeList() != null) {
                             for (MQEParser.AttributeContext attributeContext : parent.attributeList().attribute()) {
                                 String attrName = attributeContext.attributeName().getText();
                                 String attrValue = attributeContext.VALUE_STRING().getText();
                                 if (StringUtil.isNotBlank(attrValue)) {
                                     String attrValueTrim = attrValue.substring(1, attrValue.length() - 1);
-                                    int index = Integer.parseInt(attrName.substring(attrName.length() - 1));
+                                    if (attributeContext.EQ() != null) {
-                                    attrsCondition[index] = attrValueTrim;
+                                        attrConditions.add(new AttrCondition(attrName, attrValueTrim, true));
+                                    } else if (attributeContext.NEQ() != null) {
+                                        attrConditions.add(new AttrCondition(attrName, attrValueTrim, false));
+                                    }
                                 }
                             }
                         }
 
                         querySortMetrics(metricName, Integer.parseInt(parent.INTEGER().getText()),
-                                         Order.valueOf(parent.order().getText().toUpperCase()), attrsCondition, result);
+                                         Order.valueOf(parent.order().getText().toUpperCase()), attrConditions, result);
                     } else if (ctx.parent instanceof MQEParser.TrendOPContext) {
                         //trend query requires get previous data according to the trend range
                         MQEParser.TrendOPContext parent = (MQEParser.TrendOPContext) ctx.parent;
@@ -195,7 +199,7 @@ public class MQEVisitor extends MQEVisitorBase {
     private void querySortMetrics(String metricName,
                                   int topN,
                                   Order order,
-                                  String[] attrs,
+                                  List<AttrCondition> attrConditions,
                                   ExpressionResult result) throws IOException {
         TopNCondition topNCondition = new TopNCondition();
         topNCondition.setName(metricName);
@@ -203,7 +207,7 @@ public class MQEVisitor extends MQEVisitorBase {
         topNCondition.setParentService(entity.getServiceName());
         topNCondition.setOrder(order);
         topNCondition.setNormal(entity.getNormal());
-        topNCondition.setAttributes(attrs);
+        topNCondition.setAttributes(attrConditions);
 
         List<SelectedRecord> selectedRecords = getAggregationQueryService().sortMetrics(topNCondition, duration);
 

oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBAggregationQueryDAO.java

@@ -33,7 +33,6 @@ import org.apache.skywalking.oap.server.core.query.type.KeyValue;
 import org.apache.skywalking.oap.server.core.query.type.SelectedRecord;
 import org.apache.skywalking.oap.server.core.storage.query.IAggregationQueryDAO;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.apache.skywalking.oap.server.library.util.StringUtil;
 import org.apache.skywalking.oap.server.storage.plugin.banyandb.stream.AbstractBanyanDBDAO;
 import org.apache.skywalking.oap.server.storage.plugin.banyandb.util.ByteUtil;
 
@@ -124,14 +123,13 @@ public class BanyanDBAggregationQueryDAO extends AbstractBanyanDBDAO implements
                                     )));
                         }
                         if (CollectionUtils.isNotEmpty(condition.getAttributes())) {
-                            for (int i = 0; i < condition.getAttributes().length; i++) {
+                            condition.getAttributes().forEach(attr -> {
-                                if (StringUtil.isNotEmpty(condition.getAttributes()[i])) {
+                                if (attr.isEquals()) {
-                                    query.and(eq(
+                                    query.and(eq(attr.getKey(), attr.getValue()));
-                                        Metrics.ATTR_NAME_PREFIX + i,
+                                } else {
-                                        condition.getAttributes()[i]
+                                    query.and(ne(attr.getKey(), attr.getValue()));
-                                    ));
                                 }
-                            }
+                            });
                         }
                     }
                 });

oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/stream/AbstractBanyanDBDAO.java

@@ -19,7 +19,6 @@
 package org.apache.skywalking.oap.server.storage.plugin.banyandb.stream;
 
 import com.google.gson.Gson;
-import java.util.Arrays;
 import org.apache.skywalking.banyandb.model.v1.BanyandbModel;
 import org.apache.skywalking.banyandb.v1.client.AbstractCriteria;
 import org.apache.skywalking.banyandb.v1.client.AbstractQuery;
@@ -35,13 +34,12 @@ import org.apache.skywalking.banyandb.v1.client.TimestampRange;
 import org.apache.skywalking.banyandb.v1.client.TopNQuery;
 import org.apache.skywalking.banyandb.v1.client.TopNQueryResponse;
 import org.apache.skywalking.banyandb.v1.client.Trace;
-import org.apache.skywalking.oap.server.core.analysis.metrics.Metrics;
+import org.apache.skywalking.oap.server.core.query.input.AttrCondition;
 import org.apache.skywalking.oap.server.core.query.type.KeyValue;
 import org.apache.skywalking.oap.server.core.query.type.debugging.DebuggingSpan;
 import org.apache.skywalking.oap.server.core.query.type.debugging.DebuggingTraceContext;
 import org.apache.skywalking.oap.server.core.storage.AbstractDAO;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.apache.skywalking.oap.server.library.util.StringUtil;
 import org.apache.skywalking.oap.server.storage.plugin.banyandb.BanyanDBStorageClient;
 import org.apache.skywalking.oap.server.storage.plugin.banyandb.MetadataRegistry;
 import java.io.IOException;
@@ -127,7 +125,7 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
                                      TimestampRange timestampRange,
                                      int number,
                                      List<KeyValue> additionalConditions,
-                                     String[] attributes) throws IOException {
+                                     List<AttrCondition> attributes) throws IOException {
         return topNQuery(schema, timestampRange, number, AbstractQuery.Sort.DESC, additionalConditions, attributes);
     }
 
@@ -135,7 +133,7 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
                                         TimestampRange timestampRange,
                                         int number,
                                         List<KeyValue> additionalConditions,
-                                        String[] attributes) throws IOException {
+                                        List<AttrCondition> attributes) throws IOException {
         return topNQuery(schema, timestampRange, number, AbstractQuery.Sort.ASC, additionalConditions, attributes);
     }
 
@@ -144,7 +142,7 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
                                                     int number,
                                                     AbstractQuery.Sort sort,
                                                     List<KeyValue> additionalConditions,
-                                                    String[] attributes) throws IOException {
+                                                    List<AttrCondition> attributes) throws IOException {
         DebuggingTraceContext traceContext = DebuggingTraceContext.TRACE_CONTEXT.get();
         DebuggingSpan span = null;
         try {
@@ -163,7 +161,7 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
                        .append(", AdditionalConditions: ")
                        .append(additionalConditions)
                        .append(", Attributes: ")
-                       .append(Arrays.toString(attributes));
+                       .append(attributes);
                 span.setMsg(builder.toString());
             }
             TopNQueryResponse response = topNQuery(schema, timestampRange, number, sort, additionalConditions, attributes);
@@ -184,7 +182,7 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
                                         int number,
                                         AbstractQuery.Sort sort,
                                         List<KeyValue> additionalConditions,
-                                        String[] attributes) throws IOException {
+                                        List<AttrCondition> attributes) throws IOException {
         final TopNQuery q = new TopNQuery(List.of(schema.getMetadata().getGroup()), schema.getTopNSpec().getName(),
                 timestampRange,
                 number, sort);
@@ -196,12 +194,13 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
             }
         }
         if (CollectionUtils.isNotEmpty(attributes)) {
-            for (int i = 0; i < attributes.length; i++) {
+            attributes.forEach(attr -> {
-                if (StringUtil.isNotEmpty(attributes[i])) {
+                if (attr.isEquals()) {
-                    conditions.add(
+                    conditions.add(PairQueryCondition.StringQueryCondition.eq(attr.getKey(), attr.getValue()));
-                        PairQueryCondition.StringQueryCondition.eq(Metrics.ATTR_NAME_PREFIX + i, attributes[i]));
+                } else {
+                    conditions.add(PairQueryCondition.StringQueryCondition.ne(attr.getKey(), attr.getValue()));
                 }
-            }
+            });
         }
         q.setConditions(conditions);
 
@@ -326,6 +325,10 @@ public abstract class AbstractBanyanDBDAO extends AbstractDAO<BanyanDBStorageCli
             return PairQueryCondition.StringQueryCondition.eq(name, value);
         }
 
+        protected PairQueryCondition<String> ne(String name, String value) {
+            return PairQueryCondition.StringQueryCondition.ne(name, value);
+        }
+
         protected PairQueryCondition<String> match(String name, String value) {
             return PairQueryCondition.StringQueryCondition.match(name, value);
         }

oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/AggregationQueryEsDAO.java

@@ -30,6 +30,7 @@ import org.apache.skywalking.library.elasticsearch.requests.search.aggregation.T
 import org.apache.skywalking.library.elasticsearch.response.search.SearchResponse;
 import org.apache.skywalking.oap.server.core.analysis.metrics.Metrics;
 import org.apache.skywalking.oap.server.core.query.enumeration.Order;
+import org.apache.skywalking.oap.server.core.query.input.AttrCondition;
 import org.apache.skywalking.oap.server.core.query.input.Duration;
 import org.apache.skywalking.oap.server.core.query.input.TopNCondition;
 import org.apache.skywalking.oap.server.core.query.type.KeyValue;
@@ -37,7 +38,6 @@ import org.apache.skywalking.oap.server.core.query.type.SelectedRecord;
 import org.apache.skywalking.oap.server.core.storage.query.IAggregationQueryDAO;
 import org.apache.skywalking.oap.server.library.client.elasticsearch.ElasticSearchClient;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.apache.skywalking.oap.server.library.util.StringUtil;
 import org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base.EsDAO;
 import org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base.IndexController;
 import org.apache.skywalking.oap.server.storage.plugin.elasticsearch.base.TimeRangeIndexNameGenerator;
@@ -65,13 +65,15 @@ public class AggregationQueryEsDAO extends EsDAO implements IAggregationQueryDAO
         final SearchBuilder search = Search.builder();
 
         final boolean asc = condition.getOrder().equals(Order.ASC);
-        String[] attributes = condition.getAttributes();
+        List<AttrCondition> attributes = condition.getAttributes();
         if (CollectionUtils.isNotEmpty(attributes)) {
-            for (int i = 0; i < attributes.length; i++) {
+            attributes.forEach(attr -> {
-                if (StringUtil.isNotEmpty(attributes[i])) {
+                if (attr.isEquals()) {
-                    boolQuery.must(Query.term(Metrics.ATTR_NAME_PREFIX + i, attributes[i]));
+                    boolQuery.must(Query.term(attr.getKey(), attr.getValue()));
+                } else {
+                    boolQuery.mustNot(Query.terms(attr.getKey(), attr.getValue()));
                 }
-            }
+            });
         }
 
         if (CollectionUtils.isEmpty(additionalConditions)

oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/common/dao/JDBCAggregationQueryDAO.java

@@ -29,7 +29,6 @@ import org.apache.skywalking.oap.server.core.query.type.SelectedRecord;
 import org.apache.skywalking.oap.server.core.storage.query.IAggregationQueryDAO;
 import org.apache.skywalking.oap.server.library.client.jdbc.hikaricp.JDBCClient;
 import org.apache.skywalking.oap.server.library.util.CollectionUtils;
-import org.apache.skywalking.oap.server.library.util.StringUtil;
 import org.apache.skywalking.oap.server.storage.plugin.jdbc.common.JDBCTableInstaller;
 import org.apache.skywalking.oap.server.storage.plugin.jdbc.common.SQLAndParameters;
 import org.apache.skywalking.oap.server.storage.plugin.jdbc.common.TableHelper;
@@ -122,12 +121,14 @@ public class JDBCAggregationQueryDAO implements IAggregationQueryDAO {
             });
         }
         if (CollectionUtils.isNotEmpty(metrics.getAttributes())) {
-            for (int i = 0; i < metrics.getAttributes().length; i++) {
+            metrics.getAttributes().forEach(attr -> {
-                if (StringUtil.isNotEmpty(metrics.getAttributes()[i])) {
+                if (attr.isEquals()) {
-                    sql.append(" and ").append(Metrics.ATTR_NAME_PREFIX).append(i).append(" = ?");
+                    sql.append(" and ").append(attr.getKey()).append(" = ?");
-                    parameters.add(metrics.getAttributes()[i]);
+                } else {
+                    sql.append(" and ").append(attr.getKey()).append(" != ?");
                 }
-            }
+                parameters.add(attr.getValue());
+            });
         }
         sql.append(" group by ").append(Metrics.ENTITY_ID);
         sql.append(")  as T order by result")

test/e2e-v2/cases/mqe/mqe-cases.yaml

@@ -42,8 +42,10 @@ cases:
     # topN-OP-service Global with attrs
   - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0='GENERAL')/100"
     expected: expected/topN-OP-service.yml
+  - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0!='Not_GENERAL')/100"
+    expected: expected/topN-OP-service.yml
 
-  # topN-OP-service
+  # topN-OP-isntance
   - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_instance_sla,3,des)/100" --service-name=e2e-service-provider
     expected: expected/topN-OP-instance.yml
 

test/e2e-v2/cases/storage/banyandb/e2e.yaml

@@ -48,3 +48,5 @@ verify:
   cases:
     - includes:
       - ../storage-cases.yaml
+    - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0='GENERAL',attr1!='Not_exist')/100"
+      expected: ../expected/topN-OP-service.yml

test/e2e-v2/cases/storage/es/e2e.yaml

@@ -48,3 +48,5 @@ verify:
   cases:
     - includes:
       - ../storage-cases.yaml
+    - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0='GENERAL',attr1!='Not_exist')/100"
+      expected: ../expected/topN-OP-service.yml

test/e2e-v2/cases/storage/opensearch/e2e.yaml

@@ -48,3 +48,5 @@ verify:
   cases:
     - includes:
       - ../storage-cases.yaml
+    - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0='GENERAL',attr1!='Not_exist')/100"
+      expected: ../expected/topN-OP-service.yml

test/e2e-v2/cases/storage/storage-cases.yaml

@@ -166,3 +166,5 @@ cases:
     # topN-OP-service Global with attrs
   - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0='GENERAL')/100"
     expected: expected/topN-OP-service.yml
+  - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="top_n(service_sla,3,des,attr0!='Not_GENERAL')/100"
+    expected: expected/topN-OP-service.yml