Skip to content

Commit 5d9da2b

Browse files
authored
Merge branch 'main' into fix/allow-emtpy-bucket-advice
2 parents 5a31f7f + 6d276f4 commit 5d9da2b

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
@@ -18,6 +18,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
1818

1919
* fix(sdk-metrics): allow single bucket histograms [#4456](https://github.com/open-telemetry/opentelemetry-js/pull/4456) @pichlermarc
2020
* fixes a bug where `Meter.createHistogram()` with the advice `explicitBucketBoundaries: []` would throw
21+
* fix(sdk-metrics): handle zero bucket counts in exponential histogram merge [#4459](https://github.com/open-telemetry/opentelemetry-js/pull/4459) @mwear
2122

2223
### :books: (Refine Doc)
2324

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@ export class ExponentialHistogramAccumulation implements Accumulation {
344344
return;
345345
}
346346

347+
if (buckets.length === 0) {
348+
buckets.indexStart = buckets.indexEnd = buckets.indexBase = index;
349+
}
350+
347351
if (index < buckets.indexStart) {
348352
const span = buckets.indexEnd - index;
349353
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
@@ -583,6 +583,49 @@ describe('ExponentialHistogramAggregation', () => {
583583
const result = agg.merge(acc0, acc1);
584584
assert.strictEqual(result.startTime, acc0.startTime);
585585
});
586+
it('handles zero-length buckets in source histogram', () => {
587+
// https://github.com/open-telemetry/opentelemetry-js/issues/4450
588+
const delta = new ExponentialHistogramAccumulation([0, 0], 160);
589+
delta.updateByIncrement(0.0, 2); // A histogram with zero count of two and empty buckets
590+
591+
const previous = new ExponentialHistogramAccumulation([0, 0], 160);
592+
previous.updateByIncrement(0, 1);
593+
previous.updateByIncrement(0.000979, 41); //Bucket: (0.00097656, 0.0010198], Count: 41, Index: -160
594+
previous.updateByIncrement(0.001959, 17); //Bucket: (0.00195313, 0.0020396], Count: 17, Index: -144
595+
previous.updateByIncrement(0.002889, 1); //Bucket: (0.00288443, 0.00301213], Count: 1, Index: -135
596+
previous.updateByIncrement(0.003909, 1); //Bucket: (0.00390625, 0.00407919], Count: 1, Index: -128
597+
previous.updateByIncrement(0.004859, 2); //Bucket: (0.00485101, 0.00506578], Count: 2, Index: -123
598+
previous.updateByIncrement(0.008899, 1); //Bucket: (0.00889679, 0.00929068], Count: 1, Index: -109
599+
previous.updateByIncrement(0.018589, 1); //Bucket: (0.01858136, 0.01940403], Count: 1, Index: -92
600+
previous.updateByIncrement(0.020269, 2); //Bucket: (0.02026312, 0.02116024], Count: 2, Index: -90
601+
previous.updateByIncrement(0.021169, 3); //Bucket: (0.02116024, 0.02209709], Count: 3, Index: -89
602+
previous.updateByIncrement(0.023079, 2); //Bucket: (0.02307541, 0.02409704], Count: 2, Index: -87
603+
previous.updateByIncrement(0.025169, 2); //Bucket: (0.02516391, 0.02627801], Count: 2, Index: -85
604+
previous.updateByIncrement(0.026279, 1); //Bucket: (0.02627801, 0.02744144], Count: 1, Index: -84
605+
previous.updateByIncrement(0.029929, 2); //Bucket: (0.0299251, 0.03125], Count: 2, Index: -81
606+
previous.updateByIncrement(0.031259, 1); //Bucket: (0.03125, 0.03263356], Count: 1, Index: -80
607+
previous.updateByIncrement(0.032639, 1); //Bucket: (0.03263356, 0.03407837], Count: 1, Index: -79
608+
previous.updateByIncrement(0.037169, 1); //Bucket: (0.03716272, 0.03880806], Count: 1, Index: -76
609+
previous.updateByIncrement(0.038809, 1); //Bucket: (0.03880806, 0.04052624], Count: 1, Index: -75
610+
previous.updateByIncrement(0.042329, 1); //Bucket: (0.04232049, 0.04419417], Count: 1, Index: -73
611+
previous.updateByIncrement(0.044199, 1); //Bucket: (0.04419417, 0.04615082], Count: 1, Index: -72
612+
previous.updateByIncrement(0.048199, 1); //Bucket: (0.04819409, 0.05032782], Count: 1, Index: -70
613+
previous.updateByIncrement(0.065269, 1); //Bucket: (0.06526711, 0.06815673], Count: 1, Index: -63
614+
previous.updateByIncrement(0.092309, 1); //Bucket: (0.09230163, 0.09638818], Count: 1, Index: -55
615+
previous.updateByIncrement(0.100659, 1); //Bucket: (0.10065565, 0.10511205], Count: 1, Index: -53
616+
617+
const result = delta.clone();
618+
result.merge(previous);
619+
620+
assert.equal(result.count, delta.count + previous.count);
621+
assert.equal(result.count, bucketCounts(result));
622+
assert.equal(delta.count, bucketCounts(delta));
623+
assert.equal(previous.count, bucketCounts(previous));
624+
assert.equal(
625+
bucketCounts(result),
626+
bucketCounts(delta) + bucketCounts(previous)
627+
);
628+
});
586629
});
587630

588631
describe('diff', () => {
@@ -824,3 +867,14 @@ function bucketsToString(buckets: Buckets): string {
824867
str += ']\n';
825868
return str;
826869
}
870+
871+
function bucketCounts(histo: ExponentialHistogramAccumulation): number {
872+
// zero counts do not get a dedicated bucket, but they are part of the overall
873+
// histogram count
874+
return histo
875+
.toPointValue()
876+
.positive.bucketCounts.reduce(
877+
(total, current) => (total += current),
878+
histo.zeroCount
879+
);
880+
}

0 commit comments

Comments
 (0)