Skip to content

Commit

Permalink
fix(sdk-metrics): Update default Histogram's boundary to match OTEL's…
Browse files Browse the repository at this point in the history
  • Loading branch information
chigia001 authored and pichlermarc committed Jun 26, 2023
1 parent 186815b commit 4fb7f00
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :bug: (Bug Fix)

* fix(sdk-metrics): Update default Histogram's boundary to match OTEL's spec [#3893](https://github.com/open-telemetry/opentelemetry-js/pull/3893/) @chigia001

### :books: (Refine Doc)

### :house: (Internal)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,12 @@ describe('PrometheusExporter', () => {
`test_histogram_bucket{key1="attributeValue1",le="100"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="250"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="500"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="750"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="1000"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="2500"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="5000"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="7500"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="10000"} 1 ${mockedHrTimeMs}`,
`test_histogram_bucket{key1="attributeValue1",le="+Inf"} 1 ${mockedHrTimeMs}`,
'',
]);
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk-metrics/src/view/Aggregation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class LastValueAggregation extends Aggregation {
*/
export class HistogramAggregation extends Aggregation {
private static DEFAULT_INSTANCE = new HistogramAggregator(
[0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
[0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000],
true
);
createAggregator(_instrument: InstrumentDescriptor) {
Expand Down
42 changes: 30 additions & 12 deletions packages/sdk-metrics/test/Instruments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,11 @@ describe('Instruments', () => {
attributes: {},
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 2,
sum: 10,
Expand All @@ -310,8 +313,11 @@ describe('Instruments', () => {
attributes: { foo: 'bar' },
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 2,
sum: 100,
Expand Down Expand Up @@ -352,8 +358,11 @@ describe('Instruments', () => {
attributes: {},
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 2,
sum: 110,
Expand All @@ -379,8 +388,11 @@ describe('Instruments', () => {
attributes: {},
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 0, 0, 2, 0, 0, 1, 1, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 0, 0, 2, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 4,
sum: 220,
Expand Down Expand Up @@ -422,8 +434,11 @@ describe('Instruments', () => {
attributes: {},
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 2,
sum: 10.1,
Expand All @@ -435,8 +450,11 @@ describe('Instruments', () => {
attributes: { foo: 'bar' },
value: {
buckets: {
boundaries: [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000],
counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0],
boundaries: [
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000,
7500, 10000,
],
counts: [0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0],
},
count: 2,
sum: 100.1,
Expand Down
5 changes: 4 additions & 1 deletion packages/sdk-metrics/test/view/Aggregation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,10 @@ describe('HistogramAggregator', () => {
assert(aggregator instanceof HistogramAggregator);
assert.deepStrictEqual(
aggregator['_boundaries'],
[0, 5, 10, 25, 50, 75, 100, 250, 500, 1000]
[
0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500,
10000,
]
);
});
});
Expand Down

0 comments on commit 4fb7f00

Please sign in to comment.