Skip to content

Commit 0962d71

Browse files
authored
[CODE HEALTH] Fix clang-tidy warnings in base2 exponential histogram aggregation (open-telemetry#3997)
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent a42027a commit 0962d71

4 files changed

Lines changed: 36 additions & 9 deletions

File tree

‎.github/workflows/clang-tidy.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ jobs:
1717
matrix:
1818
include:
1919
- cmake_options: all-options-abiv1-preview
20-
warning_limit: 42
20+
warning_limit: 35
2121
- cmake_options: all-options-abiv2-preview
22-
warning_limit: 44
22+
warning_limit: 37
2323
env:
2424
CC: /usr/bin/clang-18
2525
CXX: /usr/bin/clang++-18

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ Increment the:
5858
* [CODE HEALTH] Fix misc clang-tidy warnings
5959
[#3993](https://github.com/open-telemetry/opentelemetry-cpp/pull/3993)
6060

61+
* [CODE HEALTH] Fix clang-tidy warnings in base2 exponential histogram aggregation
62+
[#3997](https://github.com/open-telemetry/opentelemetry-cpp/pull/3997)
63+
6164
Important changes:
6265

6366
* Enable WITH_OTLP_RETRY_PREVIEW by default

‎sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace
3232
uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_buckets) noexcept
3333
{
3434
uint32_t scale_reduction = 0;
35-
while (static_cast<size_t>(end_index - start_index + 1) > max_buckets)
35+
while (static_cast<int64_t>(end_index) - start_index + 1 > static_cast<int64_t>(max_buckets))
3636
{
3737
start_index >>= 1;
3838
end_index >>= 1;
@@ -212,7 +212,7 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept
212212
DownscaleBuckets(point_data_.negative_buckets_, by);
213213
}
214214

215-
point_data_.scale_ -= by;
215+
point_data_.scale_ -= static_cast<int32_t>(by);
216216
indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_);
217217
}
218218

@@ -324,9 +324,9 @@ std::unique_ptr<Aggregation> Base2ExponentialHistogramAggregation::Merge(
324324
DownscaleBuckets(high_res.positive_buckets_, scale_reduction);
325325
DownscaleBuckets(low_res.negative_buckets_, scale_reduction);
326326
DownscaleBuckets(high_res.negative_buckets_, scale_reduction);
327-
low_res.scale_ -= scale_reduction;
328-
high_res.scale_ -= scale_reduction;
329-
result_value.scale_ -= scale_reduction;
327+
low_res.scale_ -= static_cast<int32_t>(scale_reduction);
328+
high_res.scale_ -= static_cast<int32_t>(scale_reduction);
329+
result_value.scale_ -= static_cast<int32_t>(scale_reduction);
330330
}
331331
}
332332

@@ -409,8 +409,8 @@ std::unique_ptr<Aggregation> Base2ExponentialHistogramAggregation::Diff(
409409
{
410410
DownscaleBuckets(right.negative_buckets_, scale_reduction);
411411
}
412-
left.scale_ -= scale_reduction;
413-
right.scale_ -= scale_reduction;
412+
left.scale_ -= static_cast<int32_t>(scale_reduction);
413+
right.scale_ -= static_cast<int32_t>(scale_reduction);
414414
}
415415

416416
Base2ExponentialHistogramPointData result_value;

‎sdk/test/metrics/aggregation_test.cc‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,3 +608,27 @@ TEST(Aggregation, Base2ExponentialHistogramAggregationMergeCountInvariant)
608608
verify_invariant(point, expected_count, "Cycle 6: indices 5-9");
609609
}
610610
}
611+
612+
TEST(Aggregation, Base2ExponentialHistogramAggregationDiffDownscale)
613+
{
614+
// Force the scale-reduction branch in Diff() by giving left and right
615+
// bucket indices whose combined span exceeds max_buckets. Each side
616+
// individually has only one bucket so neither triggers Aggregate's
617+
// internal Downscale; the reduction must happen inside Diff().
618+
Base2ExponentialHistogramAggregationConfig config;
619+
config.max_scale_ = 0;
620+
config.max_buckets_ = 5;
621+
622+
Base2ExponentialHistogramAggregation left(&config);
623+
left.Aggregate(2.0, {}); // 2^1 -> index 0
624+
625+
Base2ExponentialHistogramAggregation right(&config);
626+
right.Aggregate(128.0, {}); // 2^7 -> index 6
627+
628+
// Combined span 0..6 (7 buckets) > max_buckets=5, so Diff() must downscale.
629+
// GetScaleReduction(0, 6, 5) returns 1, so the result scale drops from 0 to -1.
630+
auto diffed = left.Diff(right);
631+
auto diffed_point = nostd::get<Base2ExponentialHistogramPointData>(diffed->ToPoint());
632+
633+
EXPECT_EQ(diffed_point.scale_, -1);
634+
}

0 commit comments

Comments
 (0)