feat(views): Update addView() to disallow named views that select more than one instrument. (#2820)

experimental/packages/opentelemetry-sdk-metrics-base/src/MeterProvider.ts

@@ -24,6 +24,10 @@ 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';
 
 /**
  * MeterProviderOptions provides an interface for configuring a MeterProvider.
@@ -33,6 +37,62 @@ export interface MeterProviderOptions {
   resource?: Resource;
 }
 
+export type ViewOptions = {
+  /**
+   *  If not provided, the Instrument name will be used by default. This will be used as the name of the metrics stream.
+   */
+  name?: string,
+  /**
+   * If not provided, the Instrument description will be used by default.
+   */
+  description?: string,
+  /**
+   * If provided, the attributes that are not in the list will be ignored.
+   * If not provided, all the attribute keys will be used by default.
+   */
+  attributeKeys?: string[],
+  /**
+   * The {@link Aggregation} aggregation to be used.
+   */
+  aggregation?: Aggregation,
+
+  // TODO: Add ExemplarReservoir
+};
+
+export type SelectorOptions = {
+  instrument?: {
+    /**
+     * The type of the Instrument(s).
+     */
+    type?: InstrumentType,
+    /**
+     * Name of the Instrument(s) with wildcard support.
+     */
+    name?: string,
+  }
+  meter?: {
+    /**
+     * The name of the Meter.
+     */
+    name?: string;
+    /**
+     * The version of the Meter.
+     */
+    version?: string;
+    /**
+     * The schema URL of the Meter.
+     */
+    schemaUrl?: string;
+  }
+};
+
+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.
  */
