chore: updating aggregator MinMaxLastSumCount and use it for value ob…

…server and value recorder (#1276)

* chore: updating aggregator MinMaxLastSumCount and use it for value observer and value recorder

* chore: fix after merge

packages/opentelemetry-exporter-collector/test/helper.ts

@@ -26,7 +26,7 @@ import {
   MetricRecord,
   MetricKind,
   SumAggregator,
-  LastValueAggregator,
+  MinMaxLastSumCountAggregator,
 } from '@opentelemetry/metrics';
 
 if (typeof Buffer === 'undefined') {
@@ -85,7 +85,7 @@ export const mockObserver: MetricRecord = {
     valueType: ValueType.DOUBLE,
   },
   labels: {},
-  aggregator: new LastValueAggregator(),
+  aggregator: new MinMaxLastSumCountAggregator(),
   resource: new Resource({
     service: 'ui',
     version: 1,

packages/opentelemetry-exporter-prometheus/src/prometheus.ts

@@ -161,10 +161,10 @@ export class PrometheusExporter implements MetricExporter {
           (point.value as Histogram).sum,
           hrTimeToMilliseconds(point.timestamp)
         );
-      } else if (typeof (point.value as Distribution).max === 'number') {
+      } else if (typeof (point.value as Distribution).last === 'number') {
         metric.set(
           labels,
-          (point.value as Distribution).sum,
+          (point.value as Distribution).last,
           hrTimeToMilliseconds(point.timestamp)
         );
       }

packages/opentelemetry-metrics/src/export/Batcher.ts

@@ -57,9 +57,9 @@ export class UngroupedBatcher extends Batcher {
         return new aggregators.SumAggregator();
       case MetricKind.VALUE_RECORDER:
       case MetricKind.VALUE_OBSERVER:
-        return new aggregators.LastValueAggregator();
+        return new aggregators.MinMaxLastSumCountAggregator();
       default:
-        return new aggregators.MinMaxSumCountAggregator();
+        return new aggregators.MinMaxLastSumCountAggregator();
     }
   }
 

packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts → packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts

@@ -19,15 +19,18 @@ import { HrTime } from '@opentelemetry/api';
 import { hrTime } from '@opentelemetry/core';
 import { Distribution } from '../types';
 
-/** Basic aggregator keeping all raw values (events, sum, max and min). */
-export class MinMaxSumCountAggregator implements Aggregator {
+/**
+ * Basic aggregator keeping all raw values (events, sum, max, last and min).
+ */
+export class MinMaxLastSumCountAggregator implements Aggregator {
   private _distribution: Distribution;
   private _lastUpdateTime: HrTime = [0, 0];
 
   constructor() {
     this._distribution = {
       min: Infinity,
       max: -Infinity,
+      last: 0,
       sum: 0,
       count: 0,
     };
@@ -36,6 +39,7 @@ export class MinMaxSumCountAggregator implements Aggregator {
   update(value: number): void {
     this._distribution.count++;
     this._distribution.sum += value;
+    this._distribution.last = value;
     this._distribution.min = Math.min(this._distribution.min, value);
     this._distribution.max = Math.max(this._distribution.max, value);
     this._lastUpdateTime = hrTime();

packages/opentelemetry-metrics/src/export/aggregators/index.ts

@@ -15,6 +15,5 @@
  */
 
 export * from './histogram';
-export * from './MinMaxSumCount';
-export * from './LastValue';
+export * from './MinMaxLastSumCount';
 export * from './Sum';

packages/opentelemetry-metrics/src/export/types.ts

@@ -37,6 +37,7 @@ export type LastValue = number;
 export interface Distribution {
   min: number;
   max: number;
+  last: number;
   count: number;
   sum: number;
 }

packages/opentelemetry-metrics/test/Meter.test.ts

@@ -27,8 +27,9 @@ import {
   MetricRecord,
   Aggregator,
   MetricDescriptor,
-  LastValueAggregator,
   UpDownCounterMetric,
+  Distribution,
+  MinMaxLastSumCountAggregator,
 } from '../src';
 import * as api from '@opentelemetry/api';
 import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core';
@@ -562,7 +563,16 @@ describe('Meter', () => {
 
         await meter.collect();
         const [record1] = meter.getBatcher().checkPointSet();
-        assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0);
+        assert.deepStrictEqual(
+          record1.aggregator.toPoint().value as Distribution,
+          {
+            count: 0,
+            last: 0,
+            max: -Infinity,
+            min: Infinity,
+            sum: 0,
+          }
+        );
       });
 
       it('should not set the instrument data when disabled', async () => {
@@ -574,7 +584,16 @@ describe('Meter', () => {
 
         await meter.collect();
         const [record1] = meter.getBatcher().checkPointSet();
-        assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0);
+        assert.deepStrictEqual(
+          record1.aggregator.toPoint().value as Distribution,
+          {
+            count: 0,
+            last: 0,
+            max: -Infinity,
+            min: Infinity,
+            sum: 0,
+          }
+        );
       });
 
       it(
@@ -591,8 +610,14 @@ describe('Meter', () => {
           await meter.collect();
           const [record1] = meter.getBatcher().checkPointSet();
           assert.deepStrictEqual(
-            record1.aggregator.toPoint().value as number,
-            50
+            record1.aggregator.toPoint().value as Distribution,
+            {
+              count: 2,
+              last: 50,
+              max: 50,
+              min: -10,
+              sum: 40,
+            }
           );
           assert.ok(
             hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) >
@@ -612,8 +637,14 @@ describe('Meter', () => {
         await meter.collect();
         const [record1] = meter.getBatcher().checkPointSet();
         assert.deepStrictEqual(
-          record1.aggregator.toPoint().value as number,
-          100
+          record1.aggregator.toPoint().value as Distribution,
+          {
+            count: 2,
+            last: 100,
+            max: 100,
+            min: 10,
+            sum: 110,
+          }
         );
         assert.strictEqual(boundValueRecorder1, boundValueRecorder2);
       });
@@ -809,10 +840,34 @@ describe('Meter', () => {
       assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
       assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2');
 
-      ensureMetric(metric1, 'cpu_temp_per_app', 67);
-      ensureMetric(metric2, 'cpu_temp_per_app', 69);
-      ensureMetric(metric3, 'cpu_temp_per_app', 67);
-      ensureMetric(metric4, 'cpu_temp_per_app', 69);
+      ensureMetric(metric1, 'cpu_temp_per_app', {
+        count: 1,
+        last: 67,
+        max: 67,
+        min: 67,
+        sum: 67,
+      });
+      ensureMetric(metric2, 'cpu_temp_per_app', {
+        count: 1,
+        last: 69,
+        max: 69,
+        min: 69,
+        sum: 69,
+      });
+      ensureMetric(metric3, 'cpu_temp_per_app', {
+        count: 1,
+        last: 67,
+        max: 67,
+        min: 67,
+        sum: 67,
+      });
+      ensureMetric(metric4, 'cpu_temp_per_app', {
+        count: 1,
+        last: 69,
+        max: 69,
+        min: 69,
+        sum: 69,
+      });
 
       const metric5 = cpuUsageMetricRecords[0];
       const metric6 = cpuUsageMetricRecords[1];
@@ -823,10 +878,34 @@ describe('Meter', () => {
       assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
       assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2');
 
-      ensureMetric(metric5, 'cpu_usage_per_app', 2.1);
-      ensureMetric(metric6, 'cpu_usage_per_app', 3.1);
-      ensureMetric(metric7, 'cpu_usage_per_app', 1.2);
-      ensureMetric(metric8, 'cpu_usage_per_app', 4.5);
+      ensureMetric(metric5, 'cpu_usage_per_app', {
+        count: 1,
+        last: 2.1,
+        max: 2.1,
+        min: 2.1,
+        sum: 2.1,
+      });
+      ensureMetric(metric6, 'cpu_usage_per_app', {
+        count: 1,
+        last: 3.1,
+        max: 3.1,
+        min: 3.1,
+        sum: 3.1,
+      });
+      ensureMetric(metric7, 'cpu_usage_per_app', {
+        count: 1,
+        last: 1.2,
+        max: 1.2,
+        min: 1.2,
+        sum: 1.2,
+      });
+      ensureMetric(metric8, 'cpu_usage_per_app', {
+        count: 1,
+        last: 4.5,
+        max: 4.5,
+        min: 4.5,
+        sum: 4.5,
+      });
     });
 
     it('should not observe values when timeout', done => {
@@ -856,9 +935,15 @@ describe('Meter', () => {
             const value = cpuUsageMetric
               .bind({ foo: 'bar' })
               .getAggregator()
-              .toPoint().value as number;
-
-            assert.strictEqual(value, 0);
+              .toPoint().value as Distribution;
+
+            assert.deepStrictEqual(value, {
+              count: 0,
+              last: 0,
+              max: -Infinity,
+              min: Infinity,
+              sum: 0,
+            });
             assert.strictEqual(cpuUsageMetricRecords.length, 0);
             done();
           });
@@ -961,13 +1046,17 @@ class CustomBatcher extends Batcher {
   }
 }
 
-function ensureMetric(metric: MetricRecord, name?: string, value?: number) {
-  assert.ok(metric.aggregator instanceof LastValueAggregator);
-  const lastValue = metric.aggregator.toPoint().value;
-  if (typeof value === 'number') {
-    assert.strictEqual(lastValue, value);
+function ensureMetric(
+  metric: MetricRecord,
+  name?: string,
+  value?: Distribution
+) {
+  assert.ok(metric.aggregator instanceof MinMaxLastSumCountAggregator);
+  const distribution = metric.aggregator.toPoint().value as Distribution;
+  if (value) {
+    assert.deepStrictEqual(distribution, value);
   } else {
-    assert.ok(lastValue >= 0 && lastValue <= 1);
+    assert.ok(distribution.last >= 0 && distribution.last <= 1);
   }
   const descriptor = metric.descriptor;
   assert.strictEqual(descriptor.name, name || 'name');