logger: fix lifetime issue of AccessLogConfig in tls callback.

source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc

@@ -23,28 +23,32 @@ HttpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(
     GrpcCommon::GrpcAccessLoggerSharedPtr logger)
     : logger_(std::move(logger)) {}
 
-HttpGrpcAccessLog::HttpGrpcAccessLog(
-    AccessLog::FilterPtr&& filter,
-    envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig config,
-    ThreadLocal::SlotAllocator& tls, GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-    Stats::Scope& scope)
-    : Common::ImplBase(std::move(filter)), scope_(scope), config_(std::move(config)),
+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),
+      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()) {
+  for (const auto& header : config_->additional_request_headers_to_log()) {
     request_headers_to_log_.emplace_back(header);
   }
 
-  for (const auto& header : config_.additional_response_headers_to_log()) {
+  for (const auto& header : config_->additional_response_headers_to_log()) {
     response_headers_to_log_.emplace_back(header);
   }
 
-  for (const auto& header : config_.additional_response_trailers_to_log()) {
+  for (const auto& header : config_->additional_response_trailers_to_log()) {
     response_trailers_to_log_.emplace_back(header);
   }
-  Envoy::Config::Utility::checkTransportVersion(config_.common_config());
-  tls_slot_->set([this](Event::Dispatcher&) {
-    return std::make_shared<ThreadLocalLogger>(access_logger_cache_->getOrCreateLogger(
-        config_.common_config(), Common::GrpcAccessLoggerType::HTTP, scope_));
+  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));
   });
 }
 
@@ -56,7 +60,7 @@ void HttpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_headers,
   // TODO(mattklein123): Populate sample_rate field.
   envoy::data::accesslog::v3::HTTPAccessLogEntry log_entry;
   GrpcCommon::Utility::extractCommonAccessLogProperties(*log_entry.mutable_common_properties(),
-                                                        stream_info, config_.common_config());
+                                                        stream_info, config_->common_config());
 
   if (stream_info.protocol()) {
     switch (stream_info.protocol().value()) {

source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h

@@ -21,13 +21,15 @@ namespace HttpGrpc {
 
 // TODO(mattklein123): Stats
 
+using envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig;
+using HttpGrpcAccessLogConfigConstSharedPtr = std::shared_ptr<const HttpGrpcAccessLogConfig>;
+
 /**
  * Access log Instance that streams HTTP logs over gRPC.
  */
 class HttpGrpcAccessLog : public Common::ImplBase {
 public:
-  HttpGrpcAccessLog(AccessLog::FilterPtr&& filter,
-                    envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig config,
+  HttpGrpcAccessLog(AccessLog::FilterPtr&& filter, const HttpGrpcAccessLogConfig config,
                     ThreadLocal::SlotAllocator& tls,
                     GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
                     Stats::Scope& scope);
@@ -49,7 +51,7 @@ class HttpGrpcAccessLog : public Common::ImplBase {
                const StreamInfo::StreamInfo& stream_info) override;
 
   Stats::Scope& scope_;
-  const envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig config_;
+  const HttpGrpcAccessLogConfigConstSharedPtr config_;
   const ThreadLocal::SlotPtr tls_slot_;
   const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
   std::vector<Http::LowerCaseString> request_headers_to_log_;

source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc

@@ -17,17 +17,21 @@ namespace TcpGrpc {
 TcpGrpcAccessLog::ThreadLocalLogger::ThreadLocalLogger(GrpcCommon::GrpcAccessLoggerSharedPtr logger)
     : logger_(std::move(logger)) {}
 
-TcpGrpcAccessLog::TcpGrpcAccessLog(
-    AccessLog::FilterPtr&& filter,
-    envoy::extensions::access_loggers::grpc::v3::TcpGrpcAccessLogConfig config,
-    ThreadLocal::SlotAllocator& tls, GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
-    Stats::Scope& scope)
-    : Common::ImplBase(std::move(filter)), scope_(scope), config_(std::move(config)),
+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),
+      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());
-  tls_slot_->set([this](Event::Dispatcher&) {
-    return std::make_shared<ThreadLocalLogger>(access_logger_cache_->getOrCreateLogger(
-        config_.common_config(), Common::GrpcAccessLoggerType::TCP, scope_));
+  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));
   });
 }
 
@@ -37,7 +41,7 @@ void TcpGrpcAccessLog::emitLog(const Http::RequestHeaderMap&, const Http::Respon
   // Common log properties.
   envoy::data::accesslog::v3::TCPAccessLogEntry log_entry;
   GrpcCommon::Utility::extractCommonAccessLogProperties(*log_entry.mutable_common_properties(),
-                                                        stream_info, config_.common_config());
+                                                        stream_info, config_->common_config());
 
   envoy::data::accesslog::v3::ConnectionProperties& connection_properties =
       *log_entry.mutable_connection_properties();

source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h

@@ -20,13 +20,15 @@ namespace TcpGrpc {
 
 // TODO(mattklein123): Stats
 
+using envoy::extensions::access_loggers::grpc::v3::TcpGrpcAccessLogConfig;
+using TcpGrpcAccessLogConfigConstSharedPtr = std::shared_ptr<const TcpGrpcAccessLogConfig>;
+
 /**
  * Access log Instance that streams TCP logs over gRPC.
  */
 class TcpGrpcAccessLog : public Common::ImplBase {
 public:
-  TcpGrpcAccessLog(AccessLog::FilterPtr&& filter,
-                   envoy::extensions::access_loggers::grpc::v3::TcpGrpcAccessLogConfig config,
+  TcpGrpcAccessLog(AccessLog::FilterPtr&& filter, const TcpGrpcAccessLogConfig config,
                    ThreadLocal::SlotAllocator& tls,
                    GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache,
                    Stats::Scope& scope);
@@ -48,7 +50,7 @@ class TcpGrpcAccessLog : public Common::ImplBase {
                const StreamInfo::StreamInfo& stream_info) override;
 
   Stats::Scope& scope_;
-  const envoy::extensions::access_loggers::grpc::v3::TcpGrpcAccessLogConfig config_;
+  const TcpGrpcAccessLogConfigConstSharedPtr config_;
   const ThreadLocal::SlotPtr tls_slot_;
   const GrpcCommon::GrpcAccessLoggerCacheSharedPtr access_logger_cache_;
 };