@@ -50,8 +110,8 @@ export class MeterProvider implements metrics.MeterProvider {
   getMeter(name: string, version = '', options: metrics.MeterOptions = {}): metrics.Meter {
     // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#meter-creation
     if (this._shutdown) {
-        api.diag.warn('A shutdown MeterProvider cannot provide a Meter');
-        return metrics.NOOP_METER;
+      api.diag.warn('A shutdown MeterProvider cannot provide a Meter');
+      return metrics.NOOP_METER;
     }
 
     return new Meter(this._sharedState, { name, version, schemaUrl: options.schemaUrl });
@@ -69,9 +129,35 @@ export class MeterProvider implements metrics.MeterProvider {
     this._sharedState.metricCollectors.push(collector);
   }
 
-  addView(view: View, instrumentSelector: InstrumentSelector, meterSelector: MeterSelector) {
-    // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view
-    this._sharedState.viewRegistry.addView(view, instrumentSelector, meterSelector);
+  addView(options: ViewOptions, selectorOptions?: SelectorOptions) {
+    if (isViewOptionsEmpty(options)) {
+      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 (options.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 (options.attributeKeys != null) {
+      attributesProcessor = new FilteringAttributesProcessor(options.attributeKeys);
+    }
+
+    const view = new View({
+      name: options.name,
+      description: options.description,
+      aggregation: options.aggregation,
+      attributesProcessor: attributesProcessor
+    });
+    const instrument = new InstrumentSelector(selectorOptions?.instrument);
+    const meter = new MeterSelector(selectorOptions?.meter);
+
+    this._sharedState.viewRegistry.addView(view, instrument, meter);
   }
 
   /**

experimental/packages/opentelemetry-sdk-metrics-base/src/index.ts

@@ -16,6 +16,7 @@
 
 export * from './export/AggregationTemporality';
 export * from './export/MetricData';
+export { MeterProvider, MeterProviderOptions } from './MeterProvider';
 export * from './export/MetricExporter';
 export * from './export/MetricProducer';
 export * from './export/MetricReader';
@@ -26,3 +27,4 @@ export * from './Meter';
 export * from './MeterProvider';
 export * from './ObservableResult';
 export { TimeoutError } from './utils';
+export { FilteringAttributesProcessor } from './view/AttributesProcessor';

experimental/packages/opentelemetry-sdk-metrics-base/src/state/MetricStorageRegistry.ts

@@ -26,6 +26,10 @@ import { getConflictResolutionRecipe, getIncompatibilityDetails } from '../view/
 export class MetricStorageRegistry {
   private readonly _metricStorageRegistry = new Map<string, MetricStorage[]>();
 
+  static create(){
+    return new MetricStorageRegistry();
+  }
+
   getStorages(): MetricStorage[] {
     let storages: MetricStorage[] = [];
     for (const metricStorages of this._metricStorageRegistry.values()) {

experimental/packages/opentelemetry-sdk-metrics-base/src/view/Predicate.ts

@@ -51,6 +51,10 @@ export class PatternPredicate implements Predicate {
   static escapePattern(pattern: string): string {
     return `^${pattern.replace(ESCAPE, '\\$&').replace('*', '.*')}$`;
   }
+
+  static hasWildcard(pattern: string): boolean{
+    return pattern.includes('*');
+  }
 }
 
 export class ExactPredicate implements Predicate {

experimental/packages/opentelemetry-sdk-metrics-base/test/MeterProvider.test.ts

@@ -16,9 +16,14 @@
 
 import * as assert from 'assert';
 import { NOOP_METER } from '@opentelemetry/api-metrics-wip';
-import { Meter } from '../src/Meter';
-import { MeterProvider } from '../src/MeterProvider';
-import { defaultResource } from './util';
+import { Meter, MeterProvider, InstrumentType, PointDataType } from '../src';
+import {
+  assertInstrumentationLibraryMetrics,
+  assertMetricData,
+  assertPartialDeepStrictEqual,
+  defaultResource
+} from './util';
+import { TestMetricReader } from './export/TestMetricReader';
 
 describe('MeterProvider', () => {
   describe('constructor', () => {
@@ -47,4 +52,374 @@ describe('MeterProvider', () => {
       assert.strictEqual(meter, NOOP_METER);
     });
   });
+
+  describe('addView', () => {
+    it('with named view and instrument wildcard should throw', () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      // Throws with wildcard character only.
+      assert.throws(() => meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            name: '*'
+          }
+        }));
+
+      // Throws with wildcard character in instrument name.
+      assert.throws(() => meterProvider.addView({
+        name: 'renamed-instrument'
+      }, {
+        instrument: {
+          name: 'other.instrument.*'
+        }
+      }));
+    });
+
+    it('with named view and instrument type selector should throw', () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      assert.throws(() => meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            type: InstrumentType.COUNTER
+          }
+        }));
+    });
+
+    it('with named view and no instrument selector should throw', () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      assert.throws(() => meterProvider.addView({
+        name: 'renamed-instrument'
+      }));
+
+      assert.throws(() => meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {}));
+    });
+
+    it('with no view parameters should throw', () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      assert.throws(() => meterProvider.addView({}));
+    });
+
+    it('with existing instrument should rename', async () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      const reader = new TestMetricReader();
+      meterProvider.addMetricReader(reader);
+
+      // Add view to rename 'non-renamed-instrument' to 'renamed-instrument'
+      meterProvider.addView({
+          name: 'renamed-instrument',
+          description: 'my renamed instrument'
+        },
+        {
+          instrument: {
+            name: 'non-renamed-instrument',
+          },
+        });
+
+      // Create meter and instrument.
+      const myMeter = meterProvider.getMeter('meter1', 'v1.0.0');
+      const counter = myMeter.createCounter('non-renamed-instrument');
+      counter.add(1, { attrib1: 'attrib_value1', attrib2: 'attrib_value2' });
+
+      // Perform collection.
+      const result = await reader.collect();
+
+      // Results came only from one Meter.
+      assert.strictEqual(result?.instrumentationLibraryMetrics.length, 1);
+
+      // InstrumentationLibrary matches the only created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[0], {
+        name: 'meter1',
+        version: 'v1.0.0'
+      });
+
+      // Collected only one Metric.
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics.length, 1);
+
+      // View updated name and description.
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[0], PointDataType.SINGULAR, {
+        name: 'renamed-instrument',
+        type: InstrumentType.COUNTER,
+        description: 'my renamed instrument'
+      });
+
+      // Only one PointData added.
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics[0].pointData.length, 1);
+
+      // PointData matches attributes and point.
+      assertPartialDeepStrictEqual(result?.instrumentationLibraryMetrics[0].metrics[0].pointData[0], {
+        // Attributes are still there.
+        attributes: {
+          attrib1: 'attrib_value1',
+          attrib2: 'attrib_value2'
+        },
+        // Value that has been added to the counter.
+        point: 1
+      });
+    });
+
+    it('with attributeKeys should drop non-listed attributes', async () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      const reader = new TestMetricReader();
+      meterProvider.addMetricReader(reader);
+
+      // Add view to drop all attributes except 'attrib1'
+      meterProvider.addView({
+          attributeKeys: ['attrib1']
+        },
+        {
+          instrument: {
+            name: 'non-renamed-instrument',
+          }
+        });
+
+      // Create meter and instrument.
+      const myMeter = meterProvider.getMeter('meter1', 'v1.0.0');
+      const counter = myMeter.createCounter('non-renamed-instrument');
+      counter.add(1, { attrib1: 'attrib_value1', attrib2: 'attrib_value2' });
+
+      // Perform collection.
+      const result = await reader.collect();
+
+      // Results came only from one Meter.
+      assert.strictEqual(result?.instrumentationLibraryMetrics.length, 1);
+
+      // InstrumentationLibrary matches the only created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[0], {
+        name: 'meter1',
+        version: 'v1.0.0'
+      });
+
+      // Collected only one Metric.
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics.length, 1);
+
+      // View updated name and description.
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[0], PointDataType.SINGULAR, {
+        name: 'non-renamed-instrument',
+        type: InstrumentType.COUNTER,
+      });
+
+      // Only one PointData added.
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics[0].pointData.length, 1);
+
+      // PointData matches attributes and point.
+      assertPartialDeepStrictEqual(result?.instrumentationLibraryMetrics[0].metrics[0].pointData[0], {
+        // 'attrib_1' is still here but 'attrib_2' is not.
+        attributes: {
+          attrib1: 'attrib_value1'
+        },
+        // Value that has been added to the counter.
+        point: 1
+      });
+    });
+
+    it('with no meter name should apply view to instruments of all meters', async () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      const reader = new TestMetricReader();
+      meterProvider.addMetricReader(reader);
+
+      // Add view that renames 'test-counter' to 'renamed-instrument'
+      meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            name: 'test-counter'
+          }
+        });
+
+      // Create two meters.
+      const meter1 = meterProvider.getMeter('meter1', 'v1.0.0');
+      const meter2 = meterProvider.getMeter('meter2', 'v1.0.0');
+
+      // Create identical counters on both meters.
+      const counter1 = meter1.createCounter('test-counter', { unit: 'ms' });
+      const counter2 = meter2.createCounter('test-counter', { unit: 'ms' });
+
+      // Add values to counters.
+      counter1.add(1);
+      counter2.add(2);
+
+      // Perform collection.
+      const result = await reader.collect();
+
+      // Results came from two Meters.
+      assert.strictEqual(result?.instrumentationLibraryMetrics.length, 2);
+
+      // First InstrumentationLibrary matches the first created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[0], {
+        name: 'meter1',
+        version: 'v1.0.0'
+      });
+
+      // Collected one Metric on 'meter1'
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics.length, 1);
+
+      // View updated the name to 'renamed-instrument' and instrument is still a Counter
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[0], PointDataType.SINGULAR, {
+        name: 'renamed-instrument',
+        type: InstrumentType.COUNTER,
+      });
+
+      // Second InstrumentationLibrary matches the second created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[1], {
+        name: 'meter2',
+        version: 'v1.0.0'
+      });
+
+      // Collected one Metric on 'meter2'
+      assert.strictEqual(result?.instrumentationLibraryMetrics[1].metrics.length, 1);
+
+      // View updated the name to 'renamed-instrument' and instrument is still a Counter
+      assertMetricData(result?.instrumentationLibraryMetrics[1].metrics[0], PointDataType.SINGULAR, {
+        name: 'renamed-instrument',
+        type: InstrumentType.COUNTER
+      });
+    });
+
+    it('with meter name should apply view to only the selected meter', async () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+
+      const reader = new TestMetricReader();
+      meterProvider.addMetricReader(reader);
+
+      // Add view that renames 'test-counter' to 'renamed-instrument' on 'meter1'
+      meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            name: 'test-counter'
+          },
+          meter: {
+            name: 'meter1'
+          }
+        });
+
+      // Create two meters.
+      const meter1 = meterProvider.getMeter('meter1', 'v1.0.0');
+      const meter2 = meterProvider.getMeter('meter2', 'v1.0.0');
+
+      // Create counters with same name on both meters.
+      const counter1 = meter1.createCounter('test-counter', { unit: 'ms' });
+      const counter2 = meter2.createCounter('test-counter', { unit: 'ms' });
+
+      // Add values to both.
+      counter1.add(1);
+      counter2.add(1);
+
+      // Perform collection.
+      const result = await reader.collect();
+
+      // Results came from two Meters.
+      assert.strictEqual(result?.instrumentationLibraryMetrics.length, 2);
+
+      // First InstrumentationLibrary matches the first created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[0], {
+        name: 'meter1',
+        version: 'v1.0.0'
+      });
+
+      // Collected one Metric on 'meter1'
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics.length, 1);
+
+      // View updated the name to 'renamed-instrument' and instrument is still a Counter
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[0], PointDataType.SINGULAR, {
+        name: 'renamed-instrument',
+        type: InstrumentType.COUNTER
+      });
+
+      // Second InstrumentationLibrary matches the second created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[1], {
+        name: 'meter2',
+        version: 'v1.0.0'
+      });
+
+      // Collected one Metric on 'meter2'
+      assert.strictEqual(result?.instrumentationLibraryMetrics[1].metrics.length, 1);
+
+      // No updated name on 'test-counter'.
+      assertMetricData(result?.instrumentationLibraryMetrics[1].metrics[0], PointDataType.SINGULAR, {
+        name: 'test-counter',
+        type: InstrumentType.COUNTER
+      });
+    });
+
+    it('with different instrument types does not throw', async () => {
+      const meterProvider = new MeterProvider({ resource: defaultResource });
+      const reader = new TestMetricReader();
+      meterProvider.addMetricReader(reader);
+
+      // Add Views to rename both instruments (of different types) to the same name.
+      meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            name: 'test-counter'
+          },
+          meter: {
+            name: 'meter1'
+          }
+        });
+
+      meterProvider.addView({
+          name: 'renamed-instrument'
+        },
+        {
+          instrument: {
+            name: 'test-histogram'
+          },
+          meter: {
+            name: 'meter1'
+          }
+        });
+
+      // Create meter and instruments.
+      const meter = meterProvider.getMeter('meter1', 'v1.0.0');
+      const counter = meter.createCounter('test-counter', { unit: 'ms' });
+      const histogram = meter.createHistogram('test-histogram', { unit: 'ms' });
+
+      // Record values for both.
+      counter.add(1);
+      histogram.record(1);
+
+      // Perform collection.
+      const result = await reader.collect();
+
+      // Results came only from one Meter.
+      assert.strictEqual(result?.instrumentationLibraryMetrics.length, 1);
+
+      // InstrumentationLibrary matches the only created Meter.
+      assertInstrumentationLibraryMetrics(result?.instrumentationLibraryMetrics[0], {
+        name: 'meter1',
+        version: 'v1.0.0'
+      });
+
+      // Two metrics are collected ('renamed-instrument'-Counter and 'renamed-instrument'-Histogram)
+      assert.strictEqual(result?.instrumentationLibraryMetrics[0].metrics.length, 2);
+
+      // Both 'renamed-instrument' are still exported with their types.
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[0], PointDataType.SINGULAR, {
+        name: 'renamed-instrument',
+        type: InstrumentType.COUNTER
+      });
+      assertMetricData(result?.instrumentationLibraryMetrics[0].metrics[1], PointDataType.HISTOGRAM, {
+        name: 'renamed-instrument',
+        type: InstrumentType.HISTOGRAM
+      });
+    });
+  });
 });

experimental/packages/opentelemetry-sdk-metrics-base/test/util.ts

@@ -18,9 +18,15 @@ import { Attributes, ValueType } from '@opentelemetry/api-metrics';
 import { InstrumentationLibrary } from '@opentelemetry/core';
 import { Resource } from '@opentelemetry/resources';
 import * as assert from 'assert';
-import { InstrumentDescriptor, InstrumentType } from '../src/InstrumentDescriptor';
-import { Histogram } from '../src/Instruments';
-import { MetricData, PointData, PointDataType } from '../src/export/MetricData';
+import {
+  InstrumentDescriptor,
+  InstrumentType,
+  Histogram,
+  MetricData,
+  PointData,
+  PointDataType,
+  InstrumentationLibraryMetrics
+} from '../src';
 import { Measurement } from '../src/Measurement';
 import { isNotNullish } from '../src/utils';
 import { HrTime } from '@opentelemetry/api';
@@ -44,8 +50,8 @@ export const defaultInstrumentationLibrary: InstrumentationLibrary = {
 };
 
 export const commonValues: number[] = [1, -1, 1.0, Infinity, -Infinity, NaN];
-export const commonAttributes: Attributes[] = [{}, {1: '1'}, {a: '2'}, new (class Foo{
-a = '1';
+export const commonAttributes: Attributes[] = [{}, { 1: '1' }, { a: '2' }, new (class Foo {
+  a = '1';
 })];
 
 export const sleep = (time: number) =>
@@ -53,6 +59,15 @@ export const sleep = (time: number) =>
     return setTimeout(resolve, time);
   });
 
+export function assertInstrumentationLibraryMetrics(
+  actual: unknown,
+  instrumentationLibrary: Partial<InstrumentationLibrary>
+): asserts actual is InstrumentationLibraryMetrics {
+  const it = actual as InstrumentationLibraryMetrics;
+  assertPartialDeepStrictEqual(it.instrumentationLibrary, instrumentationLibrary);
+  assert(Array.isArray(it.metrics));
+}
+
 export function assertMetricData(
   actual: unknown,
   pointDataType?: PointDataType,