fix(sdk-metrics): metric names should be case-insensitive

CHANGELOG.md

@@ -18,6 +18,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 ### :bug: (Bug Fix)
 
 * fix(exporter-zipkin): rounding duration to the nearest int to be compliant with zipkin protocol [#4064](https://github.com/open-telemetry/opentelemetry-js/pull/4064) @n0cloud
+* fix(sdk-metrics): metric names should be case-insensitive
 
 ### :books: (Refine Doc)
 

packages/sdk-metrics/src/InstrumentDescriptor.ts

@@ -14,8 +14,9 @@
  * limitations under the License.
  */
 
-import { MetricOptions, ValueType } from '@opentelemetry/api';
+import { MetricOptions, ValueType, diag } from '@opentelemetry/api';
 import { View } from './view/View';
+import { equalsCaseInsensitive } from './utils';
 
 /**
  * Supported types of metric instruments.
@@ -45,6 +46,11 @@ export function createInstrumentDescriptor(
  type: InstrumentType,
  options?: MetricOptions
 ): InstrumentDescriptor {
+ if (!isValidName(name)) {
+ diag.warn(
+ `Invalid metric name: "${name}". The metric name should be a ASCII string with a length no greater than 255 characters.`
+ );
+ }
  return {
  name,
  type,
@@ -71,10 +77,18 @@ export function isDescriptorCompatibleWith(
  descriptor: InstrumentDescriptor,
  otherDescriptor: InstrumentDescriptor
 ) {
+ // Names are case-insensitive strings.
  return (
- descriptor.name === otherDescriptor.name &&
+ equalsCaseInsensitive(descriptor.name, otherDescriptor.name) &&
  descriptor.unit === otherDescriptor.unit &&
  descriptor.type === otherDescriptor.type &&
  descriptor.valueType === otherDescriptor.valueType
  );
 }
+
+// ASCII string with a length no greater than 255 characters.
+// NB: the first character counted separately from the rest.
+const NAME_REGEXP = /^[a-z][a-z0-9_.-]{0,254}$/i;
+export function isValidName(name: string): boolean {
+ return name.match(NAME_REGEXP) != null;
+}

packages/sdk-metrics/src/utils.ts

@@ -190,3 +190,7 @@ export function binarySearchLB(arr: number[], value: number): number {
  }
  return -1;
 }
+
+export function equalsCaseInsensitive(lhs: string, rhs: string): boolean {
+ return lhs.toLowerCase() === rhs.toLowerCase();
+}

packages/sdk-metrics/test/InstrumentDescriptor.test.ts

@@ -17,8 +17,13 @@
 import * as assert from 'assert';
 import {
  createInstrumentDescriptor,
+ InstrumentDescriptor,
  InstrumentType,
+ isValidName,
+ isDescriptorCompatibleWith,
 } from '../src/InstrumentDescriptor';
+import { invalidNames, validNames } from './util';
+import { ValueType } from '@opentelemetry/api';
 
 describe('InstrumentDescriptor', () => {
  describe('createInstrumentDescriptor', () => {
@@ -35,4 +40,105 @@ describe('InstrumentDescriptor', () => {
  });
  }
  });
+
+ describe('isDescriptorCompatibleWith', () => {
+ const tests: [
+ string,
+ boolean,
+ InstrumentDescriptor,
+ InstrumentDescriptor,
+ ][] = [
+ [
+ 'Compatible with identical descriptors',
+ true,
+ {
+ name: 'foo',
+ description: 'any descriptions',
+ unit: 'kB',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ {
+ name: 'foo',
+ description: 'any descriptions',
+ unit: 'kB',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ ],
+ [
+ 'Compatible with case-insensitive names',
+ true,
+ {
+ name: 'foo',
+ description: '',
+ unit: '',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ {
+ name: 'FoO',
+ description: '',
+ unit: '',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ ],
+ [
+ 'Incompatible with different names',
+ false,
+ {
+ name: 'foo',
+ description: '',
+ unit: '',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ {
+ name: 'foobar',
+ description: '',
+ unit: '',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ ],
+ [
+ 'Incompatible with case-sensitive units',
+ false,
+ {
+ name: 'foo',
+ description: '',
+ unit: 'kB',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ {
+ name: 'foo',
+ description: '',
+ unit: 'kb',
+ type: InstrumentType.COUNTER,
+ valueType: ValueType.DOUBLE,
+ },
+ ],
+ ];
+ for (const test of tests) {
+ it(test[0], () => {
+ assert.ok(isDescriptorCompatibleWith(test[2], test[3]) === test[1]);
+ });
+ }
+ });
+
+ describe('isValidName', () => {
+ it('should validate names', () => {
+ for (const invalidName of invalidNames) {
+ assert.ok(
+ !isValidName(invalidName),
+ `${invalidName} should be invalid`
+ );
+ }
+ for (const validName of validNames) {
+ assert.ok(isValidName(validName), `${validName} should be valid`);
+ }
+ });
+ });
 });

packages/sdk-metrics/test/Meter.test.ts

@@ -14,8 +14,9 @@
  * limitations under the License.
  */
 
-import { Observable } from '@opentelemetry/api';
+import { Observable, diag } from '@opentelemetry/api';
 import * as assert from 'assert';
+import * as sinon from 'sinon';
 import {
  CounterInstrument,
  HistogramInstrument,
@@ -27,78 +28,86 @@ import {
 import { Meter } from '../src/Meter';
 import { MeterProviderSharedState } from '../src/state/MeterProviderSharedState';
 import { MeterSharedState } from '../src/state/MeterSharedState';
-import { defaultInstrumentationScope, defaultResource } from './util';
+import {
+ defaultInstrumentationScope,
+ defaultResource,
+ invalidNames,
+ validNames,
+} from './util';
 
 describe('Meter', () => {
+ afterEach(() => {
+ sinon.restore();
+ });
+
  describe('createCounter', () => {
- it('should create counter', () => {
+ testWithNames('counter', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const counter = meter.createCounter('foobar');
+ const counter = meter.createCounter(name);
  assert(counter instanceof CounterInstrument);
  });
  });
 
  describe('createUpDownCounter', () => {
- it('should create up down counter', () => {
+ testWithNames('UpDownCounter', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const upDownCounter = meter.createUpDownCounter('foobar');
+ const upDownCounter = meter.createUpDownCounter(name);
  assert(upDownCounter instanceof UpDownCounterInstrument);
  });
  });
 
  describe('createHistogram', () => {
- it('should create histogram', () => {
+ testWithNames('Histogram', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const histogram = meter.createHistogram('foobar');
+ const histogram = meter.createHistogram(name);
  assert(histogram instanceof HistogramInstrument);
  });
  });
 
  describe('createObservableGauge', () => {
- it('should create observable gauge', () => {
+ testWithNames('ObservableGauge', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const observableGauge = meter.createObservableGauge('foobar');
+ const observableGauge = meter.createObservableGauge(name);
  assert(observableGauge instanceof ObservableGaugeInstrument);
  });
  });
 
  describe('createObservableCounter', () => {
- it('should create observable counter', () => {
+ testWithNames('ObservableCounter', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const observableCounter = meter.createObservableCounter('foobar');
+ const observableCounter = meter.createObservableCounter(name);
  assert(observableCounter instanceof ObservableCounterInstrument);
  });
  });
 
  describe('createObservableUpDownCounter', () => {
- it('should create observable up-down-counter', () => {
+ testWithNames('ObservableUpDownCounter', name => {
  const meterSharedState = new MeterSharedState(
  new MeterProviderSharedState(defaultResource),
  defaultInstrumentationScope
  );
  const meter = new Meter(meterSharedState);
- const observableUpDownCounter =
- meter.createObservableUpDownCounter('foobar');
+ const observableUpDownCounter = meter.createObservableUpDownCounter(name);
  assert(
  observableUpDownCounter instanceof ObservableUpDownCounterInstrument
  );
@@ -167,3 +176,22 @@ describe('Meter', () => {
  });
  });
 });
+
+function testWithNames(type: string, tester: (name: string) => void) {
+ for (const invalidName of invalidNames) {
+ it(`should warn with invalid name ${invalidName} for ${type}`, () => {
+ const warnStub = sinon.spy(diag, 'warn');
+ tester(invalidName);
+ assert.strictEqual(warnStub.callCount, 1);
+ assert.ok(warnStub.calledWithMatch('Invalid metric name'));
+ });
+ }
+
+ for (const validName of validNames) {
+ it(`should not warn with valid name ${validName} for ${type}`, () => {
+ const warnStub = sinon.spy(diag, 'warn');
+ tester(validName);
+ assert.strictEqual(warnStub.callCount, 0);
+ });
+ }
+}

packages/sdk-metrics/test/util.ts

@@ -66,6 +66,9 @@ export const defaultInstrumentationScope: InstrumentationScope = {
  schemaUrl: 'https://opentelemetry.io/schemas/1.7.0',
 };
 
+export const invalidNames = ['', 'a'.repeat(256), '1a', '-a', '.a', '_a'];
+export const validNames = ['a', 'a'.repeat(255), 'a1', 'a-1', 'a.1', 'a_1'];
+
 export const commonValues: number[] = [1, -1, 1.0, Infinity, -Infinity, NaN];
 export const commonAttributes: MetricAttributes[] = [
  {},