diff --git a/docs/root/configuration/cluster_manager/cds.rst b/docs/root/configuration/cluster_manager/cds.rst index 89f2dbcd4b186..ebfe5008ac92a 100644 --- a/docs/root/configuration/cluster_manager/cds.rst +++ b/docs/root/configuration/cluster_manager/cds.rst @@ -29,4 +29,4 @@ CDS has a statistics tree rooted at *cluster_manager.cds.* with the following st update_failure, Counter, Total API fetches that failed because of network errors update_rejected, Counter, Total API fetches that failed because of schema/validation errors version, Gauge, Hash of the contents from the last successful API fetch - control_plane.connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the current connection state with management server + control_plane.connected_state, BoolIndicator, Current connection state with management server diff --git a/docs/root/configuration/cluster_manager/cluster_stats.rst b/docs/root/configuration/cluster_manager/cluster_stats.rst index 5135080e356e2..38d0759aa2ca1 100644 --- a/docs/root/configuration/cluster_manager/cluster_stats.rst +++ b/docs/root/configuration/cluster_manager/cluster_stats.rst @@ -148,10 +148,10 @@ Circuit breakers statistics will be rooted at *cluster..circuit_breakers.< :header: Name, Type, Description :widths: 1, 1, 2 - cx_open, Gauge, Whether the connection circuit breaker is closed (0) or open (1) - rq_pending_open, Gauge, Whether the pending requests circuit breaker is closed (0) or open (1) - rq_open, Gauge, Whether the requests circuit breaker is closed (0) or open (1) - rq_retry_open, Gauge, Whether the retry circuit breaker is closed (0) or open (1) + cx_open, BoolIndicator, Whether the connection circuit breaker is closed (false) or open (true) + rq_pending_open, BoolIndicator, Whether the pending requests circuit breaker is closed (false) or open (true) + rq_open, BoolIndicator, Whether the requests circuit breaker is closed (false) or open (true) + rq_retry_open, BoolIndicator, Whether the retry circuit breaker is closed (false) or open (true) .. _config_cluster_manager_cluster_stats_dynamic_http: diff --git a/docs/root/configuration/listeners/lds.rst b/docs/root/configuration/listeners/lds.rst index a90fe84f6f11e..a876087bb305a 100644 --- a/docs/root/configuration/listeners/lds.rst +++ b/docs/root/configuration/listeners/lds.rst @@ -48,4 +48,4 @@ LDS has a statistics tree rooted at *listener_manager.lds.* with the following s update_failure, Counter, Total API fetches that failed because of network errors update_rejected, Counter, Total API fetches that failed because of schema/validation errors version, Gauge, Hash of the contents from the last successful API fetch - control_plane.connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the current connection state with management server + control_plane.connected_state, BoolIndicator, Current connection state with management server diff --git a/docs/root/configuration/overview/v2_overview.rst b/docs/root/configuration/overview/v2_overview.rst index 560f6f130981d..d65bf287c0e0e 100644 --- a/docs/root/configuration/overview/v2_overview.rst +++ b/docs/root/configuration/overview/v2_overview.rst @@ -590,7 +590,7 @@ Management Server has a statistics tree rooted at *control_plane.* with the foll :header: Name, Type, Description :widths: 1, 1, 2 - connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the current connection state with management server + connected_state, BoolIndicator, Current connection state with management server rate_limit_enforced, Counter, Total number of times rate limit was enforced for management server requests pending_requests, Gauge, Total number of pending requests when the rate limit was enforced diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index a1e3eedb15e72..d51ad1425a404 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -44,6 +44,7 @@ Version history * stats: added support for histograms in prometheus * stats: added usedonly flag to prometheus stats to only output metrics which have been updated at least once. +* stats: added BoolIndicator stat type, converted the following 1-or-0 Gauges: control_plane.connected_state, cx_open, rq_pending_open, rq_open, rq_retry_open, runtime.admin_overrides_active, open_gauge, config.active, server.live. * tap: added new alpha :ref:`HTTP tap filter `. * tls: enabled TLS 1.3 on the server-side (non-FIPS builds). * upstream: add hash_function to specify the hash function for :ref:`ring hash` as either xxHash or `murmurHash2 `_. MurmurHash2 is compatible with std::hash in GNU libstdc++ 3.4.20 or above. This is typically the case when compiled on Linux and not macOS. diff --git a/include/envoy/config/grpc_mux.h b/include/envoy/config/grpc_mux.h index 6872a62d875b6..af568566e7edb 100644 --- a/include/envoy/config/grpc_mux.h +++ b/include/envoy/config/grpc_mux.h @@ -13,9 +13,9 @@ namespace Config { * All control plane related stats. @see stats_macros.h */ // clang-format off -#define ALL_CONTROL_PLANE_STATS(COUNTER, GAUGE) \ +#define ALL_CONTROL_PLANE_STATS(BOOL_INDICATOR, COUNTER, GAUGE) \ COUNTER(rate_limit_enforced) \ - GAUGE(connected_state) \ + BOOL_INDICATOR(connected_state) \ GAUGE(pending_requests) \ // clang-format on @@ -23,7 +23,7 @@ namespace Config { * Struct definition for all control plane stats. @see stats_macros.h */ struct ControlPlaneStats { - ALL_CONTROL_PLANE_STATS(GENERATE_COUNTER_STRUCT,GENERATE_GAUGE_STRUCT) + ALL_CONTROL_PLANE_STATS(GENERATE_BOOL_INDICATOR_STRUCT,GENERATE_COUNTER_STRUCT,GENERATE_GAUGE_STRUCT) }; class GrpcMuxCallbacks { diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h index ef913edea6e02..ea0378bb752a9 100644 --- a/include/envoy/stats/scope.h +++ b/include/envoy/stats/scope.h @@ -51,6 +51,11 @@ class Scope { */ virtual Gauge& gauge(const std::string& name) PURE; + /** + * @return a bool within the scope's namespace. + */ + virtual BoolIndicator& boolIndicator(const std::string& name) PURE; + /** * @return a histogram within the scope's namespace with a particular value type. */ diff --git a/include/envoy/stats/source.h b/include/envoy/stats/source.h index 8dea65487a587..149b8e7fc3c70 100644 --- a/include/envoy/stats/source.h +++ b/include/envoy/stats/source.h @@ -37,6 +37,14 @@ class Source { */ virtual const std::vector& cachedGauges() PURE; + /** + * Returns all known bools. Will use cached values if already accessed and clearCache() hasn't + * been called since. + * @return std::vector& all known bools. Note: reference may not be + * valid after clearCache() is called. + */ + virtual const std::vector& cachedBoolIndicators() PURE; + /** * Returns all known parent histograms. Will use cached values if already accessed and * clearCache() hasn't been called since. diff --git a/include/envoy/stats/stat_data_allocator.h b/include/envoy/stats/stat_data_allocator.h index f0ea93e266d04..5696408e2f062 100644 --- a/include/envoy/stats/stat_data_allocator.h +++ b/include/envoy/stats/stat_data_allocator.h @@ -47,6 +47,17 @@ class StatDataAllocator { virtual GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name, std::vector&& tags) PURE; + /** + * @param name the full name of the stat. + * @param tag_extracted_name the name of the stat with tag-values stripped out. + * @param tags the extracted tag values. + * @return BoolIndicatorSharedPtr a bool, or nullptr if allocation failed, in which case + * tag_extracted_name and tags are not moved. + */ + virtual BoolIndicatorSharedPtr makeBoolIndicator(absl::string_view name, + std::string&& tag_extracted_name, + std::vector&& tags) PURE; + /** * Determines whether this stats allocator requires bounded stat-name size. */ diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h index 6d191bf24f9fb..09e5842b4d041 100644 --- a/include/envoy/stats/stats.h +++ b/include/envoy/stats/stats.h @@ -94,5 +94,18 @@ class Gauge : public virtual Metric { typedef std::shared_ptr GaugeSharedPtr; +/** + * A Boolean. + */ +class BoolIndicator : public virtual Metric { +public: + virtual ~BoolIndicator() {} + + virtual void set(bool value) PURE; + virtual bool value() const PURE; +}; + +typedef std::shared_ptr BoolIndicatorSharedPtr; + } // namespace Stats } // namespace Envoy diff --git a/include/envoy/stats/stats_macros.h b/include/envoy/stats/stats_macros.h index eb1c89557c664..99cab9aeac3ba 100644 --- a/include/envoy/stats/stats_macros.h +++ b/include/envoy/stats/stats_macros.h @@ -29,15 +29,18 @@ namespace Envoy { #define GENERATE_COUNTER_STRUCT(NAME) Stats::Counter& NAME##_; #define GENERATE_GAUGE_STRUCT(NAME) Stats::Gauge& NAME##_; +#define GENERATE_BOOL_INDICATOR_STRUCT(NAME) Stats::BoolIndicator& NAME##_; #define GENERATE_HISTOGRAM_STRUCT(NAME) Stats::Histogram& NAME##_; #define FINISH_STAT_DECL_(X) + std::string(#X)), #define POOL_COUNTER_PREFIX(POOL, PREFIX) (POOL).counter(PREFIX FINISH_STAT_DECL_ #define POOL_GAUGE_PREFIX(POOL, PREFIX) (POOL).gauge(PREFIX FINISH_STAT_DECL_ +#define POOL_BOOL_INDICATOR_PREFIX(POOL, PREFIX) (POOL).boolIndicator(PREFIX FINISH_STAT_DECL_ #define POOL_HISTOGRAM_PREFIX(POOL, PREFIX) (POOL).histogram(PREFIX FINISH_STAT_DECL_ #define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "") #define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "") +#define POOL_BOOL_INDICATOR(POOL) POOL_BOOL_INDICATOR_PREFIX(POOL, "") #define POOL_HISTOGRAM(POOL) POOL_HISTOGRAM_PREFIX(POOL, "") } // namespace Envoy diff --git a/include/envoy/stats/store.h b/include/envoy/stats/store.h index cd017f9ad8843..d22ff2b3eb67d 100644 --- a/include/envoy/stats/store.h +++ b/include/envoy/stats/store.h @@ -39,6 +39,11 @@ class Store : public Scope { */ virtual std::vector gauges() const PURE; + /** + * @return a list of all known bools. + */ + virtual std::vector boolIndicators() const PURE; + /** * @return a list of all known histograms. */ diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 79a54472fdd7d..12b1229c0a21e 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -524,11 +524,11 @@ class PrioritySet { * Cluster circuit breakers stats. */ // clang-format off -#define ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GAUGE) \ - GAUGE (cx_open) \ - GAUGE (rq_pending_open) \ - GAUGE (rq_open) \ - GAUGE (rq_retry_open) +#define ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(BOOL_INDICATOR) \ + BOOL_INDICATOR (cx_open) \ + BOOL_INDICATOR (rq_pending_open) \ + BOOL_INDICATOR (rq_open) \ + BOOL_INDICATOR (rq_retry_open) // clang-format on /** @@ -549,7 +549,7 @@ struct ClusterLoadReportStats { * Struct definition for cluster circuit breakers stats. @see stats_macros.h */ struct ClusterCircuitBreakersStats { - ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_GAUGE_STRUCT) + ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_BOOL_INDICATOR_STRUCT) }; /** diff --git a/source/common/config/grpc_stream.h b/source/common/config/grpc_stream.h index c1e5ef761255d..85d0e9899e678 100644 --- a/source/common/config/grpc_stream.h +++ b/source/common/config/grpc_stream.h @@ -58,7 +58,7 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks, setRetryTimer(); return; } - control_plane_stats_.connected_state_.set(1); + control_plane_stats_.connected_state_.set(true); handleStreamEstablished(); } @@ -88,7 +88,7 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks, void onRemoteClose(Grpc::Status::GrpcStatus status, const std::string& message) override { ENVOY_LOG(warn, "gRPC config stream closed: {}, {}", status, message); stream_ = nullptr; - control_plane_stats_.connected_state_.set(0); + control_plane_stats_.connected_state_.set(false); handleEstablishmentFailure(); setRetryTimer(); } @@ -131,7 +131,8 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks, ControlPlaneStats generateControlPlaneStats(Stats::Scope& scope) { const std::string control_plane_prefix = "control_plane."; - return {ALL_CONTROL_PLANE_STATS(POOL_COUNTER_PREFIX(scope, control_plane_prefix), + return {ALL_CONTROL_PLANE_STATS(POOL_BOOL_INDICATOR_PREFIX(scope, control_plane_prefix), + POOL_COUNTER_PREFIX(scope, control_plane_prefix), POOL_GAUGE_PREFIX(scope, control_plane_prefix))}; } diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index a7b3ee2f48565..b8e79b07c24c3 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -319,7 +319,7 @@ void AdminLayer::mergeValues(const std::unordered_map& values_.emplace(kv.first, SnapshotImpl::createEntry(kv.second)); } } - stats_.admin_overrides_active_.set(values_.empty() ? 0 : 1); + stats_.admin_overrides_active_.set(!values_.empty()); } DiskLayer::DiskLayer(const std::string& name, const std::string& path, Api::Api& api) @@ -429,8 +429,9 @@ DiskBackedLoaderImpl::DiskBackedLoaderImpl(Event::Dispatcher& dispatcher, RuntimeStats LoaderImpl::generateStats(Stats::Store& store) { std::string prefix = "runtime."; - RuntimeStats stats{ - ALL_RUNTIME_STATS(POOL_COUNTER_PREFIX(store, prefix), POOL_GAUGE_PREFIX(store, prefix))}; + RuntimeStats stats{ALL_RUNTIME_STATS(POOL_BOOL_INDICATOR_PREFIX(store, prefix), + POOL_COUNTER_PREFIX(store, prefix), + POOL_GAUGE_PREFIX(store, prefix))}; return stats; } diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index d2e80127a23df..edbed0bbba4a3 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -43,21 +43,21 @@ class RandomGeneratorImpl : public RandomGenerator { * All runtime stats. @see stats_macros.h */ // clang-format off -#define ALL_RUNTIME_STATS(COUNTER, GAUGE) \ - COUNTER(load_error) \ - COUNTER(override_dir_not_exists) \ - COUNTER(override_dir_exists) \ - COUNTER(load_success) \ - COUNTER(deprecated_feature_use) \ - GAUGE (num_keys) \ - GAUGE (admin_overrides_active) +#define ALL_RUNTIME_STATS(BOOL_INDICATOR, COUNTER, GAUGE) \ + COUNTER (load_error) \ + COUNTER (override_dir_not_exists) \ + COUNTER (override_dir_exists) \ + COUNTER (load_success) \ + COUNTER (deprecated_feature_use) \ + GAUGE (num_keys) \ + BOOL_INDICATOR (admin_overrides_active) // clang-format on /** * Struct definition for all runtime stats. @see stats_macros.h */ struct RuntimeStats { - ALL_RUNTIME_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) + ALL_RUNTIME_STATS(GENERATE_BOOL_INDICATOR_STRUCT, GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; /** diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc index d85d38ef03213..8caa8561c5fad 100644 --- a/source/common/stats/isolated_store_impl.cc +++ b/source/common/stats/isolated_store_impl.cc @@ -23,6 +23,11 @@ IsolatedStoreImpl::IsolatedStoreImpl() std::vector tags; return alloc_.makeGauge(name, std::move(tag_extracted_name), std::move(tags)); }), + bool_indicators_([this](const std::string& name) -> BoolIndicatorSharedPtr { + std::string tag_extracted_name = name; + std::vector tags; + return alloc_.makeBoolIndicator(name, std::move(tag_extracted_name), std::move(tags)); + }), histograms_([this](const std::string& name) -> HistogramSharedPtr { return std::make_shared(name, *this, std::string(name), std::vector()); }) {} @@ -38,6 +43,9 @@ struct IsolatedScopeImpl : public Scope { void deliverHistogramToSinks(const Histogram&, uint64_t) override {} Counter& counter(const std::string& name) override { return parent_.counter(prefix_ + name); } Gauge& gauge(const std::string& name) override { return parent_.gauge(prefix_ + name); } + BoolIndicator& boolIndicator(const std::string& name) override { + return parent_.boolIndicator(prefix_ + name); + } Histogram& histogram(const std::string& name) override { return parent_.histogram(prefix_ + name); } diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index 7765fd50e3e72..91b8d39d1c127 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -61,6 +61,9 @@ class IsolatedStoreImpl : public Store { ScopePtr createScope(const std::string& name) override; void deliverHistogramToSinks(const Histogram&, uint64_t) override {} Gauge& gauge(const std::string& name) override { return gauges_.get(name); } + BoolIndicator& boolIndicator(const std::string& name) override { + return bool_indicators_.get(name); + } Histogram& histogram(const std::string& name) override { Histogram& histogram = histograms_.get(name); return histogram; @@ -70,6 +73,9 @@ class IsolatedStoreImpl : public Store { // Stats::Store std::vector counters() const override { return counters_.toVector(); } std::vector gauges() const override { return gauges_.toVector(); } + std::vector boolIndicators() const override { + return bool_indicators_.toVector(); + } std::vector histograms() const override { return std::vector{}; } @@ -78,6 +84,7 @@ class IsolatedStoreImpl : public Store { HeapStatDataAllocator alloc_; IsolatedStatsCache counters_; IsolatedStatsCache gauges_; + IsolatedStatsCache bool_indicators_; IsolatedStatsCache histograms_; const StatsOptionsImpl stats_options_; }; diff --git a/source/common/stats/source_impl.cc b/source/common/stats/source_impl.cc index e6102d5a608cd..a0cbbe84ff0f5 100644 --- a/source/common/stats/source_impl.cc +++ b/source/common/stats/source_impl.cc @@ -17,6 +17,12 @@ std::vector& SourceImpl::cachedGauges() { } return *gauges_; } +std::vector& SourceImpl::cachedBoolIndicators() { + if (!bool_indicators_) { + bool_indicators_ = store_.boolIndicators(); + } + return *bool_indicators_; +} std::vector& SourceImpl::cachedHistograms() { if (!histograms_) { histograms_ = store_.histograms(); @@ -27,6 +33,7 @@ std::vector& SourceImpl::cachedHistograms() { void SourceImpl::clearCache() { counters_.reset(); gauges_.reset(); + bool_indicators_.reset(); histograms_.reset(); } diff --git a/source/common/stats/source_impl.h b/source/common/stats/source_impl.h index a3a79ba9a76ab..5c85d8c980ce8 100644 --- a/source/common/stats/source_impl.h +++ b/source/common/stats/source_impl.h @@ -16,6 +16,7 @@ class SourceImpl : public Source { // Stats::Source std::vector& cachedCounters() override; std::vector& cachedGauges() override; + std::vector& cachedBoolIndicators() override; std::vector& cachedHistograms() override; void clearCache() override; @@ -23,6 +24,7 @@ class SourceImpl : public Source { Store& store_; absl::optional> counters_; absl::optional> gauges_; + absl::optional> bool_indicators_; absl::optional> histograms_; }; diff --git a/source/common/stats/stat_data_allocator_impl.h b/source/common/stats/stat_data_allocator_impl.h index 81df5f4269576..203f9aef1ed35 100644 --- a/source/common/stats/stat_data_allocator_impl.h +++ b/source/common/stats/stat_data_allocator_impl.h @@ -34,6 +34,8 @@ template class StatDataAllocatorImpl : public StatDataAllocator std::vector&& tags) override; GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name, std::vector&& tags) override; + BoolIndicatorSharedPtr makeBoolIndicator(absl::string_view name, std::string&& tag_extracted_name, + std::vector&& tags) override; /** * @param name the full name of the stat. @@ -69,6 +71,7 @@ template class CounterImpl : public Counter, public MetricImpl // Stats::Metric std::string name() const override { return std::string(data_.name()); } const char* nameCStr() const override { return data_.name(); } + bool used() const override { return data_.flags_ & Flags::Used; } // Stats::Counter void add(uint64_t amount) override { @@ -80,7 +83,6 @@ template class CounterImpl : public Counter, public MetricImpl void inc() override { add(1); } uint64_t latch() override { return data_.pending_increment_.exchange(0); } void reset() override { data_.value_ = 0; } - bool used() const override { return data_.flags_ & Flags::Used; } uint64_t value() const override { return data_.value_; } private: @@ -121,6 +123,7 @@ template class GaugeImpl : public Gauge, public MetricImpl { // Stats::Metric std::string name() const override { return std::string(data_.name()); } const char* nameCStr() const override { return data_.name(); } + bool used() const override { return data_.flags_ & Flags::Used; } // Stats::Gauge virtual void add(uint64_t amount) override { @@ -139,7 +142,6 @@ template class GaugeImpl : public Gauge, public MetricImpl { data_.value_ -= amount; } virtual uint64_t value() const override { return data_.value_; } - bool used() const override { return data_.flags_ & Flags::Used; } private: StatData& data_; @@ -167,6 +169,50 @@ class NullGaugeImpl : public Gauge { uint64_t value() const override { return 0; } }; +/** + * BoolIndicator implementation that wraps a StatData. + */ +template class BoolIndicatorImpl : public BoolIndicator, public MetricImpl { +public: + BoolIndicatorImpl(StatData& data, StatDataAllocatorImpl& alloc, + std::string&& tag_extracted_name, std::vector&& tags) + : MetricImpl(std::move(tag_extracted_name), std::move(tags)), data_(data), alloc_(alloc) {} + ~BoolIndicatorImpl() { alloc_.free(data_); } + + // Stats::Metric + std::string name() const override { return std::string(data_.name()); } + const char* nameCStr() const override { return data_.name(); } + bool used() const override { return data_.flags_ & Flags::Used; } + + // Stats::BoolIndicator + virtual void set(bool value) override { + data_.value_ = value ? 1 : 0; + data_.flags_ |= Flags::Used; + } + virtual bool value() const override { return data_.value_; } + +private: + StatData& data_; + StatDataAllocatorImpl& alloc_; +}; + +/** + * Null bool implementation. + * No-ops on all calls and requires no underlying metric or data. + */ +class NullBoolIndicatorImpl : public BoolIndicator { +public: + NullBoolIndicatorImpl() {} + ~NullBoolIndicatorImpl() {} + std::string name() const override { return ""; } + const char* nameCStr() const override { return ""; } + const std::string& tagExtractedName() const override { CONSTRUCT_ON_FIRST_USE(std::string, ""); } + const std::vector& tags() const override { CONSTRUCT_ON_FIRST_USE(std::vector, {}); } + void set(bool) override {} + bool used() const override { return false; } + bool value() const override { return false; } +}; + template CounterSharedPtr StatDataAllocatorImpl::makeCounter(absl::string_view name, std::string&& tag_extracted_name, @@ -191,5 +237,16 @@ GaugeSharedPtr StatDataAllocatorImpl::makeGauge(absl::string_view name std::move(tags)); } +template +BoolIndicatorSharedPtr StatDataAllocatorImpl::makeBoolIndicator( + absl::string_view name, std::string&& tag_extracted_name, std::vector&& tags) { + StatData* data = alloc(name); + if (data == nullptr) { + return nullptr; + } + return std::make_shared>(*data, *this, std::move(tag_extracted_name), + std::move(tags)); +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 2c72dbf94b983..d8d7567658734 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -45,6 +45,7 @@ void ThreadLocalStoreImpl::setStatsMatcher(StatsMatcherPtr&& stats_matcher) { for (ScopeImpl* scope : scopes_) { removeRejectedStats(scope->central_cache_.counters_, deleted_counters_); removeRejectedStats(scope->central_cache_.gauges_, deleted_gauges_); + removeRejectedStats(scope->central_cache_.bool_indicators_, deleted_bool_indicators_); removeRejectedStats(scope->central_cache_.histograms_, deleted_histograms_); } } @@ -111,6 +112,22 @@ std::vector ThreadLocalStoreImpl::gauges() const { return ret; } +std::vector ThreadLocalStoreImpl::boolIndicators() const { + // Handle de-dup due to overlapping scopes. + std::vector ret; + CharStarHashSet names; + Thread::LockGuard lock(lock_); + for (ScopeImpl* scope : scopes_) { + for (auto& bool_indicator : scope->central_cache_.bool_indicators_) { + if (names.insert(bool_indicator.first).second) { + ret.push_back(bool_indicator.second); + } + } + } + + return ret; +} + std::vector ThreadLocalStoreImpl::histograms() const { std::vector ret; Thread::LockGuard lock(lock_); @@ -359,6 +376,34 @@ Gauge& ThreadLocalStoreImpl::ScopeImpl::gauge(const std::string& name) { tls_cache); } +BoolIndicator& ThreadLocalStoreImpl::ScopeImpl::boolIndicator(const std::string& name) { + // See comments in counter(). There is no super clean way (via templates or otherwise) to + // share this code so I'm leaving it largely duplicated for now. + // + // Note that we can do map.find(final_name.c_str()), but we cannot do + // map[final_name.c_str()] as the char*-keyed maps would then save the pointer to + // a temporary, and address sanitization errors would follow. Instead we must + // do a find() first, using that if it succeeds. If it fails, then after we + // construct the stat we can insert it into the required maps. + std::string final_name = prefix_ + name; + if (parent_.rejects(final_name)) { + return null_bool_; + } + + StatMap* tls_cache = nullptr; + if (!parent_.shutting_down_ && parent_.tls_) { + tls_cache = &parent_.tls_->getTyped().scope_cache_[this->scope_id_].bool_indicators_; + } + + return safeMakeStat( + final_name, central_cache_.bool_indicators_, + [](StatDataAllocator& allocator, absl::string_view name, std::string&& tag_extracted_name, + std::vector&& tags) -> BoolIndicatorSharedPtr { + return allocator.makeBoolIndicator(name, std::move(tag_extracted_name), std::move(tags)); + }, + tls_cache); +} + Histogram& ThreadLocalStoreImpl::ScopeImpl::histogram(const std::string& name) { // See comments in counter(). There is no super clean way (via templates or otherwise) to // share this code so I'm leaving it largely duplicated for now. diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 4f85c763567c2..292147f44d473 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -145,6 +145,9 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo return default_scope_->deliverHistogramToSinks(histogram, value); } Gauge& gauge(const std::string& name) override { return default_scope_->gauge(name); } + BoolIndicator& boolIndicator(const std::string& name) override { + return default_scope_->boolIndicator(name); + } Histogram& histogram(const std::string& name) override { return default_scope_->histogram(name); }; @@ -152,6 +155,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo // Stats::Store std::vector counters() const override; std::vector gauges() const override; + std::vector boolIndicators() const override; std::vector histograms() const override; // Stats::StoreRoot @@ -176,6 +180,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo struct TlsCacheEntry { StatMap counters_; StatMap gauges_; + StatMap bool_indicators_; StatMap histograms_; StatMap parent_histograms_; }; @@ -183,6 +188,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo struct CentralCacheEntry { StatMap counters_; StatMap gauges_; + StatMap bool_indicators_; StatMap histograms_; }; @@ -199,6 +205,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo } void deliverHistogramToSinks(const Histogram& histogram, uint64_t value) override; Gauge& gauge(const std::string& name) override; + BoolIndicator& boolIndicator(const std::string& name) override; Histogram& histogram(const std::string& name) override; Histogram& tlsHistogram(const std::string& name, ParentHistogramImpl& parent) override; const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); } @@ -234,6 +241,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo NullCounterImpl null_counter_; NullGaugeImpl null_gauge_; + NullBoolIndicatorImpl null_bool_; NullHistogramImpl null_histogram_; }; @@ -283,6 +291,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo // but that would be fairly complex to change. std::vector deleted_counters_; std::vector deleted_gauges_; + std::vector deleted_bool_indicators_; std::vector deleted_histograms_; }; diff --git a/source/common/upstream/resource_manager_impl.h b/source/common/upstream/resource_manager_impl.h index 35cf4bf87b7ff..3c564d4c8bb07 100644 --- a/source/common/upstream/resource_manager_impl.h +++ b/source/common/upstream/resource_manager_impl.h @@ -44,20 +44,21 @@ class ResourceManagerImpl : public ResourceManager { private: struct ResourceImpl : public Resource { ResourceImpl(uint64_t max, Runtime::Loader& runtime, const std::string& runtime_key, - Stats::Gauge& open_gauge) - : max_(max), runtime_(runtime), runtime_key_(runtime_key), open_gauge_(open_gauge) {} + Stats::BoolIndicator& circuit_breaker_open) + : max_(max), runtime_(runtime), runtime_key_(runtime_key), + circuit_breaker_open_(circuit_breaker_open) {} ~ResourceImpl() { ASSERT(current_ == 0); } // Upstream::Resource bool canCreate() override { return current_ < max(); } void inc() override { current_++; - open_gauge_.set(canCreate() ? 0 : 1); + circuit_breaker_open_.set(!canCreate()); } void dec() override { ASSERT(current_ > 0); current_--; - open_gauge_.set(canCreate() ? 0 : 1); + circuit_breaker_open_.set(!canCreate()); } uint64_t max() override { return runtime_.snapshot().getInteger(runtime_key_, max_); } @@ -67,11 +68,10 @@ class ResourceManagerImpl : public ResourceManager { const std::string runtime_key_; /** - * A gauge to notify the live circuit breaker state. The gauge is set to 0 - * to notify that the circuit breaker is closed, or to 1 to notify that it - * is open. + * The live circuit breaker state: false when the circuit breaker is closed, + * true when open. */ - Stats::Gauge& open_gauge_; + Stats::BoolIndicator& circuit_breaker_open_; }; ResourceImpl connections_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index ea8f6f6170620..0ddc9a644bbdf 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -914,7 +914,7 @@ ClusterInfoImpl::ResourceManagers::ResourceManagers(const envoy::api::v2::Cluste ClusterCircuitBreakersStats ClusterInfoImpl::generateCircuitBreakersStats(Stats::Scope& scope, const std::string& stat_prefix) { std::string prefix(fmt::format("circuit_breakers.{}.", stat_prefix)); - return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE_PREFIX(scope, prefix))}; + return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR_PREFIX(scope, prefix))}; } ResourceManagerImplPtr diff --git a/source/server/overload_manager_impl.cc b/source/server/overload_manager_impl.cc index 364f739bfaa1a..6a9401208d001 100644 --- a/source/server/overload_manager_impl.cc +++ b/source/server/overload_manager_impl.cc @@ -41,7 +41,7 @@ std::string StatsName(const std::string& a, const std::string& b) { OverloadAction::OverloadAction(const envoy::config::overload::v2alpha::OverloadAction& config, Stats::Scope& stats_scope) - : active_gauge_(stats_scope.gauge(StatsName(config.name(), "active"))) { + : active_indicator_(stats_scope.boolIndicator(StatsName(config.name(), "active"))) { for (const auto& trigger_config : config.triggers()) { TriggerPtr trigger; @@ -59,7 +59,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v2alpha::OverloadA } } - active_gauge_.set(0); + active_indicator_.set(false); } bool OverloadAction::updateResourcePressure(const std::string& name, double pressure) { @@ -69,11 +69,11 @@ bool OverloadAction::updateResourcePressure(const std::string& name, double pres ASSERT(it != triggers_.end()); if (it->second->updateValue(pressure)) { if (it->second->isFired()) { - active_gauge_.set(1); + active_indicator_.set(true); const auto result = fired_triggers_.insert(name); ASSERT(result.second); } else { - active_gauge_.set(0); + active_indicator_.set(false); const auto result = fired_triggers_.erase(name); ASSERT(result == 1); } diff --git a/source/server/overload_manager_impl.h b/source/server/overload_manager_impl.h index 9b7293ed231c9..a2519003211ca 100644 --- a/source/server/overload_manager_impl.h +++ b/source/server/overload_manager_impl.h @@ -46,7 +46,7 @@ class OverloadAction { private: std::unordered_map triggers_; std::unordered_set fired_triggers_; - Stats::Gauge& active_gauge_; + Stats::BoolIndicator& active_indicator_; }; class OverloadManagerImpl : Logger::Loggable, public OverloadManager { diff --git a/source/server/server.cc b/source/server/server.cc index 382e260181a56..1649f4f3a6a03 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -125,10 +125,7 @@ void InstanceImpl::drainListeners() { drain_manager_->startDrainSequence(nullptr); } -void InstanceImpl::failHealthcheck(bool fail) { - // We keep liveness state in shared memory so the parent process sees the same state. - server_stats_->live_.set(!fail); -} +void InstanceImpl::failHealthcheck(bool fail) { server_stats_->live_.set(!fail); } void InstanceUtil::flushMetricsToSinks(const std::list& sinks, Stats::Source& source) { @@ -168,7 +165,7 @@ void InstanceImpl::getParentStats(HotRestart::GetParentStatsInfo& info) { info.num_connections_ = numConnections(); } -bool InstanceImpl::healthCheckFailed() { return server_stats_->live_.value() == 0; } +bool InstanceImpl::healthCheckFailed() { return !server_stats_->live_.value(); } InstanceUtil::BootstrapVersion InstanceUtil::loadBootstrapConfig(envoy::config::bootstrap::v2::Bootstrap& bootstrap, @@ -235,7 +232,8 @@ void InstanceImpl::initialize(const Options& options, const std::string server_stats_prefix = "server."; server_stats_ = std::make_unique( - ServerStats{ALL_SERVER_STATS(POOL_COUNTER_PREFIX(stats_store_, server_stats_prefix), + ServerStats{ALL_SERVER_STATS(POOL_BOOL_INDICATOR_PREFIX(stats_store_, server_stats_prefix), + POOL_COUNTER_PREFIX(stats_store_, server_stats_prefix), POOL_GAUGE_PREFIX(stats_store_, server_stats_prefix))}); server_stats_->concurrency_.set(options_.concurrency()); diff --git a/source/server/server.h b/source/server/server.h index 27b02a0722f43..aa026fb65ce13 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -46,22 +46,22 @@ namespace Server { * All server wide stats. @see stats_macros.h */ // clang-format off -#define ALL_SERVER_STATS(COUNTER, GAUGE) \ - GAUGE(uptime) \ +#define ALL_SERVER_STATS(BOOL_INDICATOR, COUNTER, GAUGE) \ + BOOL_INDICATOR(live) \ + COUNTER(debug_assertion_failures) \ GAUGE(concurrency) \ + GAUGE(days_until_first_cert_expiring) \ + GAUGE(hot_restart_epoch) \ GAUGE(memory_allocated) \ GAUGE(memory_heap_size) \ - GAUGE(live) \ GAUGE(parent_connections) \ GAUGE(total_connections) \ - GAUGE(version) \ - GAUGE(days_until_first_cert_expiring) \ - GAUGE(hot_restart_epoch) \ - COUNTER(debug_assertion_failures) + GAUGE(uptime) \ + GAUGE(version) // clang-format on struct ServerStats { - ALL_SERVER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) + ALL_SERVER_STATS(GENERATE_BOOL_INDICATOR_STRUCT, GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; /** diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index d0b178a123279..4f855612e942e 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -108,7 +108,7 @@ TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { expectSendMessage("foo", {"x", "y"}, ""); expectSendMessage("bar", {}, ""); grpc_mux_->start(); - EXPECT_EQ(1, stats_.gauge("control_plane.connected_state").value()); + EXPECT_TRUE(stats_.boolIndicator("control_plane.connected_state").value()); expectSendMessage("bar", {"z"}, ""); auto bar_z_sub = grpc_mux_->subscribe("bar", {"z"}, callbacks_); expectSendMessage("bar", {"zz", "z"}, ""); @@ -146,7 +146,7 @@ TEST_F(GrpcMuxImplTest, ResetStream) { ASSERT_TRUE(timer != nullptr); // initialized from dispatcher mock. EXPECT_CALL(*timer, enableTimer(_)); grpc_mux_->onRemoteClose(Grpc::Status::GrpcStatus::Canceled, ""); - EXPECT_EQ(0, stats_.gauge("control_plane.connected_state").value()); + EXPECT_FALSE(stats_.boolIndicator("control_plane.connected_state").value()); EXPECT_CALL(*async_client_, start(_, _)).WillOnce(Return(&async_stream_)); expectSendMessage("foo", {"x", "y"}, ""); expectSendMessage("bar", {}, ""); diff --git a/test/common/config/subscription_test_harness.h b/test/common/config/subscription_test_harness.h index 0df305a00f581..1b50774a256e0 100644 --- a/test/common/config/subscription_test_harness.h +++ b/test/common/config/subscription_test_harness.h @@ -58,8 +58,8 @@ class SubscriptionTestHarness { EXPECT_EQ(version, stats_.version_.value()); } - virtual void verifyControlPlaneStats(uint32_t connected_state) { - EXPECT_EQ(connected_state, stats_store_.gauge("control_plane.connected_state").value()); + virtual void verifyControlPlaneStats(bool connected_state) { + EXPECT_EQ(connected_state, stats_store_.boolIndicator("control_plane.connected_state").value()); } Stats::IsolatedStoreImpl stats_store_; diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index cbddc096fcae6..ed06a4734b1bd 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -295,7 +295,7 @@ TEST_F(Http1ConnPoolImplTest, MultipleRequestAndResponse) { TEST_F(Http1ConnPoolImplTest, MaxPendingRequests) { cluster_->resetResourceManager(1, 1, 1024, 1); - EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_pending_open_.value()); + EXPECT_FALSE(cluster_->circuit_breakers_stats_.rq_pending_open_.value()); NiceMock outer_decoder; ConnPoolCallbacks callbacks; @@ -309,7 +309,7 @@ TEST_F(Http1ConnPoolImplTest, MaxPendingRequests) { Http::ConnectionPool::Cancellable* handle2 = conn_pool_.newStream(outer_decoder2, callbacks2); EXPECT_EQ(nullptr, handle2); - EXPECT_EQ(1U, cluster_->circuit_breakers_stats_.rq_pending_open_.value()); + EXPECT_TRUE(cluster_->circuit_breakers_stats_.rq_pending_open_.value()); handle->cancel(); @@ -436,7 +436,7 @@ TEST_F(Http1ConnPoolImplTest, DisconnectWhileBound) { TEST_F(Http1ConnPoolImplTest, MaxConnections) { InSequence s; - EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.cx_open_.value()); + EXPECT_FALSE(cluster_->circuit_breakers_stats_.cx_open_.value()); // Request 1 should kick off a new connection. NiceMock outer_decoder1; @@ -451,7 +451,7 @@ TEST_F(Http1ConnPoolImplTest, MaxConnections) { ConnPoolCallbacks callbacks2; handle = conn_pool_.newStream(outer_decoder2, callbacks2); EXPECT_EQ(1U, cluster_->stats_.upstream_cx_overflow_.value()); - EXPECT_EQ(1U, cluster_->circuit_breakers_stats_.cx_open_.value()); + EXPECT_TRUE(cluster_->circuit_breakers_stats_.cx_open_.value()); EXPECT_NE(nullptr, handle); diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 75327b04e9684..95917de1f1bad 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -440,7 +440,7 @@ TEST_F(Http2ConnPoolImplTest, LocalReset) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); EXPECT_EQ(1U, cluster_->stats_.upstream_rq_tx_reset_.value()); - EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_open_.value()); + EXPECT_FALSE(cluster_->circuit_breakers_stats_.rq_open_.value()); } TEST_F(Http2ConnPoolImplTest, RemoteReset) { @@ -581,7 +581,7 @@ TEST_F(Http2ConnPoolImplTest, DrainPrimaryNoActiveRequest) { TEST_F(Http2ConnPoolImplTest, ConnectTimeout) { InSequence s; - EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_open_.value()); + EXPECT_FALSE(cluster_->circuit_breakers_stats_.rq_open_.value()); expectClientCreate(); ActiveTestRequest r1(*this, 0, false); @@ -591,7 +591,7 @@ TEST_F(Http2ConnPoolImplTest, ConnectTimeout) { EXPECT_CALL(*this, onClientDestroy()); dispatcher_.clearDeferredDeleteList(); - EXPECT_EQ(0U, cluster_->circuit_breakers_stats_.rq_open_.value()); + EXPECT_FALSE(cluster_->circuit_breakers_stats_.rq_open_.value()); expectClientCreate(); ActiveTestRequest r2(*this, 1, false); diff --git a/test/common/router/retry_state_impl_test.cc b/test/common/router/retry_state_impl_test.cc index b126f70cca3f1..309b6592dca69 100644 --- a/test/common/router/retry_state_impl_test.cc +++ b/test/common/router/retry_state_impl_test.cc @@ -459,7 +459,7 @@ TEST_F(RouterRetryStateImplTest, MaxRetriesHeader) { EXPECT_CALL(callback_ready_, ready()); retry_timer_->callback_(); - EXPECT_EQ(1UL, cluster_.circuit_breakers_stats_.rq_retry_open_.value()); + EXPECT_TRUE(cluster_.circuit_breakers_stats_.rq_retry_open_.value()); EXPECT_EQ(RetryStatus::NoRetryLimitExceeded, state_->shouldRetryReset(connect_failure_, callback_)); @@ -499,7 +499,7 @@ TEST_F(RouterRetryStateImplTest, Backoff) { EXPECT_EQ(3UL, cluster_.stats().upstream_rq_retry_.value()); EXPECT_EQ(1UL, cluster_.stats().upstream_rq_retry_success_.value()); - EXPECT_EQ(0UL, cluster_.circuit_breakers_stats_.rq_retry_open_.value()); + EXPECT_FALSE(cluster_.circuit_breakers_stats_.rq_retry_open_.value()); } TEST_F(RouterRetryStateImplTest, HostSelectionAttempts) { diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 6cb66c3d17856..6e26b4893b106 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -250,22 +250,22 @@ void testNewOverrides(Loader& loader, Stats::Store& store) { // New string loader.mergeValues({{"foo", "bar"}}); EXPECT_EQ("bar", loader.snapshot().get("foo")); - EXPECT_EQ(1, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_TRUE(store.boolIndicator("runtime.admin_overrides_active").value()); // Remove new string loader.mergeValues({{"foo", ""}}); EXPECT_EQ("", loader.snapshot().get("foo")); - EXPECT_EQ(0, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_FALSE(store.boolIndicator("runtime.admin_overrides_active").value()); // New integer loader.mergeValues({{"baz", "42"}}); EXPECT_EQ(42, loader.snapshot().getInteger("baz", 0)); - EXPECT_EQ(1, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_TRUE(store.boolIndicator("runtime.admin_overrides_active").value()); // Remove new integer loader.mergeValues({{"baz", ""}}); EXPECT_EQ(0, loader.snapshot().getInteger("baz", 0)); - EXPECT_EQ(0, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_FALSE(store.boolIndicator("runtime.admin_overrides_active").value()); } TEST_F(DiskBackedLoaderImplTest, mergeValues) { @@ -276,32 +276,32 @@ TEST_F(DiskBackedLoaderImplTest, mergeValues) { // Override string loader->mergeValues({{"file2", "new world"}}); EXPECT_EQ("new world", loader->snapshot().get("file2")); - EXPECT_EQ(1, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_TRUE(store.boolIndicator("runtime.admin_overrides_active").value()); // Remove overridden string loader->mergeValues({{"file2", ""}}); EXPECT_EQ("world", loader->snapshot().get("file2")); - EXPECT_EQ(0, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_FALSE(store.boolIndicator("runtime.admin_overrides_active").value()); // Override integer loader->mergeValues({{"file3", "42"}}); EXPECT_EQ(42, loader->snapshot().getInteger("file3", 1)); - EXPECT_EQ(1, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_TRUE(store.boolIndicator("runtime.admin_overrides_active").value()); // Remove overridden integer loader->mergeValues({{"file3", ""}}); EXPECT_EQ(2, loader->snapshot().getInteger("file3", 1)); - EXPECT_EQ(0, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_FALSE(store.boolIndicator("runtime.admin_overrides_active").value()); // Override override string loader->mergeValues({{"file1", "hello overridden override"}}); EXPECT_EQ("hello overridden override", loader->snapshot().get("file1")); - EXPECT_EQ(1, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_TRUE(store.boolIndicator("runtime.admin_overrides_active").value()); // Remove overridden override string loader->mergeValues({{"file1", ""}}); EXPECT_EQ("hello override", loader->snapshot().get("file1")); - EXPECT_EQ(0, store.gauge("runtime.admin_overrides_active").value()); + EXPECT_FALSE(store.boolIndicator("runtime.admin_overrides_active").value()); } TEST(LoaderImplTest, All) { diff --git a/test/common/stats/isolated_store_impl_test.cc b/test/common/stats/isolated_store_impl_test.cc index b2aaa80693fdd..a6401a063aa4a 100644 --- a/test/common/stats/isolated_store_impl_test.cc +++ b/test/common/stats/isolated_store_impl_test.cc @@ -22,7 +22,7 @@ TEST(StatsIsolatedStoreImplTest, All) { EXPECT_EQ("c1", c1.tagExtractedName()); EXPECT_EQ("scope1.c2", c2.tagExtractedName()); EXPECT_EQ(0, c1.tags().size()); - EXPECT_EQ(0, c1.tags().size()); + EXPECT_EQ(0, c2.tags().size()); Gauge& g1 = store.gauge("g1"); Gauge& g2 = scope1->gauge("g2"); @@ -31,7 +31,16 @@ TEST(StatsIsolatedStoreImplTest, All) { EXPECT_EQ("g1", g1.tagExtractedName()); EXPECT_EQ("scope1.g2", g2.tagExtractedName()); EXPECT_EQ(0, g1.tags().size()); - EXPECT_EQ(0, g1.tags().size()); + EXPECT_EQ(0, g2.tags().size()); + + BoolIndicator& b1 = store.boolIndicator("b1"); + BoolIndicator& b2 = scope1->boolIndicator("b2"); + EXPECT_EQ("b1", b1.name()); + EXPECT_EQ("scope1.b2", b2.name()); + EXPECT_EQ("b1", b1.tagExtractedName()); + EXPECT_EQ("scope1.b2", b2.tagExtractedName()); + EXPECT_EQ(0, b1.tags().size()); + EXPECT_EQ(0, b2.tags().size()); Histogram& h1 = store.histogram("h1"); Histogram& h2 = scope1->histogram("h2"); @@ -54,6 +63,7 @@ TEST(StatsIsolatedStoreImplTest, All) { EXPECT_EQ(4UL, store.counters().size()); EXPECT_EQ(2UL, store.gauges().size()); + EXPECT_EQ(2UL, store.boolIndicators().size()); } TEST(StatsIsolatedStoreImplTest, LongStatName) { @@ -70,20 +80,23 @@ TEST(StatsIsolatedStoreImplTest, LongStatName) { * Test stats macros. @see stats_macros.h */ // clang-format off -#define ALL_TEST_STATS(COUNTER, GAUGE, HISTOGRAM) \ - COUNTER (test_counter) \ - GAUGE (test_gauge) \ - HISTOGRAM(test_histogram) +#define ALL_TEST_STATS(COUNTER, GAUGE, BOOL_INDICATOR, HISTOGRAM) \ + COUNTER (test_counter) \ + GAUGE (test_gauge) \ + BOOL_INDICATOR(test_bool_indicator) \ + HISTOGRAM (test_histogram) // clang-format on struct TestStats { - ALL_TEST_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT) + ALL_TEST_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_BOOL_INDICATOR_STRUCT, + GENERATE_HISTOGRAM_STRUCT) }; TEST(StatsMacros, All) { IsolatedStoreImpl stats_store; TestStats test_stats{ALL_TEST_STATS(POOL_COUNTER_PREFIX(stats_store, "test."), POOL_GAUGE_PREFIX(stats_store, "test."), + POOL_BOOL_INDICATOR_PREFIX(stats_store, "test."), POOL_HISTOGRAM_PREFIX(stats_store, "test."))}; Counter& counter = test_stats.test_counter_; @@ -92,6 +105,9 @@ TEST(StatsMacros, All) { Gauge& gauge = test_stats.test_gauge_; EXPECT_EQ("test.test_gauge", gauge.name()); + BoolIndicator& boolIndicator = test_stats.test_bool_indicator_; + EXPECT_EQ("test.test_bool_indicator", boolIndicator.name()); + Histogram& histogram = test_stats.test_histogram_; EXPECT_EQ("test.test_histogram", histogram.name()); } diff --git a/test/common/stats/source_impl_test.cc b/test/common/stats/source_impl_test.cc index bf5b15851d308..76f86905c326b 100644 --- a/test/common/stats/source_impl_test.cc +++ b/test/common/stats/source_impl_test.cc @@ -17,10 +17,12 @@ TEST(SourceImplTest, Caching) { NiceMock store; std::vector stored_counters; std::vector stored_gauges; + std::vector stored_bools; std::vector stored_histograms; ON_CALL(store, counters()).WillByDefault(ReturnPointee(&stored_counters)); ON_CALL(store, gauges()).WillByDefault(ReturnPointee(&stored_gauges)); + ON_CALL(store, boolIndicators()).WillByDefault(ReturnPointee(&stored_bools)); ON_CALL(store, histograms()).WillByDefault(ReturnPointee(&stored_histograms)); SourceImpl source(store); @@ -36,6 +38,11 @@ TEST(SourceImplTest, Caching) { stored_gauges.push_back(std::make_shared()); EXPECT_NE(source.cachedGauges(), stored_gauges); + stored_bools.push_back(std::make_shared()); + EXPECT_EQ(source.cachedBoolIndicators(), stored_bools); + stored_bools.push_back(std::make_shared()); + EXPECT_NE(source.cachedBoolIndicators(), stored_bools); + stored_histograms.push_back(std::make_shared()); EXPECT_EQ(source.cachedHistograms(), stored_histograms); stored_histograms.push_back(std::make_shared()); @@ -45,6 +52,7 @@ TEST(SourceImplTest, Caching) { source.clearCache(); EXPECT_EQ(source.cachedCounters(), stored_counters); EXPECT_EQ(source.cachedGauges(), stored_gauges); + EXPECT_EQ(source.cachedBoolIndicators(), stored_bools); EXPECT_EQ(source.cachedHistograms(), stored_histograms); } diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index dddfea3e6146e..1b47ff6128ce0 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -181,7 +181,7 @@ class HistogramTest : public testing::Test { TEST_F(StatsThreadLocalStoreTest, NoTls) { InSequence s; - EXPECT_CALL(*alloc_, alloc(_)).Times(2); + EXPECT_CALL(*alloc_, alloc(_)).Times(3); Counter& c1 = store_->counter("c1"); EXPECT_EQ(&c1, &store_->counter("c1")); @@ -189,6 +189,9 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { Gauge& g1 = store_->gauge("g1"); EXPECT_EQ(&g1, &store_->gauge("g1")); + BoolIndicator& b1 = store_->boolIndicator("b1"); + EXPECT_EQ(&b1, &store_->boolIndicator("b1")); + Histogram& h1 = store_->histogram("h1"); EXPECT_EQ(&h1, &store_->histogram("h1")); @@ -203,9 +206,12 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { EXPECT_EQ(1UL, store_->gauges().size()); EXPECT_EQ(&g1, store_->gauges().front().get()); // front() ok when size()==1 EXPECT_EQ(2L, store_->gauges().front().use_count()); + EXPECT_EQ(1UL, store_->boolIndicators().size()); + EXPECT_EQ(&b1, store_->boolIndicators().front().get()); // front() ok when size()==1 + EXPECT_EQ(2L, store_->boolIndicators().front().use_count()); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(3); + EXPECT_CALL(*alloc_, free(_)).Times(4); store_->shutdownThreading(); } @@ -214,7 +220,7 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { InSequence s; store_->initializeThreading(main_thread_dispatcher_, tls_); - EXPECT_CALL(*alloc_, alloc(_)).Times(2); + EXPECT_CALL(*alloc_, alloc(_)).Times(3); Counter& c1 = store_->counter("c1"); EXPECT_EQ(&c1, &store_->counter("c1")); @@ -222,6 +228,9 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { Gauge& g1 = store_->gauge("g1"); EXPECT_EQ(&g1, &store_->gauge("g1")); + BoolIndicator& b1 = store_->boolIndicator("b1"); + EXPECT_EQ(&b1, &store_->boolIndicator("b1")); + Histogram& h1 = store_->histogram("h1"); EXPECT_EQ(&h1, &store_->histogram("h1")); @@ -231,6 +240,9 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { EXPECT_EQ(1UL, store_->gauges().size()); EXPECT_EQ(&g1, store_->gauges().front().get()); // front() ok when size()==1 EXPECT_EQ(3L, store_->gauges().front().use_count()); + EXPECT_EQ(1UL, store_->boolIndicators().size()); + EXPECT_EQ(&b1, store_->boolIndicators().front().get()); // front() ok when size()==1 + EXPECT_EQ(3L, store_->boolIndicators().front().use_count()); store_->shutdownThreading(); tls_.shutdownThread(); @@ -241,9 +253,12 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { EXPECT_EQ(1UL, store_->gauges().size()); EXPECT_EQ(&g1, store_->gauges().front().get()); // front() ok when size()==1 EXPECT_EQ(2L, store_->gauges().front().use_count()); + EXPECT_EQ(1UL, store_->boolIndicators().size()); + EXPECT_EQ(&b1, store_->boolIndicators().front().get()); // front() ok when size()==1 + EXPECT_EQ(2L, store_->boolIndicators().front().use_count()); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(3); + EXPECT_CALL(*alloc_, free(_)).Times(4); } TEST_F(StatsThreadLocalStoreTest, BasicScope) { @@ -251,7 +266,7 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { store_->initializeThreading(main_thread_dispatcher_, tls_); ScopePtr scope1 = store_->createScope("scope1."); - EXPECT_CALL(*alloc_, alloc(_)).Times(4); + EXPECT_CALL(*alloc_, alloc(_)).Times(6); Counter& c1 = store_->counter("c1"); Counter& c2 = scope1->counter("c2"); EXPECT_EQ("c1", c1.name()); @@ -262,6 +277,11 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { EXPECT_EQ("g1", g1.name()); EXPECT_EQ("scope1.g2", g2.name()); + BoolIndicator& b1 = store_->boolIndicator("b1"); + BoolIndicator& b2 = scope1->boolIndicator("b2"); + EXPECT_EQ("b1", b1.name()); + EXPECT_EQ("scope1.b2", b2.name()); + Histogram& h1 = store_->histogram("h1"); Histogram& h2 = scope1->histogram("h2"); EXPECT_EQ("h1", h1.name()); @@ -277,7 +297,7 @@ TEST_F(StatsThreadLocalStoreTest, BasicScope) { tls_.shutdownThread(); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(5); + EXPECT_CALL(*alloc_, free(_)).Times(7); } // Validate that we sanitize away bad characters in the stats prefix. @@ -352,11 +372,15 @@ TEST_F(StatsThreadLocalStoreTest, NestedScopes) { Gauge& g1 = scope2->gauge("some_gauge"); EXPECT_EQ("scope1.foo.some_gauge", g1.name()); + EXPECT_CALL(*alloc_, alloc(_)); + BoolIndicator& b1 = scope2->boolIndicator("some_bool"); + EXPECT_EQ("scope1.foo.some_bool", b1.name()); + store_->shutdownThreading(); tls_.shutdownThread(); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(4); + EXPECT_CALL(*alloc_, free(_)).Times(5); } TEST_F(StatsThreadLocalStoreTest, OverlappingScopes) { @@ -396,8 +420,21 @@ TEST_F(StatsThreadLocalStoreTest, OverlappingScopes) { EXPECT_EQ(1UL, g2.value()); EXPECT_EQ(1UL, store_->gauges().size()); + // Bools should work just like gauges. + EXPECT_CALL(*alloc_, alloc(_)).Times(2); + BoolIndicator& b1 = scope1->boolIndicator("b"); + BoolIndicator& b2 = scope2->boolIndicator("b"); + EXPECT_NE(&b1, &b2); + b1.set(true); + EXPECT_EQ(1, b1.value()); + EXPECT_EQ(1, b2.value()); + b2.set(false); + EXPECT_EQ(0, b1.value()); + EXPECT_EQ(0, b2.value()); + EXPECT_EQ(1UL, store_->boolIndicators().size()); + // Deleting scope 1 will call free but will be reference counted. It still leaves scope 2 valid. - EXPECT_CALL(*alloc_, free(_)).Times(2); + EXPECT_CALL(*alloc_, free(_)).Times(7); scope1.reset(); c2.inc(); EXPECT_EQ(3UL, c2.value()); @@ -405,12 +442,47 @@ TEST_F(StatsThreadLocalStoreTest, OverlappingScopes) { g2.set(10); EXPECT_EQ(10UL, g2.value()); EXPECT_EQ(1UL, store_->gauges().size()); + b2.set(false); + EXPECT_EQ(0, b2.value()); + EXPECT_EQ(1UL, store_->boolIndicators().size()); store_->shutdownThreading(); tls_.shutdownThread(); +} +// Demonstrates that counters, gauges, and indicators are all mixed together in +// the shared memory, and not separated by type; only the name matters. +// This test is only here to reassure us that PR #5813, in the context of the current +// state of the Envoy codebase it is being submitted into, will not break hot restart! +// It is not meant to enforce this behavior as a desirable feature that must be kept. +TEST_F(StatsThreadLocalStoreTest, SameNameDifferentType) { + InSequence s; + store_->initializeThreading(main_thread_dispatcher_, tls_); + EXPECT_CALL(*alloc_, alloc(_)).Times(4); + + Counter& c1 = store_->counter("samename"); + EXPECT_EQ(&c1, &store_->counter("samename")); + Gauge& g1 = store_->gauge("samename"); + EXPECT_EQ(&g1, &store_->gauge("samename")); + c1.add(5); + EXPECT_EQ(5UL, c1.value()); + g1.add(3); + EXPECT_EQ(8UL, c1.value()); + + Gauge& g2 = store_->gauge("samename2"); + EXPECT_EQ(&g2, &store_->gauge("samename2")); + BoolIndicator& b1 = store_->boolIndicator("samename2"); + EXPECT_EQ(&b1, &store_->boolIndicator("samename2")); + g2.add(1); + EXPECT_EQ(1UL, g2.value()); + EXPECT_TRUE(b1.value()); + b1.set(false); + EXPECT_EQ(0UL, g2.value()); + + store_->shutdownThreading(); + tls_.shutdownThread(); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(3); + EXPECT_CALL(*alloc_, free(_)).Times(5); } TEST_F(StatsThreadLocalStoreTest, AllocFailed) { @@ -521,6 +593,21 @@ TEST_F(StatsMatcherTLSTest, TestNoOpStatImpls) { Gauge& noop_gauge_2 = store_->gauge("noop_gauge_2"); EXPECT_EQ(&noop_gauge, &noop_gauge_2); + // BoolIndicator + BoolIndicator& noop_bool = store_->boolIndicator("noop_bool"); + EXPECT_EQ(noop_bool.name(), ""); + EXPECT_EQ(0, noop_bool.value()); + noop_bool.set(true); + EXPECT_EQ(0, noop_bool.value()); + noop_bool.set(true); + EXPECT_EQ(0, noop_bool.value()); + noop_bool.set(false); + EXPECT_EQ(0, noop_bool.value()); + noop_bool.set(true); + EXPECT_EQ(0, noop_bool.value()); + BoolIndicator& noop_bool_2 = store_->boolIndicator("noop_bool_2"); + EXPECT_EQ(&noop_bool, &noop_bool_2); + // Histogram Histogram& noop_histogram = store_->histogram("noop_histogram"); EXPECT_EQ(noop_histogram.name(), ""); @@ -538,8 +625,9 @@ TEST_F(StatsMatcherTLSTest, TestNoOpStatImpls) { TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { InSequence s; - // Expected to alloc lowercase_counter, lowercase_gauge, valid_counter, valid_gauge - EXPECT_CALL(*alloc_, alloc(_)).Times(4); + // Expected to alloc lowercase_counter, lowercase_gauge, lowercase_bool, + // valid_counter, valid_gauge, valid_bool + EXPECT_CALL(*alloc_, alloc(_)).Times(6); // Will block all stats containing any capital alphanumeric letter. stats_config_.mutable_stats_matcher()->mutable_exclusion_list()->add_patterns()->set_regex( @@ -551,6 +639,8 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { EXPECT_EQ(lowercase_counter.name(), "lowercase_counter"); Gauge& lowercase_gauge = store_->gauge("lowercase_gauge"); EXPECT_EQ(lowercase_gauge.name(), "lowercase_gauge"); + BoolIndicator& lowercase_bool = store_->boolIndicator("lowercase_bool"); + EXPECT_EQ(lowercase_bool.name(), "lowercase_bool"); Histogram& lowercase_histogram = store_->histogram("lowercase_histogram"); EXPECT_EQ(lowercase_histogram.name(), "lowercase_histogram"); @@ -569,6 +659,11 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { uppercase_gauge.inc(); EXPECT_EQ(uppercase_gauge.value(), 0); + BoolIndicator& uppercase_bool = store_->boolIndicator("uppercase_BOOL"); + EXPECT_EQ(uppercase_bool.name(), ""); + uppercase_bool.set(true); + EXPECT_FALSE(uppercase_bool.value()); + // Histograms are harder to query and test, so we resort to testing that name() returns the empty // string. Histogram& uppercase_histogram = store_->histogram("upperCASE_histogram"); @@ -589,11 +684,11 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { EXPECT_EQ(invalid_counter.value(), 0); // But the old exclusion rule still holds. - Counter& invalid_counter_2 = store_->counter("also_INVALID_counter"); + Counter& invalid_counter_2 = store_->counter("also_INVLD_counter"); invalid_counter_2.inc(); EXPECT_EQ(invalid_counter_2.value(), 0); - // And we expect the same behavior from gauges and histograms. + // And we expect the same behavior from gauges, histograms, and bools. Gauge& valid_gauge = store_->gauge("valid_gauge"); valid_gauge.set(2); EXPECT_EQ(valid_gauge.value(), 2); @@ -602,10 +697,22 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { invalid_gauge_1.inc(); EXPECT_EQ(invalid_gauge_1.value(), 0); - Gauge& invalid_gauge_2 = store_->gauge("also_INVALID_gauge"); + Gauge& invalid_gauge_2 = store_->gauge("also_INVLD_gauge"); invalid_gauge_2.inc(); EXPECT_EQ(invalid_gauge_2.value(), 0); + BoolIndicator& valid_bool = store_->boolIndicator("valid_bool"); + valid_bool.set(true); + EXPECT_EQ(1, valid_bool.value()); + + BoolIndicator& invalid_bool_1 = store_->boolIndicator("invalid_bool"); + invalid_bool_1.set(true); + EXPECT_EQ(0, invalid_gauge_1.value()); + + BoolIndicator& invalid_bool_2 = store_->boolIndicator("also_INVLD_bool"); + invalid_bool_2.set(true); + EXPECT_EQ(0, invalid_bool_2.value()); + Histogram& valid_histogram = store_->histogram("valid_histogram"); EXPECT_EQ(valid_histogram.name(), "valid_histogram"); @@ -615,9 +722,9 @@ TEST_F(StatsMatcherTLSTest, TestExclusionRegex) { Histogram& invalid_histogram_2 = store_->histogram("also_INVALID_histogram"); EXPECT_EQ(invalid_histogram_2.name(), ""); - // Expected to free lowercase_counter, lowercase_gauge, valid_counter, - // valid_gauge, overflow.stats - EXPECT_CALL(*alloc_, free(_)).Times(5); + // Expected to free lowercase_counter, lowercase_gauge, lowercase_bool, + // valid_counter, valid_gauge, valid_bool, overflow.stats + EXPECT_CALL(*alloc_, free(_)).Times(7); store_->shutdownThreading(); } @@ -641,12 +748,15 @@ class HeapStatsThreadLocalStoreTest : public StatsThreadLocalStoreTest { TEST_F(HeapStatsThreadLocalStoreTest, RemoveRejectedStats) { Counter& counter = store_->counter("c1"); Gauge& gauge = store_->gauge("g1"); + BoolIndicator& boolIndicator = store_->boolIndicator("b1"); Histogram& histogram = store_->histogram("h1"); ASSERT_EQ(2, store_->counters().size()); // "stats.overflow" and "c1". EXPECT_TRUE(&counter == store_->counters()[0].get() || &counter == store_->counters()[1].get()); // counters() order is non-deterministic. ASSERT_EQ(1, store_->gauges().size()); EXPECT_EQ("g1", store_->gauges()[0]->name()); + ASSERT_EQ(1, store_->boolIndicators().size()); + EXPECT_EQ("b1", store_->boolIndicators()[0]->name()); ASSERT_EQ(1, store_->histograms().size()); EXPECT_EQ("h1", store_->histograms()[0]->name()); @@ -659,11 +769,13 @@ TEST_F(HeapStatsThreadLocalStoreTest, RemoveRejectedStats) { // They can no longer be found. EXPECT_EQ(0, store_->counters().size()); EXPECT_EQ(0, store_->gauges().size()); + EXPECT_EQ(0, store_->boolIndicators().size()); EXPECT_EQ(0, store_->histograms().size()); // However, referencing the previously allocated stats will not crash. counter.inc(); gauge.inc(); + boolIndicator.set(true); EXPECT_CALL(sink_, onHistogramComplete(Ref(histogram), 42)); histogram.recordValue(42); } @@ -733,23 +845,27 @@ TEST_F(StatsThreadLocalStoreTest, ShuttingDown) { InSequence s; store_->initializeThreading(main_thread_dispatcher_, tls_); - EXPECT_CALL(*alloc_, alloc(_)).Times(4); + EXPECT_CALL(*alloc_, alloc(_)).Times(6); store_->counter("c1"); store_->gauge("g1"); + store_->boolIndicator("b1"); store_->shutdownThreading(); store_->counter("c2"); store_->gauge("g2"); + store_->boolIndicator("b2"); - // c1, g1 should have a thread local ref, but c2, g2 should not. + // c1, g1, b1 should have a thread local ref, but c2, g2, b2 should not. EXPECT_EQ(3L, TestUtility::findCounter(*store_, "c1").use_count()); EXPECT_EQ(3L, TestUtility::findGauge(*store_, "g1").use_count()); + EXPECT_EQ(3L, TestUtility::findBoolIndicator(*store_, "b1").use_count()); EXPECT_EQ(2L, TestUtility::findCounter(*store_, "c2").use_count()); EXPECT_EQ(2L, TestUtility::findGauge(*store_, "g2").use_count()); + EXPECT_EQ(2L, TestUtility::findBoolIndicator(*store_, "b2").use_count()); tls_.shutdownThread(); // Includes overflow stat. - EXPECT_CALL(*alloc_, free(_)).Times(5); + EXPECT_CALL(*alloc_, free(_)).Times(7); } TEST_F(StatsThreadLocalStoreTest, MergeDuringShutDown) { @@ -971,6 +1087,13 @@ TEST_F(TruncatingAllocTest, GaugeNotTruncated) { }); } +TEST_F(TruncatingAllocTest, BoolNotTruncated) { + EXPECT_NO_LOGS({ + BoolIndicator& boolIndicator = store_->boolIndicator("simple"); + EXPECT_EQ(&boolIndicator, &store_->boolIndicator("simple")); + }); +} + TEST_F(TruncatingAllocTest, CounterTruncated) { Counter* counter = nullptr; EXPECT_LOG_CONTAINS("warning", "is too long with", { @@ -989,6 +1112,15 @@ TEST_F(TruncatingAllocTest, GaugeTruncated) { EXPECT_NO_LOGS(EXPECT_EQ(gauge, &store_->gauge(long_name_))); } +TEST_F(TruncatingAllocTest, BoolTruncated) { + BoolIndicator* boolIndicator = nullptr; + EXPECT_LOG_CONTAINS("warning", "is too long with", { + BoolIndicator& b = store_->boolIndicator(long_name_); + boolIndicator = &b; + }); + EXPECT_NO_LOGS(EXPECT_EQ(boolIndicator, &store_->boolIndicator(long_name_))); +} + TEST_F(TruncatingAllocTest, HistogramWithLongNameNotTruncated) { EXPECT_NO_LOGS({ Histogram& histogram = store_->histogram(long_name_); diff --git a/test/common/upstream/resource_manager_impl_test.cc b/test/common/upstream/resource_manager_impl_test.cc index ae1c7d356d628..259023a00e577 100644 --- a/test/common/upstream/resource_manager_impl_test.cc +++ b/test/common/upstream/resource_manager_impl_test.cc @@ -19,14 +19,14 @@ namespace { TEST(ResourceManagerImplTest, RuntimeResourceManager) { NiceMock runtime; - NiceMock gauge; + NiceMock bool_indicator; NiceMock store; - ON_CALL(store, gauge(_)).WillByDefault(ReturnRef(gauge)); + ON_CALL(store, boolIndicator(_)).WillByDefault(ReturnRef(bool_indicator)); ResourceManagerImpl resource_manager( runtime, "circuit_breakers.runtime_resource_manager_test.default.", 0, 0, 0, 1, - ClusterCircuitBreakersStats{ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE(store))}); + ClusterCircuitBreakersStats{ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR(store))}); EXPECT_CALL( runtime.snapshot_, diff --git a/test/integration/overload_integration_test.cc b/test/integration/overload_integration_test.cc index 27c9818f4a949..40f8c23756915 100644 --- a/test/integration/overload_integration_test.cc +++ b/test/integration/overload_integration_test.cc @@ -63,7 +63,8 @@ TEST_P(OverloadIntegrationTest, CloseStreamsWhenOverloaded) { // Put envoy in overloaded state and check that it drops new requests. // Test both header-only and header+body requests since the code paths are slightly different. updateResource(0.9); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.stop_accepting_requests.active", 1); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.stop_accepting_requests.active", true); Http::TestHeaderMapImpl request_headers{ {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}}; @@ -87,7 +88,8 @@ TEST_P(OverloadIntegrationTest, CloseStreamsWhenOverloaded) { // Deactivate overload state and check that new requests are accepted. updateResource(0.8); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.stop_accepting_requests.active", 0); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.stop_accepting_requests.active", false); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0); @@ -109,7 +111,8 @@ TEST_P(OverloadIntegrationTest, DisableKeepaliveWhenOverloaded) { // Put envoy in overloaded state and check that it disables keepalive updateResource(0.8); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.disable_http_keepalive.active", 1); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.disable_http_keepalive.active", true); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); Http::TestHeaderMapImpl request_headers{ @@ -123,7 +126,8 @@ TEST_P(OverloadIntegrationTest, DisableKeepaliveWhenOverloaded) { // Deactivate overload state and check that keepalive is not disabled updateResource(0.7); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.disable_http_keepalive.active", 0); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.disable_http_keepalive.active", false); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); response = sendRequestAndWaitForResponse(request_headers, 1, default_response_headers_, 1); @@ -139,8 +143,8 @@ TEST_P(OverloadIntegrationTest, StopAcceptingConnectionsWhenOverloaded) { // Put envoy in overloaded state and check that it doesn't accept the new client connection. updateResource(0.95); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.stop_accepting_connections.active", - 1); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.stop_accepting_connections.active", true); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); Http::TestHeaderMapImpl request_headers{ {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}}; @@ -151,8 +155,8 @@ TEST_P(OverloadIntegrationTest, StopAcceptingConnectionsWhenOverloaded) { // Reduce load a little to allow the connection to be accepted but then immediately reject the // request. updateResource(0.9); - test_server_->waitForGaugeEq("overload.envoy.overload_actions.stop_accepting_connections.active", - 0); + test_server_->waitForBoolIndicatorEq( + "overload.envoy.overload_actions.stop_accepting_connections.active", false); response->waitForEndStream(); EXPECT_TRUE(response->complete()); diff --git a/test/integration/server.h b/test/integration/server.h index 3652119081244..6648f1f9c6fa3 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -85,6 +85,11 @@ class TestScopeWrapper : public Scope { return wrapped_scope_->gauge(name); } + BoolIndicator& boolIndicator(const std::string& name) override { + Thread::LockGuard lock(lock_); + return wrapped_scope_->boolIndicator(name); + } + Histogram& histogram(const std::string& name) override { Thread::LockGuard lock(lock_); return wrapped_scope_->histogram(name); @@ -119,6 +124,10 @@ class TestIsolatedStoreImpl : public StoreRoot { Thread::LockGuard lock(lock_); return store_.gauge(name); } + BoolIndicator& boolIndicator(const std::string& name) override { + Thread::LockGuard lock(lock_); + return store_.boolIndicator(name); + } Histogram& histogram(const std::string& name) override { Thread::LockGuard lock(lock_); return store_.histogram(name); @@ -134,7 +143,10 @@ class TestIsolatedStoreImpl : public StoreRoot { Thread::LockGuard lock(lock_); return store_.gauges(); } - + std::vector boolIndicators() const override { + Thread::LockGuard lock(lock_); + return store_.boolIndicators(); + } std::vector histograms() const override { Thread::LockGuard lock(lock_); return store_.histograms(); @@ -194,6 +206,12 @@ class IntegrationTestServer : public Logger::Loggable, std::function on_server_init_function, bool deterministic, bool defer_listener_finalization); + void waitForBoolIndicatorEq(const std::string& name, uint64_t value) { + while (boolIndicator(name) == nullptr || boolIndicator(name)->value() != value) { + time_system_.sleep(std::chrono::milliseconds(10)); + } + } + void waitForCounterGe(const std::string& name, uint64_t value) override { while (counter(name) == nullptr || counter(name)->value() < value) { time_system_.sleep(std::chrono::milliseconds(10)); @@ -212,6 +230,12 @@ class IntegrationTestServer : public Logger::Loggable, } } + Stats::BoolIndicatorSharedPtr boolIndicator(const std::string& name) { + // When using the thread local store, only boolIndicators() is thread safe. This also allows us + // to test if an indicator exists at all versus just defaulting to false. + return TestUtility::findBoolIndicator(stat_store(), name); + } + Stats::CounterSharedPtr counter(const std::string& name) override { // When using the thread local store, only counters() is thread safe. This also allows us // to test if a counter exists at all versus just defaulting to zero. diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 4ee2971236d42..63a2391a04714 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -32,8 +32,8 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, StatsIntegrationTest, TEST_P(StatsIntegrationTest, WithDefaultConfig) { initialize(); - auto live = test_server_->gauge("server.live"); - EXPECT_EQ(live->value(), 1); + auto live = test_server_->boolIndicator("server.live"); + EXPECT_TRUE(live->value()); EXPECT_EQ(live->tags().size(), 0); auto counter = test_server_->counter("http.config_test.rq_total"); @@ -122,8 +122,8 @@ TEST_P(StatsIntegrationTest, WithTagSpecifierWithFixedValue) { }); initialize(); - auto live = test_server_->gauge("server.live"); - EXPECT_EQ(live->value(), 1); + auto live = test_server_->boolIndicator("server.live"); + EXPECT_TRUE(live->value()); EXPECT_EQ(live->tags().size(), 1); EXPECT_EQ(live->tags()[0].name_, "test.x"); EXPECT_EQ(live->tags()[0].value_, "xxx"); diff --git a/test/mocks/stats/mocks.cc b/test/mocks/stats/mocks.cc index 5b1dbf98df98c..ea078cf4ff7c4 100644 --- a/test/mocks/stats/mocks.cc +++ b/test/mocks/stats/mocks.cc @@ -30,6 +30,14 @@ MockGauge::MockGauge() { } MockGauge::~MockGauge() {} +MockBoolIndicator::MockBoolIndicator() { + ON_CALL(*this, tagExtractedName()).WillByDefault(ReturnRef(name_)); + ON_CALL(*this, tags()).WillByDefault(ReturnRef(tags_)); + ON_CALL(*this, used()).WillByDefault(ReturnPointee(&used_)); + ON_CALL(*this, value()).WillByDefault(ReturnPointee(&value_)); +} +MockBoolIndicator::~MockBoolIndicator() {} + MockHistogram::MockHistogram() { ON_CALL(*this, recordValue(_)).WillByDefault(Invoke([this](uint64_t value) { if (store_ != nullptr) { diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index c079e785be16b..c4dacdaaee22f 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -74,6 +74,28 @@ class MockGauge : public Gauge { std::vector tags_; }; +class MockBoolIndicator : public BoolIndicator { +public: + MockBoolIndicator(); + ~MockBoolIndicator(); + + // Note: cannot be mocked because it is accessed as a Property in a gmock EXPECT_CALL. This + // creates a deadlock in gmock and is an unintended use of mock functions. + std::string name() const override { return name_; }; + const char* nameCStr() const override { return name_.c_str(); }; + + MOCK_CONST_METHOD0(tagExtractedName, const std::string&()); + MOCK_CONST_METHOD0(tags, const std::vector&()); + MOCK_METHOD1(set, void(bool value)); + MOCK_CONST_METHOD0(used, bool()); + MOCK_CONST_METHOD0(value, bool()); + + bool used_; + uint64_t value_; + std::string name_; + std::vector tags_; +}; + class MockHistogram : public Histogram { public: MockHistogram(); @@ -129,6 +151,7 @@ class MockSource : public Source { MOCK_METHOD0(cachedCounters, const std::vector&()); MOCK_METHOD0(cachedGauges, const std::vector&()); + MOCK_METHOD0(cachedBoolIndicators, const std::vector&()); MOCK_METHOD0(cachedHistograms, const std::vector&()); MOCK_METHOD0(clearCache, void()); @@ -159,6 +182,8 @@ class MockStore : public Store { MOCK_METHOD1(createScope_, Scope*(const std::string& name)); MOCK_METHOD1(gauge, Gauge&(const std::string&)); MOCK_CONST_METHOD0(gauges, std::vector()); + MOCK_METHOD1(boolIndicator, BoolIndicator&(const std::string&)); + MOCK_CONST_METHOD0(boolIndicators, std::vector()); MOCK_METHOD1(histogram, Histogram&(const std::string& name)); MOCK_CONST_METHOD0(histograms, std::vector()); MOCK_CONST_METHOD0(statsOptions, const StatsOptions&()); diff --git a/test/server/overload_manager_impl_test.cc b/test/server/overload_manager_impl_test.cc index 439103650548f..2b7ada09678ef 100644 --- a/test/server/overload_manager_impl_test.cc +++ b/test/server/overload_manager_impl_test.cc @@ -152,7 +152,8 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) { [&](OverloadActionState) { EXPECT_TRUE(false); }); manager->start(); - Stats::Gauge& active_gauge = stats_.gauge("overload.envoy.overload_actions.dummy_action.active"); + Stats::BoolIndicator& active_indicator = + stats_.boolIndicator("overload.envoy.overload_actions.dummy_action.active"); Stats::Gauge& pressure_gauge1 = stats_.gauge("overload.envoy.resource_monitors.fake_resource1.pressure"); Stats::Gauge& pressure_gauge2 = @@ -165,7 +166,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) { EXPECT_FALSE(is_active); EXPECT_EQ(action_state, OverloadActionState::Inactive); EXPECT_EQ(0, cb_count); - EXPECT_EQ(0, active_gauge.value()); + EXPECT_FALSE(active_indicator.value()); EXPECT_EQ(50, pressure_gauge1.value()); factory1_.monitor_->setPressure(0.95); @@ -173,7 +174,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) { EXPECT_TRUE(is_active); EXPECT_EQ(action_state, OverloadActionState::Active); EXPECT_EQ(1, cb_count); - EXPECT_EQ(1, active_gauge.value()); + EXPECT_TRUE(active_indicator.value()); EXPECT_EQ(95, pressure_gauge1.value()); // Callback should not be invoked if action active state has not changed @@ -199,7 +200,7 @@ TEST_F(OverloadManagerImplTest, CallbackOnlyFiresWhenStateChanges) { EXPECT_FALSE(is_active); EXPECT_EQ(action_state, OverloadActionState::Inactive); EXPECT_EQ(2, cb_count); - EXPECT_EQ(0, active_gauge.value()); + EXPECT_FALSE(active_indicator.value()); EXPECT_EQ(40, pressure_gauge2.value()); manager->stop(); diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index c23d065303296..ef145bda07111 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -141,6 +141,11 @@ Stats::GaugeSharedPtr TestUtility::findGauge(Stats::Store& store, const std::str return findByName(store.gauges(), name); } +Stats::BoolIndicatorSharedPtr TestUtility::findBoolIndicator(Stats::Store& store, + const std::string& name) { + return findByName(store.boolIndicators(), name); +} + std::list TestUtility::makeDnsResponse(const std::list& addresses) { std::list ret; diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 43b7ed834801e..0c4a251918878 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -152,7 +152,7 @@ class TestUtility { * Find a counter in a stats store. * @param store supplies the stats store. * @param name supplies the name to search for. - * @return Stats::CounterSharedPtr the counter or nullptr if there is none. + * @return Stats::CounterSharedPtr the counter, or nullptr if there is none. */ static Stats::CounterSharedPtr findCounter(Stats::Store& store, const std::string& name); @@ -160,10 +160,19 @@ class TestUtility { * Find a gauge in a stats store. * @param store supplies the stats store. * @param name supplies the name to search for. - * @return Stats::GaugeSharedPtr the gauge or nullptr if there is none. + * @return Stats::GaugeSharedPtr the gauge, or nullptr if there is none. */ static Stats::GaugeSharedPtr findGauge(Stats::Store& store, const std::string& name); + /** + * Find a bool in a stats store. + * @param store supplies the stats store. + * @param name supplies the name to search for. + * @return Stats::BoolIndicatorSharedPtr the bool, or nullptr if there is none. + */ + static Stats::BoolIndicatorSharedPtr findBoolIndicator(Stats::Store& store, + const std::string& name); + /** * Convert a string list of IP addresses into a list of network addresses usable for DNS * response testing.