Skip to content

Commit

Permalink
fixup!
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Dec 17, 2021
1 parent 340c719 commit f64ef47
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,12 @@ export class MeterProvider {
api.diag.warn('A shutdown MeterProvider cannot provide a Meter')
return metrics.NOOP_METER;
}
// TODO: add schemaUrl to id?
const id = `${name}#${version}`;
if (this._sharedState.meters.has(id)) {
return this._sharedState.meters.get(id)!;
}

// 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.
const meter = new Meter(this._sharedState, { name, version, schemaUrl: options.schemaUrl });
this._sharedState.meters.set(id, meter);
return meter;
// TODO: consider returning the same meter if the same name/version is used
return new Meter(this._sharedState, { name, version, schemaUrl: options.schemaUrl });
}

addMetricReader(metricReader: MetricReader) {
Expand All @@ -71,7 +65,8 @@ export class MeterProvider {
}

/**
* Flush all buffered data and shut down the MeterProvider and all metric readers.
* Flush all buffered data and shut down the MeterProvider and all registered
* MetricReaders.
* Returns a promise which is resolved when all flushes are complete.
*
* TODO: return errors to caller somehow?
Expand All @@ -87,12 +82,11 @@ export class MeterProvider {
// TODO add a timeout - spec leaves it up the the SDK if this is configurable
this._shutdown = true;

// Shut down all readers.
// Log all Errors.
for (const collector of this._sharedState.metricCollectors) {
try {
await collector.metricReader.shutdown();
await collector.shutdown();
} catch (e) {
// Log all Errors.
if (e instanceof Error) {
api.diag.error(`Error shutting down: ${e.message}`)
}
Expand All @@ -101,7 +95,7 @@ export class MeterProvider {
}

/**
* Notifies all exporters and metric readers to flush any buffered data.
* Notifies all registered MetricReaders to flush any buffered data.
* Returns a promise which is resolved when all flushes are complete.
*
* TODO: return errors to caller somehow?
Expand All @@ -119,8 +113,9 @@ export class MeterProvider {

for (const collector of this._sharedState.metricCollectors) {
try {
await collector.metricReader.forceFlush();
await collector.forceFlush();
} catch (e) {
// Log all Errors.
if (e instanceof Error) {
api.diag.error(`Error flushing: ${e.message}`)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import { MeterProviderSharedState } from './MeterProviderSharedState';
*/
export class MetricCollector implements MetricProducer {
public readonly aggregatorTemporality: AggregationTemporality;
constructor(private _sharedState: MeterProviderSharedState, public metricReader: MetricReader) {
this.aggregatorTemporality = this.metricReader.getPreferredAggregationTemporality();
constructor(private _sharedState: MeterProviderSharedState, private _metricReader: MetricReader) {
this.aggregatorTemporality = this._metricReader.getPreferredAggregationTemporality();
}

async collect(): Promise<MetricData[]> {
Expand All @@ -39,6 +39,20 @@ export class MetricCollector implements MetricProducer {

return results.reduce((cumulation, current) => cumulation.concat(current), []);
}

/**
* Delegates for MetricReader.forceFlush.
*/
async forceFlush(): Promise<void> {
return this._metricReader.forceFlush();
}

/**
* Delegates for MetricReader.shutdown.
*/
async shutdown(): Promise<void> {
return this._metricReader.shutdown();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export class TemporalMetricProcessor<T> {
constructor(private _aggregator: Aggregator<T>) {}

/**
* Builds the {@link MetricData} streams to report against a specific metric reader.
* @param collector The information of the metric reader.
* Builds the {@link MetricData} streams to report against a specific MetricCollector.
* @param collector The information of the MetricCollector.
* @param collectors The registered collectors.
* @param resource The resource to attach these metrics against.
* @param instrumentationLibrary The instrumentation library that generated these metrics.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

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 { MetricReader } from '../../src/export/MetricReader';
import { Meter } from '../../src/Meter';
import { MeterProviderSharedState } from '../../src/state/MeterProviderSharedState';
import { MetricCollector } from '../../src/state/MetricCollector';
import { defaultInstrumentationLibrary, defaultResource, assertMetricData, assertPointData } from '../util';
Expand Down Expand Up @@ -64,21 +64,21 @@ describe('MetricCollector', () => {
const metricCollector = new MetricCollector(meterProviderSharedState, reader);

assert.strictEqual(metricCollector.aggregatorTemporality, exporter.getPreferredAggregationTemporality());
assert.strictEqual(metricCollector.metricReader, reader);
}
});
});

describe('collect', () => {
function setupInstruments(exporter: MetricExporter) {
const meterProvider = new MeterProvider({ resource: defaultResource });
// TODO(legendecas): setup with MeterProvider when meter identity was settled.
const meterProviderSharedState = new MeterProviderSharedState(defaultResource);

const reader = new TestMetricReader(exporter);
meterProvider.addMetricReader(reader);
const metricCollector = reader.getMetricCollector();
const metricCollector = new MetricCollector(meterProviderSharedState, reader);
meterProviderSharedState.metricCollectors.push(metricCollector);

const meter = meterProvider.getMeter(defaultInstrumentationLibrary.name, defaultInstrumentationLibrary.version, {
schemaUrl: defaultInstrumentationLibrary.schemaUrl,
});
const meter = new Meter(meterProviderSharedState, defaultInstrumentationLibrary);
meterProviderSharedState.meters.set('test-meter', meter);

return { metricCollector, meter };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('SyncMetricStorage', () => {
});
});

describe('collectAndReset', () => {
describe('collect', () => {
describe('Delta Collector', () => {
const collectors = [deltaCollector];
it('should collect and reset memos', async () => {
Expand Down

0 comments on commit f64ef47

Please sign in to comment.