diff --git a/CHANGELOG.md b/CHANGELOG.md index b7c48e22d2..b3133fc1f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,25 @@ Increment the: ## [Unreleased] +* [METRICS SDK] Enforce `MetricReader`-level cardinality limits as a fallback + during collection, when the matching view has no explicit + `aggregation_cardinality_limit` of its own (View > Reader > SDK default). + Shared recording storage for a view with no explicit limit is now sized at + the max limit across all attached readers, so a reader with a higher limit + does not lose data; each reader's own (possibly stricter) limit is then + re-applied to just its own collected output. A programmatically-constructed + `AggregationConfig`/`HistogramAggregationConfig`/ + `Base2ExponentialHistogramAggregationConfig` (e.g. one built only to carry + histogram boundaries) is now also correctly treated as not having an + explicit cardinality limit, matching the declarative-configuration path. + `AggregationConfig::cardinality_limit_`/`cardinality_limit_explicit_` are + now private, set together via `SetCardinalityLimit()`, so they cannot be + desynced by direct field assignment. The single-collector delta fast path + in `TemporalMetricStorage::buildMetrics()` now re-caps to the collector's + own limit instead of emitting the raw recording storage unchecked, and the + bound-instrument admission path (preview) now uses the same resolved + recording limit as the unbound path. + [#4387](https://github.com/open-telemetry/opentelemetry-cpp/issues/4387) * [CONFIGURATION] Add a configuration builder for the host resource detector [#4451](https://github.com/open-telemetry/opentelemetry-cpp/issues/4451) * [CONFIGURATION] Build the configured resource detectors in SdkBuilder, apply diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h index 8c80715846..ec31378016 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h @@ -3,6 +3,8 @@ #pragma once +#include +#include #include #include "opentelemetry/sdk/metrics/instruments.h" @@ -15,11 +17,21 @@ namespace sdk namespace metrics { +// Sentinel passed to AggregationConfig's cardinality_limit constructor argument to mean "the +// caller did not specify a limit", distinct from any real limit value (which is always a small +// positive count). Lets the constructor tell apart e.g. HistogramAggregationConfig() built only +// to carry boundaries_ from HistogramAggregationConfig(500) built to set a real limit of 500, +// without needing (avoided elsewhere in the SDK for ABI reasons). +constexpr size_t kCardinalityLimitUnspecified = (std::numeric_limits::max)(); + class AggregationConfig { public: - AggregationConfig(size_t cardinality_limit = kAggregationCardinalityLimit) - : cardinality_limit_(cardinality_limit) + AggregationConfig(size_t cardinality_limit = kCardinalityLimitUnspecified) + : cardinality_limit_(cardinality_limit == kCardinalityLimitUnspecified + ? kAggregationCardinalityLimit + : cardinality_limit), + cardinality_limit_explicit_(cardinality_limit != kCardinalityLimitUnspecified) {} AggregationConfig(const AggregationConfig &) = default; @@ -39,14 +51,33 @@ class AggregationConfig return &default_config; } - size_t cardinality_limit_; + std::size_t GetCardinalityLimit() const noexcept { return cardinality_limit_; } + + // Whether the cardinality limit reflects an intentionally-configured value, as opposed to + // just the compiled-in default it was left at because this config was constructed for some + // other reason (e.g. histogram boundaries) without a cardinality_limit argument. A + // MetricReader-level fallback applies whenever this is false. + bool IsCardinalityLimitExplicit() const noexcept { return cardinality_limit_explicit_; } + + // Sets the cardinality limit and marks it explicit, atomically. Use this instead of assigning + // a limit some other way, so the limit and its explicit-ness can never go out of sync. + void SetCardinalityLimit(std::size_t cardinality_limit) noexcept + { + cardinality_limit_ = cardinality_limit; + cardinality_limit_explicit_ = true; + } + virtual ~AggregationConfig() = default; + +private: + size_t cardinality_limit_; + bool cardinality_limit_explicit_; }; class HistogramAggregationConfig : public AggregationConfig { public: - HistogramAggregationConfig(size_t cardinality_limit = kAggregationCardinalityLimit) + HistogramAggregationConfig(size_t cardinality_limit = kCardinalityLimitUnspecified) : AggregationConfig(cardinality_limit) {} @@ -76,7 +107,7 @@ class Base2ExponentialHistogramAggregationConfig : public AggregationConfig { public: Base2ExponentialHistogramAggregationConfig( - size_t cardinality_limit = kAggregationCardinalityLimit) + size_t cardinality_limit = kCardinalityLimitUnspecified) : AggregationConfig(cardinality_limit) {} diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index 318c0c5391..ed30931450 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -142,6 +142,13 @@ class MeterContext : public std::enable_shared_from_this * Note: This reader may not receive any in-flight meter data, but will get newly created meter * data. * Note: This method is not thread safe, and should ideally be called from main thread. + * Note: For an instrument that already exists when this reader is added, this reader's + * cardinality limit for that instrument only takes effect up to whatever recording capacity + * was already resolved (as the highest limit across readers attached at instrument-creation + * time) when the instrument was first created; that capacity is fixed for the lifetime of the + * instrument and is not grown retroactively. A reader added with a higher limit than any + * reader present at instrument-creation time will not recover attribute sets that had already + * collapsed into the instrument's overflow point before this reader was attached. */ void AddMetricReader(std::shared_ptr reader, std::unique_ptr metric_filter = nullptr) noexcept; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h index fccaae690b..d7159a2049 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h @@ -104,6 +104,13 @@ class OPENTELEMETRY_EXPORT MeterProvider final : public opentelemetry::metrics:: * Note: This reader may not receive any in-flight meter data, but will get newly created meter * data. * Note: This method is not thread safe, and should ideally be called from main thread. + * Note: For an instrument that already exists when this reader is added, this reader's + * cardinality limit for that instrument only takes effect up to whatever recording capacity + * was already resolved (as the highest limit across readers attached at instrument-creation + * time) when the instrument was first created; that capacity is fixed for the lifetime of the + * instrument and is not grown retroactively. A reader added with a higher limit than any + * reader present at instrument-creation time will not recover attribute sets that had already + * collapsed into the instrument's overflow point before this reader was attached. */ void AddMetricReader(std::shared_ptr reader, std::unique_ptr metric_filter = nullptr) noexcept; diff --git a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h index 4813861314..eec71eb468 100644 --- a/sdk/include/opentelemetry/sdk/metrics/metric_reader.h +++ b/sdk/include/opentelemetry/sdk/metrics/metric_reader.h @@ -61,9 +61,8 @@ class MetricReader /** * Set per-instrument-type cardinality limits for this reader. * - * TODO: Reader-level limits are stored but not yet enforced as a per-collector - * fallback during the collection path. Enforcement will be added in a follow-up. - * View-level limits (via AggregationConfig) are enforced today. + * Enforced as a fallback during the collection path whenever the matching view has no + * explicit AggregationConfig cardinality limit of its own (View > Reader > SDK default). * * @param limits The cardinality limits to apply */ 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 674863428b..6ec0fbb6b1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/async_metric_storage.h @@ -35,6 +35,8 @@ namespace metrics class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStorage { public: + // Back-compat overload preserving the original constructor signature for any external caller. + // See SyncMetricStorage's constructor comment for what `recording_cardinality_limit` is for. AsyncMetricStorage(const InstrumentDescriptor &instrument_descriptor, const AggregationType aggregation_type, #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -42,13 +44,31 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora nostd::shared_ptr &&exemplar_reservoir, #endif const AggregationConfig *aggregation_config) + : AsyncMetricStorage( + instrument_descriptor, + aggregation_type, +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + exemplar_filter_type, + std::move(exemplar_reservoir), +#endif + aggregation_config, + AggregationConfig::GetOrDefault(aggregation_config)->GetCardinalityLimit()) + {} + + AsyncMetricStorage(const InstrumentDescriptor &instrument_descriptor, + const AggregationType aggregation_type, +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType exemplar_filter_type, + nostd::shared_ptr &&exemplar_reservoir, +#endif + const AggregationConfig *aggregation_config, + std::size_t recording_cardinality_limit) : instrument_descriptor_(instrument_descriptor), aggregation_type_{aggregation_type}, aggregation_config_{AggregationConfig::GetOrDefault(aggregation_config)}, - cumulative_hash_map_( - std::make_unique(aggregation_config_->cardinality_limit_)), - delta_hash_map_( - std::make_unique(aggregation_config_->cardinality_limit_)), + recording_cardinality_limit_(recording_cardinality_limit), + cumulative_hash_map_(std::make_unique(recording_cardinality_limit_)), + delta_hash_map_(std::make_unique(recording_cardinality_limit_)), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW exemplar_filter_type_(exemplar_filter_type), exemplar_reservoir_(std::move(exemplar_reservoir)), @@ -131,9 +151,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora std::shared_ptr delta_metrics = nullptr; { std::lock_guard guard(hashmap_lock_); - delta_metrics = std::move(delta_hash_map_); - delta_hash_map_ = - std::make_unique(aggregation_config_->cardinality_limit_); + delta_metrics = std::move(delta_hash_map_); + delta_hash_map_ = std::make_unique(recording_cardinality_limit_); } auto status = @@ -146,6 +165,8 @@ class AsyncMetricStorage : public MetricStorage, public AsyncWritableMetricStora InstrumentDescriptor instrument_descriptor_; AggregationType aggregation_type_; const AggregationConfig *aggregation_config_; + // Capacity used to (re)size cumulative_hash_map_/delta_hash_map_. See the constructor comment. + const std::size_t recording_cardinality_limit_; std::unique_ptr cumulative_hash_map_; std::unique_ptr delta_hash_map_; opentelemetry::common::SpinLockMutex hashmap_lock_; diff --git a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h index 1ca165f5af..c9f4e914a1 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/attributes_hashmap.h @@ -122,7 +122,12 @@ class AttributesHashMapWithCustomHash return result.first->second.get(); } /** - * Set the value for given key, overwriting the value if already present + * Set the value for given key, overwriting the value if already present. + * + * When `attributes` is a new key that has to be routed to the overflow entry (capacity + * reached), this merges into any existing overflow aggregation rather than replacing it, so + * that a second and later over-capacity attribute set doesn't discard values already + * accumulated there from an earlier one. */ void Set(const MetricAttributes &attributes, std::unique_ptr aggr) { @@ -133,7 +138,7 @@ class AttributesHashMapWithCustomHash } else if (IsOverflowAttributes(attributes)) { - hash_map_[GetOverflowAttributes()] = std::move(aggr); + SetOverflowMerged(std::move(aggr)); } else { @@ -150,7 +155,7 @@ class AttributesHashMapWithCustomHash } else if (IsOverflowAttributes(attributes)) { - hash_map_[GetOverflowAttributes()] = std::move(aggr); + SetOverflowMerged(std::move(aggr)); } else { @@ -207,6 +212,23 @@ class AttributesHashMapWithCustomHash return result.first->second.get(); } + // Merges `agg` into the existing overflow aggregation if one is already present, otherwise + // installs it as the overflow entry. Used by Set() when routing a new over-capacity key to + // overflow, so multiple distinct attribute sets funneled into the single overflow bucket + // accumulate rather than each replacing the last. + void SetOverflowMerged(std::unique_ptr agg) + { + auto it = hash_map_.find(GetOverflowAttributes()); + if (it != hash_map_.end()) + { + hash_map_[GetOverflowAttributes()] = it->second->Merge(*agg); + } + else + { + hash_map_[GetOverflowAttributes()] = std::move(agg); + } + } + bool IsOverflowAttributes(const MetricAttributes &attributes) const { // If the incoming attributes are exactly the overflow sentinel, route 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 c8d58bb7b2..7150433f2d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h +++ b/sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h @@ -50,6 +50,9 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage { public: + // Back-compat overload preserving the original constructor signature for any external caller. + // Sizes recording storage using the view's own cardinality limit (or the SDK default when no + // AggregationConfig is supplied), matching this class's original behavior exactly. SyncMetricStorage(const InstrumentDescriptor &instrument_descriptor, const AggregationType aggregation_type, std::shared_ptr attributes_processor, @@ -58,10 +61,38 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage nostd::shared_ptr &&exemplar_reservoir, #endif const AggregationConfig *aggregation_config) + : SyncMetricStorage( + instrument_descriptor, + aggregation_type, + std::move(attributes_processor), +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + exemplar_filter_type, + std::move(exemplar_reservoir), +#endif + aggregation_config, + AggregationConfig::GetOrDefault(aggregation_config)->GetCardinalityLimit()) + {} + + // `recording_cardinality_limit` sizes the storage that raw measurements are recorded into, + // separately from `aggregation_config`. When a view sets an explicit cardinality limit, + // callers should pass that same limit (the overload above does this). When a view has no + // explicit limit, callers may pass the max cardinality limit configured across all attached + // MetricReaders, so no reader loses data purely because the shared recording storage was + // capped too low for it; each reader's own (possibly stricter) limit is then re-applied to + // its own output during collection. See TemporalMetricStorage::buildMetrics(). + SyncMetricStorage(const InstrumentDescriptor &instrument_descriptor, + const AggregationType aggregation_type, + std::shared_ptr attributes_processor, +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType exemplar_filter_type, + nostd::shared_ptr &&exemplar_reservoir, +#endif + const AggregationConfig *aggregation_config, + std::size_t recording_cardinality_limit) : instrument_descriptor_(instrument_descriptor), aggregation_config_(AggregationConfig::GetOrDefault(aggregation_config)), - attributes_hashmap_( - std::make_unique(aggregation_config_->cardinality_limit_)), + recording_cardinality_limit_(recording_cardinality_limit), + attributes_hashmap_(std::make_unique(recording_cardinality_limit_)), attributes_processor_(std::move(attributes_processor)), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW exemplar_filter_type_(exemplar_filter_type), @@ -259,7 +290,12 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage { return filtered; } - const size_t limit = aggregation_config_->cardinality_limit_; + // Use the resolved recording capacity, not aggregation_config_->GetCardinalityLimit(): + // when the limit comes from a MetricReader fallback rather than an explicit view limit, + // these can differ, and attributes_hashmap_ (the unbound path) is sized to + // recording_cardinality_limit_. Using the same value here keeps bound and unbound + // admission consistent. + const size_t limit = recording_cardinality_limit_; const bool has_overflow = active_keys_.find(GetOverflowAttributes()) != active_keys_.end(); // Mirror AttributesHashMap::IsOverflowAttributes() exactly. The configured // limit applies to non-overflow attribute sets, while overflow is reserved. @@ -278,6 +314,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage InstrumentDescriptor instrument_descriptor_; // hashmap to maintain the metrics for delta collection (i.e, collection since last Collect call) const AggregationConfig *aggregation_config_; + // Capacity used to (re)size attributes_hashmap_. See the constructor comment above. + const std::size_t recording_cardinality_limit_; std::unique_ptr attributes_hashmap_; std::function()> create_default_aggregation_; std::shared_ptr attributes_processor_; diff --git a/sdk/src/configuration/sdk_builder.cc b/sdk/src/configuration/sdk_builder.cc index 27aad69cbf..8e5c20bddb 100644 --- a/sdk/src/configuration/sdk_builder.cc +++ b/sdk/src/configuration/sdk_builder.cc @@ -1989,6 +1989,11 @@ void SdkBuilder::AddView( if (stream->aggregation) { + // CreateAggregationConfig() may build a config purely for non-cardinality reasons (e.g. + // histogram boundaries) via a config's default constructor, which already leaves the + // cardinality limit not-explicit (see AggregationConfig's constructor) so it doesn't + // shadow a MetricReader-level fallback; the block below marks it explicit if + // aggregation_cardinality_limit is also configured. sdk_aggregation_config = CreateAggregationConfig(stream->aggregation, sdk_aggregation_type); } @@ -1997,7 +2002,7 @@ void SdkBuilder::AddView( { if (sdk_aggregation_config) { - sdk_aggregation_config->cardinality_limit_ = stream->aggregation_cardinality_limit; + sdk_aggregation_config->SetCardinalityLimit(stream->aggregation_cardinality_limit); } else { diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc index 0abd75265e..985c2552b9 100644 --- a/sdk/src/metrics/meter.cc +++ b/sdk/src/metrics/meter.cc @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include +#include #include #include #include @@ -21,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" @@ -28,6 +31,7 @@ #include "opentelemetry/sdk/metrics/meter_config.h" #include "opentelemetry/sdk/metrics/meter_context.h" #include "opentelemetry/sdk/metrics/state/async_metric_storage.h" +#include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" #include "opentelemetry/sdk/metrics/state/metric_collector.h" #include "opentelemetry/sdk/metrics/state/metric_storage.h" #include "opentelemetry/sdk/metrics/state/multi_metric_storage.h" @@ -83,6 +87,42 @@ std::ostream &operator<<(std::ostream &os, return os; } +// When a view sets an explicit cardinality limit (IsCardinalityLimitExplicit()), it wins +// outright (View > Reader > SDK default), so the raw recording storage is simply sized to that +// limit, unchanged. A non-null AggregationConfig does not by itself mean the view set an +// explicit cardinality limit: e.g. SdkBuilder::AddView() may build one purely to carry +// histogram boundaries, leaving the limit at its compiled-in default and +// IsCardinalityLimitExplicit() false. +// +// When a view has no explicit limit, the shared recording storage must be sized to the highest +// limit configured across all MetricReaders currently attached, so no reader loses data purely +// because the shared cap was sized for a stricter reader. Do not floor this at the SDK default: +// a reader may configure a limit lower than the default, and that stricter limit must still +// apply when it is the only (or the strictest) reader attached. Each reader's own (possibly +// lower) limit is then re-applied to just its own output during collection; see +// TemporalMetricStorage::buildMetrics(). +std::size_t ResolveRecordingCardinalityLimit( + const opentelemetry::sdk::metrics::AggregationConfig *aggregation_config, + opentelemetry::nostd::span> + collectors, + opentelemetry::sdk::metrics::InstrumentType instrument_type) +{ + if (aggregation_config && aggregation_config->IsCardinalityLimitExplicit()) + { + return aggregation_config->GetCardinalityLimit(); + } + if (collectors.empty()) + { + return opentelemetry::sdk::metrics::kAggregationCardinalityLimit; + } + std::size_t max_limit = 0; + for (auto &collector : collectors) + { + max_limit = (std::max)(max_limit, collector->GetCardinalityLimit(instrument_type)); + } + return max_limit; +} + } // namespace OPENTELEMETRY_BEGIN_NAMESPACE @@ -513,7 +553,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( auto success = view_registry->FindViews( instrument_descriptor, *scope_, - [this, &instrument_descriptor, &storages + [this, &instrument_descriptor, &storages, ctx #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW , exemplar_filter_type @@ -541,6 +581,8 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( else { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + auto recording_cardinality_limit = ResolveRecordingCardinalityLimit( + view.GetAggregationConfig(), ctx->GetCollectors(), view_instr_desc.type_); sync_storage = std::shared_ptr(new SyncMetricStorage( view_instr_desc, view.GetAggregationType(), view.GetAttributesProcessor(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -548,7 +590,7 @@ std::unique_ptr Meter::RegisterSyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc, exemplar_filter_type), #endif - view.GetAggregationConfig())); + view.GetAggregationConfig(), recording_cardinality_limit)); storage_registry_.insert({view_instr_desc, sync_storage}); } auto sync_multi_storage = static_cast(storages.get()); @@ -586,7 +628,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( auto success = view_registry->FindViews( instrument_descriptor, *GetInstrumentationScope(), - [this, &instrument_descriptor, &storages + [this, &instrument_descriptor, &storages, ctx #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW , exemplar_filter_type @@ -614,6 +656,8 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( else { WarnOnDuplicateInstrument(GetInstrumentationScope(), storage_registry_, view_instr_desc); + auto recording_cardinality_limit = ResolveRecordingCardinalityLimit( + view.GetAggregationConfig(), ctx->GetCollectors(), view_instr_desc.type_); async_storage = std::shared_ptr(new AsyncMetricStorage( view_instr_desc, view.GetAggregationType(), #ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW @@ -621,7 +665,7 @@ std::unique_ptr Meter::RegisterAsyncMetricStorage( GetExemplarReservoir(view.GetAggregationType(), view.GetAggregationConfig(), view_instr_desc, exemplar_filter_type), #endif - view.GetAggregationConfig())); + view.GetAggregationConfig(), recording_cardinality_limit)); storage_registry_.insert({view_instr_desc, async_storage}); } auto async_multi_storage = static_cast(storages.get()); diff --git a/sdk/src/metrics/metric_reader.cc b/sdk/src/metrics/metric_reader.cc index c9312fc183..549fb0f83e 100644 --- a/sdk/src/metrics/metric_reader.cc +++ b/sdk/src/metrics/metric_reader.cc @@ -111,12 +111,6 @@ std::size_t MetricReader::GetCardinalityLimit(InstrumentType instrument_type) co void MetricReader::SetCardinalityLimits(const CardinalityLimits &limits) noexcept { cardinality_limits_ = limits; - // TODO: Reader-level limits are stored but not yet enforced as a per-collector - // fallback during the collection path. Enforcement will be added in a follow-up. - OTEL_INTERNAL_LOG_WARN( - "MetricReader::SetCardinalityLimits - reader-level cardinality limits are stored " - "but not yet enforced during collection. Use view-level AggregationConfig to " - "enforce limits today."); } } // namespace metrics diff --git a/sdk/src/metrics/state/sync_metric_storage.cc b/sdk/src/metrics/state/sync_metric_storage.cc index 245ef77454..ac119b36a2 100644 --- a/sdk/src/metrics/state/sync_metric_storage.cc +++ b/sdk/src/metrics/state/sync_metric_storage.cc @@ -8,7 +8,6 @@ #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/span.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" #include "opentelemetry/sdk/metrics/state/sync_metric_storage.h" @@ -53,7 +52,7 @@ bool SyncMetricStorage::Collect(CollectorHandle *collector, { std::lock_guard guard(attribute_hashmap_lock_); delta_metrics = std::move(attributes_hashmap_); - attributes_hashmap_.reset(new AttributesHashMap(aggregation_config_->cardinality_limit_)); + attributes_hashmap_.reset(new AttributesHashMap(recording_cardinality_limit_)); #ifdef OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW // Garbage-collect entries the user has dropped that have no pending data. // Cleanup happens during Collect(); if no collection runs, dropped bound diff --git a/sdk/src/metrics/state/temporal_metric_storage.cc b/sdk/src/metrics/state/temporal_metric_storage.cc index 85a0a4b3c0..dc8c3fcf78 100644 --- a/sdk/src/metrics/state/temporal_metric_storage.cc +++ b/sdk/src/metrics/state/temporal_metric_storage.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -84,8 +85,43 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector, metric_data.end_ts = collection_ts; last_delta_collection_ts_ = collection_ts; - // Direct conversion of delta metrics to point data - delta_metrics->GetAllEntries( + // Recording capacity is resolved once at instrument-creation time (as the highest limit + // across readers attached then; see ResolveRecordingCardinalityLimit() in meter.cc) and is + // not shrunk retroactively, so it can exceed this single collector's own (possibly + // stricter) current limit, e.g. if this reader was attached after instrument creation with + // a lower limit than was resolved then. Re-cap through a hashmap sized to this collector's + // effective limit before emitting, so its own limit is honored on this fast path too; + // mirrors the precedence applied to the general merge path below (View > Reader > SDK + // default). + const bool has_explicit_view_limit = + aggregation_config_ && aggregation_config_->IsCardinalityLimitExplicit(); + const std::size_t effective_limit = + has_explicit_view_limit ? aggregation_config_->GetCardinalityLimit() + : collector->GetCardinalityLimit(instrument_descriptor_.type_); + + if (delta_metrics->Size() <= effective_limit) + { + // Direct conversion of delta metrics to point data + delta_metrics->GetAllEntries( + [&metric_data](const MetricAttributes &attributes, Aggregation &aggregation) { + PointDataAttributes point_data_attr; + point_data_attr.point_data = aggregation.ToPoint(); + point_data_attr.attributes = attributes; + metric_data.point_data_attr_.emplace_back(std::move(point_data_attr)); + return true; + }); + return callback(metric_data); + } + + AttributesHashMap recapped(effective_limit); + delta_metrics->GetAllEntries([&recapped, this](const MetricAttributes &attributes, + Aggregation &aggregation) { + recapped.Set(attributes, DefaultAggregation::CreateAggregation( + aggregation_type_, instrument_descriptor_, aggregation_config_) + ->Merge(aggregation)); + return true; + }); + recapped.GetAllEntries( [&metric_data](const MetricAttributes &attributes, Aggregation &aggregation) { PointDataAttributes point_data_attr; point_data_attr.point_data = aggregation.ToPoint(); @@ -114,9 +150,21 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector, } auto unreported_list = std::move(present->second); // Iterate over the unreporter metrics for `collector` and store result in `merged_metrics` - std::unique_ptr merged_metrics( - new AttributesHashMap(aggregation_config_ ? aggregation_config_->cardinality_limit_ - : kAggregationCardinalityLimit)); + // + // When the view has no explicit cardinality limit (aggregation_config_ == nullptr, or its + // limit was left at the compiled-in default for an unrelated reason such as histogram + // boundaries; see IsCardinalityLimitExplicit()), fall back to this specific collector's own + // MetricReader-level limit rather than a flat SDK default. The underlying recording storage + // may be sized larger (to avoid losing data for readers with a higher limit, see + // SyncMetricStorage/AsyncMetricStorage), so capping merged_metrics here to this collector's + // own limit re-applies the View > Reader > SDK default precedence per reader: + // AttributesHashMap::Set() below routes anything beyond this capacity into the overflow point, + // without affecting what other collectors of the same storage see. + const bool has_explicit_view_limit = + aggregation_config_ && aggregation_config_->IsCardinalityLimitExplicit(); + std::unique_ptr merged_metrics(new AttributesHashMap( + has_explicit_view_limit ? aggregation_config_->GetCardinalityLimit() + : collector->GetCardinalityLimit(instrument_descriptor_.type_))); for (auto &agg_hashmap : unreported_list) { agg_hashmap->GetAllEntries( diff --git a/sdk/test/configuration/sdk_builder_test.cc b/sdk/test/configuration/sdk_builder_test.cc index 7550c44fd1..e541ae9124 100644 --- a/sdk/test/configuration/sdk_builder_test.cc +++ b/sdk/test/configuration/sdk_builder_test.cc @@ -1079,7 +1079,7 @@ TEST(SdkBuilder, AddViewEmptySelectorMatchesAllSupportedInstrumentTypes) EXPECT_NE(config, nullptr); if (config != nullptr) { - EXPECT_EQ(config->cardinality_limit_, 42u); + EXPECT_EQ(config->GetCardinalityLimit(), 42u); matched++; } return true; @@ -1117,7 +1117,7 @@ TEST(SdkBuilder, AddViewHistogramCardinalityLimitOnly) if (aggregation_config) { EXPECT_EQ(aggregation_config->GetType(), metrics_sdk::AggregationType::kHistogram); - EXPECT_EQ(aggregation_config->cardinality_limit_, 42u); + EXPECT_EQ(aggregation_config->GetCardinalityLimit(), 42u); // Pin what users actually receive: building the aggregation from this config // must keep the SDK's default bucket boundaries, not silently collapse to a @@ -1168,7 +1168,7 @@ TEST(SdkBuilder, AddViewCounterCardinalityLimitOnly) if (aggregation_config) { EXPECT_EQ(aggregation_config->GetType(), metrics_sdk::AggregationType::kDefault); - EXPECT_EQ(aggregation_config->cardinality_limit_, 7u); + EXPECT_EQ(aggregation_config->GetCardinalityLimit(), 7u); } return true; }); @@ -1176,6 +1176,97 @@ TEST(SdkBuilder, AddViewCounterCardinalityLimitOnly) EXPECT_EQ(matched, 1); } +// Regression test for a bug found during review of #4388: a view that configures an +// `aggregation` block for reasons unrelated to cardinality (here, explicit histogram +// boundaries) without also setting `aggregation_cardinality_limit` must not be treated as +// having an explicit view-level cardinality limit, since the resulting AggregationConfig's +// cardinality_limit_ is just the compiled-in default (kAggregationCardinalityLimit), not a +// user choice. Otherwise a MetricReader-level fallback would be silently skipped. +TEST(SdkBuilder, AddViewHistogramBoundariesWithoutCardinalityLimitIsNotExplicit) +{ + namespace metrics_sdk = opentelemetry::sdk::metrics; + + auto model = std::make_unique(); + model->selector = std::make_unique(); + model->selector->instrument_type = config_sdk::InstrumentType::histogram; + + model->stream = std::make_unique(); + auto histogram_aggr = + std::make_unique(); + histogram_aggr->boundaries = {1.0, 2.0, 3.0}; + model->stream->aggregation = std::move(histogram_aggr); + // aggregation_cardinality_limit intentionally left at its default (0 = kInheritFromReader). + + auto registry = std::make_shared(); + config_sdk::SdkBuilder builder(registry); + + metrics_sdk::ViewRegistry view_registry; + builder.AddView(&view_registry, model); + + metrics_sdk::InstrumentDescriptor instrument_descriptor{ + "", "", "", metrics_sdk::InstrumentType::kHistogram, metrics_sdk::InstrumentValueType::kLong}; + auto instrumentation_scope = scope_sdk::InstrumentationScope::Create(""); + + int matched = 0; + view_registry.FindViews(instrument_descriptor, *instrumentation_scope, + [&](const metrics_sdk::View &view) { + matched++; + auto *aggregation_config = view.GetAggregationConfig(); + EXPECT_NE(aggregation_config, nullptr); + if (aggregation_config) + { + EXPECT_FALSE(aggregation_config->IsCardinalityLimitExplicit()); + } + return true; + }); + + EXPECT_EQ(matched, 1); +} + +// Companion to the test above: when the same stream also sets aggregation_cardinality_limit, +// the resulting config must be marked explicit and carry that value. +TEST(SdkBuilder, AddViewHistogramBoundariesWithCardinalityLimitIsExplicit) +{ + namespace metrics_sdk = opentelemetry::sdk::metrics; + + auto model = std::make_unique(); + model->selector = std::make_unique(); + model->selector->instrument_type = config_sdk::InstrumentType::histogram; + + model->stream = std::make_unique(); + auto histogram_aggr = + std::make_unique(); + histogram_aggr->boundaries = {1.0, 2.0, 3.0}; + model->stream->aggregation = std::move(histogram_aggr); + model->stream->aggregation_cardinality_limit = 99; + + auto registry = std::make_shared(); + config_sdk::SdkBuilder builder(registry); + + metrics_sdk::ViewRegistry view_registry; + builder.AddView(&view_registry, model); + + metrics_sdk::InstrumentDescriptor instrument_descriptor{ + "", "", "", metrics_sdk::InstrumentType::kHistogram, metrics_sdk::InstrumentValueType::kLong}; + auto instrumentation_scope = scope_sdk::InstrumentationScope::Create(""); + + int matched = 0; + view_registry.FindViews(instrument_descriptor, *instrumentation_scope, + [&](const metrics_sdk::View &view) { + matched++; + auto *aggregation_config = view.GetAggregationConfig(); + EXPECT_NE(aggregation_config, nullptr); + if (aggregation_config) + { + EXPECT_TRUE(aggregation_config->IsCardinalityLimitExplicit()); + EXPECT_EQ(aggregation_config->GetCardinalityLimit(), 99u); + } + return true; + }); + + EXPECT_EQ(matched, 1); +} + TEST(SdkBuilder, AddViewWithCardinalityLimitPreservesExplicitAggregation) { namespace metrics_sdk = opentelemetry::sdk::metrics; @@ -1206,7 +1297,7 @@ TEST(SdkBuilder, AddViewWithCardinalityLimitPreservesExplicitAggregation) if (aggregation_config) { EXPECT_EQ(aggregation_config->GetType(), metrics_sdk::AggregationType::kHistogram); - EXPECT_EQ(aggregation_config->cardinality_limit_, 42u); + EXPECT_EQ(aggregation_config->GetCardinalityLimit(), 42u); auto *histogram_config = static_cast(aggregation_config); EXPECT_EQ(histogram_config->boundaries_, (std::vector{1.0, 2.0})); diff --git a/sdk/test/metrics/metric_collector_test.cc b/sdk/test/metrics/metric_collector_test.cc index c97f9221f9..bd10c8c9ea 100644 --- a/sdk/test/metrics/metric_collector_test.cc +++ b/sdk/test/metrics/metric_collector_test.cc @@ -31,6 +31,7 @@ #include "opentelemetry/sdk/metrics/meter.h" #include "opentelemetry/sdk/metrics/meter_context.h" #include "opentelemetry/sdk/metrics/metric_reader.h" +#include "opentelemetry/sdk/metrics/push_metric_exporter.h" #include "opentelemetry/sdk/metrics/state/attributes_hashmap.h" #include "opentelemetry/sdk/metrics/state/metric_collector.h" #include "opentelemetry/sdk/metrics/view/instrument_selector.h" @@ -594,6 +595,269 @@ TEST_F(MetricCollectorTest, ViewCardinalityLimitEnforcedOnCollection) EXPECT_TRUE(overflow_present); } +namespace +{ +size_t CountRealPoints(const MetricProducer::Result &result, bool *overflow_present) +{ + size_t total_points = 0; + for (const ScopeMetrics &sm : result.points_.scope_metric_data_) + { + for (const MetricData &md : sm.metric_data_) + { + for (const PointDataAttributes &pda : md.point_data_attr_) + { + if (!pda.attributes.GetAttributes().empty() && + pda.attributes.GetAttributes().begin()->first == kAttributesLimitOverflowKey) + { + if (overflow_present) + { + *overflow_present = true; + } + continue; + } + ++total_points; + } + } + } + return total_points; +} + +// Sums the counter value across every point, including the overflow point. Used to prove no +// data is silently dropped when several distinct attribute sets are funneled into overflow. +int64_t SumCounterValue(const MetricProducer::Result &result) +{ + int64_t total = 0; + for (const ScopeMetrics &sm : result.points_.scope_metric_data_) + { + for (const MetricData &md : sm.metric_data_) + { + for (const PointDataAttributes &pda : md.point_data_attr_) + { + auto sum_point_data = nostd::get(pda.point_data); + total += nostd::get(sum_point_data.value_); + } + } + } + return total; +} +} // namespace + +// Regression test for the bug found during review of #4188: resolving a reader-level +// cardinality limit fallback into shared storage broke per-reader semantics, since a +// stricter reader would force every other reader sharing the same storage down to its limit. +// +// With no view-level limit configured, a stricter reader must only ever report up to its own +// limit (extra series collapse into its own overflow point), while a laxer reader sharing the +// same underlying storage must still see every recorded series with no data loss and no +// spurious overflow. +TEST_F(MetricCollectorTest, ReaderCardinalityLimitFallbackWithMultipleReaders) +{ + constexpr size_t kLowLimit = 3; + constexpr size_t kHighLimit = 50; + constexpr size_t kUniqueAttributeSets = 10; + + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + auto scope = InstrumentationScope::Create("ReaderCardinalityLimitFallbackWithMultipleReaders"); + auto meter = std::shared_ptr(new Meter(context, std::move(scope))); + context->AddMeter(meter); + + auto low_reader = std::shared_ptr(new MockMetricReader()); + CardinalityLimits low_limits; + low_limits.counter = kLowLimit; + low_reader->SetCardinalityLimits(low_limits); + auto low_collector = AddMetricReaderToMeterContext(context, low_reader).lock(); + + auto high_reader = std::shared_ptr(new MockMetricReader()); + CardinalityLimits high_limits; + high_limits.counter = kHighLimit; + high_reader->SetCardinalityLimits(high_limits); + auto high_collector = AddMetricReaderToMeterContext(context, high_reader).lock(); + + // Both readers must already be attached before the instrument is first used: recording + // storage capacity is resolved (as the max limit across attached readers) at that point. + auto counter = meter->CreateUInt64Counter("shared_counter"); + + for (size_t i = 0; i < kUniqueAttributeSets; ++i) + { + std::map attrs = {{"key", std::to_string(i)}}; + counter->Add( + 1, opentelemetry::common::KeyValueIterableView>(attrs), + opentelemetry::context::Context{}); + } + + auto low_produced = low_collector->Produce(); + auto high_produced = high_collector->Produce(); + + bool low_overflow = false; + bool high_overflow = false; + size_t low_points = CountRealPoints(low_produced, &low_overflow); + size_t high_points = CountRealPoints(high_produced, &high_overflow); + + // The stricter reader must be capped at its own limit, with overflow absorbing the rest. + EXPECT_LE(low_points, kLowLimit); + EXPECT_TRUE(low_overflow); + // No data may be silently dropped: every one of the kUniqueAttributeSets Add(1, ...) calls + // must still be reflected somewhere, whether as its own point or merged into overflow. + EXPECT_EQ(SumCounterValue(low_produced), static_cast(kUniqueAttributeSets)); + + // The laxer reader, sharing the same underlying storage, must see every recorded series: + // it must not be capped down to the stricter reader's limit, and must not report overflow. + EXPECT_EQ(high_points, kUniqueAttributeSets); + EXPECT_FALSE(high_overflow); +} + +// Regression test for a second bug found during review: the single-collector delta fast path +// in TemporalMetricStorage::buildMetrics() reads straight from the raw recording storage, +// bypassing the per-collector re-cap. With a single delta reader this is only safe if the +// *recording* storage itself was already sized to that reader's own limit (rather than floored +// at the SDK default), which is what ResolveRecordingCardinalityLimit() in meter.cc must do. +TEST_F(MetricCollectorTest, ReaderCardinalityLimitFallbackSingleDeltaReader) +{ + constexpr size_t kLimit = 3; + constexpr size_t kUniqueAttributeSets = 10; + + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + auto scope = InstrumentationScope::Create("ReaderCardinalityLimitFallbackSingleDeltaReader"); + auto meter = std::shared_ptr(new Meter(context, std::move(scope))); + context->AddMeter(meter); + + // MockMetricReader's default exporter reports kCumulative, not kDelta (see + // MockMetricExporter's default member initializer), so an explicit kDelta exporter is + // required here to actually exercise TemporalMetricStorage::buildMetrics()'s + // single-collector-delta fast path with exactly one reader attached. + auto reader = std::shared_ptr(new MockMetricReader( + std::unique_ptr(new MockMetricExporter(AggregationTemporality::kDelta)))); + CardinalityLimits limits; + limits.counter = kLimit; + reader->SetCardinalityLimits(limits); + auto collector = AddMetricReaderToMeterContext(context, reader).lock(); + + auto counter = meter->CreateUInt64Counter("delta_counter"); + + for (size_t i = 0; i < kUniqueAttributeSets; ++i) + { + std::map attrs = {{"key", std::to_string(i)}}; + counter->Add( + 1, opentelemetry::common::KeyValueIterableView>(attrs), + opentelemetry::context::Context{}); + } + + auto produced = collector->Produce(); + bool overflow_present = false; + size_t real_points = CountRealPoints(produced, &overflow_present); + + // The reader's own limit must be honored even via the single-collector delta fast path, + // not silently widened to the SDK default (2000). + EXPECT_LE(real_points, kLimit); + EXPECT_TRUE(overflow_present); + EXPECT_EQ(SumCounterValue(produced), static_cast(kUniqueAttributeSets)); +} + +// Regression test for a bug found during review of #4388: the single-collector delta fast path +// in TemporalMetricStorage::buildMetrics() returned recorded data directly, without re-capping +// to the collector's own limit. When recording capacity was resolved larger than this reader's +// limit (because the reader was attached after the instrument already existed, so its stricter +// limit was never part of the max-across-readers resolution at creation time), the fast path +// could emit more attribute sets than the reader's own configured limit allows. +TEST_F(MetricCollectorTest, ReaderCardinalityLimitFallbackSingleDeltaReaderAttachedLate) +{ + constexpr size_t kLimit = 3; + constexpr size_t kUniqueAttributeSets = 10; + + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + auto scope = + InstrumentationScope::Create("ReaderCardinalityLimitFallbackSingleDeltaReaderAttachedLate"); + auto meter = std::shared_ptr(new Meter(context, std::move(scope))); + context->AddMeter(meter); + + // No reader attached yet: recording capacity resolves to the SDK default (2000). + ASSERT_EQ(context->GetCollectors().size(), 0u); + auto counter = meter->CreateUInt64Counter("late_delta_counter"); + + // A single delta reader with a much stricter limit is attached only after the instrument + // already exists, exercising the single-collector-delta fast path with recording capacity + // (2000) larger than this reader's own limit (kLimit). An explicit kDelta exporter is + // required: MockMetricReader's default exporter reports kCumulative. + auto reader = std::shared_ptr(new MockMetricReader( + std::unique_ptr(new MockMetricExporter(AggregationTemporality::kDelta)))); + CardinalityLimits limits; + limits.counter = kLimit; + reader->SetCardinalityLimits(limits); + auto collector = AddMetricReaderToMeterContext(context, reader).lock(); + + for (size_t i = 0; i < kUniqueAttributeSets; ++i) + { + std::map attrs = {{"key", std::to_string(i)}}; + counter->Add( + 1, opentelemetry::common::KeyValueIterableView>(attrs), + opentelemetry::context::Context{}); + } + + auto produced = collector->Produce(); + bool overflow_present = false; + size_t real_points = CountRealPoints(produced, &overflow_present); + + // Even though recording capacity was resolved without this reader in the picture, its own + // limit must still be honored on the single-collector delta fast path. + EXPECT_LE(real_points, kLimit); + EXPECT_TRUE(overflow_present); + EXPECT_EQ(SumCounterValue(produced), static_cast(kUniqueAttributeSets)); +} + +// Documents and pins a known limitation raised during review of #4188: recording capacity is +// resolved once, as the highest limit across readers attached at instrument-creation time (see +// ResolveRecordingCardinalityLimit() in meter.cc), and is not grown retroactively. A reader +// added later with a higher limit cannot recover attribute sets that had already collapsed into +// the overflow point before it was attached; see the Note on MeterContext::AddMetricReader(). +TEST_F(MetricCollectorTest, ReaderAddedAfterInstrumentCreationDoesNotGrowRecordingCapacity) +{ + constexpr size_t kLowLimit = 3; + constexpr size_t kHighLimit = 50; + constexpr size_t kUniqueAttributeSets = 10; + + auto context = std::shared_ptr(new MeterContext(ViewRegistryFactory::Create())); + auto scope = InstrumentationScope::Create( + "ReaderAddedAfterInstrumentCreationDoesNotGrowRecordingCapacity"); + auto meter = std::shared_ptr(new Meter(context, std::move(scope))); + context->AddMeter(meter); + + auto low_reader = std::shared_ptr(new MockMetricReader()); + CardinalityLimits low_limits; + low_limits.counter = kLowLimit; + low_reader->SetCardinalityLimits(low_limits); + AddMetricReaderToMeterContext(context, low_reader); + + // The instrument is created (and its recording capacity resolved) with only the low-limit + // reader attached. + auto counter = meter->CreateUInt64Counter("late_reader_counter"); + + for (size_t i = 0; i < kUniqueAttributeSets; ++i) + { + std::map attrs = {{"key", std::to_string(i)}}; + counter->Add( + 1, opentelemetry::common::KeyValueIterableView>(attrs), + opentelemetry::context::Context{}); + } + + // A higher-limit reader is attached only after the instrument already exists and has recorded + // more unique attribute sets than the low limit. + auto high_reader = std::shared_ptr(new MockMetricReader()); + CardinalityLimits high_limits; + high_limits.counter = kHighLimit; + high_reader->SetCardinalityLimits(high_limits); + auto high_collector = AddMetricReaderToMeterContext(context, high_reader).lock(); + + auto high_produced = high_collector->Produce(); + bool high_overflow = false; + size_t high_points = CountRealPoints(high_produced, &high_overflow); + + // Despite its own limit of kHighLimit, the late-attached reader is still capped by the + // recording capacity resolved when the instrument was created: attribute sets already + // collapsed into overflow before this reader existed cannot be recovered. + EXPECT_LE(high_points, kLowLimit); + EXPECT_TRUE(high_overflow); +} + #if defined(__GNUC__) || defined(__clang__) || defined(__apple_build_version__) # pragma GCC diagnostic pop #endif