Metrics updates (#1700)

* feat: renaming batcher to processor, fixing aggregators, adding missing metrics in api * chore: fixing metrics in exporter collector, updated tests, fixing observer result * chore: refactoring tests for rest of collectors * chore: fixing monotonic sum observer, updated test to cover that scenario correctly
open-telemetry · Dec 2, 2020 · 781b30f · 781b30f
1 parent b22ca63
commit 781b30f
Show file tree

Hide file tree

Showing 37 changed files with 699 additions and 442 deletions.
diff --git a/doc/batcher-api.md → doc/processor-api.md b/doc/batcher-api.md → doc/processor-api.md
@@ -1,6 +1,6 @@
-# Batcher API Guide
+# Processor API Guide
 
-[The batcher](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-metrics/src/export/Batcher.ts?rgh-link-date=2020-05-25T18%3A43%3A57Z) has two responsibilities: choosing which aggregator to choose for a metric instrument and store the last record for each metric ready to be exported.
+[The processor](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-metrics/src/export/Processor.ts?rgh-link-date=2020-05-25T18%3A43%3A57Z) has two responsibilities: choosing which aggregator to choose for a metric instrument and store the last record for each metric ready to be exported.
 
 ## Selecting a specific aggregator for metrics
 
@@ -41,25 +41,25 @@ export class AverageAggregator implements Aggregator {
 }
 ```
 
-Now we will need to implement our own batcher to configure the sdk to use our new aggregator. To simplify even more, we will just extend the `UngroupedBatcher` (which is the default) to avoid re-implementing the whole `Aggregator` interface.
+Now we will need to implement our own processor to configure the sdk to use our new aggregator. To simplify even more, we will just extend the `UngroupedProcessor` (which is the default) to avoid re-implementing the whole `Aggregator` interface.
 
 Here the result:
 
 ```ts
 import {
-  UngroupedBatcher,
+  UngroupedProcessor,
   MetricDescriptor,
   CounterSumAggregator,
   ObserverAggregator,
   MeasureExactAggregator,
 } from '@opentelemetry/metrics';
 
-export class CustomBatcher extends UngroupedBatcher {
+export class CustomProcessor extends UngroupedProcessor {
   aggregatorFor (metricDescriptor: MetricDescriptor) {
     if (metricDescriptor.name === 'requests') {
       return new AverageAggregator(10);
     }
-    // this is exactly what the "UngroupedBatcher" does, we will re-use it
+    // this is exactly what the "UngroupedProcessor" does, we will re-use it
     // to fallback on the default behavior
     switch (metricDescriptor.metricKind) {
       case MetricKind.COUNTER:
@@ -73,11 +73,11 @@ export class CustomBatcher extends UngroupedBatcher {
 }
 ```
 
-Finally, we need to specify to the `MeterProvider` to use our `CustomBatcher` when creating new meter:
+Finally, we need to specify to the `MeterProvider` to use our `CustomProcessor` when creating new meter:
 
 ```ts
 import {
-  UngroupedBatcher,
+  UngroupedProcessor,
   MetricDescriptor,
   CounterSumAggregator,
   ObserverAggregator,
@@ -115,12 +115,12 @@ export class AverageAggregator implements Aggregator {
   }
 }
 
-export class CustomBatcher extends UngroupedBatcher {
+export class CustomProcessor extends UngroupedProcessor {
   aggregatorFor (metricDescriptor: MetricDescriptor) {
     if (metricDescriptor.name === 'requests') {
       return new AverageAggregator(10);
     }
-    // this is exactly what the "UngroupedBatcher" does, we will re-use it
+    // this is exactly what the "UngroupedProcessor" does, we will re-use it
     // to fallback on the default behavior
     switch (metricDescriptor.metricKind) {
       case MetricKind.COUNTER:
@@ -134,9 +134,9 @@ export class CustomBatcher extends UngroupedBatcher {
 }
 
 const meter = new MeterProvider({
-  batcher: new CustomBatcher(),
+  processor: new CustomProcessor(),
   interval: 1000,
-}).getMeter('example-custom-batcher');
+}).getMeter('example-custom-processor');
 
 const requestsLatency = meter.createValueRecorder('requests', {
   monotonic: true,

diff --git a/packages/opentelemetry-api/src/metrics/Meter.ts b/packages/opentelemetry-api/src/metrics/Meter.ts
@@ -23,6 +23,8 @@ import {
   BatchObserver,
   BatchMetricOptions,
   UpDownCounter,
+  SumObserver,
+  UpDownSumObserver,
 } from './Metric';
 import { ObserverResult } from './ObserverResult';
 
@@ -81,6 +83,30 @@ export interface Meter {
     callback?: (observerResult: ObserverResult) => void
   ): ValueObserver;
 
+  /**
+   * Creates a new `SumObserver` metric.
+   * @param name the name of the metric.
+   * @param [options] the metric options.
+   * @param [callback] the observer callback
+   */
+  createSumObserver(
+    name: string,
+    options?: MetricOptions,
+    callback?: (observerResult: ObserverResult) => void
+  ): SumObserver;
+
+  /**
+   * Creates a new `UpDownSumObserver` metric.
+   * @param name the name of the metric.
+   * @param [options] the metric options.
+   * @param [callback] the observer callback
+   */
+  createUpDownSumObserver(
+    name: string,
+    options?: MetricOptions,
+    callback?: (observerResult: ObserverResult) => void
+  ): UpDownSumObserver;
+
   /**
    * Creates a new `BatchObserver` metric, can be used to update many metrics
    * at the same time and when operations needs to be async

diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts
@@ -26,6 +26,7 @@ import {
   BatchObserver,
   UpDownCounter,
   BaseObserver,
+  UpDownSumObserver,
 } from './Metric';
 import {
   BoundValueRecorder,
@@ -84,6 +85,34 @@ export class NoopMeter implements Meter {
     return NOOP_VALUE_OBSERVER_METRIC;
   }
 
+  /**
+   * Returns constant noop sum observer.
+   * @param name the name of the metric.
+   * @param [options] the metric options.
+   * @param [callback] the sum observer callback
+   */
+  createSumObserver(
+    _name: string,
+    _options?: MetricOptions,
+    _callback?: (observerResult: ObserverResult) => void
+  ): ValueObserver {
+    return NOOP_SUM_OBSERVER_METRIC;
+  }
+
+  /**
+   * Returns constant noop up down sum observer.
+   * @param name the name of the metric.
+   * @param [options] the metric options.
+   * @param [callback] the up down sum observer callback
+   */
+  createUpDownSumObserver(
+    _name: string,
+    _options?: MetricOptions,
+    _callback?: (observerResult: ObserverResult) => void
+  ): UpDownSumObserver {
+    return NOOP_UP_DOWN_SUM_OBSERVER_METRIC;
+  }
+
   /**
    * Returns constant noop batch observer.
    * @param name the name of the metric.

diff --git a/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-grpc/test/CollectorMetricExporter.test.ts
@@ -22,7 +22,6 @@ import * as fs from 'fs';
 import * as assert from 'assert';
 import * as sinon from 'sinon';
 import { collectorTypes } from '@opentelemetry/exporter-collector';
-import { MetricRecord } from '@opentelemetry/metrics';
 import { CollectorMetricExporter } from '../src';
 import {
   mockCounter,
@@ -35,6 +34,8 @@ import {
   mockValueRecorder,
 } from './helper';
 import { ConsoleLogger, LogLevel } from '@opentelemetry/core';
+import * as api from '@opentelemetry/api';
+import * as metrics from '@opentelemetry/metrics';
 
 const metricsServiceProtoPath =
   'opentelemetry/proto/collector/metrics/v1/metrics_service.proto';
@@ -59,7 +60,7 @@ const testCollectorMetricExporter = (params: TestParams) =>
     let exportedData:
       | collectorTypes.opentelemetryProto.metrics.v1.ResourceMetrics[]
       | undefined;
-    let metrics: MetricRecord[];
+    let metrics: metrics.MetricRecord[];
     let reqMetadata: grpc.Metadata | undefined;
 
     before(done => {
@@ -134,17 +135,23 @@ const testCollectorMetricExporter = (params: TestParams) =>
         value: 1592602232694000000,
       });
       metrics = [];
-      metrics.push(await mockCounter());
-      metrics.push(await mockObserver());
-      metrics.push(await mockValueRecorder());
-
-      metrics[0].aggregator.update(1);
+      const counter: metrics.Metric<metrics.BoundCounter> &
+        api.Counter = mockCounter();
+      const observer: metrics.Metric<metrics.BoundObserver> &
+        api.ValueObserver = mockObserver(observerResult => {
+        observerResult.observe(3, {});
+        observerResult.observe(6, {});
+      });
+      const recorder: metrics.Metric<metrics.BoundValueRecorder> &
+        api.ValueRecorder = mockValueRecorder();
 
-      metrics[1].aggregator.update(3);
-      metrics[1].aggregator.update(6);
+      counter.add(1);
+      recorder.record(7);
+      recorder.record(14);
 
-      metrics[2].aggregator.update(7);
-      metrics[2].aggregator.update(14);
+      metrics.push((await counter.getMetricRecord())[0]);
+      metrics.push((await observer.getMetricRecord())[0]);
+      metrics.push((await recorder.getMetricRecord())[0]);
     });
 
     afterEach(() => {

diff --git a/packages/opentelemetry-exporter-collector-grpc/test/helper.ts b/packages/opentelemetry-exporter-collector-grpc/test/helper.ts
@@ -14,15 +14,15 @@
  * limitations under the License.
  */
 
-import { TraceFlags, ValueType, StatusCode } from '@opentelemetry/api';
+import * as api from '@opentelemetry/api';
+import * as metrics from '@opentelemetry/metrics';
 import { ReadableSpan } from '@opentelemetry/tracing';
 import { Resource } from '@opentelemetry/resources';
 import { collectorTypes } from '@opentelemetry/exporter-collector';
 import * as assert from 'assert';
-import { MetricRecord, MeterProvider } from '@opentelemetry/metrics';
 import * as grpc from 'grpc';
 
-const meterProvider = new MeterProvider({
+const meterProvider = new metrics.MeterProvider({
   interval: 30000,
   resource: new Resource({
     service: 'ui',
@@ -54,61 +54,52 @@ const traceIdArr = [
 const spanIdArr = [94, 16, 114, 97, 246, 79, 165, 62];
 const parentIdArr = [120, 168, 145, 80, 152, 134, 67, 136];
 
-export async function mockCounter(): Promise<MetricRecord> {
+export function mockCounter(): metrics.Metric<metrics.BoundCounter> &
+  api.Counter {
   const name = 'int-counter';
   const metric =
     meter['_metrics'].get(name) ||
     meter.createCounter(name, {
       description: 'sample counter description',
-      valueType: ValueType.INT,
+      valueType: api.ValueType.INT,
     });
   metric.clear();
   metric.bind({});
-
-  return (await metric.getMetricRecord())[0];
-}
-
-export async function mockDoubleCounter(): Promise<MetricRecord> {
-  const name = 'double-counter';
-  const metric =
-    meter['_metrics'].get(name) ||
-    meter.createCounter(name, {
-      description: 'sample counter description',
-      valueType: ValueType.DOUBLE,
-    });
-  metric.clear();
-  metric.bind({});
-
-  return (await metric.getMetricRecord())[0];
+  return metric;
 }
 
-export async function mockObserver(): Promise<MetricRecord> {
+export function mockObserver(
+  callback: (observerResult: api.ObserverResult) => void
+): metrics.Metric<metrics.BoundCounter> & api.ValueObserver {
   const name = 'double-observer';
   const metric =
     meter['_metrics'].get(name) ||
-    meter.createValueObserver(name, {
-      description: 'sample observer description',
-      valueType: ValueType.DOUBLE,
-    });
+    meter.createValueObserver(
+      name,
+      {
+        description: 'sample observer description',
+        valueType: api.ValueType.DOUBLE,
+      },
+      callback
+    );
   metric.clear();
   metric.bind({});
-
-  return (await metric.getMetricRecord())[0];
+  return metric;
 }
 
-export async function mockValueRecorder(): Promise<MetricRecord> {
+export function mockValueRecorder(): metrics.Metric<metrics.BoundValueRecorder> &
+  api.ValueRecorder {
   const name = 'int-recorder';
   const metric =
     meter['_metrics'].get(name) ||
     meter.createValueRecorder(name, {
       description: 'sample recorder description',
-      valueType: ValueType.INT,
+      valueType: api.ValueType.INT,
       boundaries: [0, 100],
     });
   metric.clear();
   metric.bind({});
-
-  return (await metric.getMetricRecord())[0];
+  return metric;
 }
 
 export const mockedReadableSpan: ReadableSpan = {
@@ -117,13 +108,13 @@ export const mockedReadableSpan: ReadableSpan = {
   spanContext: {
     traceId: '1f1008dc8e270e85c40a0d7c3939b278',
     spanId: '5e107261f64fa53e',
-    traceFlags: TraceFlags.SAMPLED,
+    traceFlags: api.TraceFlags.SAMPLED,
   },
   parentSpanId: '78a8915098864388',
   startTime: [1574120165, 429803070],
   endTime: [1574120165, 438688070],
   ended: true,
-  status: { code: StatusCode.OK },
+  status: { code: api.StatusCode.OK },
   attributes: { component: 'document-load' },
   links: [
     {

diff --git a/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts b/packages/opentelemetry-exporter-collector-proto/test/CollectorMetricExporter.test.ts
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import * as api from '@opentelemetry/api';
+import * as metrics from '@opentelemetry/metrics';
 import { collectorTypes } from '@opentelemetry/exporter-collector';
 import * as core from '@opentelemetry/core';
 import * as http from 'http';
@@ -32,7 +34,6 @@ import {
   ensureExportedValueRecorderIsCorrect,
   MockedResponse,
 } from './helper';
-import { MetricRecord } from '@opentelemetry/metrics';
 import { ExportResult, ExportResultCode } from '@opentelemetry/core';
 import { CollectorExporterError } from '@opentelemetry/exporter-collector/build/src/types';
 
@@ -50,7 +51,7 @@ describe('CollectorMetricExporter - node with proto over http', () => {
   let collectorExporterConfig: collectorTypes.CollectorExporterConfigBase;
   let spyRequest: sinon.SinonSpy;
   let spyWrite: sinon.SinonSpy;
-  let metrics: MetricRecord[];
+  let metrics: metrics.MetricRecord[];
   describe('export', () => {
     beforeEach(async () => {
       spyRequest = sinon.stub(http, 'request').returns(fakeRequest as any);
@@ -71,14 +72,23 @@ describe('CollectorMetricExporter - node with proto over http', () => {
         value: 1592602232694000000,
       });
       metrics = [];
-      metrics.push(await mockCounter());
-      metrics.push(await mockObserver());
-      metrics.push(await mockValueRecorder());
-      metrics[0].aggregator.update(1);
-      metrics[1].aggregator.update(3);
-      metrics[1].aggregator.update(6);
-      metrics[2].aggregator.update(7);
-      metrics[2].aggregator.update(14);
+      const counter: metrics.Metric<metrics.BoundCounter> &
+        api.Counter = mockCounter();
+      const observer: metrics.Metric<metrics.BoundObserver> &
+        api.ValueObserver = mockObserver(observerResult => {
+        observerResult.observe(3, {});
+        observerResult.observe(6, {});
+      });
+      const recorder: metrics.Metric<metrics.BoundValueRecorder> &
+        api.ValueRecorder = mockValueRecorder();
+
+      counter.add(1);
+      recorder.record(7);
+      recorder.record(14);
+
+      metrics.push((await counter.getMetricRecord())[0]);
+      metrics.push((await observer.getMetricRecord())[0]);
+      metrics.push((await recorder.getMetricRecord())[0]);
     });
     afterEach(() => {
       spyRequest.restore();