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

fix(sdk-metrics-base): remove aggregator.toMetricData dependency on AggregationTemporality #2676

Merged
merged 5 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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(),
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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(),
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Up @@ -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>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -101,8 +99,8 @@ describe('HistogramAggregator', () => {
pointData: [
{
attributes: {},
startTime: lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: {
buckets: {
boundaries: [1, 10, 100],
Expand All @@ -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);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand All @@ -109,8 +107,8 @@ describe('LastValueAggregator', () => {
pointData: [
{
attributes: {},
startTime: lastCollectionTime,
endTime: collectionTime,
startTime,
endTime,
point: 4,
},
],
Expand All @@ -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);
});
});
Expand Down
Loading