Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validate metric names #468

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions packages/opentelemetry-metrics/src/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@
*/

import * as types from '@opentelemetry/types';
import { ConsoleLogger } from '@opentelemetry/core';
import { CounterHandle, GaugeHandle, MeasureHandle } from './Handle';
import { Metric, CounterMetric, GaugeMetric } from './Metric';
import {
ConsoleLogger,
NOOP_COUNTER_METRIC,
NOOP_GAUGE_METRIC,
NOOP_MEASURE_METRIC
} from '@opentelemetry/core';
import { CounterMetric, GaugeMetric } from './Metric';
import {
MetricOptions,
DEFAULT_METRIC_OPTIONS,
Expand Down Expand Up @@ -46,7 +50,10 @@ export class Meter implements types.Meter {
createMeasure(
name: string,
options?: types.MetricOptions
): Metric<MeasureHandle> {
): types.Metric<types.MeasureHandle> {
if (!this._isValidName(name)) {
return NOOP_MEASURE_METRIC;
}
// @todo: implement this method
throw new Error('not implemented yet');
}
Expand All @@ -61,7 +68,10 @@ export class Meter implements types.Meter {
createCounter(
name: string,
options?: types.MetricOptions
): Metric<CounterHandle> {
): types.Metric<types.CounterHandle> {
if (!this._isValidName(name)) {
return NOOP_COUNTER_METRIC;
}
const opt: MetricOptions = {
// Counters are defined as monotonic by default
monotonic: true,
Expand All @@ -83,7 +93,10 @@ export class Meter implements types.Meter {
createGauge(
name: string,
options?: types.MetricOptions
): Metric<GaugeHandle> {
): types.Metric<types.GaugeHandle> {
if (!this._isValidName(name)) {
return NOOP_GAUGE_METRIC;
}
const opt: MetricOptions = {
// Gauges are defined as non-monotonic by default
monotonic: false,
Expand All @@ -93,4 +106,21 @@ export class Meter implements types.Meter {
};
return new GaugeMetric(name, opt);
}

/**
* Ensure a metric name conforms to the following rules:
*
* 1. They are non-empty strings
*
* 2. The first character must be non-numeric, non-space, non-punctuation
*
* 3. Subsequent characters must be belong to the alphanumeric characters, '_', '.', and '-'.
*
* Names are case insensitive
*
* @param name Name of metric to be created
*/
private _isValidName(name: string): boolean {
return Boolean(name.match(/^[a-z][a-z0-9_.\-]*$/i));
}
}
56 changes: 54 additions & 2 deletions packages/opentelemetry-metrics/test/Meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
*/

import * as assert from 'assert';
import { Meter, Metric } from '../src';
import { Meter, Metric, CounterMetric, GaugeMetric } from '../src';
import * as types from '@opentelemetry/types';
import { NoopLogger } from '@opentelemetry/core';
import { NoopLogger, NoopMetric } from '@opentelemetry/core';

describe('Meter', () => {
let meter: Meter;
Expand Down Expand Up @@ -129,6 +129,32 @@ describe('Meter', () => {
assert.strictEqual(counter['_handles'].size, 0);
});
});

describe('names', () => {
it('should create counter with valid names', () => {
const counter1 = meter.createCounter('name1');
const counter2 = meter.createCounter('Name_with-all.valid_CharacterClasses');
assert.ok(counter1 instanceof CounterMetric)
assert.ok(counter2 instanceof CounterMetric)
});

it('should return no op metric if name is an empty string', () => {
const counter = meter.createCounter('');
assert.ok(counter instanceof NoopMetric)
})

it('should return no op metric if name does not start with a letter', () => {
const counter1 = meter.createCounter('1name');
const counter_ = meter.createCounter('_name');
assert.ok(counter1 instanceof NoopMetric)
assert.ok(counter_ instanceof NoopMetric)
});

it('should return no op metric if name is an empty string contain only letters, numbers, ".", "_", and "-"', () => {
const counter = meter.createCounter('name with invalid characters^&*(');
assert.ok(counter instanceof NoopMetric)
});
})
});

describe('#gauge', () => {
Expand Down Expand Up @@ -230,5 +256,31 @@ describe('Meter', () => {
assert.strictEqual(gauge['_handles'].size, 0);
});
});

describe('names', () => {
it('should create gauges with valid names', () => {
const gauge1 = meter.createGauge('name1');
const gauge2 = meter.createGauge('Name_with-all.valid_CharacterClasses');
assert.ok(gauge1 instanceof GaugeMetric)
assert.ok(gauge2 instanceof GaugeMetric)
});

it('should return no op metric if name is an empty string', () => {
const gauge = meter.createGauge('');
assert.ok(gauge instanceof NoopMetric)
})

it('should return no op metric if name does not start with a letter', () => {
const gauge1 = meter.createGauge('1name');
const gauge_ = meter.createGauge('_name');
assert.ok(gauge1 instanceof NoopMetric)
assert.ok(gauge_ instanceof NoopMetric)
});

it('should return no op metric if name is an empty string contain only letters, numbers, ".", "_", and "-"', () => {
const gauge = meter.createGauge('name with invalid characters^&*(');
assert.ok(gauge instanceof NoopMetric)
});
})
});
});