diff --git a/CHANGELOG.md b/CHANGELOG.md index e9af77be8b..7a0b731305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/packages/sdk-metrics/src/view/InstrumentSelector.ts b/packages/sdk-metrics/src/view/InstrumentSelector.ts index e7bec3a44a..9b8006267f 100644 --- a/packages/sdk-metrics/src/view/InstrumentSelector.ts +++ b/packages/sdk-metrics/src/view/InstrumentSelector.ts @@ -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() { @@ -38,4 +41,8 @@ export class InstrumentSelector { getNameFilter() { return this._nameFilter; } + + getUnitFilter() { + return this._unitFilter; + } } diff --git a/packages/sdk-metrics/src/view/RegistrationConflicts.ts b/packages/sdk-metrics/src/view/RegistrationConflicts.ts index e74add7334..dbb8073c8b 100644 --- a/packages/sdk-metrics/src/view/RegistrationConflicts.ts +++ b/packages/sdk-metrics/src/view/RegistrationConflicts.ts @@ -59,6 +59,7 @@ export function getTypeConflictResolutionRecipe( const selector: InstrumentSelectorCriteria = { name: otherDescriptor.name, type: otherDescriptor.type, + unit: otherDescriptor.unit, }; const selectorString = JSON.stringify(selector); @@ -73,6 +74,7 @@ export function getDescriptionResolutionRecipe( const selector: InstrumentSelectorCriteria = { name: otherDescriptor.name, type: otherDescriptor.type, + unit: otherDescriptor.unit, }; const selectorString = JSON.stringify(selector); diff --git a/packages/sdk-metrics/src/view/View.ts b/packages/sdk-metrics/src/view/View.ts index 398f936536..1e8d4fb0e0 100644 --- a/packages/sdk-metrics/src/view/View.ts +++ b/packages/sdk-metrics/src/view/View.ts @@ -83,6 +83,14 @@ export type ViewOptions = { * instrumentName: 'my.instruments.requests' */ instrumentName?: string; + /** + * Instrument selection criteria: + * The unit of the Instrument(s). + * + * @example select all instruments with unit 'ms' + * instrumentUnit: 'ms' + */ + instrumentUnit?: string; /** * Instrument selection criteria: * The name of the Meter. No wildcard support, name must match the meter exactly. @@ -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 @@ -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. @@ -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, diff --git a/packages/sdk-metrics/src/view/ViewRegistry.ts b/packages/sdk-metrics/src/view/ViewRegistry.ts index 265f699bf9..5f4f367e92 100644 --- a/packages/sdk-metrics/src/view/ViewRegistry.ts +++ b/packages/sdk-metrics/src/view/ViewRegistry.ts @@ -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) ); } diff --git a/packages/sdk-metrics/test/MeterProvider.test.ts b/packages/sdk-metrics/test/MeterProvider.test.ts index 48055695ed..a9f8361b81 100644 --- a/packages/sdk-metrics/test/MeterProvider.test.ts +++ b/packages/sdk-metrics/test/MeterProvider.test.ts @@ -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, @@ -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', () => { diff --git a/packages/sdk-metrics/test/view/ViewRegistry.test.ts b/packages/sdk-metrics/test/view/ViewRegistry.test.ts index aea2ebe3cf..b766c0fda5 100644 --- a/packages/sdk-metrics/test/view/ViewRegistry.test.ts +++ b/packages/sdk-metrics/test/view/ViewRegistry.test.ts @@ -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', () => {