Fix bug that the same sample name in one MAL expression caused...

Fix bug that the same sample name in one MAL expression caused `IllegalArgumentException` in `Analyzer.analyse`. (#6250)

CHANGES.md

@@ -75,6 +75,7 @@ Release Notes.
 * Fix bug in `parseInternalErrorCode` where some error codes are never reached.
 * OAL supports multiple values when as numeric
 * Add node information from the Openensus proto to the labels of the samples, to support the identification of the source of the Metric data.
+* Fix bug that the same sample name in one MAL expression caused `IllegalArgumentException` in `Analyzer.analyse`.
 
 #### UI
 * Fix un-removed tags in trace query.

oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/Expression.java

@@ -95,7 +95,11 @@ public class Expression {
         expression.setDelegate(new GroovyObjectSupport() {
 
             public SampleFamily propertyMissing(String metricName) {
-                ExpressionParsingContext.get().ifPresent(ctx -> ctx.samples.add(metricName));
+                ExpressionParsingContext.get().ifPresent(ctx -> {
+                    if (!ctx.samples.contains(metricName)) {
+                        ctx.samples.add(metricName);
+                    }
+                });
                 ImmutableMap<String, SampleFamily> sampleFamilies = propertyRepository.get();
                 if (sampleFamilies == null) {
                     return SampleFamily.EMPTY;

oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/ArithmeticTest.java

@@ -238,6 +238,20 @@ public class ArithmeticTest {
                 ).build()),
                 false,
             },
+            {
+                "sameSampleFamily-minus-sameSampleFamily",
+                of("http_success_request", SampleFamilyBuilder.newBuilder(
+                    Sample.builder().labels(of("idc", "t1" , "service", "service1")).value(100).build(),
+                    Sample.builder().labels(of("idc", "t2" , "service", "service1")).value(30).build(),
+                    Sample.builder().labels(of("idc", "t3" , "service", "service1")).value(40).build(),
+                    Sample.builder().labels(of("region", "us" , "service", "service1")).value(80).build()
+                ).build()),
+                "http_success_request.sum(['service']) - http_success_request.sum(['service'])",
+                Result.success(SampleFamilyBuilder.newBuilder(
+                    Sample.builder().labels(of("service", "service1")).value(0).build()
+                ).build()),
+                false,
+                },
             {
                 "empty-multiple-empty",
                 of("http_success_request", SampleFamily.EMPTY,

oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/ExpressionParsingTest.java

@@ -18,6 +18,7 @@
 
 package org.apache.skywalking.oap.meter.analyzer.dsl;
 
+import com.google.common.collect.Lists;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.skywalking.oap.server.core.analysis.meter.ScopeType;
 import org.junit.Test;
@@ -74,6 +75,18 @@ public class ExpressionParsingTest {
                                         .percentiles(new int[]{50, 99}).build(),
                 false,
             },
+            {
+                "sameSamples",
+                "(node_cpu_seconds_total.sum(['node_identifier_host_name']) - node_cpu_seconds_total.tagEqual('mode', 'idle').sum(['node_identifier_host_name'])).service(['node_identifier_host_name']) ",
+                ExpressionParsingContext.builder()
+                                        .samples(Collections.singletonList("node_cpu_seconds_total"))
+                                        .scopeType(ScopeType.SERVICE)
+                                        .scopeLabels(Collections.singletonList("node_identifier_host_name"))
+                                        .aggregationLabels(Lists.newArrayList("node_identifier_host_name" , "node_identifier_host_name"))
+                                        .downsampling(DownsamplingType.AVG)
+                                        .isHistogram(false).build(),
+                false,
+                },
         });
     }