Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/clang-tidy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ jobs:
matrix:
include:
- cmake_options: all-options-abiv1-preview
warning_limit: 42
warning_limit: 35
- cmake_options: all-options-abiv2-preview
warning_limit: 44
warning_limit: 37
env:
CC: /usr/bin/clang-18
CXX: /usr/bin/clang++-18
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ Increment the:
* [CODE HEALTH] Fix misc clang-tidy warnings
[#3993](https://github.com/open-telemetry/opentelemetry-cpp/pull/3993)

* [CODE HEALTH] Fix clang-tidy warnings in base2 exponential histogram aggregation
[#3997](https://github.com/open-telemetry/opentelemetry-cpp/pull/3997)
Comment thread
thc1006 marked this conversation as resolved.

Important changes:

* Enable WITH_OTLP_RETRY_PREVIEW by default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace
uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_buckets) noexcept
{
uint32_t scale_reduction = 0;
while (static_cast<size_t>(end_index - start_index + 1) > max_buckets)
while (static_cast<int64_t>(end_index) - start_index + 1 > static_cast<int64_t>(max_buckets))
Comment thread
thc1006 marked this conversation as resolved.
{
start_index >>= 1;
end_index >>= 1;
Expand Down Expand Up @@ -212,7 +212,7 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept
DownscaleBuckets(point_data_.negative_buckets_, by);
}

point_data_.scale_ -= by;
point_data_.scale_ -= static_cast<int32_t>(by);
indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_);
}

Expand Down Expand Up @@ -324,9 +324,9 @@ std::unique_ptr<Aggregation> Base2ExponentialHistogramAggregation::Merge(
DownscaleBuckets(high_res.positive_buckets_, scale_reduction);
DownscaleBuckets(low_res.negative_buckets_, scale_reduction);
DownscaleBuckets(high_res.negative_buckets_, scale_reduction);
low_res.scale_ -= scale_reduction;
high_res.scale_ -= scale_reduction;
result_value.scale_ -= scale_reduction;
low_res.scale_ -= static_cast<int32_t>(scale_reduction);
high_res.scale_ -= static_cast<int32_t>(scale_reduction);
result_value.scale_ -= static_cast<int32_t>(scale_reduction);
}
}

Expand Down Expand Up @@ -409,8 +409,8 @@ std::unique_ptr<Aggregation> Base2ExponentialHistogramAggregation::Diff(
{
DownscaleBuckets(right.negative_buckets_, scale_reduction);
}
left.scale_ -= scale_reduction;
right.scale_ -= scale_reduction;
left.scale_ -= static_cast<int32_t>(scale_reduction);
right.scale_ -= static_cast<int32_t>(scale_reduction);
}

Base2ExponentialHistogramPointData result_value;
Expand Down
24 changes: 24 additions & 0 deletions sdk/test/metrics/aggregation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -608,3 +608,27 @@ TEST(Aggregation, Base2ExponentialHistogramAggregationMergeCountInvariant)
verify_invariant(point, expected_count, "Cycle 6: indices 5-9");
}
}

TEST(Aggregation, Base2ExponentialHistogramAggregationDiffDownscale)
{
// Force the scale-reduction branch in Diff() by giving left and right
// bucket indices whose combined span exceeds max_buckets. Each side
// individually has only one bucket so neither triggers Aggregate's
// internal Downscale; the reduction must happen inside Diff().
Base2ExponentialHistogramAggregationConfig config;
config.max_scale_ = 0;
config.max_buckets_ = 5;

Base2ExponentialHistogramAggregation left(&config);
left.Aggregate(2.0, {}); // 2^1 -> index 0

Base2ExponentialHistogramAggregation right(&config);
right.Aggregate(128.0, {}); // 2^7 -> index 6

// Combined span 0..6 (7 buckets) > max_buckets=5, so Diff() must downscale.
// GetScaleReduction(0, 6, 5) returns 1, so the result scale drops from 0 to -1.
auto diffed = left.Diff(right);
auto diffed_point = nostd::get<Base2ExponentialHistogramPointData>(diffed->ToPoint());

EXPECT_EQ(diffed_point.scale_, -1);
}
Loading