From 27c8585e82122ca9d7d877ecef5c308cb953f3fe Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Thu, 23 Jan 2025 13:38:14 +0000 Subject: [PATCH 01/11] generic proxy: non template formatter/logger support Signed-off-by: wangbaiping(wbpcode) --- .../generic_proxy/v3/generic_proxy.proto | 11 ++ envoy/formatter/http_formatter_context.h | 35 ++++ .../filters/network/generic_proxy/BUILD | 23 +-- .../network/generic_proxy/access_log.cc | 156 +++++++++++++----- .../network/generic_proxy/access_log.h | 47 +++--- .../filters/network/generic_proxy/config.cc | 37 +---- .../network/generic_proxy/file_access_log.h | 121 -------------- .../filters/network/generic_proxy/proxy.cc | 7 +- .../filters/network/generic_proxy/proxy.h | 9 +- .../network/generic_proxy/proxy_config.h | 3 +- .../filters/network/generic_proxy/BUILD | 4 + .../network/generic_proxy/access_log_test.cc | 137 +++++++++------ .../network/generic_proxy/config_test.cc | 2 +- .../network/generic_proxy/fake_codec.h | 12 +- .../network/generic_proxy/proxy_test.cc | 26 +-- 15 files changed, 316 insertions(+), 314 deletions(-) delete mode 100644 source/extensions/filters/network/generic_proxy/file_access_log.h diff --git a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto index 9ae2560470cf2..ebae479a44ab0 100644 --- a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto +++ b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto @@ -65,3 +65,14 @@ message GenericRds { // Envoy configuration with multiple generic proxies to use different route configurations. string route_config_name = 2 [(validate.rules).string = {min_len: 1}]; } + +// Generic proxy log formatter which could be configured at +// the :ref:`formatters ` +// extension point for generic proxy specific custom formatters (command operators) support. +// +// .. note:: +// +// If the ``envoy.access_loggers.file`` logger is used for generic proxy, this formatter will +// be enabled by default. +message GenericProxyLogFormatter { +} diff --git a/envoy/formatter/http_formatter_context.h b/envoy/formatter/http_formatter_context.h index 2ba9713eff3da..29897de6f4de8 100644 --- a/envoy/formatter/http_formatter_context.h +++ b/envoy/formatter/http_formatter_context.h @@ -17,6 +17,16 @@ using AccessLogType = envoy::data::accesslog::v3::AccessLogType; */ class HttpFormatterContext { public: + /** + * Interface for a context extension which can be used to provide non-HTTP specific data to + * formatters. This could be used for non-HTTP protocols to provide protocol specific data to + * formatters. + */ + class Extension { + public: + virtual ~Extension() = default; + }; + /** * Constructor that uses the provided request/response headers, response trailers, local reply * body, and access log type. Any of the parameters can be nullptr/empty. @@ -135,6 +145,28 @@ class HttpFormatterContext { */ static constexpr absl::string_view category() { return "http"; } + /** + * Set the context extension. + * @param extension supplies the context extension. + */ + HttpFormatterContext& setExtension(const Extension& extension) { + extension_ = extension; + return *this; + } + + /** + * @return OptRef the context extension. + */ + OptRef extension() const { return extension_; } + + /** + * @return OptRef the context extension casted to the specified type. + */ + template OptRef typedExtension() const { + const Type* typed_extension = dynamic_cast(extension_.ptr()); + return makeOptRefFromPtr(typed_extension); + } + private: const Http::RequestHeaderMap* request_headers_{}; const Http::ResponseHeaderMap* response_headers_{}; @@ -142,7 +174,10 @@ class HttpFormatterContext { absl::string_view local_reply_body_{}; AccessLogType log_type_{AccessLogType::NotSet}; const Tracing::Span* active_span_ = nullptr; + OptRef extension_; }; +using Context = HttpFormatterContext; + } // namespace Formatter } // namespace Envoy diff --git a/source/extensions/filters/network/generic_proxy/BUILD b/source/extensions/filters/network/generic_proxy/BUILD index c4759e9bf8f25..7a81689cea4b2 100644 --- a/source/extensions/filters/network/generic_proxy/BUILD +++ b/source/extensions/filters/network/generic_proxy/BUILD @@ -23,11 +23,13 @@ envoy_cc_library( ":route_lib", ":stats_lib", ":tracing_lib", + "//envoy/access_log:access_log_interface", "//envoy/network:filter_interface", "//envoy/server:factory_context_interface", "//envoy/stats:timespan_interface", "//source/common/common:linked_object", "//source/common/common:minimal_logger_lib", + "//source/common/formatter:substitution_formatter_lib", "//source/common/stats:timespan_lib", "//source/common/stream_info:stream_info_lib", "//source/common/tracing:tracer_config_lib", @@ -146,20 +148,6 @@ envoy_cc_library( ], ) -envoy_cc_library( - name = "file_access_log_lib", - hdrs = [ - "file_access_log.h", - ], - deps = [ - "//envoy/access_log:access_log_config_interface", - "//envoy/access_log:access_log_interface", - "//source/common/common:utility_lib", - "//source/common/formatter:substitution_format_string_lib", - "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto", - ], -) - envoy_cc_library( name = "access_log_lib", srcs = [ @@ -169,8 +157,12 @@ envoy_cc_library( "access_log.h", ], deps = [ - ":file_access_log_lib", + "//envoy/access_log:access_log_config_interface", + "//envoy/formatter:substitution_formatter_interface", + "//source/common/config:utility_lib", "//source/extensions/filters/network/generic_proxy/interface:stream_interface", + "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/network/generic_proxy/v3:pkg_cc_proto", ], # Ensure this factory in the source is always linked in. alwayslink = 1, @@ -231,6 +223,7 @@ envoy_cc_library( ], deps = [ ":route_interface", + "//envoy/access_log:access_log_config_interface", "//envoy/tracing:trace_config_interface", "//envoy/tracing:tracer_interface", "//source/extensions/filters/network/generic_proxy:access_log_lib", diff --git a/source/extensions/filters/network/generic_proxy/access_log.cc b/source/extensions/filters/network/generic_proxy/access_log.cc index a96df953494d8..07735fcba15ad 100644 --- a/source/extensions/filters/network/generic_proxy/access_log.cc +++ b/source/extensions/filters/network/generic_proxy/access_log.cc @@ -1,7 +1,10 @@ #include "source/extensions/filters/network/generic_proxy/access_log.h" +#include "envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.pb.h" #include "envoy/registry/registry.h" +#include "source/common/config/utility.h" + namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -29,26 +32,20 @@ ProtobufWkt::Value StringValueFormatterProvider::formatValueWithContext( absl::optional GenericStatusCodeFormatterProvider::formatWithContext(const FormatterContext& context, const StreamInfo::StreamInfo&) const { - if (context.response_ == nullptr) { - return absl::nullopt; - } - - const int code = context.response_->status().code(); + CHECK_DATA_OR_RETURN(context, response_, absl::nullopt); + const int code = checked_data->response_->status().code(); return std::to_string(code); } ProtobufWkt::Value GenericStatusCodeFormatterProvider::formatValueWithContext(const FormatterContext& context, const StreamInfo::StreamInfo&) const { - if (context.response_ == nullptr) { - return ValueUtil::nullValue(); - } - - const int code = context.response_->status().code(); + CHECK_DATA_OR_RETURN(context, response_, ValueUtil::nullValue()); + const int code = checked_data->response_->status().code(); return ValueUtil::numberValue(code); } -class SimpleCommandParser : public CommandParser { +class GenericProxyCommandParser : public Formatter::CommandParser { public: using ProviderFunc = std::function max_length)>; @@ -75,10 +72,8 @@ class SimpleCommandParser : public CommandParser { return std::make_unique( [](const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (context.request_) { - return std::string(context.request_->method()); - } - return absl::nullopt; + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + return std::string(checked_data->request_->method()); }); }}, {"HOST", @@ -86,10 +81,8 @@ class SimpleCommandParser : public CommandParser { return std::make_unique( [](const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (context.request_) { - return std::string(context.request_->host()); - } - return absl::nullopt; + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + return std::string(checked_data->request_->host()); }); }}, {"PATH", @@ -97,10 +90,8 @@ class SimpleCommandParser : public CommandParser { return std::make_unique( [](const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (context.request_) { - return std::string(context.request_->path()); - } - return absl::nullopt; + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + return std::string(checked_data->request_->path()); }); }}, {"PROTOCOL", @@ -108,10 +99,8 @@ class SimpleCommandParser : public CommandParser { return std::make_unique( [](const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (context.request_) { - return std::string(context.request_->protocol()); - } - return absl::nullopt; + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + return std::string(checked_data->request_->protocol()); }); }}, {"REQUEST_PROPERTY", @@ -120,10 +109,9 @@ class SimpleCommandParser : public CommandParser { [key = std::string(command_arg)]( const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (!context.request_) { - return absl::nullopt; - } - auto optional_view = context.request_->get(key); + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + + auto optional_view = checked_data->request_->get(key); if (!optional_view.has_value()) { return absl::nullopt; } @@ -136,10 +124,8 @@ class SimpleCommandParser : public CommandParser { [key = std::string(command_arg)]( const FormatterContext& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (!context.response_) { - return absl::nullopt; - } - auto optional_view = context.response_->get(key); + CHECK_DATA_OR_RETURN(context, response_, absl::nullopt); + auto optional_view = checked_data->response_->get(key); if (!optional_view.has_value()) { return absl::nullopt; } @@ -154,18 +140,102 @@ class SimpleCommandParser : public CommandParser { } }; -class DefaultBuiltInCommandParserFactory : public BuiltInCommandParserFactory { -public: - std::string name() const override { return "envoy.built_in_formatters.generic_poxy.default"; } +Formatter::CommandParserPtr createGenericProxyCommandParser() { + return std::make_unique(); +} - // BuiltInCommandParserFactory - CommandParserPtr createCommandParser() const override { - return std::make_unique(); +class GenericProxyCommandParserFactory : public Formatter::CommandParserFactory { +public: + // CommandParserFactory + Formatter::CommandParserPtr + createCommandParserFromProto(const Protobuf::Message&, + Server::Configuration::GenericFactoryContext&) override { + return createGenericProxyCommandParser(); + } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::filters::network::generic_proxy::v3::GenericProxyLogFormatter>(); } + + std::string name() const override { return "envoy.formatters.generic_proxy_commands"; } }; -REGISTER_FACTORY(DefaultBuiltInCommandParserFactory, BuiltInCommandParserFactory); -REGISTER_FACTORY(FileAccessLogFactory, AccessLogInstanceFactory); +REGISTER_FACTORY(GenericProxyCommandParserFactory, Formatter::CommandParserFactory); + +absl::Status initializeFormatterForFileLogger( + envoy::extensions::access_loggers::file::v3::FileAccessLog& config) { + switch (config.access_log_format_case()) { + case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kFormat: { + std::string legacy_format = config.format(); + *config.mutable_log_format()->mutable_text_format_source()->mutable_inline_string() = + legacy_format; + break; + } + case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: + kJsonFormat: { + ProtobufWkt::Struct json_format = config.json_format(); + *config.mutable_log_format()->mutable_json_format() = std::move(json_format); + break; + } + case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: + kTypedJsonFormat: { + ProtobufWkt::Struct json_format = config.typed_json_format(); + *config.mutable_log_format()->mutable_json_format() = std::move(json_format); + break; + } + case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kLogFormat: + break; + case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: + ACCESS_LOG_FORMAT_NOT_SET: + return absl::InvalidArgumentError( + "Access log: no format and no default format for file access log"); + } + + envoy::extensions::filters::network::generic_proxy::v3::GenericProxyLogFormatter formatter; + auto* formatter_extension = config.mutable_log_format()->mutable_formatters()->Add(); + formatter_extension->set_name("envoy.formatters.generic_proxy_commands"); + formatter_extension->mutable_typed_config()->PackFrom(formatter); + return absl::OkStatus(); +} + +AccessLog::FilterPtr +accessLogFilterFromProto(const envoy::config::accesslog::v3::AccessLogFilter& config, + Server::Configuration::FactoryContext& context) { + if (!config.has_extension_filter()) { + ExceptionUtil::throwEnvoyException( + "Access log filter: only extension filter is supported by non-HTTP access loggers."); + } + + auto& factory = Config::Utility::getAndCheckFactory( + config.extension_filter()); + return factory.createFilter(config.extension_filter(), context); +} + +AccessLog::InstanceSharedPtr +accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config, + Server::Configuration::FactoryContext& context) { + AccessLog::FilterPtr filter; + if (config.has_filter()) { + filter = accessLogFilterFromProto(config.filter(), context); + } + + auto& factory = + Config::Utility::getAndCheckFactory(config); + ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( + config, context.messageValidationVisitor(), factory); + + // Setup the generic proxy formatter for file access loggers by default. This is necessary + // for backwards compatibility. + if (factory.name() == "envoy.access_loggers.file") { + auto* file_config = + dynamic_cast(message.get()); + ASSERT(file_config != nullptr); + THROW_IF_NOT_OK(initializeFormatterForFileLogger(*file_config)); + } + + // Inject the command parsers from the generic proxy. + return factory.createAccessLogInstance(*message, std::move(filter), context); +} } // namespace GenericProxy } // namespace NetworkFilters diff --git a/source/extensions/filters/network/generic_proxy/access_log.h b/source/extensions/filters/network/generic_proxy/access_log.h index 99c69fc567f65..9917e4a48bc50 100644 --- a/source/extensions/filters/network/generic_proxy/access_log.h +++ b/source/extensions/filters/network/generic_proxy/access_log.h @@ -1,6 +1,9 @@ #pragma once -#include "source/extensions/filters/network/generic_proxy/file_access_log.h" +#include "envoy/access_log/access_log_config.h" +#include "envoy/extensions/access_loggers/file/v3/file.pb.h" +#include "envoy/formatter/substitution_formatter.h" + #include "source/extensions/filters/network/generic_proxy/interface/stream.h" namespace Envoy { @@ -8,34 +11,24 @@ namespace Extensions { namespace NetworkFilters { namespace GenericProxy { -struct FormatterContext { +struct FormatterContextExtension : public Formatter::Context::Extension { + FormatterContextExtension(const RequestHeaderFrame* request, const ResponseHeaderFrame* response) + : request_(request), response_(response) {} + FormatterContextExtension() = default; + const RequestHeaderFrame* request_{}; const ResponseHeaderFrame* response_{}; - - static constexpr absl::string_view category() { return "generic_proxy"; } }; -// Formatter for generic proxy. -using Formatter = Formatter::FormatterBase; -using FormatterProvider = Envoy::Formatter::FormatterProviderBase; +#define CHECK_DATA_OR_RETURN(context, field, return_value) \ + const auto checked_data = context.typedExtension(); \ + if (!checked_data.has_value() || checked_data->field == nullptr) { \ + return return_value; \ + } + +using FormatterProvider = Formatter::FormatterProvider; using FormatterProviderPtr = std::unique_ptr; -using CommandParser = Envoy::Formatter::CommandParserBase; -using CommandParserPtr = Envoy::Formatter::CommandParserBasePtr; -using CommandParserFactory = Envoy::Formatter::CommandParserFactoryBase; -using BuiltInCommandParserFactory = - Envoy::Formatter::BuiltInCommandParserFactoryBase; - -// Access log for generic proxy. -using AccessLogFilter = AccessLog::FilterBase; -using AccessLogFilterPtr = std::unique_ptr; -using AccessLogFilterFactory = AccessLog::ExtensionFilterFactoryBase; -using AccessLogInstance = AccessLog::InstanceBase; -using AccessLogInstanceSharedPtr = std::shared_ptr; -using AccessLogInstanceFactory = AccessLog::AccessLogInstanceFactoryBase; - -// File access log for generic proxy. -using FileAccessLog = FileAccessLogBase; -using FileAccessLogFactory = FileAccessLogFactoryBase; +using FormatterContext = Formatter::Context; class StringValueFormatterProvider : public FormatterProvider { public: @@ -69,6 +62,12 @@ class GenericStatusCodeFormatterProvider : public FormatterProvider { const StreamInfo::StreamInfo&) const override; }; +Formatter::CommandParserPtr createGenericProxyCommandParser(); + +AccessLog::InstanceSharedPtr +accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config, + Server::Configuration::FactoryContext& context); + } // namespace GenericProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/source/extensions/filters/network/generic_proxy/config.cc b/source/extensions/filters/network/generic_proxy/config.cc index 5daaf4fab5975..41e774c708953 100644 --- a/source/extensions/filters/network/generic_proxy/config.cc +++ b/source/extensions/filters/network/generic_proxy/config.cc @@ -12,38 +12,6 @@ namespace Extensions { namespace NetworkFilters { namespace GenericProxy { -template -static AccessLog::FilterBasePtr -accessLogFilterFromProto(const envoy::config::accesslog::v3::AccessLogFilter& config, - Server::Configuration::FactoryContext& context) { - if (!config.has_extension_filter()) { - ExceptionUtil::throwEnvoyException( - "Access log filter: only extension filter is supported by non-HTTP access loggers."); - } - - auto& factory = - Config::Utility::getAndCheckFactory>( - config.extension_filter()); - return factory.createFilter(config.extension_filter(), context); -} - -template -static AccessLog::InstanceBaseSharedPtr -accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config, - Server::Configuration::FactoryContext& context) { - AccessLog::FilterBasePtr filter; - if (config.has_filter()) { - filter = accessLogFilterFromProto(config.filter(), context); - } - - auto& factory = - Config::Utility::getAndCheckFactory>( - config); - ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( - config, context.messageValidationVisitor(), factory); - - return factory.createAccessLogInstance(*message, std::move(filter), context); -} SINGLETON_MANAGER_REGISTRATION(generic_route_config_provider_manager); SINGLETON_MANAGER_REGISTRATION(generic_proxy_code_or_flag_stats); @@ -156,10 +124,9 @@ Factory::createFilterFactoryFromProtoTyped(const ProxyConfig& proto_config, } // Access log configuration. - std::vector access_logs; + std::vector access_logs; for (const auto& access_log : proto_config.access_log()) { - AccessLogInstanceSharedPtr current_access_log = - accessLoggerFromProto(access_log, context); + AccessLog::InstanceSharedPtr current_access_log = accessLoggerFromProto(access_log, context); access_logs.push_back(current_access_log); } diff --git a/source/extensions/filters/network/generic_proxy/file_access_log.h b/source/extensions/filters/network/generic_proxy/file_access_log.h deleted file mode 100644 index 30eabb239ea78..0000000000000 --- a/source/extensions/filters/network/generic_proxy/file_access_log.h +++ /dev/null @@ -1,121 +0,0 @@ -#pragma once - -#include "envoy/access_log/access_log.h" -#include "envoy/access_log/access_log_config.h" -#include "envoy/extensions/access_loggers/file/v3/file.pb.h" -#include "envoy/extensions/access_loggers/file/v3/file.pb.validate.h" - -#include "source/common/common/utility.h" -#include "source/common/formatter/substitution_format_string.h" - -namespace Envoy { -namespace Extensions { -namespace NetworkFilters { -namespace GenericProxy { - -template class FileAccessLogBase : public AccessLog::InstanceBase { -public: - FileAccessLogBase(const Filesystem::FilePathAndType& access_log_file_info, - AccessLog::FilterBasePtr&& filter, - Formatter::FormatterBasePtr&& formatter, - AccessLog::AccessLogManager& log_manager) - : filter_(std::move(filter)), formatter_(std::move(formatter)) { - log_file_ = log_manager.createAccessLog(access_log_file_info).value(); - } - - void log(const Context& context, const StreamInfo::StreamInfo& stream_info) override { - if (filter_ != nullptr && !filter_->evaluate(context, stream_info)) { - return; - } - log_file_->write(formatter_->formatWithContext(context, stream_info)); - } - -private: - AccessLog::AccessLogFileSharedPtr log_file_; - AccessLog::FilterBasePtr filter_; - Formatter::FormatterBasePtr formatter_; -}; - -template -class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase { -public: - FileAccessLogFactoryBase() - : name_(fmt::format("envoy.{}.access_loggers.file", Context::category())) {} - - AccessLog::InstanceBaseSharedPtr - createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterBasePtr&& filter, - Server::Configuration::FactoryContext& context) override { - const auto& typed_config = MessageUtil::downcastAndValidate< - const envoy::extensions::access_loggers::file::v3::FileAccessLog&>( - config, context.messageValidationVisitor()); - - Formatter::FormatterBasePtr formatter; - - switch (typed_config.access_log_format_case()) { - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kFormat: - if (typed_config.format().empty()) { - formatter = getDefaultFormatter(); - } else { - envoy::config::core::v3::SubstitutionFormatString sff_config; - sff_config.mutable_text_format_source()->set_inline_string(typed_config.format()); - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), - Formatter::FormatterBasePtr); - } - break; - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - kJsonFormat: - formatter = Formatter::SubstitutionFormatStringUtils::createJsonFormatter( - typed_config.json_format(), false, false, false); - break; - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - kTypedJsonFormat: { - envoy::config::core::v3::SubstitutionFormatString sff_config; - *sff_config.mutable_json_format() = typed_config.typed_json_format(); - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), - Formatter::FormatterBasePtr); - break; - } - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - kLogFormat: - formatter = - THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( - typed_config.log_format(), context), - Formatter::FormatterBasePtr); - break; - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - ACCESS_LOG_FORMAT_NOT_SET: - formatter = getDefaultFormatter(); - break; - } - if (formatter == nullptr) { - ExceptionUtil::throwEnvoyException( - "Access log: no format and no default format for file access log"); - } - - Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, typed_config.path()}; - return std::make_shared>( - file_info, std::move(filter), std::move(formatter), - context.serverFactoryContext().accessLogManager()); - } - - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return ProtobufTypes::MessagePtr{ - new envoy::extensions::access_loggers::file::v3::FileAccessLog()}; - } - - std::string name() const override { return name_; } - -protected: - virtual Formatter::FormatterBasePtr getDefaultFormatter() const { return nullptr; } - -private: - const std::string name_; -}; - -} // namespace GenericProxy -} // namespace NetworkFilters -} // namespace Extensions -} // namespace Envoy diff --git a/source/extensions/filters/network/generic_proxy/proxy.cc b/source/extensions/filters/network/generic_proxy/proxy.cc index fad6c85444dbc..9edccec994006 100644 --- a/source/extensions/filters/network/generic_proxy/proxy.cc +++ b/source/extensions/filters/network/generic_proxy/proxy.cc @@ -6,6 +6,7 @@ #include "envoy/network/connection.h" #include "source/common/config/utility.h" +#include "source/common/formatter/substitution_formatter.h" #include "source/common/protobuf/protobuf.h" #include "source/common/stream_info/stream_info_impl.h" #include "source/extensions/filters/network/generic_proxy/interface/filter.h" @@ -625,8 +626,10 @@ void ActiveStream::completeStream(absl::optional re } for (const auto& access_log : parent_.config_->accessLogs()) { - const FormatterContext context{request_header_frame_.get(), response_header_frame_.get()}; - access_log->log(context, stream_info_); + const FormatterContextExtension context_extension(request_header_frame_.get(), + response_header_frame_.get()); + Formatter::Context context; + access_log->log(context.setExtension(context_extension), stream_info_); } // TODO(wbpcode): use ranges to simplify the code. diff --git a/source/extensions/filters/network/generic_proxy/proxy.h b/source/extensions/filters/network/generic_proxy/proxy.h index 27c1f63b9d587..4a80b98aa30ac 100644 --- a/source/extensions/filters/network/generic_proxy/proxy.h +++ b/source/extensions/filters/network/generic_proxy/proxy.h @@ -4,6 +4,7 @@ #include #include +#include "envoy/access_log/access_log.h" #include "envoy/config/core/v3/extension.pb.h" #include "envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.pb.h" #include "envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.pb.validate.h" @@ -63,7 +64,7 @@ class FilterConfigImpl : public FilterConfig { Rds::RouteConfigProviderSharedPtr route_config_provider, std::vector factories, Tracing::TracerSharedPtr tracer, Tracing::ConnectionManagerTracingConfigPtr tracing_config, - std::vector&& access_logs, + AccessLog::InstanceSharedPtrVector&& access_logs, const CodeOrFlags& code_or_flags, Envoy::Server::Configuration::FactoryContext& context) : stat_prefix_(stat_prefix), @@ -89,9 +90,7 @@ class FilterConfigImpl : public FilterConfig { } GenericFilterStats& stats() override { return stats_; } const CodeOrFlags& codeOrFlags() const override { return code_or_flags_; } - const std::vector& accessLogs() const override { - return access_logs_; - } + const AccessLog::InstanceSharedPtrVector& accessLogs() const override { return access_logs_; } // FilterChainFactory void createFilterChain(FilterChainManager& manager) override { @@ -118,7 +117,7 @@ class FilterConfigImpl : public FilterConfig { Tracing::TracerSharedPtr tracer_; Tracing::ConnectionManagerTracingConfigPtr tracing_config_; - std::vector access_logs_; + std::vector access_logs_; TimeSource& time_source_; }; diff --git a/source/extensions/filters/network/generic_proxy/proxy_config.h b/source/extensions/filters/network/generic_proxy/proxy_config.h index a91a35068dd67..5b754c9bfec69 100644 --- a/source/extensions/filters/network/generic_proxy/proxy_config.h +++ b/source/extensions/filters/network/generic_proxy/proxy_config.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/access_log/access_log_config.h" #include "envoy/tracing/trace_config.h" #include "envoy/tracing/tracer.h" @@ -62,7 +63,7 @@ class FilterConfig : public FilterChainFactory { /** * @return const std::vector& access logs. */ - virtual const std::vector& accessLogs() const PURE; + virtual const AccessLog::InstanceSharedPtrVector& accessLogs() const PURE; }; } // namespace GenericProxy diff --git a/test/extensions/filters/network/generic_proxy/BUILD b/test/extensions/filters/network/generic_proxy/BUILD index ac070b2c8b372..a9899b8ff6950 100644 --- a/test/extensions/filters/network/generic_proxy/BUILD +++ b/test/extensions/filters/network/generic_proxy/BUILD @@ -51,6 +51,8 @@ envoy_cc_test( ":fake_codec_lib", "//source/common/buffer:buffer_lib", "//source/common/formatter:formatter_extension_lib", + "//source/common/formatter:substitution_format_string_lib", + "//source/extensions/access_loggers/file:config", "//source/extensions/filters/network/generic_proxy:proxy_lib", "//test/extensions/filters/network/generic_proxy/mocks:codec_mocks", "//test/extensions/filters/network/generic_proxy/mocks:filter_mocks", @@ -70,6 +72,7 @@ envoy_cc_test( deps = [ ":fake_codec_lib", "//source/common/buffer:buffer_lib", + "//source/extensions/access_loggers/file:config", "//source/extensions/filters/network/generic_proxy:config", "//source/extensions/filters/network/generic_proxy/router:config", "//source/extensions/tracers/zipkin:config", @@ -140,6 +143,7 @@ envoy_cc_test( rbe_pool = "6gig", deps = [ ":fake_codec_lib", + "//source/common/formatter:substitution_formatter_lib", "//source/extensions/filters/network/generic_proxy:access_log_lib", "//test/mocks/stream_info:stream_info_mocks", ], diff --git a/test/extensions/filters/network/generic_proxy/access_log_test.cc b/test/extensions/filters/network/generic_proxy/access_log_test.cc index 04426bdb0667a..1c694cdb448d1 100644 --- a/test/extensions/filters/network/generic_proxy/access_log_test.cc +++ b/test/extensions/filters/network/generic_proxy/access_log_test.cc @@ -1,3 +1,4 @@ +#include "source/common/formatter/substitution_formatter.h" #include "source/extensions/filters/network/generic_proxy/access_log.h" #include "test/extensions/filters/network/generic_proxy/fake_codec.h" @@ -12,165 +13,203 @@ namespace GenericProxy { namespace { TEST(GenericStatusCodeFormatterProviderTest, GenericStatusCodeFormatterProviderTest) { - FormatterContext context; + FormatterContextExtension context; GenericStatusCodeFormatterProvider formatter; StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter.formatWithContext(context, stream_info), absl::nullopt); - EXPECT_TRUE(formatter.formatValueWithContext(context, stream_info).has_null_value()); + EXPECT_EQ(formatter.formatWithContext(Formatter::Context().setExtension(context), stream_info), + absl::nullopt); + EXPECT_TRUE( + formatter.formatValueWithContext(Formatter::Context().setExtension(context), stream_info) + .has_null_value()); FakeStreamCodecFactory::FakeResponse response; response.status_ = {1234, false}; context.response_ = &response; - EXPECT_EQ(formatter.formatWithContext(context, stream_info).value(), "1234"); - EXPECT_EQ(formatter.formatValueWithContext(context, stream_info).number_value(), 1234.0); + EXPECT_EQ( + formatter.formatWithContext(Formatter::Context().setExtension(context), stream_info).value(), + "1234"); + EXPECT_EQ( + formatter.formatValueWithContext(Formatter::Context().setExtension(context), stream_info) + .number_value(), + 1234.0); } TEST(StringValueFormatterProviderTest, StringValueFormatterProviderTest) { { - FormatterContext context; + FormatterContextExtension context; StringValueFormatterProvider formatter( - [](const FormatterContext& context, + [](const Formatter::Context& context, const StreamInfo::StreamInfo&) -> absl::optional { - if (context.request_ == nullptr) { - return absl::nullopt; - } - return std::string(context.request_->path()); + CHECK_DATA_OR_RETURN(context, request_, absl::nullopt); + return std::string(checked_data->request_->path()); }, 9); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter.formatWithContext(context, stream_info), absl::nullopt); - EXPECT_TRUE(formatter.formatValueWithContext(context, stream_info).has_null_value()); + EXPECT_EQ(formatter.formatWithContext(Formatter::Context().setExtension(context), stream_info), + absl::nullopt); + EXPECT_TRUE( + formatter.formatValueWithContext(Formatter::Context().setExtension(context), stream_info) + .has_null_value()); FakeStreamCodecFactory::FakeRequest request; request.path_ = "ANYTHING"; context.request_ = &request; - EXPECT_EQ(formatter.formatWithContext(context, stream_info).value(), "ANYTHING"); - EXPECT_EQ(formatter.formatValueWithContext(context, stream_info).string_value(), "ANYTHING"); + EXPECT_EQ(formatter.formatWithContext(Formatter::Context().setExtension(context), stream_info) + .value(), + "ANYTHING"); + EXPECT_EQ( + formatter.formatValueWithContext(Formatter::Context().setExtension(context), stream_info) + .string_value(), + "ANYTHING"); request.path_ = "ANYTHING_LONGER_THAN_9"; - EXPECT_EQ(formatter.formatWithContext(context, stream_info).value(), "ANYTHING_"); - EXPECT_EQ(formatter.formatValueWithContext(context, stream_info).string_value(), "ANYTHING_"); + EXPECT_EQ(formatter.formatWithContext(Formatter::Context().setExtension(context), stream_info) + .value(), + "ANYTHING_"); + EXPECT_EQ( + formatter.formatValueWithContext(Formatter::Context().setExtension(context), stream_info) + .string_value(), + "ANYTHING_"); } } TEST(AccessLogFormatterTest, AccessLogFormatterTest) { + std::vector commands; + commands.push_back(createGenericProxyCommandParser()); + { // Test for %METHOD%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create("%METHOD%"); + FormatterContextExtension context; + auto formatter = *Envoy::Formatter::FormatterImpl::create("%METHOD%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeRequest request; request.method_ = "FAKE_METHOD"; context.request_ = &request; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_METHOD"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_METHOD"); } { // Test for %HOST%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create("%HOST%"); + FormatterContextExtension context; + auto formatter = *Envoy::Formatter::FormatterImpl::create("%HOST%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeRequest request; request.host_ = "FAKE_HOST"; context.request_ = &request; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_HOST"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_HOST"); } { // Test for %PATH%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create("%PATH%"); + FormatterContextExtension context; + auto formatter = *Envoy::Formatter::FormatterImpl::create("%PATH%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeRequest request; request.path_ = "FAKE_PATH"; context.request_ = &request; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_PATH"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_PATH"); } { // Test for %PROTOCOL%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create("%PROTOCOL%"); + FormatterContextExtension context; + auto formatter = *Envoy::Formatter::FormatterImpl::create("%PROTOCOL%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeRequest request; request.protocol_ = "FAKE_PROTOCOL"; context.request_ = &request; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_PROTOCOL"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_PROTOCOL"); } { // Test for %REQUEST_PROPERTY%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create( - "%REQUEST_PROPERTY(FAKE_KEY)%"); + FormatterContextExtension context; + auto formatter = + *Envoy::Formatter::FormatterImpl::create("%REQUEST_PROPERTY(FAKE_KEY)%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeRequest request; context.request_ = &request; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); request.data_["FAKE_KEY"] = "FAKE_VALUE"; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_VALUE"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_VALUE"); } { // Test for %RESPONSE_PROPERTY%. - FormatterContext context; - auto formatter = *Envoy::Formatter::FormatterBaseImpl::create( - "%RESPONSE_PROPERTY(FAKE_KEY)%"); + FormatterContextExtension context; + auto formatter = + *Envoy::Formatter::FormatterImpl::create("%RESPONSE_PROPERTY(FAKE_KEY)%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeResponse response; context.response_ = &response; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); response.data_["FAKE_KEY"] = "FAKE_VALUE"; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "FAKE_VALUE"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "FAKE_VALUE"); } { // Test for %GENERIC_RESPONSE_CODE%. - FormatterContext context; + FormatterContextExtension context; auto formatter = - *Envoy::Formatter::FormatterBaseImpl::create("%GENERIC_RESPONSE_CODE%"); + *Envoy::Formatter::FormatterImpl::create("%GENERIC_RESPONSE_CODE%", false, commands); StreamInfo::MockStreamInfo stream_info; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-"); FakeStreamCodecFactory::FakeResponse response; response.status_ = {-1234, false}; context.response_ = &response; - EXPECT_EQ(formatter->formatWithContext(context, stream_info), "-1234"); + EXPECT_EQ(formatter->formatWithContext(Formatter::Context().setExtension(context), stream_info), + "-1234"); } } diff --git a/test/extensions/filters/network/generic_proxy/config_test.cc b/test/extensions/filters/network/generic_proxy/config_test.cc index 7197e810c3542..14c418c06b45b 100644 --- a/test/extensions/filters/network/generic_proxy/config_test.cc +++ b/test/extensions/filters/network/generic_proxy/config_test.cc @@ -539,7 +539,7 @@ TEST(BasicFilterConfigTest, TestConfigurationWithAccessLogAndLogFilter2) { Registry::InjectFactory registration(codec_factory_config); FakeAccessLogExtensionFilterFactory fake_access_log_extension_filter_factory; - Registry::InjectFactory registration_log( + Registry::InjectFactory registration_log( fake_access_log_extension_filter_factory); NiceMock factory_context; diff --git a/test/extensions/filters/network/generic_proxy/fake_codec.h b/test/extensions/filters/network/generic_proxy/fake_codec.h index 3c0b25bf9d3b6..1f9cd32f56378 100644 --- a/test/extensions/filters/network/generic_proxy/fake_codec.h +++ b/test/extensions/filters/network/generic_proxy/fake_codec.h @@ -2,6 +2,8 @@ #include +#include "envoy/access_log/access_log_config.h" + #include "source/common/buffer/buffer_impl.h" #include "source/extensions/filters/network/generic_proxy/access_log.h" #include "source/extensions/filters/network/generic_proxy/interface/codec.h" @@ -421,17 +423,17 @@ class FakeStreamCodecFactoryConfig : public CodecFactoryConfig { std::string name() const override { return "envoy.generic_proxy.codecs.fake"; } }; -class FakeAccessLogExtensionFilter : public AccessLogFilter { - bool evaluate(const FormatterContext&, const StreamInfo::StreamInfo&) const override { +class FakeAccessLogExtensionFilter : public AccessLog::Filter { + bool evaluate(const Formatter::Context&, const StreamInfo::StreamInfo&) const override { return true; } }; -class FakeAccessLogExtensionFilterFactory : public AccessLogFilterFactory { +class FakeAccessLogExtensionFilterFactory : public AccessLog::ExtensionFilterFactory { public: // AccessLogFilterFactory - AccessLogFilterPtr createFilter(const envoy::config::accesslog::v3::ExtensionFilter&, - Server::Configuration::FactoryContext&) override { + AccessLog::FilterPtr createFilter(const envoy::config::accesslog::v3::ExtensionFilter&, + Server::Configuration::FactoryContext&) override { return std::make_unique(); } diff --git a/test/extensions/filters/network/generic_proxy/proxy_test.cc b/test/extensions/filters/network/generic_proxy/proxy_test.cc index 31900ba301418..6c2a1c05c7400 100644 --- a/test/extensions/filters/network/generic_proxy/proxy_test.cc +++ b/test/extensions/filters/network/generic_proxy/proxy_test.cc @@ -2,7 +2,9 @@ #include #include +#include "source/common/formatter/substitution_format_string.h" #include "source/common/tracing/tracer_manager_impl.h" +#include "source/extensions/access_loggers/common/file_access_log_impl.h" #include "source/extensions/filters/network/generic_proxy/proxy.h" #include "test/extensions/filters/network/generic_proxy/fake_codec.h" @@ -45,7 +47,7 @@ class MockRouteConfigProvider : public Rds::RouteConfigProvider { class FilterConfigTest : public testing::Test { public: - void initializeFilterConfig(bool with_tracing = false, AccessLogInstanceSharedPtr logger = {}, + void initializeFilterConfig(bool with_tracing = false, AccessLog::InstanceSharedPtr logger = {}, bool allow_no_decoder_filter = false) { if (with_tracing) { tracer_ = std::make_shared>(); @@ -93,7 +95,7 @@ class FilterConfigTest : public testing::Test { mock_route_entry_ = std::make_shared>(); - std::vector access_logs; + std::vector access_logs; if (logger) { access_logs.push_back(logger); } @@ -104,16 +106,14 @@ class FilterConfigTest : public testing::Test { factory_context_); } - AccessLogInstanceSharedPtr loggerFormFormat(const std::string& format = DEFAULT_LOG_FORMAT) { - envoy::config::core::v3::SubstitutionFormatString sff_config; - sff_config.mutable_text_format_source()->set_inline_string(format); - auto formatter = - *Envoy::Formatter::SubstitutionFormatStringUtils::fromProtoConfig( - sff_config, factory_context_); - - return std::make_shared( - Filesystem::FilePathAndType{}, nullptr, std::move(formatter), - factory_context_.server_factory_context_.accessLogManager()); + AccessLog::InstanceSharedPtr loggerFormFormat(const std::string& format = DEFAULT_LOG_FORMAT) { + envoy::extensions::access_loggers::file::v3::FileAccessLog file_log_config; + file_log_config.mutable_log_format()->mutable_text_format_source()->set_inline_string(format); + file_log_config.set_path("/fake/log/path"); + envoy::config::accesslog::v3::AccessLog config; + config.mutable_typed_config()->PackFrom(file_log_config); + config.set_name("file"); + return accessLoggerFromProto(config, factory_context_); } NiceMock factory_context_; @@ -189,7 +189,7 @@ TEST_F(FilterConfigTest, CodecFactory) { class FilterTest : public FilterConfigTest { public: - void initializeFilter(bool with_tracing = false, AccessLogInstanceSharedPtr logger = {}, + void initializeFilter(bool with_tracing = false, AccessLog::InstanceSharedPtr logger = {}, bool allow_no_decoder_filter = false) { FilterConfigTest::initializeFilterConfig(with_tracing, logger, allow_no_decoder_filter); From 46d329510f4825743c7bd856a25012e267291839 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Thu, 23 Jan 2025 14:09:39 +0000 Subject: [PATCH 02/11] add change log Signed-off-by: wangbaiping(wbpcode) --- .../filters/network/generic_proxy/v3/generic_proxy.proto | 2 +- changelogs/current.yaml | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto index ebae479a44ab0..eec87e8c16878 100644 --- a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto +++ b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto @@ -68,7 +68,7 @@ message GenericRds { // Generic proxy log formatter which could be configured at // the :ref:`formatters ` -// extension point for generic proxy specific custom formatters (command operators) support. +// extension point of logger to support generic proxy specific substitution commands. // // .. note:: // diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 3df7e7d8d2490..1297a9f302a2d 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -102,5 +102,12 @@ new_features: change: | Reporting a locality_stats to LRS server when rq_issued > 0, disable by setting runtime guard ``envoy.reloadable_features.report_load_with_rq_issued`` to ``false``. +- area: generic_proxy + change: | + Added :ref:`generic proxy log formatter support + ` which could be configured + at the :ref:`formatters ` extension point + of logger to support generic proxy specific substitution commands. And all access loggers could be used in + generic proxy now. deprecated: From 98b996808bf51a8ed83539193609c9aaf9320135 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Fri, 24 Jan 2025 03:12:53 +0000 Subject: [PATCH 03/11] minor update Signed-off-by: wangbaiping(wbpcode) --- .../filters/network/generic_proxy/v3/generic_proxy.proto | 2 +- changelogs/current.yaml | 8 ++++---- .../filters/network/generic_proxy/access_log.cc | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto index eec87e8c16878..e0cf457da15b7 100644 --- a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto +++ b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto @@ -74,5 +74,5 @@ message GenericRds { // // If the ``envoy.access_loggers.file`` logger is used for generic proxy, this formatter will // be enabled by default. -message GenericProxyLogFormatter { +message GenericProxySubstitutionFormatter { } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 1297a9f302a2d..1b4e135f2ef13 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -105,9 +105,9 @@ new_features: - area: generic_proxy change: | Added :ref:`generic proxy log formatter support - ` which could be configured - at the :ref:`formatters ` extension point - of logger to support generic proxy specific substitution commands. And all access loggers could be used in - generic proxy now. + ` which could be + configured at the :ref:`formatters ` + extension point of logger to support generic proxy specific substitution commands. And all access loggers could be + used in generic proxy now. deprecated: diff --git a/source/extensions/filters/network/generic_proxy/access_log.cc b/source/extensions/filters/network/generic_proxy/access_log.cc index 07735fcba15ad..f5dad0ba82f8d 100644 --- a/source/extensions/filters/network/generic_proxy/access_log.cc +++ b/source/extensions/filters/network/generic_proxy/access_log.cc @@ -153,8 +153,8 @@ class GenericProxyCommandParserFactory : public Formatter::CommandParserFactory return createGenericProxyCommandParser(); } ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique< - envoy::extensions::filters::network::generic_proxy::v3::GenericProxyLogFormatter>(); + return std::make_unique(); } std::string name() const override { return "envoy.formatters.generic_proxy_commands"; } @@ -191,7 +191,8 @@ absl::Status initializeFormatterForFileLogger( "Access log: no format and no default format for file access log"); } - envoy::extensions::filters::network::generic_proxy::v3::GenericProxyLogFormatter formatter; + envoy::extensions::filters::network::generic_proxy::v3::GenericProxySubstitutionFormatter + formatter; auto* formatter_extension = config.mutable_log_format()->mutable_formatters()->Add(); formatter_extension->set_name("envoy.formatters.generic_proxy_commands"); formatter_extension->mutable_typed_config()->PackFrom(formatter); From 86a7cf985630a404bbcc307f8b4db33676f8efb4 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Fri, 21 Feb 2025 04:00:42 +0000 Subject: [PATCH 04/11] access log: new access log interface to provide custom command parsers Signed-off-by: wangbaiping(wbpcode) --- envoy/access_log/BUILD | 1 + envoy/access_log/access_log_config.h | 11 ++-- source/common/access_log/access_log_impl.cc | 9 ++-- source/common/access_log/access_log_impl.h | 3 +- .../formatter/substitution_format_string.h | 12 +++-- .../common/stream_access_log_common_impl.h | 10 ++-- .../extensions/access_loggers/file/config.cc | 30 ++++++----- .../extensions/access_loggers/file/config.h | 3 +- .../access_loggers/fluentd/config.cc | 11 ++-- .../access_loggers/fluentd/config.h | 3 +- .../access_loggers/grpc/http_config.cc | 7 ++- .../access_loggers/grpc/http_config.h | 3 +- .../access_loggers/grpc/tcp_config.cc | 7 ++- .../access_loggers/grpc/tcp_config.h | 3 +- .../access_loggers/open_telemetry/config.cc | 11 ++-- .../access_loggers/open_telemetry/config.h | 3 +- .../access_loggers/stream/config.cc | 22 ++++---- .../extensions/access_loggers/stream/config.h | 6 ++- .../extensions/access_loggers/wasm/config.cc | 7 ++- .../extensions/access_loggers/wasm/config.h | 3 +- .../network/generic_proxy/file_access_log.h | 22 ++++---- .../access_loggers/file/config_test.cc | 54 ++++++++++++++++++- test/integration/fake_access_log.h | 3 +- 23 files changed, 161 insertions(+), 83 deletions(-) diff --git a/envoy/access_log/BUILD b/envoy/access_log/BUILD index 925681617ba45..fda9ca6eb654c 100644 --- a/envoy/access_log/BUILD +++ b/envoy/access_log/BUILD @@ -27,6 +27,7 @@ envoy_cc_library( hdrs = ["access_log_config.h"], deps = [ "//envoy/access_log:access_log_interface", + "//envoy/formatter:substitution_formatter_interface", "//envoy/server:filter_config_interface", "//source/common/protobuf", ], diff --git a/envoy/access_log/access_log_config.h b/envoy/access_log/access_log_config.h index c29659b89f122..63aaccc1ce613 100644 --- a/envoy/access_log/access_log_config.h +++ b/envoy/access_log/access_log_config.h @@ -4,6 +4,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/config/typed_config.h" +#include "envoy/formatter/substitution_formatter.h" #include "envoy/server/filter_config.h" #include "source/common/protobuf/protobuf.h" @@ -70,11 +71,13 @@ template class AccessLogInstanceFactoryBase : public Config::Typ * @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 access log context through which persistent resources can be accessed. + * @param command_parsers vector of command parsers that provide by the caller to be used for + * parsing custom substitution commands. */ - virtual AccessLog::InstanceBaseSharedPtr - createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterBasePtr&& filter, - Server::Configuration::FactoryContext& context) PURE; + virtual AccessLog::InstanceBaseSharedPtr createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterBasePtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector> command_parsers = {}) 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 index 95a3bb6792bba..aeaab7d85e4ec 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -328,8 +328,10 @@ bool MetadataFilter::evaluate(const Formatter::HttpFormatterContext&, return default_match_; } -InstanceSharedPtr AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config, - Server::Configuration::FactoryContext& context) { +InstanceSharedPtr +AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config, + Server::Configuration::FactoryContext& context, + std::vector command_parsers) { FilterPtr filter; if (config.has_filter()) { filter = FilterFactory::fromProto(config.filter(), context); @@ -339,7 +341,8 @@ InstanceSharedPtr AccessLogFactory::fromProto(const envoy::config::accesslog::v3 ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( config, context.messageValidationVisitor(), factory); - return factory.createAccessLogInstance(*message, std::move(filter), context); + return factory.createAccessLogInstance(*message, std::move(filter), context, + std::move(command_parsers)); } } // namespace AccessLog diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h index 9e10c202f91a8..67ebc2788169c 100644 --- a/source/common/access_log/access_log_impl.h +++ b/source/common/access_log/access_log_impl.h @@ -268,7 +268,8 @@ class AccessLogFactory { * to create access log instances that need access to listener properties. */ static InstanceSharedPtr fromProto(const envoy::config::accesslog::v3::AccessLog& config, - Server::Configuration::FactoryContext& context); + Server::Configuration::FactoryContext& context, + std::vector command_parsers = {}); }; } // namespace AccessLog diff --git a/source/common/formatter/substitution_format_string.h b/source/common/formatter/substitution_format_string.h index 2ded802ac08de..30e3073ef3ef8 100644 --- a/source/common/formatter/substitution_format_string.h +++ b/source/common/formatter/substitution_format_string.h @@ -32,8 +32,9 @@ class SubstitutionFormatStringUtils { template static absl::StatusOr>> parseFormatters(const FormattersConfig& formatters, - Server::Configuration::GenericFactoryContext& context) { - std::vector> commands; + Server::Configuration::GenericFactoryContext& context, + std::vector> commands_parsers = {}) { + std::vector> commands = std::move(commands_parsers); for (const auto& formatter : formatters) { auto* factory = Envoy::Config::Utility::getFactory>(formatter); @@ -59,10 +60,13 @@ class SubstitutionFormatStringUtils { template static absl::StatusOr> fromProtoConfig(const envoy::config::core::v3::SubstitutionFormatString& config, - Server::Configuration::GenericFactoryContext& context) { + Server::Configuration::GenericFactoryContext& context, + std::vector> commands_parsers = {}) { // Instantiate formatter extensions. - auto commands = parseFormatters(config.formatters(), context); + auto commands = parseFormatters(config.formatters(), context, + std::move(commands_parsers)); RETURN_IF_NOT_OK_REF(commands.status()); + switch (config.format_case()) { case envoy::config::core::v3::SubstitutionFormatString::FormatCase::kTextFormat: return FormatterBaseImpl::create(config.text_format(), 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 index 6f7b3582ad372..d5e3a92d4fc70 100644 --- 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,14 +13,16 @@ namespace AccessLoggers { template AccessLog::InstanceSharedPtr createStreamAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { + Server::Configuration::FactoryContext& context, + std::vector commands_parsers = {}) { const auto& fal_config = MessageUtil::downcastAndValidate(config, context.messageValidationVisitor()); Formatter::FormatterPtr formatter; if (fal_config.access_log_format_case() == T::AccessLogFormatCase::kLogFormat) { - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(fal_config.log_format(), context), - Formatter::FormatterBasePtr); + formatter = + THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + fal_config.log_format(), context, std::move(commands_parsers)), + Formatter::FormatterBasePtr); } else if (fal_config.access_log_format_case() == T::AccessLogFormatCase::ACCESS_LOG_FORMAT_NOT_SET) { formatter = THROW_OR_RETURN_VALUE( diff --git a/source/extensions/access_loggers/file/config.cc b/source/extensions/access_loggers/file/config.cc index 33d4eefca2d39..89988a80c072b 100644 --- a/source/extensions/access_loggers/file/config.cc +++ b/source/extensions/access_loggers/file/config.cc @@ -19,10 +19,10 @@ namespace Extensions { namespace AccessLoggers { namespace File { -AccessLog::InstanceSharedPtr -FileAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector command_parsers) { const auto& fal_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::file::v3::FileAccessLog&>( config, context.messageValidationVisitor()); @@ -37,28 +37,30 @@ FileAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, } else { envoy::config::core::v3::SubstitutionFormatString sff_config; sff_config.mutable_text_format_source()->set_inline_string(fal_config.format()); - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), - Formatter::FormatterBasePtr); + formatter = + THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + sff_config, context, std::move(command_parsers)), + Formatter::FormatterBasePtr); } break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kJsonFormat: formatter = Formatter::SubstitutionFormatStringUtils::createJsonFormatter( - fal_config.json_format(), false, false, false); + fal_config.json_format(), false, false, false, command_parsers); break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: kTypedJsonFormat: { envoy::config::core::v3::SubstitutionFormatString sff_config; *sff_config.mutable_json_format() = fal_config.typed_json_format(); - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), - Formatter::FormatterBasePtr); + formatter = THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + sff_config, context, std::move(command_parsers)), + Formatter::FormatterBasePtr); break; } case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kLogFormat: - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(fal_config.log_format(), context), - Formatter::FormatterBasePtr); + formatter = + THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + fal_config.log_format(), context, std::move(command_parsers)), + Formatter::FormatterBasePtr); break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: ACCESS_LOG_FORMAT_NOT_SET: diff --git a/source/extensions/access_loggers/file/config.h b/source/extensions/access_loggers/file/config.h index 5d6cab9ee464a..0584a6cb1199f 100644 --- a/source/extensions/access_loggers/file/config.h +++ b/source/extensions/access_loggers/file/config.h @@ -14,7 +14,8 @@ class FileAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/fluentd/config.cc b/source/extensions/access_loggers/fluentd/config.cc index a6614137e676d..aa90b0f12fa79 100644 --- a/source/extensions/access_loggers/fluentd/config.cc +++ b/source/extensions/access_loggers/fluentd/config.cc @@ -31,10 +31,10 @@ getAccessLoggerCacheSingleton(Server::Configuration::ServerFactoryContext& conte /* pin = */ true); } -AccessLog::InstanceSharedPtr -FluentdAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr FluentdAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector commands_parsers) { const auto& proto_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::fluentd::v3::FluentdAccessLogConfig&>( config, context.messageValidationVisitor()); @@ -61,7 +61,8 @@ FluentdAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config // TODO(ohadvano): Improve the formatting operation by creating a dedicated formatter that // will directly serialize the record to msgpack payload. auto commands = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context), + Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context, + std::move(commands_parsers)), std::vector>); Formatter::FormatterPtr json_formatter = diff --git a/source/extensions/access_loggers/fluentd/config.h b/source/extensions/access_loggers/fluentd/config.h index b7dfff974b1a1..d6b972ce43dd7 100644 --- a/source/extensions/access_loggers/fluentd/config.h +++ b/source/extensions/access_loggers/fluentd/config.h @@ -14,7 +14,8 @@ class FluentdAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector commands_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/grpc/http_config.cc b/source/extensions/access_loggers/grpc/http_config.cc index 5e9dc3f07ae3b..838ae32895ea5 100644 --- a/source/extensions/access_loggers/grpc/http_config.cc +++ b/source/extensions/access_loggers/grpc/http_config.cc @@ -18,10 +18,9 @@ namespace Extensions { namespace AccessLoggers { namespace HttpGrpc { -AccessLog::InstanceSharedPtr -HttpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, std::vector) { GrpcCommon::validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< diff --git a/source/extensions/access_loggers/grpc/http_config.h b/source/extensions/access_loggers/grpc/http_config.h index 8615b1cc7c982..567fa63a7e0cc 100644 --- a/source/extensions/access_loggers/grpc/http_config.h +++ b/source/extensions/access_loggers/grpc/http_config.h @@ -16,7 +16,8 @@ class HttpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/grpc/tcp_config.cc b/source/extensions/access_loggers/grpc/tcp_config.cc index 7aeec7fef813f..d9538390ea448 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.cc +++ b/source/extensions/access_loggers/grpc/tcp_config.cc @@ -18,10 +18,9 @@ namespace Extensions { namespace AccessLoggers { namespace TcpGrpc { -AccessLog::InstanceSharedPtr -TcpGrpcAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr TcpGrpcAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, std::vector) { GrpcCommon::validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< diff --git a/source/extensions/access_loggers/grpc/tcp_config.h b/source/extensions/access_loggers/grpc/tcp_config.h index 7fccd3e84da8e..92d0821ad7111 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.h +++ b/source/extensions/access_loggers/grpc/tcp_config.h @@ -16,7 +16,8 @@ class TcpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/open_telemetry/config.cc b/source/extensions/access_loggers/open_telemetry/config.cc index 5b9e817f528af..0afad993fba22 100644 --- a/source/extensions/access_loggers/open_telemetry/config.cc +++ b/source/extensions/access_loggers/open_telemetry/config.cc @@ -31,10 +31,10 @@ getAccessLoggerCacheSingleton(Server::Configuration::CommonFactoryContext& conte }); } -::Envoy::AccessLog::InstanceSharedPtr -AccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - ::Envoy::AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +::Envoy::AccessLog::InstanceSharedPtr AccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, ::Envoy::AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector commands_parsers) { validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< @@ -42,7 +42,8 @@ AccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, config, context.messageValidationVisitor()); auto commands = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context), + Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context, + std::move(commands_parsers)), std::vector>); return std::make_shared( diff --git a/source/extensions/access_loggers/open_telemetry/config.h b/source/extensions/access_loggers/open_telemetry/config.h index b82e5b7b6900d..e1e90ecb45293 100644 --- a/source/extensions/access_loggers/open_telemetry/config.h +++ b/source/extensions/access_loggers/open_telemetry/config.h @@ -16,7 +16,8 @@ class AccessLogFactory : public Envoy::AccessLog::AccessLogInstanceFactory { public: ::Envoy::AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, ::Envoy::AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector commands_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/stream/config.cc b/source/extensions/access_loggers/stream/config.cc index 37409202a97ce..dc22599320120 100644 --- a/source/extensions/access_loggers/stream/config.cc +++ b/source/extensions/access_loggers/stream/config.cc @@ -20,13 +20,14 @@ namespace Extensions { namespace AccessLoggers { namespace File { -AccessLog::InstanceSharedPtr -StdoutAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr StdoutAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector commands_parsers) { return AccessLoggers::createStreamAccessLogInstance< envoy::extensions::access_loggers::stream::v3::StdoutAccessLog, - Filesystem::DestinationType::Stdout>(config, std::move(filter), context); + Filesystem::DestinationType::Stdout>(config, std::move(filter), context, + std::move(commands_parsers)); } ProtobufTypes::MessagePtr StdoutAccessLogFactory::createEmptyConfigProto() { @@ -42,13 +43,14 @@ std::string StdoutAccessLogFactory::name() const { return "envoy.access_loggers. LEGACY_REGISTER_FACTORY(StdoutAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.stdout_access_log"); -AccessLog::InstanceSharedPtr -StderrAccessLogFactory::createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr StderrAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector commands_parsers) { return createStreamAccessLogInstance< envoy::extensions::access_loggers::stream::v3::StderrAccessLog, - Filesystem::DestinationType::Stderr>(config, std::move(filter), context); + Filesystem::DestinationType::Stderr>(config, std::move(filter), context, + std::move(commands_parsers)); } ProtobufTypes::MessagePtr StderrAccessLogFactory::createEmptyConfigProto() { diff --git a/source/extensions/access_loggers/stream/config.h b/source/extensions/access_loggers/stream/config.h index c50cb12ab7b3e..36a68128e5e21 100644 --- a/source/extensions/access_loggers/stream/config.h +++ b/source/extensions/access_loggers/stream/config.h @@ -14,7 +14,8 @@ class StdoutAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector commands_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; @@ -28,7 +29,8 @@ class StderrAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector commands_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index 30aa8d7cbeba4..62936e964eb17 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -14,10 +14,9 @@ namespace Extensions { namespace AccessLoggers { namespace Wasm { -AccessLog::InstanceSharedPtr -WasmAccessLogFactory::createAccessLogInstance(const Protobuf::Message& proto_config, - AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) { +AccessLog::InstanceSharedPtr WasmAccessLogFactory::createAccessLogInstance( + const Protobuf::Message& proto_config, AccessLog::FilterPtr&& filter, + Server::Configuration::FactoryContext& context, std::vector) { const auto& config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::wasm::v3::WasmAccessLog&>( proto_config, context.messageValidationVisitor()); diff --git a/source/extensions/access_loggers/wasm/config.h b/source/extensions/access_loggers/wasm/config.h index 43fbce159ca40..45c343c8536b0 100644 --- a/source/extensions/access_loggers/wasm/config.h +++ b/source/extensions/access_loggers/wasm/config.h @@ -17,7 +17,8 @@ class WasmAccessLogFactory : public AccessLog::AccessLogInstanceFactory, public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context) override; + Server::Configuration::FactoryContext& context, + std::vector = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/filters/network/generic_proxy/file_access_log.h b/source/extensions/filters/network/generic_proxy/file_access_log.h index 30eabb239ea78..9df27f7ef8a56 100644 --- a/source/extensions/filters/network/generic_proxy/file_access_log.h +++ b/source/extensions/filters/network/generic_proxy/file_access_log.h @@ -42,10 +42,10 @@ class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase< FileAccessLogFactoryBase() : name_(fmt::format("envoy.{}.access_loggers.file", Context::category())) {} - AccessLog::InstanceBaseSharedPtr - createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterBasePtr&& filter, - Server::Configuration::FactoryContext& context) override { + AccessLog::InstanceBaseSharedPtr createAccessLogInstance( + const Protobuf::Message& config, AccessLog::FilterBasePtr&& filter, + Server::Configuration::FactoryContext& context, + std::vector> command_parsers = {}) override { const auto& typed_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::file::v3::FileAccessLog&>( config, context.messageValidationVisitor()); @@ -60,29 +60,31 @@ class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase< envoy::config::core::v3::SubstitutionFormatString sff_config; sff_config.mutable_text_format_source()->set_inline_string(typed_config.format()); formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), + Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + sff_config, context, std::move(command_parsers)), Formatter::FormatterBasePtr); } break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: kJsonFormat: formatter = Formatter::SubstitutionFormatStringUtils::createJsonFormatter( - typed_config.json_format(), false, false, false); + typed_config.json_format(), false, false, false, command_parsers); break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: kTypedJsonFormat: { envoy::config::core::v3::SubstitutionFormatString sff_config; *sff_config.mutable_json_format() = typed_config.typed_json_format(); - formatter = THROW_OR_RETURN_VALUE( - Formatter::SubstitutionFormatStringUtils::fromProtoConfig(sff_config, context), - Formatter::FormatterBasePtr); + formatter = + THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( + sff_config, context, std::move(command_parsers)), + Formatter::FormatterBasePtr); break; } case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: kLogFormat: formatter = THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( - typed_config.log_format(), context), + typed_config.log_format(), context, std::move(command_parsers)), Formatter::FormatterBasePtr); break; case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: diff --git a/test/extensions/access_loggers/file/config_test.cc b/test/extensions/access_loggers/file/config_test.cc index 6a1c08d62afcf..a455d09d4062e 100644 --- a/test/extensions/access_loggers/file/config_test.cc +++ b/test/extensions/access_loggers/file/config_test.cc @@ -21,6 +21,18 @@ namespace AccessLoggers { namespace File { namespace { +class TestCustomCommandParser : public Formatter::CommandParser { +public: + Formatter::FormatterProviderPtr parse(absl::string_view command, absl::string_view, + absl::optional) const override { + if (command == "TEST_CUSTOM") { + return std::make_unique>( + "custom"); + } + return nullptr; + } +}; + TEST(FileAccessLogNegativeTest, ValidateFail) { NiceMock context; @@ -47,7 +59,8 @@ class FileAccessLogTest : public testing::Test { public: FileAccessLogTest() = default; - void runTest(const std::string& yaml, absl::string_view expected, bool is_json) { + void runTest(const std::string& yaml, absl::string_view expected, bool is_json, + std::vector command_parsers = {}) { envoy::extensions::access_loggers::file::v3::FileAccessLog fal_config; TestUtility::loadFromYaml(yaml, fal_config); @@ -59,7 +72,8 @@ class FileAccessLogTest : public testing::Test { EXPECT_CALL(context_.server_factory_context_.access_log_manager_, createAccessLog(file_info)) .WillOnce(Return(file)); - AccessLog::InstanceSharedPtr logger = AccessLog::AccessLogFactory::fromProto(config, context_); + AccessLog::InstanceSharedPtr logger = + AccessLog::AccessLogFactory::fromProto(config, context_, std::move(command_parsers)); absl::Time abslStartTime = TestUtility::parseTime("Dec 18 01:50:34 2018 GMT", "%b %e %H:%M:%S %Y GMT"); @@ -176,6 +190,42 @@ TEST_F(FileAccessLogTest, LogFormatJson) { true); } +TEST_F(FileAccessLogTest, LogFormatJsonWithCustomCommands) { + const std::string format_yaml = R"( + path: "/foo" + log_format: + json_format: + custom: "%TEST_CUSTOM%" + text: "plain text" + path: "%REQ(:path)%" + )"; + + envoy::extensions::access_loggers::file::v3::FileAccessLog fal_config; + TestUtility::loadFromYaml(format_yaml, fal_config); + + { + // No custom command parsers and the formatter should fail. + envoy::config::accesslog::v3::AccessLog config; + config.mutable_typed_config()->PackFrom(fal_config); + config.set_name("file"); + + EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context_), + EnvoyException, "Not supported field in StreamInfo: TEST_CUSTOM"); + } + + { + std::vector command_parsers; + command_parsers.push_back(std::make_unique()); + + runTest(format_yaml, R"({ + "custom": "custom", + "text": "plain text", + "path": "/bar/foo", + })", + true, std::move(command_parsers)); + } +} + } // namespace } // namespace File } // namespace AccessLoggers diff --git a/test/integration/fake_access_log.h b/test/integration/fake_access_log.h index 9c0661245c11b..361a065ebee5b 100644 --- a/test/integration/fake_access_log.h +++ b/test/integration/fake_access_log.h @@ -30,7 +30,8 @@ class FakeAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message&, AccessLog::FilterPtr&&, - Server::Configuration::FactoryContext&) override { + Server::Configuration::FactoryContext&, + std::vector = {}) override { std::lock_guard guard(log_callback_lock_); auto access_log_instance = std::make_shared(log_cb_); access_log_instances_.push_back(access_log_instance); From 0c22978c93554d0c7fac2148e2c827f4717e0335 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Fri, 21 Feb 2025 06:31:18 +0000 Subject: [PATCH 05/11] fix test Signed-off-by: wangbaiping(wbpcode) --- test/integration/typed_metadata_integration_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/typed_metadata_integration_test.cc b/test/integration/typed_metadata_integration_test.cc index 804268821f7a5..0ef0582aed6ff 100644 --- a/test/integration/typed_metadata_integration_test.cc +++ b/test/integration/typed_metadata_integration_test.cc @@ -54,7 +54,8 @@ class TestAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message&, AccessLog::FilterPtr&&, - Server::Configuration::FactoryContext& context) override { + Server::Configuration::FactoryContext& context, + std::vector = {}) override { // Check that expected listener metadata is present EXPECT_EQ(1, context.listenerInfo().metadata().typed_filter_metadata().size()); const auto iter = context.listenerInfo().metadata().typed_filter_metadata().find( From 7bae35fa997d527ec85f70a2bc178b76a94d157e Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Sat, 22 Feb 2025 03:11:54 +0000 Subject: [PATCH 06/11] address comments Signed-off-by: wangbaiping(wbpcode) --- envoy/access_log/access_log_config.h | 2 +- source/common/access_log/access_log_impl.cc | 2 +- source/common/access_log/access_log_impl.h | 7 ++++--- source/common/formatter/substitution_format_string.h | 6 +++--- .../access_loggers/common/stream_access_log_common_impl.h | 4 ++-- source/extensions/access_loggers/file/config.cc | 2 +- source/extensions/access_loggers/file/config.h | 2 +- source/extensions/access_loggers/fluentd/config.cc | 4 ++-- source/extensions/access_loggers/fluentd/config.h | 2 +- source/extensions/access_loggers/grpc/http_config.cc | 2 +- source/extensions/access_loggers/grpc/http_config.h | 2 +- source/extensions/access_loggers/grpc/tcp_config.cc | 2 +- source/extensions/access_loggers/grpc/tcp_config.h | 2 +- source/extensions/access_loggers/open_telemetry/config.cc | 4 ++-- source/extensions/access_loggers/open_telemetry/config.h | 2 +- source/extensions/access_loggers/stream/config.cc | 8 ++++---- source/extensions/access_loggers/stream/config.h | 4 ++-- source/extensions/access_loggers/wasm/config.cc | 2 +- source/extensions/access_loggers/wasm/config.h | 2 +- .../filters/network/generic_proxy/file_access_log.h | 2 +- test/extensions/access_loggers/file/config_test.cc | 4 ++-- test/integration/fake_access_log.h | 2 +- test/integration/typed_metadata_integration_test.cc | 2 +- 23 files changed, 36 insertions(+), 35 deletions(-) diff --git a/envoy/access_log/access_log_config.h b/envoy/access_log/access_log_config.h index 63aaccc1ce613..a5573e9c5ceb5 100644 --- a/envoy/access_log/access_log_config.h +++ b/envoy/access_log/access_log_config.h @@ -77,7 +77,7 @@ template class AccessLogInstanceFactoryBase : public Config::Typ virtual AccessLog::InstanceBaseSharedPtr createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterBasePtr&& filter, Server::Configuration::FactoryContext& context, - std::vector> command_parsers = {}) PURE; + std::vector>&& command_parsers = {}) 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 index aeaab7d85e4ec..9ced24459c598 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -331,7 +331,7 @@ bool MetadataFilter::evaluate(const Formatter::HttpFormatterContext&, InstanceSharedPtr AccessLogFactory::fromProto(const envoy::config::accesslog::v3::AccessLog& config, Server::Configuration::FactoryContext& context, - std::vector command_parsers) { + std::vector&& command_parsers) { FilterPtr filter; if (config.has_filter()) { filter = FilterFactory::fromProto(config.filter(), context); diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h index 67ebc2788169c..e9bd2b1faef36 100644 --- a/source/common/access_log/access_log_impl.h +++ b/source/common/access_log/access_log_impl.h @@ -267,9 +267,10 @@ 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::FactoryContext& context, - std::vector command_parsers = {}); + static InstanceSharedPtr + fromProto(const envoy::config::accesslog::v3::AccessLog& config, + Server::Configuration::FactoryContext& context, + std::vector&& command_parsers = {}); }; } // namespace AccessLog diff --git a/source/common/formatter/substitution_format_string.h b/source/common/formatter/substitution_format_string.h index 30e3073ef3ef8..64e8b59a7352c 100644 --- a/source/common/formatter/substitution_format_string.h +++ b/source/common/formatter/substitution_format_string.h @@ -61,10 +61,10 @@ class SubstitutionFormatStringUtils { static absl::StatusOr> fromProtoConfig(const envoy::config::core::v3::SubstitutionFormatString& config, Server::Configuration::GenericFactoryContext& context, - std::vector> commands_parsers = {}) { + std::vector>&& command_parsers = {}) { // Instantiate formatter extensions. - auto commands = parseFormatters(config.formatters(), context, - std::move(commands_parsers)); + auto commands = + parseFormatters(config.formatters(), context, std::move(command_parsers)); RETURN_IF_NOT_OK_REF(commands.status()); switch (config.format_case()) { 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 index d5e3a92d4fc70..d074afc93a652 100644 --- a/source/extensions/access_loggers/common/stream_access_log_common_impl.h +++ b/source/extensions/access_loggers/common/stream_access_log_common_impl.h @@ -14,14 +14,14 @@ template AccessLog::InstanceSharedPtr createStreamAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers = {}) { + std::vector&& command_parsers = {}) { const auto& fal_config = MessageUtil::downcastAndValidate(config, context.messageValidationVisitor()); Formatter::FormatterPtr formatter; if (fal_config.access_log_format_case() == T::AccessLogFormatCase::kLogFormat) { formatter = THROW_OR_RETURN_VALUE(Formatter::SubstitutionFormatStringUtils::fromProtoConfig( - fal_config.log_format(), context, std::move(commands_parsers)), + fal_config.log_format(), context, std::move(command_parsers)), Formatter::FormatterBasePtr); } else if (fal_config.access_log_format_case() == T::AccessLogFormatCase::ACCESS_LOG_FORMAT_NOT_SET) { diff --git a/source/extensions/access_loggers/file/config.cc b/source/extensions/access_loggers/file/config.cc index 89988a80c072b..a33299272efa8 100644 --- a/source/extensions/access_loggers/file/config.cc +++ b/source/extensions/access_loggers/file/config.cc @@ -22,7 +22,7 @@ namespace File { AccessLog::InstanceSharedPtr FileAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector command_parsers) { + std::vector&& command_parsers) { const auto& fal_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::file::v3::FileAccessLog&>( config, context.messageValidationVisitor()); diff --git a/source/extensions/access_loggers/file/config.h b/source/extensions/access_loggers/file/config.h index 0584a6cb1199f..2c2b17997034d 100644 --- a/source/extensions/access_loggers/file/config.h +++ b/source/extensions/access_loggers/file/config.h @@ -15,7 +15,7 @@ class FileAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector command_parsers = {}) override; + std::vector&& command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/fluentd/config.cc b/source/extensions/access_loggers/fluentd/config.cc index aa90b0f12fa79..1ee01fb8feb57 100644 --- a/source/extensions/access_loggers/fluentd/config.cc +++ b/source/extensions/access_loggers/fluentd/config.cc @@ -34,7 +34,7 @@ getAccessLoggerCacheSingleton(Server::Configuration::ServerFactoryContext& conte AccessLog::InstanceSharedPtr FluentdAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers) { + std::vector&& command_parsers) { const auto& proto_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::fluentd::v3::FluentdAccessLogConfig&>( config, context.messageValidationVisitor()); @@ -62,7 +62,7 @@ AccessLog::InstanceSharedPtr FluentdAccessLogFactory::createAccessLogInstance( // will directly serialize the record to msgpack payload. auto commands = THROW_OR_RETURN_VALUE( Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context, - std::move(commands_parsers)), + std::move(command_parsers)), std::vector>); Formatter::FormatterPtr json_formatter = diff --git a/source/extensions/access_loggers/fluentd/config.h b/source/extensions/access_loggers/fluentd/config.h index d6b972ce43dd7..a098d9129179a 100644 --- a/source/extensions/access_loggers/fluentd/config.h +++ b/source/extensions/access_loggers/fluentd/config.h @@ -15,7 +15,7 @@ class FluentdAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers = {}) override; + std::vector&& command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/grpc/http_config.cc b/source/extensions/access_loggers/grpc/http_config.cc index 838ae32895ea5..696b3986c0377 100644 --- a/source/extensions/access_loggers/grpc/http_config.cc +++ b/source/extensions/access_loggers/grpc/http_config.cc @@ -20,7 +20,7 @@ namespace HttpGrpc { AccessLog::InstanceSharedPtr HttpGrpcAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context, std::vector) { + Server::Configuration::FactoryContext& context, std::vector&&) { GrpcCommon::validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< diff --git a/source/extensions/access_loggers/grpc/http_config.h b/source/extensions/access_loggers/grpc/http_config.h index 567fa63a7e0cc..a262b2e29b4c5 100644 --- a/source/extensions/access_loggers/grpc/http_config.h +++ b/source/extensions/access_loggers/grpc/http_config.h @@ -17,7 +17,7 @@ class HttpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector = {}) override; + std::vector&& = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/grpc/tcp_config.cc b/source/extensions/access_loggers/grpc/tcp_config.cc index d9538390ea448..744a0ef733008 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.cc +++ b/source/extensions/access_loggers/grpc/tcp_config.cc @@ -20,7 +20,7 @@ namespace TcpGrpc { AccessLog::InstanceSharedPtr TcpGrpcAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context, std::vector) { + Server::Configuration::FactoryContext& context, std::vector&&) { GrpcCommon::validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< diff --git a/source/extensions/access_loggers/grpc/tcp_config.h b/source/extensions/access_loggers/grpc/tcp_config.h index 92d0821ad7111..fce3a662f4e49 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.h +++ b/source/extensions/access_loggers/grpc/tcp_config.h @@ -17,7 +17,7 @@ class TcpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector = {}) override; + std::vector&& = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/open_telemetry/config.cc b/source/extensions/access_loggers/open_telemetry/config.cc index 0afad993fba22..24d9ce561e816 100644 --- a/source/extensions/access_loggers/open_telemetry/config.cc +++ b/source/extensions/access_loggers/open_telemetry/config.cc @@ -34,7 +34,7 @@ getAccessLoggerCacheSingleton(Server::Configuration::CommonFactoryContext& conte ::Envoy::AccessLog::InstanceSharedPtr AccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, ::Envoy::AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers) { + std::vector&& command_parsers) { validateProtoDescriptors(); const auto& proto_config = MessageUtil::downcastAndValidate< @@ -43,7 +43,7 @@ ::Envoy::AccessLog::InstanceSharedPtr AccessLogFactory::createAccessLogInstance( auto commands = THROW_OR_RETURN_VALUE( Formatter::SubstitutionFormatStringUtils::parseFormatters(proto_config.formatters(), context, - std::move(commands_parsers)), + std::move(command_parsers)), std::vector>); return std::make_shared( diff --git a/source/extensions/access_loggers/open_telemetry/config.h b/source/extensions/access_loggers/open_telemetry/config.h index e1e90ecb45293..13a85debc2633 100644 --- a/source/extensions/access_loggers/open_telemetry/config.h +++ b/source/extensions/access_loggers/open_telemetry/config.h @@ -17,7 +17,7 @@ class AccessLogFactory : public Envoy::AccessLog::AccessLogInstanceFactory { ::Envoy::AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, ::Envoy::AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers = {}) override; + std::vector&& command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/stream/config.cc b/source/extensions/access_loggers/stream/config.cc index dc22599320120..0f78a10a5d845 100644 --- a/source/extensions/access_loggers/stream/config.cc +++ b/source/extensions/access_loggers/stream/config.cc @@ -23,11 +23,11 @@ namespace File { AccessLog::InstanceSharedPtr StdoutAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers) { + std::vector&& command_parsers) { return AccessLoggers::createStreamAccessLogInstance< envoy::extensions::access_loggers::stream::v3::StdoutAccessLog, Filesystem::DestinationType::Stdout>(config, std::move(filter), context, - std::move(commands_parsers)); + std::move(command_parsers)); } ProtobufTypes::MessagePtr StdoutAccessLogFactory::createEmptyConfigProto() { @@ -46,11 +46,11 @@ LEGACY_REGISTER_FACTORY(StdoutAccessLogFactory, AccessLog::AccessLogInstanceFact AccessLog::InstanceSharedPtr StderrAccessLogFactory::createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers) { + std::vector&& command_parsers) { return createStreamAccessLogInstance< envoy::extensions::access_loggers::stream::v3::StderrAccessLog, Filesystem::DestinationType::Stderr>(config, std::move(filter), context, - std::move(commands_parsers)); + std::move(command_parsers)); } ProtobufTypes::MessagePtr StderrAccessLogFactory::createEmptyConfigProto() { diff --git a/source/extensions/access_loggers/stream/config.h b/source/extensions/access_loggers/stream/config.h index 36a68128e5e21..1be10c2df49df 100644 --- a/source/extensions/access_loggers/stream/config.h +++ b/source/extensions/access_loggers/stream/config.h @@ -15,7 +15,7 @@ class StdoutAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers = {}) override; + std::vector&& command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; @@ -30,7 +30,7 @@ class StderrAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector commands_parsers = {}) override; + std::vector&& command_parsers = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index 62936e964eb17..d2ccf7d9c72e0 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -16,7 +16,7 @@ namespace Wasm { AccessLog::InstanceSharedPtr WasmAccessLogFactory::createAccessLogInstance( const Protobuf::Message& proto_config, AccessLog::FilterPtr&& filter, - Server::Configuration::FactoryContext& context, std::vector) { + Server::Configuration::FactoryContext& context, std::vector&&) { const auto& config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::wasm::v3::WasmAccessLog&>( proto_config, context.messageValidationVisitor()); diff --git a/source/extensions/access_loggers/wasm/config.h b/source/extensions/access_loggers/wasm/config.h index 45c343c8536b0..d85ad871c2299 100644 --- a/source/extensions/access_loggers/wasm/config.h +++ b/source/extensions/access_loggers/wasm/config.h @@ -18,7 +18,7 @@ class WasmAccessLogFactory : public AccessLog::AccessLogInstanceFactory, AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, Server::Configuration::FactoryContext& context, - std::vector = {}) override; + std::vector&& = {}) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; diff --git a/source/extensions/filters/network/generic_proxy/file_access_log.h b/source/extensions/filters/network/generic_proxy/file_access_log.h index 9df27f7ef8a56..4110fc9e793af 100644 --- a/source/extensions/filters/network/generic_proxy/file_access_log.h +++ b/source/extensions/filters/network/generic_proxy/file_access_log.h @@ -45,7 +45,7 @@ class FileAccessLogFactoryBase : public AccessLog::AccessLogInstanceFactoryBase< AccessLog::InstanceBaseSharedPtr createAccessLogInstance( const Protobuf::Message& config, AccessLog::FilterBasePtr&& filter, Server::Configuration::FactoryContext& context, - std::vector> command_parsers = {}) override { + std::vector>&& command_parsers = {}) override { const auto& typed_config = MessageUtil::downcastAndValidate< const envoy::extensions::access_loggers::file::v3::FileAccessLog&>( config, context.messageValidationVisitor()); diff --git a/test/extensions/access_loggers/file/config_test.cc b/test/extensions/access_loggers/file/config_test.cc index a455d09d4062e..6a3769d7e3c80 100644 --- a/test/extensions/access_loggers/file/config_test.cc +++ b/test/extensions/access_loggers/file/config_test.cc @@ -60,7 +60,7 @@ class FileAccessLogTest : public testing::Test { FileAccessLogTest() = default; void runTest(const std::string& yaml, absl::string_view expected, bool is_json, - std::vector command_parsers = {}) { + std::vector&& command_parsers = {}) { envoy::extensions::access_loggers::file::v3::FileAccessLog fal_config; TestUtility::loadFromYaml(yaml, fal_config); @@ -214,7 +214,7 @@ TEST_F(FileAccessLogTest, LogFormatJsonWithCustomCommands) { } { - std::vector command_parsers; + std::vector&& command_parsers; command_parsers.push_back(std::make_unique()); runTest(format_yaml, R"({ diff --git a/test/integration/fake_access_log.h b/test/integration/fake_access_log.h index 361a065ebee5b..e433fb50bd0a7 100644 --- a/test/integration/fake_access_log.h +++ b/test/integration/fake_access_log.h @@ -31,7 +31,7 @@ class FakeAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message&, AccessLog::FilterPtr&&, Server::Configuration::FactoryContext&, - std::vector = {}) override { + std::vector&& = {}) override { std::lock_guard guard(log_callback_lock_); auto access_log_instance = std::make_shared(log_cb_); access_log_instances_.push_back(access_log_instance); diff --git a/test/integration/typed_metadata_integration_test.cc b/test/integration/typed_metadata_integration_test.cc index 0ef0582aed6ff..d49d318c7f50a 100644 --- a/test/integration/typed_metadata_integration_test.cc +++ b/test/integration/typed_metadata_integration_test.cc @@ -55,7 +55,7 @@ class TestAccessLogFactory : public AccessLog::AccessLogInstanceFactory { AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message&, AccessLog::FilterPtr&&, Server::Configuration::FactoryContext& context, - std::vector = {}) override { + std::vector&& = {}) override { // Check that expected listener metadata is present EXPECT_EQ(1, context.listenerInfo().metadata().typed_filter_metadata().size()); const auto iter = context.listenerInfo().metadata().typed_filter_metadata().find( From b04ac8bed26415573996b5146dad4cc80c5fbcfc Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Sat, 22 Feb 2025 04:11:49 +0000 Subject: [PATCH 07/11] minor update Signed-off-by: wangbaiping(wbpcode) --- test/extensions/access_loggers/file/config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/access_loggers/file/config_test.cc b/test/extensions/access_loggers/file/config_test.cc index 6a3769d7e3c80..8412628d472c0 100644 --- a/test/extensions/access_loggers/file/config_test.cc +++ b/test/extensions/access_loggers/file/config_test.cc @@ -214,7 +214,7 @@ TEST_F(FileAccessLogTest, LogFormatJsonWithCustomCommands) { } { - std::vector&& command_parsers; + std::vector command_parsers; command_parsers.push_back(std::make_unique()); runTest(format_yaml, R"({ From 3259435873376a997424608c2b6387a434d4b8a2 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Sat, 22 Feb 2025 07:03:45 +0000 Subject: [PATCH 08/11] no new API Signed-off-by: wangbaiping(wbpcode) --- .../generic_proxy/v3/generic_proxy.proto | 11 --- .../network/generic_proxy/access_log.cc | 94 ------------------- .../network/generic_proxy/access_log.h | 4 - .../filters/network/generic_proxy/config.cc | 7 +- 4 files changed, 5 insertions(+), 111 deletions(-) diff --git a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto index e0cf457da15b7..9ae2560470cf2 100644 --- a/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto +++ b/api/envoy/extensions/filters/network/generic_proxy/v3/generic_proxy.proto @@ -65,14 +65,3 @@ message GenericRds { // Envoy configuration with multiple generic proxies to use different route configurations. string route_config_name = 2 [(validate.rules).string = {min_len: 1}]; } - -// Generic proxy log formatter which could be configured at -// the :ref:`formatters ` -// extension point of logger to support generic proxy specific substitution commands. -// -// .. note:: -// -// If the ``envoy.access_loggers.file`` logger is used for generic proxy, this formatter will -// be enabled by default. -message GenericProxySubstitutionFormatter { -} diff --git a/source/extensions/filters/network/generic_proxy/access_log.cc b/source/extensions/filters/network/generic_proxy/access_log.cc index f5dad0ba82f8d..b946f30275620 100644 --- a/source/extensions/filters/network/generic_proxy/access_log.cc +++ b/source/extensions/filters/network/generic_proxy/access_log.cc @@ -144,100 +144,6 @@ Formatter::CommandParserPtr createGenericProxyCommandParser() { return std::make_unique(); } -class GenericProxyCommandParserFactory : public Formatter::CommandParserFactory { -public: - // CommandParserFactory - Formatter::CommandParserPtr - createCommandParserFromProto(const Protobuf::Message&, - Server::Configuration::GenericFactoryContext&) override { - return createGenericProxyCommandParser(); - } - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); - } - - std::string name() const override { return "envoy.formatters.generic_proxy_commands"; } -}; - -REGISTER_FACTORY(GenericProxyCommandParserFactory, Formatter::CommandParserFactory); - -absl::Status initializeFormatterForFileLogger( - envoy::extensions::access_loggers::file::v3::FileAccessLog& config) { - switch (config.access_log_format_case()) { - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kFormat: { - std::string legacy_format = config.format(); - *config.mutable_log_format()->mutable_text_format_source()->mutable_inline_string() = - legacy_format; - break; - } - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - kJsonFormat: { - ProtobufWkt::Struct json_format = config.json_format(); - *config.mutable_log_format()->mutable_json_format() = std::move(json_format); - break; - } - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - kTypedJsonFormat: { - ProtobufWkt::Struct json_format = config.typed_json_format(); - *config.mutable_log_format()->mutable_json_format() = std::move(json_format); - break; - } - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase::kLogFormat: - break; - case envoy::extensions::access_loggers::file::v3::FileAccessLog::AccessLogFormatCase:: - ACCESS_LOG_FORMAT_NOT_SET: - return absl::InvalidArgumentError( - "Access log: no format and no default format for file access log"); - } - - envoy::extensions::filters::network::generic_proxy::v3::GenericProxySubstitutionFormatter - formatter; - auto* formatter_extension = config.mutable_log_format()->mutable_formatters()->Add(); - formatter_extension->set_name("envoy.formatters.generic_proxy_commands"); - formatter_extension->mutable_typed_config()->PackFrom(formatter); - return absl::OkStatus(); -} - -AccessLog::FilterPtr -accessLogFilterFromProto(const envoy::config::accesslog::v3::AccessLogFilter& config, - Server::Configuration::FactoryContext& context) { - if (!config.has_extension_filter()) { - ExceptionUtil::throwEnvoyException( - "Access log filter: only extension filter is supported by non-HTTP access loggers."); - } - - auto& factory = Config::Utility::getAndCheckFactory( - config.extension_filter()); - return factory.createFilter(config.extension_filter(), context); -} - -AccessLog::InstanceSharedPtr -accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config, - Server::Configuration::FactoryContext& context) { - AccessLog::FilterPtr filter; - if (config.has_filter()) { - filter = accessLogFilterFromProto(config.filter(), context); - } - - auto& factory = - Config::Utility::getAndCheckFactory(config); - ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( - config, context.messageValidationVisitor(), factory); - - // Setup the generic proxy formatter for file access loggers by default. This is necessary - // for backwards compatibility. - if (factory.name() == "envoy.access_loggers.file") { - auto* file_config = - dynamic_cast(message.get()); - ASSERT(file_config != nullptr); - THROW_IF_NOT_OK(initializeFormatterForFileLogger(*file_config)); - } - - // Inject the command parsers from the generic proxy. - return factory.createAccessLogInstance(*message, std::move(filter), context); -} - } // namespace GenericProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/source/extensions/filters/network/generic_proxy/access_log.h b/source/extensions/filters/network/generic_proxy/access_log.h index 9917e4a48bc50..9ebb528789109 100644 --- a/source/extensions/filters/network/generic_proxy/access_log.h +++ b/source/extensions/filters/network/generic_proxy/access_log.h @@ -64,10 +64,6 @@ class GenericStatusCodeFormatterProvider : public FormatterProvider { Formatter::CommandParserPtr createGenericProxyCommandParser(); -AccessLog::InstanceSharedPtr -accessLoggerFromProto(const envoy::config::accesslog::v3::AccessLog& config, - Server::Configuration::FactoryContext& context); - } // namespace GenericProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/source/extensions/filters/network/generic_proxy/config.cc b/source/extensions/filters/network/generic_proxy/config.cc index 41e774c708953..c57dda3bcfaef 100644 --- a/source/extensions/filters/network/generic_proxy/config.cc +++ b/source/extensions/filters/network/generic_proxy/config.cc @@ -5,7 +5,7 @@ #include "source/extensions/filters/network/generic_proxy/rds.h" #include "source/extensions/filters/network/generic_proxy/rds_impl.h" -#include "access_log.h" +#include "source/extensions/filters/network/generic_proxy/access_log.h" namespace Envoy { namespace Extensions { @@ -126,7 +126,10 @@ Factory::createFilterFactoryFromProtoTyped(const ProxyConfig& proto_config, // Access log configuration. std::vector access_logs; for (const auto& access_log : proto_config.access_log()) { - AccessLog::InstanceSharedPtr current_access_log = accessLoggerFromProto(access_log, context); + std::vector command_parsers; + command_parsers.push_back(createGenericProxyCommandParser()); + AccessLog::InstanceSharedPtr current_access_log = + AccessLog::AccessLogFactory::fromProto(access_log, context, std::move(command_parsers)); access_logs.push_back(current_access_log); } From 8af8887ad48873171bf76750ab9cee741697322b Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Sun, 23 Feb 2025 02:55:03 +0000 Subject: [PATCH 09/11] fix test Signed-off-by: wangbaiping(wbpcode) --- test/extensions/filters/network/generic_proxy/BUILD | 1 + test/extensions/filters/network/generic_proxy/proxy_test.cc | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/extensions/filters/network/generic_proxy/BUILD b/test/extensions/filters/network/generic_proxy/BUILD index a9899b8ff6950..c6945d2e3aa0e 100644 --- a/test/extensions/filters/network/generic_proxy/BUILD +++ b/test/extensions/filters/network/generic_proxy/BUILD @@ -60,6 +60,7 @@ envoy_cc_test( "//test/mocks/server:factory_context_mocks", "//test/test_common:registry_lib", "//test/test_common:utility_lib", + "//source/common/access_log:access_log_lib", ], ) diff --git a/test/extensions/filters/network/generic_proxy/proxy_test.cc b/test/extensions/filters/network/generic_proxy/proxy_test.cc index 6c2a1c05c7400..1dc67a8677804 100644 --- a/test/extensions/filters/network/generic_proxy/proxy_test.cc +++ b/test/extensions/filters/network/generic_proxy/proxy_test.cc @@ -6,6 +6,7 @@ #include "source/common/tracing/tracer_manager_impl.h" #include "source/extensions/access_loggers/common/file_access_log_impl.h" #include "source/extensions/filters/network/generic_proxy/proxy.h" +#include "source/common/access_log/access_log_impl.h" #include "test/extensions/filters/network/generic_proxy/fake_codec.h" #include "test/extensions/filters/network/generic_proxy/mocks/codec.h" @@ -113,7 +114,7 @@ class FilterConfigTest : public testing::Test { envoy::config::accesslog::v3::AccessLog config; config.mutable_typed_config()->PackFrom(file_log_config); config.set_name("file"); - return accessLoggerFromProto(config, factory_context_); + return AccessLog::AccessLogFactory::fromProto(config, factory_context_); } NiceMock factory_context_; From e180a19a7b4da84824d3f43656c4785b9dbd90a1 Mon Sep 17 00:00:00 2001 From: "wangbaiping(wbpcode)" Date: Sun, 23 Feb 2025 04:01:31 +0000 Subject: [PATCH 10/11] fix test Signed-off-by: wangbaiping(wbpcode) --- .../extensions/filters/network/generic_proxy/config.cc | 3 +-- test/extensions/filters/network/generic_proxy/BUILD | 2 +- .../filters/network/generic_proxy/config_test.cc | 9 +++++---- .../filters/network/generic_proxy/proxy_test.cc | 8 ++++++-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/source/extensions/filters/network/generic_proxy/config.cc b/source/extensions/filters/network/generic_proxy/config.cc index c57dda3bcfaef..a922cb83dbaa3 100644 --- a/source/extensions/filters/network/generic_proxy/config.cc +++ b/source/extensions/filters/network/generic_proxy/config.cc @@ -2,11 +2,10 @@ #include "source/common/access_log/access_log_impl.h" #include "source/common/tracing/tracer_manager_impl.h" +#include "source/extensions/filters/network/generic_proxy/access_log.h" #include "source/extensions/filters/network/generic_proxy/rds.h" #include "source/extensions/filters/network/generic_proxy/rds_impl.h" -#include "source/extensions/filters/network/generic_proxy/access_log.h" - namespace Envoy { namespace Extensions { namespace NetworkFilters { diff --git a/test/extensions/filters/network/generic_proxy/BUILD b/test/extensions/filters/network/generic_proxy/BUILD index c6945d2e3aa0e..1d1a360ea3671 100644 --- a/test/extensions/filters/network/generic_proxy/BUILD +++ b/test/extensions/filters/network/generic_proxy/BUILD @@ -49,6 +49,7 @@ envoy_cc_test( rbe_pool = "6gig", deps = [ ":fake_codec_lib", + "//source/common/access_log:access_log_lib", "//source/common/buffer:buffer_lib", "//source/common/formatter:formatter_extension_lib", "//source/common/formatter:substitution_format_string_lib", @@ -60,7 +61,6 @@ envoy_cc_test( "//test/mocks/server:factory_context_mocks", "//test/test_common:registry_lib", "//test/test_common:utility_lib", - "//source/common/access_log:access_log_lib", ], ) diff --git a/test/extensions/filters/network/generic_proxy/config_test.cc b/test/extensions/filters/network/generic_proxy/config_test.cc index 14c418c06b45b..d21737c819798 100644 --- a/test/extensions/filters/network/generic_proxy/config_test.cc +++ b/test/extensions/filters/network/generic_proxy/config_test.cc @@ -496,10 +496,11 @@ TEST(BasicFilterConfigTest, TestConfigurationWithAccessLogAndLogFilter1) { EXPECT_CALL(codec_factory_config, createCodecFactory(_, _)) .WillOnce(Return(testing::ByMove(std::move(mock_codec_factory)))); - EXPECT_THROW_WITH_MESSAGE( - { auto status_or = factory.createFilterFactoryFromProto(config, factory_context); }, - EnvoyException, - "Access log filter: only extension filter is supported by non-HTTP access loggers."); + Network::FilterFactoryCb cb = + factory.createFilterFactoryFromProto(config, factory_context).value(); + EXPECT_NE(nullptr, cb); + NiceMock filter_manager; + cb(filter_manager); } TEST(BasicFilterConfigTest, TestConfigurationWithAccessLogAndLogFilter2) { diff --git a/test/extensions/filters/network/generic_proxy/proxy_test.cc b/test/extensions/filters/network/generic_proxy/proxy_test.cc index 1dc67a8677804..f9addf64e52ae 100644 --- a/test/extensions/filters/network/generic_proxy/proxy_test.cc +++ b/test/extensions/filters/network/generic_proxy/proxy_test.cc @@ -2,11 +2,11 @@ #include #include +#include "source/common/access_log/access_log_impl.h" #include "source/common/formatter/substitution_format_string.h" #include "source/common/tracing/tracer_manager_impl.h" #include "source/extensions/access_loggers/common/file_access_log_impl.h" #include "source/extensions/filters/network/generic_proxy/proxy.h" -#include "source/common/access_log/access_log_impl.h" #include "test/extensions/filters/network/generic_proxy/fake_codec.h" #include "test/extensions/filters/network/generic_proxy/mocks/codec.h" @@ -114,7 +114,11 @@ class FilterConfigTest : public testing::Test { envoy::config::accesslog::v3::AccessLog config; config.mutable_typed_config()->PackFrom(file_log_config); config.set_name("file"); - return AccessLog::AccessLogFactory::fromProto(config, factory_context_); + + std::vector command_parsers; + command_parsers.push_back(createGenericProxyCommandParser()); + return AccessLog::AccessLogFactory::fromProto(config, factory_context_, + std::move(command_parsers)); } NiceMock factory_context_; From ef55040827cb8ec81075e0fe71b45a4d29731e90 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Feb 2025 06:39:13 +0000 Subject: [PATCH 11/11] build(deps): bump pygithub from 2.5.0 to 2.6.1 in /tools/base Bumps [pygithub](https://github.com/pygithub/pygithub) from 2.5.0 to 2.6.1. - [Release notes](https://github.com/pygithub/pygithub/releases) - [Changelog](https://github.com/PyGithub/PyGithub/blob/v2.6.1/doc/changes.rst) - [Commits](https://github.com/pygithub/pygithub/compare/v2.5.0...v2.6.1) --- updated-dependencies: - dependency-name: pygithub dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- tools/base/requirements.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/base/requirements.txt b/tools/base/requirements.txt index 5b0faf771eeb8..f7febf2414b98 100644 --- a/tools/base/requirements.txt +++ b/tools/base/requirements.txt @@ -1150,9 +1150,9 @@ pyflakes==3.2.0 \ --hash=sha256:1c61603ff154621fb2a9172037d84dca3500def8c8b630657d1701f026f8af3f \ --hash=sha256:84b5be138a2dfbb40689ca07e2152deb896a65c3a3e24c251c5c62489568074a # via flake8 -pygithub==2.5.0 \ - --hash=sha256:b0b635999a658ab8e08720bdd3318893ff20e2275f6446fcf35bf3f44f2c0fd2 \ - --hash=sha256:e1613ac508a9be710920d26eb18b1905ebd9926aa49398e88151c1b526aad3cf +pygithub==2.6.1 \ + --hash=sha256:6f2fa6d076ccae475f9fc392cc6cdbd54db985d4f69b8833a28397de75ed6ca3 \ + --hash=sha256:b5c035392991cca63959e9453286b41b54d83bf2de2daa7d7ff7e4312cebf3bf # via -r requirements.in pygments==2.18.0 \ --hash=sha256:786ff802f32e91311bff3889f6e9a86e81505fe99f2735bb6d60ae0c5004f199 \