From b0ecb249da18027a0676fdbcc04e58066c93aade Mon Sep 17 00:00:00 2001 From: Ravi Date: Wed, 15 Jul 2026 20:54:14 -0400 Subject: [PATCH 01/10] rename max_buckets_ to max_size_ --- examples/metrics_simple/metrics_ostream.cc | 2 +- .../sdk/metrics/aggregation/aggregation_config.h | 2 +- .../sdk/metrics/exemplar/reservoir_utils.h | 3 +-- sdk/src/configuration/sdk_builder.cc | 2 +- .../base2_exponential_histogram_aggregation.cc | 2 +- sdk/test/metrics/aggregation_test.cc | 16 ++++++++-------- sdk/test/metrics/histogram_aggregation_test.cc | 4 ++-- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index 46a775d32f..1834b40b9f 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -124,7 +124,7 @@ void InitMetrics(const std::string &name) new metrics_sdk::Base2ExponentialHistogramAggregationConfig); histogram_base2_aggregation_config->max_scale_ = 3; histogram_base2_aggregation_config->record_min_max_ = true; - histogram_base2_aggregation_config->max_buckets_ = 100; + histogram_base2_aggregation_config->max_size_ = 100; std::shared_ptr base2_aggregation_config( std::move(histogram_base2_aggregation_config)); diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h index 33792ca688..6b1ebff935 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h @@ -69,7 +69,7 @@ class Base2ExponentialHistogramAggregationConfig : public AggregationConfig return AggregationType::kBase2ExponentialHistogram; } - size_t max_buckets_ = 160; + size_t max_size_ = 160; int32_t max_scale_ = 20; bool record_min_max_ = true; }; diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h index 6ca88411af..f820eddc25 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_utils.h @@ -39,8 +39,7 @@ static inline size_t GetSimpleFixedReservoirDefaultSize(const AggregationType ag { const auto *histogram_agg_config = static_cast(agg_config); - return (std::min)(kMaxBase2ExponentialHistogramReservoirSize, - histogram_agg_config->max_buckets_); + return (std::min)(kMaxBase2ExponentialHistogramReservoirSize, histogram_agg_config->max_size_); } return SimpleFixedSizeExemplarReservoir::kDefaultSimpleReservoirSize; diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index 897413643d..19641c9241 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -1608,7 +1608,7 @@ SdkBuilder::CreateBase2ExponentialBucketHistogramAggregation( auto sdk = std::make_unique(); - sdk->max_buckets_ = model->max_size; + sdk->max_size_ = model->max_size; sdk->max_scale_ = static_cast(model->max_scale); sdk->record_min_max_ = model->record_min_max; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 7909b0a384..b55f99c05d 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -124,7 +124,7 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( ac = &default_config; } - point_data_.max_buckets_ = (std::max)(ac->max_buckets_, static_cast(2)); + point_data_.max_buckets_ = (std::max)(ac->max_size_, static_cast(2)); point_data_.scale_ = ac->max_scale_; point_data_.record_min_max_ = ac->record_min_max_; point_data_.min_ = (std::numeric_limits::max)(); diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index e8c7ab0dcb..3841157761 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -51,11 +51,11 @@ uint64_t SumAllBuckets(const Base2ExponentialHistogramPointData &point) return total; } -Base2ExponentialHistogramAggregationConfig MakeAggregationConfig(int max_scale, size_t max_buckets) +Base2ExponentialHistogramAggregationConfig MakeAggregationConfig(int max_scale, size_t max_size) { Base2ExponentialHistogramAggregationConfig config; - config.max_scale_ = max_scale; - config.max_buckets_ = max_buckets; + config.max_scale_ = max_scale; + config.max_size_ = max_size; return config; } @@ -315,7 +315,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) auto MAX_BUCKETS0 = 7; Base2ExponentialHistogramAggregationConfig scale0_config; scale0_config.max_scale_ = SCALE0; - scale0_config.max_buckets_ = MAX_BUCKETS0; + scale0_config.max_size_ = MAX_BUCKETS0; scale0_config.record_min_max_ = true; Base2ExponentialHistogramAggregation scale0_aggr(&scale0_config); auto point = scale0_aggr.ToPoint(); @@ -388,7 +388,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) Base2ExponentialHistogramAggregationConfig scale1_config; scale1_config.max_scale_ = 1; - scale1_config.max_buckets_ = 14; + scale1_config.max_size_ = 14; scale1_config.record_min_max_ = true; Base2ExponentialHistogramAggregation scale1_aggr(&scale1_config); @@ -445,7 +445,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregationMerge) { Base2ExponentialHistogramAggregationConfig config; config.max_scale_ = 10; - config.max_buckets_ = 100; + config.max_size_ = 100; config.record_min_max_ = true; Base2ExponentialHistogramAggregation aggr(&config); @@ -468,7 +468,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregationMerge) ASSERT_DOUBLE_EQ(aggr_point.sum_, expected_sum); ASSERT_EQ(aggr_point.zero_count_, 0); ASSERT_GT(aggr_point.scale_, -10); - ASSERT_EQ(aggr_point.max_buckets_, config.max_buckets_); + ASSERT_EQ(aggr_point.max_buckets_, config.max_size_); auto test_merge = [](const std::unique_ptr &merged_aggr, int expected_count, double expected_sum, int expected_zero_count, int expected_scale, @@ -943,7 +943,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregationDefaultConfigMerge) ExpectCountInvariant(pb.count_, pb, "b"); EXPECT_LE(pa.scale_, 20); EXPECT_LE(pb.scale_, 20); - EXPECT_EQ(pa.max_buckets_, config.max_buckets_); + EXPECT_EQ(pa.max_buckets_, config.max_size_); EXPECT_GT(pa.max_, pa.min_); EXPECT_GT(pb.max_, pb.min_); diff --git a/sdk/test/metrics/histogram_aggregation_test.cc b/sdk/test/metrics/histogram_aggregation_test.cc index 7c74b8b385..23f14f095d 100644 --- a/sdk/test/metrics/histogram_aggregation_test.cc +++ b/sdk/test/metrics/histogram_aggregation_test.cc @@ -195,8 +195,8 @@ std::shared_ptr MakeBase2ExponentialHistogramViewProvider( meter_provider->AddMetricReader(reader); Base2ExponentialHistogramAggregationConfig config; - config.max_scale_ = 5; - config.max_buckets_ = 160; + config.max_scale_ = 5; + config.max_size_ = 160; auto view = std::make_unique("exponential_histogram", "exponential_histogram_description", AggregationType::kBase2ExponentialHistogram, From 769aa4de635592d49a5c3cb8a996ac9f48a0e430 Mon Sep 17 00:00:00 2001 From: Ravi Date: Wed, 15 Jul 2026 21:42:25 -0400 Subject: [PATCH 02/10] update type to int32 --- ...e2_exponential_bucket_histogram_aggregation_configuration.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h b/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h index e684eadae7..4ddb1f04a2 100644 --- a/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h +++ b/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include "opentelemetry/sdk/configuration/aggregation_configuration.h" #include "opentelemetry/sdk/configuration/aggregation_configuration_visitor.h" @@ -32,7 +33,7 @@ class Base2ExponentialBucketHistogramAggregationConfiguration : public Aggregati visitor->VisitBase2ExponentialBucketHistogram(this); } - std::size_t max_scale{kDefaultMaxScale}; + std::int32_t max_scale{kDefaultMaxScale}; std::size_t max_size{kDefaultMaxSize}; bool record_min_max{kDefaultRecordMinMax}; }; From c5a5c917e34dfa64ab9cc995b8d911243233023f Mon Sep 17 00:00:00 2001 From: Ravi Date: Wed, 15 Jul 2026 22:16:21 -0400 Subject: [PATCH 03/10] validate config --- .../sdk/configuration/document_node.h | 5 + .../sdk/configuration/ryml_document_node.h | 3 + .../metrics/aggregation/aggregation_config.h | 7 ++ sdk/src/configuration/configuration_parser.cc | 23 +++- sdk/src/configuration/document_node.cc | 16 +++ sdk/src/configuration/ryml_document_node.cc | 27 +++++ sdk/src/configuration/sdk_builder.cc | 2 +- sdk/test/configuration/yaml_metrics_test.cc | 100 +++++++++++++++++- 8 files changed, 177 insertions(+), 6 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/configuration/document_node.h b/sdk/include/opentelemetry/sdk/configuration/document_node.h index 7a725a3116..2dc715eec9 100644 --- a/sdk/include/opentelemetry/sdk/configuration/document_node.h +++ b/sdk/include/opentelemetry/sdk/configuration/document_node.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include @@ -49,6 +50,9 @@ class DocumentNode virtual std::size_t GetRequiredInteger(const std::string &name) const = 0; virtual std::size_t GetInteger(const std::string &name, std::size_t default_value) const = 0; + virtual std::int64_t GetSignedInteger(const std::string &name, + std::int64_t default_value) const = 0; + virtual double GetRequiredDouble(const std::string &name) const = 0; virtual double GetDouble(const std::string &name, double default_value) const = 0; @@ -71,6 +75,7 @@ class DocumentNode bool BooleanFromString(const std::string &value) const; std::size_t IntegerFromString(const std::string &value) const; + std::int64_t SignedIntegerFromString(const std::string &value) const; double DoubleFromString(const std::string &value) const; }; diff --git a/sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h b/sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h index 23e559977a..1d37b0a7d2 100644 --- a/sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h +++ b/sdk/include/opentelemetry/sdk/configuration/ryml_document_node.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include #include @@ -49,6 +50,8 @@ class RymlDocumentNode : public DocumentNode std::size_t GetRequiredInteger(const std::string &name) const override; std::size_t GetInteger(const std::string &name, std::size_t default_value) const override; + std::int64_t GetSignedInteger(const std::string &name, std::int64_t default_value) const override; + double GetRequiredDouble(const std::string &name) const override; double GetDouble(const std::string &name, double default_value) const override; diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h index 6b1ebff935..064427a75f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h @@ -56,6 +56,13 @@ class HistogramAggregationConfig : public AggregationConfig bool record_min_max_ = true; }; +// Valid ranges per the declarative configuration schema; the schema defines no maximum for +// max_size. +// https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema/meter_provider.yaml +constexpr int32_t kMaxScaleMin = -10; +constexpr int32_t kMaxScaleMax = 20; +constexpr size_t kMaxSizeMin = 2; + class Base2ExponentialHistogramAggregationConfig : public AggregationConfig { public: diff --git a/sdk/src/configuration/configuration_parser.cc b/sdk/src/configuration/configuration_parser.cc index cc83201a97..08fa0987e3 100644 --- a/sdk/src/configuration/configuration_parser.cc +++ b/sdk/src/configuration/configuration_parser.cc @@ -128,6 +128,7 @@ #include "opentelemetry/sdk/configuration/view_configuration.h" #include "opentelemetry/sdk/configuration/view_selector_configuration.h" #include "opentelemetry/sdk/configuration/view_stream_configuration.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -1370,9 +1371,25 @@ ConfigurationParser::ParseBase2ExponentialBucketHistogramAggregationConfiguratio using Config = Base2ExponentialBucketHistogramAggregationConfiguration; auto model = std::make_unique(); - model->max_scale = node->GetInteger("max_scale", Config::kDefaultMaxScale); - model->max_size = node->GetInteger("max_size", Config::kDefaultMaxSize); - model->record_min_max = node->GetBoolean("record_min_max", Config::kDefaultRecordMinMax); + std::int64_t max_scale = node->GetSignedInteger("max_scale", Config::kDefaultMaxScale); + if (max_scale < opentelemetry::sdk::metrics::kMaxScaleMin || + max_scale > opentelemetry::sdk::metrics::kMaxScaleMax) + { + std::string message("Illegal max_scale: "); + message.append(std::to_string(max_scale)); + throw InvalidSchemaException(node->Location(), message); + } + model->max_scale = static_cast(max_scale); + + model->max_size = node->GetInteger("max_size", 160); + if (model->max_size < opentelemetry::sdk::metrics::kMaxSizeMin) + { + std::string message("Illegal max_size: "); + message.append(std::to_string(model->max_size)); + throw InvalidSchemaException(node->Location(), message); + } + + model->record_min_max = node->GetBoolean("record_min_max", true); return model; } diff --git a/sdk/src/configuration/document_node.cc b/sdk/src/configuration/document_node.cc index f5aafe9f76..56dc5a2762 100644 --- a/sdk/src/configuration/document_node.cc +++ b/sdk/src/configuration/document_node.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include @@ -250,6 +251,21 @@ size_t DocumentNode::IntegerFromString(const std::string &value) const return val; } +std::int64_t DocumentNode::SignedIntegerFromString(const std::string &value) const +{ + const char *ptr = value.c_str(); + char *end = nullptr; + size_t len = value.length(); + std::int64_t val = strtoll(ptr, &end, 10); + if (ptr + len != end) + { + std::string message("Illegal integer value: "); + message.append(value); + throw InvalidSchemaException(Location(), message); + } + return val; +} + double DocumentNode::DoubleFromString(const std::string &value) const { const char *ptr = value.c_str(); diff --git a/sdk/src/configuration/ryml_document_node.cc b/sdk/src/configuration/ryml_document_node.cc index ea5986ee3c..062b3214d1 100644 --- a/sdk/src/configuration/ryml_document_node.cc +++ b/sdk/src/configuration/ryml_document_node.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -281,6 +282,32 @@ size_t RymlDocumentNode::GetInteger(const std::string &name, size_t default_valu return IntegerFromString(value); } +std::int64_t RymlDocumentNode::GetSignedInteger(const std::string &name, + std::int64_t default_value) const +{ + OTEL_INTERNAL_LOG_DEBUG("RymlDocumentNode::GetSignedInteger(" << name << ", " << default_value + << ")"); + + auto ryml_child = GetRymlChildNode(name); + + if (ryml_child.invalid()) + { + return default_value; + } + + ryml::csubstr view = ryml_child.val(); + std::string value(view.str, view.len); + + value = DoSubstitution(value); + + if (value.empty()) + { + return default_value; + } + + return SignedIntegerFromString(value); +} + double RymlDocumentNode::GetRequiredDouble(const std::string &name) const { OTEL_INTERNAL_LOG_DEBUG("RymlDocumentNode::GetRequiredDouble(" << name << ")"); diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index 19641c9241..526917b03e 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -1609,7 +1609,7 @@ SdkBuilder::CreateBase2ExponentialBucketHistogramAggregation( std::make_unique(); sdk->max_size_ = model->max_size; - sdk->max_scale_ = static_cast(model->max_scale); + sdk->max_scale_ = model->max_scale; sdk->record_min_max_ = model->record_min_max; return sdk; diff --git a/sdk/test/configuration/yaml_metrics_test.cc b/sdk/test/configuration/yaml_metrics_test.cc index 7272d2fc02..304a7301a0 100644 --- a/sdk/test/configuration/yaml_metrics_test.cc +++ b/sdk/test/configuration/yaml_metrics_test.cc @@ -925,7 +925,7 @@ file_format: "1.0-metrics" stream: aggregation: base2_exponential_bucket_histogram: - max_scale: 40 + max_scale: 10 max_size: 320 record_min_max: false )"; @@ -943,12 +943,108 @@ file_format: "1.0-metrics" auto *base2_exponential_bucket_histogram = reinterpret_cast< opentelemetry::sdk::configuration::Base2ExponentialBucketHistogramAggregationConfiguration *>( aggregation); - ASSERT_EQ(base2_exponential_bucket_histogram->max_scale, 40); + ASSERT_EQ(base2_exponential_bucket_histogram->max_scale, 10); ASSERT_EQ(base2_exponential_bucket_histogram->max_size, 320); ASSERT_EQ(base2_exponential_bucket_histogram->record_min_max, false); ASSERT_EQ(view->stream->attribute_keys, nullptr); } +TEST(YamlMetrics, stream_aggregation_base2_exponential_bucket_histogram_min_values) +{ + std::string yaml = R"( +file_format: "1.0-metrics" +meter_provider: + readers: + - periodic: + exporter: + console: + views: + - selector: + stream: + aggregation: + base2_exponential_bucket_histogram: + max_scale: -10 + max_size: 2 +)"; + + auto config = DoParse(yaml); + ASSERT_NE(config, nullptr); + ASSERT_NE(config->meter_provider, nullptr); + ASSERT_EQ(config->meter_provider->views.size(), 1); + auto *view = config->meter_provider->views[0].get(); + ASSERT_NE(view, nullptr); + ASSERT_NE(view->stream, nullptr); + ASSERT_NE(view->stream->aggregation, nullptr); + auto *base2_exponential_bucket_histogram = reinterpret_cast< + opentelemetry::sdk::configuration::Base2ExponentialBucketHistogramAggregationConfiguration *>( + view->stream->aggregation.get()); + ASSERT_EQ(base2_exponential_bucket_histogram->max_scale, -10); + ASSERT_EQ(base2_exponential_bucket_histogram->max_size, 2); +} + +TEST(YamlMetrics, stream_aggregation_base2_exponential_bucket_histogram_max_scale_too_small) +{ + std::string yaml = R"( +file_format: "1.0-metrics" +meter_provider: + readers: + - periodic: + exporter: + console: + views: + - selector: + stream: + aggregation: + base2_exponential_bucket_histogram: + max_scale: -11 +)"; + + auto config = DoParse(yaml); + ASSERT_EQ(config, nullptr); +} + +TEST(YamlMetrics, stream_aggregation_base2_exponential_bucket_histogram_max_scale_too_large) +{ + std::string yaml = R"( +file_format: "1.0-metrics" +meter_provider: + readers: + - periodic: + exporter: + console: + views: + - selector: + stream: + aggregation: + base2_exponential_bucket_histogram: + max_scale: 21 +)"; + + auto config = DoParse(yaml); + ASSERT_EQ(config, nullptr); +} + +TEST(YamlMetrics, stream_aggregation_base2_exponential_bucket_histogram_max_size_too_small) +{ + std::string yaml = R"( +file_format: "1.0-metrics" +meter_provider: + readers: + - periodic: + exporter: + console: + views: + - selector: + stream: + aggregation: + base2_exponential_bucket_histogram: + max_size: 1 +)"; + + auto config = DoParse(yaml); + ASSERT_EQ(config, nullptr); +} + TEST(YamlMetrics, stream_aggregation_last_value) { std::string yaml = R"( From 4372a2af9f6182d12ea180bef381e76199591d9d Mon Sep 17 00:00:00 2001 From: Ravi Date: Wed, 15 Jul 2026 22:18:10 -0400 Subject: [PATCH 04/10] log and use default for invalid config values --- ...base2_exponential_histogram_aggregation.cc | 22 ++++++++++++-- sdk/test/metrics/aggregation_test.cc | 30 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index b55f99c05d..4803a4674f 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -124,8 +124,26 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( ac = &default_config; } - point_data_.max_buckets_ = (std::max)(ac->max_size_, static_cast(2)); - point_data_.scale_ = ac->max_scale_; + size_t max_size = ac->max_size_; + if (max_size < kMaxSizeMin) + { + OTEL_INTERNAL_LOG_WARN("[Base2ExponentialHistogramAggregation] max_size " + << max_size << " is less than " << kMaxSizeMin << "; using default " + << default_config.max_size_); + max_size = default_config.max_size_; + } + + int32_t max_scale = ac->max_scale_; + if (max_scale < kMaxScaleMin || max_scale > kMaxScaleMax) + { + OTEL_INTERNAL_LOG_WARN("[Base2ExponentialHistogramAggregation] max_scale " + << max_scale << " is out of range [" << kMaxScaleMin << ", " + << kMaxScaleMax << "]; using default " << default_config.max_scale_); + max_scale = default_config.max_scale_; + } + + point_data_.max_buckets_ = max_size; + point_data_.scale_ = max_scale; point_data_.record_min_max_ = ac->record_min_max_; point_data_.min_ = (std::numeric_limits::max)(); point_data_.max_ = (std::numeric_limits::min)(); diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 3841157761..8cd8a1a4ec 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -441,6 +441,36 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(diffd_point.positive_buckets_->Get(2), 1); } +TEST(Aggregation, Base2ExponentialHistogramAggregationInvalidConfigFallsBackToDefault) +{ + const Base2ExponentialHistogramAggregationConfig default_config; + + auto point_of = [](const Base2ExponentialHistogramAggregationConfig &config) { + Base2ExponentialHistogramAggregation aggr(&config); + return nostd::get(aggr.ToPoint()); + }; + + { + auto point = point_of(MakeAggregationConfig(kMaxScaleMax + 1, 100)); + EXPECT_EQ(point.scale_, default_config.max_scale_); + EXPECT_EQ(point.max_buckets_, 100); + } + { + auto point = point_of(MakeAggregationConfig(kMaxScaleMin - 1, 100)); + EXPECT_EQ(point.scale_, default_config.max_scale_); + } + { + auto point = point_of(MakeAggregationConfig(0, kMaxSizeMin - 1)); + EXPECT_EQ(point.max_buckets_, default_config.max_size_); + EXPECT_EQ(point.scale_, 0); + } + { + auto point = point_of(MakeAggregationConfig(kMaxScaleMin, kMaxSizeMin)); + EXPECT_EQ(point.scale_, kMaxScaleMin); + EXPECT_EQ(point.max_buckets_, kMaxSizeMin); + } +} + TEST(Aggregation, Base2ExponentialHistogramAggregationMerge) { Base2ExponentialHistogramAggregationConfig config; From d0dfdf0940d8d4f3984e9a3a44684a1016ee2400 Mon Sep 17 00:00:00 2001 From: Ravi Date: Thu, 16 Jul 2026 06:02:17 -0400 Subject: [PATCH 05/10] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d521375a6..c784c8d30f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -297,6 +297,9 @@ Increment the: * [CODE HEALTH] Fix clang-tidy `bugprone-throwing-static-initialization` warnings [#4206](https://github.com/open-telemetry/opentelemetry-cpp/pull/4206) +* [METRICS SDK] Validate Base2 Exponential Histogram Aggregation config + [#4253](https://github.com/open-telemetry/opentelemetry-cpp/pull/4253) + * Bump rules_cc from 0.2.21 to 0.2.22 [#4223](https://github.com/open-telemetry/opentelemetry-cpp/pull/4223) From 76c1dfa8a645822e95b3c2cceb15556701aeaa21 Mon Sep 17 00:00:00 2001 From: Ravi Date: Thu, 16 Jul 2026 06:56:50 -0400 Subject: [PATCH 06/10] rebase updates --- ...onential_bucket_histogram_aggregation_configuration.h | 9 +++------ sdk/src/configuration/configuration_parser.cc | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h b/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h index 4ddb1f04a2..adfaac3f94 100644 --- a/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h +++ b/sdk/include/opentelemetry/sdk/configuration/base2_exponential_bucket_histogram_aggregation_configuration.h @@ -21,12 +21,9 @@ namespace configuration class Base2ExponentialBucketHistogramAggregationConfiguration : public AggregationConfiguration { public: - // TODO: max_scale range is [-10, 20]. Negative values require GetSignedInteger in the parser and - // changing the type of max_scale to an int32_t to match the - // Base2ExponentialHistogramAggregationConfig. - static constexpr std::size_t kDefaultMaxScale = 20; // schema: minimum -10, maximum 20 - static constexpr std::size_t kDefaultMaxSize = 160; - static constexpr bool kDefaultRecordMinMax = true; + static constexpr std::int32_t kDefaultMaxScale = 20; // schema: minimum -10, maximum 20 + static constexpr std::size_t kDefaultMaxSize = 160; + static constexpr bool kDefaultRecordMinMax = true; void Accept(AggregationConfigurationVisitor *visitor) const override { diff --git a/sdk/src/configuration/configuration_parser.cc b/sdk/src/configuration/configuration_parser.cc index 08fa0987e3..bfe4dfa373 100644 --- a/sdk/src/configuration/configuration_parser.cc +++ b/sdk/src/configuration/configuration_parser.cc @@ -1381,7 +1381,7 @@ ConfigurationParser::ParseBase2ExponentialBucketHistogramAggregationConfiguratio } model->max_scale = static_cast(max_scale); - model->max_size = node->GetInteger("max_size", 160); + model->max_size = node->GetInteger("max_size", Config::kDefaultMaxSize); if (model->max_size < opentelemetry::sdk::metrics::kMaxSizeMin) { std::string message("Illegal max_size: "); @@ -1389,7 +1389,7 @@ ConfigurationParser::ParseBase2ExponentialBucketHistogramAggregationConfiguratio throw InvalidSchemaException(node->Location(), message); } - model->record_min_max = node->GetBoolean("record_min_max", true); + model->record_min_max = node->GetBoolean("record_min_max", Config::kDefaultRecordMinMax); return model; } From 1f076dbf168a36130f1ef86bc327c11ed2db8153 Mon Sep 17 00:00:00 2001 From: Ravi Date: Thu, 16 Jul 2026 20:09:19 -0400 Subject: [PATCH 07/10] use std:: for int constants --- .../sdk/metrics/aggregation/aggregation_config.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h index 064427a75f..e96736fca9 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h @@ -59,9 +59,9 @@ class HistogramAggregationConfig : public AggregationConfig // Valid ranges per the declarative configuration schema; the schema defines no maximum for // max_size. // https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema/meter_provider.yaml -constexpr int32_t kMaxScaleMin = -10; -constexpr int32_t kMaxScaleMax = 20; -constexpr size_t kMaxSizeMin = 2; +constexpr std::int32_t kMaxScaleMin = -10; +constexpr std::int32_t kMaxScaleMax = 20; +constexpr std::size_t kMaxSizeMin = 2; class Base2ExponentialHistogramAggregationConfig : public AggregationConfig { From 1e7c8ad3b9cb37e56ed3bede60fd203fbce2d9b3 Mon Sep 17 00:00:00 2001 From: Ravi Date: Thu, 16 Jul 2026 20:09:19 -0400 Subject: [PATCH 08/10] use std::stoll for signed int parsing --- sdk/src/configuration/document_node.cc | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/sdk/src/configuration/document_node.cc b/sdk/src/configuration/document_node.cc index 56dc5a2762..61af16de90 100644 --- a/sdk/src/configuration/document_node.cc +++ b/sdk/src/configuration/document_node.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "opentelemetry/sdk/common/env_variables.h" @@ -253,17 +254,30 @@ size_t DocumentNode::IntegerFromString(const std::string &value) const std::int64_t DocumentNode::SignedIntegerFromString(const std::string &value) const { - const char *ptr = value.c_str(); - char *end = nullptr; - size_t len = value.length(); - std::int64_t val = strtoll(ptr, &end, 10); - if (ptr + len != end) + try + { + std::size_t pos = 0; + std::int64_t val = std::stoll(value, &pos); + if (pos != value.length()) + { + std::string message("Illegal integer value: "); + message.append(value); + throw InvalidSchemaException(Location(), message); + } + return val; + } + catch (const std::invalid_argument &) + { + std::string message("Illegal integer value: "); + message.append(value); + throw InvalidSchemaException(Location(), message); + } + catch (const std::out_of_range &) { std::string message("Illegal integer value: "); message.append(value); throw InvalidSchemaException(Location(), message); } - return val; } double DocumentNode::DoubleFromString(const std::string &value) const From 4f99033e80c6b0cd4e984abdbcb9e7ff88638901 Mon Sep 17 00:00:00 2001 From: Ravi Date: Thu, 16 Jul 2026 20:14:59 -0400 Subject: [PATCH 09/10] move changelog entry to correct place after rebase --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c784c8d30f..fafe97c8b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ Increment the: * [RELEASE] Bump main branch to 1.29.0-dev [#4259](https://github.com/open-telemetry/opentelemetry-cpp/pull/4259) +* [METRICS SDK] Validate Base2 Exponential Histogram Aggregation config + [#4253](https://github.com/open-telemetry/opentelemetry-cpp/pull/4253) + ## [1.28.0] 2026-07-16 * [RELEASE] Bump main branch to 1.28.0-dev @@ -297,9 +300,6 @@ Increment the: * [CODE HEALTH] Fix clang-tidy `bugprone-throwing-static-initialization` warnings [#4206](https://github.com/open-telemetry/opentelemetry-cpp/pull/4206) -* [METRICS SDK] Validate Base2 Exponential Histogram Aggregation config - [#4253](https://github.com/open-telemetry/opentelemetry-cpp/pull/4253) - * Bump rules_cc from 0.2.21 to 0.2.22 [#4223](https://github.com/open-telemetry/opentelemetry-cpp/pull/4223) From aa498c713de32eae4f48347c74af302e5e9b04d0 Mon Sep 17 00:00:00 2001 From: Ravi Date: Sat, 18 Jul 2026 08:08:44 -0400 Subject: [PATCH 10/10] add breaking change section to changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e23a186dd..906d4c3e2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,13 @@ Increment the: * [METRICS SDK] Validate Base2 Exponential Histogram Aggregation config [#4253](https://github.com/open-telemetry/opentelemetry-cpp/pull/4253) +Breaking changes: + +* [METRICS SDK] Rename Base2 Exponential Histogram Aggregation config field + [#4253](https://github.com/open-telemetry/opentelemetry-cpp/pull/4253) + * The public configuration member `max_buckets_` was renamed to `max_size_` to + match the configuration schema. Please adjust SDK configuration accordingly. + ## [1.28.0] 2026-07-16 * [RELEASE] Bump main branch to 1.28.0-dev