Skip to content

Commit

Permalink
feat(metrics): use MetricDescriptor to determine aggregator #989 (#1014)
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarchaud authored May 4, 2020
1 parent 817397b commit 16ae2a7
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 10 deletions.
8 changes: 4 additions & 4 deletions packages/opentelemetry-metrics/src/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export abstract class Metric<T extends BaseBoundInstrument>
protected readonly _disabled: boolean;
protected readonly _valueType: api.ValueType;
protected readonly _logger: api.Logger;
private readonly _descriptor: MetricDescriptor;
protected readonly _descriptor: MetricDescriptor;
private readonly _instruments: Map<string, T> = new Map();

constructor(
Expand Down Expand Up @@ -124,7 +124,7 @@ export class CounterMetric extends Metric<BoundCounter>
this._valueType,
this._logger,
// @todo: consider to set to CounterSumAggregator always.
this._batcher.aggregatorFor(MetricKind.COUNTER)
this._batcher.aggregatorFor(this._descriptor)
);
}

Expand Down Expand Up @@ -161,7 +161,7 @@ export class MeasureMetric extends Metric<BoundMeasure>
this._absolute,
this._valueType,
this._logger,
this._batcher.aggregatorFor(MetricKind.MEASURE)
this._batcher.aggregatorFor(this._descriptor)
);
}

Expand Down Expand Up @@ -191,7 +191,7 @@ export class ObserverMetric extends Metric<BoundObserver>
this._monotonic,
this._valueType,
this._logger,
this._batcher.aggregatorFor(MetricKind.OBSERVER)
this._batcher.aggregatorFor(this._descriptor)
);
}

Expand Down
15 changes: 10 additions & 5 deletions packages/opentelemetry-metrics/src/export/Batcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ import {
MeasureExactAggregator,
ObserverAggregator,
} from './aggregators';
import { MetricRecord, MetricKind, Aggregator } from './types';
import {
MetricRecord,
MetricKind,
Aggregator,
MetricDescriptor,
} from './types';

/**
* Base class for all batcher types.
Expand All @@ -31,8 +36,8 @@ import { MetricRecord, MetricKind, Aggregator } from './types';
export abstract class Batcher {
protected readonly _batchMap = new Map<string, MetricRecord>();

/** Returns an aggregator based off metric kind. */
abstract aggregatorFor(metricKind: MetricKind): Aggregator;
/** Returns an aggregator based off metric descriptor. */
abstract aggregatorFor(metricKind: MetricDescriptor): Aggregator;

/** Stores record information to be ready for exporting. */
abstract process(record: MetricRecord): void;
Expand All @@ -47,8 +52,8 @@ export abstract class Batcher {
* passes them for exporting.
*/
export class UngroupedBatcher extends Batcher {
aggregatorFor(metricKind: MetricKind): Aggregator {
switch (metricKind) {
aggregatorFor(metricDescriptor: MetricDescriptor): Aggregator {
switch (metricDescriptor.metricKind) {
case MetricKind.COUNTER:
return new CounterSumAggregator();
case MetricKind.OBSERVER:
Expand Down
3 changes: 2 additions & 1 deletion packages/opentelemetry-metrics/test/Meter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
MetricRecord,
Aggregator,
MetricObservable,
MetricDescriptor,
} from '../src';
import * as types from '@opentelemetry/api';
import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core';
Expand Down Expand Up @@ -566,7 +567,7 @@ class CustomBatcher extends Batcher {
process(record: MetricRecord): void {
throw new Error('process method not implemented.');
}
aggregatorFor(metricKind: MetricKind): Aggregator {
aggregatorFor(metricKind: MetricDescriptor): Aggregator {
throw new Error('aggregatorFor method not implemented.');
}
}
Expand Down

0 comments on commit 16ae2a7

Please sign in to comment.