feat(sdk-metrics-base): distinguish between Sum and Gauge in MetricDa…

…ta (#3079) * feature(sdk-metrics-base): distinguish between Sum and Gauge in MetricData * feat(sdk-metrics-base): fix test. * fix(changelog): add changelog entry. * feat(prometheus-serializer): add test for gauge serialization. * feat(sdk-metrics-base): add assertions for isMonotonic in Instrument tests. * feat(otlp-transformer): add test-cases for non-monotonic sums. * feat(prometheus-exporter): add test-cases for gauges. * fix(sdk-metrics-base): remove DataPointType.SINGULAR * fix(changelog): add more details about breaking changes.
open-telemetry · Jul 19, 2022 · 9ac9976 · 9ac9976
1 parent 4ee8fe3
commit 9ac9976
Show file tree

Hide file tree

Showing 17 changed files with 391 additions and 157 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -7,6 +7,9 @@ All notable changes to experimental packages in this project will be documented
 ### :boom: Breaking Change
 
 * feature(views): move views registration to MeterProvider constructor [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3066) @pichlermarc
+* feat(sdk-metrics-base): split up Singular into Sum and Gauge in MetricData [#3079](https://github.com/open-telemetry/opentelemetry-js/pull/3079) @pichlermarc
+  * removes `DataPointType.SINGULAR`, and replaces it with `DataPointType.SUM` and `DataPointType.GAUGE`
+  * removes `SingularMetricData` and replaces it with `SumMetricData` (including an additional `isMonotonic` flag) and `GaugeMetricData`
 
 ### :rocket: (Enhancement)
 

diff --git a/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
@@ -104,23 +104,15 @@ function valueString(value: number) {
 }
 
 function toPrometheusType(
-  instrumentType: InstrumentType,
-  dataPointType: DataPointType,
+  metricData: MetricData,
 ): PrometheusDataTypeLiteral {
-  switch (dataPointType) {
-    case DataPointType.SINGULAR:
-      if (
-        instrumentType === InstrumentType.COUNTER ||
-        instrumentType === InstrumentType.OBSERVABLE_COUNTER
-      ) {
+  switch (metricData.dataPointType) {
+    case DataPointType.SUM:
+      if (metricData.isMonotonic) {
         return 'counter';
       }
-      /**
-       * - HISTOGRAM
-       * - UP_DOWN_COUNTER
-       * - OBSERVABLE_GAUGE
-       * - OBSERVABLE_UP_DOWN_COUNTER
-       */
+      return 'gauge';
+    case DataPointType.GAUGE:
       return 'gauge';
     case DataPointType.HISTOGRAM:
       return 'histogram';
@@ -210,13 +202,13 @@ export class PrometheusSerializer {
       metricData.descriptor.description || 'description missing'
     )}`;
     const type = `# TYPE ${name} ${toPrometheusType(
-      metricData.descriptor.type,
-      dataPointType
+      metricData
     )}`;
 
     let results = '';
     switch (dataPointType) {
-      case DataPointType.SINGULAR: {
+      case DataPointType.SUM:
+      case DataPointType.GAUGE: {
         results = metricData.dataPoints
           .map(it => this._serializeSingularDataPoint(name, metricData.descriptor.type, it))
           .join('');

diff --git a/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/experimental/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts
@@ -18,13 +18,14 @@ import * as assert from 'assert';
 import { MetricAttributes, UpDownCounter } from '@opentelemetry/api-metrics';
 import {
   AggregationTemporality,
-  MeterProvider,
-  MetricReader,
   DataPoint,
   DataPointType,
   ExplicitBucketHistogramAggregation,
-  SumAggregation,
   Histogram,
+  LastValueAggregation,
+  MeterProvider,
+  MetricReader,
+  SumAggregation,
   View,
 } from '@opentelemetry/sdk-metrics-base';
 import * as sinon from 'sinon';
@@ -85,7 +86,7 @@ describe('PrometheusSerializer', () => {
         assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);
         assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 1);
         const metric = resourceMetrics.scopeMetrics[0].metrics[0];
-        assert.strictEqual(metric.dataPointType, DataPointType.SINGULAR);
+        assert.strictEqual(metric.dataPointType, DataPointType.SUM);
         const pointData = metric.dataPoints as DataPoint<number>[];
         assert.strictEqual(pointData.length, 1);
 
@@ -171,7 +172,7 @@ describe('PrometheusSerializer', () => {
   });
 
   describe('serialize an instrumentation metrics', () => {
-    describe('Singular', () => {
+    describe('monotonic Sum', () => {
       async function testSerializer(serializer: PrometheusSerializer) {
         const reader = new TestMetricReader();
         const meterProvider = new MeterProvider({
@@ -196,7 +197,7 @@ describe('PrometheusSerializer', () => {
         return result;
       }
 
-      it('should serialize metric record with sum aggregator', async () => {
+      it('should serialize metric record', async () => {
         const serializer = new PrometheusSerializer();
         const result = await testSerializer(serializer);
         assert.strictEqual(
@@ -208,7 +209,7 @@ describe('PrometheusSerializer', () => {
         );
       });
 
-      it('serialize metric record with sum aggregator without timestamp', async () => {
+      it('should serialize metric record without timestamp', async () => {
         const serializer = new PrometheusSerializer(undefined, false);
         const result = await testSerializer(serializer);
         assert.strictEqual(
@@ -221,6 +222,108 @@ describe('PrometheusSerializer', () => {
       });
     });
 
+    describe('non-monotonic Sum', () => {
+      async function testSerializer(serializer: PrometheusSerializer) {
+        const reader = new TestMetricReader();
+        const meterProvider = new MeterProvider({
+          views: [
+            new View({ aggregation: new SumAggregation(), instrumentName: '*' })
+          ]
+        });
+        meterProvider.addMetricReader(reader);
+        const meter = meterProvider.getMeter('test');
+
+        const counter = meter.createUpDownCounter('test_total', {
+          description: 'foobar',
+        });
+        counter.add(1, { val: '1' });
+        counter.add(1, { val: '2' });
+
+        const { resourceMetrics, errors } = await reader.collect();
+        assert.strictEqual(errors.length, 0);
+        assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);
+        assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 1);
+        const scopeMetrics = resourceMetrics.scopeMetrics[0];
+
+        return serializer['_serializeScopeMetrics'](scopeMetrics);
+      }
+
+      it('should serialize metric record', async () => {
+        const serializer = new PrometheusSerializer();
+        const result = await testSerializer(serializer);
+        assert.strictEqual(
+          result,
+          '# HELP test_total foobar\n' +
+          '# TYPE test_total gauge\n' +
+          `test_total{val="1"} 1 ${mockedHrTimeMs}\n` +
+          `test_total{val="2"} 1 ${mockedHrTimeMs}\n`
+        );
+      });
+
+      it('serialize metric record without timestamp', async () => {
+        const serializer = new PrometheusSerializer(undefined, false);
+        const result = await testSerializer(serializer);
+        assert.strictEqual(
+          result,
+          '# HELP test_total foobar\n' +
+          '# TYPE test_total gauge\n' +
+          'test_total{val="1"} 1\n' +
+          'test_total{val="2"} 1\n'
+        );
+      });
+    });
+
+    describe('Gauge', () => {
+      async function testSerializer(serializer: PrometheusSerializer) {
+        const reader = new TestMetricReader();
+        const meterProvider = new MeterProvider({
+          views: [
+            new View({aggregation: new LastValueAggregation(), instrumentName: '*' })
+          ]
+        });
+        meterProvider.addMetricReader(reader);
+        const meter = meterProvider.getMeter('test');
+
+        const counter = meter.createUpDownCounter('test_total', {
+          description: 'foobar',
+        });
+        counter.add(1, { val: '1' });
+        counter.add(1, { val: '2' });
+
+        const { resourceMetrics, errors } = await reader.collect();
+        assert.strictEqual(errors.length, 0);
+        assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);
+        assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 1);
+        const scopeMetrics = resourceMetrics.scopeMetrics[0];
+
+        return serializer['_serializeScopeMetrics'](scopeMetrics);
+      }
+
+      it('should serialize metric record', async () => {
+        const serializer = new PrometheusSerializer();
+        const result = await testSerializer(serializer);
+        assert.strictEqual(
+          result,
+          '# HELP test_total foobar\n' +
+          '# TYPE test_total gauge\n' +
+          `test_total{val="1"} 1 ${mockedHrTimeMs}\n` +
+          `test_total{val="2"} 1 ${mockedHrTimeMs}\n`
+        );
+      });
+
+      it('serialize metric record without timestamp', async () => {
+        const serializer = new PrometheusSerializer(undefined, false);
+        const result = await testSerializer(serializer);
+        assert.strictEqual(
+          result,
+          '# HELP test_total foobar\n' +
+          '# TYPE test_total gauge\n' +
+          'test_total{val="1"} 1\n' +
+          'test_total{val="2"} 1\n'
+        );
+      });
+    });
+
     describe('with ExplicitBucketHistogramAggregation', () => {
       async function testSerializer(serializer: PrometheusSerializer) {
         const reader = new TestMetricReader();
@@ -293,7 +396,7 @@ describe('PrometheusSerializer', () => {
       assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);
       assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 1);
       const metric = resourceMetrics.scopeMetrics[0].metrics[0];
-      assert.strictEqual(metric.dataPointType, DataPointType.SINGULAR);
+      assert.strictEqual(metric.dataPointType, DataPointType.SUM);
       const pointData = metric.dataPoints as DataPoint<number>[];
       assert.strictEqual(pointData.length, 1);
 
@@ -333,7 +436,7 @@ describe('PrometheusSerializer', () => {
       assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);
       assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 1);
       const metric = resourceMetrics.scopeMetrics[0].metrics[0];
-      assert.strictEqual(metric.dataPointType, DataPointType.SINGULAR);
+      assert.strictEqual(metric.dataPointType, DataPointType.SUM);
       const pointData = metric.dataPoints as DataPoint<number>[];
       assert.strictEqual(pointData.length, 1);
 

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/LastValue.ts
@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-import { LastValue, AggregatorKind, Aggregator, Accumulation, AccumulationRecord } from './types';
+import { Accumulation, AccumulationRecord, Aggregator, AggregatorKind, LastValue } from './types';
 import { HrTime } from '@opentelemetry/api';
 import { hrTime, hrTimeToMicroseconds } from '@opentelemetry/core';
-import { DataPointType, SingularMetricData } from '../export/MetricData';
+import { DataPointType, GaugeMetricData } from '../export/MetricData';
 import { InstrumentDescriptor } from '../InstrumentDescriptor';
 import { Maybe } from '../utils';
 import { AggregationTemporality } from '../export/AggregationTemporality';
@@ -74,11 +74,11 @@ export class LastValueAggregator implements Aggregator<LastValueAccumulation> {
     descriptor: InstrumentDescriptor,
     aggregationTemporality: AggregationTemporality,
     accumulationByAttributes: AccumulationRecord<LastValueAccumulation>[],
-    endTime: HrTime): Maybe<SingularMetricData> {
+    endTime: HrTime): Maybe<GaugeMetricData> {
     return {
       descriptor,
       aggregationTemporality,
-      dataPointType: DataPointType.SINGULAR,
+      dataPointType: DataPointType.GAUGE,
       dataPoints: accumulationByAttributes.map(([attributes, accumulation]) => {
         return {
           attributes,

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/aggregator/Sum.ts
@@ -16,7 +16,7 @@
 
 import { Sum, AggregatorKind, Aggregator, Accumulation, AccumulationRecord } from './types';
 import { HrTime } from '@opentelemetry/api';
-import { DataPointType, SingularMetricData } from '../export/MetricData';
+import { DataPointType, SumMetricData } from '../export/MetricData';
 import { InstrumentDescriptor } from '../InstrumentDescriptor';
 import { Maybe } from '../utils';
 import { AggregationTemporality } from '../export/AggregationTemporality';
@@ -83,19 +83,20 @@ export class SumAggregator implements Aggregator<SumAccumulation> {
     descriptor: InstrumentDescriptor,
     aggregationTemporality: AggregationTemporality,
     accumulationByAttributes: AccumulationRecord<SumAccumulation>[],
-    endTime: HrTime): Maybe<SingularMetricData> {
+    endTime: HrTime): Maybe<SumMetricData> {
     return {
       descriptor,
       aggregationTemporality,
-      dataPointType: DataPointType.SINGULAR,
+      dataPointType: DataPointType.SUM,
       dataPoints: accumulationByAttributes.map(([attributes, accumulation]) => {
         return {
           attributes,
           startTime: accumulation.startTime,
           endTime,
           value: accumulation.toPointValue(),
         };
-      })
+      }),
+      isMonotonic: this.monotonic
     };
   }
 }
diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/export/MetricData.ts
@@ -38,8 +38,14 @@ export interface BaseMetricData {
  * Represents a metric data aggregated by either a LastValueAggregation or
  * SumAggregation.
  */
-export interface SingularMetricData extends BaseMetricData {
-  readonly dataPointType: DataPointType.SINGULAR;
+export interface SumMetricData extends BaseMetricData {
+  readonly dataPointType: DataPointType.SUM;
+  readonly dataPoints: DataPoint<number>[];
+  readonly isMonotonic: boolean;
+}
+
+export interface GaugeMetricData extends BaseMetricData {
+  readonly dataPointType: DataPointType.GAUGE;
   readonly dataPoints: DataPoint<number>[];
 }
 
@@ -54,7 +60,7 @@ export interface HistogramMetricData extends BaseMetricData {
 /**
  * Represents an aggregated metric data.
  */
-export type MetricData = SingularMetricData | HistogramMetricData;
+export type MetricData = SumMetricData | GaugeMetricData | HistogramMetricData;
 
 export interface ScopeMetrics {
   scope: InstrumentationScope;
@@ -87,10 +93,6 @@ export interface CollectionResult {
  * The aggregated point data type.
  */
 export enum DataPointType {
-  /**
-   * A singular metric data point has only a single numeric value.
-   */
-  SINGULAR,
   /**
    * A histogram data point contains a histogram statistics of collected
    * values with a list of explicit bucket boundaries and statistics such
@@ -104,6 +106,15 @@ export enum DataPointType {
    * and sum of all collected values.
    */
   EXPONENTIAL_HISTOGRAM,
+  /**
+   * A gauge metric data point has only a single numeric value.
+   */
+  GAUGE,
+  /**
+   * A sum metric data point has a single numeric value and a
+   * monotonicity-indicator.
+   */
+  SUM
 }
 
 /**