Skip to content

Commit

Permalink
fixup!
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Apr 18, 2022
1 parent af786dd commit 3508a7f
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 22 deletions.
2 changes: 1 addition & 1 deletion experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ All notable changes to experimental packages in this project will be documented
* feat(instrumentation-xhr): add applyCustomAttributesOnSpan hook #2134 @mhennoch
* feat(proto): add @opentelemetry/otlp-transformer package with hand-rolled transformation #2746 @dyladan
* feat(sdk-metrics-base): shutdown and forceflush on MeterProvider #2890 @legendecas
* feat(sdk-metrics-base): meter identity #2901 @legendecas
* feat(sdk-metrics-base): return the same meter for identical input to getMeter #2901 @legendecas

### :bug: (Bug Fix)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,15 @@
*/

import * as metrics from '@opentelemetry/api-metrics';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { createInstrumentDescriptor, InstrumentType } from './InstrumentDescriptor';
import { CounterInstrument, HistogramInstrument, UpDownCounterInstrument } from './Instruments';
import { MeterProviderSharedState } from './state/MeterProviderSharedState';
import { MeterSharedState } from './state/MeterSharedState';

/**
* This class implements the {@link metrics.Meter} interface.
*/
export class Meter implements metrics.Meter {
private _meterSharedState: MeterSharedState;

constructor(meterProviderSharedState: MeterProviderSharedState, instrumentationLibrary: InstrumentationLibrary) {
this._meterSharedState = meterProviderSharedState.getMeterSharedState(instrumentationLibrary);
}
constructor(private _meterSharedState: MeterSharedState) {}

/**
* Create a {@link metrics.Histogram} instrument.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import * as api from '@opentelemetry/api';
import * as metrics from '@opentelemetry/api-metrics';
import { Resource } from '@opentelemetry/resources';
import { Meter } from './Meter';
import { MetricReader } from './export/MetricReader';
import { MeterProviderSharedState } from './state/MeterProviderSharedState';
import { InstrumentSelector } from './view/InstrumentSelector';
Expand Down Expand Up @@ -115,7 +114,9 @@ export class MeterProvider implements metrics.MeterProvider {
return metrics.NOOP_METER;
}

return new Meter(this._sharedState, { name, version, schemaUrl: options.schemaUrl });
return this._sharedState
.getMeterSharedState({ name, version, schemaUrl: options.schemaUrl })
.meter;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import { HrTime } from '@opentelemetry/api';
import { hrTime, InstrumentationLibrary } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { instrumentationLibraryId } from '../utils';
import { ViewRegistry } from '../view/ViewRegistry';
import { MeterSharedState } from './MeterSharedState';
import { MetricCollector } from './MetricCollector';
Expand All @@ -35,16 +36,12 @@ export class MeterProviderSharedState {
constructor(public resource: Resource) {}

getMeterSharedState(instrumentationLibrary: InstrumentationLibrary) {
const id = this.instrumentationLibraryId(instrumentationLibrary);
const id = instrumentationLibraryId(instrumentationLibrary);
let meterSharedState = this.meterSharedStates.get(id);
if (meterSharedState == null) {
meterSharedState = new MeterSharedState(this, instrumentationLibrary);
this.meterSharedStates.set(id, meterSharedState);
}
return meterSharedState;
}

instrumentationLibraryId(instrumentationLibrary: InstrumentationLibrary) {
return `${instrumentationLibrary.name}:${instrumentationLibrary.version ?? ''}:${instrumentationLibrary.schemaUrl ?? ''}`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as metrics from '@opentelemetry/api-metrics';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { InstrumentationLibraryMetrics } from '../export/MetricData';
import { createInstrumentDescriptorWithView, InstrumentDescriptor } from '../InstrumentDescriptor';
import { Meter } from '../Meter';
import { isNotNullish } from '../utils';
import { AsyncMetricStorage } from './AsyncMetricStorage';
import { MeterProviderSharedState } from './MeterProviderSharedState';
Expand All @@ -32,8 +33,11 @@ import { SyncMetricStorage } from './SyncMetricStorage';
*/
export class MeterSharedState {
private _metricStorageRegistry = new MetricStorageRegistry();
meter: Meter;

constructor(private _meterProviderSharedState: MeterProviderSharedState, private _instrumentationLibrary: InstrumentationLibrary) {}
constructor(private _meterProviderSharedState: MeterProviderSharedState, private _instrumentationLibrary: InstrumentationLibrary) {
this.meter = new Meter(this);
}

registerMetricStorage(descriptor: InstrumentDescriptor) {
const views = this._meterProviderSharedState.viewRegistry.findViews(descriptor, this._instrumentationLibrary);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { MetricAttributes } from '@opentelemetry/api-metrics';
import { InstrumentationLibrary } from '@opentelemetry/core';

export type Maybe<T> = T | undefined;

Expand All @@ -39,6 +40,14 @@ export function hashAttributes(attributes: MetricAttributes): string {
}, '|#');
}

/**
* Converting the instrumentation library object to a unique identifier string.
* @param instrumentationLibrary
*/
export function instrumentationLibraryId(instrumentationLibrary: InstrumentationLibrary): string {
return `${instrumentationLibrary.name}:${instrumentationLibrary.version ?? ''}:${instrumentationLibrary.schemaUrl ?? ''}`;
}

/**
* Error that is thrown on timeouts.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,71 @@ import * as assert from 'assert';
import { CounterInstrument, HistogramInstrument, UpDownCounterInstrument } from '../src/Instruments';
import { Meter } from '../src/Meter';
import { MeterProviderSharedState } from '../src/state/MeterProviderSharedState';
import { MeterSharedState } from '../src/state/MeterSharedState';
import { defaultInstrumentationLibrary, defaultResource } from './util';

const noopObservableCallback: ObservableCallback = _observableResult => {};

describe('Meter', () => {
describe('createCounter', () => {
it('should create counter', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
const counter = meter.createCounter('foobar');
assert(counter instanceof CounterInstrument);
});
});

describe('createUpDownCounter', () => {
it('should create up down counter', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
const counter = meter.createUpDownCounter('foobar');
assert(counter instanceof UpDownCounterInstrument);
});
});

describe('createHistogram', () => {
it('should create histogram', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
const counter = meter.createHistogram('foobar');
assert(counter instanceof HistogramInstrument);
});
});

describe('createObservableGauge', () => {
it('should create observable gauge', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
meter.createObservableGauge('foobar', noopObservableCallback);
});
});

describe('createObservableCounter', () => {
it('should create observable counter', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
meter.createObservableCounter('foobar', noopObservableCallback);
});
});

describe('createObservableUpDownCounter', () => {
it('should create observable up-down-counter', () => {
const meter = new Meter(new MeterProviderSharedState(defaultResource), defaultInstrumentationLibrary);
const meterSharedState = new MeterSharedState(
new MeterProviderSharedState(defaultResource),
defaultInstrumentationLibrary);
const meter = new Meter(meterSharedState);
meter.createObservableUpDownCounter('foobar', noopObservableCallback);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ describe('MeterProvider', () => {
assert(meter instanceof Meter);
});

it('should get an identical meter on duplicated calls', () => {
const meterProvider = new MeterProvider();
const meter1 = meterProvider.getMeter('meter1', '1.0.0');
const meter2 = meterProvider.getMeter('meter1', '1.0.0');
assert.strictEqual(meter1, meter2);
});

it('get a noop meter on shutdown', () => {
const meterProvider = new MeterProvider();
meterProvider.shutdown();
Expand Down

0 comments on commit 3508a7f

Please sign in to comment.