Skip to content

Commit 8c00b6f

Browse files
authored
Merge branch 'main' into fix/recording-NaN-values
2 parents 1414982 + 6d276f4 commit 8c00b6f

File tree

3 files changed

+59
-0
lines changed

3 files changed

+59
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
1515

1616
### :bug: (Bug Fix)
1717

18+
* fix(sdk-metrics): handle zero bucket counts in exponential histogram merge [#4459](https://github.com/open-telemetry/opentelemetry-js/pull/4459) @mwear
1819
* fix(sdk-metrics): ignore `NaN` value recordings in Histograms [#4455](https://github.com/open-telemetry/opentelemetry-js/pull/4455) @pichlermarc
1920
* fixes a bug where recording `NaN` on a histogram would result in the sum of bucket count values not matching the overall count
2021

packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,10 @@ export class ExponentialHistogramAccumulation implements Accumulation {
350350
return;
351351
}
352352

353+
if (buckets.length === 0) {
354+
buckets.indexStart = buckets.indexEnd = buckets.indexBase = index;
355+
}
356+
353357
if (index < buckets.indexStart) {
354358
const span = buckets.indexEnd - index;
355359
if (span >= buckets.backing.length) {

packages/sdk-metrics/test/aggregator/ExponentialHistogram.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,49 @@ describe('ExponentialHistogramAggregation', () => {
597597
const result = agg.merge(acc0, acc1);
598598
assert.strictEqual(result.startTime, acc0.startTime);
599599
});
600+
it('handles zero-length buckets in source histogram', () => {
601+
// https://github.com/open-telemetry/opentelemetry-js/issues/4450
602+
const delta = new ExponentialHistogramAccumulation([0, 0], 160);
603+
delta.updateByIncrement(0.0, 2); // A histogram with zero count of two and empty buckets
604+
605+
const previous = new ExponentialHistogramAccumulation([0, 0], 160);
606+
previous.updateByIncrement(0, 1);
607+
previous.updateByIncrement(0.000979, 41); //Bucket: (0.00097656, 0.0010198], Count: 41, Index: -160
608+
previous.updateByIncrement(0.001959, 17); //Bucket: (0.00195313, 0.0020396], Count: 17, Index: -144
609+
previous.updateByIncrement(0.002889, 1); //Bucket: (0.00288443, 0.00301213], Count: 1, Index: -135
610+
previous.updateByIncrement(0.003909, 1); //Bucket: (0.00390625, 0.00407919], Count: 1, Index: -128
611+
previous.updateByIncrement(0.004859, 2); //Bucket: (0.00485101, 0.00506578], Count: 2, Index: -123
612+
previous.updateByIncrement(0.008899, 1); //Bucket: (0.00889679, 0.00929068], Count: 1, Index: -109
613+
previous.updateByIncrement(0.018589, 1); //Bucket: (0.01858136, 0.01940403], Count: 1, Index: -92
614+
previous.updateByIncrement(0.020269, 2); //Bucket: (0.02026312, 0.02116024], Count: 2, Index: -90
615+
previous.updateByIncrement(0.021169, 3); //Bucket: (0.02116024, 0.02209709], Count: 3, Index: -89
616+
previous.updateByIncrement(0.023079, 2); //Bucket: (0.02307541, 0.02409704], Count: 2, Index: -87
617+
previous.updateByIncrement(0.025169, 2); //Bucket: (0.02516391, 0.02627801], Count: 2, Index: -85
618+
previous.updateByIncrement(0.026279, 1); //Bucket: (0.02627801, 0.02744144], Count: 1, Index: -84
619+
previous.updateByIncrement(0.029929, 2); //Bucket: (0.0299251, 0.03125], Count: 2, Index: -81
620+
previous.updateByIncrement(0.031259, 1); //Bucket: (0.03125, 0.03263356], Count: 1, Index: -80
621+
previous.updateByIncrement(0.032639, 1); //Bucket: (0.03263356, 0.03407837], Count: 1, Index: -79
622+
previous.updateByIncrement(0.037169, 1); //Bucket: (0.03716272, 0.03880806], Count: 1, Index: -76
623+
previous.updateByIncrement(0.038809, 1); //Bucket: (0.03880806, 0.04052624], Count: 1, Index: -75
624+
previous.updateByIncrement(0.042329, 1); //Bucket: (0.04232049, 0.04419417], Count: 1, Index: -73
625+
previous.updateByIncrement(0.044199, 1); //Bucket: (0.04419417, 0.04615082], Count: 1, Index: -72
626+
previous.updateByIncrement(0.048199, 1); //Bucket: (0.04819409, 0.05032782], Count: 1, Index: -70
627+
previous.updateByIncrement(0.065269, 1); //Bucket: (0.06526711, 0.06815673], Count: 1, Index: -63
628+
previous.updateByIncrement(0.092309, 1); //Bucket: (0.09230163, 0.09638818], Count: 1, Index: -55
629+
previous.updateByIncrement(0.100659, 1); //Bucket: (0.10065565, 0.10511205], Count: 1, Index: -53
630+
631+
const result = delta.clone();
632+
result.merge(previous);
633+
634+
assert.equal(result.count, delta.count + previous.count);
635+
assert.equal(result.count, bucketCounts(result));
636+
assert.equal(delta.count, bucketCounts(delta));
637+
assert.equal(previous.count, bucketCounts(previous));
638+
assert.equal(
639+
bucketCounts(result),
640+
bucketCounts(delta) + bucketCounts(previous)
641+
);
642+
});
600643
});
601644

602645
describe('diff', () => {
@@ -838,3 +881,14 @@ function bucketsToString(buckets: Buckets): string {
838881
str += ']\n';
839882
return str;
840883
}
884+
885+
function bucketCounts(histo: ExponentialHistogramAccumulation): number {
886+
// zero counts do not get a dedicated bucket, but they are part of the overall
887+
// histogram count
888+
return histo
889+
.toPointValue()
890+
.positive.bucketCounts.reduce(
891+
(total, current) => (total += current),
892+
histo.zeroCount
893+
);
894+
}

0 commit comments

Comments
 (0)