Adds 0 count for underflow/first bucket to [StackdriverStatsExporter] (#181)

* Drops 0 bound, moves [BucketBoundaries] logic to separate class
* Fixes boundaries for [prometeus]
* Removes redundant [BucketBondaries] tests from [opencensus-exporter-zpages]
* Adds 0 count for underflow/first bucket to [StackdriverStatsExporter]

Author: isaikevych

packages/opencensus-core/src/index.ts

@@ -46,6 +46,7 @@ export * from './exporters/console-exporter';
 export * from './stats/stats';
 export * from './stats/view';
 export * from './stats/recorder';
+export * from './stats/bucket-boundaries';
 
 // interfaces
 export * from './stats/types';

packages/opencensus-core/src/stats/bucket-boundaries.ts

@@ -0,0 +1,98 @@
+/**
+ * Copyright 2018, OpenCensus Authors
+ *
+ * Licensed 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.
+ */
+
+import * as defaultLogger from '../common/console-logger';
+import * as loggerTypes from '../common/types';
+import {Bucket} from './types';
+
+export class BucketBoundaries {
+  readonly buckets: Bucket[];
+  readonly bucketCounts: number[];
+  /** An object to log information to */
+  private logger: loggerTypes.Logger;
+
+  constructor(boundaries: number[], logger = defaultLogger) {
+    this.logger = logger.logger();
+    this.buckets = this.dropNegativeBucketBounds(boundaries);
+    this.bucketCounts = this.getBucketCounts(this.buckets);
+  }
+
+  /**
+   * Gets bucket boundaries
+   */
+  getBoundaries(): Bucket[] {
+    return this.buckets;
+  }
+
+  /**
+   * Gets initial bucket counts
+   */
+  getCounts(): number[] {
+    return this.bucketCounts;
+  }
+
+  /**
+   * Drops negative (BucketBounds) are currently not supported by
+   * any of the backends that OC supports
+   * @param bucketBoundaries a list with the bucket boundaries
+   */
+  private dropNegativeBucketBounds(bucketBoundaries: number[]): Bucket[] {
+    let negative = 0;
+    if (!bucketBoundaries) return [];
+    const result = bucketBoundaries.reduce((accumulator, boundary, index) => {
+      if (boundary > 0) {
+        const nextBoundary = bucketBoundaries[index + 1];
+        this.validateBoundary(boundary, nextBoundary);
+        accumulator.push(boundary);
+      } else {
+        negative++;
+      }
+      return accumulator;
+    }, []);
+    if (negative) {
+      this.logger.warn(`Dropping ${
+          negative} negative bucket boundaries, the values must be strictly > 0.`);
+    }
+    return result;
+  }
+
+  /**
+   * Gets initial list of bucket counters
+   * @param buckets Bucket boundaries
+   */
+  private getBucketCounts(buckets: Bucket[]): number[] {
+    if (!buckets) return [];
+    const bucketsCount = new Array(buckets.length + 1);
+    bucketsCount.fill(0);
+    return bucketsCount;
+  }
+
+  /**
+   * Checks boundaries order and duplicates
+   * @param current Boundary
+   * @param next Next boundary
+   */
+  private validateBoundary(current: number, next: number) {
+    if (next) {
+      if (current > next) {
+        this.logger.error('Bucket boundaries not sorted.');
+      }
+      if (current === next) {
+        this.logger.error('Bucket boundaries not unique.');
+      }
+    }
+  }
+}

packages/opencensus-core/src/stats/recorder.ts

@@ -44,10 +44,14 @@ export class Recorder {
       distributionData: DistributionData, value: number): DistributionData {
     distributionData.count += 1;
 
-    const inletBucket = distributionData.buckets.find((bucket) => {
-      return bucket.lowBoundary <= value && value < bucket.highBoundary;
-    });
-    inletBucket.count += 1;
+    let bucketIndex =
+        distributionData.buckets.findIndex(bucket => bucket > value);
+
+    if (bucketIndex < 0) {
+      bucketIndex = distributionData.buckets.length;
+    }
+
+    distributionData.bucketCounts[bucketIndex] += 1;
 
     if (value > distributionData.max) {
       distributionData.max = value;

packages/opencensus-core/src/stats/types.ts

@@ -192,16 +192,9 @@ export interface DistributionData extends AggregationMetadata {
   sumSquaredDeviations: number;
   /** Bucket distribution of the histogram */
   buckets: Bucket[];
+  /** Buckets count */
+  bucketCounts: number[];
 }
 
-/** A simple histogram bucket interface. */
-export interface Bucket {
-  /** Number of occurrences in the domain */
-  count: number;
-  /** The maximum possible value for a data point to fall in this bucket */
-  readonly highBoundary: number;
-  /** The minimum possible value for a data point to fall in this bucket */
-  readonly lowBoundary: number;
-}
-
+export type Bucket = number;
 export type AggregationData = SumData|CountData|LastValueData|DistributionData;

packages/opencensus-core/src/stats/view.ts

@@ -17,8 +17,9 @@
 import * as defaultLogger from '../common/console-logger';
 import * as loggerTypes from '../common/types';
 
+import {BucketBoundaries} from './bucket-boundaries';
 import {Recorder} from './recorder';
-import {AggregationData, AggregationMetadata, AggregationType, Bucket, CountData, DistributionData, LastValueData, Measure, Measurement, MeasureType, SumData, Tags, View} from './types';
+import {AggregationData, AggregationMetadata, AggregationType, CountData, DistributionData, LastValueData, Measure, Measurement, MeasureType, SumData, Tags, View} from './types';
 
 const RECORD_SEPARATOR = String.fromCharCode(30);
 const UNIT_SEPARATOR = String.fromCharCode(31);
@@ -55,7 +56,7 @@ export class BaseView implements View {
   /** The start time for this view */
   readonly startTime: number;
   /** The bucket boundaries in a Distribution Aggregation */
-  private bucketBoundaries?: number[];
+  private bucketBoundaries: BucketBoundaries;
   /**
    * The end time for this view - represents the last time a value was recorded
    */
@@ -91,7 +92,7 @@ export class BaseView implements View {
     this.columns = tagsKeys;
     this.aggregation = aggregation;
     this.startTime = Date.now();
-    this.bucketBoundaries = bucketBoundaries;
+    this.bucketBoundaries = new BucketBoundaries(bucketBoundaries);
   }
 
   /** Gets the view's tag keys */
@@ -156,7 +157,7 @@ export class BaseView implements View {
    */
   private createAggregationData(tags: Tags): AggregationData {
     const aggregationMetadata = {tags, timestamp: Date.now()};
-
+    const {buckets, bucketCounts} = this.bucketBoundaries;
     switch (this.aggregation) {
       case AggregationType.DISTRIBUTION:
         return {
@@ -170,7 +171,8 @@ export class BaseView implements View {
           mean: null as number,
           stdDeviation: null as number,
           sumSquaredDeviations: null as number,
-          buckets: this.createBuckets(this.bucketBoundaries)
+          buckets,
+          bucketCounts
         };
       case AggregationType.SUM:
         return {...aggregationMetadata, type: AggregationType.SUM, value: 0};
@@ -185,64 +187,6 @@ export class BaseView implements View {
     }
   }
 
-  /**
-   * Creates empty Buckets, given a list of bucket boundaries.
-   * @param bucketBoundaries a list with the bucket boundaries
-   */
-  private createBuckets(bucketBoundaries: number[]): Bucket[] {
-    let negative = 0;
-    const result = bucketBoundaries.reduce((accumulator, boundary, index) => {
-      if (boundary >= 0) {
-        const nextBoundary = bucketBoundaries[index + 1];
-        this.validateBoundary(boundary, nextBoundary);
-        const len = bucketBoundaries.length - negative;
-        const position = index - negative;
-        const bucket = this.createBucket(boundary, nextBoundary, position, len);
-        accumulator.push(bucket);
-      } else {
-        negative++;
-      }
-      return accumulator;
-    }, []);
-    if (negative) {
-      this.logger.warn(`Dropping ${
-          negative} negative bucket boundaries, the values must be strictly > 0.`);
-    }
-    return result;
-  }
-
-  /**
-   * Checks boundaries order and duplicates
-   * @param current Boundary
-   * @param next Next boundary
-   */
-  private validateBoundary(current: number, next: number) {
-    if (next) {
-      if (current > next) {
-        this.logger.error('Bucket boundaries not sorted.');
-      }
-      if (current === next) {
-        this.logger.error('Bucket boundaries not unique.');
-      }
-    }
-  }
-
-  /**
-   * Creates empty bucket boundary.
-   * @param current Current boundary
-   * @param next Next boundary
-   * @param position Index of boundary
-   * @param max Maximum length of boundaries
-   */
-  private createBucket(
-      current: number, next: number, position: number, max: number): Bucket {
-    return {
-      count: 0,
-      lowBoundary: position ? current : -Infinity,
-      highBoundary: (position === max - 1) ? Infinity : next
-    };
-  }
-
   /**
    * Returns a snapshot of an AggregationData for that tags/labels values.
    * @param tags The desired data's tags

packages/opencensus-core/test/test-bucket-boundaries.ts

@@ -0,0 +1,40 @@
+/**
+ * Copyright 2018, OpenCensus Authors
+ *
+ * Licensed 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.
+ */
+
+import * as assert from 'assert';
+import {BucketBoundaries} from '../src';
+
+describe('BucketBoundaries', () => {
+  it('should return boundaries', () => {
+    const buckets = new BucketBoundaries([1, 2, 3]);
+    assert.deepStrictEqual(buckets.getBoundaries(), [1, 2, 3]);
+  });
+
+  it('should has bucket counts', () => {
+    const buckets = new BucketBoundaries([1, 2, 3]);
+    assert.deepStrictEqual(buckets.getCounts(), [0, 0, 0, 0]);
+  });
+
+  describe('Drop negative and 0 boundaries', () => {
+    const buckets = new BucketBoundaries([-Infinity, -3, -2, -1, 0, 1, 2, 3]);
+    it('should drop negative and 0 boundaries', () => {
+      assert.deepStrictEqual(buckets.getBoundaries(), [1, 2, 3]);
+    });
+    it('should has bucket counts', () => {
+      assert.deepStrictEqual(buckets.getCounts(), [0, 0, 0, 0]);
+    });
+  });
+});

packages/opencensus-core/test/test-recorder.ts

@@ -42,15 +42,6 @@ function assertDistributionData(
   assert.strictEqual(distributionData.count, values.length);
   assert.strictEqual(distributionData.sum, valuesSum);
 
-  for (const bucket of distributionData.buckets) {
-    const expectedBucketCount = values
-                                    .filter(
-                                        value => bucket.lowBoundary <= value &&
-                                            value < bucket.highBoundary)
-                                    .length;
-    assert.strictEqual(bucket.count, expectedBucketCount);
-  }
-
   const expectedMean = valuesSum / values.length;
   assert.ok(isAlmostEqual(distributionData.mean, expectedMean, EPSILON));
 
@@ -173,13 +164,8 @@ describe('Recorder', () => {
                    mean: 0,
                    stdDeviation: 0,
                    sumSquaredDeviations: 0,
-                   buckets: [
-                     {highBoundary: 0, lowBoundary: -Infinity, count: 0},
-                     {highBoundary: 2, lowBoundary: 0, count: 0},
-                     {highBoundary: 4, lowBoundary: 2, count: 0},
-                     {highBoundary: 6, lowBoundary: 4, count: 0},
-                     {highBoundary: Infinity, lowBoundary: 6, count: 0}
-                   ]
+                   buckets: [2, 4, 6],
+                   bucketCounts: [0, 0, 0, 0]
                  };
                  const sentValues = [];
                  for (const value of testCase.values) {

packages/opencensus-core/test/test-view.ts

@@ -48,15 +48,6 @@ function assertDistributionData(
   assert.strictEqual(distributionData.count, values.length);
   assert.strictEqual(distributionData.sum, valuesSum);
 
-  for (const bucket of distributionData.buckets) {
-    const expectedBucketCount = values
-                                    .filter(
-                                        value => bucket.lowBoundary <= value &&
-                                            value < bucket.highBoundary)
-                                    .length;
-    assert.strictEqual(bucket.count, expectedBucketCount);
-  }
-
   const expectedMean = valuesSum / values.length;
   assert.ok(isAlmostEqual(distributionData.mean, expectedMean, EPSILON));
 
@@ -125,7 +116,7 @@ describe('BaseView', () => {
 
   describe('recordMeasurement()', () => {
     const measurementValues = [1.1, 2.3, 3.2, 4.3, 5.2];
-    const bucketBoundaries = [0, 2, 4, 6];
+    const bucketBoundaries = [2, 4, 6];
     const emptyAggregation = {};
     const tags: Tags = {testKey1: 'testValue', testKey2: 'testValue'};
 
@@ -161,16 +152,8 @@ describe('BaseView', () => {
         view.recordMeasurement(measurement);
       }
       const data = view.getSnapshot(tags) as DistributionData;
-      const expectedBuckets = [
-        {count: 1, lowBoundary: -Infinity, highBoundary: 2},
-        {count: 2, lowBoundary: 2, highBoundary: 4},
-        {count: 2, lowBoundary: 4, highBoundary: 6},
-        {count: 0, lowBoundary: 6, highBoundary: Infinity}
-      ];
-      assert.equal(data.buckets.length, expectedBuckets.length);
-      expectedBuckets.forEach((bucket, index) => {
-        assert.deepStrictEqual(data.buckets[index], bucket);
-      });
+      assert.deepStrictEqual(data.buckets, [2, 4, 6]);
+      assert.deepStrictEqual(data.bucketCounts, [1, 2, 2, 0]);
     });
 
     const view = new BaseView(

packages/opencensus-exporter-prometheus/src/prometheus-stats.ts

@@ -189,7 +189,7 @@ export class PrometheusStatsExporter implements StatsEventListener {
    */
   private getBoundaries(view: View, tags: Tags): number[] {
     const data = view.getSnapshot(tags) as DistributionData;
-    return data.buckets.map(b => b.lowBoundary).filter(b => b !== -Infinity);
+    return data.buckets;
   }
 
   /**