Skip to content

Commit d454492

Browse files
obecnyobecny
authored
chore: updating aggregator MinMaxLastSumCount and use it for value observer and value recorder (open-telemetry#1276)
* chore: updating aggregator MinMaxLastSumCount and use it for value observer and value recorder * chore: fix after merge
1 parent 85c430a commit d454492

File tree

8 files changed

+127
-71
lines changed

8 files changed

+127
-71
lines changed

‎packages/opentelemetry-exporter-collector/test/helper.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
MetricRecord,
2727
MetricKind,
2828
SumAggregator,
29-
LastValueAggregator,
29+
MinMaxLastSumCountAggregator,
3030
} from '@opentelemetry/metrics';
3131

3232
if (typeof Buffer === 'undefined') {
@@ -85,7 +85,7 @@ export const mockObserver: MetricRecord = {
8585
valueType: ValueType.DOUBLE,
8686
},
8787
labels: {},
88-
aggregator: new LastValueAggregator(),
88+
aggregator: new MinMaxLastSumCountAggregator(),
8989
resource: new Resource({
9090
service: 'ui',
9191
version: 1,

‎packages/opentelemetry-exporter-prometheus/src/prometheus.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,10 @@ export class PrometheusExporter implements MetricExporter {
161161
(point.value as Histogram).sum,
162162
hrTimeToMilliseconds(point.timestamp)
163163
);
164-
} else if (typeof (point.value as Distribution).max === 'number') {
164+
} else if (typeof (point.value as Distribution).last === 'number') {
165165
metric.set(
166166
labels,
167-
(point.value as Distribution).sum,
167+
(point.value as Distribution).last,
168168
hrTimeToMilliseconds(point.timestamp)
169169
);
170170
}

‎packages/opentelemetry-metrics/src/export/Batcher.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ export class UngroupedBatcher extends Batcher {
5757
return new aggregators.SumAggregator();
5858
case MetricKind.VALUE_RECORDER:
5959
case MetricKind.VALUE_OBSERVER:
60-
return new aggregators.LastValueAggregator();
60+
return new aggregators.MinMaxLastSumCountAggregator();
6161
default:
62-
return new aggregators.MinMaxSumCountAggregator();
62+
return new aggregators.MinMaxLastSumCountAggregator();
6363
}
6464
}
6565

‎packages/opentelemetry-metrics/src/export/aggregators/LastValue.ts

-37
This file was deleted.

‎packages/opentelemetry-metrics/src/export/aggregators/MinMaxSumCount.ts renamed to ‎packages/opentelemetry-metrics/src/export/aggregators/MinMaxLastSumCount.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,18 @@ import { HrTime } from '@opentelemetry/api';
1919
import { hrTime } from '@opentelemetry/core';
2020
import { Distribution } from '../types';
2121

22-
/** Basic aggregator keeping all raw values (events, sum, max and min). */
23-
export class MinMaxSumCountAggregator implements Aggregator {
22+
/**
23+
* Basic aggregator keeping all raw values (events, sum, max, last and min).
24+
*/
25+
export class MinMaxLastSumCountAggregator implements Aggregator {
2426
private _distribution: Distribution;
2527
private _lastUpdateTime: HrTime = [0, 0];
2628

2729
constructor() {
2830
this._distribution = {
2931
min: Infinity,
3032
max: -Infinity,
33+
last: 0,
3134
sum: 0,
3235
count: 0,
3336
};
@@ -36,6 +39,7 @@ export class MinMaxSumCountAggregator implements Aggregator {
3639
update(value: number): void {
3740
this._distribution.count++;
3841
this._distribution.sum += value;
42+
this._distribution.last = value;
3943
this._distribution.min = Math.min(this._distribution.min, value);
4044
this._distribution.max = Math.max(this._distribution.max, value);
4145
this._lastUpdateTime = hrTime();

‎packages/opentelemetry-metrics/src/export/aggregators/index.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@
1515
*/
1616

1717
export * from './histogram';
18-
export * from './MinMaxSumCount';
19-
export * from './LastValue';
18+
export * from './MinMaxLastSumCount';
2019
export * from './Sum';

‎packages/opentelemetry-metrics/src/export/types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export type LastValue = number;
3737
export interface Distribution {
3838
min: number;
3939
max: number;
40+
last: number;
4041
count: number;
4142
sum: number;
4243
}

‎packages/opentelemetry-metrics/test/Meter.test.ts

+113-24
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ import {
2727
MetricRecord,
2828
Aggregator,
2929
MetricDescriptor,
30-
LastValueAggregator,
3130
UpDownCounterMetric,
31+
Distribution,
32+
MinMaxLastSumCountAggregator,
3233
} from '../src';
3334
import * as api from '@opentelemetry/api';
3435
import { NoopLogger, hrTime, hrTimeToNanoseconds } from '@opentelemetry/core';
@@ -562,7 +563,16 @@ describe('Meter', () => {
562563

563564
await meter.collect();
564565
const [record1] = meter.getBatcher().checkPointSet();
565-
assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0);
566+
assert.deepStrictEqual(
567+
record1.aggregator.toPoint().value as Distribution,
568+
{
569+
count: 0,
570+
last: 0,
571+
max: -Infinity,
572+
min: Infinity,
573+
sum: 0,
574+
}
575+
);
566576
});
567577

568578
it('should not set the instrument data when disabled', async () => {
@@ -574,7 +584,16 @@ describe('Meter', () => {
574584

575585
await meter.collect();
576586
const [record1] = meter.getBatcher().checkPointSet();
577-
assert.deepStrictEqual(record1.aggregator.toPoint().value as number, 0);
587+
assert.deepStrictEqual(
588+
record1.aggregator.toPoint().value as Distribution,
589+
{
590+
count: 0,
591+
last: 0,
592+
max: -Infinity,
593+
min: Infinity,
594+
sum: 0,
595+
}
596+
);
578597
});
579598

580599
it(
@@ -591,8 +610,14 @@ describe('Meter', () => {
591610
await meter.collect();
592611
const [record1] = meter.getBatcher().checkPointSet();
593612
assert.deepStrictEqual(
594-
record1.aggregator.toPoint().value as number,
595-
50
613+
record1.aggregator.toPoint().value as Distribution,
614+
{
615+
count: 2,
616+
last: 50,
617+
max: 50,
618+
min: -10,
619+
sum: 40,
620+
}
596621
);
597622
assert.ok(
598623
hrTimeToNanoseconds(record1.aggregator.toPoint().timestamp) >
@@ -612,8 +637,14 @@ describe('Meter', () => {
612637
await meter.collect();
613638
const [record1] = meter.getBatcher().checkPointSet();
614639
assert.deepStrictEqual(
615-
record1.aggregator.toPoint().value as number,
616-
100
640+
record1.aggregator.toPoint().value as Distribution,
641+
{
642+
count: 2,
643+
last: 100,
644+
max: 100,
645+
min: 10,
646+
sum: 110,
647+
}
617648
);
618649
assert.strictEqual(boundValueRecorder1, boundValueRecorder2);
619650
});
@@ -809,10 +840,34 @@ describe('Meter', () => {
809840
assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
810841
assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2');
811842

812-
ensureMetric(metric1, 'cpu_temp_per_app', 67);
813-
ensureMetric(metric2, 'cpu_temp_per_app', 69);
814-
ensureMetric(metric3, 'cpu_temp_per_app', 67);
815-
ensureMetric(metric4, 'cpu_temp_per_app', 69);
843+
ensureMetric(metric1, 'cpu_temp_per_app', {
844+
count: 1,
845+
last: 67,
846+
max: 67,
847+
min: 67,
848+
sum: 67,
849+
});
850+
ensureMetric(metric2, 'cpu_temp_per_app', {
851+
count: 1,
852+
last: 69,
853+
max: 69,
854+
min: 69,
855+
sum: 69,
856+
});
857+
ensureMetric(metric3, 'cpu_temp_per_app', {
858+
count: 1,
859+
last: 67,
860+
max: 67,
861+
min: 67,
862+
sum: 67,
863+
});
864+
ensureMetric(metric4, 'cpu_temp_per_app', {
865+
count: 1,
866+
last: 69,
867+
max: 69,
868+
min: 69,
869+
sum: 69,
870+
});
816871

817872
const metric5 = cpuUsageMetricRecords[0];
818873
const metric6 = cpuUsageMetricRecords[1];
@@ -823,10 +878,34 @@ describe('Meter', () => {
823878
assert.strictEqual(hashLabels(metric3.labels), '|#app:app2,core:1');
824879
assert.strictEqual(hashLabels(metric4.labels), '|#app:app2,core:2');
825880

826-
ensureMetric(metric5, 'cpu_usage_per_app', 2.1);
827-
ensureMetric(metric6, 'cpu_usage_per_app', 3.1);
828-
ensureMetric(metric7, 'cpu_usage_per_app', 1.2);
829-
ensureMetric(metric8, 'cpu_usage_per_app', 4.5);
881+
ensureMetric(metric5, 'cpu_usage_per_app', {
882+
count: 1,
883+
last: 2.1,
884+
max: 2.1,
885+
min: 2.1,
886+
sum: 2.1,
887+
});
888+
ensureMetric(metric6, 'cpu_usage_per_app', {
889+
count: 1,
890+
last: 3.1,
891+
max: 3.1,
892+
min: 3.1,
893+
sum: 3.1,
894+
});
895+
ensureMetric(metric7, 'cpu_usage_per_app', {
896+
count: 1,
897+
last: 1.2,
898+
max: 1.2,
899+
min: 1.2,
900+
sum: 1.2,
901+
});
902+
ensureMetric(metric8, 'cpu_usage_per_app', {
903+
count: 1,
904+
last: 4.5,
905+
max: 4.5,
906+
min: 4.5,
907+
sum: 4.5,
908+
});
830909
});
831910

832911
it('should not observe values when timeout', done => {
@@ -856,9 +935,15 @@ describe('Meter', () => {
856935
const value = cpuUsageMetric
857936
.bind({ foo: 'bar' })
858937
.getAggregator()
859-
.toPoint().value as number;
860-
861-
assert.strictEqual(value, 0);
938+
.toPoint().value as Distribution;
939+
940+
assert.deepStrictEqual(value, {
941+
count: 0,
942+
last: 0,
943+
max: -Infinity,
944+
min: Infinity,
945+
sum: 0,
946+
});
862947
assert.strictEqual(cpuUsageMetricRecords.length, 0);
863948
done();
864949
});
@@ -961,13 +1046,17 @@ class CustomBatcher extends Batcher {
9611046
}
9621047
}
9631048

964-
function ensureMetric(metric: MetricRecord, name?: string, value?: number) {
965-
assert.ok(metric.aggregator instanceof LastValueAggregator);
966-
const lastValue = metric.aggregator.toPoint().value;
967-
if (typeof value === 'number') {
968-
assert.strictEqual(lastValue, value);
1049+
function ensureMetric(
1050+
metric: MetricRecord,
1051+
name?: string,
1052+
value?: Distribution
1053+
) {
1054+
assert.ok(metric.aggregator instanceof MinMaxLastSumCountAggregator);
1055+
const distribution = metric.aggregator.toPoint().value as Distribution;
1056+
if (value) {
1057+
assert.deepStrictEqual(distribution, value);
9691058
} else {
970-
assert.ok(lastValue >= 0 && lastValue <= 1);
1059+
assert.ok(distribution.last >= 0 && distribution.last <= 1);
9711060
}
9721061
const descriptor = metric.descriptor;
9731062
assert.strictEqual(descriptor.name, name || 'name');

0 commit comments

Comments
 (0)