Skip to content

Commit

Permalink
skeleton(metrics): createMeasure (#563)
Browse files Browse the repository at this point in the history
* feat: implement createMeasure

* fix: linting

* fix: disable broken tests

* fix: labelSet log statement

* fix: absolute always set to true

* fix: linting

* fix: add jsdoc, adjust optionals

* fix: failing tests

* fix: linting

* refactor: rename test handle var

Co-authored-by: Mayur Kale <mayurkale@google.com>
  • Loading branch information
Mark Wolff and mayurkale22 authored Feb 5, 2020
1 parent 4b764e5 commit 470fc62
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 8 deletions.
9 changes: 7 additions & 2 deletions packages/opentelemetry-api/src/metrics/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,16 @@ export interface MetricOptions {
disabled?: boolean;

/**
* Monotonic allows this metric to accept negative values. If `true` only
* non-negative values are expected.
* Asserts that this metric may only increase (e.g. time spent).
*/
monotonic?: boolean;

/**
* (Measure only, default true) Asserts that this metric will only accept
* non-negative values (e.g. disk usage).
*/
absolute?: boolean;

/**
* Indicates the type of the recorded value.
* @default {@link ValueType.DOUBLE}
Expand Down
36 changes: 34 additions & 2 deletions packages/opentelemetry-metrics/src/BoundInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,44 @@ export class BoundGauge extends BaseBoundInstrument
*/
export class BoundMeasure extends BaseBoundInstrument
implements types.BoundMeasure {
constructor(
labelSet: types.LabelSet,
private readonly _disabled: boolean,
private readonly _absolute: boolean,
private readonly _valueType: types.ValueType,
private readonly _logger: types.Logger,
private readonly _onUpdate: Function
) {
super(labelSet);
}

record(
value: number,
distContext?: types.DistributedContext,
spanContext?: types.SpanContext
): void {
// @todo: implement this method.
return;
if (this._disabled) return;

if (this._absolute && value < 0) {
this._logger.error(
`Absolute measure cannot contain negative values for ${Object.values(
this._labelSet.labels
)}}`
);
return;
}

if (this._valueType === types.ValueType.INT && !Number.isInteger(value)) {
this._logger.warn(
`INT measure cannot accept a floating-point value for ${Object.values(
this._labelSet.labels
)}; truncating the value.`
);
value = Math.trunc(value);
}

//@todo: implement this._data logic

this._onUpdate();
}
}
20 changes: 17 additions & 3 deletions packages/opentelemetry-metrics/src/Meter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import * as types from '@opentelemetry/api';
import { ConsoleLogger } from '@opentelemetry/core';
import { BaseBoundInstrument } from './BoundInstrument';
import { Metric, CounterMetric, GaugeMetric } from './Metric';
import { Metric, CounterMetric, GaugeMetric, MeasureMetric } from './Metric';
import {
MetricOptions,
DEFAULT_METRIC_OPTIONS,
Expand Down Expand Up @@ -61,8 +61,20 @@ export class Meter implements types.Meter {
);
return types.NOOP_MEASURE_METRIC;
}
// @todo: implement this method
throw new Error('not implemented yet');
const opt: MetricOptions = {
// Measures are defined as absolute by default
absolute: true,
monotonic: false, // not applicable to measure, set to false
logger: this._logger,
...DEFAULT_METRIC_OPTIONS,
...options,
};

const measure = new MeasureMetric(name, opt, () => {
this._exportOneMetric(name);
});
this._registerMetric(name, measure);
return measure;
}

/**
Expand All @@ -85,6 +97,7 @@ export class Meter implements types.Meter {
const opt: MetricOptions = {
// Counters are defined as monotonic by default
monotonic: true,
absolute: false, // not applicable to counter, set to false
logger: this._logger,
...DEFAULT_METRIC_OPTIONS,
...options,
Expand Down Expand Up @@ -117,6 +130,7 @@ export class Meter implements types.Meter {
const opt: MetricOptions = {
// Gauges are defined as non-monotonic by default
monotonic: false,
absolute: false, // not applicable for gauges, set to false
logger: this._logger,
...DEFAULT_METRIC_OPTIONS,
...options,
Expand Down
36 changes: 36 additions & 0 deletions packages/opentelemetry-metrics/src/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
BoundCounter,
BoundGauge,
BaseBoundInstrument,
BoundMeasure,
} from './BoundInstrument';
import { MetricOptions } from './types';
import {
Expand Down Expand Up @@ -199,3 +200,38 @@ export class GaugeMetric extends Metric<BoundGauge>
this.bind(labelSet).set(value);
}
}

export class MeasureMetric extends Metric<BoundMeasure>
implements Pick<types.MetricUtils, 'record'> {
protected readonly _absolute: boolean;

constructor(
name: string,
options: MetricOptions,
private readonly _onUpdate: Function
) {
super(
name,
options,
options.valueType === types.ValueType.DOUBLE
? MetricDescriptorType.MEASURE_DOUBLE
: MetricDescriptorType.MEASURE_INT64
);

this._absolute = options.absolute !== undefined ? options.absolute : true; // Absolute default is true
}
protected _makeInstrument(labelSet: types.LabelSet): BoundMeasure {
return new BoundMeasure(
labelSet,
this._disabled,
this._absolute,
this._valueType,
this._logger,
this._onUpdate
);
}

record(value: number, labelSet: types.LabelSet) {
this.bind(labelSet).record(value);
}
}
8 changes: 8 additions & 0 deletions packages/opentelemetry-metrics/src/export/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ export enum MetricDescriptorType {
* window. This is not recommended, since it cannot be aggregated.
*/
SUMMARY,
/**
* Integer measure. The value can be positive or negative.
*/
MEASURE_INT64,
/**
* Floating point measure. The value can be positive or negative.
*/
MEASURE_DOUBLE,
}

