logger: use server stats scope at grpc logger

envoy/server/factory_context.h

@@ -89,6 +89,11 @@ class FactoryContextBase {
    */
   virtual Stats::Scope& scope() PURE;
 
+  /**
+   * @return Stats::Scope& the server wide stats scope.
+   */
+  virtual Stats::Scope& serverScope() PURE;
+
   /**
    * @return ThreadLocal::SlotAllocator& the thread local storage engine for the server. This is
    *         used to allow runtime lockless updates to configuration, etc. across multiple threads.

envoy/upstream/cluster_factory.h

@@ -75,6 +75,8 @@ class ClusterFactoryContext : public Server::Configuration::FactoryContextBase {
 
   // Server::Configuration::FactoryContextBase
   Stats::Scope& scope() override { return stats(); }
+
+  Stats::Scope& serverScope() override { return stats(); }
 };
 
 /**

source/common/upstream/upstream_impl.cc

@@ -676,8 +676,9 @@ 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()), stats_scope_(stats_scope), cluster_manager_(c.clusterManager()),
-        local_info_(c.localInfo()), dispatcher_(c.mainThreadDispatcher()), runtime_(runtime),
+      : admin_(c.admin()), server_scope_(c.stats()), 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()),
         options_(c.options()), message_validation_visitor_(c.messageValidationVisitor()) {}
 
@@ -687,6 +688,7 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {
   const LocalInfo::LocalInfo& localInfo() const override { return local_info_; }
   Envoy::Runtime::Loader& runtime() override { return runtime_; }
   Stats::Scope& scope() override { return stats_scope_; }
+  Stats::Scope& serverScope() override { return server_scope_; }
   Singleton::Manager& singletonManager() override { return singleton_manager_; }
   ThreadLocal::SlotAllocator& threadLocal() override { return tls_; }
   Server::Admin& admin() override { return admin_; }
@@ -719,6 +721,7 @@ class FactoryContextImpl : public Server::Configuration::CommonFactoryContext {
 
 private:
   Server::Admin& admin_;
+  Stats::Scope& server_scope_;
   Stats::Scope& stats_scope_;
   Upstream::ClusterManager& cluster_manager_;
   const LocalInfo::LocalInfo& local_info_;

source/extensions/access_loggers/common/grpc_access_logger.h

@@ -68,9 +68,8 @@ template <typename GrpcAccessLogger, typename ConfigProto> class GrpcAccessLogge
    * @param config supplies the configuration for the logger.
    * @return GrpcAccessLoggerSharedPtr ready for logging requests.
    */
-  virtual typename GrpcAccessLogger::SharedPtr getOrCreateLogger(const ConfigProto& config,
-                                                                 GrpcAccessLoggerType logger_type,
-                                                                 Stats::Scope& scope) PURE;
+  virtual typename GrpcAccessLogger::SharedPtr
+  getOrCreateLogger(const ConfigProto& config, GrpcAccessLoggerType logger_type) PURE;
 };
 
 template <typename LogRequest, typename LogResponse> class GrpcAccessLogClient {
@@ -252,15 +251,14 @@ class GrpcAccessLoggerCache : public Singleton::Instance,
 
   GrpcAccessLoggerCache(Grpc::AsyncClientManager& async_client_manager, Stats::Scope& scope,
                         ThreadLocal::SlotAllocator& tls)
-      : async_client_manager_(async_client_manager), scope_(scope), tls_slot_(tls.allocateSlot()) {
+      : scope_(scope), async_client_manager_(async_client_manager), tls_slot_(tls.allocateSlot()) {
     tls_slot_->set([](Event::Dispatcher& dispatcher) {
       return std::make_shared<ThreadLocalCache>(dispatcher);
     });
   }
 
-  typename GrpcAccessLogger::SharedPtr getOrCreateLogger(const ConfigProto& config,
-                                                         GrpcAccessLoggerType logger_type,
-                                                         Stats::Scope& scope) override {
+  typename GrpcAccessLogger::SharedPtr
+  getOrCreateLogger(const ConfigProto& config, GrpcAccessLoggerType logger_type) override {
     // TODO(euroelessar): Consider cleaning up loggers.
     auto& cache = tls_slot_->getTyped<ThreadLocalCache>();
     const auto cache_key = std::make_pair(MessageUtil::hash(config), logger_type);
@@ -277,12 +275,14 @@ class GrpcAccessLoggerCache : public Singleton::Instance,
     const auto logger = createLogger(
         config, std::move(client),
         std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config, buffer_flush_interval, 1000)),
-        PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, buffer_size_bytes, 16384), cache.dispatcher_,
-        scope);
+        PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, buffer_size_bytes, 16384), cache.dispatcher_);
     cache.access_loggers_.emplace(cache_key, logger);
     return logger;
   }
 
