From a44657ea9677ae4f08e008ca82c1b8f447ee48a4 Mon Sep 17 00:00:00 2001 From: "marc.pichler" Date: Mon, 4 Jul 2022 15:19:35 +0200 Subject: [PATCH] feature(views): introduce UserView class. --- .../src/MeterProvider.ts | 51 ++----------- .../src/view/UserView.ts | 59 ++++++++++++++ .../src/view/ViewRegistry.ts | 32 ++++---- .../test/MeterProvider.test.ts | 10 --- .../test/view/ViewRegistry.test.ts | 76 +++++++++---------- 5 files changed, 116 insertions(+), 112 deletions(-) create mode 100644 experimental/packages/opentelemetry-sdk-metrics-base/src/view/UserView.ts diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts index 50d51e48cd..e7c3431f7d 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts @@ -19,15 +19,11 @@ import * as metrics from '@opentelemetry/api-metrics'; import { Resource } from '@opentelemetry/resources'; import { MetricReader } from './export/MetricReader'; import { MeterProviderSharedState } from './state/MeterProviderSharedState'; -import { InstrumentSelector } from './view/InstrumentSelector'; -import { MeterSelector } from './view/MeterSelector'; -import { View } from './view/View'; import { MetricCollector } from './state/MetricCollector'; import { Aggregation } from './view/Aggregation'; -import { FilteringAttributesProcessor } from './view/AttributesProcessor'; import { InstrumentType } from './InstrumentDescriptor'; -import { PatternPredicate } from './view/Predicate'; import { ForceFlushOptions, ShutdownOptions } from './types'; +import { UserView } from './view/UserView'; /** * MeterProviderOptions provides an interface for configuring a MeterProvider. @@ -92,13 +88,6 @@ export type SelectorOptions = { } }; -function isViewOptionsEmpty(options: ViewOptions): boolean { - return (options.name == null && - options.aggregation == null && - options.attributeKeys == null && - options.description == null); -} - /** * This class implements the {@link metrics.MeterProvider} interface. */ @@ -109,8 +98,8 @@ export class MeterProvider implements metrics.MeterProvider { constructor(options?: MeterProviderOptions) { this._sharedState = new MeterProviderSharedState(options?.resource ?? Resource.empty()); if(options?.views != null && options.views.length > 0){ - for(const view of options.views){ - this.addView(view); + for(const viewOptions of options.views){ + this.addView(new UserView(viewOptions.view, viewOptions.selector)); } } } @@ -142,38 +131,8 @@ export class MeterProvider implements metrics.MeterProvider { this._sharedState.metricCollectors.push(collector); } - private addView(registrationOptions: ViewRegistrationOptions) { - const viewOptions = registrationOptions.view; - const selectorOptions = registrationOptions.selector; - - if (isViewOptionsEmpty(viewOptions)) { - throw new Error('Cannot create view with no view arguments supplied'); - } - - // the SDK SHOULD NOT allow Views with a specified name to be declared with instrument selectors that - // may select more than one instrument (e.g. wild card instrument name) in the same Meter. - if (viewOptions.name != null && - (selectorOptions?.instrument?.name == null || - PatternPredicate.hasWildcard(selectorOptions.instrument.name))) { - throw new Error('Views with a specified name must be declared with an instrument selector that selects at most one instrument per meter.'); - } - - // Create AttributesProcessor if attributeKeys are defined set. - let attributesProcessor = undefined; - if (viewOptions.attributeKeys != null) { - attributesProcessor = new FilteringAttributesProcessor(viewOptions.attributeKeys); - } - - const view = new View({ - name: viewOptions.name, - description: viewOptions.description, - aggregation: viewOptions.aggregation, - attributesProcessor: attributesProcessor - }); - const instrument = new InstrumentSelector(selectorOptions?.instrument); - const meter = new MeterSelector(selectorOptions?.meter); - - this._sharedState.viewRegistry.addView(view, instrument, meter); + private addView(view: UserView) { + this._sharedState.viewRegistry.addView(view); } /** diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/view/UserView.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/UserView.ts new file mode 100644 index 0000000000..6e62b55af9 --- /dev/null +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/UserView.ts @@ -0,0 +1,59 @@ +/* + * 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 { PatternPredicate } from './Predicate'; +import { AttributesProcessor, FilteringAttributesProcessor } from './AttributesProcessor'; +import { InstrumentSelector } from './InstrumentSelector'; +import { MeterSelector } from './MeterSelector'; +import { SelectorOptions, ViewOptions } from '../MeterProvider'; +import { Aggregation } from './Aggregation'; + +export class UserView { + readonly name?: string; + readonly description?: string; + readonly aggregation: Aggregation; + readonly attributesProcessor: AttributesProcessor; + readonly instrumentSelector: InstrumentSelector; + readonly meterSelector: MeterSelector; + + constructor(viewOptions: ViewOptions, selectorOptions?: SelectorOptions) { + // TODO: spec says that: If no criteria is provided, the SDK SHOULD treat it as an error. This is regearding to selectorOptions, not viewOptions. + /*if (isViewOptionsEmpty(viewOptions)) { + throw new Error('Cannot create view with no view arguments supplied'); + }*/ + + // the SDK SHOULD NOT allow Views with a specified name to be declared with instrument selectors that + // may select more than one instrument (e.g. wild card instrument name) in the same Meter. + if (viewOptions.name != null && + (selectorOptions?.instrument?.name == null || + PatternPredicate.hasWildcard(selectorOptions.instrument.name))) { + throw new Error('Views with a specified name must be declared with an instrument selector that selects at most one instrument per meter.'); + } + + // Create AttributesProcessor if attributeKeys are defined set. + if (viewOptions.attributeKeys != null) { + this.attributesProcessor = new FilteringAttributesProcessor(viewOptions.attributeKeys); + } else { + this.attributesProcessor = AttributesProcessor.Noop(); + } + + this.name = viewOptions.name; + this.description = viewOptions.description; + this.aggregation = viewOptions.aggregation ?? Aggregation.Default(); + this.instrumentSelector = new InstrumentSelector(selectorOptions?.instrument); + this.meterSelector = new MeterSelector(selectorOptions?.meter); + } +} diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts index 40fb5ebc51..6cb0ca4716 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/src/view/ViewRegistry.ts @@ -18,33 +18,29 @@ import { InstrumentationScope } from '@opentelemetry/core'; import { InstrumentDescriptor } from '../InstrumentDescriptor'; import { InstrumentSelector } from './InstrumentSelector'; import { MeterSelector } from './MeterSelector'; -import { View } from './View'; - -interface RegisteredView { - instrumentSelector: InstrumentSelector; - meterSelector: MeterSelector; - view: View; -} +import { UserView } from './UserView'; export class ViewRegistry { - private static DEFAULT_VIEW = new View(); - private _registeredViews: RegisteredView[] = []; + private static DEFAULT_VIEW = new UserView({}, { + instrument: { + name: '*' + }, + meter: { + name: '*' + } + }); + private _registeredViews: UserView[] = []; - addView(view: View, instrumentSelector: InstrumentSelector = new InstrumentSelector(), meterSelector: MeterSelector = new MeterSelector()) { - this._registeredViews.push({ - instrumentSelector, - meterSelector, - view, - }); + addView(view: UserView) { + this._registeredViews.push(view); } - findViews(instrument: InstrumentDescriptor, meter: InstrumentationScope): View[] { + findViews(instrument: InstrumentDescriptor, meter: InstrumentationScope): UserView[] { const views = this._registeredViews .filter(registeredView => { return this._matchInstrument(registeredView.instrumentSelector, instrument) && this._matchMeter(registeredView.meterSelector, meter); - }) - .map(it => it.view); + }); if (views.length === 0) { return [ViewRegistry.DEFAULT_VIEW]; diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts index 999f3d83dd..80c5eb041a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts @@ -176,16 +176,6 @@ describe('MeterProvider', () => { })); }); - it('with no view parameters should throw', () => { - assert.throws(() => new MeterProvider({ - resource: defaultResource, views: [ - { - view: {} - } - ] - })); - }); - it('with existing instrument should rename', async () => { const meterProvider = new MeterProvider({ resource: defaultResource, diff --git a/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts b/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts index 3e8aaaa1a2..102d63314a 100644 --- a/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts +++ b/experimental/packages/opentelemetry-sdk-metrics-base/test/view/ViewRegistry.test.ts @@ -15,12 +15,10 @@ */ import * as assert from 'assert'; -import { InstrumentType } from '../../src/InstrumentDescriptor'; +import { InstrumentType } from '../../src'; import { ViewRegistry } from '../../src/view/ViewRegistry'; -import { View } from '../../src/view/View'; -import { InstrumentSelector } from '../../src/view/InstrumentSelector'; -import { MeterSelector } from '../../src/view/MeterSelector'; import { defaultInstrumentationScope, defaultInstrumentDescriptor } from '../util'; +import { UserView } from '../../src/view/UserView'; describe('ViewRegistry', () => { @@ -35,12 +33,15 @@ describe('ViewRegistry', () => { describe('InstrumentSelector', () => { it('should match view with instrument name', () => { const registry = new ViewRegistry(); - registry.addView(new View({ name: 'no-filter' })); - registry.addView(new View({ name: 'foo' }), new InstrumentSelector({ - name: 'foo', + registry.addView(new UserView({ name: 'foo' }, { + instrument: { + name: 'foo', + } })); - registry.addView(new View({ name: 'bar' }), new InstrumentSelector({ - name: 'bar' + registry.addView(new UserView({ name: 'bar' }, { + instrument: { + name: 'bar', + } })); { @@ -49,9 +50,8 @@ describe('ViewRegistry', () => { name: 'foo' }, defaultInstrumentationScope); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'foo'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'foo'); } { @@ -60,20 +60,24 @@ describe('ViewRegistry', () => { name: 'bar' }, defaultInstrumentationScope); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'bar'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'bar'); } }); it('should match view with instrument type', () => { const registry = new ViewRegistry(); - registry.addView(new View({ name: 'no-filter' })); - registry.addView(new View({ name: 'counter' }), new InstrumentSelector({ - type: InstrumentType.COUNTER, + registry.addView(new UserView({ name: 'counter' }, { + instrument: { + name: 'default_metric', + type: InstrumentType.COUNTER + } })); - registry.addView(new View({ name: 'histogram' }), new InstrumentSelector({ - type: InstrumentType.HISTOGRAM, + registry.addView(new UserView({ name: 'histogram' }, { + instrument: { + name: 'default_metric', + type: InstrumentType.HISTOGRAM + } })); { @@ -82,9 +86,8 @@ describe('ViewRegistry', () => { type: InstrumentType.COUNTER }, defaultInstrumentationScope); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'counter'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'counter'); } { @@ -93,9 +96,8 @@ describe('ViewRegistry', () => { type: InstrumentType.HISTOGRAM }, defaultInstrumentationScope); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'histogram'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'histogram'); } }); }); @@ -103,23 +105,22 @@ describe('ViewRegistry', () => { describe('MeterSelector', () => { it('should match view with meter name', () => { const registry = new ViewRegistry(); - registry.addView(new View({ name: 'no-filter' })); - registry.addView(new View({ name: 'foo' }), undefined, new MeterSelector({ - name: 'foo' + registry.addView(new UserView({ name: 'foo' }, { + instrument: { name: 'default_metric' }, + meter: { name: 'foo' } })); - registry.addView(new View({ name: 'bar' }), undefined, new MeterSelector({ - name: 'bar' + registry.addView(new UserView({ name: 'bar' }, { + instrument: { name: 'default_metric' }, + meter: { name: 'bar' } })); - { const views = registry.findViews(defaultInstrumentDescriptor, { ...defaultInstrumentationScope, name: 'foo', }); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'foo'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'foo'); } { @@ -128,9 +129,8 @@ describe('ViewRegistry', () => { name: 'bar' }); - assert.strictEqual(views.length, 2); - assert.strictEqual(views[0].name, 'no-filter'); - assert.strictEqual(views[1].name, 'bar'); + assert.strictEqual(views.length, 1); + assert.strictEqual(views[0].name, 'bar'); } }); });