From cefd660c089a697df9f3a82ab6d8119e2ed151dc Mon Sep 17 00:00:00 2001 From: code Date: Wed, 8 Nov 2023 02:54:37 +0800 Subject: [PATCH] access log formatter: use new formatter context rather than multiple parameters (1/3) (#30734) access log formatter: use new formatter context rather than multiple parameters (1/3) Additional Description: Part of templated formatter and access log support. HttpFormatterContext is used by the formatter to extract information of HTTP stream, like headers, trailers, etc. And now, this patchs will update the API of logger to take HttpFormatterContext rather than multiple independent parameters. NOTE 1: to ensure these changes could get reasonable review, I split it to three sub-patches. This is the first one and only update the emitLog(). NOTE 2: this PR won't change any logic or behavior be may effect building of third party folks. Sorry for that. :( NOTE3: this PR moved HttpFormatterContext to separated header file to avoid cycle dependency. NOTE4: this PR moved AccessLogInstanceFactory from envoy/server/access_log_config.h to envoy/access_log/access_log_config.h. Because this factory should be replaced by template finally and moved it in this patch could avoid further updates to loggers in following PRs. Risk Level: low. No logic update. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. Signed-off-by: wbpcode --- envoy/access_log/BUILD | 1 + envoy/access_log/access_log.h | 1 + envoy/access_log/access_log_config.h | 37 ++++++ envoy/formatter/BUILD | 13 +- envoy/formatter/http_formatter_context.h | 115 ++++++++++++++++ envoy/formatter/substitution_formatter.h | 102 +------------- envoy/server/BUILD | 10 -- envoy/server/access_log_config.h | 54 -------- source/common/access_log/BUILD | 1 - source/common/access_log/access_log_impl.cc | 3 +- source/common/access_log/access_log_impl.h | 2 +- .../access_loggers/common/access_log_base.cc | 7 +- .../access_loggers/common/access_log_base.h | 13 +- .../common/file_access_log_impl.cc | 12 +- .../common/file_access_log_impl.h | 7 +- .../common/stream_access_log_common_impl.h | 2 +- .../extensions/access_loggers/file/config.cc | 2 +- .../extensions/access_loggers/file/config.h | 4 +- source/extensions/access_loggers/grpc/BUILD | 4 +- .../access_loggers/grpc/http_config.cc | 2 +- .../access_loggers/grpc/http_config.h | 4 +- .../grpc/http_grpc_access_log_impl.cc | 19 +-- .../grpc/http_grpc_access_log_impl.h | 7 +- .../access_loggers/grpc/tcp_config.cc | 2 +- .../access_loggers/grpc/tcp_config.h | 4 +- .../grpc/tcp_grpc_access_log_impl.cc | 12 +- .../grpc/tcp_grpc_access_log_impl.h | 7 +- .../access_loggers/open_telemetry/BUILD | 2 +- .../open_telemetry/access_log_impl.cc | 15 +-- .../open_telemetry/access_log_impl.h | 7 +- .../access_loggers/open_telemetry/config.cc | 5 +- .../access_loggers/open_telemetry/config.h | 4 +- .../open_telemetry/substitution_formatter.cc | 26 ++-- .../open_telemetry/substitution_formatter.h | 13 +- .../access_loggers/stream/config.cc | 4 +- .../extensions/access_loggers/stream/config.h | 6 +- source/extensions/access_loggers/wasm/BUILD | 2 +- .../extensions/access_loggers/wasm/config.cc | 2 +- .../extensions/access_loggers/wasm/config.h | 4 +- .../common/access_log_base_test.cc | 4 +- .../access_loggers/grpc/http_config_test.cc | 9 +- .../access_loggers/grpc/tcp_config_test.cc | 9 +- .../open_telemetry/config_test.cc | 9 +- .../substitution_formatter_speed_test.cc | 10 +- .../substitution_formatter_test.cc | 124 ++++-------------- .../access_loggers/wasm/config_test.cc | 20 ++- test/integration/fake_access_log.h | 4 +- .../integration/tcp_proxy_integration_test.cc | 3 +- .../typed_metadata_integration_test.cc | 5 +- 49 files changed, 302 insertions(+), 432 deletions(-) create mode 100644 envoy/formatter/http_formatter_context.h delete mode 100644 envoy/server/access_log_config.h diff --git a/envoy/access_log/BUILD b/envoy/access_log/BUILD index 207c63e5a25e..925681617ba4 100644 --- a/envoy/access_log/BUILD +++ b/envoy/access_log/BUILD @@ -14,6 +14,7 @@ envoy_cc_library( deps = [ "//envoy/config:typed_config_interface", "//envoy/filesystem:filesystem_interface", + "//envoy/formatter:http_formatter_context_interface", "//envoy/http:header_map_interface", "//envoy/stream_info:stream_info_interface", "//source/common/protobuf", diff --git a/envoy/access_log/access_log.h b/envoy/access_log/access_log.h index 6bc75b9c398c..57c4bcea8f2e 100644 --- a/envoy/access_log/access_log.h +++ b/envoy/access_log/access_log.h @@ -6,6 +6,7 @@ #include "envoy/common/pure.h" #include "envoy/data/accesslog/v3/accesslog.pb.h" #include "envoy/filesystem/filesystem.h" +#include "envoy/formatter/http_formatter_context.h" #include "envoy/http/header_map.h" #include "envoy/stream_info/stream_info.h" diff --git a/envoy/access_log/access_log_config.h b/envoy/access_log/access_log_config.h index ee0810aa6e30..2e27bd85b472 100644 --- a/envoy/access_log/access_log_config.h +++ b/envoy/access_log/access_log_config.h @@ -84,5 +84,42 @@ template class AccessLogInstanceFactoryBase : public Config::Typ const std::string category_; }; +/** + * Implemented for each AccessLog::Instance and registered via Registry::registerFactory or the + * convenience class RegisterFactory. + */ +class AccessLogInstanceFactory : public Config::TypedFactory { +public: + ~AccessLogInstanceFactory() override = default; + + /** + * Create a particular AccessLog::Instance implementation from a config proto. If the + * implementation is unable to produce a factory with the provided parameters, it should throw an + * EnvoyException. The returned pointer should never be nullptr. + * @param config the custom configuration for this access log type. + * @param filter filter to determine whether a particular request should be logged. If no filter + * was specified in the configuration, argument will be nullptr. + * @param context access log context through which persistent resources can be accessed. + */ + virtual AccessLog::InstanceSharedPtr + createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::ListenerAccessLogFactoryContext& context) PURE; + + /** + * Create a particular AccessLog::Instance implementation from a config proto. If the + * implementation is unable to produce a factory with the provided parameters, it should throw an + * EnvoyException. The returned pointer should never be nullptr. + * @param config the custom configuration for this access log type. + * @param filter filter to determine whether a particular request should be logged. If no filter + * was specified in the configuration, argument will be nullptr. + * @param context general filter context through which persistent resources can be accessed. + */ + virtual AccessLog::InstanceSharedPtr + createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, + Server::Configuration::CommonFactoryContext& context) PURE; + + std::string category() const override { return "envoy.access_loggers"; } +}; + } // namespace AccessLog } // namespace Envoy diff --git a/envoy/formatter/BUILD b/envoy/formatter/BUILD index 86bb31ad37b7..d2da478b19c9 100644 --- a/envoy/formatter/BUILD +++ b/envoy/formatter/BUILD @@ -8,6 +8,17 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_library( + name = "http_formatter_context_interface", + hdrs = [ + "http_formatter_context.h", + ], + deps = [ + "//envoy/http:header_map_interface", + "@envoy_api//envoy/data/accesslog/v3:pkg_cc_proto", + ], +) + envoy_cc_library( name = "substitution_formatter_interface", hdrs = [ @@ -15,7 +26,7 @@ envoy_cc_library( "substitution_formatter_base.h", ], deps = [ - "//envoy/access_log:access_log_interface", + ":http_formatter_context_interface", "//envoy/config:typed_config_interface", "//envoy/http:header_map_interface", "//envoy/server:factory_context_interface", diff --git a/envoy/formatter/http_formatter_context.h b/envoy/formatter/http_formatter_context.h new file mode 100644 index 000000000000..3057f3747c59 --- /dev/null +++ b/envoy/formatter/http_formatter_context.h @@ -0,0 +1,115 @@ +#pragma once + +#include "envoy/data/accesslog/v3/accesslog.pb.h" +#include "envoy/http/header_map.h" + +namespace Envoy { +namespace Formatter { + +using AccessLogType = envoy::data::accesslog::v3::AccessLogType; + +/** + * HTTP specific substitution formatter context for HTTP access logs or formatters. + * TODO(wbpcode): maybe we should move this class to envoy/http folder and rename it + * for more general usage. + */ +class HttpFormatterContext { +public: + /** + * 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. + * + * @param request_headers supplies the request headers. + * @param response_headers supplies the response headers. + * @param response_trailers supplies the response trailers. + * @param local_reply_body supplies the local reply body. + * @param log_type supplies the access log type. + */ + HttpFormatterContext(const Http::RequestHeaderMap* request_headers = nullptr, + const Http::ResponseHeaderMap* response_headers = nullptr, + const Http::ResponseTrailerMap* response_trailers = nullptr, + absl::string_view local_reply_body = {}, + AccessLogType log_type = AccessLogType::NotSet); + /** + * Set or overwrite the request headers. + * @param request_headers supplies the request headers. + */ + HttpFormatterContext& setRequestHeaders(const Http::RequestHeaderMap& request_headers) { + request_headers_ = &request_headers; + return *this; + } + /** + * Set or overwrite the response headers. + * @param response_headers supplies the response headers. + */ + HttpFormatterContext& setResponseHeaders(const Http::ResponseHeaderMap& response_headers) { + response_headers_ = &response_headers; + return *this; + } + + /** + * Set or overwrite the response trailers. + * @param response_trailers supplies the response trailers. + */ + HttpFormatterContext& setResponseTrailers(const Http::ResponseTrailerMap& response_trailers) { + response_trailers_ = &response_trailers; + return *this; + } + + /** + * Set or overwrite the local reply body. + * @param local_reply_body supplies the local reply body. + */ + HttpFormatterContext& setLocalReplyBody(absl::string_view local_reply_body) { + local_reply_body_ = local_reply_body; + return *this; + } + + /** + * Set or overwrite the access log type. + * @param log_type supplies the access log type. + */ + HttpFormatterContext& setAccessLogType(AccessLogType log_type) { + log_type_ = log_type; + return *this; + } + + /** + * @return const Http::RequestHeaderMap& the request headers. Empty request header map if no + * request headers are available. + */ + const Http::RequestHeaderMap& requestHeaders() const; + + /** + * @return const Http::ResponseHeaderMap& the response headers. Empty respnose header map if + * no response headers are available. + */ + const Http::ResponseHeaderMap& responseHeaders() const; + + /** + * @return const Http::ResponseTrailerMap& the response trailers. Empty response trailer map + * if no response trailers are available. + */ + const Http::ResponseTrailerMap& responseTrailers() const; + + /** + * @return absl::string_view the local reply body. Empty if no local reply body. + */ + absl::string_view localReplyBody() const; + + /** + * @return AccessLog::AccessLogType the type of access log. NotSet if this is not used for + * access logging. + */ + AccessLogType accessLogType() const; + +private: + const Http::RequestHeaderMap* request_headers_{}; + const Http::ResponseHeaderMap* response_headers_{}; + const Http::ResponseTrailerMap* response_trailers_{}; + absl::string_view local_reply_body_{}; + AccessLogType log_type_{AccessLogType::NotSet}; +}; + +} // namespace Formatter +} // namespace Envoy diff --git a/envoy/formatter/substitution_formatter.h b/envoy/formatter/substitution_formatter.h index 01ffae286eee..60c413d018f8 100644 --- a/envoy/formatter/substitution_formatter.h +++ b/envoy/formatter/substitution_formatter.h @@ -1,112 +1,12 @@ #pragma once +#include "envoy/formatter/http_formatter_context.h" #include "envoy/formatter/substitution_formatter_base.h" #include "envoy/http/header_map.h" namespace Envoy { namespace Formatter { -/** - * HTTP specific substitution formatter context for HTTP access logs or formatters. - */ -class HttpFormatterContext { -public: - /** - * 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. - * - * @param request_headers supplies the request headers. - * @param response_headers supplies the response headers. - * @param response_trailers supplies the response trailers. - * @param local_reply_body supplies the local reply body. - * @param log_type supplies the access log type. - */ - HttpFormatterContext(const Http::RequestHeaderMap* request_headers = nullptr, - const Http::ResponseHeaderMap* response_headers = nullptr, - const Http::ResponseTrailerMap* response_trailers = nullptr, - absl::string_view local_reply_body = {}, - AccessLog::AccessLogType log_type = AccessLog::AccessLogType::NotSet); - /** - * Set or overwrite the request headers. - * @param request_headers supplies the request headers. - */ - HttpFormatterContext& setRequestHeaders(const Http::RequestHeaderMap& request_headers) { - request_headers_ = &request_headers; - return *this; - } - /** - * Set or overwrite the response headers. - * @param response_headers supplies the response headers. - */ - HttpFormatterContext& setResponseHeaders(const Http::ResponseHeaderMap& response_headers) { - response_headers_ = &response_headers; - return *this; - } - - /** - * Set or overwrite the response trailers. - * @param response_trailers supplies the response trailers. - */ - HttpFormatterContext& setResponseTrailers(const Http::ResponseTrailerMap& response_trailers) { - response_trailers_ = &response_trailers; - return *this; - } - - /** - * Set or overwrite the local reply body. - * @param local_reply_body supplies the local reply body. - */ - HttpFormatterContext& setLocalReplyBody(absl::string_view local_reply_body) { - local_reply_body_ = local_reply_body; - return *this; - } - - /** - * Set or overwrite the access log type. - * @param log_type supplies the access log type. - */ - HttpFormatterContext& setAccessLogType(AccessLog::AccessLogType log_type) { - log_type_ = log_type; - return *this; - } - - /** - * @return const Http::RequestHeaderMap& the request headers. Empty request header map if no - * request headers are available. - */ - const Http::RequestHeaderMap& requestHeaders() const; - - /** - * @return const Http::ResponseHeaderMap& the response headers. Empty respnose header map if - * no response headers are available. - */ - const Http::ResponseHeaderMap& responseHeaders() const; - - /** - * @return const Http::ResponseTrailerMap& the response trailers. Empty response trailer map - * if no response trailers are available. - */ - const Http::ResponseTrailerMap& responseTrailers() const; - - /** - * @return absl::string_view the local reply body. Empty if no local reply body. - */ - absl::string_view localReplyBody() const; - - /** - * @return AccessLog::AccessLogType the type of access log. NotSet if this is not used for - * access logging. - */ - AccessLog::AccessLogType accessLogType() const; - -private: - const Http::RequestHeaderMap* request_headers_{}; - const Http::ResponseHeaderMap* response_headers_{}; - const Http::ResponseTrailerMap* response_trailers_{}; - absl::string_view local_reply_body_{}; - AccessLog::AccessLogType log_type_{AccessLog::AccessLogType::NotSet}; -}; - using Formatter = FormatterBase; using FormatterPtr = std::unique_ptr; using FormatterConstSharedPtr = std::shared_ptr; diff --git a/envoy/server/BUILD b/envoy/server/BUILD index cb5c06ec04ed..a412c4e164dd 100644 --- a/envoy/server/BUILD +++ b/envoy/server/BUILD @@ -8,16 +8,6 @@ licenses(["notice"]) # Apache 2 envoy_package() -envoy_cc_library( - name = "access_log_config_interface", - hdrs = ["access_log_config.h"], - deps = [ - ":filter_config_interface", - "//envoy/access_log:access_log_interface", - "//source/common/protobuf", - ], -) - envoy_cc_library( name = "admin_interface", hdrs = ["admin.h"], diff --git a/envoy/server/access_log_config.h b/envoy/server/access_log_config.h deleted file mode 100644 index 0d07554a7d8b..000000000000 --- a/envoy/server/access_log_config.h +++ /dev/null @@ -1,54 +0,0 @@ -#pragma once - -#include - -#include "envoy/access_log/access_log.h" -#include "envoy/config/typed_config.h" -#include "envoy/server/filter_config.h" - -#include "source/common/protobuf/protobuf.h" - -namespace Envoy { -namespace Server { -namespace Configuration { - -/** - * Implemented for each AccessLog::Instance and registered via Registry::registerFactory or the - * convenience class RegisterFactory. - */ -class AccessLogInstanceFactory : public Config::TypedFactory { -public: - ~AccessLogInstanceFactory() override = default; - - /** - * Create a particular AccessLog::Instance implementation from a config proto. If the - * implementation is unable to produce a factory with the provided parameters, it should throw an - * EnvoyException. The returned pointer should never be nullptr. - * @param config the custom configuration for this access log type. - * @param filter filter to determine whether a particular request should be logged. If no filter - * was specified in the configuration, argument will be nullptr. - * @param context access log context through which persistent resources can be accessed. - */ - virtual AccessLog::InstanceSharedPtr - createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, - ListenerAccessLogFactoryContext& context) PURE; - - /** - * Create a particular AccessLog::Instance implementation from a config proto. If the - * implementation is unable to produce a factory with the provided parameters, it should throw an - * EnvoyException. The returned pointer should never be nullptr. - * @param config the custom configuration for this access log type. - * @param filter filter to determine whether a particular request should be logged. If no filter - * was specified in the configuration, argument will be nullptr. - * @param context general filter context through which persistent resources can be accessed. - */ - virtual AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, - AccessLog::FilterPtr&& filter, - CommonFactoryContext& context) PURE; - - std::string category() const override { return "envoy.access_loggers"; } -}; - -} // namespace Configuration -} // namespace Server -} // namespace Envoy diff --git a/source/common/access_log/BUILD b/source/common/access_log/BUILD index 92959e853067..c7aa9a6d8675 100644 --- a/source/common/access_log/BUILD +++ b/source/common/access_log/BUILD @@ -22,7 +22,6 @@ envoy_cc_library( "//envoy/filesystem:filesystem_interface", "//envoy/http:header_map_interface", "//envoy/runtime:runtime_interface", - "//envoy/server:access_log_config_interface", "//envoy/upstream:upstream_interface", "//source/common/common:assert_lib", "//source/common/common:utility_lib", diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc index 7df906595462..3e6751115f5d 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -341,8 +341,7 @@ InstanceSharedPtr makeAccessLogInstance(const envoy::config::accesslog::v3::Acce filter = FilterFactory::fromProto(config.filter(), context); } - auto& factory = - Config::Utility::getAndCheckFactory(config); + auto& factory = Config::Utility::getAndCheckFactory(config); ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( config, context.messageValidationVisitor(), factory); diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h index a60a09bbcb56..49efa084fb29 100644 --- a/source/common/access_log/access_log_impl.h +++ b/source/common/access_log/access_log_impl.h @@ -10,12 +10,12 @@ #include "envoy/config/accesslog/v3/accesslog.pb.h" #include "envoy/config/typed_config.h" #include "envoy/runtime/runtime.h" -#include "envoy/server/access_log_config.h" #include "envoy/type/v3/percent.pb.h" #include "source/common/common/matchers.h" #include "source/common/common/utility.h" #include "source/common/config/utility.h" +#include "source/common/formatter/http_specific_formatter.h" #include "source/common/grpc/status.h" #include "source/common/http/header_utility.h" #include "source/common/protobuf/protobuf.h" diff --git a/source/extensions/access_loggers/common/access_log_base.cc b/source/extensions/access_loggers/common/access_log_base.cc index f90781a86125..897e125dc688 100644 --- a/source/extensions/access_loggers/common/access_log_base.cc +++ b/source/extensions/access_loggers/common/access_log_base.cc @@ -13,6 +13,9 @@ void ImplBase::log(const Http::RequestHeaderMap* request_headers, const Http::ResponseTrailerMap* response_trailers, const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType access_log_type) { + Formatter::HttpFormatterContext log_context{ + request_headers, response_headers, response_trailers, {}, access_log_type}; + if (!request_headers) { request_headers = Http::StaticEmptyHeaders::get().request_headers.get(); } @@ -26,8 +29,8 @@ void ImplBase::log(const Http::RequestHeaderMap* request_headers, *response_trailers, access_log_type)) { return; } - return emitLog(*request_headers, *response_headers, *response_trailers, stream_info, - access_log_type); + + return emitLog(log_context, stream_info); } } // namespace Common diff --git a/source/extensions/access_loggers/common/access_log_base.h b/source/extensions/access_loggers/common/access_log_base.h index c15b6003c310..dca4fbf663c1 100644 --- a/source/extensions/access_loggers/common/access_log_base.h +++ b/source/extensions/access_loggers/common/access_log_base.h @@ -6,8 +6,8 @@ #include "envoy/access_log/access_log.h" #include "envoy/runtime/runtime.h" -#include "envoy/server/access_log_config.h" +#include "source/common/access_log/access_log_impl.h" #include "source/common/http/header_utility.h" #include "source/common/protobuf/protobuf.h" @@ -35,17 +35,12 @@ class ImplBase : public AccessLog::Instance { private: /** * Log a completed request. - * @param request_headers supplies the incoming request headers after filtering. - * @param response_headers supplies response headers. - * @param response_trailers supplies response trailers. + * @param context supplies the necessary context to log. * @param stream_info supplies additional information about the request not * contained in the request headers. */ - virtual void emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) PURE; + virtual void emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) PURE; AccessLog::FilterPtr filter_; }; diff --git a/source/extensions/access_loggers/common/file_access_log_impl.cc b/source/extensions/access_loggers/common/file_access_log_impl.cc index c06fff4fa8ee..e8dd581660ef 100644 --- a/source/extensions/access_loggers/common/file_access_log_impl.cc +++ b/source/extensions/access_loggers/common/file_access_log_impl.cc @@ -12,15 +12,9 @@ FileAccessLog::FileAccessLog(const Filesystem::FilePathAndType& access_log_file_ log_file_ = log_manager.createAccessLog(access_log_file_info); } -void FileAccessLog::emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { - log_file_->write( - formatter_->formatWithContext({&request_headers, &response_headers, &response_trailers, - absl::string_view(), access_log_type}, - stream_info)); +void FileAccessLog::emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) { + log_file_->write(formatter_->formatWithContext(context, stream_info)); } } // namespace File diff --git a/source/extensions/access_loggers/common/file_access_log_impl.h b/source/extensions/access_loggers/common/file_access_log_impl.h index 4b866879fbbb..457cdc50a07b 100644 --- a/source/extensions/access_loggers/common/file_access_log_impl.h +++ b/source/extensions/access_loggers/common/file_access_log_impl.h @@ -19,11 +19,8 @@ class FileAccessLog : public Common::ImplBase { private: // Common::ImplBase - void emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) override; + void emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) override; AccessLog::AccessLogFileSharedPtr log_file_; Formatter::FormatterPtr formatter_; 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 635a3ce6604e..f717e71769f3 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 @@ -1,6 +1,6 @@ #pragma once -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" #include "source/common/formatter/substitution_format_string.h" #include "source/common/formatter/substitution_formatter.h" diff --git a/source/extensions/access_loggers/file/config.cc b/source/extensions/access_loggers/file/config.cc index 008b715489ea..0f98633223f4 100644 --- a/source/extensions/access_loggers/file/config.cc +++ b/source/extensions/access_loggers/file/config.cc @@ -81,7 +81,7 @@ std::string FileAccessLogFactory::name() const { return "envoy.access_loggers.fi /** * Static registration for the file access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(FileAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(FileAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.file_access_log"); } // namespace File diff --git a/source/extensions/access_loggers/file/config.h b/source/extensions/access_loggers/file/config.h index 8a18d2903f31..6d0c40be4c15 100644 --- a/source/extensions/access_loggers/file/config.h +++ b/source/extensions/access_loggers/file/config.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" namespace Envoy { namespace Extensions { @@ -10,7 +10,7 @@ namespace File { /** * Config registration for the file access log. @see AccessLogInstanceFactory. */ -class FileAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class FileAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, diff --git a/source/extensions/access_loggers/grpc/BUILD b/source/extensions/access_loggers/grpc/BUILD index 45bec6334796..9be93daf2aa3 100644 --- a/source/extensions/access_loggers/grpc/BUILD +++ b/source/extensions/access_loggers/grpc/BUILD @@ -100,7 +100,7 @@ envoy_cc_extension( hdrs = ["http_config.h"], deps = [ ":config_utils", - "//envoy/server:access_log_config_interface", + "//envoy/access_log:access_log_config_interface", "//source/common/common:assert_lib", "//source/common/grpc:async_client_lib", "//source/common/protobuf", @@ -116,7 +116,7 @@ envoy_cc_extension( hdrs = ["tcp_config.h"], deps = [ ":config_utils", - "//envoy/server:access_log_config_interface", + "//envoy/access_log:access_log_config_interface", "//source/common/common:assert_lib", "//source/common/grpc:async_client_lib", "//source/common/protobuf", diff --git a/source/extensions/access_loggers/grpc/http_config.cc b/source/extensions/access_loggers/grpc/http_config.cc index 7b2e1fbedab9..ce7d055b6aac 100644 --- a/source/extensions/access_loggers/grpc/http_config.cc +++ b/source/extensions/access_loggers/grpc/http_config.cc @@ -49,7 +49,7 @@ std::string HttpGrpcAccessLogFactory::name() const { return "envoy.access_logger /** * Static registration for the HTTP gRPC access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(HttpGrpcAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(HttpGrpcAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.http_grpc_access_log"); } // namespace HttpGrpc diff --git a/source/extensions/access_loggers/grpc/http_config.h b/source/extensions/access_loggers/grpc/http_config.h index 066f0d495bb2..83555e5852ef 100644 --- a/source/extensions/access_loggers/grpc/http_config.h +++ b/source/extensions/access_loggers/grpc/http_config.h @@ -2,7 +2,7 @@ #include -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" namespace Envoy { namespace Extensions { @@ -12,7 +12,7 @@ namespace HttpGrpc { /** * Config registration for the HTTP gRPC access log. @see AccessLogInstanceFactory. */ -class HttpGrpcAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class HttpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, diff --git a/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc b/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc index b89f2cab0134..afa78ac36a43 100644 --- a/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc +++ b/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.cc @@ -50,17 +50,17 @@ HttpGrpcAccessLog::HttpGrpcAccessLog(AccessLog::FilterPtr&& filter, }); } -void HttpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { +void HttpGrpcAccessLog::emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) { // Common log properties. // TODO(mattklein123): Populate sample_rate field. envoy::data::accesslog::v3::HTTPAccessLogEntry log_entry; - GrpcCommon::Utility::extractCommonAccessLogProperties(*log_entry.mutable_common_properties(), - request_headers, stream_info, - config_->common_config(), access_log_type); + + const auto& request_headers = context.requestHeaders(); + + GrpcCommon::Utility::extractCommonAccessLogProperties( + *log_entry.mutable_common_properties(), request_headers, stream_info, + config_->common_config(), context.accessLogType()); if (stream_info.protocol()) { switch (stream_info.protocol().value()) { @@ -135,6 +135,9 @@ void HttpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_headers, } // HTTP response properties. + const auto& response_headers = context.responseHeaders(); + const auto& response_trailers = context.responseTrailers(); + auto* response_properties = log_entry.mutable_response(); if (stream_info.responseCode()) { response_properties->mutable_response_code()->set_value(stream_info.responseCode().value()); diff --git a/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h b/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h index f11d22cbd61c..d99add367f7c 100644 --- a/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h +++ b/source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h @@ -44,11 +44,8 @@ class HttpGrpcAccessLog : public Common::ImplBase { }; // Common::ImplBase - void emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) override; + void emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) override; const HttpGrpcAccessLogConfigConstSharedPtr config_; const ThreadLocal::SlotPtr tls_slot_; diff --git a/source/extensions/access_loggers/grpc/tcp_config.cc b/source/extensions/access_loggers/grpc/tcp_config.cc index 9f1fdb55a737..44089eee34c4 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.cc +++ b/source/extensions/access_loggers/grpc/tcp_config.cc @@ -48,7 +48,7 @@ std::string TcpGrpcAccessLogFactory::name() const { return "envoy.access_loggers /** * Static registration for the TCP gRPC access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(TcpGrpcAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(TcpGrpcAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.tcp_grpc_access_log"); } // namespace TcpGrpc diff --git a/source/extensions/access_loggers/grpc/tcp_config.h b/source/extensions/access_loggers/grpc/tcp_config.h index 1e3481e5fea2..a9be520affbb 100644 --- a/source/extensions/access_loggers/grpc/tcp_config.h +++ b/source/extensions/access_loggers/grpc/tcp_config.h @@ -2,7 +2,7 @@ #include -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" namespace Envoy { namespace Extensions { @@ -12,7 +12,7 @@ namespace TcpGrpc { /** * Config registration for the TCP gRPC access log. @see AccessLogInstanceFactory. */ -class TcpGrpcAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class TcpGrpcAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, diff --git a/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc b/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc index 10f9caa14a5d..288c002a40c5 100644 --- a/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc +++ b/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.cc @@ -32,15 +32,13 @@ TcpGrpcAccessLog::TcpGrpcAccessLog(AccessLog::FilterPtr&& filter, }); } -void TcpGrpcAccessLog::emitLog(const Http::RequestHeaderMap& request_header, - const Http::ResponseHeaderMap&, const Http::ResponseTrailerMap&, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { +void TcpGrpcAccessLog::emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) { // Common log properties. envoy::data::accesslog::v3::TCPAccessLogEntry log_entry; - GrpcCommon::Utility::extractCommonAccessLogProperties(*log_entry.mutable_common_properties(), - request_header, stream_info, - config_->common_config(), access_log_type); + GrpcCommon::Utility::extractCommonAccessLogProperties( + *log_entry.mutable_common_properties(), context.requestHeaders(), stream_info, + config_->common_config(), context.accessLogType()); envoy::data::accesslog::v3::ConnectionProperties& connection_properties = *log_entry.mutable_connection_properties(); diff --git a/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h b/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h index 71b5dfd81f9b..0bedd395cf6b 100644 --- a/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h +++ b/source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h @@ -43,11 +43,8 @@ class TcpGrpcAccessLog : public Common::ImplBase { }; // Common::ImplBase - void emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) override; + void emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) override; const TcpGrpcAccessLogConfigConstSharedPtr config_; const ThreadLocal::SlotPtr tls_slot_; diff --git a/source/extensions/access_loggers/open_telemetry/BUILD b/source/extensions/access_loggers/open_telemetry/BUILD index fef41675f552..d782d8252deb 100644 --- a/source/extensions/access_loggers/open_telemetry/BUILD +++ b/source/extensions/access_loggers/open_telemetry/BUILD @@ -62,7 +62,7 @@ envoy_cc_extension( srcs = ["config.cc"], hdrs = ["config.h"], deps = [ - "//envoy/server:access_log_config_interface", + "//envoy/access_log:access_log_config_interface", "//source/common/common:assert_lib", "//source/common/grpc:async_client_lib", "//source/common/protobuf", diff --git a/source/extensions/access_loggers/open_telemetry/access_log_impl.cc b/source/extensions/access_loggers/open_telemetry/access_log_impl.cc index 77c72c84fbd5..c742935af18e 100644 --- a/source/extensions/access_loggers/open_telemetry/access_log_impl.cc +++ b/source/extensions/access_loggers/open_telemetry/access_log_impl.cc @@ -76,11 +76,8 @@ AccessLog::AccessLog( attributes_formatter_ = std::make_unique(config.attributes()); } -void AccessLog::emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - Envoy::AccessLog::AccessLogType access_log_type) { +void AccessLog::emitLog(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { opentelemetry::proto::logs::v1::LogRecord log_entry; log_entry.set_time_unix_nano(std::chrono::duration_cast( stream_info.startTime().time_since_epoch()) @@ -88,14 +85,10 @@ void AccessLog::emitLog(const Http::RequestHeaderMap& request_headers, // Unpacking the body "KeyValueList" to "AnyValue". if (body_formatter_) { - const auto formatted_body = - unpackBody(body_formatter_->format(request_headers, response_headers, response_trailers, - stream_info, absl::string_view(), access_log_type)); + const auto formatted_body = unpackBody(body_formatter_->format(log_context, stream_info)); *log_entry.mutable_body() = formatted_body; } - const auto formatted_attributes = - attributes_formatter_->format(request_headers, response_headers, response_trailers, - stream_info, absl::string_view(), access_log_type); + const auto formatted_attributes = attributes_formatter_->format(log_context, stream_info); *log_entry.mutable_attributes() = formatted_attributes.values(); tls_slot_->getTyped().logger_->log(std::move(log_entry)); diff --git a/source/extensions/access_loggers/open_telemetry/access_log_impl.h b/source/extensions/access_loggers/open_telemetry/access_log_impl.h index be5456d08c8d..7d1013488eb4 100644 --- a/source/extensions/access_loggers/open_telemetry/access_log_impl.h +++ b/source/extensions/access_loggers/open_telemetry/access_log_impl.h @@ -49,11 +49,8 @@ class AccessLog : public Common::ImplBase { }; // Common::ImplBase - void emitLog(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, - Envoy::AccessLog::AccessLogType access_log_type) override; + void emitLog(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) override; const ThreadLocal::SlotPtr tls_slot_; const GrpcAccessLoggerCacheSharedPtr access_logger_cache_; diff --git a/source/extensions/access_loggers/open_telemetry/config.cc b/source/extensions/access_loggers/open_telemetry/config.cc index fd3477689fc2..228102a57018 100644 --- a/source/extensions/access_loggers/open_telemetry/config.cc +++ b/source/extensions/access_loggers/open_telemetry/config.cc @@ -3,7 +3,6 @@ #include "envoy/extensions/access_loggers/open_telemetry/v3/logs_service.pb.h" #include "envoy/extensions/access_loggers/open_telemetry/v3/logs_service.pb.validate.h" #include "envoy/registry/registry.h" -#include "envoy/server/access_log_config.h" #include "envoy/server/filter_config.h" #include "source/common/common/assert.h" @@ -63,8 +62,8 @@ std::string AccessLogFactory::name() const { return "envoy.access_loggers.open_t /** * Static registration for the OpenTelemetry (gRPC) access log. @see RegisterFactory. */ -REGISTER_FACTORY(AccessLogFactory, Server::Configuration::AccessLogInstanceFactory){ - "envoy.open_telemetry_access_log"}; +REGISTER_FACTORY(AccessLogFactory, + Envoy::AccessLog::AccessLogInstanceFactory){"envoy.open_telemetry_access_log"}; } // namespace OpenTelemetry } // namespace AccessLoggers diff --git a/source/extensions/access_loggers/open_telemetry/config.h b/source/extensions/access_loggers/open_telemetry/config.h index f71c541316d2..81631e814fe3 100644 --- a/source/extensions/access_loggers/open_telemetry/config.h +++ b/source/extensions/access_loggers/open_telemetry/config.h @@ -2,7 +2,7 @@ #include -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" namespace Envoy { namespace Extensions { @@ -12,7 +12,7 @@ namespace OpenTelemetry { /** * Config registration for the OpenTelemetry (gRPC) access log. @see AccessLogInstanceFactory. */ -class AccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class AccessLogFactory : public Envoy::AccessLog::AccessLogInstanceFactory { public: ::Envoy::AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, ::Envoy::AccessLog::FilterPtr&& filter, diff --git a/source/extensions/access_loggers/open_telemetry/substitution_formatter.cc b/source/extensions/access_loggers/open_telemetry/substitution_formatter.cc index 2d3c76544469..e48522bc54ef 100644 --- a/source/extensions/access_loggers/open_telemetry/substitution_formatter.cc +++ b/source/extensions/access_loggers/open_telemetry/substitution_formatter.cc @@ -82,21 +82,17 @@ OpenTelemetryFormatter::FormatBuilder::toFormatStringValue(const std::string& st ::opentelemetry::proto::common::v1::AnyValue OpenTelemetryFormatter::providersCallback( const std::vector& providers, - const Http::RequestHeaderMap& request_headers, const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, const StreamInfo::StreamInfo& stream_info, - absl::string_view local_reply_body, AccessLog::AccessLogType access_log_type) const { + const Formatter::HttpFormatterContext& context, const StreamInfo::StreamInfo& info) const { ASSERT(!providers.empty()); ::opentelemetry::proto::common::v1::AnyValue output; std::vector bits(providers.size()); - const Formatter::HttpFormatterContext formatter_context{ - &request_headers, &response_headers, &response_trailers, local_reply_body, access_log_type}; + std::transform( + providers.begin(), providers.end(), bits.begin(), + [&](const Formatter::FormatterProviderPtr& provider) { + return provider->formatWithContext(context, info).value_or(DefaultUnspecifiedValueString); + }); - std::transform(providers.begin(), providers.end(), bits.begin(), - [&](const Formatter::FormatterProviderPtr& provider) { - return provider->formatWithContext(formatter_context, stream_info) - .value_or(DefaultUnspecifiedValueString); - }); output.set_string_value(absl::StrJoin(bits, "")); return output; } @@ -128,14 +124,12 @@ OpenTelemetryFormatter::openTelemetryFormatListCallback( return output; } -::opentelemetry::proto::common::v1::KeyValueList OpenTelemetryFormatter::format( - const Http::RequestHeaderMap& request_headers, const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, const StreamInfo::StreamInfo& stream_info, - absl::string_view local_reply_body, AccessLog::AccessLogType access_log_type) const { +::opentelemetry::proto::common::v1::KeyValueList +OpenTelemetryFormatter::format(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) const { OpenTelemetryFormatMapVisitor visitor{ [&](const std::vector& providers) { - return providersCallback(providers, request_headers, response_headers, response_trailers, - stream_info, local_reply_body, access_log_type); + return providersCallback(providers, context, info); }, [&, this](const OpenTelemetryFormatter::OpenTelemetryFormatMapWrapper& format_map) { return openTelemetryFormatMapCallback(format_map, visitor); diff --git a/source/extensions/access_loggers/open_telemetry/substitution_formatter.h b/source/extensions/access_loggers/open_telemetry/substitution_formatter.h index 3ae2188f8461..ea797fff13e2 100644 --- a/source/extensions/access_loggers/open_telemetry/substitution_formatter.h +++ b/source/extensions/access_loggers/open_telemetry/substitution_formatter.h @@ -29,11 +29,7 @@ class OpenTelemetryFormatter { OpenTelemetryFormatter(const ::opentelemetry::proto::common::v1::KeyValueList& format_mapping); ::opentelemetry::proto::common::v1::KeyValueList - format(const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, absl::string_view local_reply_body, - AccessLog::AccessLogType access_log_type) const; + format(const Formatter::HttpFormatterContext& context, const StreamInfo::StreamInfo& info) const; private: struct OpenTelemetryFormatMapWrapper; @@ -77,11 +73,8 @@ class OpenTelemetryFormatter { // Methods for doing the actual formatting. ::opentelemetry::proto::common::v1::AnyValue providersCallback(const std::vector& providers, - const Http::RequestHeaderMap& request_headers, - const Http::ResponseHeaderMap& response_headers, - const Http::ResponseTrailerMap& response_trailers, - const StreamInfo::StreamInfo& stream_info, absl::string_view local_reply_body, - AccessLog::AccessLogType access_log_type) const; + const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) const; ::opentelemetry::proto::common::v1::AnyValue openTelemetryFormatMapCallback( const OpenTelemetryFormatter::OpenTelemetryFormatMapWrapper& format_map, const OpenTelemetryFormatMapVisitor& visitor) const; diff --git a/source/extensions/access_loggers/stream/config.cc b/source/extensions/access_loggers/stream/config.cc index e9159e6f3556..b607e9284df0 100644 --- a/source/extensions/access_loggers/stream/config.cc +++ b/source/extensions/access_loggers/stream/config.cc @@ -46,7 +46,7 @@ std::string StdoutAccessLogFactory::name() const { return "envoy.access_loggers. /** * Static registration for the file access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(StdoutAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(StdoutAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.stdout_access_log"); AccessLog::InstanceSharedPtr StderrAccessLogFactory::createAccessLogInstance( @@ -75,7 +75,7 @@ std::string StderrAccessLogFactory::name() const { return "envoy.access_loggers. /** * Static registration for the `stderr` access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(StderrAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(StderrAccessLogFactory, AccessLog::AccessLogInstanceFactory, "envoy.stderr_access_log"); } // namespace File diff --git a/source/extensions/access_loggers/stream/config.h b/source/extensions/access_loggers/stream/config.h index 6fb9522fd229..492e90acbb37 100644 --- a/source/extensions/access_loggers/stream/config.h +++ b/source/extensions/access_loggers/stream/config.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" namespace Envoy { namespace Extensions { @@ -10,7 +10,7 @@ namespace File { /** * Config registration for the standard output access log. @see AccessLogInstanceFactory. */ -class StdoutAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class StdoutAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, @@ -28,7 +28,7 @@ class StdoutAccessLogFactory : public Server::Configuration::AccessLogInstanceFa /** * Config registration for the standard error access log. @see AccessLogInstanceFactory. */ -class StderrAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class StderrAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message& config, AccessLog::FilterPtr&& filter, diff --git a/source/extensions/access_loggers/wasm/BUILD b/source/extensions/access_loggers/wasm/BUILD index 5765746237c4..077e8f0239ec 100644 --- a/source/extensions/access_loggers/wasm/BUILD +++ b/source/extensions/access_loggers/wasm/BUILD @@ -28,8 +28,8 @@ envoy_cc_extension( hdrs = ["config.h"], deps = [ ":wasm_access_log_lib", + "//envoy/access_log:access_log_config_interface", "//envoy/registry", - "//envoy/server:access_log_config_interface", "//source/common/config:datasource_lib", "//source/common/protobuf", "//source/extensions/common/wasm:wasm_lib", diff --git a/source/extensions/access_loggers/wasm/config.cc b/source/extensions/access_loggers/wasm/config.cc index e9c2772f92d5..e240faf4bd12 100644 --- a/source/extensions/access_loggers/wasm/config.cc +++ b/source/extensions/access_loggers/wasm/config.cc @@ -71,7 +71,7 @@ std::string WasmAccessLogFactory::name() const { return "envoy.access_loggers.wa /** * Static registration for the wasm access log. @see RegisterFactory. */ -LEGACY_REGISTER_FACTORY(WasmAccessLogFactory, Server::Configuration::AccessLogInstanceFactory, +LEGACY_REGISTER_FACTORY(WasmAccessLogFactory, Envoy::AccessLog::AccessLogInstanceFactory, "envoy.wasm_access_log"); } // namespace Wasm diff --git a/source/extensions/access_loggers/wasm/config.h b/source/extensions/access_loggers/wasm/config.h index e198f906f489..d197e0529199 100644 --- a/source/extensions/access_loggers/wasm/config.h +++ b/source/extensions/access_loggers/wasm/config.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" #include "source/common/config/datasource.h" @@ -12,7 +12,7 @@ namespace Wasm { /** * Config registration for the file access log. @see AccessLogInstanceFactory. */ -class WasmAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory, +class WasmAccessLogFactory : public AccessLog::AccessLogInstanceFactory, Logger::Loggable { public: AccessLog::InstanceSharedPtr diff --git a/test/extensions/access_loggers/common/access_log_base_test.cc b/test/extensions/access_loggers/common/access_log_base_test.cc index 72d88b87c3eb..74bcdf62d219 100644 --- a/test/extensions/access_loggers/common/access_log_base_test.cc +++ b/test/extensions/access_loggers/common/access_log_base_test.cc @@ -23,9 +23,7 @@ class TestImpl : public ImplBase { int count() { return count_; }; private: - void emitLog(const Http::RequestHeaderMap&, const Http::ResponseHeaderMap&, - const Http::ResponseTrailerMap&, const StreamInfo::StreamInfo&, - AccessLog::AccessLogType) override { + void emitLog(const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo&) override { count_++; } diff --git a/test/extensions/access_loggers/grpc/http_config_test.cc b/test/extensions/access_loggers/grpc/http_config_test.cc index 209763cf6f05..80a6d57de33a 100644 --- a/test/extensions/access_loggers/grpc/http_config_test.cc +++ b/test/extensions/access_loggers/grpc/http_config_test.cc @@ -1,7 +1,7 @@ +#include "envoy/access_log/access_log_config.h" #include "envoy/config/core/v3/grpc_service.pb.h" #include "envoy/extensions/access_loggers/grpc/v3/als.pb.h" #include "envoy/registry/registry.h" -#include "envoy/server/access_log_config.h" #include "envoy/stats/scope.h" #include "source/extensions/access_loggers/grpc/http_grpc_access_log_impl.h" @@ -23,9 +23,8 @@ namespace { class HttpGrpcAccessLogConfigTest : public testing::Test { public: void SetUp() override { - factory_ = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.http_grpc"); + factory_ = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.http_grpc"); ASSERT_NE(nullptr, factory_); message_ = factory_->createEmptyConfigProto(); @@ -61,7 +60,7 @@ class HttpGrpcAccessLogConfigTest : public testing::Test { NiceMock context_; envoy::extensions::access_loggers::grpc::v3::HttpGrpcAccessLogConfig http_grpc_access_log_; ProtobufTypes::MessagePtr message_; - Server::Configuration::AccessLogInstanceFactory* factory_{}; + AccessLog::AccessLogInstanceFactory* factory_{}; }; // Normal OK configuration. diff --git a/test/extensions/access_loggers/grpc/tcp_config_test.cc b/test/extensions/access_loggers/grpc/tcp_config_test.cc index bb18e7b81b42..268f54240b4b 100644 --- a/test/extensions/access_loggers/grpc/tcp_config_test.cc +++ b/test/extensions/access_loggers/grpc/tcp_config_test.cc @@ -1,7 +1,7 @@ +#include "envoy/access_log/access_log_config.h" #include "envoy/config/core/v3/grpc_service.pb.h" #include "envoy/extensions/access_loggers/grpc/v3/als.pb.h" #include "envoy/registry/registry.h" -#include "envoy/server/access_log_config.h" #include "envoy/stats/scope.h" #include "source/extensions/access_loggers/grpc/tcp_grpc_access_log_impl.h" @@ -23,9 +23,8 @@ namespace { class TcpGrpcAccessLogConfigTest : public testing::Test { public: void SetUp() override { - factory_ = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.tcp_grpc"); + factory_ = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.tcp_grpc"); ASSERT_NE(nullptr, factory_); message_ = factory_->createEmptyConfigProto(); @@ -61,7 +60,7 @@ class TcpGrpcAccessLogConfigTest : public testing::Test { NiceMock context_; envoy::extensions::access_loggers::grpc::v3::TcpGrpcAccessLogConfig tcp_grpc_access_log_; ProtobufTypes::MessagePtr message_; - Server::Configuration::AccessLogInstanceFactory* factory_{}; + AccessLog::AccessLogInstanceFactory* factory_{}; }; // Normal OK configuration. diff --git a/test/extensions/access_loggers/open_telemetry/config_test.cc b/test/extensions/access_loggers/open_telemetry/config_test.cc index 30db7a23d947..2890be64064e 100644 --- a/test/extensions/access_loggers/open_telemetry/config_test.cc +++ b/test/extensions/access_loggers/open_telemetry/config_test.cc @@ -1,7 +1,7 @@ +#include "envoy/access_log/access_log_config.h" #include "envoy/extensions/access_loggers/grpc/v3/als.pb.h" #include "envoy/extensions/access_loggers/open_telemetry/v3/logs_service.pb.h" #include "envoy/registry/registry.h" -#include "envoy/server/access_log_config.h" #include "envoy/stats/scope.h" #include "source/extensions/access_loggers/open_telemetry/access_log_impl.h" @@ -24,9 +24,8 @@ namespace { class OpenTelemetryAccessLogConfigTest : public testing::Test { public: void SetUp() override { - factory_ = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.open_telemetry"); + factory_ = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.open_telemetry"); ASSERT_NE(nullptr, factory_); message_ = factory_->createEmptyConfigProto(); @@ -49,7 +48,7 @@ class OpenTelemetryAccessLogConfigTest : public testing::Test { envoy::extensions::access_loggers::open_telemetry::v3::OpenTelemetryAccessLogConfig access_log_config_; ProtobufTypes::MessagePtr message_; - Server::Configuration::AccessLogInstanceFactory* factory_{}; + Envoy::AccessLog::AccessLogInstanceFactory* factory_{}; }; // Normal OK configuration. diff --git a/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc b/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc index 5cf53412cda3..11557702df2b 100644 --- a/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc +++ b/test/extensions/access_loggers/open_telemetry/substitution_formatter_speed_test.cc @@ -72,15 +72,9 @@ static void BM_OpenTelemetryAccessLogFormatter(benchmark::State& state) { std::unique_ptr otel_formatter = makeOpenTelemetryFormatter(); size_t output_bytes = 0; - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; - std::string body; + for (auto _ : state) { // NOLINT: Silences warning about dead store - output_bytes += otel_formatter - ->format(request_headers, response_headers, response_trailers, *stream_info, - body, AccessLog::AccessLogType::NotSet) - .ByteSize(); + output_bytes += otel_formatter->format({}, *stream_info).ByteSize(); } benchmark::DoNotOptimize(output_bytes); } diff --git a/test/extensions/access_loggers/open_telemetry/substitution_formatter_test.cc b/test/extensions/access_loggers/open_telemetry/substitution_formatter_test.cc index 1322b282ebf1..2adedb958afc 100644 --- a/test/extensions/access_loggers/open_telemetry/substitution_formatter_test.cc +++ b/test/extensions/access_loggers/open_telemetry/substitution_formatter_test.cc @@ -113,10 +113,6 @@ void verifyOpenTelemetryOutput(KeyValueList output, OpenTelemetryFormatMap expec TEST(SubstitutionFormatterTest, OpenTelemetryFormatterPlainStringTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); @@ -133,17 +129,11 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterPlainStringTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterTypesTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); @@ -198,18 +188,13 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterTypesTest) { - string_value: "HTTP/1.1" )EOF", expected); - const KeyValueList output = formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet); + const KeyValueList output = formatter.format({}, stream_info); EXPECT_TRUE(TestUtility::protoEqual(output, expected)); } // Test that nested values are formatted properly, including inter-type nesting. TEST(SubstitutionFormatterTest, OpenTelemetryFormatterNestedObjectsTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); @@ -411,17 +396,12 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterNestedObjectsTest) { - string_value: "HTTP/1.1" )EOF", expected); - const KeyValueList output = formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet); + const KeyValueList output = formatter.format({}, stream_info); EXPECT_TRUE(TestUtility::protoEqual(output, expected)); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterSingleOperatorTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); @@ -438,17 +418,11 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterSingleOperatorTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } TEST(SubstitutionFormatterTest, EmptyOpenTelemetryFormatterTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); @@ -465,17 +439,13 @@ TEST(SubstitutionFormatterTest, EmptyOpenTelemetryFormatterTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterNonExistentHeaderTest) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestResponseHeaderMapImpl response_header{{"some_response_header", "SOME_RESPONSE_HEADER"}}; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; OpenTelemetryFormatMap expected = {{"protocol", "HTTP/1.1"}, {"some_request_header", "SOME_REQUEST_HEADER"}, @@ -504,8 +474,7 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterNonExistentHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), + verifyOpenTelemetryOutput(formatter.format({&request_header, &response_header}, stream_info), expected); } @@ -515,8 +484,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterAlternateHeaderTest) { {"request_present_header", "REQUEST_PRESENT_HEADER"}}; Http::TestResponseHeaderMapImpl response_header{ {"response_present_header", "RESPONSE_PRESENT_HEADER"}}; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; OpenTelemetryFormatMap expected = { {"request_present_header_or_request_absent_header", "REQUEST_PRESENT_HEADER"}, @@ -546,8 +513,7 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterAlternateHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), + verifyOpenTelemetryOutput(formatter.format({&request_header, &response_header}, stream_info), expected); } @@ -556,7 +522,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterDynamicMetadataTest) { Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; Http::TestResponseTrailerMapImpl response_trailer{{"third", "POST"}, {"test-2", "test-2"}}; - std::string body; envoy::config::core::v3::Metadata metadata; populateMetadataTestData(metadata); @@ -583,9 +548,9 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterDynamicMetadataTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput( + formatter.format({&request_header, &response_header, &response_trailer}, stream_info), + expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataTest) { @@ -593,7 +558,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataTest) { Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; Http::TestResponseTrailerMapImpl response_trailer{{"third", "POST"}, {"test-2", "test-2"}}; - std::string body; envoy::config::core::v3::Metadata metadata; populateMetadataTestData(metadata); @@ -629,9 +593,9 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput( + formatter.format({&request_header, &response_header, &response_trailer}, stream_info), + expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataNoClusterInfoTest) { @@ -639,7 +603,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataNoClusterIn Http::TestRequestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestResponseHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; Http::TestResponseTrailerMapImpl response_trailer{{"third", "POST"}, {"test-2", "test-2"}}; - std::string body; OpenTelemetryFormatMap expected = {{"test_key", "-"}}; @@ -656,25 +619,21 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterClusterMetadataNoClusterIn // Empty optional (absl::nullopt) { EXPECT_CALL(Const(stream_info), upstreamClusterInfo()).WillOnce(Return(absl::nullopt)); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput( + formatter.format({&request_header, &response_header, &response_trailer}, stream_info), + expected); } // Empty cluster info (nullptr) { EXPECT_CALL(Const(stream_info), upstreamClusterInfo()).WillOnce(Return(nullptr)); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput( + formatter.format({&request_header, &response_header, &response_trailer}, stream_info), + expected); } } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterFilterStateTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; StreamInfo::MockStreamInfo stream_info; - std::string body; stream_info.filter_state_->setData("test_key", std::make_unique("test_value"), StreamInfo::FilterState::StateType::ReadOnly); @@ -699,17 +658,12 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterFilterStateTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_headers, response_headers, response_trailers, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterUpstreamFilterStateTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; + StreamInfo::MockStreamInfo stream_info; - std::string body; const StreamInfo::FilterStateSharedPtr upstream_filter_state = std::make_shared(StreamInfo::FilterState::LifeSpan::Request); @@ -744,19 +698,13 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterUpstreamFilterStateTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_headers, response_headers, response_trailers, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } // Test new specifier (PLAIN/TYPED) of FilterState. Ensure that after adding additional specifier, // the FilterState can call the serializeAsProto or serializeAsString methods correctly. TEST(SubstitutionFormatterTest, OpenTelemetryFormatterFilterStateSpeciferTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; StreamInfo::MockStreamInfo stream_info; - std::string body; stream_info.filter_state_->setData( "test_key", std::make_unique("test_value"), StreamInfo::FilterState::StateType::ReadOnly); @@ -780,19 +728,13 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterFilterStateSpeciferTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_headers, response_headers, response_trailers, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } // Test new specifier (PLAIN/TYPED) of FilterState. Ensure that after adding additional specifier, // the FilterState can call the serializeAsProto or serializeAsString methods correctly. TEST(SubstitutionFormatterTest, OpenTelemetryFormatterUpstreamFilterStateSpeciferTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; StreamInfo::MockStreamInfo stream_info; - std::string body; stream_info.upstream_info_ = std::make_shared(); stream_info.upstream_info_->setUpstreamFilterState( @@ -822,16 +764,11 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterUpstreamFilterStateSpecife key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_headers, response_headers, response_trailers, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } // Error specifier will cause an exception to be thrown. TEST(SubstitutionFormatterTest, OpenTelemetryFormatterFilterStateErrorSpeciferTest) { - Http::TestRequestHeaderMapImpl request_headers; - Http::TestResponseHeaderMapImpl response_headers; - Http::TestResponseTrailerMapImpl response_trailers; StreamInfo::MockStreamInfo stream_info; std::string body; stream_info.filter_state_->setData( @@ -887,10 +824,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterUpstreamFilterStateErrorSp TEST(SubstitutionFormatterTest, OpenTelemetryFormatterStartTimeTest) { StreamInfo::MockStreamInfo stream_info; - Http::TestRequestHeaderMapImpl request_header; - Http::TestResponseHeaderMapImpl response_header; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; time_t expected_time_in_epoch = 1522280158; SystemTime time = std::chrono::system_clock::from_time_t(expected_time_in_epoch); @@ -924,9 +857,7 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterStartTimeTest) { key_mapping); OpenTelemetryFormatter formatter(key_mapping); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), - expected); + verifyOpenTelemetryOutput(formatter.format({}, stream_info), expected); } TEST(SubstitutionFormatterTest, OpenTelemetryFormatterMultiTokenTest) { @@ -935,8 +866,6 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterMultiTokenTest) { Http::TestRequestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestResponseHeaderMapImpl response_header{ {"some_response_header", "SOME_RESPONSE_HEADER"}}; - Http::TestResponseTrailerMapImpl response_trailer; - std::string body; OpenTelemetryFormatMap expected = { {"multi_token_field", "HTTP/1.1 plainstring SOME_REQUEST_HEADER SOME_RESPONSE_HEADER"}}; @@ -954,8 +883,7 @@ TEST(SubstitutionFormatterTest, OpenTelemetryFormatterMultiTokenTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyOpenTelemetryOutput(formatter.format(request_header, response_header, response_trailer, - stream_info, body, AccessLog::AccessLogType::NotSet), + verifyOpenTelemetryOutput(formatter.format({&request_header, &response_header}, stream_info), expected); } } diff --git a/test/extensions/access_loggers/wasm/config_test.cc b/test/extensions/access_loggers/wasm/config_test.cc index 1ba26b57e53d..767a48d57d6d 100644 --- a/test/extensions/access_loggers/wasm/config_test.cc +++ b/test/extensions/access_loggers/wasm/config_test.cc @@ -71,9 +71,8 @@ INSTANTIATE_TEST_SUITE_P(Runtimes, WasmAccessLogConfigTest, Envoy::Extensions::Common::Wasm::wasmTestParamsToString); TEST_P(WasmAccessLogConfigTest, CreateWasmFromEmpty) { - auto factory = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.wasm"); + auto factory = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.wasm"); ASSERT_NE(factory, nullptr); ProtobufTypes::MessagePtr message = factory->createEmptyConfigProto(); @@ -88,9 +87,8 @@ TEST_P(WasmAccessLogConfigTest, CreateWasmFromEmpty) { } TEST_P(WasmAccessLogConfigTest, CreateWasmFromWASM) { - auto factory = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.wasm"); + auto factory = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.wasm"); ASSERT_NE(factory, nullptr); envoy::extensions::access_loggers::wasm::v3::WasmAccessLog config; @@ -135,9 +133,8 @@ TEST_P(WasmAccessLogConfigTest, YamlLoadFromFileWasmInvalidConfig) { if (std::get<0>(GetParam()) == "null") { return; } - auto factory = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.wasm"); + auto factory = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.wasm"); ASSERT_NE(factory, nullptr); const std::string invalid_yaml = @@ -245,9 +242,8 @@ TEST_P(WasmAccessLogConfigTest, FailedToGetThreadLocalPlugin) { if (std::get<0>(GetParam()) == "null") { return; } - auto factory = - Registry::FactoryRegistry::getFactory( - "envoy.access_loggers.wasm"); + auto factory = Registry::FactoryRegistry::getFactory( + "envoy.access_loggers.wasm"); ASSERT_NE(factory, nullptr); NiceMock threadlocal; diff --git a/test/integration/fake_access_log.h b/test/integration/fake_access_log.h index 49c339aef2ff..e21cc44c0712 100644 --- a/test/integration/fake_access_log.h +++ b/test/integration/fake_access_log.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/server/access_log_config.h" +#include "envoy/access_log/access_log_config.h" #include "source/common/protobuf/protobuf.h" @@ -30,7 +30,7 @@ class FakeAccessLog : public AccessLog::Instance { LogSignature log_cb_; }; -class FakeAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class FakeAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance(const Protobuf::Message&, AccessLog::FilterPtr&&, diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 439f109445e4..25d73e3740e3 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -1002,8 +1002,7 @@ TEST_P(TcpProxyIntegrationTest, RecordsUpstreamConnectionTimeLatency) { stream_info.upstreamInfo()->upstreamTiming().connectionPoolCallbackLatency().has_value()); }); - Registry::InjectFactory factory_register( - factory); + Registry::InjectFactory factory_register(factory); config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* filter_chain = diff --git a/test/integration/typed_metadata_integration_test.cc b/test/integration/typed_metadata_integration_test.cc index e5c8ce2dc2dc..82ba73d62c37 100644 --- a/test/integration/typed_metadata_integration_test.cc +++ b/test/integration/typed_metadata_integration_test.cc @@ -51,7 +51,7 @@ class MockAccessLog : public AccessLog::Instance { AccessLog::AccessLogType)); }; -class TestAccessLogFactory : public Server::Configuration::AccessLogInstanceFactory { +class TestAccessLogFactory : public AccessLog::AccessLogInstanceFactory { public: AccessLog::InstanceSharedPtr createAccessLogInstance( const Protobuf::Message&, AccessLog::FilterPtr&&, @@ -82,8 +82,7 @@ class TestAccessLogFactory : public Server::Configuration::AccessLogInstanceFact // Validate that access logger gets the right context with access to listener metadata TEST_P(ListenerTypedMetadataIntegrationTest, ListenerMetadataPlumbingToAccessLog) { TestAccessLogFactory factory; - Registry::InjectFactory factory_register( - factory); + Registry::InjectFactory factory_register(factory); // Add some typed metadata to the listener. ProtobufWkt::StringValue value;