logger api refactoring: unified two methods to one to use factory con…

…text (envoyproxy#31012) Commit Message: logger api refactoring: unified two methods to one to use factory context Additional Description: Part of factory context refactoring and clean up. This patch merged two createAccessLogInstance method to one and take the FactoryContext as the parameter directly. Risk Level: low. (Parameter type only update and didn't change any logic). Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. Signed-off-by: wbpcode <[email protected]>
dynatrace-oss-contrib · Nov 27, 2023 · 1ef48e6 · 1ef48e6
1 parent fdb3541
commit 1ef48e6
Show file tree

Hide file tree

Showing 33 changed files with 132 additions and 255 deletions.
diff --git a/contrib/generic_proxy/filters/network/source/file_access_log.h b/contrib/generic_proxy/filters/network/source/file_access_log.h
@@ -45,7 +45,7 @@ class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase<
   AccessLog::InstanceBaseSharedPtr<Context>
   createAccessLogInstance(const Protobuf::Message& config,
                           AccessLog::FilterBasePtr<Context>&& filter,
-                          Server::Configuration::CommonFactoryContext& context) override {
+                          Server::Configuration::FactoryContext& context) override {
     const auto& typed_config = MessageUtil::downcastAndValidate<
         const envoy::extensions::access_loggers::file::v3::FileAccessLog&>(
         config, context.messageValidationVisitor());
@@ -93,15 +93,8 @@ class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase<
 
     Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, typed_config.path()};
     return std::make_shared<FileAccessLogBase<Context>>(
-        file_info, std::move(filter), std::move(formatter), context.accessLogManager());
-  }
-
-  AccessLog::InstanceBaseSharedPtr<Context> createAccessLogInstance(
-      const Protobuf::Message& config, AccessLog::FilterBasePtr<Context>&& filter,
-      Server::Configuration::ListenerAccessLogFactoryContext& context) override {
-    return createAccessLogInstance(
-        config, std::move(filter),
-        static_cast<Server::Configuration::CommonFactoryContext&>(context));
+        file_info, std::move(filter), std::move(formatter),
+        context.getServerFactoryContext().accessLogManager());
   }
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {

diff --git a/contrib/generic_proxy/filters/network/test/proxy_test.cc b/contrib/generic_proxy/filters/network/test/proxy_test.cc
@@ -102,9 +102,9 @@ class FilterConfigTest : public testing::Test {
         Envoy::Formatter::SubstitutionFormatStringUtils::fromProtoConfig<FormatterContext>(
             sff_config, factory_context_);
 
-    return std::make_shared<FileAccessLog>(Filesystem::FilePathAndType{}, nullptr,
-                                           std::move(formatter),
-                                           factory_context_.accessLogManager());
+    return std::make_shared<FileAccessLog>(
+        Filesystem::FilePathAndType{}, nullptr, std::move(formatter),
+        factory_context_.server_factory_context_.accessLogManager());
   }
 
   std::shared_ptr<NiceMock<Tracing::MockTracer>> tracer_;
@@ -708,7 +708,7 @@ TEST_F(FilterTest, NewStreamAndReplyNormally) {
       }));
 
   EXPECT_CALL(
-      *factory_context_.access_log_manager_.file_,
+      *factory_context_.server_factory_context_.access_log_manager_.file_,
       write("host-value /path-value method-value protocol-value request-value response-value -"));
 
   EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false));
