stats: get most of the cluster stats into the new 2-phase macro system.

include/envoy/upstream/cluster_manager.h

@@ -345,6 +345,10 @@ class ClusterManager {
    * @return the stat names.
    */
   virtual const ClusterStatNames& clusterStatNames() const PURE;
+  virtual const ClusterLoadReportStatNames& clusterLoadReportStatNames() const PURE;
+  virtual const ClusterRequestResponseSizeStatNames&
+  clusterRequestResponseSizeStatNames() const PURE;
+  virtual const ClusterTimeoutBudgetStatNames& clusterTimeoutBudgetStatNames() const PURE;
 };
 
 using ClusterManagerPtr = std::unique_ptr<ClusterManager>;

include/envoy/upstream/upstream.h

@@ -606,7 +606,8 @@ class PrioritySet {
  * stats sink. See envoy.api.v2.endpoint.ClusterStats for the definition of upstream_rq_dropped.
  * These are latched by LoadStatsReporter, independent of the normal stats sink flushing.
  */
-#define ALL_CLUSTER_LOAD_REPORT_STATS(COUNTER) COUNTER(upstream_rq_dropped)
+#define ALL_CLUSTER_LOAD_REPORT_STATS(COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, STATNAME)           \
+  COUNTER(upstream_rq_dropped)
 
 /**
  * Cluster circuit breakers stats. Open circuit breaker stats and remaining resource stats
@@ -627,7 +628,7 @@ class PrioritySet {
 /**
  * All stats tracking request/response headers and body sizes. Not used by default.
  */
-#define ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS(HISTOGRAM)                                         \
+#define ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS(COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, STATNAME) \
   HISTOGRAM(upstream_rq_headers_size, Bytes)                                                       \
   HISTOGRAM(upstream_rq_body_size, Bytes)                                                          \
   HISTOGRAM(upstream_rs_headers_size, Bytes)                                                       \
@@ -636,7 +637,7 @@ class PrioritySet {
 /**
  * All stats around timeout budgets. Not used by default.
  */
-#define ALL_CLUSTER_TIMEOUT_BUDGET_STATS(HISTOGRAM)                                                \
+#define ALL_CLUSTER_TIMEOUT_BUDGET_STATS(COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, STATNAME)        \
   HISTOGRAM(upstream_rq_timeout_budget_percent_used, Unspecified)                                  \
   HISTOGRAM(upstream_rq_timeout_budget_per_try_percent_used, Unspecified)
 
@@ -646,12 +647,18 @@ class PrioritySet {
 MAKE_STAT_NAMES_STRUCT(ClusterStatNames, ALL_CLUSTER_STATS);
 MAKE_STATS_STRUCT(ClusterStats, ClusterStatNames, ALL_CLUSTER_STATS);
 
-/**
- * Struct definition for all cluster load report stats. @see stats_macros.h
- */
-struct ClusterLoadReportStats {
-  ALL_CLUSTER_LOAD_REPORT_STATS(GENERATE_COUNTER_STRUCT)
-};
+MAKE_STAT_NAMES_STRUCT(ClusterLoadReportStatNames, ALL_CLUSTER_LOAD_REPORT_STATS);
+MAKE_STATS_STRUCT(ClusterLoadReportStats, ClusterLoadReportStatNames,
+                  ALL_CLUSTER_LOAD_REPORT_STATS);
+
+MAKE_STAT_NAMES_STRUCT(ClusterRequestResponseSizeStatNames,
+                       ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS);
+MAKE_STATS_STRUCT(ClusterRequestResponseSizeStats, ClusterRequestResponseSizeStatNames,
+                  ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS);
+
+MAKE_STAT_NAMES_STRUCT(ClusterTimeoutBudgetStatNames, ALL_CLUSTER_TIMEOUT_BUDGET_STATS);
+MAKE_STATS_STRUCT(ClusterTimeoutBudgetStats, ClusterTimeoutBudgetStatNames,
+                  ALL_CLUSTER_TIMEOUT_BUDGET_STATS);
 
 /**
  * Struct definition for cluster circuit breakers stats. @see stats_macros.h
@@ -660,24 +667,10 @@ struct ClusterCircuitBreakersStats {
   ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_GAUGE_STRUCT, GENERATE_GAUGE_STRUCT)
 };
 
-/**
- * Struct definition for cluster timeout budget stats. @see stats_macros.h
- */
-struct ClusterRequestResponseSizeStats {
-  ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS(GENERATE_HISTOGRAM_STRUCT)
-};
-
 using ClusterRequestResponseSizeStatsPtr = std::unique_ptr<ClusterRequestResponseSizeStats>;
 using ClusterRequestResponseSizeStatsOptRef =
     absl::optional<std::reference_wrapper<ClusterRequestResponseSizeStats>>;
 
-/**
- * Struct definition for cluster timeout budget stats. @see stats_macros.h
- */
-struct ClusterTimeoutBudgetStats {
-  ALL_CLUSTER_TIMEOUT_BUDGET_STATS(GENERATE_HISTOGRAM_STRUCT)
-};
-
 using ClusterTimeoutBudgetStatsPtr = std::unique_ptr<ClusterTimeoutBudgetStats>;
 using ClusterTimeoutBudgetStatsOptRef =
     absl::optional<std::reference_wrapper<ClusterTimeoutBudgetStats>>;

source/common/upstream/cluster_manager_impl.cc

@@ -260,6 +260,9 @@ ClusterManagerImpl::ClusterManagerImpl(
       time_source_(main_thread_dispatcher.timeSource()), dispatcher_(main_thread_dispatcher),
       http_context_(http_context), router_context_(router_context),
       cluster_stat_names_(stats.symbolTable()),
+      cluster_load_report_stat_names_(stats.symbolTable()),
+      cluster_request_response_size_stat_names_(stats.symbolTable()),
+      cluster_timeout_budget_stat_names_(stats.symbolTable()),
       subscription_factory_(local_info, main_thread_dispatcher, *this,
                             validation_context.dynamicValidationVisitor(), api, runtime_) {
   async_client_manager_ = std::make_unique<Grpc::AsyncClientManagerImpl>(

source/common/upstream/cluster_manager_impl.h

@@ -299,6 +299,15 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   initializeSecondaryClusters(const envoy::config::bootstrap::v3::Bootstrap& bootstrap) override;
 
   const ClusterStatNames& clusterStatNames() const override { return cluster_stat_names_; }
+  const ClusterLoadReportStatNames& clusterLoadReportStatNames() const override {
+    return cluster_load_report_stat_names_;
+  }
+  const ClusterRequestResponseSizeStatNames& clusterRequestResponseSizeStatNames() const override {
+    return cluster_request_response_size_stat_names_;
+  }
+  const ClusterTimeoutBudgetStatNames& clusterTimeoutBudgetStatNames() const override {
+    return cluster_timeout_budget_stat_names_;
+  }
 
 protected:
   virtual void postThreadLocalDrainConnections(const Cluster& cluster,
@@ -587,6 +596,10 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   Http::Context& http_context_;
   Router::Context& router_context_;
   ClusterStatNames cluster_stat_names_;
+  ClusterLoadReportStatNames cluster_load_report_stat_names_;
+  ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_;
+  ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_;
+
   Config::SubscriptionFactoryImpl subscription_factory_;
   ClusterSet primary_clusters_;
 };

source/common/upstream/upstream_impl.cc

@@ -641,17 +641,21 @@ ClusterStats ClusterInfoImpl::generateStats(Stats::Scope& scope,
   return ClusterStats(stat_names, scope);
 }
 
-ClusterRequestResponseSizeStats
-ClusterInfoImpl::generateRequestResponseSizeStats(Stats::Scope& scope) {
-  return {ALL_CLUSTER_REQUEST_RESPONSE_SIZE_STATS(POOL_HISTOGRAM(scope))};
+ClusterRequestResponseSizeStats ClusterInfoImpl::generateRequestResponseSizeStats(
+    Stats::Scope& scope, const ClusterRequestResponseSizeStatNames& stat_names) {
+  return ClusterRequestResponseSizeStats(stat_names, scope);
 }
 
-ClusterLoadReportStats ClusterInfoImpl::generateLoadReportStats(Stats::Scope& scope) {
-  return {ALL_CLUSTER_LOAD_REPORT_STATS(POOL_COUNTER(scope))};
+ClusterLoadReportStats
+ClusterInfoImpl::generateLoadReportStats(Stats::Scope& scope,
+                                         const ClusterLoadReportStatNames& stat_names) {
+  return ClusterLoadReportStats(stat_names, scope);
 }
 
-ClusterTimeoutBudgetStats ClusterInfoImpl::generateTimeoutBudgetStats(Stats::Scope& scope) {
-  return {ALL_CLUSTER_TIMEOUT_BUDGET_STATS(POOL_HISTOGRAM(scope))};
+ClusterTimeoutBudgetStats
+ClusterInfoImpl::generateTimeoutBudgetStats(Stats::Scope& scope,
+                                            const ClusterTimeoutBudgetStatNames& stat_names) {
+  return ClusterTimeoutBudgetStats(stat_names, scope);
 }
 
 // Implements the FactoryContext interface required by network filters.
@@ -745,9 +749,11 @@ ClusterInfoImpl::ClusterInfoImpl(
       socket_matcher_(std::move(socket_matcher)), stats_scope_(std::move(stats_scope)),
       stats_(generateStats(*stats_scope_, factory_context.clusterManager().clusterStatNames())),
       load_report_stats_store_(stats_scope_->symbolTable()),
-      load_report_stats_(generateLoadReportStats(load_report_stats_store_)),
+      load_report_stats_(generateLoadReportStats(
+          load_report_stats_store_, factory_context.clusterManager().clusterLoadReportStatNames())),
       optional_cluster_stats_((config.has_track_cluster_stats() || config.track_timeout_budgets())
-                                  ? std::make_unique<OptionalClusterStats>(config, *stats_scope_)
+                                  ? std::make_unique<OptionalClusterStats>(
+                                        config, *stats_scope_, factory_context.clusterManager())
                                   : nullptr),
       features_(parseFeatures(config, http_protocol_options_)),
       resource_managers_(config, runtime, name_, *stats_scope_),
@@ -1139,15 +1145,18 @@ void ClusterImplBase::validateEndpointsForZoneAwareRouting(
 }
 
 ClusterInfoImpl::OptionalClusterStats::OptionalClusterStats(
-    const envoy::config::cluster::v3::Cluster& config, Stats::Scope& stats_scope)
+    const envoy::config::cluster::v3::Cluster& config, Stats::Scope& stats_scope,
+    const ClusterManager& manager)
     : timeout_budget_stats_(
           (config.track_cluster_stats().timeout_budgets() || config.track_timeout_budgets())
-              ? std::make_unique<ClusterTimeoutBudgetStats>(generateTimeoutBudgetStats(stats_scope))
+              ? std::make_unique<ClusterTimeoutBudgetStats>(generateTimeoutBudgetStats(
+                    stats_scope, manager.clusterTimeoutBudgetStatNames()))
               : nullptr),
-      request_response_size_stats_(config.track_cluster_stats().request_response_sizes()
-                                       ? std::make_unique<ClusterRequestResponseSizeStats>(
-                                             generateRequestResponseSizeStats(stats_scope))
-                                       : nullptr) {}
+      request_response_size_stats_(
+          (config.track_cluster_stats().request_response_sizes()
+               ? std::make_unique<ClusterRequestResponseSizeStats>(generateRequestResponseSizeStats(
+                     stats_scope, manager.clusterRequestResponseSizeStatNames()))
+               : nullptr)) {}
 
 ClusterInfoImpl::ResourceManagers::ResourceManagers(
     const envoy::config::cluster::v3::Cluster& config, Runtime::Loader& runtime,

source/common/upstream/upstream_impl.h

@@ -528,12 +528,16 @@ class ClusterInfoImpl : public ClusterInfo, protected Logger::Loggable<Logger::I
 
   static ClusterStats generateStats(Stats::Scope& scope,
                                     const ClusterStatNames& cluster_stat_names);
-  static ClusterLoadReportStats generateLoadReportStats(Stats::Scope& scope);
+  static ClusterLoadReportStats
+  generateLoadReportStats(Stats::Scope& scope, const ClusterLoadReportStatNames& stat_names);
   static ClusterCircuitBreakersStats generateCircuitBreakersStats(Stats::Scope& scope,
                                                                   const std::string& stat_prefix,
                                                                   bool track_remaining);
-  static ClusterRequestResponseSizeStats generateRequestResponseSizeStats(Stats::Scope&);
-  static ClusterTimeoutBudgetStats generateTimeoutBudgetStats(Stats::Scope&);
+  static ClusterRequestResponseSizeStats
+  generateRequestResponseSizeStats(Stats::Scope&,
+                                   const ClusterRequestResponseSizeStatNames& stat_names);
+  static ClusterTimeoutBudgetStats
+  generateTimeoutBudgetStats(Stats::Scope&, const ClusterTimeoutBudgetStatNames& stat_names);
 
   // Upstream::ClusterInfo
   bool addedViaApi() const override { return added_via_api_; }
@@ -662,7 +666,7 @@ class ClusterInfoImpl : public ClusterInfo, protected Logger::Loggable<Logger::I
 
   struct OptionalClusterStats {
     OptionalClusterStats(const envoy::config::cluster::v3::Cluster& config,
-                         Stats::Scope& stats_scope);
+                         Stats::Scope& stats_scope, const ClusterManager& manager);
     const ClusterTimeoutBudgetStatsPtr timeout_budget_stats_;
     const ClusterRequestResponseSizeStatsPtr request_response_size_stats_;
   };

test/mocks/upstream/cluster_info.cc

@@ -40,13 +40,19 @@ MockClusterInfo::MockClusterInfo()
     : http2_options_(::Envoy::Http2::Utility::initializeAndValidateOptions(
           envoy::config::core::v3::Http2ProtocolOptions())),
       stat_names_(stats_store_.symbolTable()),
+      cluster_load_report_stat_names_(stats_store_.symbolTable()),
+      cluster_request_response_size_stat_names_(stats_store_.symbolTable()),
+      cluster_timeout_budget_stat_names_(stats_store_.symbolTable()),
       stats_(ClusterInfoImpl::generateStats(stats_store_, stat_names_)),
       transport_socket_matcher_(new NiceMock<Upstream::MockTransportSocketMatcher>()),
-      load_report_stats_(ClusterInfoImpl::generateLoadReportStats(load_report_stats_store_)),
+      load_report_stats_(ClusterInfoImpl::generateLoadReportStats(load_report_stats_store_,
+                                                                  cluster_load_report_stat_names_)),
       request_response_size_stats_(std::make_unique<ClusterRequestResponseSizeStats>(
-          ClusterInfoImpl::generateRequestResponseSizeStats(request_response_size_stats_store_))),
-      timeout_budget_stats_(std::make_unique<ClusterTimeoutBudgetStats>(
-          ClusterInfoImpl::generateTimeoutBudgetStats(timeout_budget_stats_store_))),
+          ClusterInfoImpl::generateRequestResponseSizeStats(
+              request_response_size_stats_store_, cluster_request_response_size_stat_names_))),
+      timeout_budget_stats_(
+          std::make_unique<ClusterTimeoutBudgetStats>(ClusterInfoImpl::generateTimeoutBudgetStats(
+              timeout_budget_stats_store_, cluster_timeout_budget_stat_names_))),
       circuit_breakers_stats_(
           ClusterInfoImpl::generateCircuitBreakersStats(stats_store_, "default", true)),
       resource_manager_(new Upstream::ResourceManagerImpl(

test/mocks/upstream/cluster_info.h

@@ -157,6 +157,9 @@ class MockClusterInfo : public ClusterInfo {
   uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT};
   NiceMock<Stats::MockIsolatedStatsStore> stats_store_;
   ClusterStatNames stat_names_;
+  ClusterLoadReportStatNames cluster_load_report_stat_names_;
+  ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_;
+  ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_;
   ClusterStats stats_;
   Upstream::TransportSocketMatcherPtr transport_socket_matcher_;
   NiceMock<Stats::MockIsolatedStatsStore> load_report_stats_store_;

test/mocks/upstream/cluster_manager.cc

@@ -16,7 +16,10 @@ using ::testing::ReturnRef;
 
 MockClusterManager::MockClusterManager(TimeSource&) : MockClusterManager() {}
 
-MockClusterManager::MockClusterManager() : cluster_stat_names_(*symbol_table_) {
+MockClusterManager::MockClusterManager()
+    : cluster_stat_names_(*symbol_table_), cluster_load_report_stat_names_(*symbol_table_),
+      cluster_request_response_size_stat_names_(*symbol_table_),
+      cluster_timeout_budget_stat_names_(*symbol_table_) {
   ON_CALL(*this, httpConnPoolForCluster(_, _, _, _)).WillByDefault(Return(&conn_pool_));
   ON_CALL(*this, tcpConnPoolForCluster(_, _, _)).WillByDefault(Return(&tcp_conn_pool_));
   ON_CALL(*this, httpAsyncClientForCluster(_)).WillByDefault(ReturnRef(async_client_));

test/mocks/upstream/cluster_manager.h

@@ -71,6 +71,15 @@ class MockClusterManager : public ClusterManager {
               (ClusterUpdateCallbacks & callbacks));
   MOCK_METHOD(Config::SubscriptionFactory&, subscriptionFactory, ());
   const ClusterStatNames& clusterStatNames() const override { return cluster_stat_names_; }
+  const ClusterLoadReportStatNames& clusterLoadReportStatNames() const override {
+    return cluster_load_report_stat_names_;
+  }
+  const ClusterRequestResponseSizeStatNames& clusterRequestResponseSizeStatNames() const override {
+    return cluster_request_response_size_stat_names_;
+  }
+  const ClusterTimeoutBudgetStatNames& clusterTimeoutBudgetStatNames() const override {
+    return cluster_timeout_budget_stat_names_;
+  }
 
   NiceMock<Http::ConnectionPool::MockInstance> conn_pool_;
   NiceMock<Http::MockAsyncClient> async_client_;
@@ -86,6 +95,9 @@ class MockClusterManager : public ClusterManager {
   absl::flat_hash_map<std::string, std::unique_ptr<MockCluster>> warming_clusters_;
   Stats::TestSymbolTable symbol_table_;
   ClusterStatNames cluster_stat_names_;
+  ClusterLoadReportStatNames cluster_load_report_stat_names_;
+  ClusterRequestResponseSizeStatNames cluster_request_response_size_stat_names_;
+  ClusterTimeoutBudgetStatNames cluster_timeout_budget_stat_names_;
 };
 } // namespace Upstream