Revert "stats: add new BoolIndicator stat type (#5813)"

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, BoolIndicator, Current connection state with management server
+  control_plane.connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the current connection state with management server

docs/root/configuration/cluster_manager/cluster_stats.rst

@@ -148,10 +148,10 @@ Circuit breakers statistics will be rooted at *cluster.<name>.circuit_breakers.<
   :header: Name, Type, Description
   :widths: 1, 1, 2
 
-  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)
+  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)
 
 .. _config_cluster_manager_cluster_stats_dynamic_http:
 

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, BoolIndicator, Current connection state with management server
+  control_plane.connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the current connection state with management server

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, BoolIndicator, Current connection state with management server
+   connected_state, Gauge, A boolean (1 for connected and 0 for disconnected) that indicates the 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
 

docs/root/intro/version_history.rst

@@ -53,7 +53,6 @@ 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 <config_http_filters_tap>`.
 * 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<envoy_api_msg_Cluster.RingHashLbConfig>` as either xxHash or `murmurHash2 <https://sites.google.com/site/murmurhash>`_. 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.

include/envoy/config/grpc_mux.h

@@ -13,17 +13,17 @@ namespace Config {
  * All control plane related stats. @see stats_macros.h
  */
 // clang-format off
-#define ALL_CONTROL_PLANE_STATS(BOOL_INDICATOR, COUNTER, GAUGE)                                    \
+#define ALL_CONTROL_PLANE_STATS(COUNTER, GAUGE)                                                    \
   COUNTER(rate_limit_enforced)                                                                     \
-  BOOL_INDICATOR(connected_state)                                                                  \
+  GAUGE(connected_state)                                                                           \
   GAUGE(pending_requests)                                                                          \
 // clang-format on
 
 /**
  * Struct definition for all control plane stats. @see stats_macros.h
  */
 struct ControlPlaneStats {
-  ALL_CONTROL_PLANE_STATS(GENERATE_BOOL_INDICATOR_STRUCT,GENERATE_COUNTER_STRUCT,GENERATE_GAUGE_STRUCT)
+  ALL_CONTROL_PLANE_STATS(GENERATE_COUNTER_STRUCT,GENERATE_GAUGE_STRUCT)
 };
 
 class GrpcMuxCallbacks {

include/envoy/stats/scope.h

@@ -51,11 +51,6 @@ 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.
    */

include/envoy/stats/source.h

@@ -37,14 +37,6 @@ class Source {
    */
   virtual const std::vector<GaugeSharedPtr>& cachedGauges() PURE;
 
-  /**
-   * Returns all known bools. Will use cached values if already accessed and clearCache() hasn't
-   * been called since.
-   * @return std::vector<BoolIndicatorSharedPtr>& all known bools. Note: reference may not be
-   * valid after clearCache() is called.
-   */
-  virtual const std::vector<BoolIndicatorSharedPtr>& cachedBoolIndicators() PURE;
-
   /**
    * Returns all known parent histograms. Will use cached values if already accessed and
    * clearCache() hasn't been called since.

include/envoy/stats/stat_data_allocator.h

@@ -47,17 +47,6 @@ class StatDataAllocator {
   virtual GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name,
                                    std::vector<Tag>&& 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<Tag>&& tags) PURE;
-
   /**
    * Determines whether this stats allocator requires bounded stat-name size.
    */

include/envoy/stats/stats.h

@@ -94,18 +94,5 @@ class Gauge : public virtual Metric {
 
 typedef std::shared_ptr<Gauge> 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<BoolIndicator> BoolIndicatorSharedPtr;
-
 } // namespace Stats
 } // namespace Envoy

include/envoy/stats/stats_macros.h

@@ -29,18 +29,15 @@ 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

include/envoy/stats/store.h

@@ -39,11 +39,6 @@ class Store : public Scope {
    */
   virtual std::vector<GaugeSharedPtr> gauges() const PURE;
 
-  /**
-   * @return a list of all known bools.
-   */
-  virtual std::vector<BoolIndicatorSharedPtr> boolIndicators() const PURE;
-
   /**
    * @return a list of all known histograms.
    */

include/envoy/upstream/upstream.h

@@ -524,11 +524,11 @@ class PrioritySet {
  * Cluster circuit breakers stats.
  */
 // clang-format off
-#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)
+#define ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GAUGE)                                                  \
+  GAUGE (cx_open)                                                                                  \
+  GAUGE (rq_pending_open)                                                                          \
+  GAUGE (rq_open)                                                                                  \
+  GAUGE (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_BOOL_INDICATOR_STRUCT)
+  ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_GAUGE_STRUCT)
 };
 
 /**

source/common/config/grpc_stream.h

@@ -66,7 +66,7 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks<ResponseProto>,
       setRetryTimer();
       return;
     }
-    control_plane_stats_.connected_state_.set(true);
+    control_plane_stats_.connected_state_.set(1);
     handleStreamEstablished();
   }
 
@@ -100,7 +100,7 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks<ResponseProto>,
   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(false);
+    control_plane_stats_.connected_state_.set(0);
     handleEstablishmentFailure();
     setRetryTimer();
   }
@@ -142,8 +142,7 @@ class GrpcStream : public Grpc::TypedAsyncStreamCallbacks<ResponseProto>,
 
   ControlPlaneStats generateControlPlaneStats(Stats::Scope& scope) {
     const std::string control_plane_prefix = "control_plane.";
-    return {ALL_CONTROL_PLANE_STATS(POOL_BOOL_INDICATOR_PREFIX(scope, control_plane_prefix),
-                                    POOL_COUNTER_PREFIX(scope, control_plane_prefix),
+    return {ALL_CONTROL_PLANE_STATS(POOL_COUNTER_PREFIX(scope, control_plane_prefix),
                                     POOL_GAUGE_PREFIX(scope, control_plane_prefix))};
   }
 

source/common/runtime/runtime_impl.cc

@@ -319,7 +319,7 @@ void AdminLayer::mergeValues(const std::unordered_map<std::string, std::string>&
       values_.emplace(kv.first, SnapshotImpl::createEntry(kv.second));
     }
   }
-  stats_.admin_overrides_active_.set(!values_.empty());
+  stats_.admin_overrides_active_.set(values_.empty() ? 0 : 1);
 }
 
 DiskLayer::DiskLayer(const std::string& name, const std::string& path, Api::Api& api)
@@ -429,9 +429,8 @@ DiskBackedLoaderImpl::DiskBackedLoaderImpl(Event::Dispatcher& dispatcher,
 
 RuntimeStats LoaderImpl::generateStats(Stats::Store& store) {
   std::string prefix = "runtime.";
-  RuntimeStats stats{ALL_RUNTIME_STATS(POOL_BOOL_INDICATOR_PREFIX(store, prefix),
-                                       POOL_COUNTER_PREFIX(store, prefix),
-                                       POOL_GAUGE_PREFIX(store, prefix))};
+  RuntimeStats stats{
+      ALL_RUNTIME_STATS(POOL_COUNTER_PREFIX(store, prefix), POOL_GAUGE_PREFIX(store, prefix))};
   return stats;
 }
 

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(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)
+#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)
 // clang-format on
 
 /**
  * Struct definition for all runtime stats. @see stats_macros.h
  */
 struct RuntimeStats {
-  ALL_RUNTIME_STATS(GENERATE_BOOL_INDICATOR_STRUCT, GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT)
+  ALL_RUNTIME_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT)
 };
 
 /**

source/common/stats/isolated_store_impl.cc

@@ -23,11 +23,6 @@ IsolatedStoreImpl::IsolatedStoreImpl()
         std::vector<Tag> 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<Tag> tags;
-        return alloc_.makeBoolIndicator(name, std::move(tag_extracted_name), std::move(tags));
-      }),
       histograms_([this](const std::string& name) -> HistogramSharedPtr {
         return std::make_shared<HistogramImpl>(name, *this, std::string(name), std::vector<Tag>());
       }) {}
@@ -43,9 +38,6 @@ 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);
   }

source/common/stats/isolated_store_impl.h

@@ -61,9 +61,6 @@ 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;
@@ -73,9 +70,6 @@ class IsolatedStoreImpl : public Store {
   // Stats::Store
   std::vector<CounterSharedPtr> counters() const override { return counters_.toVector(); }
   std::vector<GaugeSharedPtr> gauges() const override { return gauges_.toVector(); }
-  std::vector<BoolIndicatorSharedPtr> boolIndicators() const override {
-    return bool_indicators_.toVector();
-  }
   std::vector<ParentHistogramSharedPtr> histograms() const override {
     return std::vector<ParentHistogramSharedPtr>{};
   }
@@ -84,7 +78,6 @@ class IsolatedStoreImpl : public Store {
   HeapStatDataAllocator alloc_;
   IsolatedStatsCache<Counter> counters_;
   IsolatedStatsCache<Gauge> gauges_;
-  IsolatedStatsCache<BoolIndicator> bool_indicators_;
   IsolatedStatsCache<Histogram> histograms_;
   const StatsOptionsImpl stats_options_;
 };

source/common/stats/source_impl.cc

@@ -17,12 +17,6 @@ std::vector<GaugeSharedPtr>& SourceImpl::cachedGauges() {
   }
   return *gauges_;
 }
-std::vector<BoolIndicatorSharedPtr>& SourceImpl::cachedBoolIndicators() {
-  if (!bool_indicators_) {
-    bool_indicators_ = store_.boolIndicators();
-  }
-  return *bool_indicators_;
-}
 std::vector<ParentHistogramSharedPtr>& SourceImpl::cachedHistograms() {
   if (!histograms_) {
     histograms_ = store_.histograms();
@@ -33,7 +27,6 @@ std::vector<ParentHistogramSharedPtr>& SourceImpl::cachedHistograms() {
 void SourceImpl::clearCache() {
   counters_.reset();
   gauges_.reset();
-  bool_indicators_.reset();
   histograms_.reset();
 }
 

source/common/stats/source_impl.h

@@ -16,15 +16,13 @@ class SourceImpl : public Source {
   // Stats::Source
   std::vector<CounterSharedPtr>& cachedCounters() override;
   std::vector<GaugeSharedPtr>& cachedGauges() override;
-  std::vector<BoolIndicatorSharedPtr>& cachedBoolIndicators() override;
   std::vector<ParentHistogramSharedPtr>& cachedHistograms() override;
   void clearCache() override;
 
 private:
   Store& store_;
   absl::optional<std::vector<CounterSharedPtr>> counters_;
   absl::optional<std::vector<GaugeSharedPtr>> gauges_;
-  absl::optional<std::vector<BoolIndicatorSharedPtr>> bool_indicators_;
   absl::optional<std::vector<ParentHistogramSharedPtr>> histograms_;
 };