diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts index b47da6e835..a9a3beb556 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/Meter.ts @@ -34,7 +34,9 @@ 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 - constructor(private _meterProviderSharedState: MeterProviderSharedState, private _instrumentationLibrary: InstrumentationLibrary) { } + constructor(private _meterProviderSharedState: MeterProviderSharedState, private _instrumentationLibrary: InstrumentationLibrary) { + this._meterProviderSharedState.meters.push(this); + } /** this exists just to prevent ts errors from unused variables and may be removed */ getInstrumentationLibrary(): InstrumentationLibrary { diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 62c8892cbe..5a53453303 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -35,8 +35,8 @@ export class MeterProvider { private _sharedState: MeterProviderSharedState; private _shutdown = false; - constructor(options: MeterProviderOptions) { - this._sharedState = new MeterProviderSharedState(options.resource ?? Resource.empty()); + constructor(options?: MeterProviderOptions) { + this._sharedState = new MeterProviderSharedState(options?.resource ?? Resource.empty()); } getMeter(name: string, version = '', options: metrics.MeterOptions = {}): metrics.Meter { @@ -46,10 +46,6 @@ export class MeterProvider { return metrics.NOOP_METER; } - // Spec leaves it unspecified if creating a meter with duplicate - // name/version returns the same meter. We create a new one here - // for simplicity. This may change in the future. - // TODO: consider returning the same meter if the same name/version is used return new Meter(this._sharedState, { name, version, schemaUrl: options.schemaUrl }); } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts index 2f59eade1f..f6d81a82f9 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MeterProviderSharedState.ts @@ -30,7 +30,7 @@ export class MeterProviderSharedState { metricCollectors: MetricCollector[] = []; - meters: Map = new Map(); + meters: Meter[] = []; constructor(public resource: Resource) {} } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts index 3841354e4b..46fc576f50 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricCollector.ts @@ -34,7 +34,7 @@ export class MetricCollector implements MetricProducer { async collect(): Promise { const collectionTime = hrTime(); - const results = await Promise.all(Array.from(this._sharedState.meters.values()) + const results = await Promise.all(this._sharedState.meters .map(meter => meter.collect(this, collectionTime))); return results.reduce((cumulation, current) => cumulation.concat(current), []); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts new file mode 100644 index 0000000000..a2c7802b87 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts @@ -0,0 +1,50 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import { NOOP_METER } from '@opentelemetry/api-metrics-wip'; +import { Meter } from '../src/Meter'; +import { MeterProvider } from '../src/MeterProvider'; +import { defaultResource } from './util'; + +describe('MeterProvider', () => { + describe('constructor', () => { + it('should construct without exceptions', () => { + const meterProvider = new MeterProvider(); + assert(meterProvider instanceof MeterProvider); + }); + + it('construct with resource', () => { + const meterProvider = new MeterProvider({ resource: defaultResource }); + assert(meterProvider instanceof MeterProvider); + }); + }); + + describe('getMeter', () => { + it('should get a meter', () => { + const meterProvider = new MeterProvider(); + const meter = meterProvider.getMeter('meter1', '1.0.0'); + assert(meter instanceof Meter); + }); + + it('get a noop meter on shutdown', () => { + const meterProvider = new MeterProvider(); + meterProvider.shutdown(); + const meter = meterProvider.getMeter('meter1', '1.0.0'); + assert.strictEqual(meter, NOOP_METER); + }); + }); +}); diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts index 02c076e675..172301d958 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/export/TestMetricReader.ts @@ -15,6 +15,7 @@ */ import { MetricReader } from '../../src'; +import { MetricCollector } from '../../src/state/MetricCollector'; /** * A test metric reader that implements no-op onForceFlush() and onShutdown() handlers. @@ -27,4 +28,8 @@ export class TestMetricReader extends MetricReader { protected onShutdown(): Promise { return Promise.resolve(undefined); } + + getMetricCollector(): MetricCollector { + return this['_metricProducer'] as MetricCollector; + } } diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts index ce94c080cb..4aba1f7146 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/state/MetricCollector.test.ts @@ -16,10 +16,10 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; +import { MeterProvider } from '../../src'; import { AggregationTemporality } from '../../src/export/AggregationTemporality'; import { MetricData, PointDataType } from '../../src/export/MetricData'; import { MetricExporter } from '../../src/export/MetricExporter'; -import { Meter } from '../../src/Meter'; import { MeterProviderSharedState } from '../../src/state/MeterProviderSharedState'; import { MetricCollector } from '../../src/state/MetricCollector'; import { defaultInstrumentationLibrary, defaultResource, assertMetricData, assertPointData } from '../util'; @@ -64,15 +64,15 @@ describe('MetricCollector', () => { describe('collect', () => { function setupInstruments(exporter: MetricExporter) { - // TODO(legendecas): setup with MeterProvider when meter identity was settled. - const meterProviderSharedState = new MeterProviderSharedState(defaultResource); + const meterProvider = new MeterProvider({ resource: defaultResource }); const reader = new TestMetricReader(exporter.getPreferredAggregationTemporality()); - const metricCollector = new MetricCollector(meterProviderSharedState, reader); - meterProviderSharedState.metricCollectors.push(metricCollector); + meterProvider.addMetricReader(reader); + const metricCollector = reader.getMetricCollector(); - const meter = new Meter(meterProviderSharedState, defaultInstrumentationLibrary); - meterProviderSharedState.meters.set('test-meter', meter); + const meter = meterProvider.getMeter(defaultInstrumentationLibrary.name, defaultInstrumentationLibrary.version, { + schemaUrl: defaultInstrumentationLibrary.schemaUrl, + }); return { metricCollector, meter }; }