Skip to content

Commit

Permalink
feat: add unit to view instrument selection criteria (#3647)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Dyla <[email protected]>
Co-authored-by: Marc Pichler <[email protected]>
  • Loading branch information
3 people authored Mar 6, 2023
1 parent ebc8575 commit 3a2ca58
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
* perf(propagator-jaeger): improve deserializeSpanContext performance [#3541](https://github.com/open-telemetry/opentelemetry-js/pull/3541) @doochik
* feat: support TraceState in SamplingResult [#3530](https://github.com/open-telemetry/opentelemetry-js/pull/3530) @raphael-theriault-swi
* feat(sdk-trace-base): add diagnostic logging when spans are dropped [#3610](https://github.com/open-telemetry/opentelemetry-js/pull/3610) @neoeinstein
* feat: add unit to view instrument selection criteria [#3647](https://github.com/open-telemetry/opentelemetry-js/pull/3647) @jlabatut

### :bug: (Bug Fix)

Expand Down
9 changes: 8 additions & 1 deletion packages/sdk-metrics/src/view/InstrumentSelector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,23 @@
*/

import { InstrumentType } from '../InstrumentDescriptor';
import { PatternPredicate, Predicate } from './Predicate';
import { ExactPredicate, PatternPredicate, Predicate } from './Predicate';

export interface InstrumentSelectorCriteria {
name?: string;
type?: InstrumentType;
unit?: string;
}

export class InstrumentSelector {
private _nameFilter: Predicate;
private _type?: InstrumentType;
private _unitFilter: Predicate;

constructor(criteria?: InstrumentSelectorCriteria) {
this._nameFilter = new PatternPredicate(criteria?.name ?? '*');
this._type = criteria?.type;
this._unitFilter = new ExactPredicate(criteria?.unit);
}

getType() {
Expand All @@ -38,4 +41,8 @@ export class InstrumentSelector {
getNameFilter() {
return this._nameFilter;
}

getUnitFilter() {
return this._unitFilter;
}
}
2 changes: 2 additions & 0 deletions packages/sdk-metrics/src/view/RegistrationConflicts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function getTypeConflictResolutionRecipe(
const selector: InstrumentSelectorCriteria = {
name: otherDescriptor.name,
type: otherDescriptor.type,
unit: otherDescriptor.unit,
};

const selectorString = JSON.stringify(selector);
Expand All @@ -73,6 +74,7 @@ export function getDescriptionResolutionRecipe(
const selector: InstrumentSelectorCriteria = {
name: otherDescriptor.name,
type: otherDescriptor.type,
unit: otherDescriptor.unit,
};

const selectorString = JSON.stringify(selector);
Expand Down
13 changes: 13 additions & 0 deletions packages/sdk-metrics/src/view/View.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ export type ViewOptions = {
* instrumentName: 'my.instruments.requests'
*/
instrumentName?: string;
/**
* Instrument selection criteria:
* The unit of the Instrument(s).
*
* @example <caption>select all instruments with unit 'ms'</caption>
* instrumentUnit: 'ms'
*/
instrumentUnit?: string;
/**
* Instrument selection criteria:
* The name of the Meter. No wildcard support, name must match the meter exactly.
Expand Down Expand Up @@ -113,6 +121,7 @@ function isSelectorNotProvided(options: ViewOptions): boolean {
return (
options.instrumentName == null &&
options.instrumentType == null &&
options.instrumentUnit == null &&
options.meterName == null &&
options.meterVersion == null &&
options.meterSchemaUrl == null
Expand Down Expand Up @@ -161,6 +170,9 @@ export class View {
* @param viewOptions.instrumentType
* Instrument selection criteria:
* The original type of the Instrument(s).
* @param viewOptions.instrumentUnit
* Instrument selection criteria:
* The unit of the Instrument(s).
* @param viewOptions.meterName
* Instrument selection criteria:
* The name of the Meter. No wildcard support, name must match the meter exactly.
Expand Down Expand Up @@ -213,6 +225,7 @@ export class View {
this.instrumentSelector = new InstrumentSelector({
name: viewOptions.instrumentName,
type: viewOptions.instrumentType,
unit: viewOptions.instrumentUnit,
});
this.meterSelector = new MeterSelector({
name: viewOptions.meterName,
Expand Down
3 changes: 2 additions & 1 deletion packages/sdk-metrics/src/view/ViewRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export class ViewRegistry {
return (
(selector.getType() === undefined ||
instrument.type === selector.getType()) &&
selector.getNameFilter().match(instrument.name)
selector.getNameFilter().match(instrument.name) &&
selector.getUnitFilter().match(instrument.unit)
);
}

Expand Down
72 changes: 71 additions & 1 deletion packages/sdk-metrics/test/MeterProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
*/

import * as assert from 'assert';
import { MeterProvider, InstrumentType, DataPointType } from '../src';
import {
MeterProvider,
InstrumentType,
DataPointType,
ExplicitBucketHistogramAggregation,
HistogramMetricData,
} from '../src';
import {
assertScopeMetrics,
assertMetricData,
Expand Down Expand Up @@ -463,6 +469,70 @@ describe('MeterProvider', () => {
}
);
});

it('with instrument unit should apply view to only the selected instrument unit', async () => {
// Add views with different boundaries for each unit.
const msBoundaries = [0, 1, 2, 3, 4, 5];
const sBoundaries = [10, 50, 250, 1000];

const meterProvider = new MeterProvider({
resource: defaultResource,
views: [
new View({
instrumentUnit: 'ms',
aggregation: new ExplicitBucketHistogramAggregation(msBoundaries),
}),
new View({
instrumentUnit: 's',
aggregation: new ExplicitBucketHistogramAggregation(sBoundaries),
}),
],
});

const reader = new TestMetricReader();
meterProvider.addMetricReader(reader);

// Create meter and histograms, with different units.
const meter = meterProvider.getMeter('meter1', 'v1.0.0');
const histogram1 = meter.createHistogram('test-histogram-ms', {
unit: 'ms',
});
const histogram2 = meter.createHistogram('test-histogram-s', {
unit: 's',
});

// Record values for both.
histogram1.record(1);
histogram2.record(1);

// Perform collection.
const { resourceMetrics, errors } = await reader.collect();

assert.strictEqual(errors.length, 0);
// Results came only from one Meter
assert.strictEqual(resourceMetrics.scopeMetrics.length, 1);

// InstrumentationScope matches the only created Meter.
assertScopeMetrics(resourceMetrics.scopeMetrics[0], {
name: 'meter1',
version: 'v1.0.0',
});

// Two metrics are collected ('test-histogram-ms' and 'test-histogram-s')
assert.strictEqual(resourceMetrics.scopeMetrics[0].metrics.length, 2);

// Check if the boundaries are applied to the correct instrument.
assert.deepStrictEqual(
(resourceMetrics.scopeMetrics[0].metrics[0] as HistogramMetricData)
.dataPoints[0].value.buckets.boundaries,
msBoundaries
);
assert.deepStrictEqual(
(resourceMetrics.scopeMetrics[0].metrics[1] as HistogramMetricData)
.dataPoints[0].value.buckets.boundaries,
sBoundaries
);
});
});

describe('shutdown', () => {
Expand Down
44 changes: 44 additions & 0 deletions packages/sdk-metrics/test/view/ViewRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,50 @@ describe('ViewRegistry', () => {
assert.strictEqual(views[0].name, 'histogram');
}
});

it('should match view with instrument unit', () => {
const registry = new ViewRegistry();
registry.addView(
new View({
name: 'ms_view',
instrumentName: 'default_metric',
instrumentUnit: 'ms',
})
);
registry.addView(
new View({
name: 's_view',
instrumentName: 'default_metric',
instrumentUnit: 's',
})
);

{
const views = registry.findViews(
{
...defaultInstrumentDescriptor,
unit: 'ms',
},
defaultInstrumentationScope
);

assert.strictEqual(views.length, 1);
assert.strictEqual(views[0].name, 'ms_view');
}

{
const views = registry.findViews(
{
...defaultInstrumentDescriptor,
unit: 's',
},
defaultInstrumentationScope
);

assert.strictEqual(views.length, 1);
assert.strictEqual(views[0].name, 's_view');
}
});
});

describe('MeterSelector', () => {
Expand Down

0 comments on commit 3a2ca58

Please sign in to comment.