[METRICS SDK] Enforce a runtime minimum scale for Base2ExponentialHistogramAggregation - #4353
[METRICS SDK] Enforce a runtime minimum scale for Base2ExponentialHistogramAggregation#4353ThomsonTan wants to merge 11 commits into
Conversation
…togramAggregation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4353 +/- ##
==========================================
- Coverage 82.49% 82.48% -0.00%
==========================================
Files 502 502
Lines 19899 19934 +35
==========================================
+ Hits 16414 16441 +27
- Misses 3485 3493 +8
🚀 New features to boost your workflow:
|
| // max_size_ == 2 configuration, and the extra slot is only reachable at the floor. | ||
| size_t BucketCapacity(size_t max_buckets) noexcept | ||
| { | ||
| return (std::max)(max_buckets, kMinBucketsAtFloor); |
There was a problem hiding this comment.
This can create 3 buckets when the user configured max_size = 2. Since max_size is the maximum number of buckets, can we use -11 as the runtime floor instead? At -11, the full double range fits in 2 buckets, so we do not need this exception.
There was a problem hiding this comment.
Good catch, thanks. Changed the runtime floor to -11 and dropped kMinBucketsAtFloor, so max_size is now a hard cap. At -11 every finite double maps to an index in [-1, 0], and the max_size = 2 repro from #4325 now ends at scale -11 with a two-bucket span and capacity 2.
Fixes #4325
Changes
This change enforces a runtime minimum scale of -10 for base2 exponential histograms, guarantees sufficient bucket capacity at that floor to preserve recorded counts, centralizes scale reduction across record, merge, and diff paths, guards the indexer against oversized shifts, and adds regression tests plus changelog documentation for the new behavior.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes