This is an automated email from the ASF dual-hosted git repository. wankai pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push: new a8b0d18943 Remove group and support labels in the meter histogram-percentile function. (#12101) a8b0d18943 is described below commit a8b0d18943514164d231ac5a4e305b1439445d8f Author: Wan Kai <wankai...@foxmail.com> AuthorDate: Mon Apr 15 15:22:36 2024 +0800 Remove group and support labels in the meter histogram-percentile function. (#12101) --- .../skywalking/oap/meter/analyzer/Analyzer.java | 14 +- .../oap/meter/analyzer/dsl/AnalyzerTest.java | 14 +- .../analysis/meter/function/BucketedValues.java | 6 +- .../meter/function/PercentileFunction.java | 318 --------------------- .../avg/AvgHistogramPercentileFunction.java | 26 +- .../sum/SumHistogramPercentileFunction.java | 26 +- .../meter/function/PercentileFunctionTest.java | 230 --------------- .../function/avg/AvgHistogramFunctionTest.java | 3 - .../avg/AvgHistogramPercentileFunctionTest.java | 12 +- .../sum/SumHistogramPercentileFunctionTest.java | 14 +- .../expected/metrics-has-latency-value-label.yml | 147 +++++++++- .../expected/metrics-has-value-percentile.yml} | 21 +- test/e2e-v2/cases/so11y/so11y-cases.yaml | 2 + 13 files changed, 206 insertions(+), 627 deletions(-) diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java index 82af27879f..debc3d37b0 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.Predicate; -import java.util.stream.Collectors; import java.util.stream.Stream; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; @@ -159,9 +158,9 @@ public class Analyzer { break; case histogram: case histogramPercentile: - Stream.of(ss).map(s -> Tuple.of(composeGroup(s.getLabels(), k -> !Objects.equals("le", k)), s)) + Stream.of(ss).map(s -> Tuple.of(getDataLabels(s.getLabels(), k -> !Objects.equals("le", k)), s)) .collect(groupingBy(Tuple2::_1, mapping(Tuple2::_2, toList()))) - .forEach((group, subSs) -> { + .forEach((dataLabel, subSs) -> { if (subSs.size() < 1) { return; } @@ -178,7 +177,7 @@ public class Analyzer { vv[i] = getValue(s); } BucketedValues bv = new BucketedValues(bb, vv); - bv.setGroup(group); + bv.setLabels(dataLabel); long time = subSs.get(0).getTimestamp(); if (metricType == MetricType.histogram) { AcceptableValue<BucketedValues> v = meterSystem.buildMetrics( @@ -207,9 +206,10 @@ public class Analyzer { return Math.round(sample.getValue()); } - private String composeGroup(ImmutableMap<String, String> labels, Predicate<String> filter) { - return labels.keySet().stream().filter(filter).sorted().map(labels::get) - .collect(Collectors.joining("-")); + private DataLabel getDataLabels(ImmutableMap<String, String> labels, Predicate<String> filter) { + DataLabel dataLabel = new DataLabel(); + labels.keySet().stream().filter(filter).forEach(k -> dataLabel.put(k, labels.get(k))); + return dataLabel; } @RequiredArgsConstructor diff --git a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/AnalyzerTest.java b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/AnalyzerTest.java index a1868fdfc9..be3b27bdfe 100644 --- a/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/AnalyzerTest.java +++ b/oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/AnalyzerTest.java @@ -209,7 +209,7 @@ public class AnalyzerTest { doAnswer(invocationOnMock -> { AvgHistogramPercentileFunction actValue = (AvgHistogramPercentileFunction) invocationOnMock.getArgument( 0, AcceptableValue.class); - if (actValue.getSummation().hasKey("instance1:25")) { + if (actValue.getSummation().hasKey("{instance=instance1}:25")) { actValues.put("instance1", actValue); } else { actValues.put("instance2", actValue); @@ -233,12 +233,12 @@ public class AnalyzerTest { }); AvgHistogramPercentileFunction instance1 = actValues.get("instance1"); AvgHistogramPercentileFunction instance2 = actValues.get("instance2"); - assertEquals(100L, instance1.getSummation().get("instance1:25"), 0.0); - assertEquals(300L, instance1.getSummation().get("instance1:1250"), 0.0); - assertEquals(1L, instance1.getCount().get("instance1:25"), 0.0); - assertEquals(1L, instance1.getCount().get("instance1:1250"), 0.0); + assertEquals(100L, instance1.getSummation().get("{instance=instance1}:25"), 0.0); + assertEquals(300L, instance1.getSummation().get("{instance=instance1}:1250"), 0.0); + assertEquals(1L, instance1.getCount().get("{instance=instance1}:25"), 0.0); + assertEquals(1L, instance1.getCount().get("{instance=instance1}:1250"), 0.0); - assertEquals(122L, instance2.getSummation().get("instance2:750"), 0.0); - assertEquals(1L, instance2.getCount().get("instance2:750"), 0.0); + assertEquals(122L, instance2.getSummation().get("{instance=instance2}:750"), 0.0); + assertEquals(1L, instance2.getCount().get("{instance=instance2}:750"), 0.0); } } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/BucketedValues.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/BucketedValues.java index 432f7b8614..ed1264912a 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/BucketedValues.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/BucketedValues.java @@ -24,6 +24,7 @@ import lombok.Getter; import lombok.Setter; import lombok.ToString; import org.apache.commons.lang3.StringUtils; +import org.apache.skywalking.oap.server.core.analysis.metrics.DataLabel; import org.apache.skywalking.oap.server.core.analysis.metrics.DataTable; import org.apache.skywalking.oap.server.core.query.type.Bucket; import org.apache.skywalking.oap.server.core.query.type.HeatMap; @@ -34,9 +35,8 @@ import org.apache.skywalking.oap.server.core.query.type.HeatMap; @ToString @Getter public class BucketedValues { - @Setter - private String group; + private DataLabel labels = new DataLabel(); /** * The element in the buckets represent the minimal value of this bucket, the max is defined by the next element. * Such as 0, 10, 50, 100 means buckets are [0, 10), [10, 50), [50, 100), [100, infinite+). @@ -76,7 +76,7 @@ public class BucketedValues { if (key.equals(Bucket.INFINITE_NEGATIVE)) { existedBuckets[i] = Long.MIN_VALUE; } else { - // remove the group name + // remove the label name if (key.contains(":")) { key = StringUtils.substringAfterLast(key, ":"); } diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunction.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunction.java deleted file mode 100644 index 33af4489b5..0000000000 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunction.java +++ /dev/null @@ -1,318 +0,0 @@ -/* - * 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.analysis.meter.function; - -import org.apache.skywalking.oap.server.core.UnexpectedException; -import org.apache.skywalking.oap.server.core.analysis.meter.Meter; -import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity; -import org.apache.skywalking.oap.server.core.analysis.metrics.DataLabel; -import org.apache.skywalking.oap.server.core.analysis.metrics.DataTable; -import org.apache.skywalking.oap.server.core.analysis.metrics.IntList; -import org.apache.skywalking.oap.server.core.analysis.metrics.LabeledValueHolder; -import org.apache.skywalking.oap.server.core.analysis.metrics.Metrics; -import org.apache.skywalking.oap.server.core.analysis.metrics.PercentileMetrics; -import org.apache.skywalking.oap.server.core.query.type.Bucket; -import org.apache.skywalking.oap.server.core.remote.grpc.proto.RemoteData; -import org.apache.skywalking.oap.server.core.storage.StorageID; -import org.apache.skywalking.oap.server.core.storage.annotation.BanyanDB; -import org.apache.skywalking.oap.server.core.storage.annotation.Column; -import org.apache.skywalking.oap.server.core.storage.annotation.ElasticSearch; -import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity; -import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage; -import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder; -import java.util.Comparator; -import java.util.List; -import java.util.Objects; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import lombok.Setter; -import lombok.extern.slf4j.Slf4j; - -import static org.apache.skywalking.oap.server.core.analysis.metrics.DataLabel.PERCENTILE_LABEL_NAME; - -/** - * PercentileFunction is the implementation of {@link PercentileMetrics} in the meter system. The major difference is - * the PercentileFunction accepts the {@link PercentileArgument} as input rather than every single request. - */ -@MeterFunction(functionName = "percentile") -@Slf4j -public abstract class PercentileFunction extends Meter implements AcceptableValue<PercentileFunction.PercentileArgument>, LabeledValueHolder { - public static final String DATASET = "dataset"; - public static final String RANKS = "ranks"; - public static final String VALUE = "datatable_value"; - - @Setter - @Getter - @Column(name = ENTITY_ID, length = 512) - @BanyanDB.SeriesID(index = 0) - private String entityId; - @Getter - @Setter - @Column(name = VALUE, dataType = Column.ValueDataType.LABELED_VALUE, storageOnly = true) - @ElasticSearch.Column(legacyName = "value") - @BanyanDB.MeasureField - private DataTable percentileValues = new DataTable(10); - @Getter - @Setter - @Column(name = DATASET, storageOnly = true) - @BanyanDB.MeasureField - private DataTable dataset = new DataTable(30); - /** - * Rank - */ - @Getter - @Setter - @Column(name = RANKS, storageOnly = true) - @BanyanDB.MeasureField - private IntList ranks = new IntList(10); - - private boolean isCalculated = false; - - @Override - public void accept(final MeterEntity entity, final PercentileArgument value) { - if (dataset.size() > 0) { - if (!value.getBucketedValues().isCompatible(dataset)) { - throw new IllegalArgumentException( - "Incompatible BucketedValues [" + value + "] for current PercentileFunction[" + dataset + "]"); - } - } - - for (final int rank : value.getRanks()) { - if (rank <= 0) { - throw new IllegalArgumentException("Illegal rank value " + rank + ", must be positive"); - } - } - - if (ranks.size() > 0) { - if (ranks.size() != value.getRanks().length) { - throw new IllegalArgumentException( - "Incompatible ranks size = [" + value.getRanks().length + "] for current PercentileFunction[" + ranks - .size() + "]"); - } else { - for (final int rank : value.getRanks()) { - if (!ranks.include(rank)) { - throw new IllegalArgumentException( - "Rank " + rank + " doesn't exist in the previous ranks " + ranks); - } - } - } - } else { - for (final int rank : value.getRanks()) { - ranks.add(rank); - } - } - - this.entityId = entity.id(); - - final long[] values = value.getBucketedValues().getValues(); - for (int i = 0; i < values.length; i++) { - final long bucket = value.getBucketedValues().getBuckets()[i]; - String bucketName = bucket == Long.MIN_VALUE ? Bucket.INFINITE_NEGATIVE : String.valueOf(bucket); - final long bucketValue = values[i]; - dataset.valueAccumulation(bucketName, bucketValue); - } - - this.isCalculated = false; - } - - @Override - public boolean combine(final Metrics metrics) { - PercentileFunction percentile = (PercentileFunction) metrics; - - if (!dataset.keysEqual(percentile.getDataset())) { - log.warn("Incompatible input [{}}] for current PercentileFunction[{}], entity {}", - percentile, this, entityId - ); - return true; - } - if (this.ranks.size() > 0) { - IntList ranksOfThat = percentile.getRanks(); - if (this.ranks.size() != ranksOfThat.size()) { - log.warn("Incompatible ranks size = [{}}] for current PercentileFunction[{}]", - ranksOfThat.size(), this.ranks.size() - ); - return true; - } else { - if (!this.ranks.equals(ranksOfThat)) { - log.warn("Rank {} doesn't exist in the previous ranks {}", ranksOfThat, this.ranks); - return true; - } - } - } - - this.dataset.append(percentile.dataset); - - this.isCalculated = false; - return true; - } - - @Override - public void calculate() { - if (!isCalculated) { - long total = dataset.sumOfValues(); - - int[] roofs = new int[ranks.size()]; - for (int i = 0; i < ranks.size(); i++) { - roofs[i] = Math.round(total * ranks.get(i) * 1.0f / 100); - } - - int count = 0; - final List<String> sortedKeys = dataset.sortedKeys(Comparator.comparingInt(Integer::parseInt)); - - int loopIndex = 0; - - for (String key : sortedKeys) { - final Long value = dataset.get(key); - - count += value; - for (int rankIdx = loopIndex; rankIdx < roofs.length; rankIdx++) { - int roof = roofs[rankIdx]; - - if (count >= roof) { - DataLabel label = new DataLabel(); - label.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); - percentileValues.put(label, Long.parseLong(key)); - loopIndex++; - } else { - break; - } - } - } - } - } - - @Override - public Metrics toHour() { - PercentileFunction metrics = (PercentileFunction) createNew(); - metrics.setEntityId(getEntityId()); - metrics.setTimeBucket(toTimeBucketInHour()); - metrics.setDataset(getDataset()); - metrics.setRanks(getRanks()); - metrics.setPercentileValues(getPercentileValues()); - return metrics; - } - - @Override - public Metrics toDay() { - PercentileFunction metrics = (PercentileFunction) createNew(); - metrics.setEntityId(getEntityId()); - metrics.setTimeBucket(toTimeBucketInDay()); - metrics.setDataset(getDataset()); - metrics.setRanks(getRanks()); - metrics.setPercentileValues(getPercentileValues()); - return metrics; - } - - @Override - public DataTable getValue() { - return percentileValues; - } - - @Override - public int remoteHashCode() { - return entityId.hashCode(); - } - - @Override - public void deserialize(final RemoteData remoteData) { - this.setTimeBucket(remoteData.getDataLongs(0)); - - this.setEntityId(remoteData.getDataStrings(0)); - - this.setDataset(new DataTable(remoteData.getDataObjectStrings(0))); - this.setRanks(new IntList(remoteData.getDataObjectStrings(1))); - this.setPercentileValues(new DataTable(remoteData.getDataObjectStrings(2))); - } - - @Override - public RemoteData.Builder serialize() { - RemoteData.Builder remoteBuilder = RemoteData.newBuilder(); - remoteBuilder.addDataLongs(getTimeBucket()); - - remoteBuilder.addDataStrings(entityId); - - remoteBuilder.addDataObjectStrings(dataset.toStorageData()); - remoteBuilder.addDataObjectStrings(ranks.toStorageData()); - remoteBuilder.addDataObjectStrings(percentileValues.toStorageData()); - - return remoteBuilder; - } - - @Override - protected StorageID id0() { - return new StorageID() - .append(TIME_BUCKET, getTimeBucket()) - .append(ENTITY_ID, getEntityId()); - } - - @Override - public Class<? extends StorageBuilder> builder() { - return PercentileFunctionBuilder.class; - } - - @RequiredArgsConstructor - @Getter - public static class PercentileArgument { - private final BucketedValues bucketedValues; - private final int[] ranks; - } - - public static class PercentileFunctionBuilder implements StorageBuilder<PercentileFunction> { - @Override - public PercentileFunction storage2Entity(final Convert2Entity converter) { - PercentileFunction metrics = new PercentileFunction() { - @Override - public AcceptableValue<PercentileArgument> createNew() { - throw new UnexpectedException("createNew should not be called"); - } - }; - metrics.setDataset(new DataTable((String) converter.get(DATASET))); - metrics.setRanks(new IntList((String) converter.get(RANKS))); - metrics.setPercentileValues(new DataTable((String) converter.get(VALUE))); - metrics.setTimeBucket(((Number) converter.get(TIME_BUCKET)).longValue()); - metrics.setEntityId((String) converter.get(ENTITY_ID)); - return metrics; - } - - @Override - public void entity2Storage(final PercentileFunction storageData, final Convert2Storage converter) { - converter.accept(DATASET, storageData.getDataset()); - converter.accept(RANKS, storageData.getRanks()); - converter.accept(VALUE, storageData.getPercentileValues()); - converter.accept(TIME_BUCKET, storageData.getTimeBucket()); - converter.accept(ENTITY_ID, storageData.getEntityId()); - } - } - - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (!(o instanceof PercentileFunction)) - return false; - PercentileFunction function = (PercentileFunction) o; - return Objects.equals(entityId, function.entityId) && - getTimeBucket() == function.getTimeBucket(); - } - - @Override - public int hashCode() { - return Objects.hash(entityId, getTimeBucket()); - } -} diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunction.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunction.java index 38ab223d10..570d7ab247 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunction.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunction.java @@ -38,7 +38,6 @@ import org.apache.skywalking.oap.server.core.storage.annotation.ElasticSearch; import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity; import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage; import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder; -import com.google.common.base.Strings; import io.vavr.Tuple; import io.vavr.Tuple2; import java.util.Comparator; @@ -53,6 +52,7 @@ import static org.apache.skywalking.oap.server.core.analysis.metrics.DataLabel.P import lombok.Getter; import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.library.util.CollectionUtils; /** * AvgPercentile intends to calculate percentile based on the average of raw values over the interval(minute, hour or day). @@ -68,7 +68,6 @@ import lombok.extern.slf4j.Slf4j; @MeterFunction(functionName = "avgHistogramPercentile") @Slf4j public abstract class AvgHistogramPercentileFunction extends Meter implements AcceptableValue<PercentileArgument>, LabeledValueHolder { - private static final String DEFAULT_GROUP = "pD"; public static final String DATASET = "dataset"; public static final String RANKS = "ranks"; public static final String VALUE = "datatable_value"; @@ -150,8 +149,8 @@ public abstract class AvgHistogramPercentileFunction extends Meter implements Ac this.entityId = entity.id(); String template = "%s"; - if (!Strings.isNullOrEmpty(value.getBucketedValues().getGroup())) { - template = value.getBucketedValues().getGroup() + ":%s"; + if (CollectionUtils.isNotEmpty(value.getBucketedValues().getLabels())) { + template = value.getBucketedValues().getLabels() + ":%s"; } final long[] values = value.getBucketedValues().getValues(); for (int i = 0; i < values.length; i++) { @@ -207,11 +206,13 @@ public abstract class AvgHistogramPercentileFunction extends Meter implements Ac } dataset.keys().stream() .map(key -> { + DataLabel dataLabel = new DataLabel(); if (key.contains(":")) { int index = key.lastIndexOf(":"); - return Tuple.of(key.substring(0, index), key); + dataLabel.put(key.substring(0, index)); + return Tuple.of(dataLabel, key); } else { - return Tuple.of(DEFAULT_GROUP, key); + return Tuple.of(dataLabel, key); } }) .collect(groupingBy(Tuple2::_1, mapping(Tuple2::_2, Collector.of( @@ -228,7 +229,7 @@ public abstract class AvgHistogramPercentileFunction extends Meter implements Ac }, DataTable::append )))) - .forEach((group, subDataset) -> { + .forEach((labels, subDataset) -> { long total; total = subDataset.sumOfValues(); @@ -250,13 +251,12 @@ public abstract class AvgHistogramPercentileFunction extends Meter implements Ac int roof = roofs[rankIdx]; if (count >= roof) { - DataLabel label = new DataLabel(); - if (group.equals(DEFAULT_GROUP)) { - label.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); - percentileValues.put(label, Long.parseLong(key)); + if (labels.isEmpty()) { + labels.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); + percentileValues.put(labels, Long.parseLong(key)); } else { - label.put(PERCENTILE_LABEL_NAME, String.format("%s:%s", group, ranks.get(rankIdx))); - percentileValues.put(label, Long.parseLong(key)); + labels.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); + percentileValues.put(labels, Long.parseLong(key)); } loopIndex++; } else { diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunction.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunction.java index e67f317a3e..5b1af897f6 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunction.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunction.java @@ -38,7 +38,6 @@ import org.apache.skywalking.oap.server.core.storage.annotation.ElasticSearch; import org.apache.skywalking.oap.server.core.storage.type.Convert2Entity; import org.apache.skywalking.oap.server.core.storage.type.Convert2Storage; import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder; -import com.google.common.base.Strings; import io.vavr.Tuple; import io.vavr.Tuple2; import java.util.Comparator; @@ -52,6 +51,7 @@ import static org.apache.skywalking.oap.server.core.analysis.metrics.DataLabel.P import lombok.Getter; import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import org.apache.skywalking.oap.server.library.util.CollectionUtils; /** * SumPercentile intends to calculate percentile based on the summary of raw values over the interval(minute, hour or day). @@ -59,7 +59,6 @@ import lombok.extern.slf4j.Slf4j; @MeterFunction(functionName = "sumHistogramPercentile") @Slf4j public abstract class SumHistogramPercentileFunction extends Meter implements AcceptableValue<PercentileArgument>, LabeledValueHolder { - private static final String DEFAULT_GROUP = "pD"; public static final String DATASET = "dataset"; public static final String RANKS = "ranks"; public static final String VALUE = "datatable_value"; @@ -130,8 +129,8 @@ public abstract class SumHistogramPercentileFunction extends Meter implements Ac this.entityId = entity.id(); String template = "%s"; - if (!Strings.isNullOrEmpty(value.getBucketedValues().getGroup())) { - template = value.getBucketedValues().getGroup() + ":%s"; + if (CollectionUtils.isNotEmpty(value.getBucketedValues().getLabels())) { + template = value.getBucketedValues().getLabels() + ":%s"; } final long[] values = value.getBucketedValues().getValues(); for (int i = 0; i < values.length; i++) { @@ -174,11 +173,13 @@ public abstract class SumHistogramPercentileFunction extends Meter implements Ac if (!isCalculated) { summation.keys().stream() .map(key -> { + DataLabel dataLabel = new DataLabel(); if (key.contains(":")) { int index = key.lastIndexOf(":"); - return Tuple.of(key.substring(0, index), key); + dataLabel.put(key.substring(0, index)); + return Tuple.of(dataLabel, key); } else { - return Tuple.of(DEFAULT_GROUP, key); + return Tuple.of(dataLabel, key); } }) .collect(groupingBy(Tuple2::_1, mapping(Tuple2::_2, Collector.of( @@ -195,7 +196,7 @@ public abstract class SumHistogramPercentileFunction extends Meter implements Ac }, DataTable::append )))) - .forEach((group, subDataset) -> { + .forEach((labels, subDataset) -> { long total; total = subDataset.sumOfValues(); @@ -217,13 +218,12 @@ public abstract class SumHistogramPercentileFunction extends Meter implements Ac int roof = roofs[rankIdx]; if (count >= roof) { - DataLabel label = new DataLabel(); - if (group.equals(DEFAULT_GROUP)) { - label.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); - percentileValues.put(label, Long.parseLong(key)); + if (labels.isEmpty()) { + labels.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); + percentileValues.put(labels, Long.parseLong(key)); } else { - label.put(PERCENTILE_LABEL_NAME, String.format("%s:%s", group, ranks.get(rankIdx))); - percentileValues.put(label, Long.parseLong(key)); + labels.put(PERCENTILE_LABEL_NAME, String.valueOf(ranks.get(rankIdx))); + percentileValues.put(labels, Long.parseLong(key)); } loopIndex++; } else { diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunctionTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunctionTest.java deleted file mode 100644 index 4c9ec4951f..0000000000 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/PercentileFunctionTest.java +++ /dev/null @@ -1,230 +0,0 @@ -/* - * 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.analysis.meter.function; - -import org.apache.skywalking.oap.server.core.analysis.Layer; -import org.apache.skywalking.oap.server.core.analysis.meter.MeterEntity; -import org.apache.skywalking.oap.server.core.analysis.metrics.DataTable; -import org.apache.skywalking.oap.server.core.analysis.metrics.IntList; -import org.apache.skywalking.oap.server.core.config.NamingControl; -import org.apache.skywalking.oap.server.core.config.group.EndpointNameGrouping; -import org.apache.skywalking.oap.server.core.storage.type.HashMapConverter; -import org.apache.skywalking.oap.server.core.storage.type.StorageBuilder; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; - -import java.util.Map; - -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class PercentileFunctionTest { - private static final long[] BUCKETS = new long[] { - 0, - 50, - 100, - 250 - }; - - private static final long[] BUCKETS_2ND = new long[] { - 0, - 51, - 100, - 250 - }; - - private static final int[] RANKS = new int[] { - 50, - 90 - }; - - @BeforeAll - public static void setup() { - MeterEntity.setNamingControl( - new NamingControl(512, 512, 512, new EndpointNameGrouping())); - } - - @AfterAll - public static void tearDown() { - MeterEntity.setNamingControl(null); - } - - @Test - public void testFunction() { - PercentileFunctionInst inst = new PercentileFunctionInst(); - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS, - new long[] { - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS, - new long[] { - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - - inst.calculate(); - final DataTable values = inst.getValue(); - /** - * Expected percentile dataset - * <pre> - * 0 , 20 - * 50 , 40 - * 100, 60 <- P50 - * 250, 80 <- P90 - * </pre> - */ - Assertions.assertEquals(new DataTable("{p=50},100|{p=90},250"), values); - } - - @Test - public void testIncompatible() { - assertThrows(IllegalArgumentException.class, () -> { - PercentileFunctionInst inst = new PercentileFunctionInst(); - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS, - new long[]{ - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS_2ND, - new long[]{ - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - }); - } - - @Test - public void testSerialization() { - PercentileFunctionInst inst = new PercentileFunctionInst(); - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS, - new long[] { - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - - PercentileFunctionInst inst2 = new PercentileFunctionInst(); - inst2.deserialize(inst.serialize().build()); - - Assertions.assertEquals(inst, inst2); - // HistogramFunction equal doesn't include dataset. - Assertions.assertEquals(inst.getDataset(), inst2.getDataset()); - Assertions.assertEquals(inst.getRanks(), inst2.getRanks()); - Assertions.assertEquals(0, inst2.getPercentileValues().size()); - } - - @Test - public void testBuilder() throws IllegalAccessException, InstantiationException { - PercentileFunctionInst inst = new PercentileFunctionInst(); - inst.accept( - MeterEntity.newService("service-test", Layer.GENERAL), - new PercentileFunction.PercentileArgument( - new BucketedValues( - BUCKETS, - new long[] { - 10, - 20, - 30, - 40 - } - ), - RANKS - ) - ); - inst.calculate(); - - final StorageBuilder storageBuilder = inst.builder().newInstance(); - - // Simulate the storage layer do, convert the datatable to string. - final HashMapConverter.ToStorage hashMapConverter = new HashMapConverter.ToStorage(); - storageBuilder.entity2Storage(inst, hashMapConverter); - final Map<String, Object> map = hashMapConverter.obtain(); - map.put(PercentileFunction.DATASET, ((DataTable) map.get(PercentileFunction.DATASET)).toStorageData()); - map.put(PercentileFunction.VALUE, ((DataTable) map.get(PercentileFunction.VALUE)).toStorageData()); - map.put(PercentileFunction.RANKS, ((IntList) map.get(PercentileFunction.RANKS)).toStorageData()); - - final PercentileFunction inst2 = (PercentileFunction) storageBuilder.storage2Entity( - new HashMapConverter.ToEntity(map)); - Assertions.assertEquals(inst, inst2); - // HistogramFunction equal doesn't include dataset. - Assertions.assertEquals(inst.getDataset(), inst2.getDataset()); - Assertions.assertEquals(inst.getPercentileValues(), inst2.getPercentileValues()); - Assertions.assertEquals(inst.getRanks(), inst2.getRanks()); - } - - private static class PercentileFunctionInst extends PercentileFunction { - @Override - public AcceptableValue<PercentileArgument> createNew() { - return new PercentileFunctionInst(); - } - } -} diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramFunctionTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramFunctionTest.java index ddba0a4744..ac5bd82550 100644 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramFunctionTest.java +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramFunctionTest.java @@ -222,7 +222,6 @@ public class AvgHistogramFunctionTest { 10, 10 }); - bv1.setGroup("g1"); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), bv1 @@ -235,7 +234,6 @@ public class AvgHistogramFunctionTest { 3, 4 }); - bv2.setGroup("g1"); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), bv2 @@ -247,7 +245,6 @@ public class AvgHistogramFunctionTest { 6, 8 }); - bv3.setGroup("g2"); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), bv3 diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunctionTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunctionTest.java index 9ce21be907..60a88f774a 100644 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunctionTest.java +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/avg/AvgHistogramPercentileFunctionTest.java @@ -198,7 +198,7 @@ public class AvgHistogramPercentileFunctionTest { } @Test - public void testFunctionWhenGroupContainsColon() { + public void testFunctionWithLabel() { BucketedValues valuesA = new BucketedValues( BUCKETS, new long[] { @@ -208,8 +208,8 @@ public class AvgHistogramPercentileFunctionTest { 40 } ); - valuesA.setGroup("localhost:3306/swtestA"); - + valuesA.getLabels().put("url", "localhost:3306/swtestA"); + valuesA.getLabels().put("instance", "instance1"); PercentileFunctionInst inst = new PercentileFunctionInst(); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), @@ -227,8 +227,8 @@ public class AvgHistogramPercentileFunctionTest { 10 } ); - valuesB.setGroup("localhost:3306/swtestB"); - + valuesB.getLabels().put("url", "localhost:3306/swtestB"); + valuesB.getLabels().put("instance", "instance2"); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), new PercentileArgument( @@ -250,7 +250,7 @@ public class AvgHistogramPercentileFunctionTest { */ assertEquals( new DataTable( - "{p=localhost:3306/swtestB:50},50|{p=localhost:3306/swtestA:50},100|{p=localhost:3306/swtestB:90},100|{p=localhost:3306/swtestA:90},250"), + "{url=localhost:3306/swtestB,instance=instance2,p=50},50|{url=localhost:3306/swtestA,instance=instance1,p=50},100|{url=localhost:3306/swtestB,instance=instance2,p=90},100|{url=localhost:3306/swtestA,instance=instance1,p=90},250"), values ); } diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunctionTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunctionTest.java index 226e40091b..22f89ca29b 100644 --- a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunctionTest.java +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/function/sum/SumHistogramPercentileFunctionTest.java @@ -105,7 +105,7 @@ public class SumHistogramPercentileFunctionTest { * <pre> * 0 , 20 * 50 , 40 - * 100, 50 <- P50 + * 100, 60 <- P50 * 250, 80 <- P90 * </pre> */ @@ -188,7 +188,7 @@ public class SumHistogramPercentileFunctionTest { } @Test - public void testFunctionWhenGroupContainsColon() { + public void testFunctionWithLabel() { BucketedValues valuesA = new BucketedValues( BUCKETS, new long[] { @@ -198,8 +198,8 @@ public class SumHistogramPercentileFunctionTest { 40 } ); - valuesA.setGroup("localhost:3306/swtestA"); - + valuesA.getLabels().put("url", "localhost:3306/swtestA"); + valuesA.getLabels().put("instance", "instance1"); PercentileFunctionInst inst = new PercentileFunctionInst(); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), @@ -217,8 +217,8 @@ public class SumHistogramPercentileFunctionTest { 10 } ); - valuesB.setGroup("localhost:3306/swtestB"); - + valuesB.getLabels().put("url", "localhost:3306/swtestB"); + valuesB.getLabels().put("instance", "instance2"); inst.accept( MeterEntity.newService("service-test", Layer.GENERAL), new PercentileArgument( @@ -240,7 +240,7 @@ public class SumHistogramPercentileFunctionTest { */ assertEquals( new DataTable( - "{p=localhost:3306/swtestB:50},50|{p=localhost:3306/swtestA:50},100|{p=localhost:3306/swtestB:90},100|{p=localhost:3306/swtestA:90},250"), + "{url=localhost:3306/swtestB,instance=instance2,p=50},50|{url=localhost:3306/swtestA,instance=instance1,p=50},100|{url=localhost:3306/swtestB,instance=instance2,p=90},100|{url=localhost:3306/swtestA,instance=instance1,p=90},250"), values ); } diff --git a/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml b/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml index 730fee5ef2..306128f011 100644 --- a/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml +++ b/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml @@ -18,8 +18,10 @@ results: {{- contains .results }} - metric: labels: + - key: type + value: "apisix" - key: p - value: "apisix:50" + value: "50" values: {{- contains .values }} - id: {{ notEmpty .id }} @@ -31,8 +33,10 @@ results: {{- end}} - metric: labels: + - key: type + value: "apisix" - key: p - value: "apisix:99" + value: "70" values: {{- contains .values }} - id: {{ notEmpty .id }} @@ -44,8 +48,145 @@ results: {{- end}} - metric: labels: + - key: type + value: "apisix" - key: p - value: "request:99" + value: "90" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "apisix" + - key: p + value: "99" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "request" + - key: p + value: "50" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "request" + - key: p + value: "70" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "request" + - key: p + value: "90" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "request" + - key: p + value: "99" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "upstream" + - key: p + value: "50" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "upstream" + - key: p + value: "70" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "upstream" + - key: p + value: "90" + values: + {{- contains .values }} + - id: {{ notEmpty .id }} + value: {{ .value }} + traceid: null + - id: {{ notEmpty .id }} + value: null + traceid: null + {{- end}} +- metric: + labels: + - key: type + value: "upstream" + - key: p + value: "99" values: {{- contains .values }} - id: {{ notEmpty .id }} diff --git a/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml b/test/e2e-v2/cases/so11y/expected/metrics-has-value-percentile.yml similarity index 79% copy from test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml copy to test/e2e-v2/cases/so11y/expected/metrics-has-value-percentile.yml index 730fee5ef2..3e7fa80eb8 100644 --- a/test/e2e-v2/cases/apisix/expected/metrics-has-latency-value-label.yml +++ b/test/e2e-v2/cases/so11y/expected/metrics-has-value-percentile.yml @@ -19,7 +19,7 @@ results: - metric: labels: - key: p - value: "apisix:50" + value: "90" values: {{- contains .values }} - id: {{ notEmpty .id }} @@ -32,7 +32,7 @@ results: - metric: labels: - key: p - value: "apisix:99" + value: "99" values: {{- contains .values }} - id: {{ notEmpty .id }} @@ -41,19 +41,6 @@ results: - id: {{ notEmpty .id }} value: null traceid: null - {{- end}} -- metric: - labels: - - key: p - value: "request:99" - values: - {{- contains .values }} - - id: {{ notEmpty .id }} - value: {{ .value }} - traceid: null - - id: {{ notEmpty .id }} - value: null - traceid: null - {{- end}} -{{- end}} + {{- end }} + {{- end}} error: null diff --git a/test/e2e-v2/cases/so11y/so11y-cases.yaml b/test/e2e-v2/cases/so11y/so11y-cases.yaml index f0fda21fc3..331f0f88b2 100644 --- a/test/e2e-v2/cases/so11y/so11y-cases.yaml +++ b/test/e2e-v2/cases/so11y/so11y-cases.yaml @@ -44,4 +44,6 @@ expected: expected/metrics-has-value.yml - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression=meter_oap_jvm_class_total_loaded_count --instance-name=http://localhost:1234 --service-name=oap-server expected: expected/metrics-has-value.yml + - query: swctl --display yaml --base-url=http://${oap_host}:${oap_12800}/graphql metrics exec --expression="meter_oap_instance_trace_latency_percentile{p='90,99'}" --instance-name=http://localhost:1234 --service-name=oap-server + expected: expected/metrics-has-value-percentile.yml