Skip to content

Commit

Permalink
fix(sdk-metrics-base): remove aggregator.toMetricData dependency on A…
Browse files Browse the repository at this point in the history
…ggregationTemporality (#2676)

Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
legendecas and vmarchaud authored Jan 10, 2022
1 parent 6b94e26 commit 4909cf1
Showing 10 changed files with 46 additions and 199 deletions.
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
import { HrTime } from '@opentelemetry/api';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { MetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { Maybe } from '../utils';
@@ -48,10 +47,8 @@ export class DropAggregator implements Aggregator<undefined> {
_instrumentationLibrary: InstrumentationLibrary,
_instrumentDescriptor: InstrumentDescriptor,
_accumulationByAttributes: AccumulationRecord<undefined>[],
_temporality: AggregationTemporality,
_sdkStartTime: HrTime,
_lastCollectionTime: HrTime,
_collectionTime: HrTime): Maybe<MetricData> {
_startTime: HrTime,
_endTime: HrTime): Maybe<MetricData> {
return undefined;
}
}
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ import { HistogramMetricData, PointDataType } from '../export/MetricData';
import { Resource } from '@opentelemetry/resources';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { HrTime } from '@opentelemetry/api';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { Maybe } from '../utils';

@@ -143,10 +142,8 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
instrumentationLibrary: InstrumentationLibrary,
metricDescriptor: InstrumentDescriptor,
accumulationByAttributes: AccumulationRecord<HistogramAccumulation>[],
temporality: AggregationTemporality,
sdkStartTime: HrTime,
lastCollectionTime: HrTime,
collectionTime: HrTime): Maybe<HistogramMetricData> {
startTime: HrTime,
endTime: HrTime): Maybe<HistogramMetricData> {
return {
resource,
instrumentationLibrary,
@@ -155,8 +152,8 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
pointData: accumulationByAttributes.map(([attributes, accumulation]) => {
return {
attributes,
startTime: temporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: accumulation.toPoint(),
}
})
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ import { LastValue, AggregatorKind, Aggregator, Accumulation, AccumulationRecord
import { HrTime } from '@opentelemetry/api';
import { hrTime, hrTimeToMicroseconds, InstrumentationLibrary } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { PointDataType, SingularMetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { Maybe } from '../utils';
@@ -72,10 +71,8 @@ export class LastValueAggregator implements Aggregator<LastValueAccumulation> {
instrumentationLibrary: InstrumentationLibrary,
instrumentDescriptor: InstrumentDescriptor,
accumulationByAttributes: AccumulationRecord<LastValueAccumulation>[],
temporality: AggregationTemporality,
sdkStartTime: HrTime,
lastCollectionTime: HrTime,
collectionTime: HrTime): Maybe<SingularMetricData> {
startTime: HrTime,
endTime: HrTime): Maybe<SingularMetricData> {
return {
resource,
instrumentationLibrary,
@@ -84,8 +81,8 @@ export class LastValueAggregator implements Aggregator<LastValueAccumulation> {
pointData: accumulationByAttributes.map(([attributes, accumulation]) => {
return {
attributes,
startTime: temporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: accumulation.toPoint(),
}
})
Original file line number Diff line number Diff line change
@@ -19,7 +19,6 @@ import { HrTime } from '@opentelemetry/api';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { PointDataType, SingularMetricData } from '../export/MetricData';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { Maybe } from '../utils';

@@ -62,10 +61,8 @@ export class SumAggregator implements Aggregator<SumAccumulation> {
instrumentationLibrary: InstrumentationLibrary,
instrumentDescriptor: InstrumentDescriptor,
accumulationByAttributes: AccumulationRecord<SumAccumulation>[],
temporality: AggregationTemporality,
sdkStartTime: HrTime,
lastCollectionTime: HrTime,
collectionTime: HrTime): Maybe<SingularMetricData> {
startTime: HrTime,
endTime: HrTime): Maybe<SingularMetricData> {
return {
resource,
instrumentationLibrary,
@@ -74,8 +71,8 @@ export class SumAggregator implements Aggregator<SumAccumulation> {
pointData: accumulationByAttributes.map(([attributes, accumulation]) => {
return {
attributes,
startTime: temporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: accumulation.toPoint(),
}
})
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ import { HrTime } from '@opentelemetry/api';
import { Attributes } from '@opentelemetry/api-metrics';
import { InstrumentationLibrary } from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { AggregationTemporality } from '../export/AggregationTemporality';
import { MetricData } from '../export/MetricData';
import { InstrumentDescriptor } from '../InstrumentDescriptor';
import { Maybe } from '../utils';
@@ -113,18 +112,14 @@ export interface Aggregator<T> {
* @param instrumentationLibrary the library that instrumented the metric
* @param instrumentDescriptor the metric instrument descriptor.
* @param accumulationByAttributes the array of attributes and accumulation pairs.
* @param temporality the temporality of the accumulation.
* @param sdkStartTime the start time of the sdk.
* @param lastCollectionTime the last collection time of the instrument.
* @param collectionTime the active collection time of the instrument.
* @param startTime the start time of the metric data.
* @param endTime the end time of the metric data.
* @return the {@link MetricData} that this {@link Aggregator} will produce.
*/
toMetricData(resource: Resource,
instrumentationLibrary: InstrumentationLibrary,
instrumentDescriptor: InstrumentDescriptor,
accumulationByAttributes: AccumulationRecord<T>[],
temporality: AggregationTemporality,
sdkStartTime: HrTime,
lastCollectionTime: HrTime,
collectionTime: HrTime): Maybe<MetricData>;
startTime: HrTime,
endTime: HrTime): Maybe<MetricData>;
}
Original file line number Diff line number Diff line change
@@ -107,18 +107,16 @@ export class TemporalMetricProcessor<T> {
collectionTime,
});

// Metric data time span is determined in Aggregator.toMetricData with aggregation temporality:
// Metric data time span is determined as:
// 1. Cumulative Aggregation time span: (sdkStartTime, collectionTime]
// 2. Delta Aggregation time span: (lastCollectionTime, collectionTime]
return this._aggregator.toMetricData(
resource,
instrumentationLibrary,
instrumentDescriptor,
AttributesMapToAccumulationRecords(result),
aggregationTemporality,
sdkStartTime,
lastCollectionTime,
collectionTime);
/* startTime */ aggregationTemporality === AggregationTemporality.CUMULATIVE ? sdkStartTime : lastCollectionTime,
/* endTime */ collectionTime);
}

private _stashAccumulations(collectors: MetricCollectorHandle[], currentAccumulation: AttributeHashMap<T>) {
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
import { HrTime } from '@opentelemetry/api';
import * as assert from 'assert';
import { DropAggregator } from '../../src/aggregator';
import { AggregationTemporality } from '../../src/export/AggregationTemporality';
import { defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util';

describe('DropAggregator', () => {
@@ -51,19 +50,16 @@ describe('DropAggregator', () => {
it('no exceptions', () => {
const aggregator = new DropAggregator();

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];
const startTime: HrTime = [0, 0];
const endTime: HrTime = [1, 1];

assert.strictEqual(aggregator.toMetricData(
defaultResource,
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, undefined]],
AggregationTemporality.DELTA,
sdkStartTime,
lastCollectionTime,
collectionTime,
startTime,
endTime,
), undefined);
});
});
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
import { HrTime } from '@opentelemetry/api';
import * as assert from 'assert';
import { HistogramAccumulation, HistogramAggregator } from '../../src/aggregator';
import { AggregationTemporality } from '../../src/export/AggregationTemporality';
import { MetricData, PointDataType } from '../../src/export/MetricData';
import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util';

@@ -82,16 +81,15 @@ describe('HistogramAggregator', () => {
});

describe('toMetricData', () => {
it('transform with AggregationTemporality.DELTA', () => {
it('transform without exception', () => {
const aggregator = new HistogramAggregator([1, 10, 100]);

const accumulation = aggregator.createAccumulation();
accumulation.record(0);
accumulation.record(1);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];
const startTime: HrTime = [0, 0];
const endTime: HrTime = [1, 1];

const expected: MetricData = {
resource: defaultResource,
@@ -101,8 +99,8 @@ describe('HistogramAggregator', () => {
pointData: [
{
attributes: {},
startTime: lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: {
buckets: {
boundaries: [1, 10, 100],
@@ -119,54 +117,8 @@ describe('HistogramAggregator', () => {
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.DELTA,
sdkStartTime,
lastCollectionTime,
collectionTime,
), expected);
});

it('transform with AggregationTemporality.CUMULATIVE', () => {
const aggregator = new HistogramAggregator([1, 10, 100]);

const accumulation = aggregator.createAccumulation();
accumulation.record(0);
accumulation.record(1);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];

const expected: MetricData = {
resource: defaultResource,
instrumentationLibrary: defaultInstrumentationLibrary,
instrumentDescriptor: defaultInstrumentDescriptor,
pointDataType: PointDataType.HISTOGRAM,
pointData: [
{
attributes: {},
startTime: sdkStartTime,
endTime: collectionTime,
point: {
buckets: {
boundaries: [1, 10, 100],
counts: [1, 1, 0, 0],
},
count: 2,
sum: 1,
},
},
],
};
assert.deepStrictEqual(aggregator.toMetricData(
defaultResource,
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.CUMULATIVE,
sdkStartTime,
lastCollectionTime,
collectionTime,
startTime,
endTime,
), expected);
});
});
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
import { HrTime } from '@opentelemetry/api';
import * as assert from 'assert';
import { LastValueAccumulation, LastValueAggregator } from '../../src/aggregator';
import { AggregationTemporality } from '../../src/export/AggregationTemporality';
import { MetricData, PointDataType } from '../../src/export/MetricData';
import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource, sleep } from '../util';

@@ -88,7 +87,7 @@ describe('LastValueAggregator', () => {
});

describe('toMetricData', () => {
it('transform with AggregationTemporality.DELTA', () => {
it('transform without exception', () => {
const aggregator = new LastValueAggregator();

const accumulation = aggregator.createAccumulation();
@@ -97,9 +96,8 @@ describe('LastValueAggregator', () => {
accumulation.record(1);
accumulation.record(4);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];
const startTime: HrTime = [0, 0];
const endTime: HrTime = [1, 1];

const expected: MetricData = {
resource: defaultResource,
@@ -109,8 +107,8 @@ describe('LastValueAggregator', () => {
pointData: [
{
attributes: {},
startTime: lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: 4,
},
],
@@ -120,48 +118,8 @@ describe('LastValueAggregator', () => {
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.DELTA,
sdkStartTime,
lastCollectionTime,
collectionTime,
), expected);
});

it('transform with AggregationTemporality.CUMULATIVE', () => {
const aggregator = new LastValueAggregator();

const accumulation = aggregator.createAccumulation();
accumulation.record(1);
accumulation.record(2);
accumulation.record(1);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];

const expected: MetricData = {
resource: defaultResource,
instrumentationLibrary: defaultInstrumentationLibrary,
instrumentDescriptor: defaultInstrumentDescriptor,
pointDataType: PointDataType.SINGULAR,
pointData: [
{
attributes: {},
startTime: sdkStartTime,
endTime: collectionTime,
point: 1,
},
],
};
assert.deepStrictEqual(aggregator.toMetricData(
defaultResource,
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.CUMULATIVE,
sdkStartTime,
lastCollectionTime,
collectionTime,
startTime,
endTime,
), expected);
});
});
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@
import { HrTime } from '@opentelemetry/api';
import * as assert from 'assert';
import { SumAccumulation, SumAggregator } from '../../src/aggregator';
import { AggregationTemporality } from '../../src/export/AggregationTemporality';
import { MetricData, PointDataType } from '../../src/export/MetricData';
import { commonValues, defaultInstrumentationLibrary, defaultInstrumentDescriptor, defaultResource } from '../util';

@@ -69,15 +68,14 @@ describe('SumAggregator', () => {
});

describe('toMetricData', () => {
it('transform with AggregationTemporality.DELTA', () => {
it('transform without exception', () => {
const aggregator = new SumAggregator();
const accumulation = aggregator.createAccumulation();
accumulation.record(1);
accumulation.record(2);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];
const startTime: HrTime = [0, 0];
const endTime: HrTime = [1, 1];

const expected: MetricData = {
resource: defaultResource,
@@ -87,8 +85,8 @@ describe('SumAggregator', () => {
pointData: [
{
attributes: {},
startTime: lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: 3,
},
],
@@ -98,46 +96,8 @@ describe('SumAggregator', () => {
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.DELTA,
sdkStartTime,
lastCollectionTime,
collectionTime,
), expected);
});

it('transform with AggregationTemporality.CUMULATIVE', () => {
const aggregator = new SumAggregator();
const accumulation = aggregator.createAccumulation();
accumulation.record(1);
accumulation.record(2);

const sdkStartTime: HrTime = [0, 0];
const lastCollectionTime: HrTime = [1, 1];
const collectionTime: HrTime = [2, 2];

const expected: MetricData = {
resource: defaultResource,
instrumentationLibrary: defaultInstrumentationLibrary,
instrumentDescriptor: defaultInstrumentDescriptor,
pointDataType: PointDataType.SINGULAR,
pointData: [
{
attributes: {},
startTime: sdkStartTime,
endTime: collectionTime,
point: 3,
},
],
};
assert.deepStrictEqual(aggregator.toMetricData(
defaultResource,
defaultInstrumentationLibrary,
defaultInstrumentDescriptor,
[[{}, accumulation]],
AggregationTemporality.CUMULATIVE,
sdkStartTime,
lastCollectionTime,
collectionTime,
startTime,
endTime,
), expected);
});
});

0 comments on commit 4909cf1

Please sign in to comment.