stats: Factor out creation of cluster-stats StatNames from creation o…

…f the stats, to save CPU during xDS updates (#14028)

Commit Message: Adds a new set of macros in stat_macros.h for separately declaring the names of stats, to hold in a factory, and then for creating the stats based on the symbolized names. This removes a symbol-table contention bottleneck for stats that are created by name during operation. Two such instances are resolved in this PR: Cluster stats and Route filter stats.
Additional Description: There were two infrastructural issues expanding the file-count in this PR, but hopefully not the conceptual complexity:

the addition of Router::Context and plumbing that through various factories and their mocks
the saving of StatNames in mock structures that are subsequently used elsewhere requires that a global symbol table be used. This was not difficult as there was already a mechanism for a test-only global symbol table, but it turned out to be easiest to move the declaration of the global symbol table structure from test/mocks/stats/mocks.h to test/common/stats/stat_test_utility.h. Doing this avoided switching from IsolatedStoreImpl to the mock version, which has some subtle functional changes.
Risk Level: low
Testing: //test/...
Docs Changes: will update stats.md as a follow-up
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>

include/envoy/http/context.h

@@ -29,6 +29,7 @@ class Context {
 
   virtual CodeStats& codeStats() PURE;
   virtual const UserAgentContext& userAgentContext() const PURE;
+  virtual const Stats::StatName& asyncClientStatPrefix() const PURE;
 };
 
 using ContextPtr = std::unique_ptr<Context>;

include/envoy/router/BUILD

@@ -8,6 +8,11 @@ licenses(["notice"])  # Apache 2
 
 envoy_package()
 
+envoy_cc_library(
+    name = "context_interface",
+    hdrs = ["context.h"],
+)
+
 envoy_cc_library(
     name = "rds_interface",
     hdrs = ["rds.h"],

include/envoy/router/context.h

@@ -0,0 +1,21 @@
+#pragma once
+
+#include "envoy/common/pure.h"
+
+namespace Envoy {
+namespace Router {
+
+struct StatNames;
+
+class Context {
+public:
+  virtual ~Context() = default;
+
+  /**
+   * @return a struct containing StatNames for router stats.
+   */
+  virtual const StatNames& statNames() const PURE;
+};
+
+} // namespace Router
+} // namespace Envoy

include/envoy/server/BUILD

@@ -182,6 +182,7 @@ envoy_cc_library(
         "//include/envoy/init:manager_interface",
         "//include/envoy/local_info:local_info_interface",
         "//include/envoy/network:drain_decision_interface",
+        "//include/envoy/router:context_interface",
         "//include/envoy/runtime:runtime_interface",
         "//include/envoy/server/overload:overload_manager_interface",
         "//include/envoy/singleton:manager_interface",

include/envoy/server/factory_context.h

@@ -15,6 +15,7 @@
 #include "envoy/init/manager.h"
 #include "envoy/network/drain_decision.h"
 #include "envoy/network/filter.h"
+#include "envoy/router/context.h"
 #include "envoy/runtime/runtime.h"
 #include "envoy/server/admin.h"
 #include "envoy/server/drain_manager.h"
@@ -115,6 +116,11 @@ class ServerFactoryContext : public virtual CommonFactoryContext {
    */
   virtual Grpc::Context& grpcContext() PURE;
 
+  /**
+   * @return Router::Context& a reference to the router context.
+   */
+  virtual Router::Context& routerContext() PURE;
+
   /**
    * @return DrainManager& the server-wide drain manager.
    */
@@ -223,6 +229,11 @@ class FactoryContext : public virtual CommonFactoryContext {
    */
   virtual Grpc::Context& grpcContext() PURE;
 
+  /**
+   * @return Router::Context& a reference to the router context.
+   */
+  virtual Router::Context& routerContext() PURE;
+
   /**
    * @return ProcessContextOptRef an optional reference to the
    * process context. Will be unset when running in validation mode.

include/envoy/server/instance.h

@@ -190,6 +190,11 @@ class Instance {
    */
   virtual Http::Context& httpContext() PURE;
 
+  /**
+   * @return the server-wide router context.
+   */
+  virtual Router::Context& routerContext() PURE;
+
   /**
    * @return the server-wide process context.
    */

include/envoy/stats/BUILD

@@ -66,7 +66,11 @@ envoy_cc_library(
 envoy_cc_library(
     name = "stats_macros",
     hdrs = ["stats_macros.h"],
-    deps = [":stats_interface"],
+    deps = [
+        ":stats_interface",
+        "//source/common/stats:symbol_table_lib",
+        "//source/common/stats:utility_lib",
+    ],
 )
 
 envoy_cc_library(

include/envoy/stats/stats_macros.h

@@ -5,6 +5,9 @@
 #include "envoy/stats/histogram.h"
 #include "envoy/stats/stats.h"
 
+#include "common/stats/symbol_table_impl.h"
+#include "common/stats/utility.h"
+
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 
@@ -33,6 +36,36 @@ namespace Envoy {
  * Finally, when you want to actually instantiate the above struct using a Stats::Pool, you do:
  *   MyCoolStats stats{
  *     MY_COOL_STATS(POOL_COUNTER(...), POOL_GAUGE(...), POOL_HISTOGRAM(...))};
+ *
+ *
+ * The above constructs are the simplest way to declare counters, gauges,
+ * histograms, and text-readouts in your data structures. However they incur
+ * some overhead to symbolize the names every time they are instantiated. For
+ * data structures that are re-instantiated extensively during operation,
+ * e.g. in response to an xDS update, We can separately instantiate a
+ * StatNameStruct, containing symbolized names for each stat. That should be
+ * instantiated once at startup and held in some context or factory. Do that
+ * with:
+ *
+ *    MAKE_STAT_NAMES_STRUCT(MyStatNames, MY_COOL_STATS);
+ *
+ * This generates a structure definition with a constructor that requires a
+ * SymbolTable. So you must, in a context instantiated once, initialize with:
+ *
+ *    : my_cool_stat_names_(stat_store.symbolTable())
+ *
+ * Once you have the StatNamesStruct instance declared, you can create a stats
+ * struct efficiently during operation (e.g. in an xDS handler) with
+ *
+ *    MAKE_STATS_STRUCT(MyStats, MyStatNames, MY_COOL_STATS);
+ *
+ * This new structure is constructed with 2 or 3 args:
+ *    1. The stat_names struct created from MAKE_STAT_NAMES_STRUCT
+ *    2. The scope in which to instantiate the stats
+ *    3. An optional prefix, which will be prepended to each stat name.
+ * For example:
+ *
+ *    : my_cool_stats_(context.my_cool_stat_names_, scope, opt_prefix)
  */
 
 // Fully-qualified for use in external callsites.
@@ -59,6 +92,7 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_
 #define POOL_GAUGE_PREFIX(POOL, PREFIX) (POOL).gaugeFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_MODE_
 #define POOL_HISTOGRAM_PREFIX(POOL, PREFIX) (POOL).histogramFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_UNIT_
 #define POOL_TEXT_READOUT_PREFIX(POOL, PREFIX) (POOL).textReadoutFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_
+#define POOL_STAT_NAME_PREFIX(POOL, PREFIX) (POOL).symbolTable().textReadoutFromString(Envoy::statPrefixJoin(PREFIX, FINISH_STAT_DECL_
 
 #define POOL_COUNTER(POOL) POOL_COUNTER_PREFIX(POOL, "")
 #define POOL_GAUGE(POOL) POOL_GAUGE_PREFIX(POOL, "")
@@ -69,4 +103,65 @@ static inline std::string statPrefixJoin(absl::string_view prefix, absl::string_
 #define NULL_STAT_DECL_IGNORE_MODE_(X, MODE) std::string(#X)),
 
 #define NULL_POOL_GAUGE(POOL) (POOL).nullGauge(NULL_STAT_DECL_IGNORE_MODE_
+
+// Used for declaring StatNames in a structure.
+#define GENERATE_STAT_NAME_STRUCT(NAME, ...) Envoy::Stats::StatName NAME##_;
+#define GENERATE_STAT_NAME_INIT(NAME, ...) , NAME##_(pool_.add(#NAME))
+
+// Macros for declaring stat-structures using StatNames, for those that must be
+// instantiated during operation, and where speed and scale matters. These
+// macros are not for direct use; they are only for use from
+// MAKE_STAT_NAMES_STRUCT. and MAKE_STAT_STRUCT.
+#define MAKE_STATS_STRUCT_COUNTER_HELPER_(NAME)                                                    \
+  , NAME##_(Envoy::Stats::Utility::counterFromStatNames(scope, {prefix, stat_names.NAME##_}))
+#define MAKE_STATS_STRUCT_GAUGE_HELPER_(NAME, MODE)                                                \
+  , NAME##_(Envoy::Stats::Utility::gaugeFromStatNames(scope, {prefix, stat_names.NAME##_},         \
+                                                      Envoy::Stats::Gauge::ImportMode::MODE))
+#define MAKE_STATS_STRUCT_HISTOGRAM_HELPER_(NAME, UNIT)                                            \
+  , NAME##_(Envoy::Stats::Utility::histogramFromStatNames(scope, {prefix, stat_names.NAME##_},     \
+                                                          Envoy::Stats::Histogram::Unit::UNIT))
+#define MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_(NAME)                                               \
+  , NAME##_(Envoy::Stats::Utility::textReadoutFromStatNames(scope, {prefix, stat_names.NAME##_}))
+
+#define MAKE_STATS_STRUCT_STATNAME_HELPER_(name)
+#define GENERATE_STATNAME_STRUCT(name)
+
+/**
+ * Generates a struct with StatNames for a subsystem, based on the stats macro
+ * with COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, and STATNAME calls. The
+ * ALL_STATS macro must have all 5 parameters.
+ */
+#define MAKE_STAT_NAMES_STRUCT(StatNamesStruct, ALL_STATS)                                         \
+  struct StatNamesStruct {                                                                         \
+    explicit StatNamesStruct(Envoy::Stats::SymbolTable& symbol_table)                              \
+        : pool_(symbol_table)                                                                      \
+              ALL_STATS(GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT, \
+                        GENERATE_STAT_NAME_INIT, GENERATE_STAT_NAME_INIT) {}                       \
+    Envoy::Stats::StatNamePool pool_;                                                              \
+    ALL_STATS(GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT,     \
+              GENERATE_STAT_NAME_STRUCT, GENERATE_STAT_NAME_STRUCT)                                \
+  }
+
+/**
+ * Instantiates a structure of stats based on a new scope and optional prefix,
+ * using a predefined structure of stat names. A reference to the stat_names is
+ * also stored in the structure, for two reasons: (a) as a syntactic convenience
+ * for using macros to generate the comma separators for the initializer and (b)
+ * as a convenience at the call-site to access STATNAME-declared names from the
+ * stats structure.
+ */
+#define MAKE_STATS_STRUCT(StatsStruct, StatNamesStruct, ALL_STATS)                                 \
+  struct StatsStruct {                                                                             \
+    StatsStruct(const StatNamesStruct& stat_names, Envoy::Stats::Scope& scope,                     \
+                Envoy::Stats::StatName prefix = Envoy::Stats::StatName())                          \
+        : stat_names_(stat_names)                                                                  \
+              ALL_STATS(MAKE_STATS_STRUCT_COUNTER_HELPER_, MAKE_STATS_STRUCT_GAUGE_HELPER_,        \
+                        MAKE_STATS_STRUCT_HISTOGRAM_HELPER_,                                       \
+                        MAKE_STATS_STRUCT_TEXT_READOUT_HELPER_,                                    \
+                        MAKE_STATS_STRUCT_STATNAME_HELPER_) {}                                     \
+    const StatNamesStruct& stat_names_;                                                            \
+    ALL_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT,           \
+              GENERATE_TEXT_READOUT_STRUCT, GENERATE_STATNAME_STRUCT)                              \
+  }
+
 } // namespace Envoy

include/envoy/upstream/cluster_manager.h

@@ -24,6 +24,7 @@
 #include "envoy/singleton/manager.h"
 #include "envoy/ssl/context_manager.h"
 #include "envoy/stats/store.h"
+#include "envoy/stats/symbol_table.h"
 #include "envoy/tcp/conn_pool.h"
 #include "envoy/thread_local/thread_local.h"
 #include "envoy/upstream/health_checker.h"
@@ -334,6 +335,16 @@ class ClusterManager {
    * @return Config::SubscriptionFactory& the subscription factory.
    */
   virtual Config::SubscriptionFactory& subscriptionFactory() PURE;
+
+  /**
+   * Returns a struct with all the Stats::StatName objects needed by
+   * Clusters. This helps factor out some relatively heavy name
+   * construction which occur when there is a large CDS update during operation,
+   * relative to recreating all stats from strings on-the-fly.
+   *
+   * @return the stat names.
+   */
+  virtual const ClusterStatNames& clusterStatNames() const PURE;
 };
 
 using ClusterManagerPtr = std::unique_ptr<ClusterManager>;

include/envoy/upstream/upstream.h

@@ -515,7 +515,7 @@ class PrioritySet {
 /**
  * All cluster stats. @see stats_macros.h
  */
-#define ALL_CLUSTER_STATS(COUNTER, GAUGE, HISTOGRAM)                                               \
+#define ALL_CLUSTER_STATS(COUNTER, GAUGE, HISTOGRAM, TEXT_READOUT, STATNAME)                       \
   COUNTER(assignment_stale)                                                                        \
   COUNTER(assignment_timeout_received)                                                             \
   COUNTER(bind_errors)                                                                             \
@@ -643,9 +643,8 @@ class PrioritySet {
 /**
  * Struct definition for all cluster stats. @see stats_macros.h
  */
-struct ClusterStats {
-  ALL_CLUSTER_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT, GENERATE_HISTOGRAM_STRUCT)
-};
+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

source/common/http/BUILD

@@ -20,6 +20,7 @@ envoy_cc_library(
         "//include/envoy/http:context_interface",
         "//include/envoy/http:header_map_interface",
         "//include/envoy/http:message_interface",
+        "//include/envoy/router:context_interface",
         "//include/envoy/router:router_interface",
         "//include/envoy/router:router_ratelimit_interface",
         "//include/envoy/router:shadow_writer_interface",

source/common/http/async_client_impl.cc

@@ -41,10 +41,10 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster,
                                  Upstream::ClusterManager& cm, Runtime::Loader& runtime,
                                  Random::RandomGenerator& random,
                                  Router::ShadowWriterPtr&& shadow_writer,
-                                 Http::Context& http_context)
-    : cluster_(cluster), config_("http.async-client.", local_info, stats_store, cm, runtime, random,
-                                 std::move(shadow_writer), true, false, false, false, {},
-                                 dispatcher.timeSource(), http_context),
+                                 Http::Context& http_context, Router::Context& router_context)
+    : cluster_(cluster), config_(http_context.asyncClientStatPrefix(), local_info, stats_store, cm,
+                                 runtime, random, std::move(shadow_writer), true, false, false,
+                                 false, {}, dispatcher.timeSource(), http_context, router_context),
       dispatcher_(dispatcher) {}
 
 AsyncClientImpl::~AsyncClientImpl() {

source/common/http/async_client_impl.h

@@ -20,6 +20,7 @@
 #include "envoy/http/context.h"
 #include "envoy/http/header_map.h"
 #include "envoy/http/message.h"
+#include "envoy/router/context.h"
 #include "envoy/router/router.h"
 #include "envoy/router/router_ratelimit.h"
 #include "envoy/router/shadow_writer.h"
@@ -49,7 +50,7 @@ class AsyncClientImpl final : public AsyncClient {
                   Event::Dispatcher& dispatcher, const LocalInfo::LocalInfo& local_info,
                   Upstream::ClusterManager& cm, Runtime::Loader& runtime,
                   Random::RandomGenerator& random, Router::ShadowWriterPtr&& shadow_writer,
-                  Http::Context& http_context);
+                  Http::Context& http_context, Router::Context& router_context);
   ~AsyncClientImpl() override;
 
   // Http::AsyncClient

source/common/http/context_impl.cc

@@ -4,7 +4,8 @@ namespace Envoy {
 namespace Http {
 
 ContextImpl::ContextImpl(Stats::SymbolTable& symbol_table)
-    : code_stats_(symbol_table), user_agent_context_(symbol_table) {}
+    : code_stats_(symbol_table), user_agent_context_(symbol_table),
+      async_client_stat_prefix_(user_agent_context_.pool_.add("http.async-client")) {}
 
 } // namespace Http
 } // namespace Envoy

source/common/http/context_impl.h

@@ -27,10 +27,14 @@ class ContextImpl : public Context {
   }
 
   const UserAgentContext& userAgentContext() const override { return user_agent_context_; }
+  const Stats::StatName& asyncClientStatPrefix() const override {
+    return async_client_stat_prefix_;
+  }
 
 private:
   CodeStatsImpl code_stats_;
   UserAgentContext user_agent_context_;
+  const Stats::StatName async_client_stat_prefix_;
   envoy::config::trace::v3::Tracing default_tracing_config_;
 };
 

source/common/router/BUILD

@@ -91,6 +91,16 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "context_lib",
+    srcs = ["context_impl.cc"],
+    hdrs = ["context_impl.h"],
+    deps = [
+        "//include/envoy/router:context_interface",
+        "//include/envoy/stats:stats_macros",
+    ],
+)
+
 envoy_cc_library(
     name = "debug_config_lib",
     srcs = ["debug_config.cc"],
@@ -270,6 +280,7 @@ envoy_cc_library(
     ],
     deps = [
         ":config_lib",
+        ":context_lib",
         ":debug_config_lib",
         ":header_parser_lib",
         ":retry_state_lib",

source/common/router/context_impl.cc

@@ -0,0 +1,9 @@
+#include "common/router/context_impl.h"
+
+namespace Envoy {
+namespace Router {
+
+ContextImpl::ContextImpl(Stats::SymbolTable& symbol_table) : stat_names_(symbol_table) {}
+
+} // namespace Router
+} // namespace Envoy