Merge branch 'master' into faas-attributes

open-telemetry · Nov 11, 2020 · b967998 · b967998
2 parents fb62223 + ceba9be
commit b967998
Show file tree

Hide file tree

Showing 13 changed files with 106 additions and 18 deletions.
diff --git a/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts
@@ -197,7 +197,9 @@ const testCollectorMetricExporter = (params: TestParams) =>
             );
             ensureExportedValueRecorderIsCorrect(
               recorder,
-              recorder.intHistogram?.dataPoints[0].timeUnixNano
+              recorder.intHistogram?.dataPoints[0].timeUnixNano,
+              [0, 100],
+              ['0', '2', '0']
             );
             assert.ok(
               typeof resource !== 'undefined',

diff --git a/packages/opentelemetry-exporter-collector-grpc/test/helper.ts b/packages/opentelemetry-exporter-collector-grpc/test/helper.ts
@@ -372,7 +372,9 @@ export function ensureExportedObserverIsCorrect(
 
 export function ensureExportedValueRecorderIsCorrect(
   metric: collectorTypes.opentelemetryProto.metrics.v1.Metric,
-  time?: number
+  time?: number,
+  explicitBounds: number[] = [Infinity],
+  bucketCounts: string[] = ['2', '0']
 ) {
   assert.deepStrictEqual(metric, {
     name: 'int-recorder',
@@ -388,8 +390,8 @@ export function ensureExportedValueRecorderIsCorrect(
           count: '2',
           startTimeUnixNano: '1592602232694000128',
           timeUnixNano: String(time),
-          bucketCounts: ['2', '0'],
-          explicitBounds: [Infinity],
+          bucketCounts,
+          explicitBounds,
         },
       ],
       aggregationTemporality: 'AGGREGATION_TEMPORALITY_CUMULATIVE',

diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts
@@ -142,7 +142,9 @@ describe('CollectorMetricExporter - node with proto over http', () => {
         );
         ensureExportedValueRecorderIsCorrect(
           metric3,
-          metric3.intHistogram?.dataPoints[0].timeUnixNano
+          metric3.intHistogram?.dataPoints[0].timeUnixNano,
+          [0, 100],
+          ['0', '2', '0']
         );
 
         ensureExportMetricsServiceRequestIsSet(json);

diff --git a/packages/opentelemetry-exporter-collector-proto/test/helper.ts b/packages/opentelemetry-exporter-collector-proto/test/helper.ts
@@ -338,7 +338,9 @@ export function ensureExportedObserverIsCorrect(
 
 export function ensureExportedValueRecorderIsCorrect(
   metric: collectorTypes.opentelemetryProto.metrics.v1.Metric,
-  time?: number
+  time?: number,
+  explicitBounds: number[] = [Infinity],
+  bucketCounts: string[] = ['2', '0']
 ) {
   assert.deepStrictEqual(metric, {
     name: 'int-recorder',
@@ -351,8 +353,8 @@ export function ensureExportedValueRecorderIsCorrect(
           count: '2',
           startTimeUnixNano: '1592602232694000128',
           timeUnixNano: time,
-          bucketCounts: ['2', '0'],
-          explicitBounds: ['Infinity'],
+          bucketCounts,
+          explicitBounds,
         },
       ],
       aggregationTemporality: 'AGGREGATION_TEMPORALITY_CUMULATIVE',

diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorMetricExporter.test.ts
@@ -123,7 +123,8 @@ describe('CollectorMetricExporter - web', () => {
             ensureValueRecorderIsCorrect(
               metric3,
               hrTimeToNanoseconds(metrics[2].aggregator.toPoint().timestamp),
-              true
+              [0, 100],
+              [0, 2, 0]
             );
           }
 
@@ -236,7 +237,8 @@ describe('CollectorMetricExporter - web', () => {
             ensureValueRecorderIsCorrect(
               metric3,
               hrTimeToNanoseconds(metrics[2].aggregator.toPoint().timestamp),
-              true
+              [0, 100],
+              [0, 2, 0]
             );
           }
 

diff --git a/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts b/packages/opentelemetry-exporter-collector/test/common/transformMetrics.test.ts
@@ -67,7 +67,9 @@ describe('transformMetrics', () => {
 
       ensureValueRecorderIsCorrect(
         transform.toCollectorMetric(recorder, 1592602232694000000),
-        hrTimeToNanoseconds(recorder.aggregator.toPoint().timestamp)
+        hrTimeToNanoseconds(recorder.aggregator.toPoint().timestamp),
+        [0, 100],
+        [0, 2, 0]
       );
 
       ensureDoubleCounterIsCorrect(

diff --git a/packages/opentelemetry-exporter-collector/test/helper.ts b/packages/opentelemetry-exporter-collector/test/helper.ts
@@ -577,7 +577,8 @@ export function ensureObserverIsCorrect(
 export function ensureValueRecorderIsCorrect(
   metric: collectorTypes.opentelemetryProto.metrics.v1.Metric,
   time: number,
-  infinityIsNull?: boolean
+  explicitBounds: (number | null)[] = [Infinity],
+  bucketCounts: number[] = [2, 0]
 ) {
   assert.deepStrictEqual(metric, {
     name: 'int-recorder',
@@ -591,8 +592,8 @@ export function ensureValueRecorderIsCorrect(
           count: 2,
           startTimeUnixNano: 1592602232694000000,
           timeUnixNano: time,
-          bucketCounts: [2, 0],
-          explicitBounds: [infinityIsNull ? null : Infinity],
+          bucketCounts,
+          explicitBounds,
         },
       ],
       aggregationTemporality:

diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorMetricExporter.test.ts
@@ -158,7 +158,8 @@ describe('CollectorMetricExporter - node with json over http', () => {
         ensureValueRecorderIsCorrect(
           metric3,
           core.hrTimeToNanoseconds(metrics[2].aggregator.toPoint().timestamp),
-          true
+          [0, 100],
+          [0, 2, 0]
         );
 
         ensureExportMetricsServiceRequestIsSet(json);

diff --git a/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts b/packages/opentelemetry-exporter-prometheus/src/PrometheusSerializer.ts
@@ -206,16 +206,33 @@ export class PrometheusSerializer {
         }
 
         let cumulativeSum = 0;
-        for (const [idx, val] of value.buckets.counts.entries()) {
+        const countEntries = value.buckets.counts.entries();
+        let infiniteBoundaryDefined = false;
+        for (const [idx, val] of countEntries) {
           cumulativeSum += val;
           const upperBound = value.buckets.boundaries[idx];
+          /** HistogramAggregator is producing different boundary output -
+           * in one case not including inifinity values, in other -
+           * full, e.g. [0, 100] and [0, 100, Infinity]
+           * we should consider that in export, if Infinity is defined, use it
+           * as boundary
+           */
+          if (upperBound === undefined && infiniteBoundaryDefined) {
+            break;
+          }
+          if (upperBound === Infinity) {
+            infiniteBoundaryDefined = true;
+          }
           results += stringify(
             name + '_bucket',
             record.labels,
             cumulativeSum,
             this._appendTimestamp ? timestamp : undefined,
             {
-              le: upperBound === undefined ? '+Inf' : String(upperBound),
+              le:
+                upperBound === undefined || upperBound === Infinity
+                  ? '+Inf'
+                  : String(upperBound),
             }
           );
         }

diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusExporter.test.ts
@@ -538,7 +538,6 @@ describe('PrometheusExporter', () => {
                   '# TYPE value_recorder histogram',
                   `value_recorder_count{key1="labelValue1"} 1 ${mockedHrTimeMs}`,
                   `value_recorder_sum{key1="labelValue1"} 20 ${mockedHrTimeMs}`,
-                  `value_recorder_bucket{key1="labelValue1",le="Infinity"} 1 ${mockedHrTimeMs}`,
                   `value_recorder_bucket{key1="labelValue1",le="+Inf"} 1 ${mockedHrTimeMs}`,
                   '',
                 ]);

diff --git a/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts b/packages/opentelemetry-exporter-prometheus/test/PrometheusSerializer.test.ts
@@ -175,6 +175,34 @@ describe('PrometheusSerializer', () => {
         );
       });
 
+      it('should serialize metric record with sum aggregator, boundaries defined in constructor', async () => {
+        const serializer = new PrometheusSerializer();
+
+        const meter = new MeterProvider().getMeter('test');
+        const recorder = meter.createValueRecorder('test', {
+          description: 'foobar',
+          boundaries: [1, 10, 100],
+        }) as ValueRecorderMetric;
+        recorder.bind(labels).record(5);
+
+        const records = await recorder.getMetricRecord();
+        const record = records[0];
+
+        const result = serializer.serializeRecord(
+          record.descriptor.name,
+          record
+        );
+        assert.strictEqual(
+          result,
+          `test_count{foo1="bar1",foo2="bar2"} 1 ${mockedHrTimeMs}\n` +
+            `test_sum{foo1="bar1",foo2="bar2"} 5 ${mockedHrTimeMs}\n` +
+            `test_bucket{foo1="bar1",foo2="bar2",le="1"} 0 ${mockedHrTimeMs}\n` +
+            `test_bucket{foo1="bar1",foo2="bar2",le="10"} 1 ${mockedHrTimeMs}\n` +
+            `test_bucket{foo1="bar1",foo2="bar2",le="100"} 1 ${mockedHrTimeMs}\n` +
+            `test_bucket{foo1="bar1",foo2="bar2",le="+Inf"} 1 ${mockedHrTimeMs}\n`
+        );
+      });
+
       it('serialize metric record with sum aggregator without timestamp', async () => {
         const serializer = new PrometheusSerializer(undefined, false);
 

diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts
@@ -28,6 +28,7 @@ export abstract class Metric<T extends BaseBoundInstrument>
   protected readonly _valueType: api.ValueType;
   protected readonly _logger: api.Logger;
   protected readonly _descriptor: MetricDescriptor;
+  protected readonly _boundaries: number[] | undefined;
   private readonly _instruments: Map<string, T> = new Map();
 
   constructor(
@@ -43,6 +44,7 @@ export abstract class Metric<T extends BaseBoundInstrument>
         ? _options.valueType
         : api.ValueType.DOUBLE;
     this._logger = _options.logger ?? new NoopLogger();
+    this._boundaries = _options.boundaries;
     this._descriptor = this._getMetricDescriptor();
   }
 
@@ -105,6 +107,7 @@ export abstract class Metric<T extends BaseBoundInstrument>
       unit: this._options.unit || '1',
       metricKind: this._kind,
       valueType: this._valueType,
+      ...(this._boundaries && { boundaries: this._boundaries }),
     };
   }
 

diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts
@@ -556,6 +556,33 @@ describe('Meter', () => {
       assert.ok(valueRecorder instanceof Metric);
     });
 
+    it('should set histogram boundaries for value recorder', async () => {
+      const valueRecorder = meter.createValueRecorder('name', {
+        description: 'desc',
+        unit: '1',
+        disabled: false,
+        boundaries: [10, 20, 30, 100],
+      }) as ValueRecorderMetric;
+
+      valueRecorder.record(10);
+      valueRecorder.record(30);
+      valueRecorder.record(50);
+      valueRecorder.record(200);
+
+      await meter.collect();
+      const [record] = meter.getBatcher().checkPointSet();
+      assert.deepStrictEqual(record.aggregator.toPoint().value as Histogram, {
+        buckets: {
+          boundaries: [10, 20, 30, 100],
+          counts: [0, 1, 0, 2, 1],
+        },
+        count: 4,
+        sum: 290,
+      });
+
+      assert.ok(valueRecorder instanceof Metric);
+    });
+
     it('should pipe through resource', async () => {
       const valueRecorder = meter.createValueRecorder(
         'name'