Merge branch 'wip-metrics-sdk' of github.com:dyladan/opentelemetry-js…

… into wip-metrics-sdk
open-telemetry · Nov 15, 2021 · f2d79aa · f2d79aa
2 parents 753d0f9 + 892c92b
commit f2d79aa
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 12 deletions.
diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Instruments.ts
@@ -18,6 +18,8 @@ import * as api from '@opentelemetry/api';
 import * as metrics from '@opentelemetry/api-metrics';
 import { Meter } from './Meter';
 
+// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#instrument
+
 export enum InstrumentType {
     COUNTER = 'COUNTER',
     HISTOGRAM = 'HISTOGRAM',

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Measurement.ts
@@ -17,6 +17,8 @@
 import * as api from '@opentelemetry/api'
 import { Attributes } from '@opentelemetry/api-metrics'
 
+// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#measurement
+
 export type Measurement = {
     value: number;
     // TODO use common attributes

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts
@@ -20,6 +20,8 @@ import { Counter, Histogram, UpDownCounter } from './Instruments';
 import { Measurement } from './Measurement';
 import { MeterProvider } from './MeterProvider';
 
+// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#meter
+
 export class Meter implements metrics.Meter {
     // instrumentation library required by spec to be on meter
     // spec requires provider config changes to apply to previously created meters, achieved by holding a reference to the provider

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts
@@ -75,6 +75,8 @@ export class MeterProvider {
     /**
      * Flush all buffered data and shut down the MeterProvider and all exporters and metric readers.
      * Returns a promise which is resolved when all flushes are complete.
+     * 
+     * TODO: return errors to caller somehow?
      */
     async shutdown(): Promise<void> {
         // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#shutdown
@@ -87,12 +89,24 @@ export class MeterProvider {
         // TODO add a timeout - spec leaves it up the the SDK if this is configurable
         this._shutdown = true;
 
-        await this._forceFlush();
+        // Shut down all exporters and readers.
+        // Log all Errors.
+        for (const exporter of this._metricExporters) {
+            try {
+                await exporter.shutdown();
+            } catch (e) {
+                if (e instanceof Error) {
+                    api.diag.error(`Error shutting down: ${e.message}`)
+                }
+            }
+        }
     }
 
     /**
      * Notifies all exporters and metric readers to flush any buffered data.
      * Returns a promise which is resolved when all flushes are complete.
+     * 
+     * TODO: return errors to caller somehow?
      */
     async forceFlush(): Promise<void> {
         // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#forceflush
@@ -105,21 +119,12 @@ export class MeterProvider {
             return;
         }
 
-        await this._forceFlush();
-    }
-
-    /** 
-     * A private implementation of force flush which doesn't check if the function is shut down
-     */
-    private async _forceFlush() {
-        // Shut down all exporters and readers.
-        // Catch and log all errors
         for (const exporter of [...this._metricExporters, ...this._metricReaders]) {
             try {
-                await exporter.shutdown();
+                await exporter.forceFlush();
             } catch (e) {
                 if (e instanceof Error) {
-                    api.diag.error(`Error shutting down: ${e.message}`)
+                    api.diag.error(`Error flushing: ${e.message}`)
                 }
             }
         }

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricExporter.ts
@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricexporter
 
 // TODO should this just be an interface and exporters can implement their own shutdown?
 export abstract class MetricExporter {

diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MetricReader.ts
@@ -16,6 +16,8 @@
 
 import { MetricExporter } from '.';
 
+// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#metricreader
+
 export class MetricReader {
     private _shutdown = false;