diff --git a/api/envoy/api/v2/cluster/circuit_breaker.proto b/api/envoy/api/v2/cluster/circuit_breaker.proto index 50b20c08e5e7e..ebee99dae163d 100644 --- a/api/envoy/api/v2/cluster/circuit_breaker.proto +++ b/api/envoy/api/v2/cluster/circuit_breaker.proto @@ -46,6 +46,11 @@ message CircuitBreakers { // The maximum number of parallel retries that Envoy will allow to the // upstream cluster. If not specified, the default is 3. google.protobuf.UInt32Value max_retries = 5; + + // If track_remaining is true, then stats will be published that expose + // the number of resources remaining until the circuit breakers open. If + // not specified, the default is false. + bool track_remaining = 6; } // If multiple :ref:`Thresholds` diff --git a/docs/root/configuration/cluster_manager/cluster_stats.rst b/docs/root/configuration/cluster_manager/cluster_stats.rst index 5135080e356e2..47c2a011c3e66 100644 --- a/docs/root/configuration/cluster_manager/cluster_stats.rst +++ b/docs/root/configuration/cluster_manager/cluster_stats.rst @@ -152,6 +152,10 @@ Circuit breakers statistics will be rooted at *cluster..circuit_breakers.< 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) + remaining_cx, Gauge, Number of remaining connections until the circuit breaker opens + remaining_pending, Gauge, Number of remaining pending requests until the circuit breaker opens + remaining_rq, Gauge, Number of remaining requests until the circuit breaker opens + remaining_retries, Gauge, Number of remaining retries until the circuit breaker opens .. _config_cluster_manager_cluster_stats_dynamic_http: diff --git a/docs/root/intro/arch_overview/circuit_breaking.rst b/docs/root/intro/arch_overview/circuit_breaking.rst index a92bd622ee573..b2b6e31aa8c30 100644 --- a/docs/root/intro/arch_overview/circuit_breaking.rst +++ b/docs/root/intro/arch_overview/circuit_breaking.rst @@ -37,7 +37,8 @@ configure and code each application independently. Envoy supports various types Each circuit breaking limit is :ref:`configurable ` and tracked on a per upstream cluster and per priority basis. This allows different components of the distributed system to be tuned independently and have different limits. The live state of these -circuit breakers can be observed via :ref:`statistics `. +circuit breakers, including the number of resources remaining until a circuit breaker opens, can +be observed via :ref:`statistics `. Note that circuit breaking will cause the :ref:`x-envoy-overloaded ` header to be set by the router filter in the diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 7e6f93544c682..07dba365b7e3a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -56,6 +56,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 gauges tracking remaining resources before circuit breakers open. * 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/stats/scope.h b/include/envoy/stats/scope.h index ef913edea6e02..8785fae15e313 100644 --- a/include/envoy/stats/scope.h +++ b/include/envoy/stats/scope.h @@ -16,6 +16,7 @@ class Gauge; class Histogram; class Scope; class StatsOptions; +class NullGaugeImpl; typedef std::unique_ptr ScopePtr; typedef std::shared_ptr ScopeSharedPtr; @@ -51,6 +52,11 @@ class Scope { */ virtual Gauge& gauge(const std::string& name) PURE; + /** + * @return a null gauge within the scope's namespace. + */ + virtual NullGaugeImpl& nullGauge(const std::string& name) PURE; + /** * @return a histogram within the scope's namespace with a particular value type. */ diff --git a/include/envoy/stats/stats_macros.h b/include/envoy/stats/stats_macros.h index eb1c89557c664..66a39eac171fb 100644 --- a/include/envoy/stats/stats_macros.h +++ b/include/envoy/stats/stats_macros.h @@ -40,4 +40,8 @@ namespace Envoy { #define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "") #define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "") #define POOL_HISTOGRAM(POOL) POOL_HISTOGRAM_PREFIX(POOL, "") + +#define NULL_STAT_DECL_(X) std::string(#X)), + +#define NULL_POOL_GAUGE(POOL) (POOL).nullGauge(NULL_STAT_DECL_ } // namespace Envoy diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 79a54472fdd7d..c2ebd6026698c 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -521,14 +521,19 @@ class PrioritySet { // clang-format on /** - * Cluster circuit breakers stats. + * Cluster circuit breakers stats. Open circuit breaker stats and remaining resource stats + * can be handled differently by passing in different macros. */ // 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(OPEN_GAUGE, REMAINING_GAUGE) \ + OPEN_GAUGE (cx_open) \ + OPEN_GAUGE (rq_pending_open) \ + OPEN_GAUGE (rq_open) \ + OPEN_GAUGE (rq_retry_open) \ + REMAINING_GAUGE (remaining_cx) \ + REMAINING_GAUGE (remaining_pending) \ + REMAINING_GAUGE (remaining_rq) \ + REMAINING_GAUGE (remaining_retries) // clang-format on /** @@ -549,7 +554,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_GAUGE_STRUCT, GENERATE_GAUGE_STRUCT) }; /** diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc index d85d38ef03213..79769de1d3b65 100644 --- a/source/common/stats/isolated_store_impl.cc +++ b/source/common/stats/isolated_store_impl.cc @@ -38,12 +38,14 @@ 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); } + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } Histogram& histogram(const std::string& name) override { return parent_.histogram(prefix_ + name); } const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); } IsolatedStoreImpl& parent_; + NullGaugeImpl null_gauge_; const std::string prefix_; }; diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index 7765fd50e3e72..593bf6024713a 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -61,6 +61,7 @@ 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); } + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } Histogram& histogram(const std::string& name) override { Histogram& histogram = histograms_.get(name); return histogram; @@ -80,6 +81,7 @@ class IsolatedStoreImpl : public Store { IsolatedStatsCache gauges_; IsolatedStatsCache histograms_; const StatsOptionsImpl stats_options_; + NullGaugeImpl null_gauge_; }; } // namespace Stats diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 4f85c763567c2..dcba73676bb87 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -145,6 +145,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo return default_scope_->deliverHistogramToSinks(histogram, value); } Gauge& gauge(const std::string& name) override { return default_scope_->gauge(name); } + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } Histogram& histogram(const std::string& name) override { return default_scope_->histogram(name); }; @@ -199,6 +200,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo } void deliverHistogramToSinks(const Histogram& histogram, uint64_t value) override; Gauge& gauge(const std::string& name) override; + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } 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(); } @@ -272,6 +274,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo Counter& num_last_resort_stats_; HeapStatDataAllocator heap_allocator_; SourceImpl source_; + NullGaugeImpl null_gauge_; // Retain storage for deleted stats; these are no longer in maps because the // matcher-pattern was established after they were created. Since the stats diff --git a/source/common/upstream/resource_manager_impl.h b/source/common/upstream/resource_manager_impl.h index 35cf4bf87b7ff..4ee575a639042 100644 --- a/source/common/upstream/resource_manager_impl.h +++ b/source/common/upstream/resource_manager_impl.h @@ -29,11 +29,14 @@ class ResourceManagerImpl : public ResourceManager { uint64_t max_connections, uint64_t max_pending_requests, uint64_t max_requests, uint64_t max_retries, ClusterCircuitBreakersStats cb_stats) - : connections_(max_connections, runtime, runtime_key + "max_connections", cb_stats.cx_open_), + : connections_(max_connections, runtime, runtime_key + "max_connections", cb_stats.cx_open_, + cb_stats.remaining_cx_), pending_requests_(max_pending_requests, runtime, runtime_key + "max_pending_requests", - cb_stats.rq_pending_open_), - requests_(max_requests, runtime, runtime_key + "max_requests", cb_stats.rq_open_), - retries_(max_retries, runtime, runtime_key + "max_retries", cb_stats.rq_retry_open_) {} + cb_stats.rq_pending_open_, cb_stats.remaining_pending_), + requests_(max_requests, runtime, runtime_key + "max_requests", cb_stats.rq_open_, + cb_stats.remaining_rq_), + retries_(max_retries, runtime, runtime_key + "max_retries", cb_stats.rq_retry_open_, + cb_stats.remaining_retries_) {} // Upstream::ResourceManager Resource& connections() override { return connections_; } @@ -44,23 +47,42 @@ 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::Gauge& open_gauge, Stats::Gauge& remaining) + : max_(max), runtime_(runtime), runtime_key_(runtime_key), open_gauge_(open_gauge), + remaining_(remaining) { + remaining_.set(max); + } ~ResourceImpl() { ASSERT(current_ == 0); } // Upstream::Resource bool canCreate() override { return current_ < max(); } void inc() override { current_++; + updateRemaining(); open_gauge_.set(canCreate() ? 0 : 1); } void dec() override { ASSERT(current_ > 0); current_--; + updateRemaining(); open_gauge_.set(canCreate() ? 0 : 1); } uint64_t max() override { return runtime_.snapshot().getInteger(runtime_key_, max_); } + /** + * We set the gauge instead of incrementing and decrementing because, + * though atomics are used, it is possible for the current resource count + * to be greater than the supplied max. + */ + void updateRemaining() { + /** + * We cannot use std::max here because max() and current_ are + * unsigned and subtracting them may overflow. + */ + const uint64_t current_copy = current_; + remaining_.set(max() > current_copy ? max() - current_copy : 0); + } + const uint64_t max_; std::atomic current_{}; Runtime::Loader& runtime_; @@ -72,6 +94,11 @@ class ResourceManagerImpl : public ResourceManager { * is open. */ Stats::Gauge& open_gauge_; + + /** + * The number of resources remaining before the circuit breaker opens. + */ + Stats::Gauge& remaining_; }; ResourceImpl connections_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index e959961e139b9..ef6c27bbf27a5 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -816,9 +816,16 @@ ClusterInfoImpl::ResourceManagers::ResourceManagers(const envoy::api::v2::Cluste } ClusterCircuitBreakersStats -ClusterInfoImpl::generateCircuitBreakersStats(Stats::Scope& scope, const std::string& stat_prefix) { +ClusterInfoImpl::generateCircuitBreakersStats(Stats::Scope& scope, const std::string& stat_prefix, + bool track_remaining) { std::string prefix(fmt::format("circuit_breakers.{}.", stat_prefix)); - return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE_PREFIX(scope, prefix))}; + if (track_remaining) { + return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE_PREFIX(scope, prefix), + POOL_GAUGE_PREFIX(scope, prefix))}; + } else { + return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE_PREFIX(scope, prefix), + NULL_POOL_GAUGE(scope))}; + } } ResourceManagerImplPtr @@ -831,6 +838,8 @@ ClusterInfoImpl::ResourceManagers::load(const envoy::api::v2::Cluster& config, uint64_t max_requests = 1024; uint64_t max_retries = 3; + bool track_remaining = false; + std::string priority_name; switch (priority) { case envoy::api::v2::core::RoutingPriority::DEFAULT: @@ -858,10 +867,11 @@ ClusterInfoImpl::ResourceManagers::load(const envoy::api::v2::Cluster& config, PROTOBUF_GET_WRAPPED_OR_DEFAULT(*it, max_pending_requests, max_pending_requests); max_requests = PROTOBUF_GET_WRAPPED_OR_DEFAULT(*it, max_requests, max_requests); max_retries = PROTOBUF_GET_WRAPPED_OR_DEFAULT(*it, max_retries, max_retries); + track_remaining = it->track_remaining(); } return std::make_unique( runtime, runtime_prefix, max_connections, max_pending_requests, max_requests, max_retries, - ClusterInfoImpl::generateCircuitBreakersStats(stats_scope, priority_name)); + ClusterInfoImpl::generateCircuitBreakersStats(stats_scope, priority_name, track_remaining)); } PriorityStateManager::PriorityStateManager(ClusterImplBase& cluster, diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 417d3d8a7cd96..10c292fae08cd 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -490,7 +490,8 @@ class ClusterInfoImpl : public ClusterInfo { static ClusterStats generateStats(Stats::Scope& scope); static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope); static ClusterCircuitBreakersStats generateCircuitBreakersStats(Stats::Scope& scope, - const std::string& stat_prefix); + const std::string& stat_prefix, + bool track_remaining); // Upstream::ClusterInfo bool addedViaApi() const override { return added_via_api_; } diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 4b29fb71b1fb5..3d5ba3d40c73b 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -128,10 +128,7 @@ class Http1ConnPoolImplTest : public testing::Test { conn_pool_(dispatcher_, cluster_, upstream_ready_timer_) {} ~Http1ConnPoolImplTest() { - // Make sure all gauges are 0. - for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) { - EXPECT_EQ(0U, gauge->value()); - } + EXPECT_TRUE(TestUtility::gaugesZeroed(cluster_->stats_store_.gauges())); } NiceMock dispatcher_; diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 75327b04e9684..554b873fdea8f 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -68,10 +68,7 @@ class Http2ConnPoolImplTest : public testing::Test { pool_(dispatcher_, host_, Upstream::ResourcePriority::Default, nullptr) {} ~Http2ConnPoolImplTest() { - // Make sure all gauges are 0. - for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) { - EXPECT_EQ(0U, gauge->value()); - } + EXPECT_TRUE(TestUtility::gaugesZeroed(cluster_->stats_store_.gauges())); } // Creates a new test client, expecting a new connection to be created and associated diff --git a/test/common/tcp/conn_pool_test.cc b/test/common/tcp/conn_pool_test.cc index b3f26916eabad..034e4098dbaca 100644 --- a/test/common/tcp/conn_pool_test.cc +++ b/test/common/tcp/conn_pool_test.cc @@ -159,10 +159,7 @@ class TcpConnPoolImplTest : public testing::Test { conn_pool_(dispatcher_, cluster_, upstream_ready_timer_) {} ~TcpConnPoolImplTest() { - // Make sure all gauges are 0. - for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) { - EXPECT_EQ(0U, gauge->value()); - } + EXPECT_TRUE(TestUtility::gaugesZeroed(cluster_->stats_store_.gauges())); } NiceMock dispatcher_; diff --git a/test/common/upstream/resource_manager_impl_test.cc b/test/common/upstream/resource_manager_impl_test.cc index ae1c7d356d628..0e9c6e1eb3bc2 100644 --- a/test/common/upstream/resource_manager_impl_test.cc +++ b/test/common/upstream/resource_manager_impl_test.cc @@ -26,7 +26,8 @@ TEST(ResourceManagerImplTest, RuntimeResourceManager) { 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_GAUGE(store), POOL_GAUGE(store))}); EXPECT_CALL( runtime.snapshot_, @@ -59,6 +60,52 @@ TEST(ResourceManagerImplTest, RuntimeResourceManager) { EXPECT_FALSE(resource_manager.retries().canCreate()); } +TEST(ResourceManagerImplTest, RemainingResourceGauges) { + NiceMock runtime; + Stats::IsolatedStoreImpl store; + + auto stats = ClusterCircuitBreakersStats{ + ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_GAUGE(store), POOL_GAUGE(store))}; + ResourceManagerImpl resource_manager( + runtime, "circuit_breakers.runtime_resource_manager_test.default.", 1, 2, 1, 0, stats); + + // Test remaining_cx_ gauge + EXPECT_EQ(1U, resource_manager.connections().max()); + EXPECT_EQ(1U, stats.remaining_cx_.value()); + resource_manager.connections().inc(); + EXPECT_EQ(0U, stats.remaining_cx_.value()); + resource_manager.connections().dec(); + EXPECT_EQ(1U, stats.remaining_cx_.value()); + + // Test remaining_pending_ gauge + EXPECT_EQ(2U, resource_manager.pendingRequests().max()); + EXPECT_EQ(2U, stats.remaining_pending_.value()); + resource_manager.pendingRequests().inc(); + EXPECT_EQ(1U, stats.remaining_pending_.value()); + resource_manager.pendingRequests().inc(); + EXPECT_EQ(0U, stats.remaining_pending_.value()); + resource_manager.pendingRequests().dec(); + EXPECT_EQ(1U, stats.remaining_pending_.value()); + resource_manager.pendingRequests().dec(); + EXPECT_EQ(2U, stats.remaining_pending_.value()); + + // Test remaining_rq_ gauge + EXPECT_EQ(1U, resource_manager.requests().max()); + EXPECT_EQ(1U, stats.remaining_rq_.value()); + resource_manager.requests().inc(); + EXPECT_EQ(0U, stats.remaining_rq_.value()); + resource_manager.requests().dec(); + EXPECT_EQ(1U, stats.remaining_rq_.value()); + + // Test remaining_retries_ gauge. Confirm that the value will not be negative + // despite having more retries than the configured max + EXPECT_EQ(0U, resource_manager.retries().max()); + EXPECT_EQ(0U, stats.remaining_retries_.value()); + resource_manager.retries().inc(); + EXPECT_EQ(0U, stats.remaining_retries_.value()); + resource_manager.retries().dec(); + EXPECT_EQ(0U, stats.remaining_retries_.value()); +} } // namespace } // namespace Upstream } // namespace Envoy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 9d9fa09b6ebae..b54f3d8e08d67 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1873,6 +1873,47 @@ TEST_F(ClusterInfoImplTest, OneofExtensionProtocolOptionsForUnknownFilter) { "Only one of typed_extension_protocol_options or " "extension_protocol_options can be specified"); } +TEST_F(ClusterInfoImplTest, TestTrackRemainingResourcesGauges) { + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: ROUND_ROBIN + + circuit_breakers: + thresholds: + - priority: DEFAULT + max_connections: 1 + max_pending_requests: 2 + max_requests: 3 + max_retries: 4 + track_remaining: false + - priority: HIGH + max_connections: 1 + max_pending_requests: 2 + max_requests: 3 + max_retries: 4 + track_remaining: true + )EOF"; + + auto cluster = makeCluster(yaml); + + // The value of a remaining resource gauge will always be 0 for the default + // priority circuit breaker since track_remaining is false + EXPECT_EQ(0U, stats_.gauge("cluster.name.circuit_breakers.default.remaining_retries").value()); + cluster->info()->resourceManager(ResourcePriority::Default).retries().inc(); + EXPECT_EQ(0U, stats_.gauge("cluster.name.circuit_breakers.default.remaining_retries").value()); + cluster->info()->resourceManager(ResourcePriority::Default).retries().dec(); + EXPECT_EQ(0U, stats_.gauge("cluster.name.circuit_breakers.default.remaining_retries").value()); + + // This gauge will be correctly set since we have opted in to tracking remaining + // resource gauges in the high priority circuit breaker. + EXPECT_EQ(4U, stats_.gauge("cluster.name.circuit_breakers.high.remaining_retries").value()); + cluster->info()->resourceManager(ResourcePriority::High).retries().inc(); + EXPECT_EQ(3U, stats_.gauge("cluster.name.circuit_breakers.high.remaining_retries").value()); + cluster->info()->resourceManager(ResourcePriority::High).retries().dec(); + EXPECT_EQ(4U, stats_.gauge("cluster.name.circuit_breakers.high.remaining_retries").value()); +} class TestFilterConfigFactoryBase { public: diff --git a/test/extensions/filters/network/common/redis/client_impl_test.cc b/test/extensions/filters/network/common/redis/client_impl_test.cc index 72a0952f45fca..6bd29673b350d 100644 --- a/test/extensions/filters/network/common/redis/client_impl_test.cc +++ b/test/extensions/filters/network/common/redis/client_impl_test.cc @@ -46,13 +46,8 @@ class RedisClientImplTest : public testing::Test, public Common::Redis::DecoderF ~RedisClientImplTest() { client_.reset(); - // Make sure all gauges are 0. - for (const Stats::GaugeSharedPtr& gauge : host_->cluster_.stats_store_.gauges()) { - EXPECT_EQ(0U, gauge->value()); - } - for (const Stats::GaugeSharedPtr& gauge : host_->stats_store_.gauges()) { - EXPECT_EQ(0U, gauge->value()); - } + EXPECT_TRUE(TestUtility::gaugesZeroed(host_->cluster_.stats_store_.gauges())); + EXPECT_TRUE(TestUtility::gaugesZeroed(host_->stats_store_.gauges())); } void setup() { diff --git a/test/integration/server.h b/test/integration/server.h index 3652119081244..2f584d4597e0b 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -85,6 +85,8 @@ class TestScopeWrapper : public Scope { return wrapped_scope_->gauge(name); } + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } + Histogram& histogram(const std::string& name) override { Thread::LockGuard lock(lock_); return wrapped_scope_->histogram(name); @@ -96,6 +98,7 @@ class TestScopeWrapper : public Scope { Thread::MutexBasicLockable& lock_; ScopePtr wrapped_scope_; StatsOptionsImpl stats_options_; + NullGaugeImpl null_gauge_; }; /** @@ -119,6 +122,7 @@ class TestIsolatedStoreImpl : public StoreRoot { Thread::LockGuard lock(lock_); return store_.gauge(name); } + NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } Histogram& histogram(const std::string& name) override { Thread::LockGuard lock(lock_); return store_.histogram(name); @@ -154,6 +158,7 @@ class TestIsolatedStoreImpl : public StoreRoot { IsolatedStoreImpl store_; SourceImpl source_; StatsOptionsImpl stats_options_; + NullGaugeImpl null_gauge_; }; } // namespace Stats diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index c079e785be16b..c99d93d929add 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -158,6 +158,7 @@ class MockStore : public Store { MOCK_CONST_METHOD0(counters, std::vector()); MOCK_METHOD1(createScope_, Scope*(const std::string& name)); MOCK_METHOD1(gauge, Gauge&(const std::string&)); + MOCK_METHOD1(nullGauge, NullGaugeImpl&(const std::string&)); MOCK_CONST_METHOD0(gauges, std::vector()); MOCK_METHOD1(histogram, Histogram&(const std::string& name)); MOCK_CONST_METHOD0(histograms, std::vector()); diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 72e9ddfbe8a3c..a0ef49d23331b 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -33,7 +33,7 @@ MockClusterInfo::MockClusterInfo() transport_socket_factory_(new Network::RawBufferSocketFactory), load_report_stats_(ClusterInfoImpl::generateLoadReportStats(load_report_stats_store_)), circuit_breakers_stats_( - ClusterInfoImpl::generateCircuitBreakersStats(stats_store_, "default")), + ClusterInfoImpl::generateCircuitBreakersStats(stats_store_, "default", true)), resource_manager_(new Upstream::ResourceManagerImpl(runtime_, "fake_key", 1, 1024, 1024, 1, circuit_breakers_stats_)) { ON_CALL(*this, connectTimeout()).WillByDefault(Return(std::chrono::milliseconds(1))); diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 263971a611c83..d1a37ac281f87 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -260,6 +260,19 @@ std::string TestUtility::convertTime(const std::string& input, const std::string return TestUtility::formatTime(TestUtility::parseTime(input, input_format), output_format); } +// static +bool TestUtility::gaugesZeroed(const std::vector gauges) { + // Returns true if all gauges are 0 except the circuit_breaker remaining resource + // gauges which default to the resource max. + std::regex omitted(".*circuit_breakers\\..*\\.remaining.*"); + for (const Stats::GaugeSharedPtr& gauge : gauges) { + if (!std::regex_match(gauge->name(), omitted) && gauge->value() != 0) { + return false; + } + } + return true; +} + void ConditionalInitializer::setReady() { Thread::LockGuard lock(mutex_); EXPECT_FALSE(ready_); diff --git a/test/test_common/utility.h b/test/test_common/utility.h index a91e68d74176b..89fb3b09d2d16 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -350,6 +350,16 @@ class TestUtility { matcher.set_regex(str); return matcher; } + + /** + * Checks that passed gauges have a value of 0. Gauges can be omitted from + * this check by modifying the regex that matches gauge names in the + * implementation. + * + * @param vector of gauges to check. + * @return bool indicating that passed gauges not matching the omitted regex have a value of 0. + */ + static bool gaugesZeroed(const std::vector gauges); }; /**