merge createMeasure PR and fix the tests

open-telemetry · Feb 10, 2020 · cf49191 · cf49191
1 parent 89c0b68
commit cf49191
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 23 deletions.
diff --git a/packages/opentelemetry-metrics/src/BoundInstrument.ts b/packages/opentelemetry-metrics/src/BoundInstrument.ts
@@ -133,15 +133,19 @@ export class BoundGauge extends BaseBoundInstrument
  */
 export class BoundMeasure extends BaseBoundInstrument
   implements types.BoundMeasure {
+  private readonly _absolute: boolean;
+
   constructor(
     labelSet: types.LabelSet,
     disabled: boolean,
+    monotonic: boolean,
     absolute: boolean,
     valueType: types.ValueType,
     logger: types.Logger,
     aggregator: Aggregator
   ) {
-    super(labelSet, logger, absolute, disabled, valueType, aggregator);
+    super(labelSet, logger, monotonic, disabled, valueType, aggregator);
+    this._absolute = absolute;
   }
 
   record(

diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts
@@ -66,8 +66,7 @@ export class Meter implements types.Meter {
       return types.NOOP_MEASURE_METRIC;
     }
     const opt: MetricOptions = {
-      // Measures are defined as absolute by default
-      absolute: true,
+      absolute: true, // Measures are defined as absolute by default
       monotonic: false, // not applicable to measure, set to false
       logger: this._logger,
       ...DEFAULT_METRIC_OPTIONS,
@@ -97,8 +96,7 @@ export class Meter implements types.Meter {
       return types.NOOP_COUNTER_METRIC;
     }
     const opt: MetricOptions = {
-      // Counters are defined as monotonic by default
-      monotonic: true,
+      monotonic: true, // Counters are defined as monotonic by default
       absolute: false, // not applicable to counter, set to false
       logger: this._logger,
       ...DEFAULT_METRIC_OPTIONS,
@@ -128,8 +126,7 @@ export class Meter implements types.Meter {
       return types.NOOP_GAUGE_METRIC;
     }
     const opt: MetricOptions = {
-      // Gauges are defined as non-monotonic by default
-      monotonic: false,
+      monotonic: false, // Gauges are defined as non-monotonic by default
       absolute: false, // not applicable for gauges, set to false
       logger: this._logger,
       ...DEFAULT_METRIC_OPTIONS,

diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts
@@ -186,18 +186,15 @@ export class MeasureMetric extends Metric<BoundMeasure>
     options: MetricOptions,
     private readonly _batcher: Batcher
   ) {
-    super(
-      name,
-      options,
-      MetricKind.MEASURE
-    );
+    super(name, options, MetricKind.MEASURE);
 
     this._absolute = options.absolute !== undefined ? options.absolute : true; // Absolute default is true
   }
   protected _makeInstrument(labelSet: types.LabelSet): BoundMeasure {
     return new BoundMeasure(
       labelSet,
       this._disabled,
+      this._monotonic,
       this._absolute,
       this._valueType,
       this._logger,

diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts
@@ -25,19 +25,14 @@ import {
   Sum,
   MeterProvider,
   MeasureMetric,
-  Distribution
+  Distribution,
 } from '../src';
 import * as types from '@opentelemetry/api';
 import { LabelSet } from '../src/LabelSet';
-import {
-  NoopLogger,
-  hrTime,
-  hrTimeToMilliseconds,
-} from '@opentelemetry/core';
+import { NoopLogger, hrTime, hrTimeToMilliseconds } from '@opentelemetry/core';
 import {
   CounterSumAggregator,
   GaugeAggregator,
-  MeasureExactAggregator,
 } from '../src/export/Aggregator';
 import { ValueType } from '@opentelemetry/api';
 
@@ -497,8 +492,12 @@ describe('Meter', () => {
 
         meter.collect();
         const [record1] = meter.getBatcher().checkPointSet();
-
-        assert.strictEqual((record1.aggregator.value() as Distribution), 10);
+        assert.deepStrictEqual(record1.aggregator.value() as Distribution, {
+          count: 0,
+          max: -Infinity,
+          min: Infinity,
+          sum: 0,
+        });
       });
 
       it('should accept negative (and positive) values when monotonic is set to false', () => {
@@ -511,8 +510,14 @@ describe('Meter', () => {
         boundMeasure1.record(10);
         const boundMeasure2 = measure.bind(labelSet);
         boundMeasure2.record(100);
-        // @todo: re-add once record is implemented
-        // assert.strictEqual(boundMeasure1['_data'], 100);
+        meter.collect();
+        const [record1] = meter.getBatcher().checkPointSet();
+        assert.deepStrictEqual(record1.aggregator.value() as Distribution, {
+          count: 2,
+          max: 100,
+          min: 10,
+          sum: 110,
+        });
         assert.strictEqual(boundMeasure1, boundMeasure2);
       });
     });