/**
Expand Down
5 changes: 4 additions & 1 deletion packages/opentelemetry-metrics/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ export interface MetricOptions {
/** The map of constant labels for the Metric. */
constantLabels?: Map<string, string>;

/** Indicates the metric is a verbose metric that is disabled by default */
/** Indicates the metric is a verbose metric that is disabled by default. */
disabled: boolean;

/** Monotonic metrics may only increase. */
monotonic: boolean;

/** (Measure only) Asserts that this metric will only accept non-negative values. */
absolute: boolean;

/** User provided logger. */
logger: Logger;

Expand Down
100 changes: 100 additions & 0 deletions packages/opentelemetry-metrics/test/Meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
CounterMetric,
GaugeMetric,
MetricDescriptorType,
MeasureMetric,
} from '../src';
import * as types from '@opentelemetry/api';
import { LabelSet } from '../src/LabelSet';
Expand Down Expand Up @@ -359,6 +360,39 @@ describe('Meter', () => {
});

describe('#measure', () => {
it('should create a measure', () => {
const measure = meter.createMeasure('name') as MeasureMetric;
assert.ok(measure instanceof Metric);
});

it('should create a measure with options', () => {
const measure = meter.createMeasure('name', {
description: 'desc',
unit: '1',
disabled: false,
});
assert.ok(measure instanceof Metric);
});

it('should be absolute by default', () => {
const measure = meter.createMeasure('name', {
description: 'desc',
unit: '1',
disabled: false,
});
assert.strictEqual((measure as MeasureMetric)['_absolute'], true);
});

it('should be able to set absolute to false', () => {
const measure = meter.createMeasure('name', {
description: 'desc',
unit: '1',
disabled: false,
absolute: false,
});
assert.strictEqual((measure as MeasureMetric)['_absolute'], false);
});

describe('names', () => {
it('should return no op metric if name is an empty string', () => {
const gauge = meter.createMeasure('');
Expand All @@ -377,6 +411,72 @@ describe('Meter', () => {
assert.ok(gauge instanceof types.NoopMetric);
});
});

describe('.bind()', () => {
it('should create a measure instrument', () => {
const measure = meter.createMeasure('name') as MeasureMetric;
const boundMeasure = measure.bind(labelSet);
assert.doesNotThrow(() => boundMeasure.record(10));
});

it('should return the timeseries', () => {
// @todo: implement once record is implemented
});

it('should not accept negative values by default', () => {
// @todo: implement once record is implemented
});

it('should not set the instrument data when disabled', () => {
const measure = meter.createMeasure('name', {
disabled: true,
}) as MeasureMetric;
const boundMeasure = measure.bind(labelSet);
boundMeasure.record(10);
assert.strictEqual(boundMeasure['_data'], 0);
});

it('should accept negative (and positive) values when monotonic is set to false', () => {
// @todo: implement once record is implemented
});

it('should return same instrument on same label values', () => {
const measure = meter.createMeasure('name') as MeasureMetric;
const boundMeasure1 = measure.bind(labelSet);
boundMeasure1.record(10);
const boundMeasure2 = measure.bind(labelSet);
boundMeasure2.record(100);
// @todo: re-add once record is implemented
// assert.strictEqual(boundMeasure1['_data'], 100);
assert.strictEqual(boundMeasure1, boundMeasure2);
});
});

describe('.unbind()', () => {
it('should remove the measure instrument', () => {
const measure = meter.createMeasure('name') as MeasureMetric;
const boundMeasure = measure.bind(labelSet);
assert.strictEqual(measure['_instruments'].size, 1);
measure.unbind(labelSet);
assert.strictEqual(measure['_instruments'].size, 0);
const boundMeasure2 = measure.bind(labelSet);
assert.strictEqual(measure['_instruments'].size, 1);
assert.notStrictEqual(boundMeasure, boundMeasure2);
});

it('should not fail when removing non existing instrument', () => {
const measure = meter.createMeasure('name');
measure.unbind(new LabelSet('nonexistant', {}));
});

it('should clear all instruments', () => {
const measure = meter.createMeasure('name') as MeasureMetric;
measure.bind(labelSet);
assert.strictEqual(measure['_instruments'].size, 1);
measure.clear();
assert.strictEqual(measure['_instruments'].size, 0);
});
});
});

describe('#getMetrics', () => {
Expand Down

0 comments on commit 470fc62

Please sign in to comment.