stats: Stats remove operator scope hack phase 1 (envoyproxy#24567)

Signed-off-by: Joshua Marantz <[email protected]>

envoy/stats/store.h

@@ -155,14 +155,16 @@ class Store {
   // inheritance of Scope as a parent of Store. There is semantic complexity to
   // that PR, so it's going to be easier review if it's as small as possible.
   //
-  // A follow-up PR is required to remove the functions below, which will
-  // require a large number of files to be trivially changed, by explicitly
-  // accessing the rootScope() to call these methods.
+  // A series of follow-up PRs is required to remove the functions below, which
+  // will require a large number of files to be trivially changed, by explicitly
+  // accessing the rootScope() to call these methods. The first in the series is
+  // https://github.com/envoyproxy/envoy/pull/24567 which just takes care of
+  // test/common/...
   operator Scope&() { return *rootScope(); }
 
   // Delegate some methods to the root scope; these are exposed to make it more
   // convenient to use stats_macros.h. We may consider dropping them if desired,
-  // when we resovle #24007 or in the next follow-up.
+  // when we resolve #24007 or in the next follow-up.
   Counter& counterFromString(const std::string& name) {
     return rootScope()->counterFromString(name);
   }
@@ -177,7 +179,7 @@ class Store {
   }
 
   /**
-   * @returns a scape of the given name.
+   * @returns a scope of the given name.
    */
   ScopeSharedPtr createScope(const std::string& name) { return rootScope()->createScope(name); }
 };

envoy/upstream/cluster_factory.h

@@ -74,9 +74,9 @@ class ClusterFactoryContext : public Server::Configuration::FactoryContextBase {
   virtual Outlier::EventLoggerSharedPtr outlierEventLogger() PURE;
 
   // Server::Configuration::FactoryContextBase
-  Stats::Scope& scope() override { return stats(); }
+  Stats::Scope& scope() override { return *stats().rootScope(); }
 
-  Stats::Scope& serverScope() override { return stats(); }
+  Stats::Scope& serverScope() override { return *stats().rootScope(); }
 };
 
 /**

source/common/api/api_impl.h

@@ -36,7 +36,7 @@ class Impl : public Api {
   Thread::ThreadFactory& threadFactory() override { return thread_factory_; }
   Filesystem::Instance& fileSystem() override { return file_system_; }
   TimeSource& timeSource() override { return time_system_; }
-  Stats::Scope& rootScope() override { return store_; }
+  Stats::Scope& rootScope() override { return *store_.rootScope(); }
   Random::RandomGenerator& randomGenerator() override { return random_generator_; }
   Stats::CustomStatNamespaces& customStatNamespaces() override { return custom_stat_namespaces_; }
   const envoy::config::bootstrap::v3::Bootstrap& bootstrap() const override { return bootstrap_; }

source/common/http/async_client_impl.cc

@@ -41,8 +41,8 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster,
                                  Router::ShadowWriterPtr&& shadow_writer,
                                  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, false, {},
+      config_(http_context.asyncClientStatPrefix(), local_info, *stats_store.rootScope(), cm,
+              runtime, random, std::move(shadow_writer), true, false, false, false, false, {},
               dispatcher.timeSource(), http_context, router_context),
       dispatcher_(dispatcher), singleton_manager_(cm.clusterManagerFactory().singletonManager()) {}
 

source/common/runtime/runtime_impl.cc

@@ -581,7 +581,7 @@ void LoaderImpl::mergeValues(const absl::node_hash_map<std::string, std::string>
   loadNewSnapshot();
 }
 
-Stats::Scope& LoaderImpl::getRootScope() { return store_; }
+Stats::Scope& LoaderImpl::getRootScope() { return *store_.rootScope(); }
 
 void LoaderImpl::countDeprecatedFeatureUse() const { countDeprecatedFeatureUseInternal(stats_); }
 

source/common/upstream/cluster_manager_impl.cc

@@ -292,7 +292,7 @@ ClusterManagerImpl::ClusterManagerImpl(
       bind_config_(bootstrap.cluster_manager().has_upstream_bind_config()
                        ? absl::make_optional(bootstrap.cluster_manager().upstream_bind_config())
                        : absl::nullopt),
-      local_info_(local_info), cm_stats_(generateStats(stats)),
+      local_info_(local_info), cm_stats_(generateStats(*stats.rootScope())),
       init_helper_(*this, [this](ClusterManagerCluster& cluster) { onClusterInit(cluster); }),
       time_source_(main_thread_dispatcher.timeSource()), dispatcher_(main_thread_dispatcher),
       http_context_(http_context), router_context_(router_context),
@@ -341,7 +341,7 @@ ClusterManagerImpl::ClusterManagerImpl(
         bootstrap.xds_config_tracker_extension());
     xds_config_tracker_ = tracer_factory.createXdsConfigTracker(
         bootstrap.xds_config_tracker_extension().typed_config(),
-        validation_context.dynamicValidationVisitor(), main_thread_dispatcher, stats);
+        validation_context.dynamicValidationVisitor(), main_thread_dispatcher, *stats.rootScope());
   }
 
   subscription_factory_ = std::make_unique<Config::SubscriptionFactoryImpl>(
@@ -393,26 +393,26 @@ ClusterManagerImpl::ClusterManagerImpl(
       Config::Utility::checkTransportVersion(dyn_resources.ads_config());
       if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.unified_mux")) {
         ads_mux_ = std::make_shared<Config::XdsMux::GrpcMuxDelta>(
-            Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_,
-                                                           dyn_resources.ads_config(), stats, false)
+            Config::Utility::factoryForGrpcApiConfigSource(
+                *async_client_manager_, dyn_resources.ads_config(), *stats.rootScope(), false)
                 ->createUncachedRawAsyncClient(),
             main_thread_dispatcher,
             *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
                 "envoy.service.discovery.v3.AggregatedDiscoveryService."
                 "DeltaAggregatedResources"),
-            random_, stats_,
+            random_, *stats_.rootScope(),
             Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), local_info,
             dyn_resources.ads_config().set_node_on_first_message_only(),
             std::move(custom_config_validators), makeOptRefFromPtr(xds_config_tracker_.get()));
       } else {
         ads_mux_ = std::make_shared<Config::NewGrpcMuxImpl>(
-            Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_,
-                                                           dyn_resources.ads_config(), stats, false)
+            Config::Utility::factoryForGrpcApiConfigSource(
+                *async_client_manager_, dyn_resources.ads_config(), *stats.rootScope(), false)
                 ->createUncachedRawAsyncClient(),
             main_thread_dispatcher,
             *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
                 "envoy.service.discovery.v3.AggregatedDiscoveryService.DeltaAggregatedResources"),
-            random_, stats_,
+            random_, *stats_.rootScope(),
             Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), local_info,
             std::move(custom_config_validators), makeOptRefFromPtr(xds_config_tracker_.get()));
       }
@@ -424,28 +424,28 @@ ClusterManagerImpl::ClusterManagerImpl(
 
       if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.unified_mux")) {
         ads_mux_ = std::make_shared<Config::XdsMux::GrpcMuxSotw>(
-            Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_,
-                                                           dyn_resources.ads_config(), stats, false)
+            Config::Utility::factoryForGrpcApiConfigSource(
+                *async_client_manager_, dyn_resources.ads_config(), *stats.rootScope(), false)
                 ->createUncachedRawAsyncClient(),
             main_thread_dispatcher,
             *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
                 "envoy.service.discovery.v3.AggregatedDiscoveryService."
                 "StreamAggregatedResources"),
-            random_, stats_,
+            random_, *stats_.rootScope(),
             Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), local_info,
             bootstrap.dynamic_resources().ads_config().set_node_on_first_message_only(),
             std::move(custom_config_validators), makeOptRefFromPtr(xds_config_tracker_.get()),
             xds_delegate_opt_ref, target_xds_authority);
       } else {
         ads_mux_ = std::make_shared<Config::GrpcMuxImpl>(
             local_info,
-            Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_,
-                                                           dyn_resources.ads_config(), stats, false)
+            Config::Utility::factoryForGrpcApiConfigSource(
+                *async_client_manager_, dyn_resources.ads_config(), *stats.rootScope(), false)
                 ->createUncachedRawAsyncClient(),
             main_thread_dispatcher,
             *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
                 "envoy.service.discovery.v3.AggregatedDiscoveryService.StreamAggregatedResources"),
-            random_, stats_,
+            random_, *stats_.rootScope(),
             Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()),
             bootstrap.dynamic_resources().ads_config().set_node_on_first_message_only(),
             std::move(custom_config_validators), makeOptRefFromPtr(xds_config_tracker_.get()),
@@ -525,9 +525,9 @@ void ClusterManagerImpl::initializeSecondaryClusters(
 
     Config::Utility::checkTransportVersion(load_stats_config);
     load_stats_reporter_ = std::make_unique<LoadStatsReporter>(
-        local_info_, *this, stats_,
+        local_info_, *this, *stats_.rootScope(),
         Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, load_stats_config,
-                                                       stats_, false)
+                                                       *stats_.rootScope(), false)
             ->createUncachedRawAsyncClient(),
         dispatcher_);
   }
@@ -1242,8 +1242,8 @@ ClusterManagerImpl::allocateOdCdsApi(const envoy::config::core::v3::ConfigSource
   // TODO(krnowak): Instead of creating a new handle every time, store the handles internally and
   // return an already existing one if the config or locator matches. Note that this may need a way
   // to clean up the unused handles, so we can close the unnecessary connections.
-  auto odcds = OdCdsApiImpl::create(odcds_config, odcds_resources_locator, *this, *this, stats_,
-                                    validation_visitor);
+  auto odcds = OdCdsApiImpl::create(odcds_config, odcds_resources_locator, *this, *this,
+                                    *stats_.rootScope(), validation_visitor);
   return OdCdsApiHandleImpl::create(*this, std::move(odcds));
 }
 
@@ -1899,7 +1899,7 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool(
     return std::make_unique<Http::ConnectivityGrid>(
         dispatcher, context_.api().randomGenerator(), host, priority, options,
         transport_socket_options, state, source, alternate_protocols_cache, coptions,
-        quic_stat_names_, stats_, *quic_info);
+        quic_stat_names_, *stats_.rootScope(), *quic_info);
 #else
     (void)quic_info;
     // Should be blocked by configuration checking at an earlier point.
@@ -1936,7 +1936,7 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool(
     }
     return Http::Http3::allocateConnPool(dispatcher, context_.api().randomGenerator(), host,
                                          priority, options, transport_socket_options, state,
-                                         quic_stat_names_, {}, stats_, {}, *quic_info);
+                                         quic_stat_names_, {}, *stats_.rootScope(), {}, *quic_info);
 #else
     UNREFERENCED_PARAMETER(source);
     // Should be blocked by configuration checking at an earlier point.
@@ -1974,7 +1974,7 @@ ProdClusterManagerFactory::createCds(const envoy::config::core::v3::ConfigSource
                                      const xds::core::v3::ResourceLocator* cds_resources_locator,
                                      ClusterManager& cm) {
   // TODO(htuch): Differentiate static vs. dynamic validation visitors.
-  return CdsApiImpl::create(cds_config, cds_resources_locator, cm, stats_,
+  return CdsApiImpl::create(cds_config, cds_resources_locator, cm, *stats_.rootScope(),
                             validation_context_.dynamicValidationVisitor());
 }
 

source/common/upstream/upstream_impl.cc

@@ -875,7 +875,7 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {
   // other contexts taken from TransportSocketFactoryContext.
   FactoryContextImpl(Stats::Scope& stats_scope, Envoy::Runtime::Loader& runtime,
                      Server::Configuration::TransportSocketFactoryContext& c)
-      : admin_(c.admin()), server_scope_(c.stats()), stats_scope_(stats_scope),
+      : admin_(c.admin()), server_scope_(*c.stats().rootScope()), stats_scope_(stats_scope),
         cluster_manager_(c.clusterManager()), local_info_(c.localInfo()),
         dispatcher_(c.mainThreadDispatcher()), runtime_(runtime),
         singleton_manager_(c.singletonManager()), tls_(c.threadLocal()), api_(c.api()),
@@ -1001,8 +1001,9 @@ ClusterInfoImpl::ClusterInfoImpl(
       lb_stats_(factory_context.clusterManager().clusterLbStatNames(), *stats_scope_),
       endpoint_stats_(factory_context.clusterManager().clusterEndpointStatNames(), *stats_scope_),
       load_report_stats_store_(stats_scope_->symbolTable()),
-      load_report_stats_(generateLoadReportStats(
-          load_report_stats_store_, factory_context.clusterManager().clusterLoadReportStatNames())),
+      load_report_stats_(
+          generateLoadReportStats(*load_report_stats_store_.rootScope(),
+                                  factory_context.clusterManager().clusterLoadReportStatNames())),
       optional_cluster_stats_((config.has_track_cluster_stats() || config.track_timeout_budgets())
                                   ? std::make_unique<OptionalClusterStats>(
                                         config, *stats_scope_, factory_context.clusterManager())

source/extensions/listener_managers/listener_manager/listener_impl.h

@@ -142,7 +142,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex
   Init::Manager& initManager() override;
   const LocalInfo::LocalInfo& localInfo() const override;
   Envoy::Runtime::Loader& runtime() override;
-  Stats::Scope& serverScope() override { return server_.stats(); }
+  Stats::Scope& serverScope() override { return *server_.stats().rootScope(); }
   Stats::Scope& scope() override;
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;

source/extensions/listener_managers/listener_manager/listener_manager_impl.h

@@ -75,7 +75,7 @@ class ProdListenerComponentFactory : public ListenerComponentFactory,
                          const xds::core::v3::ResourceLocator* lds_resources_locator) override {
     return std::make_unique<LdsApiImpl>(
         lds_config, lds_resources_locator, server_.clusterManager(), server_.initManager(),
-        server_.stats(), server_.listenerManager(),
+        *server_.stats().rootScope(), server_.listenerManager(),
         server_.messageValidationContext().dynamicValidationVisitor());
   }
   std::vector<Network::FilterFactoryCb> createNetworkFilterFactoryList(

source/server/admin/admin.cc

@@ -90,11 +90,11 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server,
     : server_(server),
       request_id_extension_(Extensions::RequestId::UUIDRequestIDExtension::defaultInstance(
           server_.api().randomGenerator())),
-      profile_path_(profile_path),
-      stats_(Http::ConnectionManagerImpl::generateStats("http.admin.", server_.stats())),
+      profile_path_(profile_path), stats_(Http::ConnectionManagerImpl::generateStats(
+                                       "http.admin.", *server_.stats().rootScope())),
       null_overload_manager_(server_.threadLocal()),
-      tracing_stats_(
-          Http::ConnectionManagerImpl::generateTracingStats("http.admin.", no_op_store_)),
+      tracing_stats_(Http::ConnectionManagerImpl::generateTracingStats("http.admin.",
+                                                                       *no_op_store_.rootScope())),
       route_config_provider_(server.timeSource()),
       scoped_route_config_provider_(server.timeSource()), clusters_handler_(server),
       config_dump_handler_(config_tracker_, server), init_dump_handler_(server),
@@ -243,7 +243,7 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection
                                                  const Buffer::Instance& data,
                                                  Http::ServerConnectionCallbacks& callbacks) {
   return Http::ConnectionManagerUtility::autoCreateCodec(
-      connection, data, callbacks, server_.stats(), server_.api().randomGenerator(),
+      connection, data, callbacks, *server_.stats().rootScope(), server_.api().randomGenerator(),
       http1_codec_stats_, http2_codec_stats_, Http::Http1Settings(),
       ::Envoy::Http2::Utility::initializeAndValidateOptions(
           envoy::config::core::v3::Http2ProtocolOptions()),

source/server/factory_context_base_impl.h

@@ -12,11 +12,11 @@ class FactoryContextBaseImpl : public Configuration::FactoryContextBase {
                          OptRef<Server::Admin> admin, Runtime::Loader& runtime,
                          Singleton::Manager& singleton_manager,
                          ProtobufMessage::ValidationVisitor& validation_visitor,
-                         Stats::Store& scope, ThreadLocal::Instance& local)
+                         Stats::Store& store, ThreadLocal::Instance& local)
       : options_(options), main_thread_dispatcher_(main_thread_dispatcher), api_(api),
         local_info_(local_info), admin_(admin), runtime_(runtime),
         singleton_manager_(singleton_manager), validation_visitor_(validation_visitor),
-        scope_(scope), thread_local_(local) {}
+        scope_(*store.rootScope()), thread_local_(local) {}
 
   FactoryContextBaseImpl(Configuration::FactoryContextBase& config)
       : options_(config.options()), main_thread_dispatcher_(config.mainThreadDispatcher()),

source/server/factory_context_impl.h

@@ -28,7 +28,7 @@ class FactoryContextImpl : public Configuration::FactoryContext {
   const LocalInfo::LocalInfo& localInfo() const override;
   Envoy::Runtime::Loader& runtime() override;
   Stats::Scope& scope() override;
-  Stats::Scope& serverScope() override { return server_.stats(); }
+  Stats::Scope& serverScope() override { return *server_.stats().rootScope(); }
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;
   ThreadLocal::SlotAllocator& threadLocal() override;