Skip to content

[METRICS SDK] Validate Base2 Exponential Histogram Aggregation config - #4253

Merged
marcalff merged 11 commits into
open-telemetry:mainfrom
RaviTriv:ravtrive/base2-histogram-config-validation
Jul 18, 2026
Merged

marcalff merged 11 commits into
open-telemetry:mainfrom
RaviTriv:ravtrive/base2-histogram-config-validation

Conversation

@RaviTriv

@RaviTriv RaviTriv commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #4250

Changes

Validates the Base2 Exponential Histogram Aggregation parameters to align with the OTEL configuration spec.

Naming and Types

  • Updated Base2ExponentialHistogramAggregationConfig::max_buckets_ to max_size_
  • Set Base2ExponentialBucketHistogramAggregationConfiguration::max_scale type to int32
    • Added DocumentNode::GetSignedInteger to handle negative values

Validation

  • Throw InvalidSchemaException if config value is out of range.
  • Log warning and set to default value.
  • Updated defaults to align with the spec.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@codecov

codecov Bot commented Jul 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.64286% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.12%. Comparing base (b1d403d) to head (aa498c7).

Files with missing lines Patch % Lines
sdk/src/configuration/document_node.cc 26.32% 14 Missing ⚠️
sdk/src/configuration/sdk_builder.cc 0.00% 2 Missing ⚠️
sdk/src/configuration/ryml_document_node.cc 90.91% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4253      +/-   ##
==========================================
- Coverage   81.16%   81.12%   -0.04%     
==========================================
  Files         444      444              
  Lines       18800    18848      +48     
==========================================
+ Hits        15258    15289      +31     
- Misses       3542     3559      +17     
Files with missing lines Coverage Δ
...ntial_bucket_histogram_aggregation_configuration.h 0.00% <ø> (ø)
...de/opentelemetry/sdk/configuration/document_node.h 100.00% <ø> (ø)
...entelemetry/sdk/configuration/ryml_document_node.h 100.00% <ø> (ø)
...metry/sdk/metrics/aggregation/aggregation_config.h 100.00% <ø> (ø)
...entelemetry/sdk/metrics/exemplar/reservoir_utils.h 100.00% <100.00%> (ø)
sdk/src/configuration/configuration_parser.cc 78.30% <100.00%> (+0.24%) ⬆️
...egation/base2_exponential_histogram_aggregation.cc 96.74% <100.00%> (+0.12%) ⬆️
sdk/src/configuration/ryml_document_node.cc 80.40% <90.91%> (+0.49%) ⬆️
sdk/src/configuration/sdk_builder.cc 45.05% <0.00%> (ø)
sdk/src/configuration/document_node.cc 72.90% <26.32%> (-6.01%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RaviTriv
RaviTriv force-pushed the ravtrive/base2-histogram-config-validation branch from e193ae0 to 9c689b9 Compare July 16, 2026 10:56
@RaviTriv
RaviTriv marked this pull request as ready for review July 16, 2026 12:43
@RaviTriv
RaviTriv requested a review from a team as a code owner July 16, 2026 12:43

@dbarker dbarker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Please see a few minor comments.

Comment thread sdk/src/configuration/document_node.cc Resolved
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: RaviTriv / name: Ravi (2769b780f9a1e7210f605f720d9416ef9db9dab1, 469a3fcda92704e43a315a0bae3803c342da5d3f, 72a07691df51c88d91a78635b867d0e13efe3b11, 9c689b90700994d71b5db63880c43d078672154d, 9e1fdfdde9d8212f7466fe94068c847922c33a00, 9e60342c9bdb362ff0cdd1c5ff31dcda6fe51d2e, ebabe244c9b0988cab8a4129f6855da42eb6a461, fb79f7d47f6f16fbdae9b2b2ae2f6a9d7b8f027e)

@RaviTriv
RaviTriv force-pushed the ravtrive/base2-histogram-config-validation branch from b2bd516 to 72a0769 Compare July 17, 2026 00:09
@RaviTriv
RaviTriv force-pushed the ravtrive/base2-histogram-config-validation branch from 72a0769 to 4f99033 Compare July 17, 2026 00:15

@dbarker dbarker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Please see a non blocking comment below.

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please add a breaking change section in the changelog.

}

size_t max_buckets_ = 160;
size_t max_size_ = 160;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a visible change in the SDK configuration interface.

Please add a "breaking change" section in the CHANGELOG, to say max_buckets_ was renamed to max_size_.

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix.

@marcalff
marcalff merged commit 54f57f3 into open-telemetry:main Jul 18, 2026
72 checks passed
@RaviTriv
RaviTriv deleted the ravtrive/base2-histogram-config-validation branch July 20, 2026 10:56
ayush-that pushed a commit to ayush-that/opentelemetry-cpp that referenced this pull request Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Base2ExponentialHistogramAggregation configuration validation

3 participants