From 8d2967e80084d3cf4b5f79a1d9fadf57e6168184 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 30 Jun 2026 10:16:01 +0530 Subject: [PATCH 1/6] [Metrics SDK] Implement configurable cardinality limit --- CHANGELOG.md | 3 ++ .../opentelemetry/sdk/metrics/meter_context.h | 10 ++++ .../opentelemetry/sdk/metrics/metric_reader.h | 34 ++++++++++++++ .../sdk/metrics/state/async_metric_storage.h | 8 +++- .../sdk/metrics/state/metric_collector.h | 16 +++++++ .../sdk/metrics/state/sync_metric_storage.h | 8 +++- sdk/src/configuration/sdk_builder.cc | 44 +++++++++++++++-- sdk/src/metrics/meter.cc | 46 +++++++++++++++++- sdk/src/metrics/meter_context.cc | 16 +++++++ sdk/src/metrics/metric_reader.cc | 47 ++++++++++++++++++- sdk/src/metrics/state/metric_collector.cc | 6 +++ 11 files changed, 226 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c462112a7..88719eed4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ Increment the: ## [Unreleased] +* [METRICS SDK] Implement configurable cardinality limit at three priority levels + [#3292](https://github.com/open-telemetry/opentelemetry-cpp/issues/3292) + * [SDK] Add `TracerProvider::UpdateTracerConfigurator()` and example [#4065](https://github.com/open-telemetry/opentelemetry-cpp/issues/4065) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index 9ccd766565..eb585cfbb4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -108,6 +109,15 @@ class MeterContext : public std::enable_shared_from_this */ nostd::span> GetCollectors() noexcept; + /** + * Get the cardinality limit for a given instrument type from configured readers. + * Returns the maximum limit across all readers, or 0 if no reader specifies a limit. + * + * @param instrument_type The instrument type to get the limit for + * @return The maximum cardinality limit, or 0 if not configured + */ + size_t GetReaderCardinalityLimit(InstrumentType instrument_type) noexcept; + /** * GET SDK Start time * diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index a9a60753c0..a17fbf8b83 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -5,6 +5,7 @@ #include #include +#include #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" @@ -16,6 +17,22 @@ namespace sdk { namespace metrics { +/** + * Configuration options for cardinality limits per instrument type. + * A value of 0 means use the SDK default (2000). + */ +struct CardinalityLimitOptions +{ + size_t default_limit = 0; // 0 means use SDK default (2000) + size_t counter = 0; + size_t gauge = 0; + size_t histogram = 0; + size_t observable_counter = 0; + size_t observable_gauge = 0; + size_t observable_up_down_counter = 0; + size_t up_down_counter = 0; +}; + /** * MetricReader defines the interface to collect metrics from SDK */ @@ -45,6 +62,22 @@ class MetricReader virtual AggregationTemporality GetAggregationTemporality( InstrumentType instrument_type) const noexcept = 0; + /** + * Get the cardinality limit for a given instrument type. + * Returns 0 if no limit is configured for this instrument type. + * + * @param instrument_type The instrument type to get the limit for + * @return The cardinality limit, or 0 if not configured + */ + size_t GetCardinalityLimit(InstrumentType instrument_type) const noexcept; + + /** + * Set cardinality limit options for this reader. + * + * @param options The cardinality limit options + */ + void SetCardinalityLimitOptions(const CardinalityLimitOptions &options) noexcept; + /** * Shutdown the metric reader. */ @@ -73,6 +106,7 @@ class MetricReader private: MetricProducer *metric_producer_{nullptr}; std::atomic shutdown_{false}; + CardinalityLimitOptions cardinality_limit_options_; }; } // namespace metrics } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index a3f56ef64d..5efc256586 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -40,8 +40,10 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora ExemplarFilterType exempler_filter_type, nostd::shared_ptr &&exemplar_reservoir, #endif - const AggregationConfig *aggregation_config) - : instrument_descriptor_(instrument_descriptor), + const AggregationConfig *aggregation_config, + std::shared_ptr owned_aggregation_config = nullptr) + : owned_aggregation_config_(std::move(owned_aggregation_config)), + instrument_descriptor_(instrument_descriptor), aggregation_type_{aggregation_type}, aggregation_config_{AggregationConfig::GetOrDefault(aggregation_config)}, cumulative_hash_map_( @@ -139,6 +141,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora } private: + // Keep owned config alive if created from reader-level defaults + std::shared_ptr owned_aggregation_config_; InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; const AggregationConfig *aggregation_config_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h index f01a36854b..9d6e79eb83 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include "opentelemetry/nostd/function_ref.h" @@ -36,6 +37,19 @@ class CollectorHandle virtual AggregationTemporality GetAggregationTemporality( InstrumentType instrument_type) noexcept = 0; + + /** + * Get the cardinality limit for a given instrument type. + * Returns 0 if no limit is configured. + * + * @param instrument_type The instrument type to get the limit for + * @return The cardinality limit, or 0 if not configured + */ + virtual size_t GetCardinalityLimit(InstrumentType instrument_type) noexcept + { + (void)instrument_type; + return 0; + } }; /** @@ -61,6 +75,8 @@ class MetricCollector : public MetricProducer, public CollectorHandle AggregationTemporality GetAggregationTemporality( InstrumentType instrument_type) noexcept override; + size_t GetCardinalityLimit(InstrumentType instrument_type) noexcept override; + /** * The callback to be called for each metric exporter. This will only be those * metrics that have been produced since the last time this method was called. diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index 0c77f74c63..dc9beca8a1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -69,8 +69,10 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage ExemplarFilterType exempler_filter_type, nostd::shared_ptr &&exemplar_reservoir, #endif - const AggregationConfig *aggregation_config) - : instrument_descriptor_(instrument_descriptor), + const AggregationConfig *aggregation_config, + std::shared_ptr owned_aggregation_config = nullptr) + : owned_aggregation_config_(std::move(owned_aggregation_config)), + instrument_descriptor_(instrument_descriptor), aggregation_config_(AggregationConfig::GetOrDefault(aggregation_config)), attributes_hashmap_( std::make_unique(aggregation_config_->cardinality_limit_)), @@ -291,6 +293,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage } #endif + // Keep owned config alive if created from reader-level defaults + std::shared_ptr owned_aggregation_config_; InstrumentDescriptor instrument_descriptor_; // hashmap to maintain the metrics for delta collection (i.e, collection since last Collect call) const AggregationConfig *aggregation_config_; diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index f60362fa9e..cc62362aba 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -1471,14 +1471,24 @@ std::unique_ptr SdkBuilder::CreatePer OTEL_INTERNAL_LOG_WARN("metric producer not supported, ignoring"); } + sdk = opentelemetry::sdk::metrics::PeriodicExportingMetricReaderFactory::Create( + std::move(exporter_sdk), options); + if (model->cardinality_limits != nullptr) { - OTEL_INTERNAL_LOG_WARN("cardinality limits not supported, ignoring"); + opentelemetry::sdk::metrics::CardinalityLimitOptions cardinality_options; + cardinality_options.default_limit = model->cardinality_limits->default_limit; + cardinality_options.counter = model->cardinality_limits->counter; + cardinality_options.gauge = model->cardinality_limits->gauge; + cardinality_options.histogram = model->cardinality_limits->histogram; + cardinality_options.observable_counter = model->cardinality_limits->observable_counter; + cardinality_options.observable_gauge = model->cardinality_limits->observable_gauge; + cardinality_options.observable_up_down_counter = + model->cardinality_limits->observable_up_down_counter; + cardinality_options.up_down_counter = model->cardinality_limits->up_down_counter; + sdk->SetCardinalityLimitOptions(cardinality_options); } - sdk = opentelemetry::sdk::metrics::PeriodicExportingMetricReaderFactory::Create( - std::move(exporter_sdk), options); - return sdk; } @@ -1496,7 +1506,17 @@ std::unique_ptr SdkBuilder::CreatePul if (model->cardinality_limits != nullptr) { - OTEL_INTERNAL_LOG_WARN("cardinality limits not supported, ignoring"); + opentelemetry::sdk::metrics::CardinalityLimitOptions cardinality_options; + cardinality_options.default_limit = model->cardinality_limits->default_limit; + cardinality_options.counter = model->cardinality_limits->counter; + cardinality_options.gauge = model->cardinality_limits->gauge; + cardinality_options.histogram = model->cardinality_limits->histogram; + cardinality_options.observable_counter = model->cardinality_limits->observable_counter; + cardinality_options.observable_gauge = model->cardinality_limits->observable_gauge; + cardinality_options.observable_up_down_counter = + model->cardinality_limits->observable_up_down_counter; + cardinality_options.up_down_counter = model->cardinality_limits->up_down_counter; + sdk->SetCardinalityLimitOptions(cardinality_options); } return sdk; @@ -1600,6 +1620,20 @@ void SdkBuilder::AddView( sdk_aggregation_config = CreateAggregationConfig(stream->aggregation, sdk_aggregation_type); + // Apply aggregation_cardinality_limit from the view stream configuration + if (stream->aggregation_cardinality_limit != 0) + { + if (sdk_aggregation_config) + { + sdk_aggregation_config->cardinality_limit_ = stream->aggregation_cardinality_limit; + } + else + { + sdk_aggregation_config = std::make_shared( + stream->aggregation_cardinality_limit); + } + } + std::unique_ptr sdk_attribute_processor; if (stream->attribute_keys != nullptr) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 4a4c2f6b68..212b12a81a 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #include #include #include @@ -22,6 +23,7 @@ #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/async_instruments.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/instruments.h" @@ -542,6 +544,26 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( else { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + + // Determine effective aggregation config with reader-level fallback + const AggregationConfig *effective_config = view.GetAggregationConfig(); + std::shared_ptr reader_config; + + // If the view does not specify a cardinality limit, use the MetricReader-level default + if (!effective_config) + { + auto ctx_ptr = meter_context_.lock(); + if (ctx_ptr) + { + size_t reader_limit = ctx_ptr->GetReaderCardinalityLimit(instrument_descriptor.type_); + if (reader_limit != 0) + { + reader_config = std::make_shared(reader_limit); + effective_config = reader_config.get(); + } + } + } + sync_storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), view.GetAttributesProcessor(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -549,7 +571,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc), #endif - view.GetAggregationConfig())); + effective_config, std::move(reader_config))); storage_registry_.insert({view_instr_desc, sync_storage}); } auto sync_multi_storage = static_cast(storages.get()); @@ -615,6 +637,26 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( else { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + + // Determine effective aggregation config with reader-level fallback + const AggregationConfig *effective_config = view.GetAggregationConfig(); + std::shared_ptr reader_config; + + // If the view does not specify a cardinality limit, use the MetricReader-level default + if (!effective_config) + { + auto ctx_ptr = meter_context_.lock(); + if (ctx_ptr) + { + size_t reader_limit = ctx_ptr->GetReaderCardinalityLimit(instrument_descriptor.type_); + if (reader_limit != 0) + { + reader_config = std::make_shared(reader_limit); + effective_config = reader_config.get(); + } + } + } + async_storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -622,7 +664,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc), #endif - view.GetAggregationConfig())); + effective_config, std::move(reader_config))); storage_registry_.insert({view_instr_desc, async_storage}); } auto async_multi_storage = static_cast(storages.get()); diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 863ef21839..69e43c3a02 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -18,6 +19,7 @@ #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" #include "opentelemetry/sdk/metrics/export/metric_filter.h" +#include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/meter.h" #include "opentelemetry/sdk/metrics/meter_config.h" #include "opentelemetry/sdk/metrics/meter_context.h" @@ -91,6 +93,20 @@ nostd::span> MeterContext::GetCollectors() noex return nostd::span>(collectors_.data(), collectors_.size()); } +size_t MeterContext::GetReaderCardinalityLimit(InstrumentType instrument_type) noexcept +{ + size_t max_limit = 0; + for (const auto &collector : collectors_) + { + size_t limit = collector->GetCardinalityLimit(instrument_type); + if (limit > max_limit) + { + max_limit = limit; + } + } + return max_limit; +} + opentelemetry::common::SystemTimestamp MeterContext::GetSDKStartTime() noexcept { return sdk_start_ts_; diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc index 65cd6be338..004263d4bb 100644 --- a/sdk/src/metrics/metric_reader.cc +++ b/sdk/src/metrics/metric_reader.cc @@ -1,9 +1,15 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include "opentelemetry/sdk/metrics/metric_reader.h" +#include +#include +#include + +#include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" +#include "opentelemetry/sdk/metrics/instruments.h" +#include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -84,6 +90,45 @@ bool MetricReader::IsShutdown() const noexcept return shutdown_.load(std::memory_order_acquire); } +size_t MetricReader::GetCardinalityLimit(InstrumentType instrument_type) const noexcept +{ + switch (instrument_type) + { + case InstrumentType::kCounter: + return cardinality_limit_options_.counter != 0 ? cardinality_limit_options_.counter + : cardinality_limit_options_.default_limit; + case InstrumentType::kHistogram: + return cardinality_limit_options_.histogram != 0 ? cardinality_limit_options_.histogram + : cardinality_limit_options_.default_limit; + case InstrumentType::kUpDownCounter: + return cardinality_limit_options_.up_down_counter != 0 + ? cardinality_limit_options_.up_down_counter + : cardinality_limit_options_.default_limit; + case InstrumentType::kObservableCounter: + return cardinality_limit_options_.observable_counter != 0 + ? cardinality_limit_options_.observable_counter + : cardinality_limit_options_.default_limit; + case InstrumentType::kObservableGauge: + return cardinality_limit_options_.observable_gauge != 0 + ? cardinality_limit_options_.observable_gauge + : cardinality_limit_options_.default_limit; + case InstrumentType::kObservableUpDownCounter: + return cardinality_limit_options_.observable_up_down_counter != 0 + ? cardinality_limit_options_.observable_up_down_counter + : cardinality_limit_options_.default_limit; + case InstrumentType::kGauge: + return cardinality_limit_options_.gauge != 0 ? cardinality_limit_options_.gauge + : cardinality_limit_options_.default_limit; + default: + return cardinality_limit_options_.default_limit; + } +} + +void MetricReader::SetCardinalityLimitOptions(const CardinalityLimitOptions &options) noexcept +{ + cardinality_limit_options_ = options; +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/metrics/state/metric_collector.cc b/sdk/src/metrics/state/metric_collector.cc index d733218835..245fef98a7 100644 --- a/sdk/src/metrics/state/metric_collector.cc +++ b/sdk/src/metrics/state/metric_collector.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -45,6 +46,11 @@ AggregationTemporality MetricCollector::GetAggregationTemporality( return aggregation_temporality; } +size_t MetricCollector::GetCardinalityLimit(InstrumentType instrument_type) noexcept +{ + return metric_reader_->GetCardinalityLimit(instrument_type); +} + MetricProducer::Result MetricCollector::Produce() noexcept { if (!meter_context_) From f7a3e8ebc9c12ddc2b8cb9a58ff3d4a8d2d4b487 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 30 Jun 2026 08:28:56 +0530 Subject: [PATCH 2/6] Retrigger CI From b18e8afd9fe5ddb6688b35b652c5975bf5e73a11 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 30 Jun 2026 21:04:05 +0530 Subject: [PATCH 3/6] Simplify implementation: remove reader-level fallback to fix per-reader semantics As noted in review, the reader-level fallback breaks per-reader semantics because all readers share the same storage with a single cardinality limit. This commit simplifies the implementation to support only: 1. View-level: aggregation_cardinality_limit in YAML view configuration 2. SDK default: 2000 (existing behavior via AggregationConfig::GetOrDefault) Reader-level limits are kept in the API for future implementation but are not applied during storage creation. This avoids the heap/memory regression for low-limit readers and maintains correct per-reader semantics. The view-level implementation still provides significant value as it's the primary configuration mechanism for users who need custom cardinality limits. --- .../sdk/metrics/state/async_metric_storage.h | 8 +--- .../sdk/metrics/state/sync_metric_storage.h | 8 +--- sdk/src/metrics/meter.cc | 42 +------------------ 3 files changed, 6 insertions(+), 52 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h index 5efc256586..a3f56ef64d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -40,10 +40,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora ExemplarFilterType exempler_filter_type, nostd::shared_ptr &&exemplar_reservoir, #endif - const AggregationConfig *aggregation_config, - std::shared_ptr owned_aggregation_config = nullptr) - : owned_aggregation_config_(std::move(owned_aggregation_config)), - instrument_descriptor_(instrument_descriptor), + const AggregationConfig *aggregation_config) + : instrument_descriptor_(instrument_descriptor), aggregation_type_{aggregation_type}, aggregation_config_{AggregationConfig::GetOrDefault(aggregation_config)}, cumulative_hash_map_( @@ -141,8 +139,6 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora } private: - // Keep owned config alive if created from reader-level defaults - std::shared_ptr owned_aggregation_config_; InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; const AggregationConfig *aggregation_config_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h index dc9beca8a1..0c77f74c63 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -69,10 +69,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage ExemplarFilterType exempler_filter_type, nostd::shared_ptr &&exemplar_reservoir, #endif - const AggregationConfig *aggregation_config, - std::shared_ptr owned_aggregation_config = nullptr) - : owned_aggregation_config_(std::move(owned_aggregation_config)), - instrument_descriptor_(instrument_descriptor), + const AggregationConfig *aggregation_config) + : instrument_descriptor_(instrument_descriptor), aggregation_config_(AggregationConfig::GetOrDefault(aggregation_config)), attributes_hashmap_( std::make_unique(aggregation_config_->cardinality_limit_)), @@ -293,8 +291,6 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage } #endif - // Keep owned config alive if created from reader-level defaults - std::shared_ptr owned_aggregation_config_; InstrumentDescriptor instrument_descriptor_; // hashmap to maintain the metrics for delta collection (i.e, collection since last Collect call) const AggregationConfig *aggregation_config_; diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 212b12a81a..1e2e378e7e 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -545,25 +545,6 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); - // Determine effective aggregation config with reader-level fallback - const AggregationConfig *effective_config = view.GetAggregationConfig(); - std::shared_ptr reader_config; - - // If the view does not specify a cardinality limit, use the MetricReader-level default - if (!effective_config) - { - auto ctx_ptr = meter_context_.lock(); - if (ctx_ptr) - { - size_t reader_limit = ctx_ptr->GetReaderCardinalityLimit(instrument_descriptor.type_); - if (reader_limit != 0) - { - reader_config = std::make_shared(reader_limit); - effective_config = reader_config.get(); - } - } - } - sync_storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), view.GetAttributesProcessor(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -571,7 +552,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc), #endif - effective_config, std::move(reader_config))); + view.GetAggregationConfig())); storage_registry_.insert({view_instr_desc, sync_storage}); } auto sync_multi_storage = static_cast(storages.get()); @@ -638,25 +619,6 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); - // Determine effective aggregation config with reader-level fallback - const AggregationConfig *effective_config = view.GetAggregationConfig(); - std::shared_ptr reader_config; - - // If the view does not specify a cardinality limit, use the MetricReader-level default - if (!effective_config) - { - auto ctx_ptr = meter_context_.lock(); - if (ctx_ptr) - { - size_t reader_limit = ctx_ptr->GetReaderCardinalityLimit(instrument_descriptor.type_); - if (reader_limit != 0) - { - reader_config = std::make_shared(reader_limit); - effective_config = reader_config.get(); - } - } - } - async_storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -664,7 +626,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc), #endif - effective_config, std::move(reader_config))); + view.GetAggregationConfig())); storage_registry_.insert({view_instr_desc, async_storage}); } auto async_multi_storage = static_cast(storages.get()); From 9c0922d79a9a22a6d83fc725d7e35a4ea143d922 Mon Sep 17 00:00:00 2001 From: om7057 Date: Tue, 30 Jun 2026 21:23:38 +0530 Subject: [PATCH 4/6] Fix IWYU warning: Add missing cardinality_limits_configuration.h include Include What You Use (IWYU) reported that sdk_builder.cc was missing an explicit include for cardinality_limits_configuration.h even though it uses CardinalityLimitOptions types defined in that header. Added the missing include to fix the IWYU warning. --- sdk/src/configuration/sdk_builder.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index cc62362aba..5d875491a4 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -31,6 +31,7 @@ #include "opentelemetry/sdk/configuration/batch_span_processor_configuration.h" #include "opentelemetry/sdk/configuration/boolean_array_attribute_value_configuration.h" #include "opentelemetry/sdk/configuration/boolean_attribute_value_configuration.h" +#include "opentelemetry/sdk/configuration/cardinality_limits_configuration.h" #include "opentelemetry/sdk/configuration/composable_probability_sampler_configuration.h" #include "opentelemetry/sdk/configuration/configuration.h" #include "opentelemetry/sdk/configuration/configured_sdk.h" From dc2df5db8204b17c53bdf0a4d94799fd4b74ad97 Mon Sep 17 00:00:00 2001 From: om7057 Date: Wed, 1 Jul 2026 00:33:55 +0530 Subject: [PATCH 5/6] Add unit tests for cardinality limit configuration Add comprehensive unit tests to cover the cardinality limit implementation: metric_reader_test.cc: - CardinalityLimitOptions: Tests setting and getting all instrument-specific limits - CardinalityLimitOptionsDefaultFallback: Tests fallback to default_limit when specific limits are 0 - CardinalityLimitOptionsMixedConfiguration: Tests mixed configuration scenarios metric_collector_test.cc: - CardinalityLimitDelegation: Tests MetricCollector delegates to MetricReader - MeterContextMaxCardinalityLimit: Tests MeterContext returns max limit across readers - MeterContextCardinalityLimitWithMultipleReaders: Tests complex multi-reader scenarios These tests cover the code paths in: - sdk/src/metrics/metric_reader.cc (GetCardinalityLimit switch statement) - sdk/src/metrics/state/metric_collector.cc (delegation) - sdk/src/metrics/meter_context.cc (max-across-readers logic) - sdk/include/opentelemetry/sdk/metrics/state/metric_collector.h (interface) Addresses Codecov coverage gaps for PR #4188. --- sdk/test/metrics/metric_collector_test.cc | 89 +++++++++++++++++++++++ sdk/test/metrics/metric_reader_test.cc | 89 +++++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/sdk/test/metrics/metric_collector_test.cc b/sdk/test/metrics/metric_collector_test.cc index fb877f84f2..4ec0d3229d 100644 --- a/sdk/test/metrics/metric_collector_test.cc +++ b/sdk/test/metrics/metric_collector_test.cc @@ -402,6 +402,95 @@ TEST_F(MetricCollectorTest, CollectWithMetricFilterTestAttributesTest2) } } +TEST_F(MetricCollectorTest, CardinalityLimitDelegation) +{ + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + + // Create reader with cardinality limits + auto reader = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options; + options.default_limit = 1000; + options.counter = 100; + options.histogram = 200; + reader->SetCardinalityLimitOptions(options); + + auto collector = AddMetricReaderToMeterContext(context, reader).lock(); + + // Test that MetricCollector delegates to MetricReader + EXPECT_EQ(collector->GetCardinalityLimit(InstrumentType::kCounter), 100); + EXPECT_EQ(collector->GetCardinalityLimit(InstrumentType::kHistogram), 200); + EXPECT_EQ(collector->GetCardinalityLimit(InstrumentType::kUpDownCounter), 1000); + EXPECT_EQ(collector->GetCardinalityLimit(InstrumentType::kObservableCounter), 1000); +} + +TEST_F(MetricCollectorTest, MeterContextMaxCardinalityLimit) +{ + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + + // Test with no readers - should return 0 + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kCounter), 0); + + // Add first reader with limits + auto reader1 = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options1; + options1.counter = 100; + options1.histogram = 500; + reader1->SetCardinalityLimitOptions(options1); + AddMetricReaderToMeterContext(context, reader1); + + // Should return reader1's limits + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kCounter), 100); + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kHistogram), 500); + + // Add second reader with different limits + auto reader2 = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options2; + options2.counter = 1000; // Higher than reader1 + options2.histogram = 200; // Lower than reader1 + reader2->SetCardinalityLimitOptions(options2); + AddMetricReaderToMeterContext(context, reader2); + + // Should return the max across both readers + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kCounter), 1000); // max(100, 1000) + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kHistogram), 500); // max(500, 200) +} + +TEST_F(MetricCollectorTest, MeterContextCardinalityLimitWithMultipleReaders) +{ + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + + // Add three readers with different configurations + auto reader1 = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options1; + options1.default_limit = 500; + options1.counter = 100; + reader1->SetCardinalityLimitOptions(options1); + AddMetricReaderToMeterContext(context, reader1); + + auto reader2 = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options2; + options2.default_limit = 1000; + options2.histogram = 300; + reader2->SetCardinalityLimitOptions(options2); + AddMetricReaderToMeterContext(context, reader2); + + auto reader3 = std::shared_ptr(new MockMetricReader()); + CardinalityLimitOptions options3; + options3.counter = 200; + options3.histogram = 400; + reader3->SetCardinalityLimitOptions(options3); + AddMetricReaderToMeterContext(context, reader3); + + // Counter: max(100, 1000 (default), 200) = 1000 + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kCounter), 1000); + + // Histogram: max(500 (default), 300, 400) = 500 + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kHistogram), 500); + + // UpDownCounter: max(500 (default), 1000 (default), 0) = 1000 + EXPECT_EQ(context->GetReaderCardinalityLimit(InstrumentType::kUpDownCounter), 1000); +} + #if defined(__GNUC__) || defined(__clang__) || defined(__apple_build_version__) # pragma GCC diagnostic pop #endif diff --git a/sdk/test/metrics/metric_reader_test.cc b/sdk/test/metrics/metric_reader_test.cc index 3019a1225b..0fa91abc10 100644 --- a/sdk/test/metrics/metric_reader_test.cc +++ b/sdk/test/metrics/metric_reader_test.cc @@ -33,3 +33,92 @@ TEST(MetricReaderTest, BasicTests) new MetricCollector(meter_context2.get(), std::move(metric_reader2))}; metric_producer->Produce(); } + +TEST(MetricReaderTest, CardinalityLimitOptions) +{ + std::unique_ptr metric_reader(new MockMetricReader()); + + // Test default values (all 0) + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kCounter), 0); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kHistogram), 0); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kUpDownCounter), 0); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableCounter), 0); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableGauge), 0); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableUpDownCounter), 0); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kGauge), 0); +#endif + + // Set cardinality limit options + CardinalityLimitOptions options; + options.default_limit = 1000; + options.counter = 100; + options.histogram = 200; + options.up_down_counter = 300; + options.observable_counter = 400; + options.observable_gauge = 500; + options.observable_up_down_counter = 600; + options.gauge = 700; + + metric_reader->SetCardinalityLimitOptions(options); + + // Test instrument-specific limits + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kCounter), 100); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kHistogram), 200); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kUpDownCounter), 300); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableCounter), 400); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableGauge), 500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableUpDownCounter), 600); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kGauge), 700); +#endif +} + +TEST(MetricReaderTest, CardinalityLimitOptionsDefaultFallback) +{ + std::unique_ptr metric_reader(new MockMetricReader()); + + // Set only default_limit, leave instrument-specific limits at 0 + CardinalityLimitOptions options; + options.default_limit = 1500; + + metric_reader->SetCardinalityLimitOptions(options); + + // All instrument types should fall back to default_limit + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kCounter), 1500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kHistogram), 1500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kUpDownCounter), 1500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableCounter), 1500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableGauge), 1500); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableUpDownCounter), 1500); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kGauge), 1500); +#endif +} + +TEST(MetricReaderTest, CardinalityLimitOptionsMixedConfiguration) +{ + std::unique_ptr metric_reader(new MockMetricReader()); + + // Set default and only some instrument-specific limits + CardinalityLimitOptions options; + options.default_limit = 1000; + options.counter = 100; + options.histogram = 200; + // Leave other instrument types at 0 to test fallback + + metric_reader->SetCardinalityLimitOptions(options); + + // Configured instruments use their specific values + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kCounter), 100); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kHistogram), 200); + + // Unconfigured instruments fall back to default + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kUpDownCounter), 1000); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableCounter), 1000); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableGauge), 1000); + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kObservableUpDownCounter), 1000); +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + EXPECT_EQ(metric_reader->GetCardinalityLimit(InstrumentType::kGauge), 1000); +#endif +} From 2895f04fe082270536742b3d4670d4e9321a0c38 Mon Sep 17 00:00:00 2001 From: om7057 Date: Wed, 1 Jul 2026 08:25:50 +0530 Subject: [PATCH 6/6] Fix IWYU warnings: Remove unused includes from meter.cc Remove unnecessary includes as reported by include-what-you-use: - Remove (unused in meter.cc) - Remove aggregation/aggregation_config.h (provided via view/view.h) This resolves IWYU warnings in all build configurations (abiv1, abiv1-preview, abiv2-preview). --- sdk/src/metrics/meter.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 1e2e378e7e..bd111cb63c 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include #include #include #include @@ -23,7 +22,6 @@ #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" #include "opentelemetry/sdk/instrumentationscope/scope_configurator.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/async_instruments.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/instruments.h"