Remove min/max from Distribution. (#1542)

* Remove min/max from Distribution.

* Add changelog entry.

Author: Bogdan Drutu

CHANGELOG.md

@@ -17,6 +17,7 @@
 - The histogram bucket boundaries (`BucketBoundaries`) and values (`Count` and `Sum`) are no longer
   supported for negative values. The Record API drops the negative `value` and logs the warning.
   This could be a breaking change if you are recording negative value for any `measure`.
+- Remove support for min/max in the stats Distribution to make it compatible with Metrics.
 
 ## 0.16.1 - 2018-09-18
 - Fix ClassCastException in Log4j log correlation

api/src/main/java/io/opencensus/stats/AggregationData.java

@@ -275,7 +275,10 @@ public abstract static class DistributionData extends AggregationData {
      * @param exemplars the exemplars associated with histogram buckets.
      * @return a {@code DistributionData}.
      * @since 0.16
+     * @deprecated since 0.17. Use {@link #create(double, long, double, List, List)}
      */
+    @Deprecated
+    @SuppressWarnings("InconsistentOverloads")
     public static DistributionData create(
         double mean,
         long count,
@@ -284,26 +287,41 @@ public static DistributionData create(
         double sumOfSquaredDeviations,
         List<Long> bucketCounts,
         List<Exemplar> exemplars) {
-      if (min != Double.POSITIVE_INFINITY || max != Double.NEGATIVE_INFINITY) {
-        Utils.checkArgument(min <= max, "max should be greater or equal to min.");
-      }
+      return create(mean, count, sumOfSquaredDeviations, bucketCounts, exemplars);
+    }
 
-      Utils.checkNotNull(bucketCounts, "bucketCounts");
-      List<Long> bucketCountsCopy = Collections.unmodifiableList(new ArrayList<Long>(bucketCounts));
-      for (Long bucket : bucketCountsCopy) {
-        Utils.checkNotNull(bucket, "bucket");
+    /**
+     * Creates a {@code DistributionData}.
+     *
+     * @param mean mean value.
+     * @param count count value.
+     * @param sumOfSquaredDeviations sum of squared deviations.
+     * @param bucketCounts histogram bucket counts.
+     * @param exemplars the exemplars associated with histogram buckets.
+     * @return a {@code DistributionData}.
+     * @since 0.17
+     */
+    public static DistributionData create(
+        double mean,
+        long count,
+        double sumOfSquaredDeviations,
+        List<Long> bucketCounts,
+        List<Exemplar> exemplars) {
+      List<Long> bucketCountsCopy =
+          Collections.unmodifiableList(
+              new ArrayList<Long>(Utils.checkNotNull(bucketCounts, "bucketCounts")));
+      for (Long bucketCount : bucketCountsCopy) {
+        Utils.checkNotNull(bucketCount, "bucketCount");
       }
 
-      Utils.checkNotNull(exemplars, "exemplar list should not be null.");
+      Utils.checkNotNull(exemplars, "exemplars");
       for (Exemplar exemplar : exemplars) {
         Utils.checkNotNull(exemplar, "exemplar");
       }
 
       return new AutoValue_AggregationData_DistributionData(
           mean,
           count,
-          min,
-          max,
           sumOfSquaredDeviations,
           bucketCountsCopy,
           Collections.<Exemplar>unmodifiableList(new ArrayList<Exemplar>(exemplars)));
@@ -320,7 +338,10 @@ public static DistributionData create(
      * @param bucketCounts histogram bucket counts.
      * @return a {@code DistributionData}.
      * @since 0.8
+     * @deprecated since 0.17. Use {@link #create(double, long, double, List)}.
      */
+    @Deprecated
+    @SuppressWarnings("InconsistentOverloads")
     public static DistributionData create(
         double mean,
         long count,
@@ -329,13 +350,23 @@ public static DistributionData create(
         double sumOfSquaredDeviations,
         List<Long> bucketCounts) {
       return create(
-          mean,
-          count,
-          min,
-          max,
-          sumOfSquaredDeviations,
-          bucketCounts,
-          Collections.<Exemplar>emptyList());
+          mean, count, sumOfSquaredDeviations, bucketCounts, Collections.<Exemplar>emptyList());
+    }
+
+    /**
+     * Creates a {@code DistributionData}.
+     *
+     * @param mean mean value.
+     * @param count count value.
+     * @param sumOfSquaredDeviations sum of squared deviations.
+     * @param bucketCounts histogram bucket counts.
+     * @return a {@code DistributionData}.
+     * @since 0.17
+     */
+    public static DistributionData create(
+        double mean, long count, double sumOfSquaredDeviations, List<Long> bucketCounts) {
+      return create(
+          mean, count, sumOfSquaredDeviations, bucketCounts, Collections.<Exemplar>emptyList());
     }
 
     /**
@@ -359,16 +390,24 @@ public static DistributionData create(
      *
      * @return the minimum of the population values.
      * @since 0.8
+     * @deprecated since 0.17. Returns {@code 0}.
      */
-    public abstract double getMin();
+    @Deprecated
+    public double getMin() {
+      return 0;
+    }
 
     /**
      * Returns the maximum of the population values.
      *
      * @return the maximum of the population values.
      * @since 0.8
+     * @deprecated since 0.17. Returns {@code 0}.
      */
-    public abstract double getMax();
+    @Deprecated
+    public double getMax() {
+      return 0;
+    }
 
     /**
      * Returns the aggregated sum of squared deviations.

api/src/test/java/io/opencensus/stats/AggregationDataTest.java

@@ -55,11 +55,9 @@ public class AggregationDataTest {
   @Test
   public void testCreateDistributionData() {
     DistributionData distributionData =
-        DistributionData.create(7.7, 10, 1.1, 9.9, 32.2, Arrays.asList(4L, 1L, 5L));
+        DistributionData.create(7.7, 10, 32.2, Arrays.asList(4L, 1L, 5L));
     assertThat(distributionData.getMean()).isWithin(TOLERANCE).of(7.7);
     assertThat(distributionData.getCount()).isEqualTo(10);
-    assertThat(distributionData.getMin()).isWithin(TOLERANCE).of(1.1);
-    assertThat(distributionData.getMax()).isWithin(TOLERANCE).of(9.9);
     assertThat(distributionData.getSumOfSquaredDeviations()).isWithin(TOLERANCE).of(32.2);
     assertThat(distributionData.getBucketCounts()).containsExactly(4L, 1L, 5L).inOrder();
   }
@@ -70,14 +68,14 @@ public void testCreateDistributionDataWithExemplar() {
     Exemplar exemplar2 = Exemplar.create(1, TIMESTAMP_1, ATTACHMENTS);
     DistributionData distributionData =
         DistributionData.create(
-            7.7, 10, 1.1, 9.9, 32.2, Arrays.asList(4L, 1L), Arrays.asList(exemplar1, exemplar2));
+            7.7, 10, 32.2, Arrays.asList(4L, 1L), Arrays.asList(exemplar1, exemplar2));
     assertThat(distributionData.getExemplars()).containsExactly(exemplar1, exemplar2).inOrder();
   }
 
   @Test
   public void testExemplar() {
     Exemplar exemplar = Exemplar.create(15.0, TIMESTAMP_1, ATTACHMENTS);
-    assertThat(exemplar.getValue()).isEqualTo(15.0);
+    assertThat(exemplar.getValue()).isWithin(TOLERANCE).of(15.0);
     assertThat(exemplar.getTimestamp()).isEqualTo(TIMESTAMP_1);
     assertThat(exemplar.getAttachments()).isEqualTo(ATTACHMENTS);
   }
@@ -109,36 +107,29 @@ public void testExemplar_PreventNullAttachmentValue() {
   public void preventNullBucketCountList() {
     thrown.expect(NullPointerException.class);
     thrown.expectMessage("bucketCounts");
-    DistributionData.create(1, 1, 1, 1, 0, null);
+    DistributionData.create(1, 1, 0, null);
   }
 
   @Test
   public void preventNullBucket() {
     thrown.expect(NullPointerException.class);
-    thrown.expectMessage("bucket");
-    DistributionData.create(1, 1, 1, 1, 0, Arrays.asList(0L, 1L, null));
+    thrown.expectMessage("bucketCount");
+    DistributionData.create(1, 1, 0, Arrays.asList(0L, 1L, null));
   }
 
   @Test
   public void preventNullExemplarList() {
     thrown.expect(NullPointerException.class);
-    thrown.expectMessage("exemplar list should not be null.");
-    DistributionData.create(1, 1, 1, 1, 0, Arrays.asList(0L, 1L, 1L), null);
+    thrown.expectMessage("exemplars");
+    DistributionData.create(1, 1, 0, Arrays.asList(0L, 1L, 1L), null);
   }
 
   @Test
   public void preventNullExemplar() {
     thrown.expect(NullPointerException.class);
     thrown.expectMessage("exemplar");
     DistributionData.create(
-        1, 1, 1, 1, 0, Arrays.asList(0L, 1L, 1L), Collections.<Exemplar>singletonList(null));
-  }
-
-  @Test
-  public void preventMinIsGreaterThanMax() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("max should be greater or equal to min.");
-    DistributionData.create(1, 1, 10, 1, 0, Arrays.asList(0L, 1L, 0L));
+        1, 1, 0, Arrays.asList(0L, 1L, 1L), Collections.<Exemplar>singletonList(null));
   }
 
   @Test
@@ -150,14 +141,13 @@ public void testEquals() {
         .addEqualityGroup(CountData.create(40), CountData.create(40))
         .addEqualityGroup(CountData.create(80), CountData.create(80))
         .addEqualityGroup(
-            DistributionData.create(10, 10, 1, 1, 0, Arrays.asList(0L, 10L, 0L)),
-            DistributionData.create(10, 10, 1, 1, 0, Arrays.asList(0L, 10L, 0L)))
-        .addEqualityGroup(DistributionData.create(10, 10, 1, 1, 0, Arrays.asList(0L, 10L, 100L)))
-        .addEqualityGroup(DistributionData.create(110, 10, 1, 1, 0, Arrays.asList(0L, 10L, 0L)))
-        .addEqualityGroup(DistributionData.create(10, 110, 1, 1, 0, Arrays.asList(0L, 10L, 0L)))
-        .addEqualityGroup(DistributionData.create(10, 10, -1, 1, 0, Arrays.asList(0L, 10L, 0L)))
-        .addEqualityGroup(DistributionData.create(10, 10, 1, 5, 0, Arrays.asList(0L, 10L, 0L)))
-        .addEqualityGroup(DistributionData.create(10, 10, 1, 1, 55.5, Arrays.asList(0L, 10L, 0L)))
+            DistributionData.create(10, 10, 0, Arrays.asList(0L, 10L, 0L)),
+            DistributionData.create(10, 10, 0, Arrays.asList(0L, 10L, 0L)))
+        .addEqualityGroup(DistributionData.create(10, 10, 0, Arrays.asList(0L, 10L, 100L)))
+        .addEqualityGroup(DistributionData.create(110, 10, 0, Arrays.asList(0L, 10L, 0L)))
+        .addEqualityGroup(DistributionData.create(10, 110, 0, Arrays.asList(0L, 10L, 0L)))
+        .addEqualityGroup(DistributionData.create(10, 10, 10, Arrays.asList(0L, 10L, 0L)))
+        .addEqualityGroup(DistributionData.create(10, 10, 0, Arrays.asList(0L, 110L, 0L)))
         .addEqualityGroup(MeanData.create(5.0, 1), MeanData.create(5.0, 1))
         .addEqualityGroup(MeanData.create(-5.0, 1), MeanData.create(-5.0, 1))
         .addEqualityGroup(LastValueDataDouble.create(20.0), LastValueDataDouble.create(20.0))
@@ -172,7 +162,7 @@ public void testMatchAndGet() {
             SumDataDouble.create(10.0),
             SumDataLong.create(100000000),
             CountData.create(40),
-            DistributionData.create(1, 1, 1, 1, 0, Arrays.asList(0L, 10L, 0L)),
+            DistributionData.create(1, 1, 0, Arrays.asList(0L, 10L, 0L)),
             LastValueDataDouble.create(20.0),
             LastValueDataLong.create(200000000L));
 

api/src/test/java/io/opencensus/stats/ViewDataTest.java

@@ -285,9 +285,9 @@ private void aggregationAndAggregationDataMismatch(
   private static final ImmutableMap<List<TagValue>, DistributionData> ENTRIES =
       ImmutableMap.of(
           Arrays.asList(V1, V2),
-          DistributionData.create(1, 1, 1, 1, 0, Arrays.asList(0L, 1L, 0L)),
+          DistributionData.create(1, 1, 0, Arrays.asList(0L, 1L, 0L)),
           Arrays.asList(V10, V20),
-          DistributionData.create(-5, 6, -20, 5, 100.1, Arrays.asList(5L, 0L, 1L)));
+          DistributionData.create(-5, 6, 100.1, Arrays.asList(5L, 0L, 1L)));
 
   // name
   private static final View.Name NAME = View.Name.create("test-view");

impl_core/src/main/java/io/opencensus/implcore/stats/MutableAggregation.java

@@ -258,10 +258,6 @@ static final class MutableDistribution extends MutableAggregation {
     private long count = 0;
     private double sumOfSquaredDeviations = 0.0;
 
-    // Initial "impossible" values, that will get reset as soon as first value is added.
-    private double min = Double.POSITIVE_INFINITY;
-    private double max = Double.NEGATIVE_INFINITY;
-
     private final BucketBoundaries bucketBoundaries;
     private final long[] bucketCounts;
 
@@ -308,13 +304,6 @@ void add(double value, Map<String, String> attachments, Timestamp timestamp) {
       double deltaFromMean2 = value - mean;
       sumOfSquaredDeviations += deltaFromMean * deltaFromMean2;
 
-      if (value < min) {
-        min = value;
-      }
-      if (value > max) {
-        max = value;
-      }
-
       int bucket = 0;
       for (; bucket < bucketBoundaries.getBoundaries().size(); bucket++) {
         if (value < bucketBoundaries.getBoundaries().get(bucket)) {
@@ -360,13 +349,6 @@ void combine(MutableAggregation other, double fraction) {
       this.sum += mutableDistribution.sum;
       this.mean = this.sum / this.count;
 
-      if (mutableDistribution.min < this.min) {
-        this.min = mutableDistribution.min;
-      }
-      if (mutableDistribution.max > this.max) {
-        this.max = mutableDistribution.max;
-      }
-
       long[] bucketCounts = mutableDistribution.getBucketCounts();
       for (int i = 0; i < bucketCounts.length; i++) {
         this.bucketCounts[i] += bucketCounts[i];
@@ -401,7 +383,7 @@ AggregationData toAggregationData() {
         }
       }
       return DistributionData.create(
-          mean, count, min, max, sumOfSquaredDeviations, boxedBucketCounts, exemplarList);
+          mean, count, sumOfSquaredDeviations, boxedBucketCounts, exemplarList);
     }
 
     @Override
@@ -446,14 +428,6 @@ long getCount() {
       return count;
     }
 
-    double getMin() {
-      return min;
-    }
-
-    double getMax() {
-      return max;
-    }
-
     // Returns the aggregated sum of squared deviations.
     double getSumOfSquaredDeviations() {
       return sumOfSquaredDeviations;