Merge pull request #526 from burmanm/hwkmetrics-426

[HWKMETRICS-426] Change Percentiles to return original percentile
hawkular · Jun 27, 2016 · 8faf30e · 8faf30e
2 parents f6dd1c7 + 03f7db1
commit 8faf30e
Show file tree

Hide file tree

Showing 17 changed files with 124 additions and 84 deletions.
diff --git a/...etrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/CounterHandler.java b/...etrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/CounterHandler.java
@@ -60,6 +60,7 @@
 import org.hawkular.metrics.model.MetricId;
 import org.hawkular.metrics.model.MetricType;
 import org.hawkular.metrics.model.NumericBucketPoint;
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.exception.RuntimeApiError;
 import org.hawkular.metrics.model.param.BucketConfig;
 import org.hawkular.metrics.model.param.Duration;
@@ -432,7 +433,7 @@ public void findCounterData(
         }
 
         final Percentiles lPercentiles = percentiles != null ? percentiles
-                : new Percentiles(Collections.<Double> emptyList());
+                : new Percentiles(Collections.<Percentile> emptyList());
 
         observableConfig
                 .flatMap((config) -> metricsService.findCounterStats(metricId,
@@ -563,7 +564,7 @@ public void findStatsCounterData(
         }
 
         final Percentiles lPercentiles = percentiles != null ? percentiles
-                : new Percentiles(Collections.<Double> emptyList());
+                : new Percentiles(Collections.<Percentile> emptyList());
 
         observableConfig
                 .flatMap((config) -> metricsService.findCounterStats(metricId,
@@ -640,7 +641,7 @@ public void findRate(
                     .subscribe(asyncResponse::resume, t -> asyncResponse.resume(serverError(t)));
         } else {
             if(percentiles == null) {
-                percentiles = new Percentiles(Collections.<Double>emptyList());
+                percentiles = new Percentiles(Collections.<Percentile>emptyList());
             }
 
             metricsService.findRateStats(metricId, timeRange.getStart(), timeRange.getEnd(), buckets,
@@ -697,7 +698,7 @@ public void findStatsRate(
         Buckets buckets = bucketConfig.getBuckets();
 
         if (percentiles == null) {
-            percentiles = new Percentiles(Collections.<Double> emptyList());
+            percentiles = new Percentiles(Collections.<Percentile> emptyList());
         }
 
         metricsService.findRateStats(metricId, timeRange.getStart(), timeRange.getEnd(), buckets,
@@ -757,7 +758,7 @@ public void findCounterDataStats(
         }
 
         if (percentiles == null) {
-            percentiles = new Percentiles(Collections.<Double> emptyList());
+            percentiles = new Percentiles(Collections.<Percentile> emptyList());
         }
 
         if (metricNames.isEmpty()) {
@@ -844,7 +845,7 @@ public void findCounterRateDataStats(
         }
 
         if (percentiles == null) {
-            percentiles = new Percentiles(Collections.<Double> emptyList());
+            percentiles = new Percentiles(Collections.<Percentile> emptyList());
         }
 
         if (metricNames.isEmpty()) {
@@ -908,7 +909,7 @@ public void findStatsByTags(
         }
         MetricId<Long> metricId = new MetricId<>(tenantId, COUNTER, id);
         final Percentiles lPercentiles = percentiles != null ? percentiles
-                : new Percentiles(Collections.<Double> emptyList());
+                : new Percentiles(Collections.<Percentile> emptyList());
         metricsService.findCounterStats(metricId, tags.getTags(), timeRange.getStart(), timeRange.getEnd(),
                 lPercentiles.getPercentiles())
                 .map(ApiUtils::mapToResponse)

diff --git a/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/GaugeHandler.java b/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/GaugeHandler.java
@@ -61,6 +61,7 @@
 import org.hawkular.metrics.model.Metric;
 import org.hawkular.metrics.model.MetricId;
 import org.hawkular.metrics.model.NumericBucketPoint;
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.exception.RuntimeApiError;
 import org.hawkular.metrics.model.param.BucketConfig;
 import org.hawkular.metrics.model.param.Duration;
@@ -434,7 +435,7 @@ public void findGaugeData(
 
         observableConfig
                 .flatMap((config) -> {
-                    List<Double> perc = percentiles == null ? Collections.emptyList() : percentiles.getPercentiles();
+                    List<Percentile> perc = percentiles == null ? Collections.emptyList() : percentiles.getPercentiles();
                     return metricsService.findGaugeStats(metricId, config, perc);
                 })
                 .flatMap(Observable::from)
@@ -559,7 +560,7 @@ public void findStatsData(
 
         observableConfig
                 .flatMap((config) -> {
-                    List<Double> perc = percentiles == null ? Collections.emptyList() : percentiles.getPercentiles();
+                    List<Percentile> perc = percentiles == null ? Collections.emptyList() : percentiles.getPercentiles();
                     return metricsService.findGaugeStats(metricId, config, perc);
                 })
                 .flatMap(Observable::from)

diff --git a/...cs-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/param/PercentilesConverter.java b/...cs-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/param/PercentilesConverter.java
@@ -21,6 +21,7 @@
 
 import javax.ws.rs.ext.ParamConverter;
 
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.param.Percentiles;
 
 
@@ -32,11 +33,12 @@
 public class PercentilesConverter implements ParamConverter<Percentiles> {
     @Override
     public Percentiles fromString(String param) {
-        return new Percentiles(Arrays.stream(param.split(",")).map(Double::valueOf).collect(Collectors.toList()));
+        return new Percentiles(Arrays.stream(param.split(",")).map(Percentile::new).collect(Collectors.toList()));
     }
 
     @Override
     public String toString(Percentiles percentiles) {
-        return percentiles.getPercentiles().stream().map(Object::toString).collect(Collectors.joining(","));
+        return percentiles.getPercentiles().stream().map(Percentile::getOriginalQuantile)
+                .collect(Collectors.joining(","));
     }
 }
diff --git a/.../metrics-core-service/src/main/java/org/hawkular/metrics/core/service/MetricsService.java b/.../metrics-core-service/src/main/java/org/hawkular/metrics/core/service/MetricsService.java
@@ -31,6 +31,7 @@
 import org.hawkular.metrics.model.MetricType;
 import org.hawkular.metrics.model.NamedDataPoint;
 import org.hawkular.metrics.model.NumericBucketPoint;
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.TaggedBucketPoint;
 import org.hawkular.metrics.model.Tenant;
 import org.hawkular.metrics.model.exception.MetricAlreadyExistsException;
@@ -181,7 +182,7 @@ <T> Observable<T> findGaugeData(MetricId<Double> id, long start, long end,
                                     Func1<Observable<DataPoint<Double>>, Observable<T>>... funcs);
 
     Observable<List<NumericBucketPoint>> findGaugeStats(MetricId<Double> metricId, BucketConfig bucketConfig,
-            List<Double> percentiles);
+            List<Percentile> percentiles);
 
     /**
      * Queries data points from a single metric and groups results using tag filter expressions that are applied
@@ -197,7 +198,7 @@ Observable<List<NumericBucketPoint>> findGaugeStats(MetricId<Double> metricId, B
      * the form tag_name:tag_value,tag_name:tag_value.
      */
     Observable<Map<String, TaggedBucketPoint>> findGaugeStats(MetricId<Double> metricId, Map<String, String> tags,
-            long start, long end, List<Double> percentiles);
+            long start, long end, List<Percentile> percentiles);
 
     /**
      * Fetches data points from multiple metrics that are determined by a tags filter query. Down sampling is performed
@@ -213,7 +214,7 @@ Observable<Map<String, TaggedBucketPoint>> findGaugeStats(MetricId<Double> metri
      * @return An {@link Observable} that emits a single list of {@link NumericBucketPoint}
      */
     <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(String tenantId, MetricType<T> metricType,
-            Map<String, String> tagFilters, long start, long end, Buckets buckets, List<Double> percentiles,
+            Map<String, String> tagFilters, long start, long end, Buckets buckets, List<Percentile> percentiles,
             boolean stacked);
 
     /**
@@ -230,7 +231,7 @@ <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(String
      * @return An {@link Observable} that emits a single list of {@link NumericBucketPoint}
      */
     <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(String tenantId, MetricType<T> metricType,
-            List<String> metrics, long start, long end, Buckets buckets, List<Double> percentiles, boolean stacked);
+            List<String> metrics, long start, long end, Buckets buckets, List<Percentile> percentiles, boolean stacked);
 
     Observable<DataPoint<AvailabilityType>> findAvailabilityData(MetricId<AvailabilityType> id, long start, long end,
             boolean distinct, int limit, Order order);
@@ -257,7 +258,7 @@ Observable<DataPoint<String>> findStringData(MetricId<String> id, long start, lo
      * @return an {@link Observable} emitting a single {@link List} of {@link NumericBucketPoint}
      */
     Observable<List<NumericBucketPoint>> findCounterStats(MetricId<Long> id, long start, long end, Buckets buckets,
-                                                          List<Double> percentiles);
+                                                          List<Percentile> percentiles);
 
     /**
      * Queries data points from a single metric and groups results using tag filter expressions that are applied
@@ -273,7 +274,7 @@ Observable<List<NumericBucketPoint>> findCounterStats(MetricId<Long> id, long st
      * the form tag_name:tag_value,tag_name:tag_value.
      */
     Observable<Map<String, TaggedBucketPoint>> findCounterStats(MetricId<Long> metricId, Map<String, String> tags,
-            long start, long end, List<Double> percentiles);
+            long start, long end, List<Percentile> percentiles);
 
     /**
      * Fetches gauge or counter data points and calculates per-minute rates.
@@ -306,7 +307,7 @@ Observable<NamedDataPoint<Double>> findRateData(List<MetricId<? extends Number>>
      * @return an {@link Observable} emitting a single {@link List} of {@link NumericBucketPoint}
      */
     Observable<List<NumericBucketPoint>> findRateStats(MetricId<? extends Number> id, long start, long end,
-                                                       Buckets buckets, List<Double> percentiles);
+                                                       Buckets buckets, List<Percentile> percentiles);
 
     /**
      * <p>

diff --git a/...rics-core-service/src/main/java/org/hawkular/metrics/core/service/MetricsServiceImpl.java b/...rics-core-service/src/main/java/org/hawkular/metrics/core/service/MetricsServiceImpl.java
@@ -61,6 +61,7 @@
 import org.hawkular.metrics.model.MetricType;
 import org.hawkular.metrics.model.NamedDataPoint;
 import org.hawkular.metrics.model.NumericBucketPoint;
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.Retention;
 import org.hawkular.metrics.model.TaggedBucketPoint;
 import org.hawkular.metrics.model.Tenant;
@@ -765,7 +766,7 @@ public Observable<NamedDataPoint<Double>> findRateData(List<MetricId<? extends N
 
     @Override
     public Observable<List<NumericBucketPoint>> findRateStats(MetricId<? extends Number> id, long start, long end,
-                                                              Buckets buckets, List<Double> percentiles) {
+                                                              Buckets buckets, List<Percentile> percentiles) {
         checkArgument(isValidTimeRange(start, end), "Invalid time range");
         return findRateData(id, start, end, 0, ASC)
                 .compose(new NumericBucketPointTransformer(buckets, percentiles));
@@ -781,7 +782,7 @@ public <T> Observable<T> findGaugeData(MetricId<Double> id, long start, long end
 
     @Override
     public Observable<List<NumericBucketPoint>> findGaugeStats(MetricId<Double> metricId, BucketConfig bucketConfig,
-                List<Double> percentiles) {
+                List<Percentile> percentiles) {
         TimeRange timeRange = bucketConfig.getTimeRange();
         checkArgument(isValidTimeRange(timeRange.getStart(), timeRange.getEnd()), "Invalid time range");
         return findDataPoints(metricId, timeRange.getStart(), timeRange.getEnd(), 0, Order.DESC)
@@ -790,15 +791,15 @@ public Observable<List<NumericBucketPoint>> findGaugeStats(MetricId<Double> metr
 
     @Override
     public Observable<Map<String, TaggedBucketPoint>> findGaugeStats(MetricId<Double> metricId,
-            Map<String, String> tags, long start, long end, List<Double> percentiles) {
+            Map<String, String> tags, long start, long end, List<Percentile> percentiles) {
         return findDataPoints(metricId, start, end, 0, Order.DESC)
                 .compose(new TaggedBucketPointTransformer(tags, percentiles));
     }
 
     @Override
     public <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(String tenantId,
             MetricType<T> metricType, Map<String, String> tagFilters, long start, long end, Buckets buckets,
-            List<Double> percentiles, boolean stacked) {
+            List<Percentile> percentiles, boolean stacked) {
 
         checkArgument(isValidTimeRange(start, end), "Invalid time range");
         checkArgument(metricType == GAUGE || metricType == GAUGE_RATE
@@ -847,7 +848,7 @@ public <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(
     @Override
     public <T extends Number> Observable<List<NumericBucketPoint>> findNumericStats(String tenantId,
             MetricType<T> metricType, List<String> metrics, long start, long end, Buckets buckets,
-            List<Double> percentiles, boolean stacked) {
+            List<Percentile> percentiles, boolean stacked) {
 
         checkArgument(isValidTimeRange(start, end), "Invalid time range");
         checkArgument(metricType == GAUGE || metricType == GAUGE_RATE
@@ -951,15 +952,15 @@ public Observable<Boolean> idExists(final MetricId<?> metricId) {
 
     @Override
     public Observable<List<NumericBucketPoint>> findCounterStats(MetricId<Long> id, long start, long end,
-            Buckets buckets, List<Double> percentiles) {
+            Buckets buckets, List<Percentile> percentiles) {
         checkArgument(isValidTimeRange(start, end), "Invalid time range");
         return findDataPoints(id, start, end, 0, ASC)
                 .compose(new NumericBucketPointTransformer(buckets, percentiles));
     }
 
     @Override
     public Observable<Map<String, TaggedBucketPoint>> findCounterStats(MetricId<Long> metricId,
-            Map<String, String> tags, long start, long end, List<Double> percentiles) {
+            Map<String, String> tags, long start, long end, List<Percentile> percentiles) {
         return findDataPoints(metricId, start, end, 0, ASC)
                 .compose(new TaggedBucketPointTransformer(tags, percentiles));
     }

diff --git a/...in/java/org/hawkular/metrics/core/service/transformers/NumericBucketPointTransformer.java b/...in/java/org/hawkular/metrics/core/service/transformers/NumericBucketPointTransformer.java
@@ -22,6 +22,7 @@
 import org.hawkular.metrics.model.Buckets;
 import org.hawkular.metrics.model.DataPoint;
 import org.hawkular.metrics.model.NumericBucketPoint;
+import org.hawkular.metrics.model.Percentile;
 
 import rx.Observable;
 import rx.Observable.Transformer;
@@ -33,9 +34,9 @@ public class NumericBucketPointTransformer
         implements Transformer<DataPoint<? extends Number>, List<NumericBucketPoint>> {
 
     private final Buckets buckets;
-    private final List<Double> percentiles;
+    private final List<Percentile> percentiles;
 
-    public NumericBucketPointTransformer(Buckets buckets, List<Double> percentiles) {
+    public NumericBucketPointTransformer(Buckets buckets, List<Percentile> percentiles) {
         this.buckets = buckets;
         this.percentiles = percentiles;
     }

diff --git a/...c/main/java/org/hawkular/metrics/core/service/transformers/NumericDataPointCollector.java b/...c/main/java/org/hawkular/metrics/core/service/transformers/NumericDataPointCollector.java
@@ -20,7 +20,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Function;
-import java.util.stream.Collectors;
 
 import org.apache.commons.math3.stat.descriptive.moment.Mean;
 import org.apache.commons.math3.stat.descriptive.rank.Max;
@@ -67,41 +66,53 @@ public synchronized double getResult() {
     private int samples = 0;
     private Min min = new Min();
     private Mean average = new Mean();
-    private PercentileWrapper median = createPercentile.apply(50.0);
     private Max max = new Max();
     private Sum sum = new Sum();
-    private List<PSquarePercentile> percentiles;
+    private List<PercentileWrapper> percentiles;
+    private List<Percentile> percentileList;
 
-    public NumericDataPointCollector(Buckets buckets, int bucketIndex, List<Double> percentileList) {
+    public NumericDataPointCollector(Buckets buckets, int bucketIndex, List<Percentile> percentilesList) {
         this.buckets = buckets;
         this.bucketIndex = bucketIndex;
-        this.percentiles = new ArrayList<>();
-        percentileList.stream().forEach(d -> percentiles.add(new PSquarePercentile(d)));
+        this.percentiles = new ArrayList<>(percentilesList.size() + 1);
+        this.percentileList = percentilesList;
+        percentilesList.stream().forEach(d -> percentiles.add(createPercentile.apply(d.getQuantile())));
+        percentiles.add(createPercentile.apply(50.0)); // Important to be the last one
     }
 
     public void increment(DataPoint<? extends Number> dataPoint) {
         Number value = dataPoint.getValue();
         min.increment(value.doubleValue());
         average.increment(value.doubleValue());
-        median.addValue(value.doubleValue());
         max.increment(value.doubleValue());
         sum.increment(value.doubleValue());
         samples++;
-        percentiles.stream().forEach(p -> p.increment(value.doubleValue()));
+        percentiles.stream().forEach(p -> p.addValue(value.doubleValue()));
     }
 
     public NumericBucketPoint toBucketPoint() {
         long from = buckets.getBucketStart(bucketIndex);
         long to = from + buckets.getStep();
+
+        // Original percentilesList can't be modified as it is used elsewhere
+        List<Percentile> percentileReturns = new ArrayList<>(percentileList.size());
+
+        if(percentileList.size() > 0) {
+            for(int i = 0; i < percentileList.size(); i++) {
+                Percentile p = percentileList.get(i);
+                PercentileWrapper pw = percentiles.get(i);
+                percentileReturns.add(new Percentile(p.getOriginalQuantile(), pw.getResult()));
+            }
+        }
+
         return new NumericBucketPoint.Builder(from, to)
                 .setMin(min.getResult())
                 .setAvg(average.getResult())
-                .setMedian(median.getResult())
+                .setMedian(this.percentiles.get(this.percentiles.size() - 1).getResult())
                 .setMax(max.getResult())
                 .setSum(sum.getResult())
                 .setSamples(samples)
-                .setPercentiles(percentiles.stream()
-                        .map(p -> new Percentile(p.quantile(), p.getResult())).collect(Collectors.toList()))
+                .setPercentiles(percentileReturns)
                 .build();
     }
 }
diff --git a/...ain/java/org/hawkular/metrics/core/service/transformers/TaggedBucketPointTransformer.java b/...ain/java/org/hawkular/metrics/core/service/transformers/TaggedBucketPointTransformer.java
@@ -29,6 +29,7 @@
 import java.util.regex.Pattern;
 
 import org.hawkular.metrics.model.DataPoint;
+import org.hawkular.metrics.model.Percentile;
 import org.hawkular.metrics.model.TaggedBucketPoint;
 
 import rx.Observable;
@@ -41,9 +42,9 @@ public class TaggedBucketPointTransformer
         implements Transformer<DataPoint<? extends Number>, Map<String, TaggedBucketPoint>> {
 
     private final Map<String, String> tags;
-    private final List<Double> percentiles;
+    private final List<Percentile> percentiles;
 
-    public TaggedBucketPointTransformer(Map<String, String> tags, List<Double> percentiles) {
+    public TaggedBucketPointTransformer(Map<String, String> tags, List<Percentile> percentiles) {
         this.tags = tags;
         this.percentiles = percentiles;
     }