Add BucketOptions (#410)

Replaces DistributionValue's bucket_bounds with a new BucketOptions class
following the spec, and fixes #399.

Author: c24t

opencensus/metrics/export/value.py

@@ -11,6 +11,14 @@
 # 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.
+"""
+The classes in this module implement the spec for v1 Metrics as of
+opencensus-proto release v0.0.2. See opencensus-proto for details:
+
+https://github.com/census-instrumentation/opencensus-proto/blob/24333298e36590ea0716598caacc8959fc393c48/src/opencensus/proto/metrics/v1/metrics.proto
+"""  # noqa
+
+from copy import copy
 
 
 class Value(object):
@@ -154,12 +162,48 @@ def exemplar(self):
         return self._exemplar
 
 
-def window(ible, width):
-    win = []
-    for x in ible:
-        win = (win + [x])[-width:]
-        if len(win) == width:
-            yield win
+class Explicit(object):
+    """Set of explicit bucket boundaries.
+
+    Specifies a set of buckets with arbitrary upper-bounds.  This defines
+    size(bounds) + 1 (= N) buckets. The boundaries for bucket index i are:
+
+        - [0, bounds[i]) for i == 0
+        - [bounds[i-1], bounds[i]) for 0 < i < N-1
+        - [bounds[i-1], +infinity) for i == N-1
+    """
+
+    def __init__(self, bounds):
+        if not bounds:
+            raise ValueError("Bounds must not be null or empty")
+        if bounds != sorted(set(bounds)):
+            raise ValueError("Bounds must be strictly increasing")
+        if bounds[0] <= 0:
+            raise ValueError("Bounds must be positive")
+        self._bounds = bounds
+
+    @property
+    def bounds(self):
+        return copy(self._bounds)
+
+
+class BucketOptions(object):
+    """Container for bucket options, including explicit boundaries.
+
+    A Distribution may optionally contain a histogram of the values in the
+    population. The bucket boundaries for that histogram are described by
+    BucketOptions.
+
+    If bucket_options has no type, then there is no histogram associated with
+    the Distribution.
+    """
+
+    def __init__(self, type_=None):
+        self._type = type_
+
+    @property
+    def type_(self):
+        return self._type
 
 
 class ValueDistribution(Value):
@@ -179,17 +223,20 @@ class ValueDistribution(Value):
     :param sum_of_squared_deviation: The sum of squared deviations from the
     mean of the values in the population.
 
-    TODO: update to BucketOptions
-    :type bucket_bounds: list(float)
-    :param bucket_bounds: Bucket boundaries for the histogram of the values in
+    :type bucket_options: :class: 'BucketOptions'
+    :param bucket_options: Bucket boundaries for the histogram of the values in
     the population.
 
     :type buckets: list(:class: 'Bucket')
     :param buckets: Histogram buckets for the given bucket boundaries.
     """
 
-    def __init__(self, count, sum_, sum_of_squared_deviation, bucket_bounds,
-                 buckets):
+    def __init__(self,
+                 count,
+                 sum_,
+                 sum_of_squared_deviation,
+                 bucket_options,
+                 buckets=None):
         if count < 0:
             raise ValueError("count must be non-negative")
         elif count == 0:
@@ -198,22 +245,27 @@ def __init__(self, count, sum_, sum_of_squared_deviation, bucket_bounds,
             if sum_of_squared_deviation != 0:
                 raise ValueError("sum_of_squared_deviation must be 0 if count "
                                  "is 0")
-        if not bucket_bounds:
+        if bucket_options is None:
+            raise ValueError("bucket_options must not be null")
+        if bucket_options.type_ is None:
             if buckets is not None:
-                raise ValueError("buckets must be null if bucket_bounds is "
-                                 "empty")
+                raise ValueError("buckets must be null if the distribution has"
+                                 "no histogram (i.e. bucket_options.type is "
+                                 "null)")
         else:
-            for (lo, hi) in window(bucket_bounds, 2):
-                if lo >= hi:
-                    raise ValueError("bucket_bounds must be monotonically "
-                                     "increasing")
+            if len(buckets) != len(bucket_options.type_.bounds) + 1:
+                # Note that this includes the implicit 0 and positive-infinity
+                # boundaries, so bounds [1, 2] implies three buckets: [[0, 1),
+                # [1, 2), [2, inf)].
+                raise ValueError("There must be one bucket for each pair of "
+                                 "boundaries")
             if count != sum(bucket.count for bucket in buckets):
                 raise ValueError("The distribution count must equal the sum "
                                  "of bucket counts")
         self._count = count
         self._sum = sum_
         self._sum_of_squared_deviation = sum_of_squared_deviation
-        self._bucket_bounds = bucket_bounds
+        self._bucket_options = bucket_options
         self._buckets = buckets
 
     @property
@@ -229,8 +281,8 @@ def sum_of_squared_deviation(self):
         return self._sum_of_squared_deviation
 
     @property
-    def bucket_bounds(self):
-        return self._bucket_bounds
+    def bucket_options(self):
+        return self._bucket_options
 
     @property
     def buckets(self):

tests/unit/metrics/export/test_point.py

@@ -32,7 +32,8 @@ def setUp(self):
             100,
             1000.0,
             10.0,
-            list(range(11)),
+            value_module.BucketOptions(
+                value_module.Explicit(list(range(1, 10)))),
             [value_module.Bucket(10, None) for ii in range(10)],
         )
 

tests/unit/metrics/export/test_value.py

@@ -48,68 +48,79 @@ def test_create_summary_value(self):
 VD_COUNT = 100
 VD_SUM = 1000.0
 VD_SUM_OF_SQUARED_DEVIATION = 10.0
-BUCKET_BOUNDS = list(range(11))
+BOUNDS = list(range(1, 10))
+BUCKET_OPTIONS = value_module.BucketOptions(value_module.Explicit(BOUNDS))
 BUCKETS = [value_module.Bucket(10, None) for ii in range(10)]
 
 
 class TestValueDistribution(unittest.TestCase):
     def test_init(self):
         distribution = value_module.ValueDistribution(
-            VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, BUCKET_BOUNDS,
+            VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, BUCKET_OPTIONS,
             BUCKETS)
         self.assertEqual(distribution.count, VD_COUNT)
         self.assertEqual(distribution.sum, VD_SUM)
         self.assertEqual(distribution.sum_of_squared_deviation,
                          VD_SUM_OF_SQUARED_DEVIATION)
-        self.assertEqual(distribution.bucket_bounds, BUCKET_BOUNDS)
+        self.assertEqual(distribution.bucket_options, BUCKET_OPTIONS)
+        self.assertEqual(distribution.bucket_options.type_.bounds, BOUNDS)
         self.assertEqual(distribution.buckets, BUCKETS)
 
     def test_init_no_histogram(self):
         distribution = value_module.ValueDistribution(
-            VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, [], None)
+            VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION,
+            value_module.BucketOptions(), None)
         self.assertEqual(distribution.count, VD_COUNT)
         self.assertEqual(distribution.sum, VD_SUM)
         self.assertEqual(distribution.sum_of_squared_deviation,
                          VD_SUM_OF_SQUARED_DEVIATION)
-        self.assertEqual(distribution.bucket_bounds, [])
+        self.assertIsNone(distribution.bucket_options.type_)
         self.assertEqual(distribution.buckets, None)
 
     def test_init_bad_args(self):
+        value_module.ValueDistribution(VD_COUNT, VD_SUM,
+                                       VD_SUM_OF_SQUARED_DEVIATION,
+                                       BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
             value_module.ValueDistribution(-1, VD_SUM,
                                            VD_SUM_OF_SQUARED_DEVIATION,
-                                           BUCKET_BOUNDS, BUCKETS)
+                                           BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
-            value_module.ValueDistribution(
-                0, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, BUCKET_BOUNDS, BUCKETS)
+            value_module.ValueDistribution(0, VD_SUM,
+                                           VD_SUM_OF_SQUARED_DEVIATION,
+                                           BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
             value_module.ValueDistribution(0, 0, VD_SUM_OF_SQUARED_DEVIATION,
-                                           BUCKET_BOUNDS, BUCKETS)
+                                           BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
             value_module.ValueDistribution(
                 VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, None, BUCKETS)
 
         with self.assertRaises(ValueError):
-            value_module.ValueDistribution(
-                VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, [], BUCKETS)
+            value_module.ValueDistribution(0, 0, 0, BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
-            value_module.ValueDistribution(0, 0, 0, BUCKET_BOUNDS, BUCKETS)
+            value_module.ValueDistribution(VD_COUNT - 1, VD_SUM,
+                                           VD_SUM_OF_SQUARED_DEVIATION,
+                                           BUCKET_OPTIONS, BUCKETS)
 
         with self.assertRaises(ValueError):
-            value_module.ValueDistribution(
-                VD_COUNT, VD_SUM, VD_SUM_OF_SQUARED_DEVIATION, [1, 1],
-                [value_module.Bucket(1, None),
-                 value_module.Bucket(1, None)])
+            value_module.ValueDistribution(VD_COUNT, VD_SUM,
+                                           VD_SUM_OF_SQUARED_DEVIATION,
+                                           value_module.BucketOptions(),
+                                           BUCKETS)
 
         with self.assertRaises(ValueError):
-            value_module.ValueDistribution(VD_COUNT - 1, VD_SUM,
+            value_module.ValueDistribution(VD_COUNT, VD_SUM,
                                            VD_SUM_OF_SQUARED_DEVIATION,
-                                           BUCKET_BOUNDS, BUCKETS)
+                                           BUCKET_OPTIONS, BUCKETS[:-1])
+
+    def test_init_empty_buckets_null_bucket_options(self):
+        pass
 
 
 EX_VALUE = 1.0
@@ -135,3 +146,24 @@ def test_init(self):
         bucket = value_module.Bucket(1, self.exemplar)
         self.assertEqual(bucket.count, 1)
         self.assertEqual(bucket.exemplar, self.exemplar)
+
+
+class TestExplicit(unittest.TestCase):
+    def test_init(self):
+        bounds = [1, 2]
+        explicit = value_module.Explicit(bounds)
+        self.assertEqual(explicit.bounds, bounds)
+
+    def test_bad_init(self):
+        with self.assertRaises(ValueError):
+            value_module.Explicit(None)
+        with self.assertRaises(ValueError):
+            value_module.Explicit([])
+        with self.assertRaises(ValueError):
+            value_module.Explicit([-1, 1, 2])
+        with self.assertRaises(ValueError):
+            value_module.Explicit([0, 1, 2])
+        with self.assertRaises(ValueError):
+            value_module.Explicit([1, 1])
+        with self.assertRaises(ValueError):
+            value_module.Explicit([2, 1])