From 74f91ae0268024a9766853730f349923844298d2 Mon Sep 17 00:00:00 2001 From: Jigar Joshi Date: Fri, 1 Mar 2019 09:00:45 -0800 Subject: [PATCH] Added bucket creation method with custom buckets in ValueBuckets and DurationBuckets (#35) * Added custom buckets for Value and Duration --- .../com/uber/m3/tally/DurationBuckets.java | 19 ++++++++ .../java/com/uber/m3/tally/ValueBuckets.java | 23 +++++++++ .../uber/m3/tally/DurationBucketsTest.java | 48 +++++++++++++++++-- .../com/uber/m3/tally/ValueBucketsTest.java | 29 ++++++++++- 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/uber/m3/tally/DurationBuckets.java b/core/src/main/java/com/uber/m3/tally/DurationBuckets.java index 8068934..91a9d77 100644 --- a/core/src/main/java/com/uber/m3/tally/DurationBuckets.java +++ b/core/src/main/java/com/uber/m3/tally/DurationBuckets.java @@ -103,4 +103,23 @@ public static DurationBuckets exponential(Duration start, double factor, int num return new DurationBuckets(buckets); } + + /** + * Allows to create bucket with finer bucket creation control + * + * @param sortedDurations sorted values (ascending) of upper bound of the buckets + * @return {@link DurationBuckets} of the specified parameters + */ + public static DurationBuckets custom(Duration... sortedDurations) { + if (sortedDurations == null || sortedDurations.length == 0) { + throw new IllegalArgumentException("at least one upper bucket value has to be specified"); + } + + for (int i = 0; i < sortedDurations.length - 1; i++) { + if (sortedDurations[i].compareTo(sortedDurations[i + 1]) >= 0) { + throw new IllegalArgumentException("bucketUpperMillis has to be sorted in ascending order with unique values"); + } + } + return new DurationBuckets(sortedDurations); + } } diff --git a/core/src/main/java/com/uber/m3/tally/ValueBuckets.java b/core/src/main/java/com/uber/m3/tally/ValueBuckets.java index 23acf9d..387a56a 100644 --- a/core/src/main/java/com/uber/m3/tally/ValueBuckets.java +++ b/core/src/main/java/com/uber/m3/tally/ValueBuckets.java @@ -103,4 +103,27 @@ public static ValueBuckets exponential(double start, double factor, int numBucke return new ValueBuckets(buckets); } + + /** + * Helper function to create {@link ValueBuckets} with custom buckets. + * + * @param sortedBucketUpperValues sorted (ascending order) values of bucket's upper bound + * @return {@link ValueBuckets} of the specified paramters + */ + public static ValueBuckets custom(double... sortedBucketUpperValues) { + if (sortedBucketUpperValues == null || sortedBucketUpperValues.length == 0) { + throw new IllegalArgumentException("Must have a positive number of buckets"); + } + for (int i = 0; i < sortedBucketUpperValues.length - 1; i++) { + if (sortedBucketUpperValues[i] >= sortedBucketUpperValues[i + 1]) { + throw new IllegalArgumentException("bucketUpperValues has to be sorted and unique values in ascending order"); + } + } + + Double[] buckets = new Double[sortedBucketUpperValues.length]; + for (int i = 0; i < sortedBucketUpperValues.length; i++) { + buckets[i] = sortedBucketUpperValues[i]; + } + return new ValueBuckets(buckets); + } } diff --git a/core/src/test/java/com/uber/m3/tally/DurationBucketsTest.java b/core/src/test/java/com/uber/m3/tally/DurationBucketsTest.java index 797a5d5..1973063 100644 --- a/core/src/test/java/com/uber/m3/tally/DurationBucketsTest.java +++ b/core/src/test/java/com/uber/m3/tally/DurationBucketsTest.java @@ -20,18 +20,21 @@ package com.uber.m3.tally; -import com.uber.m3.util.Duration; -import org.junit.Test; - import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import org.hamcrest.CoreMatchers; +import org.junit.Test; + +import com.uber.m3.util.Duration; + import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThat; public class DurationBucketsTest { @Test @@ -203,6 +206,45 @@ public void exponential() { )); } + @Test + public void custom() { + DurationBuckets expectedBuckets = new DurationBuckets( + new Duration[] { + Duration.ofMillis(1), + Duration.ofMillis(2), + Duration.ofMillis(3), + Duration.ofMillis(5), + Duration.ofMillis(7), + Duration.ofMillis(10), + } + ); + + assertThat("custom buckets are created as per our expectations", + DurationBuckets.custom( + Duration.ofMillis(1), + Duration.ofMillis(2), + Duration.ofMillis(3), + Duration.ofMillis(5), + Duration.ofMillis(7), + Duration.ofMillis(10) + ), + CoreMatchers.equalTo(expectedBuckets)); + } + + @Test(expected = IllegalArgumentException.class) + public void customFailWithEmptyBuckets() { + DurationBuckets.custom(); + } + + @Test(expected = IllegalArgumentException.class) + public void customFailWithUnsortedBuckets() { + DurationBuckets.custom( + Duration.ofMillis(1), + Duration.ofMillis(3), + Duration.ofMillis(2) + ); + } + @Test public void testToString() { DurationBuckets buckets = DurationBuckets.linear(Duration.ZERO, Duration.ofMillis(10), 6); diff --git a/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java b/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java index 5dc7a6a..27c9f7b 100644 --- a/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java +++ b/core/src/test/java/com/uber/m3/tally/ValueBucketsTest.java @@ -20,13 +20,16 @@ package com.uber.m3.tally; -import com.uber.m3.util.Duration; +import org.hamcrest.CoreMatchers; import org.junit.Test; +import com.uber.m3.util.Duration; + import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThat; public class ValueBucketsTest { @Test @@ -110,6 +113,30 @@ public void exponential() { )); } + @Test + public void custom() { + ValueBuckets expectedBuckets = new ValueBuckets(new Double[] { + 1d, + 2d, + 3d, + 5d, + 7d + }); + assertThat("Buckets are created as per our expectations", + ValueBuckets.custom(1D, 2D, 3D, 5D, 7D), + CoreMatchers.equalTo(expectedBuckets)); + } + + @Test(expected = IllegalArgumentException.class) + public void customFailOnEmptyBucket() { + ValueBuckets.custom(); + } + + @Test(expected = IllegalArgumentException.class) + public void customFailOnUnsortedBuckets() { + ValueBuckets.custom(1, 3, 2); + } + @Test public void testToString() { ValueBuckets buckets = ValueBuckets.linear(0, 10, 6);