@@ -767,7 +767,7 @@ TEST_F(FilterTest, NewStreamAndReplyNormallyWithMultipleFrames) {
   auto active_stream = filter_->activeStreamsForTest().begin()->get();
 
   EXPECT_CALL(
-      *factory_context_.access_log_manager_.file_,
+      *factory_context_.server_factory_context_.access_log_manager_.file_,
       write("host-value /path-value method-value protocol-value request-value response-value -"));
 
   EXPECT_CALL(factory_context_.drain_manager_, drainClose()).WillOnce(Return(false));

diff --git a/envoy/access_log/access_log_config.h b/envoy/access_log/access_log_config.h
@@ -25,12 +25,12 @@ template <class Context> class ExtensionFilterFactoryBase : public Config::Typed
    * implementation is unable to produce a filter with the provided parameters, it should throw an
    * EnvoyException. The returned pointer should never be nullptr.
    * @param config supplies the custom configuration for this filter type.
-   * @param context supplies the server factory context.
+   * @param context supplies the factory context.
    * @return an instance of extension filter implementation from a config proto.
    */
   virtual FilterBasePtr<Context>
   createFilter(const envoy::config::accesslog::v3::ExtensionFilter& config,
-               Server::Configuration::CommonFactoryContext& context) PURE;
+               Server::Configuration::FactoryContext& context) PURE;
 
   std::string category() const override { return category_; }
 
@@ -74,21 +74,7 @@ template <class Context> class AccessLogInstanceFactoryBase : public Config::Typ
   virtual AccessLog::InstanceBaseSharedPtr<Context>
   createAccessLogInstance(const Protobuf::Message& config,
                           AccessLog::FilterBasePtr<Context>&& filter,
-                          Server::Configuration::ListenerAccessLogFactoryContext& context) PURE;
-
-  /**
-   * Create a particular AccessLog::Instance implementation from a config proto. If the
-   * implementation is unable to produce a factory with the provided parameters, it should throw an
-   * EnvoyException. The returned pointer should never be nullptr.
-   * @param config the custom configuration for this access log type.
-   * @param filter filter to determine whether a particular request should be logged. If no filter
-   * was specified in the configuration, argument will be nullptr.
-   * @param context general filter context through which persistent resources can be accessed.
-   */
-  virtual AccessLog::InstanceBaseSharedPtr<Context>
-  createAccessLogInstance(const Protobuf::Message& config,
-                          AccessLog::FilterBasePtr<Context>&& filter,
-                          Server::Configuration::CommonFactoryContext& context) PURE;
+                          Server::Configuration::FactoryContext& context) PURE;
 
   std::string category() const override { return category_; }
 

diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc
@@ -53,9 +53,9 @@ bool ComparisonFilter::compareAgainstValue(uint64_t lhs) const {
 }
 
 FilterPtr FilterFactory::fromProto(const envoy::config::accesslog::v3::AccessLogFilter& config,
-                                   Server::Configuration::CommonFactoryContext& context) {
-  Runtime::Loader& runtime = context.runtime();
-  Random::RandomGenerator& random = context.api().randomGenerator();
+                                   Server::Configuration::FactoryContext& context) {
+  Runtime::Loader& runtime = context.getServerFactoryContext().runtime();
+  Random::RandomGenerator& random = context.getServerFactoryContext().api().randomGenerator();
   ProtobufMessage::ValidationVisitor& validation_visitor = context.messageValidationVisitor();
   switch (config.filter_specifier_case()) {
   case envoy::config::accesslog::v3::AccessLogFilter::FilterSpecifierCase::kStatusCodeFilter:
@@ -156,7 +156,7 @@ bool RuntimeFilter::evaluate(const Formatter::HttpFormatterContext&,
 
 OperatorFilter::OperatorFilter(
     const Protobuf::RepeatedPtrField<envoy::config::accesslog::v3::AccessLogFilter>& configs,
-    Server::Configuration::CommonFactoryContext& context) {
+    Server::Configuration::FactoryContext& context) {
   for (const auto& config : configs) {
     auto filter = FilterFactory::fromProto(config, context);
     if (filter != nullptr) {
@@ -166,11 +166,11 @@ OperatorFilter::OperatorFilter(
 }
 
 OrFilter::OrFilter(const envoy::config::accesslog::v3::OrFilter& config,
-                   Server::Configuration::CommonFactoryContext& context)
+                   Server::Configuration::FactoryContext& context)
     : OperatorFilter(config.filters(), context) {}
 
 AndFilter::AndFilter(const envoy::config::accesslog::v3::AndFilter& config,
-                     Server::Configuration::CommonFactoryContext& context)
+                     Server::Configuration::FactoryContext& context)
     : OperatorFilter(config.filters(), context) {}
 
 bool OrFilter::evaluate(const Formatter::HttpFormatterContext& context,
@@ -311,11 +311,8 @@ bool MetadataFilter::evaluate(const Formatter::HttpFormatterContext&,
   return default_match_;
 }
 
-namespace {
-
-template <typename FactoryContext>
-InstanceSharedPtr makeAccessLogInstance(const envoy::config::accesslog::v3::AccessLog& config,
-                                        FactoryContext& context) {
+InstanceSharedPtr AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config,
+                                              Server::Configuration::FactoryContext& context) {
   FilterPtr filter;
   if (config.has_filter()) {
     filter = FilterFactory::fromProto(config.filter(), context);
@@ -328,19 +325,5 @@ InstanceSharedPtr makeAccessLogInstance(const envoy::config::accesslog::v3::Acce
   return factory.createAccessLogInstance(*message, std::move(filter), context);
 }
 
-} // namespace
-
-InstanceSharedPtr
-AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config,
-                            Server::Configuration::ListenerAccessLogFactoryContext& context) {
-  return makeAccessLogInstance(config, context);
-}
-
-InstanceSharedPtr
-AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config,
-                            Server::Configuration::CommonFactoryContext& context) {
-  return makeAccessLogInstance(config, context);
-}
-
 } // namespace AccessLog
 } // namespace Envoy
diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h
@@ -35,7 +35,7 @@ class FilterFactory {
    * Read a filter definition from proto and instantiate a concrete filter class.
    */
   static FilterPtr fromProto(const envoy::config::accesslog::v3::AccessLogFilter& config,
-                             Server::Configuration::CommonFactoryContext& context);
+                             Server::Configuration::FactoryContext& context);
 };
 
 /**
@@ -87,7 +87,7 @@ class OperatorFilter : public Filter {
 public:
   OperatorFilter(
       const Protobuf::RepeatedPtrField<envoy::config::accesslog::v3::AccessLogFilter>& configs,
-      Server::Configuration::CommonFactoryContext& context);
+      Server::Configuration::FactoryContext& context);
 
 protected:
   std::vector<FilterPtr> filters_;
@@ -99,7 +99,7 @@ class OperatorFilter : public Filter {
 class AndFilter : public OperatorFilter {
 public:
   AndFilter(const envoy::config::accesslog::v3::AndFilter& config,
-            Server::Configuration::CommonFactoryContext& context);
+            Server::Configuration::FactoryContext& context);
 
   // AccessLog::Filter
   bool evaluate(const Formatter::HttpFormatterContext& context,
@@ -112,7 +112,7 @@ class AndFilter : public OperatorFilter {
 class OrFilter : public OperatorFilter {
 public:
   OrFilter(const envoy::config::accesslog::v3::OrFilter& config,
-           Server::Configuration::CommonFactoryContext& context);
+           Server::Configuration::FactoryContext& context);
 
   // AccessLog::Filter
   bool evaluate(const Formatter::HttpFormatterContext& context,
@@ -265,16 +265,8 @@ class AccessLogFactory {
    * Read a filter definition from proto and instantiate an Instance. This method is used
    * to create access log instances that need access to listener properties.
    */
-  static InstanceSharedPtr
-  fromProto(const envoy::config::accesslog::v3::AccessLog& config,
-            Server::Configuration::ListenerAccessLogFactoryContext& context);
-
-  /**
-   * Read a filter definition from proto and instantiate an Instance. This method does not
-   * have access to listener properties, for example for access loggers of admin interface.
-   */
   static InstanceSharedPtr fromProto(const envoy::config::accesslog::v3::AccessLog& config,
-                                     Server::Configuration::CommonFactoryContext& context);
+                                     Server::Configuration::FactoryContext& context);
 
   /**
    * Template method to create an access log filter from proto configuration for non-HTTP access
@@ -283,7 +275,7 @@ class AccessLogFactory {
   template <class Context>
   static FilterBasePtr<Context>
   accessLogFilterFromProto(const envoy::config::accesslog::v3::AccessLogFilter& config,
-                           Server::Configuration::CommonFactoryContext& context) {
+                           Server::Configuration::FactoryContext& context) {
     if (!config.has_extension_filter()) {
       ExceptionUtil::throwEnvoyException(
           "Access log filter: only extension filter is supported by non-HTTP access loggers.");
@@ -301,7 +293,7 @@ class AccessLogFactory {
   template <class Context>
   static InstanceBaseSharedPtr<Context>
   accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config,
-                        Server::Configuration::CommonFactoryContext& context) {
+                        Server::Configuration::FactoryContext& context) {
     FilterBasePtr<Context> filter;
     if (config.has_filter()) {
       filter = accessLogFilterFromProto<Context>(config.filter(), context);

diff --git a/source/extensions/access_loggers/common/stream_access_log_common_impl.h b/source/extensions/access_loggers/common/stream_access_log_common_impl.h
@@ -13,7 +13,7 @@ namespace AccessLoggers {
 template <class T, Filesystem::DestinationType destination_type>
 AccessLog::InstanceSharedPtr
 createStreamAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-                              Server::Configuration::CommonFactoryContext& context) {
+                              Server::Configuration::FactoryContext& context) {
   const auto& fal_config =
       MessageUtil::downcastAndValidate<const T&>(config, context.messageValidationVisitor());
   Formatter::FormatterPtr formatter;
@@ -26,7 +26,8 @@ createStreamAccessLogInstance(const Protobuf::Message& config, AccessLog::Filter
   }
   Filesystem::FilePathAndType file_info{destination_type, ""};
   return std::make_shared<AccessLoggers::File::FileAccessLog>(
-      file_info, std::move(filter), std::move(formatter), context.accessLogManager());
+      file_info, std::move(filter), std::move(formatter),
+      context.getServerFactoryContext().accessLogManager());
 }
 
 } // namespace AccessLoggers

diff --git a/source/extensions/access_loggers/file/config.cc b/source/extensions/access_loggers/file/config.cc
@@ -19,17 +19,10 @@ namespace Extensions {
 namespace AccessLoggers {
 namespace File {
 
-AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance(
-    const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-    Server::Configuration::ListenerAccessLogFactoryContext& context) {
-  return createAccessLogInstance(
-      config, std::move(filter),
-      static_cast<Server::Configuration::CommonFactoryContext&>(context));
-}
-
-AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance(
-    const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-    Server::Configuration::CommonFactoryContext& context) {
+AccessLog::InstanceSharedPtr
+FileAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
+                                              AccessLog::FilterPtr&& filter,
+                                              Server::Configuration::FactoryContext& context) {
   const auto& fal_config = MessageUtil::downcastAndValidate<
       const envoy::extensions::access_loggers::file::v3::FileAccessLog&>(
       config, context.messageValidationVisitor());
@@ -68,7 +61,7 @@ AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance(
 
   Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, fal_config.path()};
   return std::make_shared<FileAccessLog>(file_info, std::move(filter), std::move(formatter),
-                                         context.accessLogManager());
+                                         context.getServerFactoryContext().accessLogManager());
 }
 
 ProtobufTypes::MessagePtr FileAccessLogFactory::createEmptyConfigProto() {

diff --git a/source/extensions/access_loggers/file/config.h b/source/extensions/access_loggers/file/config.h
@@ -14,11 +14,7 @@ class FileAccessLogFactory : public AccessLog::AccessLogInstanceFactory {
 public:
   AccessLog::InstanceSharedPtr
   createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-                          Server::Configuration::ListenerAccessLogFactoryContext& context) override;
-
-  AccessLog::InstanceSharedPtr
-  createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-                          Server::Configuration::CommonFactoryContext& context) override;
+                          Server::Configuration::FactoryContext& context) override;
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override;
 

diff --git a/source/extensions/access_loggers/filters/cel/config.cc b/source/extensions/access_loggers/filters/cel/config.cc
@@ -16,7 +16,7 @@ namespace CEL {
 
 Envoy::AccessLog::FilterPtr CELAccessLogExtensionFilterFactory::createFilter(
     const envoy::config::accesslog::v3::ExtensionFilter& config,
-    Server::Configuration::CommonFactoryContext& context) {
+    Server::Configuration::FactoryContext& context) {
 
   auto factory_config =
       Config::Utility::translateToFactoryConfig(config, context.messageValidationVisitor(), *this);
@@ -33,7 +33,8 @@ Envoy::AccessLog::FilterPtr CELAccessLogExtensionFilterFactory::createFilter(
   }
 
   return std::make_unique<CELAccessLogExtensionFilter>(
-      Extensions::Filters::Common::Expr::getBuilder(context), parse_status.value().expr());
+      Extensions::Filters::Common::Expr::getBuilder(context.getServerFactoryContext()),
+      parse_status.value().expr());
 #else
   throw EnvoyException("CEL is not available for use in this environment.");
 #endif

diff --git a/source/extensions/access_loggers/filters/cel/config.h b/source/extensions/access_loggers/filters/cel/config.h
@@ -22,7 +22,7 @@ class CELAccessLogExtensionFilterFactory : public Envoy::AccessLog::ExtensionFil
 public:
   Envoy::AccessLog::FilterPtr
   createFilter(const envoy::config::accesslog::v3::ExtensionFilter& config,
-               Server::Configuration::CommonFactoryContext& context) override;
+               Server::Configuration::FactoryContext& context) override;
   ProtobufTypes::MessagePtr createEmptyConfigProto() override;
   std::string name() const override { return "envoy.access_loggers.extension_filters.cel"; }
 };

diff --git a/source/extensions/access_loggers/grpc/http_config.cc b/source/extensions/access_loggers/grpc/http_config.cc
@@ -18,26 +18,19 @@ namespace Extensions {
 namespace AccessLoggers {
 namespace HttpGrpc {
 
-AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance(
-    const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-    Server::Configuration::ListenerAccessLogFactoryContext& context) {
-  return createAccessLogInstance(
-      config, std::move(filter),
-      static_cast<Server::Configuration::CommonFactoryContext&>(context));
-}
-
-AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance(
-    const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-    Server::Configuration::CommonFactoryContext& context) {
+AccessLog::InstanceSharedPtr
+HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config,
+                                                  AccessLog::FilterPtr&& filter,
+                                                  Server::Configuration::FactoryContext& context) {
   GrpcCommon::validateProtoDescriptors();
 
   const auto& proto_config = MessageUtil::downcastAndValidate<
       const envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig&>(
       config, context.messageValidationVisitor());
 
   return std::make_shared<HttpGrpcAccessLog>(
-      std::move(filter), proto_config, context.threadLocal(),
-      GrpcCommon::getGrpcAccessLoggerCacheSingleton(context));
+      std::move(filter), proto_config, context.getServerFactoryContext().threadLocal(),
+      GrpcCommon::getGrpcAccessLoggerCacheSingleton(context.getServerFactoryContext()));
 }
 
 ProtobufTypes::MessagePtr HttpGrpcAccessLogFactory::createEmptyConfigProto() {

diff --git a/source/extensions/access_loggers/grpc/http_config.h b/source/extensions/access_loggers/grpc/http_config.h
@@ -16,10 +16,7 @@ class HttpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory {
 public:
   AccessLog::InstanceSharedPtr
   createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-                          Server::Configuration::ListenerAccessLogFactoryContext& context) override;
-  AccessLog::InstanceSharedPtr
-  createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter,
-                          Server::Configuration::CommonFactoryContext& context) override;
+                          Server::Configuration::FactoryContext& context) override;
 
   ProtobufTypes::MessagePtr createEmptyConfigProto() override;