+protected:
+  Stats::Scope& scope_;
+
 private:
   /**
    * Per-thread cache.
@@ -301,10 +301,9 @@ class GrpcAccessLoggerCache : public Singleton::Instance,
   virtual typename GrpcAccessLogger::SharedPtr
   createLogger(const ConfigProto& config, const Grpc::RawAsyncClientSharedPtr& client,
                std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
-               Event::Dispatcher& dispatcher, Stats::Scope& scope) PURE;
+               Event::Dispatcher& dispatcher) PURE;
 
   Grpc::AsyncClientManager& async_client_manager_;
-  Stats::Scope& scope_;
   ThreadLocal::SlotPtr tls_slot_;
 };
 

source/extensions/access_loggers/grpc/config_utils.cc

@@ -15,7 +15,7 @@ getGrpcAccessLoggerCacheSingleton(Server::Configuration::CommonFactoryContext& c
   return context.singletonManager().getTyped<GrpcCommon::GrpcAccessLoggerCacheImpl>(
       SINGLETON_MANAGER_REGISTERED_NAME(grpc_access_logger_cache), [&context] {
         return std::make_shared<GrpcCommon::GrpcAccessLoggerCacheImpl>(
-            context.clusterManager().grpcAsyncClientManager(), context.scope(),
+            context.clusterManager().grpcAsyncClientManager(), context.serverScope(),
             context.threadLocal(), context.localInfo());
       });
 }

source/extensions/access_loggers/grpc/grpc_access_log_impl.cc

@@ -53,10 +53,10 @@ GrpcAccessLoggerImpl::SharedPtr GrpcAccessLoggerCacheImpl::createLogger(
     const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
     const Grpc::RawAsyncClientSharedPtr& client,
     std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
-    Event::Dispatcher& dispatcher, Stats::Scope& scope) {
+    Event::Dispatcher& dispatcher) {
   return std::make_shared<GrpcAccessLoggerImpl>(client, config.log_name(),
                                                 buffer_flush_interval_msec, max_buffer_size_bytes,
-                                                dispatcher, local_info_, scope);
+                                                dispatcher, local_info_, scope_);
 }
 
 } // namespace GrpcCommon

source/extensions/access_loggers/grpc/grpc_access_log_impl.h

@@ -54,7 +54,7 @@ class GrpcAccessLoggerCacheImpl
   createLogger(const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
                const Grpc::RawAsyncClientSharedPtr& client,
                std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
-               Event::Dispatcher& dispatcher, Stats::Scope& scope) override;
+               Event::Dispatcher& dispatcher) override;
 
   const LocalInfo::LocalInfo& local_info_;
 };

source/extensions/access_loggers/grpc/http_config.cc

@@ -31,9 +31,9 @@ AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance(
   if (service_config.has_envoy_grpc()) {
     context.clusterManager().checkActiveStaticCluster(service_config.envoy_grpc().cluster_name());
   }
-  return std::make_shared<HttpGrpcAccessLog>(std::move(filter), proto_config, context.threadLocal(),
-                                             GrpcCommon::getGrpcAccessLoggerCacheSingleton(context),
-                                             context.scope());
+  return std::make_shared<HttpGrpcAccessLog>(
+      std::move(filter), proto_config, context.threadLocal(),
+      GrpcCommon::getGrpcAccessLoggerCacheSingleton(context));
 }
 
 ProtobufTypes::MessagePtr HttpGrpcAccessLogFactory::createEmptyConfigProto() {

source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc

@@ -26,9 +26,8 @@ HttpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(
 HttpGrpcAccessLog::HttpGrpcAccessLog(AccessLog::FilterPtr&& filter,
                                      const HttpGrpcAccessLogConfig config,
                                      ThreadLocal::SlotAllocator& tls,
-                                     GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-                                     Stats::Scope& scope)
-    : Common::ImplBase(std::move(filter)), scope_(scope),
+                                     GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache)
+    : Common::ImplBase(std::move(filter)),
       config_(std::make_shared<const HttpGrpcAccessLogConfig>(std::move(config))),
       tls_slot_(tls.allocateSlot()), access_logger_cache_(std::move(access_logger_cache)) {
   for (const auto& header : config_->additional_request_headers_to_log()) {
@@ -43,13 +42,11 @@ HttpGrpcAccessLog::HttpGrpcAccessLog(AccessLog::FilterPtr&& filter,
     response_trailers_to_log_.emplace_back(header);
   }
   Envoy::Config::Utility::checkTransportVersion(config_->common_config());
-  // Note that &scope might have died by the time when this callback is called on each thread.
-  // This is supposed to be fixed by https://github.com/envoyproxy/envoy/issues/18066.
-  tls_slot_->set([config = config_, access_logger_cache = access_logger_cache_,
-                  &scope = scope_](Event::Dispatcher&) {
-    return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
-        config->common_config(), Common::GrpcAccessLoggerType::HTTP, scope));
-  });
+  tls_slot_->set(
+      [config = config_, access_logger_cache = access_logger_cache_](Event::Dispatcher&) {
+        return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
+            config->common_config(), Common::GrpcAccessLoggerType::HTTP));
+      });
 }
 
 void HttpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_headers,

source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h

@@ -31,8 +31,7 @@ class HttpGrpcAccessLog : public Common::ImplBase {
 public:
   HttpGrpcAccessLog(AccessLog::FilterPtr&& filter, const HttpGrpcAccessLogConfig config,
                     ThreadLocal::SlotAllocator& tls,
-                    GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-                    Stats::Scope& scope);
+                    GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache);
 
 private:
   /**
@@ -50,7 +49,6 @@ class HttpGrpcAccessLog : public Common::ImplBase {
                const Http::ResponseTrailerMap& response_trailers,
                const StreamInfo::StreamInfo& stream_info) override;
 
-  Stats::Scope& scope_;
   const HttpGrpcAccessLogConfigConstSharedPtr config_;
   const ThreadLocal::SlotPtr tls_slot_;
   const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;

source/extensions/access_loggers/grpc/tcp_config.cc

@@ -32,8 +32,7 @@ AccessLog::InstanceSharedPtr TcpGrpcAccessLogFactory::createAccessLogInstance(
     context.clusterManager().checkActiveStaticCluster(service_config.envoy_grpc().cluster_name());
   }
   return std::make_shared<TcpGrpcAccessLog>(std::move(filter), proto_config, context.threadLocal(),
-                                            GrpcCommon::getGrpcAccessLoggerCacheSingleton(context),
-                                            context.scope());
+                                            GrpcCommon::getGrpcAccessLoggerCacheSingleton(context));
 }
 
 ProtobufTypes::MessagePtr TcpGrpcAccessLogFactory::createEmptyConfigProto() {

source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc

@@ -20,19 +20,16 @@ TcpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(GrpcCommon::GrpcAccessLog
 TcpGrpcAccessLog::TcpGrpcAccessLog(AccessLog::FilterPtr&& filter,
                                    const TcpGrpcAccessLogConfig config,
                                    ThreadLocal::SlotAllocator& tls,
-                                   GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-                                   Stats::Scope& scope)
-    : Common::ImplBase(std::move(filter)), scope_(scope),
+                                   GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache)
+    : Common::ImplBase(std::move(filter)),
       config_(std::make_shared<const TcpGrpcAccessLogConfig>(std::move(config))),
       tls_slot_(tls.allocateSlot()), access_logger_cache_(std::move(access_logger_cache)) {
   Config::Utility::checkTransportVersion(config_->common_config());
-  // Note that &scope might have died by the time when this callback is called on each thread.
-  // This is supposed to be fixed by https://github.com/envoyproxy/envoy/issues/18066.
-  tls_slot_->set([config = config_, access_logger_cache = access_logger_cache_,
-                  &scope = scope_](Event::Dispatcher&) {
-    return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
-        config->common_config(), Common::GrpcAccessLoggerType::TCP, scope));
-  });
+  tls_slot_->set(
+      [config = config_, access_logger_cache = access_logger_cache_](Event::Dispatcher&) {
+        return std::make_shared<ThreadLocalLogger>(access_logger_cache->getOrCreateLogger(
+            config->common_config(), Common::GrpcAccessLoggerType::TCP));
+      });
 }
 
 void TcpGrpcAccessLog::emitLog(const Http::RequestHeaderMap&, const Http::ResponseHeaderMap&,

source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h

@@ -30,8 +30,7 @@ class TcpGrpcAccessLog : public Common::ImplBase {
 public:
   TcpGrpcAccessLog(AccessLog::FilterPtr&& filter, const TcpGrpcAccessLogConfig config,
                    ThreadLocal::SlotAllocator& tls,
-                   GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-                   Stats::Scope& scope);
+                   GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache);
 
 private:
   /**
@@ -49,7 +48,6 @@ class TcpGrpcAccessLog : public Common::ImplBase {
                const Http::ResponseTrailerMap& response_trailers,
                const StreamInfo::StreamInfo& stream_info) override;
 
-  Stats::Scope& scope_;
   const TcpGrpcAccessLogConfigConstSharedPtr config_;
   const ThreadLocal::SlotPtr tls_slot_;
   const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;

source/extensions/access_loggers/open_telemetry/access_log_impl.cc

@@ -35,15 +35,14 @@ AccessLog::ThreadLocalLogger::ThreadLocalLogger(GrpcAccessLoggerSharedPtr logger
 AccessLog::AccessLog(
     ::Envoy::AccessLog::FilterPtr&& filter,
     envoy::extensions::access_loggers::open_telemetry::v3alpha::OpenTelemetryAccessLogConfig config,
-    ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-    Stats::Scope& scope)
-    : Common::ImplBase(std::move(filter)), scope_(scope), tls_slot_(tls.allocateSlot()),
+    ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache)
+    : Common::ImplBase(std::move(filter)), tls_slot_(tls.allocateSlot()),
       access_logger_cache_(std::move(access_logger_cache)) {
 
   Envoy::Config::Utility::checkTransportVersion(config.common_config());
   tls_slot_->set([this, config](Event::Dispatcher&) {
     return std::make_shared<ThreadLocalLogger>(access_logger_cache_->getOrCreateLogger(
-        config.common_config(), Common::GrpcAccessLoggerType::HTTP, scope_));
+        config.common_config(), Common::GrpcAccessLoggerType::HTTP));
   });
 
   ProtobufWkt::Struct body_format;

source/extensions/access_loggers/open_telemetry/access_log_impl.h

@@ -36,8 +36,7 @@ class AccessLog : public Common::ImplBase {
   AccessLog(::Envoy::AccessLog::FilterPtr&& filter,
             envoy::extensions::access_loggers::open_telemetry::v3alpha::OpenTelemetryAccessLogConfig
                 config,
-            ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-            Stats::Scope& scope);
+            ThreadLocal::SlotAllocator& tls, GrpcAccessLoggerCacheSharedPtr access_logger_cache);
 
 private:
   /**
@@ -55,7 +54,6 @@ class AccessLog : public Common::ImplBase {
                const Http::ResponseTrailerMap& response_trailers,
                const StreamInfo::StreamInfo& stream_info) override;
 
-  Stats::Scope& scope_;
   const ThreadLocal::SlotPtr tls_slot_;
   const GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
   std::unique_ptr<Formatter::StructFormatter> body_formatter_;

source/extensions/access_loggers/open_telemetry/config.cc

@@ -43,7 +43,7 @@ AccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
           config, context.messageValidationVisitor());
 
   return std::make_shared<AccessLog>(std::move(filter), proto_config, context.threadLocal(),
-                                     getAccessLoggerCacheSingleton(context), context.scope());
+                                     getAccessLoggerCacheSingleton(context));
 }
 
 ProtobufTypes::MessagePtr AccessLogFactory::createEmptyConfigProto() {

source/extensions/access_loggers/open_telemetry/grpc_access_log_impl.cc

@@ -76,10 +76,10 @@ GrpcAccessLoggerImpl::SharedPtr GrpcAccessLoggerCacheImpl::createLogger(
     const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
     const Grpc::RawAsyncClientSharedPtr& client,
     std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
-    Event::Dispatcher& dispatcher, Stats::Scope& scope) {
+    Event::Dispatcher& dispatcher) {
   return std::make_shared<GrpcAccessLoggerImpl>(client, config.log_name(),
                                                 buffer_flush_interval_msec, max_buffer_size_bytes,
-                                                dispatcher, local_info_, scope);
+                                                dispatcher, local_info_, scope_);
 }
 
 } // namespace OpenTelemetry

source/extensions/access_loggers/open_telemetry/grpc_access_log_impl.h

@@ -68,7 +68,7 @@ class GrpcAccessLoggerCacheImpl
   createLogger(const envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig& config,
                const Grpc::RawAsyncClientSharedPtr& client,
                std::chrono::milliseconds buffer_flush_interval_msec, uint64_t max_buffer_size_bytes,
-               Event::Dispatcher& dispatcher, Stats::Scope& scope) override;
+               Event::Dispatcher& dispatcher) override;
 
   const LocalInfo::LocalInfo& local_info_;
 };

source/server/factory_context_base_impl.h

@@ -30,6 +30,7 @@ class FactoryContextBaseImpl : public Configuration::FactoryContextBase {
     return validation_visitor_;
   };
   Stats::Scope& scope() override { return scope_; };
+  Stats::Scope& serverScope() override { return scope_; }
   ThreadLocal::SlotAllocator& threadLocal() override { return thread_local_; };
 
 private:

source/server/filter_chain_manager_impl.h

@@ -66,6 +66,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor
   const LocalInfo::LocalInfo& localInfo() const override;
   Envoy::Runtime::Loader& runtime() override;
   Stats::Scope& scope() override;
+  Stats::Scope& serverScope() override { return parent_context_.serverScope(); }
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;
   ThreadLocal::SlotAllocator& threadLocal() override;
@@ -151,6 +152,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(); }
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;
   ThreadLocal::SlotAllocator& threadLocal() override;

source/server/listener_impl.h

@@ -114,6 +114,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& scope() override;
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;
@@ -188,6 +189,7 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte
   const LocalInfo::LocalInfo& localInfo() const override;
   Envoy::Runtime::Loader& runtime() override;
   Stats::Scope& scope() override;
+  Stats::Scope& serverScope() override { return listener_factory_context_base_->serverScope(); }
   Singleton::Manager& singletonManager() override;
   OverloadManager& overloadManager() override;
   ThreadLocal::Instance& threadLocal() override;