chore: collection from observers when using batch observer (#1470)

open-telemetry · Sep 3, 2020 · 4c83b27 · 4c83b27
1 parent c70dabf
commit 4c83b27
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 20 deletions.
diff --git a/packages/opentelemetry-metrics/src/BatchObserverMetric.ts b/packages/opentelemetry-metrics/src/BatchObserverMetric.ts
@@ -44,7 +44,7 @@ export class BatchObserverMetric
     super(
       name,
       options,
-      MetricKind.VALUE_OBSERVER,
+      MetricKind.BATCH_OBSERVER,
       resource,
       instrumentationLibrary
     );

diff --git a/packages/opentelemetry-metrics/src/Meter.ts b/packages/opentelemetry-metrics/src/Meter.ts
@@ -19,6 +19,7 @@ import { ConsoleLogger, InstrumentationLibrary } from '@opentelemetry/core';
 import { Resource } from '@opentelemetry/resources';
 import { BatchObserverMetric } from './BatchObserverMetric';
 import { BaseBoundInstrument } from './BoundInstrument';
+import { MetricKind } from './export/types';
 import { UpDownCounterMetric } from './UpDownCounterMetric';
 import { CounterMetric } from './CounterMetric';
 import { UpDownSumObserverMetric } from './UpDownSumObserverMetric';
@@ -295,9 +296,29 @@ export class Meter implements api.Meter {
    * meter instance.
    */
   async collect(): Promise<void> {
-    const metrics = Array.from(this._metrics.values()).map(metric => {
-      return metric.getMetricRecord();
+    // call batch observers first
+    const batchObservers = Array.from(this._metrics.values())
+      .filter(metric => {
+        return metric.getKind() === MetricKind.BATCH_OBSERVER;
+      })
+      .map(metric => {
+        return metric.getMetricRecord();
+      });
+    await Promise.all(batchObservers).then(records => {
+      records.forEach(metrics => {
+        metrics.forEach(metric => this._batcher.process(metric));
+      });
     });
+
+    // after this all remaining metrics can be run
+    const metrics = Array.from(this._metrics.values())
+      .filter(metric => {
+        return metric.getKind() !== MetricKind.BATCH_OBSERVER;
+      })
+      .map(metric => {
+        return metric.getMetricRecord();
+      });
+
     await Promise.all(metrics).then(records => {
       records.forEach(metrics => {
         metrics.forEach(metric => this._batcher.process(metric));

diff --git a/packages/opentelemetry-metrics/src/Metric.ts b/packages/opentelemetry-metrics/src/Metric.ts
@@ -77,6 +77,13 @@ export abstract class Metric<T extends BaseBoundInstrument>
     this._instruments.clear();
   }
 
+  /**
+   * Returns kind of metric
+   */
+  getKind(): MetricKind {
+    return this._kind;
+  }
+
   getMetricRecord(): Promise<MetricRecord[]> {
     return new Promise(resolve => {
       resolve(

diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts
@@ -26,6 +26,7 @@ export enum MetricKind {
   SUM_OBSERVER,
   UP_DOWN_SUM_OBSERVER,
   VALUE_OBSERVER,
+  BATCH_OBSERVER,
 }
 
 /** The kind of aggregator. */

diff --git a/packages/opentelemetry-metrics/test/Meter.test.ts b/packages/opentelemetry-metrics/test/Meter.test.ts
@@ -1180,16 +1180,13 @@ describe('Meter', () => {
       );
 
       await meter.collect();
+      const records = meter.getBatcher().checkPointSet();
+      assert.strictEqual(records.length, 8);
 
-      const tempMetricRecords: MetricRecord[] = await tempMetric.getMetricRecord();
-      const cpuUsageMetricRecords: MetricRecord[] = await cpuUsageMetric.getMetricRecord();
-      assert.strictEqual(tempMetricRecords.length, 4);
-      assert.strictEqual(cpuUsageMetricRecords.length, 4);
-
-      const metric1 = tempMetricRecords[0];
-      const metric2 = tempMetricRecords[1];
-      const metric3 = tempMetricRecords[2];
-      const metric4 = tempMetricRecords[3];
+      const metric1 = records[0];
+      const metric2 = records[1];
+      const metric3 = records[2];
+      const metric4 = records[3];
       assert.strictEqual(hashLabels(metric1.labels), '|#app:app1,core:1');
       assert.strictEqual(hashLabels(metric2.labels), '|#app:app1,core:2');
       assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
@@ -1224,14 +1221,14 @@ describe('Meter', () => {
         sum: 69,
       });
 
-      const metric5 = cpuUsageMetricRecords[0];
-      const metric6 = cpuUsageMetricRecords[1];
-      const metric7 = cpuUsageMetricRecords[2];
-      const metric8 = cpuUsageMetricRecords[3];
-      assert.strictEqual(hashLabels(metric1.labels), '|#app:app1,core:1');
-      assert.strictEqual(hashLabels(metric2.labels), '|#app:app1,core:2');
-      assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
-      assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2');
+      const metric5 = records[4];
+      const metric6 = records[5];
+      const metric7 = records[6];
+      const metric8 = records[7];
+      assert.strictEqual(hashLabels(metric5.labels), '|#app:app1,core:1');
+      assert.strictEqual(hashLabels(metric6.labels), '|#app:app1,core:2');
+      assert.strictEqual(hashLabels(metric7.labels), '|#app:app2,core:1');
+      assert.strictEqual(hashLabels(metric8.labels), '|#app:app2,core:2');
 
       ensureMetric(metric5, 'cpu_usage_per_app', {
         count: 1,