diff --git a/contrib/golang/filters/http/source/golang_filter.cc b/contrib/golang/filters/http/source/golang_filter.cc index 01642e6ac74b..306336fc6000 100644 --- a/contrib/golang/filters/http/source/golang_filter.cc +++ b/contrib/golang/filters/http/source/golang_filter.cc @@ -211,11 +211,10 @@ void Filter::onDestroy() { } // access_log is executed before the log of the stream filter -void Filter::log(const Http::RequestHeaderMap* headers, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo&, - Envoy::AccessLog::AccessLogType type) { +void Filter::log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo&) { // `log` may be called multiple times with different log type - switch (type) { + switch (log_context.accessLogType()) { case Envoy::AccessLog::AccessLogType::DownstreamStart: case Envoy::AccessLog::AccessLogType::DownstreamPeriodic: case Envoy::AccessLog::AccessLogType::DownstreamEnd: { @@ -226,12 +225,12 @@ void Filter::log(const Http::RequestHeaderMap* headers, const Http::ResponseHead initRequest(state); request_headers_ = static_cast( - const_cast(headers)); + const_cast(&log_context.requestHeaders())); } state.enterLog(); req_->phase = static_cast(state.phase()); - dynamic_lib_->envoyGoFilterOnHttpLog(req_, int(type)); + dynamic_lib_->envoyGoFilterOnHttpLog(req_, int(log_context.accessLogType())); state.leaveLog(); } break; default: diff --git a/contrib/golang/filters/http/source/golang_filter.h b/contrib/golang/filters/http/source/golang_filter.h index 925c182e44ae..cb342c437327 100644 --- a/contrib/golang/filters/http/source/golang_filter.h +++ b/contrib/golang/filters/http/source/golang_filter.h @@ -204,11 +204,8 @@ class Filter : public Http::StreamFilter, } // AccessLog::Instance - void log(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 log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info) override; void onStreamComplete() override {} diff --git a/envoy/access_log/access_log.h b/envoy/access_log/access_log.h index a3aadb4538c2..460a18f96c49 100644 --- a/envoy/access_log/access_log.h +++ b/envoy/access_log/access_log.h @@ -99,28 +99,9 @@ using Filter = FilterBase; using FilterPtr = std::unique_ptr; /** - * Abstract access logger for requests and connections. + * Abstract access logger for HTTP requests and TCP connections. */ -class Instance { -public: - virtual ~Instance() = default; - - /** - * 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 stream_info supplies additional information about the request not - * contained in the request headers. - * @param access_log_type supplies additional information about the type of the - * log record, i.e the location in the code which recorded the log. - */ - virtual void log(const Http::RequestHeaderMap* request_headers, - const Http::ResponseHeaderMap* response_headers, - const Http::ResponseTrailerMap* response_trailers, - const StreamInfo::StreamInfo& stream_info, AccessLogType access_log_type) PURE; -}; - +using Instance = InstanceBase; using InstanceSharedPtr = std::shared_ptr; } // namespace AccessLog diff --git a/envoy/access_log/access_log_config.h b/envoy/access_log/access_log_config.h index 8252d7d42d5c..6f6328003ba4 100644 --- a/envoy/access_log/access_log_config.h +++ b/envoy/access_log/access_log_config.h @@ -58,8 +58,7 @@ using ExtensionFilterFactory = ExtensionFilterFactoryBase class AccessLogInstanceFactoryBase : public Config::TypedFactory { public: - AccessLogInstanceFactoryBase() - : category_(fmt::format("envoy.{}.access_loggers", Context::category())) {} + AccessLogInstanceFactoryBase() : category_(categoryByType()) {} ~AccessLogInstanceFactoryBase() override = default; @@ -94,45 +93,19 @@ template class AccessLogInstanceFactoryBase : public Config::Typ std::string category() const override { return category_; } private: + std::string categoryByType() { + if constexpr (std::is_same_v) { + // This is a special case for the HTTP formatter context to ensure backwards compatibility. + return "envoy.access_loggers"; + } else { + return fmt::format("envoy.{}.access_loggers", Context::category()); + } + } + 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"; } -}; +using AccessLogInstanceFactory = AccessLogInstanceFactoryBase; } // namespace AccessLog } // namespace Envoy diff --git a/source/common/access_log/BUILD b/source/common/access_log/BUILD index c7aa9a6d8675..869662afa0f4 100644 --- a/source/common/access_log/BUILD +++ b/source/common/access_log/BUILD @@ -26,6 +26,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:utility_lib", "//source/common/config:utility_lib", + "//source/common/formatter:substitution_formatter_lib", "//source/common/http:header_map_lib", "//source/common/http:header_utility_lib", "//source/common/http:headers_lib", diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index d972f693a651..e859b5bd452d 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -688,22 +688,15 @@ class FilterManager : public ScopeTrackedObject, void applyFilterFactoryCb(FilterContext context, FilterFactoryCb& factory) override; void log(AccessLog::AccessLogType access_log_type) { - RequestHeaderMap* request_headers = nullptr; - if (filter_manager_callbacks_.requestHeaders()) { - request_headers = filter_manager_callbacks_.requestHeaders().ptr(); - } - ResponseHeaderMap* response_headers = nullptr; - if (filter_manager_callbacks_.responseHeaders()) { - response_headers = filter_manager_callbacks_.responseHeaders().ptr(); - } - ResponseTrailerMap* response_trailers = nullptr; - if (filter_manager_callbacks_.responseTrailers()) { - response_trailers = filter_manager_callbacks_.responseTrailers().ptr(); - } + const Formatter::HttpFormatterContext log_context{ + filter_manager_callbacks_.requestHeaders().ptr(), + filter_manager_callbacks_.responseHeaders().ptr(), + filter_manager_callbacks_.responseTrailers().ptr(), + {}, + access_log_type}; for (const auto& log_handler : access_log_handlers_) { - log_handler->log(request_headers, response_headers, response_trailers, streamInfo(), - access_log_type); + log_handler->log(log_context, streamInfo()); } } diff --git a/source/common/quic/quic_stats_gatherer.cc b/source/common/quic/quic_stats_gatherer.cc index d7acb9ffc0e6..cd663aa3cb15 100644 --- a/source/common/quic/quic_stats_gatherer.cc +++ b/source/common/quic/quic_stats_gatherer.cc @@ -28,12 +28,15 @@ void QuicStatsGatherer::maybeDoDeferredLog(bool record_ack_timing) { } stream_info_->addBytesRetransmitted(retransmitted_bytes_); stream_info_->addPacketsRetransmitted(retransmitted_packets_); - const Http::RequestHeaderMap* request_headers = request_header_map_.get(); - const Http::ResponseHeaderMap* response_headers = response_header_map_.get(); - const Http::ResponseTrailerMap* response_trailers = response_trailer_map_.get(); + + const Formatter::HttpFormatterContext log_context{request_header_map_.get(), + response_header_map_.get(), + response_trailer_map_.get(), + {}, + AccessLog::AccessLogType::DownstreamEnd}; + for (const AccessLog::InstanceSharedPtr& log_handler : access_log_handlers_) { - log_handler->log(request_headers, response_headers, response_trailers, *stream_info_, - AccessLog::AccessLogType::DownstreamEnd); + log_handler->log(log_context, *stream_info_); } } diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 54d940b3c36a..d26c375932e2 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -231,9 +231,14 @@ void UpstreamRequest::cleanUp() { } void UpstreamRequest::upstreamLog(AccessLog::AccessLogType access_log_type) { + const Formatter::HttpFormatterContext log_context{parent_.downstreamHeaders(), + upstream_headers_.get(), + upstream_trailers_.get(), + {}, + access_log_type}; + for (const auto& upstream_log : parent_.config().upstream_logs_) { - upstream_log->log(parent_.downstreamHeaders(), upstream_headers_.get(), - upstream_trailers_.get(), stream_info_, access_log_type); + upstream_log->log(log_context, stream_info_); } } diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index a5a7eed68c5c..1ce46d0f895c 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -223,10 +223,12 @@ Filter::~Filter() { // Disable access log flush timer if it is enabled. disableAccessLogFlushTimer(); + const Formatter::HttpFormatterContext log_context{ + nullptr, nullptr, nullptr, {}, AccessLog::AccessLogType::TcpConnectionEnd}; + // Flush the final end stream access log entry. for (const auto& access_log : config_->accessLogs()) { - access_log->log(nullptr, nullptr, nullptr, getStreamInfo(), - AccessLog::AccessLogType::TcpConnectionEnd); + access_log->log(log_context, getStreamInfo()); } ASSERT(generic_conn_pool_ == nullptr); @@ -852,9 +854,11 @@ void Filter::onUpstreamConnection() { } if (config_->flushAccessLogOnConnected()) { + const Formatter::HttpFormatterContext log_context{ + nullptr, nullptr, nullptr, {}, AccessLog::AccessLogType::TcpUpstreamConnected}; + for (const auto& access_log : config_->accessLogs()) { - access_log->log(nullptr, nullptr, nullptr, getStreamInfo(), - AccessLog::AccessLogType::TcpUpstreamConnected); + access_log->log(log_context, getStreamInfo()); } } } @@ -878,9 +882,11 @@ void Filter::onMaxDownstreamConnectionDuration() { } void Filter::onAccessLogFlushInterval() { + const Formatter::HttpFormatterContext log_context{ + nullptr, nullptr, nullptr, {}, AccessLog::AccessLogType::TcpPeriodic}; + for (const auto& access_log : config_->accessLogs()) { - access_log->log(nullptr, nullptr, nullptr, getStreamInfo(), - AccessLog::AccessLogType::TcpPeriodic); + access_log->log(log_context, getStreamInfo()); } const SystemTime now = read_callbacks_->connection().dispatcher().timeSource().systemTime(); getStreamInfo().getDownstreamBytesMeter()->takeDownstreamPeriodicLoggingSnapshot(now); diff --git a/source/extensions/access_loggers/common/access_log_base.cc b/source/extensions/access_loggers/common/access_log_base.cc index 1296e0eb72d7..01f114297e21 100644 --- a/source/extensions/access_loggers/common/access_log_base.cc +++ b/source/extensions/access_loggers/common/access_log_base.cc @@ -8,13 +8,8 @@ namespace Extensions { namespace AccessLoggers { namespace Common { -void ImplBase::log(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) { - Formatter::HttpFormatterContext log_context{ - request_headers, response_headers, response_trailers, {}, access_log_type}; +void ImplBase::log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { if (filter_ && !filter_->evaluate(log_context, stream_info)) { return; diff --git a/source/extensions/access_loggers/common/access_log_base.h b/source/extensions/access_loggers/common/access_log_base.h index dca4fbf663c1..35ba33344f72 100644 --- a/source/extensions/access_loggers/common/access_log_base.h +++ b/source/extensions/access_loggers/common/access_log_base.h @@ -26,11 +26,8 @@ class ImplBase : public AccessLog::Instance { /** * Log a completed request if the underlying AccessLog `filter_` allows it. */ - void log(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 log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) override; private: /** diff --git a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h index dec9cf8e46b9..fbfb17fa70dc 100644 --- a/source/extensions/access_loggers/wasm/wasm_access_log_impl.h +++ b/source/extensions/access_loggers/wasm/wasm_access_log_impl.h @@ -20,30 +20,20 @@ class WasmAccessLog : public AccessLog::Instance { AccessLog::FilterPtr filter) : plugin_(plugin), tls_slot_(std::move(tls_slot)), filter_(std::move(filter)) {} - void log(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 log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) override { if (filter_) { - const Formatter::HttpFormatterContext log_context{ - request_headers, response_headers, response_trailers, {}, access_log_type}; - if (!filter_->evaluate(log_context, stream_info)) { return; } } - if (!tls_slot_) { - return; - } - auto handle = tls_slot_->get()->handle(); - if (!handle) { - return; - } - if (handle->wasmHandle()) { - handle->wasmHandle()->wasm()->log(plugin_, request_headers, response_headers, - response_trailers, stream_info, access_log_type); + if (tls_slot_ != nullptr) { + if (auto handle = tls_slot_->get()->handle(); handle != nullptr) { + if (handle->wasmHandle()) { + handle->wasmHandle()->wasm()->log(plugin_, log_context, stream_info); + } + } } } diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index e43f296255e9..57c0c24146b9 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -1476,10 +1476,8 @@ void Context::initializeWriteFilterCallbacks(Network::WriteFilterCallbacks& call network_write_filter_callbacks_ = &callbacks; } -void Context::log(const Http::RequestHeaderMap* request_headers, - const Http::ResponseHeaderMap* response_headers, - const Http::ResponseTrailerMap* response_trailers, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { +void Context::log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { // `log` may be called multiple times due to mid-request logging -- we only want to run on the // last call. if (!stream_info.requestComplete().has_value()) { @@ -1495,10 +1493,10 @@ void Context::log(const Http::RequestHeaderMap* request_headers, } access_log_phase_ = true; - access_log_request_headers_ = request_headers; + access_log_request_headers_ = &log_context.requestHeaders(); // ? request_trailers ? - access_log_response_headers_ = response_headers; - access_log_response_trailers_ = response_trailers; + access_log_response_headers_ = &log_context.responseHeaders(); + access_log_response_trailers_ = &log_context.responseTrailers(); access_log_stream_info_ = &stream_info; onLog(); diff --git a/source/extensions/common/wasm/context.h b/source/extensions/common/wasm/context.h index e755d26a0871..9a6f556b1141 100644 --- a/source/extensions/common/wasm/context.h +++ b/source/extensions/common/wasm/context.h @@ -148,11 +148,8 @@ class Context : public proxy_wasm::ContextBase, const std::shared_ptr& plugin); // deprecated // AccessLog::Instance - void log(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 log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info) override; uint32_t getLogLevel() override; diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index 17d40d003a9c..c8e969f0793b 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -226,13 +226,10 @@ ContextBase* Wasm::createRootContext(const std::shared_ptr& plugin) ContextBase* Wasm::createVmContext() { return new Context(this); } -void Wasm::log(const PluginSharedPtr& plugin, 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 Wasm::log(const PluginSharedPtr& plugin, const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info) { auto context = getRootContext(plugin, true); - context->log(request_headers, response_headers, response_trailers, stream_info, access_log_type); + context->log(log_context, info); } void Wasm::onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot) { diff --git a/source/extensions/common/wasm/wasm.h b/source/extensions/common/wasm/wasm.h index dc2b5d704d16..17cf43e028b0 100644 --- a/source/extensions/common/wasm/wasm.h +++ b/source/extensions/common/wasm/wasm.h @@ -66,10 +66,8 @@ class Wasm : public WasmBase, Logger::Loggable { void getFunctions() override; // AccessLog::Instance - void log(const PluginSharedPtr& plugin, 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 log(const PluginSharedPtr& plugin, const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info); void onStatsUpdate(const PluginSharedPtr& plugin, Envoy::Stats::MetricSnapshot& snapshot); diff --git a/source/extensions/filters/http/composite/filter.h b/source/extensions/filters/http/composite/filter.h index bd0e56c8571a..9f73a566b034 100644 --- a/source/extensions/filters/http/composite/filter.h +++ b/source/extensions/filters/http/composite/filter.h @@ -75,13 +75,10 @@ class Filter : public Http::StreamFilter, void onMatchCallback(const Matcher::Action& action) override; // AccessLog::Instance - void log(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 log(const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& info) override { for (const auto& log : access_loggers_) { - log->log(request_headers, response_headers, response_trailers, stream_info, access_log_type); + log->log(log_context, info); } } diff --git a/source/extensions/filters/http/tap/tap_filter.cc b/source/extensions/filters/http/tap/tap_filter.cc index d701a882873c..51719e068368 100644 --- a/source/extensions/filters/http/tap/tap_filter.cc +++ b/source/extensions/filters/http/tap/tap_filter.cc @@ -69,9 +69,7 @@ Http::FilterTrailersStatus Filter::encodeTrailers(Http::ResponseTrailerMap& trai return Http::FilterTrailersStatus::Continue; } -void Filter::log(const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo&, - AccessLog::AccessLogType) { +void Filter::log(const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo&) { if (tapper_ != nullptr && tapper_->onDestroyLog()) { config_->stats().rq_tapped_.inc(); } diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h index febd06b6368c..b779e6f8426b 100644 --- a/source/extensions/filters/http/tap/tap_filter.h +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -122,11 +122,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { void setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks&) override {} // AccessLog::Instance - void log(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 log(const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo&) override; private: FilterConfigSharedPtr config_; diff --git a/source/extensions/filters/network/thrift_proxy/BUILD b/source/extensions/filters/network/thrift_proxy/BUILD index 6e6faafb15d0..2d2b0448d640 100644 --- a/source/extensions/filters/network/thrift_proxy/BUILD +++ b/source/extensions/filters/network/thrift_proxy/BUILD @@ -52,7 +52,6 @@ envoy_cc_extension( ":unframed_transport_lib", "//envoy/access_log:access_log_interface", "//envoy/registry", - "//source/common/access_log:access_log_lib", "//source/common/config:utility_lib", "//source/extensions/filters/network:well_known_names", "//source/extensions/filters/network/common:factory_base_lib", @@ -85,6 +84,7 @@ envoy_cc_library( "//envoy/network:filter_interface", "//envoy/stats:stats_interface", "//envoy/stats:timespan_interface", + "//source/common/access_log:access_log_lib", "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:linked_object", diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.cc b/source/extensions/filters/network/thrift_proxy/conn_manager.cc index 77bc7f02da24..b98471fe8720 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.cc +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.cc @@ -54,9 +54,10 @@ Network::FilterStatus ConnectionManager::onData(Buffer::Instance& data, bool end void ConnectionManager::emitLogEntry(const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const StreamInfo::StreamInfo& stream_info) { + const Formatter::HttpFormatterContext log_context{request_headers, response_headers}; + for (const auto& access_log : config_.accessLogs()) { - access_log->log(request_headers, response_headers, nullptr, stream_info, - AccessLog::AccessLogType::NotSet); + access_log->log(log_context, stream_info); } } diff --git a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc index baf567bc1438..4dd49ecbb703 100644 --- a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc +++ b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.cc @@ -34,8 +34,7 @@ UdpProxyFilter::~UdpProxyFilter() { if (!config_->proxyAccessLogs().empty()) { fillProxyStreamInfo(); for (const auto& access_log : config_->proxyAccessLogs()) { - access_log->log(nullptr, nullptr, nullptr, udp_proxy_stats_.value(), - AccessLog::AccessLogType::NotSet); + access_log->log({}, udp_proxy_stats_.value()); } } } @@ -312,8 +311,7 @@ UdpProxyFilter::ActiveSession::~ActiveSession() { if (!cluster_.filter_.config_->sessionAccessLogs().empty()) { fillSessionStreamInfo(); for (const auto& access_log : cluster_.filter_.config_->sessionAccessLogs()) { - access_log->log(nullptr, nullptr, nullptr, udp_session_info_, - AccessLog::AccessLogType::NotSet); + access_log->log({}, udp_session_info_); } } } diff --git a/source/extensions/listener_managers/listener_manager/BUILD b/source/extensions/listener_managers/listener_manager/BUILD index fd51badb09d8..b8161fabbec1 100644 --- a/source/extensions/listener_managers/listener_manager/BUILD +++ b/source/extensions/listener_managers/listener_manager/BUILD @@ -235,6 +235,7 @@ envoy_cc_library( "//envoy/server:listener_manager_interface", "//source/common/common:assert_lib", "//source/common/common:linked_object", + "//source/common/formatter:substitution_formatter_lib", "//source/common/network:connection_lib", "//source/common/network:generic_listener_filter_impl_base_lib", "//source/common/stats:timespan_lib", diff --git a/source/extensions/listener_managers/listener_manager/active_stream_listener_base.cc b/source/extensions/listener_managers/listener_manager/active_stream_listener_base.cc index b56dbf8bda7f..32aaae137218 100644 --- a/source/extensions/listener_managers/listener_manager/active_stream_listener_base.cc +++ b/source/extensions/listener_managers/listener_manager/active_stream_listener_base.cc @@ -31,7 +31,7 @@ void ActiveStreamListenerBase::emitLogs(Network::ListenerConfig& config, StreamInfo::StreamInfo& stream_info) { stream_info.onRequestComplete(); for (const auto& access_log : config.accessLogs()) { - access_log->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log->log({}, stream_info); } } diff --git a/source/extensions/listener_managers/listener_manager/active_stream_listener_base.h b/source/extensions/listener_managers/listener_manager/active_stream_listener_base.h index 7ec1c509d575..fbfd919b6ff1 100644 --- a/source/extensions/listener_managers/listener_manager/active_stream_listener_base.h +++ b/source/extensions/listener_managers/listener_manager/active_stream_listener_base.h @@ -14,6 +14,7 @@ #include "envoy/stream_info/stream_info.h" #include "source/common/common/linked_object.h" +#include "source/common/formatter/http_specific_formatter.h" #include "source/extensions/listener_managers/listener_manager/active_tcp_socket.h" #include "source/server/active_listener_base.h" diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index 2d6cfda8f2c5..c005764bdc74 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -93,8 +93,7 @@ name: accesslog request_headers_.addCopy(Http::Headers::get().Host, "host"); request_headers_.addCopy(Http::Headers::get().ForwardedFor, "x.x.x.x"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 UF 1 2 3 - \"x.x.x.x\" " "\"user-agent-set\" \"id\" \"host\" \"-\"\n", output_); @@ -117,8 +116,7 @@ name: accesslog Upstream::makeTestHostDescription(cluster, "tcp://10.0.0.5:1234", simTime())); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::DownstreamConnectionTermination); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 DC 1 2 3 - \"-\" \"-\" \"-\" \"-\" " "\"10.0.0.5:1234\"\n", output_); @@ -149,8 +147,7 @@ name: accesslog request_headers_.addCopy(Http::Headers::get().Host, "host"); request_headers_.addCopy(Http::Headers::get().ForwardedFor, "x.x.x.x"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 UF route-test-name 1 2 0 0 3 - " "\"x.x.x.x\" " @@ -187,8 +184,7 @@ name: accesslog // response trailers: // response_trailer_key: response_trailer_val - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ(output_, "52 38 40"); } @@ -206,8 +202,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); response_headers_.addCopy(Http::Headers::get().EnvoyUpstreamServiceTime, "999"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 - 1 2 3 999 \"-\" \"-\" \"-\" \"-\" " "\"-\"\n", output_); @@ -224,8 +219,7 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ( "[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 - 1 2 3 - \"-\" \"-\" \"-\" \"-\" \"-\"\n", output_); @@ -246,8 +240,7 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET / HTTP/1.1\" 0 - 1 2 3 - \"-\" \"-\" \"-\" \"-\" " "\"10.0.0.5:1234\"\n", output_); @@ -279,12 +272,10 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseCode(200); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, WithFilterHit) { @@ -319,18 +310,15 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(3); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseCode(500); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseCode(200); stream_info_.end_time_ = stream_info_.startTimeMonotonic() + std::chrono::microseconds(1001000000000000); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, RuntimeFilter) { @@ -352,29 +340,25 @@ name: accesslog EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 42, 100)) .WillOnce(Return(true)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_CALL(context_.api_.random_, random()).WillOnce(Return(43)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 43, 100)) .WillOnce(Return(false)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.stream_id_provider_ = std::make_shared("000000ff-0000-0000-0000-000000000000"); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 55, 100)) .WillOnce(Return(true)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 55, 100)) .WillOnce(Return(false)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, RuntimeFilterV2) { @@ -399,29 +383,25 @@ name: accesslog EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 42, 10000)) .WillOnce(Return(true)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_CALL(context_.api_.random_, random()).WillOnce(Return(43)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 43, 10000)) .WillOnce(Return(false)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.stream_id_provider_ = std::make_shared("000000ff-0000-0000-0000-000000000000"); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 255, 10000)) .WillOnce(Return(true)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 255, 10000)) .WillOnce(Return(false)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, RuntimeFilterV2IndependentRandomness) { @@ -447,15 +427,13 @@ name: accesslog EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 42, 1000000)) .WillOnce(Return(true)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_CALL(context_.api_.random_, random()).WillOnce(Return(43)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 43, 1000000)) .WillOnce(Return(false)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, PathRewrite) { @@ -471,8 +449,7 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("[1999-01-01T00:00:00.000Z] \"GET /bar HTTP/1.1\" 0 - 1 2 3 - \"-\" \"-\" \"-\" \"-\" " "\"-\"\n", output_); @@ -494,8 +471,7 @@ name: accesslog stream_info_.health_check_request_ = true; EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, HealthCheckFalse) { @@ -513,8 +489,7 @@ name: accesslog Http::TestRequestHeaderMapImpl header_map{}; EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, RequestTracing) { @@ -534,22 +509,19 @@ name: accesslog { stream_info_.setTraceReason(Tracing::Reason::ServiceForced); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } { stream_info_.setTraceReason(Tracing::Reason::NotTraceable); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } { stream_info_.setTraceReason(Tracing::Reason::Sampling); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } } @@ -610,16 +582,14 @@ name: accesslog EXPECT_CALL(*file_, write(_)); Http::TestRequestHeaderMapImpl header_map{{"user-agent", "NOT/Envoy/HC"}}; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } { EXPECT_CALL(*file_, write(_)).Times(0); Http::TestRequestHeaderMapImpl header_map{}; stream_info_.health_check_request_ = true; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } } @@ -648,15 +618,13 @@ name: accesslog EXPECT_CALL(*file_, write(_)); Http::TestRequestHeaderMapImpl header_map{{"user-agent", "NOT/Envoy/HC"}}; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } { EXPECT_CALL(*file_, write(_)); Http::TestRequestHeaderMapImpl header_map{{"user-agent", "Envoy/HC"}}; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } } @@ -693,8 +661,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); Http::TestRequestHeaderMapImpl header_map{}; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } { @@ -702,8 +669,7 @@ name: accesslog Http::TestRequestHeaderMapImpl header_map{}; stream_info_.health_check_request_ = true; - log->log(&header_map, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&header_map, &response_headers_, &response_trailers_}, stream_info_); } } @@ -826,14 +792,12 @@ name: accesslog stream_info_.setResponseCode(499); EXPECT_CALL(runtime_.snapshot_, getInteger("hello", 499)).WillOnce(Return(499)); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseCode(500); EXPECT_CALL(runtime_.snapshot_, getInteger("hello", 499)).WillOnce(Return(499)); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, HeaderPresence) { @@ -851,13 +815,11 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.addCopy("test-header", "present"); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, HeaderExactMatch) { @@ -878,19 +840,16 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.addCopy("test-header", "exact-match-value"); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "not-exact-match-value"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, HeaderRegexMatch) { @@ -911,25 +870,21 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.addCopy("test-header", "123"); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "1234"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "123.456"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, HeaderRangeMatch) { @@ -950,37 +905,31 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.addCopy("test-header", "-1"); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "0"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "somestring"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "10.9"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("test-header"); request_headers_.addCopy("test-header", "-1somestring"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, ResponseFlagFilterAnyFlag) { @@ -996,13 +945,11 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, ResponseFlagFilterSpecificFlag) { @@ -1020,18 +967,15 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::UpstreamOverflow); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, ResponseFlagFilterSeveralFlags) { @@ -1050,18 +994,15 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); stream_info_.setResponseFlag(StreamInfo::ResponseFlag::UpstreamOverflow); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, ResponseFlagFilterAllFlagsInPGV) { @@ -1110,8 +1051,7 @@ name: accesslog TestStreamInfo stream_info(time_source_); stream_info.setResponseFlag(response_flag); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); } } @@ -1216,8 +1156,7 @@ name: accesslog { EXPECT_CALL(*file_, write(_)); response_trailers_.addCopy(Http::Headers::get().GrpcStatus, "0"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("OK 0 OK OK OK 0\n", output_); response_trailers_.remove(Http::Headers::get().GrpcStatus); } @@ -1225,8 +1164,7 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); response_headers_.addCopy(Http::Headers::get().GrpcStatus, "1"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("Canceled 1 Canceled Canceled CANCELLED 1\n", output_); response_headers_.remove(Http::Headers::get().GrpcStatus); } @@ -1234,8 +1172,7 @@ name: accesslog InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); response_headers_.addCopy(Http::Headers::get().GrpcStatus, "-1"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); EXPECT_EQ("-1 -1 -1 -1 -1 -1\n", output_); response_headers_.remove(Http::Headers::get().GrpcStatus); } @@ -1267,8 +1204,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); response_trailers_.addCopy(Http::Headers::get().GrpcStatus, std::to_string(i)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); response_trailers_.remove(Http::Headers::get().GrpcStatus); } } @@ -1307,8 +1243,7 @@ name: accesslog response_trailers_.addCopy(Http::Headers::get().GrpcStatus, "1"); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, GrpcStatusFilterHttpCodes) { @@ -1339,8 +1274,7 @@ name: accesslog parseAccessLogFromV3Yaml(fmt::format(yaml_template, response_string)), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } } @@ -1360,8 +1294,7 @@ name: accesslog AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, GrpcStatusFilterExclude) { @@ -1384,8 +1317,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)).Times(i == 0 ? 0 : 1); response_trailers_.addCopy(Http::Headers::get().GrpcStatus, std::to_string(i)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); response_trailers_.remove(Http::Headers::get().GrpcStatus); } } @@ -1409,8 +1341,7 @@ name: accesslog response_trailers_.addCopy(Http::Headers::get().GrpcStatus, "0"); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, GrpcStatusFilterHeader) { @@ -1431,8 +1362,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); response_headers_.addCopy(Http::Headers::get().GrpcStatus, "0"); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, LogTypeFilterUnsupportedValue) { @@ -1468,8 +1398,12 @@ name: accesslog AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::TcpConnectionEnd); // Blocked + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::TcpConnectionEnd}, + stream_info_); // Blocked } TEST_F(AccessLogImplTest, LogTypeFilterAllow) { @@ -1489,12 +1423,24 @@ name: accesslog AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(2); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::TcpUpstreamConnected); // Allowed - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::DownstreamEnd); // Allowed - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::UpstreamPoolReady); // Blocked + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::TcpUpstreamConnected}, + stream_info_); // Allowed + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::DownstreamEnd}, + stream_info_); // Allowed + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::UpstreamPoolReady}, + stream_info_); // Blocked } TEST_F(AccessLogImplTest, LogTypeFilterExclude) { @@ -1515,12 +1461,24 @@ name: accesslog AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::TcpUpstreamConnected); // Blocked - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::DownstreamEnd); // Blocked - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::UpstreamPoolReady); // Allowed + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::TcpUpstreamConnected}, + stream_info_); // Blocked + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::DownstreamEnd}, + stream_info_); // Blocked + log->log({&request_headers_, + &response_headers_, + &response_trailers_, + {}, + AccessLog::AccessLogType::UpstreamPoolReady}, + stream_info_); // Allowed } TEST_F(AccessLogImplTest, MetadataFilter) { @@ -1558,8 +1516,7 @@ name: accesslog EXPECT_CALL(*file_, write(_)); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); fields_c["c"].set_bool_value(false); EXPECT_CALL(*file_, write(_)).Times(0); @@ -1588,8 +1545,7 @@ name: accesslog // If no matcher is set, then expect no logs. EXPECT_CALL(*file_, write(_)).Times(0); - log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); } TEST_F(AccessLogImplTest, MetadataFilterNoKey) { @@ -1640,15 +1596,13 @@ name: accesslog AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(default_false_yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - default_false_log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + default_false_log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); const InstanceSharedPtr default_true_log = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(default_true_yaml), context_); EXPECT_CALL(*file_, write(_)); - default_true_log->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + default_true_log->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); } class TestHeaderFilterFactory : public ExtensionFilterFactory { @@ -1693,13 +1647,11 @@ name: accesslog InstanceSharedPtr logger = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.addCopy("test-header", "foo/bar"); EXPECT_CALL(*file_, write(_)); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } /** @@ -1772,16 +1724,13 @@ name: accesslog InstanceSharedPtr logger = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); // For rate=5 expect 1st request to be recorded, 2nd-5th skipped, and 6th recorded. EXPECT_CALL(*file_, write(_)); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); for (int i = 0; i <= 3; ++i) { EXPECT_CALL(*file_, write(_)).Times(0); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } EXPECT_CALL(*file_, write(_)); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, UnregisteredExtensionFilter) { @@ -1840,13 +1789,11 @@ name: accesslog request_headers_.addCopy("log", "true"); stream_info_.setResponseCode(404); EXPECT_CALL(*file_, write(_)); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); request_headers_.remove("log"); EXPECT_CALL(*file_, write(_)).Times(0); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, CelExtensionFilterExpressionError) { @@ -1866,8 +1813,7 @@ name: accesslog InstanceSharedPtr logger = AccessLogFactory::fromProto(parseAccessLogFromV3Yaml(yaml), context_); EXPECT_CALL(*file_, write(_)).Times(0); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } TEST_F(AccessLogImplTest, CelExtensionFilterExpressionUnparsable) { diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 5e06e3e5cc4c..7bac6ac9027b 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -2142,9 +2142,9 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLog) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([&](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([&](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_EQ(&decoder_->streamInfo(), &stream_info); EXPECT_TRUE(stream_info.responseCode()); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); @@ -2208,12 +2208,12 @@ TEST_F(HttpConnectionManagerImplTest, TestFilterCanEnrichAccessLogs) { filter->callbacks_->streamInfo().setDynamicMetadata("metadata_key", metadata); })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - auto dynamic_meta = stream_info.dynamicMetadata().filter_metadata().at("metadata_key"); - EXPECT_EQ("value", dynamic_meta.fields().at("field").string_value()); - })); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + auto dynamic_meta = stream_info.dynamicMetadata().filter_metadata().at("metadata_key"); + EXPECT_EQ("value", dynamic_meta.fields().at("field").string_value()); + })); EXPECT_CALL(*codec_, dispatch(_)) .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -2252,9 +2252,9 @@ TEST_F(HttpConnectionManagerImplTest, TestRemoteDownstreamDisconnectAccessLog) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_FALSE(stream_info.responseCode()); EXPECT_TRUE(stream_info.hasAnyResponseFlag()); EXPECT_TRUE( @@ -2296,12 +2296,12 @@ TEST_F(HttpConnectionManagerImplTest, TestLocalDownstreamDisconnectAccessLog) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_EQ("downstream_local_disconnect(reason_for_local_close)", - stream_info.responseCodeDetails().value()); - })); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ("downstream_local_disconnect(reason_for_local_close)", + stream_info.responseCodeDetails().value()); + })); EXPECT_CALL(*codec_, dispatch(_)) .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -2338,16 +2338,16 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithTrailers) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_TRUE(stream_info.responseCode()); - EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().remoteAddress()); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().directRemoteAddress()); - EXPECT_NE(nullptr, stream_info.route()); - })); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_TRUE(stream_info.responseCode()); + EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().remoteAddress()); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().directRemoteAddress()); + EXPECT_NE(nullptr, stream_info.route()); + })); EXPECT_CALL(*codec_, dispatch(_)) .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -2391,9 +2391,9 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithInvalidRequest) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([this](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([this](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_TRUE(stream_info.responseCode()); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(400)); EXPECT_EQ("missing_host_header", stream_info.responseCodeDetails().value()); @@ -2439,21 +2439,19 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogOnNewRequest) { flush_access_log_on_new_request_ = true; - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { // First call to log() is made when a new HTTP request has been received // On the first call it is expected that there is no response code. - EXPECT_EQ(AccessLog::AccessLogType::DownstreamStart, access_log_type); + EXPECT_EQ(AccessLog::AccessLogType::DownstreamStart, log_context.accessLogType()); EXPECT_FALSE(stream_info.responseCode()); })) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { // Second call to log() is made when filter is destroyed, so it is expected // that the response code is available and matches the response headers. - EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, access_log_type); + EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, log_context.accessLogType()); EXPECT_TRUE(stream_info.responseCode()); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); @@ -2503,21 +2501,19 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogOnTunnelEstablished) { flush_log_on_tunnel_successfully_established_ = true; - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { // First call to log() is made when a new HTTP tunnel has been established. - EXPECT_EQ(access_log_type, + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::DownstreamTunnelSuccessfullyEstablished); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); })) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { // Second call to log() is made when the request is completed, so it is expected // that the response code is available and matches the response headers. - EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, access_log_type); + EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, log_context.accessLogType()); EXPECT_TRUE(stream_info.responseCode()); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); @@ -2587,22 +2583,18 @@ TEST_F(HttpConnectionManagerImplTest, TestPeriodicAccessLogging) { Buffer::OwnedImpl fake_input("1234"); conn_manager_->onData(fake_input, false); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([&](const HeaderMap* request_headers, const HeaderMap* response_headers, - const HeaderMap*, const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { - EXPECT_EQ(AccessLog::AccessLogType::DownstreamPeriodic, access_log_type); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([&](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(AccessLog::AccessLogType::DownstreamPeriodic, log_context.accessLogType()); EXPECT_EQ(&decoder_->streamInfo(), &stream_info); - EXPECT_THAT(request_headers, testing::NotNull()); - EXPECT_THAT(response_headers, testing::IsNull()); EXPECT_EQ(stream_info.requestComplete(), absl::nullopt); EXPECT_THAT(stream_info.getDownstreamBytesMeter()->bytesAtLastDownstreamPeriodicLog(), testing::IsNull()); })) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { - EXPECT_EQ(AccessLog::AccessLogType::DownstreamPeriodic, access_log_type); + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(AccessLog::AccessLogType::DownstreamPeriodic, log_context.accessLogType()); EXPECT_EQ(stream_info.getDownstreamBytesMeter() ->bytesAtLastDownstreamPeriodicLog() ->wire_bytes_received, @@ -2614,14 +2606,11 @@ TEST_F(HttpConnectionManagerImplTest, TestPeriodicAccessLogging) { // Add additional bytes. response_encoder_.stream_.bytes_meter_->addWireBytesReceived(12); periodic_log_timer->invokeCallback(); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([&](const HeaderMap* request_headers, const HeaderMap* response_headers, - const HeaderMap*, const StreamInfo::StreamInfo& stream_info, - AccessLog::AccessLogType access_log_type) { - EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, access_log_type); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([&](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(AccessLog::AccessLogType::DownstreamEnd, log_context.accessLogType()); EXPECT_EQ(&decoder_->streamInfo(), &stream_info); - EXPECT_THAT(request_headers, testing::NotNull()); - EXPECT_THAT(response_headers, testing::NotNull()); EXPECT_THAT(stream_info.responseCodeDetails(), testing::Optional(testing::StrEq("details"))); EXPECT_THAT(stream_info.responseCode(), testing::Optional(200)); @@ -2733,17 +2722,17 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogSsl) { return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_TRUE(stream_info.responseCode()); - EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().remoteAddress()); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().directRemoteAddress()); - EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().sslConnection()); - EXPECT_NE(nullptr, stream_info.route()); - })); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_TRUE(stream_info.responseCode()); + EXPECT_EQ(stream_info.responseCode().value(), uint32_t(200)); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().localAddress()); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().remoteAddress()); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().directRemoteAddress()); + EXPECT_NE(nullptr, stream_info.downstreamAddressProvider().sslConnection()); + EXPECT_NE(nullptr, stream_info.route()); + })); EXPECT_CALL(*codec_, dispatch(_)) .WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status { @@ -2976,13 +2965,13 @@ TEST_F(HttpConnectionManagerImplTest, TestStreamIdleAccessLog) { std::string response_body; EXPECT_CALL(response_encoder_, encodeData(_, true)).WillOnce(AddBufferToString(&response_body)); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_TRUE(stream_info.responseCode()); - EXPECT_TRUE(stream_info.hasAnyResponseFlag()); - EXPECT_TRUE(stream_info.hasResponseFlag(StreamInfo::ResponseFlag::StreamIdleTimeout)); - })); + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_TRUE(stream_info.responseCode()); + EXPECT_TRUE(stream_info.hasAnyResponseFlag()); + EXPECT_TRUE(stream_info.hasResponseFlag(StreamInfo::ResponseFlag::StreamIdleTimeout)); + })); EXPECT_CALL(filter_factory_, createFilterChain(_)) .WillOnce(Invoke([&](FilterChainManager& manager) -> bool { diff --git a/test/common/http/conn_manager_impl_test_2.cc b/test/common/http/conn_manager_impl_test_2.cc index dec00c0bfa54..8207141eec19 100644 --- a/test/common/http/conn_manager_impl_test_2.cc +++ b/test/common/http/conn_manager_impl_test_2.cc @@ -174,9 +174,9 @@ TEST_F(HttpConnectionManagerImplTest, TestDownstreamProtocolErrorAccessLog) { access_logs_ = {handler}; setup(false, ""); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_FALSE(stream_info.responseCode()); EXPECT_TRUE(stream_info.hasAnyResponseFlag()); EXPECT_TRUE(stream_info.hasResponseFlag(StreamInfo::ResponseFlag::DownstreamProtocolError)); @@ -207,9 +207,9 @@ TEST_F(HttpConnectionManagerImplTest, TestDownstreamProtocolErrorAfterHeadersAcc return true; })); - EXPECT_CALL(*handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*handler, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_FALSE(stream_info.responseCode()); EXPECT_TRUE(stream_info.hasAnyResponseFlag()); EXPECT_TRUE(stream_info.hasResponseFlag(StreamInfo::ResponseFlag::DownstreamProtocolError)); @@ -248,13 +248,13 @@ TEST_F(HttpConnectionManagerImplTest, FrameFloodError) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWriteAndDelay, _)); - EXPECT_CALL(*log_handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - ASSERT_TRUE(stream_info.responseCodeDetails().has_value()); - EXPECT_EQ("codec_error:too_many_outbound_frames", - stream_info.responseCodeDetails().value()); - })); + EXPECT_CALL(*log_handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + ASSERT_TRUE(stream_info.responseCodeDetails().has_value()); + EXPECT_EQ("codec_error:too_many_outbound_frames", + stream_info.responseCodeDetails().value()); + })); // Kick off the incoming data. Buffer::OwnedImpl fake_input("1234"); EXPECT_LOG_NOT_CONTAINS("warning", "downstream HTTP flood", @@ -283,12 +283,12 @@ TEST_F(HttpConnectionManagerImplTest, EnvoyOverloadError) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWriteAndDelay, _)); - EXPECT_CALL(*log_handler, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - ASSERT_TRUE(stream_info.responseCodeDetails().has_value()); - EXPECT_EQ("overload_error:Envoy_Overloaded", stream_info.responseCodeDetails().value()); - })); + EXPECT_CALL(*log_handler, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + ASSERT_TRUE(stream_info.responseCodeDetails().has_value()); + EXPECT_EQ("overload_error:Envoy_Overloaded", stream_info.responseCodeDetails().value()); + })); // Kick off the incoming data. Buffer::OwnedImpl fake_input("1234"); conn_manager_->onData(fake_input, false); @@ -1476,11 +1476,11 @@ TEST_F(HttpConnectionManagerImplTest, HitFilterWatermarkLimits) { EXPECT_CALL(callbacks2, onBelowWriteBufferLowWatermark()).Times(0); encoder_filters_[1]->callbacks_->setEncoderBufferLimit((buffer_len + 1) * 2); - EXPECT_CALL(*log_handler_, log(_, _, _, _, _)) - .WillOnce(Invoke([](const HeaderMap*, const HeaderMap*, const HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_FALSE(stream_info.hasAnyResponseFlag()); - })); + EXPECT_CALL(*log_handler_, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_FALSE(stream_info.hasAnyResponseFlag()); + })); expectOnDestroy(); EXPECT_CALL(response_encoder_.stream_, removeCallbacks(_)).Times(2); diff --git a/test/common/quic/envoy_quic_server_stream_test.cc b/test/common/quic/envoy_quic_server_stream_test.cc index 5cdb1d9bb3a8..883fb61ef174 100644 --- a/test/common/quic/envoy_quic_server_stream_test.cc +++ b/test/common/quic/envoy_quic_server_stream_test.cc @@ -845,7 +845,7 @@ TEST_F(EnvoyQuicServerStreamTest, StatsGathererLogsOnStreamDestruction) { EXPECT_GT(quic_stream_->statsGatherer()->bytesOutstanding(), 0); // Close the stream; incoming acks will no longer invoke the stats gatherer but // the stats gatherer should log on stream close despite not receiving final downstream ack. - EXPECT_CALL(*mock_logger, log(_, _, _, _, _)); + EXPECT_CALL(*mock_logger, log(_, _)); quic_stream_->resetStream(Http::StreamResetReason::LocalRefusedStreamReset); } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 08c559acac20..cce10fb84764 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -71,9 +71,7 @@ class TestAccessLog : public AccessLog::Instance { public: explicit TestAccessLog(std::function func) : func_(func) {} - void log(const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo& info, - AccessLog::AccessLogType) override { + void log(const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& info) override { func_(info); } diff --git a/test/common/router/router_upstream_log_test.cc b/test/common/router/router_upstream_log_test.cc index a2673ba3fc75..627fe388fe35 100644 --- a/test/common/router/router_upstream_log_test.cc +++ b/test/common/router/router_upstream_log_test.cc @@ -490,9 +490,11 @@ TEST_F(RouterUpstreamLogTest, PeriodicLog) { encoder.stream_.bytes_meter_->addWireBytesReceived(9); EXPECT_CALL(*periodic_log_flush_, enableTimer(_, _)); - EXPECT_CALL(*mock_upstream_log_, log(_, _, _, _, AccessLog::AccessLogType::UpstreamPeriodic)) - .WillOnce(Invoke([](const Http::HeaderMap*, const Http::HeaderMap*, const Http::HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*mock_upstream_log_, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::UpstreamPeriodic); + EXPECT_EQ(stream_info.getDownstreamBytesMeter()->wireBytesReceived(), 10); EXPECT_THAT(stream_info.getDownstreamBytesMeter()->bytesAtLastUpstreamPeriodicLog(), @@ -507,9 +509,11 @@ TEST_F(RouterUpstreamLogTest, PeriodicLog) { encoder.stream_.bytes_meter_->addWireBytesReceived(7); EXPECT_CALL(*periodic_log_flush_, enableTimer(_, _)); - EXPECT_CALL(*mock_upstream_log_, log(_, _, _, _, AccessLog::AccessLogType::UpstreamPeriodic)) - .WillOnce(Invoke([](const Http::HeaderMap*, const Http::HeaderMap*, const Http::HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*mock_upstream_log_, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::UpstreamPeriodic); + EXPECT_EQ(stream_info.getDownstreamBytesMeter()->wireBytesReceived(), 10 + 8); EXPECT_EQ(stream_info.getDownstreamBytesMeter() ->bytesAtLastUpstreamPeriodicLog() @@ -531,9 +535,11 @@ TEST_F(RouterUpstreamLogTest, PeriodicLog) { EXPECT_CALL(context_.cluster_manager_.thread_local_cluster_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); - EXPECT_CALL(*mock_upstream_log_, log(_, _, _, _, AccessLog::AccessLogType::UpstreamEnd)) - .WillOnce(Invoke([](const Http::HeaderMap*, const Http::HeaderMap*, const Http::HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*mock_upstream_log_, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::UpstreamEnd); + EXPECT_EQ(stream_info.getDownstreamBytesMeter()->wireBytesReceived(), 10 + 8 + 6); EXPECT_EQ(stream_info.getDownstreamBytesMeter() ->bytesAtLastUpstreamPeriodicLog() diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 39deeb7cc586..68341b191af6 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1005,9 +1005,11 @@ TEST_F(TcpProxyTest, IntermediateLogEntry) { // The timer will be enabled cyclically. EXPECT_CALL(*flush_timer, enableTimer(std::chrono::milliseconds(1000), _)); filter_callbacks_.connection_.stream_info_.downstream_bytes_meter_->addWireBytesReceived(10); - EXPECT_CALL(*mock_access_logger_, log(_, _, _, _, AccessLog::AccessLogType::TcpPeriodic)) - .WillOnce(Invoke([](const Http::HeaderMap*, const Http::HeaderMap*, const Http::HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*mock_access_logger_, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::TcpPeriodic); + EXPECT_EQ(stream_info.getDownstreamBytesMeter()->wireBytesReceived(), 10); EXPECT_THAT(stream_info.getDownstreamBytesMeter()->bytesAtLastDownstreamPeriodicLog(), testing::IsNull()); @@ -1018,9 +1020,11 @@ TEST_F(TcpProxyTest, IntermediateLogEntry) { EXPECT_EQ(access_log_data_.value(), AccessLogType_Name(AccessLog::AccessLogType::TcpPeriodic)); filter_callbacks_.connection_.stream_info_.downstream_bytes_meter_->addWireBytesReceived(9); - EXPECT_CALL(*mock_access_logger_, log(_, _, _, _, AccessLog::AccessLogType::TcpPeriodic)) - .WillOnce(Invoke([](const Http::HeaderMap*, const Http::HeaderMap*, const Http::HeaderMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + EXPECT_CALL(*mock_access_logger_, log(_, _)) + .WillOnce(Invoke([](const Formatter::HttpFormatterContext& log_context, + const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::TcpPeriodic); + EXPECT_EQ(stream_info.getDownstreamBytesMeter()->wireBytesReceived(), 19); EXPECT_EQ(stream_info.getDownstreamBytesMeter() ->bytesAtLastDownstreamPeriodicLog() @@ -1030,7 +1034,11 @@ TEST_F(TcpProxyTest, IntermediateLogEntry) { EXPECT_CALL(*flush_timer, enableTimer(std::chrono::milliseconds(1000), _)); flush_timer->invokeCallback(); - EXPECT_CALL(*mock_access_logger_, log(_, _, _, _, AccessLog::AccessLogType::TcpConnectionEnd)); + EXPECT_CALL(*mock_access_logger_, log(_, _)) + .WillOnce(Invoke( + [](const Formatter::HttpFormatterContext& log_context, const StreamInfo::StreamInfo&) { + EXPECT_EQ(log_context.accessLogType(), AccessLog::AccessLogType::TcpConnectionEnd); + })); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); filter_.reset(); 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 1057bb83ec98..a2816d3557b2 100644 --- a/test/extensions/access_loggers/common/access_log_base_test.cc +++ b/test/extensions/access_loggers/common/access_log_base_test.cc @@ -34,7 +34,7 @@ TEST(AccessLogBaseTest, NoFilter) { StreamInfo::MockStreamInfo stream_info; TestImpl logger(nullptr); EXPECT_EQ(logger.count(), 0); - logger.log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + logger.log({}, stream_info); EXPECT_EQ(logger.count(), 1); } @@ -45,7 +45,7 @@ TEST(AccessLogBaseTest, FilterReject) { EXPECT_CALL(*filter, evaluate(_, _)).WillOnce(Return(false)); TestImpl logger(std::move(filter)); EXPECT_EQ(logger.count(), 0); - logger.log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + logger.log({}, stream_info); EXPECT_EQ(logger.count(), 0); } diff --git a/test/extensions/access_loggers/file/config_test.cc b/test/extensions/access_loggers/file/config_test.cc index ff2527af5941..a9eed4c8e040 100644 --- a/test/extensions/access_loggers/file/config_test.cc +++ b/test/extensions/access_loggers/file/config_test.cc @@ -73,8 +73,7 @@ class FileAccessLogTest : public testing::Test { EXPECT_EQ(got, expected); } })); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } Http::TestRequestHeaderMapImpl request_headers_{{":method", "GET"}, {":path", "/bar/foo"}}; diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc index 99a769325e34..a8e8a17e757d 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc @@ -155,8 +155,7 @@ class HttpGrpcAccessLogTest : public testing::Test { response: {{}} )EOF", request_method, request_method.length() + 7)); - access_log_->log(&request_headers, nullptr, nullptr, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers}, stream_info); } Stats::IsolatedStoreImpl scope_; @@ -248,7 +247,7 @@ TEST_F(HttpGrpcAccessLogTest, Marshalling) { request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } { @@ -286,7 +285,7 @@ response: {} request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } { @@ -406,8 +405,7 @@ protocol_version: HTTP10 response_body_bytes: 20 response_code_details: "via_upstream" )EOF"); - access_log_->log(&request_headers, &response_headers, nullptr, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers, &response_headers}, stream_info); } { @@ -448,8 +446,7 @@ protocol_version: HTTP10 request_headers_bytes: 16 response: {} )EOF"); - access_log_->log(&request_headers, nullptr, nullptr, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers}, stream_info); } { @@ -521,8 +518,7 @@ response: {} request_headers_bytes: 16 response: {} )EOF"); - access_log_->log(&request_headers, nullptr, nullptr, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers}, stream_info); } // TLSv1.2 @@ -578,7 +574,7 @@ response: {} request_method: "METHOD_UNSPECIFIED" response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } // TLSv1.1 @@ -634,7 +630,7 @@ response: {} request_method: "METHOD_UNSPECIFIED" response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } // TLSv1 @@ -690,7 +686,7 @@ response: {} request_method: "METHOD_UNSPECIFIED" response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } // Unknown TLS version (TLSv1.4) @@ -746,7 +742,7 @@ response: {} request_method: "METHOD_UNSPECIFIED" response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } // Intermediate log entry. @@ -814,7 +810,7 @@ response: {} request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } } @@ -910,8 +906,7 @@ TEST_F(HttpGrpcAccessLogTest, MarshallingAdditionalHeaders) { "x-logged-trailer": "value,response_trailer_value" "x-empty-trailer": "" )EOF"); - access_log_->log(&request_headers, &response_headers, &response_trailers, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers, &response_headers, &response_trailers}, stream_info); } } @@ -994,8 +989,7 @@ TEST_F(HttpGrpcAccessLogTest, SanitizeUTF8) { "x-trailer": "{0},{0}" )EOF", "prefix!!suffix")); - access_log_->log(&request_headers, &response_headers, &response_trailers, stream_info, - AccessLog::AccessLogType::NotSet); + access_log_->log({&request_headers, &response_headers, &response_trailers}, stream_info); } } @@ -1057,7 +1051,7 @@ tag: ltag request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } TEST_F(HttpGrpcAccessLogTest, CustomTagTestMetadata) { @@ -1116,7 +1110,7 @@ tag: mtag request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } TEST_F(HttpGrpcAccessLogTest, CustomTagTestMetadataDefaultValue) { @@ -1172,7 +1166,7 @@ tag: mtag request: {} response: {} )EOF"); - access_log_->log(nullptr, nullptr, nullptr, stream_info, AccessLog::AccessLogType::NotSet); + access_log_->log({}, stream_info); } } // namespace diff --git a/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc b/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc index 0e343cdecb5e..069462aa9665 100644 --- a/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc +++ b/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc @@ -144,8 +144,7 @@ TEST_F(AccessLogTest, Marshalling) { value: string_value: "10" )EOF"); - access_log->log(&request_headers, &response_headers, nullptr, stream_info, - Envoy::AccessLog::AccessLogType::NotSet); + access_log->log({&request_headers, &response_headers}, stream_info); } // Test log with empty config. @@ -159,8 +158,7 @@ TEST_F(AccessLogTest, EmptyConfig) { expectLog(R"EOF( time_unix_nano: 3600000000000 )EOF"); - access_log->log(&request_headers, &response_headers, nullptr, stream_info, - Envoy::AccessLog::AccessLogType::NotSet); + access_log->log({&request_headers, &response_headers}, stream_info); } } // namespace diff --git a/test/extensions/access_loggers/stream/stream_test_base.h b/test/extensions/access_loggers/stream/stream_test_base.h index 790ff373d79c..6520be278cb0 100644 --- a/test/extensions/access_loggers/stream/stream_test_base.h +++ b/test/extensions/access_loggers/stream/stream_test_base.h @@ -54,8 +54,7 @@ class StreamAccessLogTest : public testing::Test { EXPECT_EQ(got, expected); } })); - logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info_, - AccessLog::AccessLogType::NotSet); + logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info_); } Http::TestRequestHeaderMapImpl request_headers_{{":method", "GET"}, {":path", "/bar/foo"}}; diff --git a/test/extensions/access_loggers/wasm/config_test.cc b/test/extensions/access_loggers/wasm/config_test.cc index 767a48d57d6d..7b76e24a8764 100644 --- a/test/extensions/access_loggers/wasm/config_test.cc +++ b/test/extensions/access_loggers/wasm/config_test.cc @@ -119,14 +119,12 @@ TEST_P(WasmAccessLogConfigTest, CreateWasmFromWASM) { Http::TestRequestHeaderMapImpl request_header; Http::TestResponseHeaderMapImpl response_header; Http::TestResponseTrailerMapImpl response_trailer; - instance->log(&request_header, &response_header, &response_trailer, log_stream_info_, - AccessLog::AccessLogType::NotSet); + instance->log({&request_header, &response_header, &response_trailer}, log_stream_info_); filter = std::make_unique>(); AccessLog::InstanceSharedPtr filter_instance = factory->createAccessLogInstance(config, std::move(filter), context_); - filter_instance->log(&request_header, &response_header, &response_trailer, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({&request_header, &response_header, &response_trailer}, log_stream_info_); } TEST_P(WasmAccessLogConfigTest, YamlLoadFromFileWasmInvalidConfig) { @@ -179,8 +177,7 @@ TEST_P(WasmAccessLogConfigTest, YamlLoadFromFileWasmInvalidConfig) { AccessLog::InstanceSharedPtr filter_instance = factory->createAccessLogInstance(proto_config, nullptr, context_); filter_instance = factory->createAccessLogInstance(proto_config, nullptr, context_); - filter_instance->log(nullptr, nullptr, nullptr, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({}, log_stream_info_); } TEST_P(WasmAccessLogConfigTest, YamlLoadFromRemoteWasmCreateFilter) { @@ -225,8 +222,7 @@ TEST_P(WasmAccessLogConfigTest, YamlLoadFromRemoteWasmCreateFilter) { })); AccessLog::InstanceSharedPtr filter_instance = factory.createAccessLogInstance(proto_config, nullptr, context_); - filter_instance->log(nullptr, nullptr, nullptr, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({}, log_stream_info_); EXPECT_CALL(init_watcher_, ready()); context_.initManager().initialize(init_watcher_); auto response = Http::ResponseMessagePtr{new Http::ResponseMessageImpl( @@ -234,8 +230,7 @@ TEST_P(WasmAccessLogConfigTest, YamlLoadFromRemoteWasmCreateFilter) { response->body().add(code); async_callbacks->onSuccess(request, std::move(response)); EXPECT_EQ(context_.initManager().state(), Init::Manager::State::Initialized); - filter_instance->log(nullptr, nullptr, nullptr, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({}, log_stream_info_); } TEST_P(WasmAccessLogConfigTest, FailedToGetThreadLocalPlugin) { @@ -275,13 +270,11 @@ TEST_P(WasmAccessLogConfigTest, FailedToGetThreadLocalPlugin) { Http::TestResponseHeaderMapImpl response_header; Http::TestResponseTrailerMapImpl response_trailer; - filter_instance->log(&request_header, &response_header, &response_trailer, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({&request_header, &response_header, &response_trailer}, log_stream_info_); // Even if the thread local plugin handle returns nullptr, `log` should not raise error or // exception. threadlocal.data_[0] = std::make_shared(nullptr); - filter_instance->log(&request_header, &response_header, &response_trailer, log_stream_info_, - AccessLog::AccessLogType::NotSet); + filter_instance->log({&request_header, &response_header, &response_trailer}, log_stream_info_); } } // namespace Wasm diff --git a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc index b31209dba88d..1cb688b86be3 100644 --- a/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc +++ b/test/extensions/bootstrap/internal_listener/active_internal_listener_test.cc @@ -456,7 +456,7 @@ TEST_F(ConnectionHandlerTest, InternalListenerInplaceUpdate) { EXPECT_CALL(manager_, findFilterChain(_, _)).Times(0); EXPECT_CALL(*overridden_filter_chain_manager, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); internal_listener_cb.value().get().onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); diff --git a/test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h b/test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h index 7e415e679992..81877f8e4932 100644 --- a/test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h +++ b/test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h @@ -38,8 +38,7 @@ class HttpFilterFuzzer { // This executes the access logger with the fuzzed headers/trailers. void accessLog(AccessLog::Instance* access_logger, const StreamInfo::StreamInfo& stream_info) { ENVOY_LOG_MISC(debug, "Access logging"); - access_logger->log(&request_headers_, &response_headers_, &response_trailers_, stream_info, - AccessLog::AccessLogType::NotSet); + access_logger->log({&request_headers_, &response_headers_, &response_trailers_}, stream_info); } // Fuzzed headers and trailers are needed for access logging, reset the data and destroy filters. diff --git a/test/extensions/filters/http/composite/filter_test.cc b/test/extensions/filters/http/composite/filter_test.cc index 4085800097ac..4d37596ed7f6 100644 --- a/test/extensions/filters/http/composite/filter_test.cc +++ b/test/extensions/filters/http/composite/filter_test.cc @@ -228,10 +228,9 @@ TEST_F(FilterTest, StreamFilterDelegationMultipleAccessLoggers) { EXPECT_CALL(*encode_filter, onDestroy()); filter_.onDestroy(); - EXPECT_CALL(*access_log_1, log(_, _, _, _, _)); - EXPECT_CALL(*access_log_2, log(_, _, _, _, _)); - filter_.log(nullptr, nullptr, nullptr, StreamInfo::MockStreamInfo(), - AccessLog::AccessLogType::NotSet); + EXPECT_CALL(*access_log_1, log(_, _)); + EXPECT_CALL(*access_log_2, log(_, _)); + filter_.log({}, StreamInfo::MockStreamInfo()); } } // namespace diff --git a/test/extensions/filters/http/tap/tap_filter_test.cc b/test/extensions/filters/http/tap/tap_filter_test.cc index 70bad2406d08..2455fac66005 100644 --- a/test/extensions/filters/http/tap/tap_filter_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_test.cc @@ -94,8 +94,7 @@ TEST_F(TapFilterTest, NoConfig) { Http::MetadataMap metadata; EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(metadata)); - filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_, - AccessLog::AccessLogType::NotSet); + filter_->log({&request_headers, &response_headers, &response_trailers}, stream_info_); } // Verify filter functionality when there is a tap config. @@ -125,8 +124,7 @@ TEST_F(TapFilterTest, Config) { EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); EXPECT_CALL(*http_per_request_tapper_, onDestroyLog()).WillOnce(Return(true)); - filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_, - AccessLog::AccessLogType::NotSet); + filter_->log({&request_headers, &response_headers, &response_trailers}, stream_info_); EXPECT_EQ(1UL, filter_config_->stats_.rq_tapped_.value()); // Workaround InSequence/shared_ptr mock leak. diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index 5ca67af6eb6f..1cffc3a67b8c 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -678,8 +678,7 @@ TEST_P(WasmHttpFilterTest, AccessLog) { StreamInfo::MockStreamInfo log_stream_info; EXPECT_CALL(log_stream_info, requestComplete()) .WillRepeatedly(testing::Return(std::chrono::milliseconds(30))); - filter().log(&request_headers, &response_headers, &response_trailers, log_stream_info, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers, &response_headers, &response_trailers}, log_stream_info); filter().onDestroy(); } @@ -698,8 +697,7 @@ TEST_P(WasmHttpFilterTest, AccessLogClientDisconnected) { StreamInfo::MockStreamInfo log_stream_info; EXPECT_CALL(log_stream_info, requestComplete()) .WillRepeatedly(testing::Return(std::chrono::milliseconds(30))); - filter().log(&request_headers, nullptr, nullptr, log_stream_info, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, log_stream_info); filter().onDestroy(); } @@ -716,8 +714,7 @@ TEST_P(WasmHttpFilterTest, AccessLogDisabledForIncompleteStream) { StreamInfo::MockStreamInfo log_stream_info; EXPECT_CALL(log_stream_info, requestComplete()).WillRepeatedly(testing::Return(absl::nullopt)); - filter().log(&request_headers, nullptr, nullptr, log_stream_info, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, log_stream_info); filter().onDestroy(); } @@ -733,8 +730,7 @@ TEST_P(WasmHttpFilterTest, AccessLogCreate) { Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; Http::TestResponseTrailerMapImpl response_trailers{}; - filter().log(&request_headers, &response_headers, &response_trailers, log_stream_info, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers, &response_headers, &response_trailers}, log_stream_info); filter().onDestroy(); } @@ -1763,8 +1759,7 @@ TEST_P(WasmHttpFilterTest, Metadata) { Buffer::OwnedImpl data("hello"); EXPECT_EQ(Http::FilterDataStatus::Continue, filter().decodeData(data, true)); - filter().log(&request_headers, nullptr, nullptr, request_stream_info_, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, request_stream_info_); const auto* result = request_stream_info_.filterState()->getDataReadOnly( @@ -1831,8 +1826,7 @@ TEST_P(WasmHttpFilterTest, Property) { request_stream_info_.upstreamInfo()->setUpstreamHost(host_description); EXPECT_CALL(request_stream_info_, requestComplete()) .WillRepeatedly(Return(std::chrono::milliseconds(30))); - filter().log(&request_headers, nullptr, nullptr, request_stream_info_, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, request_stream_info_); } TEST_P(WasmHttpFilterTest, ClusterMetadata) { @@ -1863,16 +1857,14 @@ TEST_P(WasmHttpFilterTest, ClusterMetadata) { request_stream_info_.upstreamInfo()->setUpstreamHost(host_description); EXPECT_CALL(request_stream_info_, requestComplete) .WillRepeatedly(Return(std::chrono::milliseconds(30))); - filter().log(&request_headers, nullptr, nullptr, request_stream_info_, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, request_stream_info_); // If upstream host is empty, fallback to upstream cluster info for cluster metadata. request_stream_info_.upstreamInfo()->setUpstreamHost(nullptr); EXPECT_CALL(request_stream_info_, upstreamClusterInfo()).WillRepeatedly(Return(cluster)); EXPECT_CALL(filter(), log_(spdlog::level::warn, Eq(absl::string_view("cluster metadata: cluster")))); - filter().log(&request_headers, nullptr, nullptr, request_stream_info_, - AccessLog::AccessLogType::NotSet); + filter().log({&request_headers}, request_stream_info_); } TEST_P(WasmHttpFilterTest, SharedData) { diff --git a/test/integration/fake_access_log.h b/test/integration/fake_access_log.h index e21cc44c0712..603f8e0d8746 100644 --- a/test/integration/fake_access_log.h +++ b/test/integration/fake_access_log.h @@ -8,21 +8,17 @@ namespace Envoy { -using LogSignature = std::function; +using LogSignature = + std::function; class FakeAccessLog : public AccessLog::Instance { public: FakeAccessLog(LogSignature cb) : log_cb_(cb) {} - void log(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 log(const Formatter::HttpFormatterContext& context, + const StreamInfo::StreamInfo& info) override { if (log_cb_) { - log_cb_(request_headers, response_headers, response_trailers, stream_info, access_log_type); + log_cb_(context, info); } } diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 25d73e3740e3..90ad072b819a 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -995,9 +995,8 @@ TEST_P(TcpProxyIntegrationTest, TestCloseOnHealthFailure) { TEST_P(TcpProxyIntegrationTest, RecordsUpstreamConnectionTimeLatency) { FakeAccessLogFactory factory; - factory.setLogCallback([](const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { + factory.setLogCallback([](const Formatter::HttpFormatterContext&, + const StreamInfo::StreamInfo& stream_info) { EXPECT_TRUE( stream_info.upstreamInfo()->upstreamTiming().connectionPoolCallbackLatency().has_value()); }); diff --git a/test/integration/typed_metadata_integration_test.cc b/test/integration/typed_metadata_integration_test.cc index 82ba73d62c37..ba53a5498219 100644 --- a/test/integration/typed_metadata_integration_test.cc +++ b/test/integration/typed_metadata_integration_test.cc @@ -45,10 +45,7 @@ TEST_P(ListenerTypedMetadataIntegrationTest, Hello) { class MockAccessLog : public AccessLog::Instance { public: - MOCK_METHOD(void, log, - (const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo&, - AccessLog::AccessLogType)); + MOCK_METHOD(void, log, (const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo&)); }; class TestAccessLogFactory : public AccessLog::AccessLogInstanceFactory { diff --git a/test/mocks/access_log/mocks.h b/test/mocks/access_log/mocks.h index 5910e6b9c50f..66b2afd37d5d 100644 --- a/test/mocks/access_log/mocks.h +++ b/test/mocks/access_log/mocks.h @@ -50,11 +50,7 @@ class MockInstance : public Instance { ~MockInstance() override; // AccessLog::Instance - MOCK_METHOD(void, log, - (const Http::RequestHeaderMap* request_headers, - const Http::ResponseHeaderMap* response_headers, - const Http::ResponseTrailerMap* response_trailers, - const StreamInfo::StreamInfo& stream_info, AccessLogType access_log_type)); + MOCK_METHOD(void, log, (const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo&)); }; } // namespace AccessLog diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 09fa7d3feaf9..5bd902a3b9bd 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -445,7 +445,7 @@ class ConnectionHandlerTest : public testing::Test, EXPECT_EQ(1UL, handler_->numConnections()); EXPECT_CALL(*listener, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } Stats::TestUtil::TestStore stats_store_; @@ -494,7 +494,7 @@ TEST_F(ConnectionHandlerTest, RemoveListenerDuringRebalance) { Network::MockConnectionSocket* connection = new NiceMock(); current_handler->incNumConnections(); #ifndef NDEBUG - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); #endif current_handler->post(Network::ConnectionSocketPtr{connection}); @@ -557,7 +557,7 @@ TEST_F(ConnectionHandlerTest, ListenerConnectionLimitEnforced) { // We expect that listener 2 accepts the connection, so there will be a call to // createServerConnection and active cx should increase, while cx overflow remains the same. - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks2->onAccept( Network::ConnectionSocketPtr{new NiceMock()}); EXPECT_EQ(0, handler_->numConnections()); @@ -609,7 +609,7 @@ TEST_F(ConnectionHandlerTest, RemoveListener) { handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -655,7 +655,7 @@ TEST_F(ConnectionHandlerTest, RemoveListenerWithMultiAddrs) { handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks1->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -765,7 +765,7 @@ TEST_F(ConnectionHandlerTest, RebalanceWithMultiAddressListener) { // then mock_connection_balancer1 will balance the connection to the same listener. EXPECT_CALL(*mock_connection_balancer1, pickTargetHandler(_)) .WillOnce(ReturnRef(*current_handler1)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); current_handler1->incNumConnections(); @@ -775,7 +775,7 @@ TEST_F(ConnectionHandlerTest, RebalanceWithMultiAddressListener) { // then mock_connection_balancer2 will balance the connection to the same listener. EXPECT_CALL(*mock_connection_balancer2, pickTargetHandler(_)) .WillOnce(ReturnRef(*current_handler2)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); current_handler2->incNumConnections(); @@ -897,7 +897,7 @@ TEST_F(ConnectionHandlerTest, SetsTransportSocketConnectTimeout) { .WillOnce(Return(std::chrono::seconds(5))); EXPECT_CALL(*server_connection, setTransportSocketConnectTimeout(std::chrono::milliseconds(5 * 1000), _)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(std::make_unique>()); @@ -914,7 +914,7 @@ TEST_F(ConnectionHandlerTest, DestroyCloseConnections) { handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -938,7 +938,7 @@ TEST_F(ConnectionHandlerTest, CloseDuringFilterChainCreate) { EXPECT_CALL(factory_, createNetworkFilterChain(_, _)).WillOnce(Return(true)); EXPECT_CALL(*connection, state()).WillOnce(Return(Network::Connection::State::Closed)); EXPECT_CALL(*connection, addConnectionCallbacks(_)).Times(0); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); Network::MockConnectionSocket* accepted_socket = new NiceMock(); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -961,7 +961,7 @@ TEST_F(ConnectionHandlerTest, CloseConnectionOnEmptyFilterChain) { EXPECT_CALL(factory_, createNetworkFilterChain(_, _)).WillOnce(Return(false)); EXPECT_CALL(*connection, close(Network::ConnectionCloseType::NoFlush, _)); EXPECT_CALL(*connection, addConnectionCallbacks(_)).Times(0); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); Network::MockConnectionSocket* accepted_socket = new NiceMock(); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -1017,12 +1017,11 @@ TEST_F(ConnectionHandlerTest, NormalRedirect) { EXPECT_EQ(1UL, TestUtility::findCounter(stats_store_, "test.downstream_cx_total")->value()); EXPECT_EQ(1UL, TestUtility::findGauge(stats_store_, "test.downstream_cx_active")->value()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)) - .WillOnce(Invoke([&](const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_EQ(alt_address, stream_info.downstreamAddressProvider().localAddress()); - })); + EXPECT_CALL(*access_log_, log(_, _)) + .WillOnce(Invoke( + [&](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(alt_address, stream_info.downstreamAddressProvider().localAddress()); + })); connection->close(Network::ConnectionCloseType::NoFlush); dispatcher_.clearDeferredDeleteList(); EXPECT_EQ(0UL, TestUtility::findGauge(stats_store_, "downstream_cx_active")->value()); @@ -1087,12 +1086,11 @@ TEST_F(ConnectionHandlerTest, NormalRedirectWithMultiAddrs) { EXPECT_EQ(1UL, TestUtility::findCounter(stats_store_, "test.downstream_cx_total")->value()); EXPECT_EQ(1UL, TestUtility::findGauge(stats_store_, "test.downstream_cx_active")->value()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)) - .WillOnce(Invoke([&](const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, - const Http::ResponseTrailerMap*, - const StreamInfo::StreamInfo& stream_info, AccessLog::AccessLogType) { - EXPECT_EQ(alt_address, stream_info.downstreamAddressProvider().localAddress()); - })); + EXPECT_CALL(*access_log_, log(_, _)) + .WillOnce(Invoke( + [&](const Formatter::HttpFormatterContext&, const StreamInfo::StreamInfo& stream_info) { + EXPECT_EQ(alt_address, stream_info.downstreamAddressProvider().localAddress()); + })); connection->close(Network::ConnectionCloseType::NoFlush); dispatcher_.clearDeferredDeleteList(); EXPECT_EQ(0UL, TestUtility::findGauge(stats_store_, "downstream_cx_active")->value()); @@ -1180,7 +1178,7 @@ TEST_F(ConnectionHandlerTest, MatchLatestListener) { EXPECT_CALL(*listener3, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedListener) { @@ -1240,7 +1238,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedListener) { EXPECT_EQ(1UL, handler_->numConnections()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) { @@ -1300,7 +1298,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) { EXPECT_EQ(1UL, handler_->numConnections()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { @@ -1350,7 +1348,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListener) { @@ -1421,7 +1419,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListener) { EXPECT_CALL(*listener3, onDestroy()); EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This tests the ConnectionHandler's `getBalancedHandlerByAddress` will match @@ -1486,7 +1484,7 @@ TEST_F(ConnectionHandlerTest, MatchIPv6WildcardListenerWithAnyAddressAndIpv4Comp EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This tests the ConnectionHandler's `getBalancedHandlerByAddress` will match @@ -1551,7 +1549,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4CompatiableIPv6ListenerWithIpv4CompatFla EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This tests the ConnectionHandler's `getBalancedHandlerByAddress` won't match @@ -1615,7 +1613,7 @@ TEST_F(ConnectionHandlerTest, NotMatchIPv6WildcardListenerWithoutIpv4CompatFlag) EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This tests the case both "0.0.0.0" and "::" with ipv4_compat are added. The @@ -1695,7 +1693,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag) { EXPECT_CALL(*listener3, onDestroy()); EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This test is same as above except the Ipv4 listener is added first, then Ipv6 @@ -1775,7 +1773,7 @@ TEST_F(ConnectionHandlerTest, MatchhIpv4WhenBothIpv4AndIPv6WithIpv4CompatFlag2) EXPECT_CALL(*listener3, onDestroy()); EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } // This test the case of an update for listener which listening @@ -1855,7 +1853,7 @@ TEST_F(ConnectionHandlerTest, UpdateIpv4MappedListener) { EXPECT_CALL(*listener3, onDestroy()); EXPECT_CALL(*listener2, onDestroy()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, WildcardListenerWithOriginalDstInbound) { @@ -1905,7 +1903,7 @@ TEST_F(ConnectionHandlerTest, WildcardListenerWithNoOriginalDst) { EXPECT_EQ(1UL, handler_->numConnections()); EXPECT_CALL(*listener1, onDestroy()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); } TEST_F(ConnectionHandlerTest, TransportProtocolDefault) { @@ -1920,7 +1918,7 @@ TEST_F(ConnectionHandlerTest, TransportProtocolDefault) { .WillOnce(Return(absl::string_view(""))); EXPECT_CALL(*accepted_socket, setDetectedTransportProtocol(absl::string_view("raw_buffer"))); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); EXPECT_CALL(*listener, onDestroy()); @@ -1950,7 +1948,7 @@ TEST_F(ConnectionHandlerTest, TransportProtocolCustom) { EXPECT_CALL(*accepted_socket, setDetectedTransportProtocol(dummy)); EXPECT_CALL(*accepted_socket, detectedTransportProtocol()).WillOnce(Return(dummy)); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); EXPECT_CALL(*listener, onDestroy()); @@ -1993,7 +1991,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeout) { EXPECT_EQ(1UL, downstream_pre_cx_active.value()); EXPECT_CALL(*timeout, disableTimer()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); timeout->invokeCallback(); EXPECT_CALL(*test_filter, destroy_()); dispatcher_.clearDeferredDeleteList(); @@ -2046,7 +2044,7 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { EXPECT_CALL(*test_filter, destroy_()); // Barrier: test_filter must be destructed before findFilterChain EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); EXPECT_CALL(*timeout, disableTimer()); timeout->invokeCallback(); dispatcher_.clearDeferredDeleteList(); @@ -2114,7 +2112,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeoutResetOnSuccess) { EXPECT_CALL(io_handle, resetFileEvents()); EXPECT_CALL(*test_filter, destroy_()); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); EXPECT_CALL(*timeout, disableTimer()); file_event_callback(Event::FileReadyType::Read); @@ -2190,7 +2188,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterReportError) { cb.socket().close(); return Network::FilterStatus::StopIteration; })); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); // The last filter won't be invoked EXPECT_CALL(*last_filter, onAccept(_)).Times(0); EXPECT_CALL(*first_filter, destroy_()); @@ -2315,7 +2313,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdate) { .WillOnce(ReturnRef(*current_handler)); EXPECT_CALL(manager_, findFilterChain(_, _)).Times(0); EXPECT_CALL(*overridden_filter_chain_manager, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); EXPECT_CALL(*mock_connection_balancer, unregisterHandler(_)); old_listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -2336,7 +2334,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveFilterChain) { auto* server_connection = new NiceMock(); EXPECT_CALL(dispatcher_, createServerConnection_()).WillOnce(Return(server_connection)); EXPECT_CALL(factory_, createNetworkFilterChain(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); @@ -2396,7 +2394,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveFilterChainCalledAfterListenerIsR handler_->stopListeners(listener_tag, {}); EXPECT_CALL(dispatcher_, clearDeferredDeleteList()); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); { // Filter chain removal in the same poll cycle but earlier. @@ -2440,7 +2438,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveListener) { handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -2472,7 +2470,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveIpv6AnyAddressWithIpv4CompatListe handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -2501,7 +2499,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveIpv4CompatAddressListener) { handler_->addListener(absl::nullopt, *test_listener, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -2538,12 +2536,12 @@ TEST_F(ConnectionHandlerTest, TcpListenerRemoveWithBothIpv4AnyAndIpv6Any) { handler_->addListener(absl::nullopt, *test_listener2, runtime_); Network::MockConnectionSocket* connection = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{connection}); EXPECT_EQ(0UL, handler_->numConnections()); Network::MockConnectionSocket* connection2 = new NiceMock(); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks2->onAccept(Network::ConnectionSocketPtr{connection2}); EXPECT_EQ(0UL, handler_->numConnections()); @@ -2624,7 +2622,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterWorks) { EXPECT_CALL(*disabled_listener_filter, destroy_()); EXPECT_CALL(*enabled_filter, destroy_()); EXPECT_CALL(manager_, findFilterChain(_, _)).WillOnce(Return(nullptr)); - EXPECT_CALL(*access_log_, log(_, _, _, _, _)); + EXPECT_CALL(*access_log_, log(_, _)); listener_callbacks->onAccept(std::make_unique>()); EXPECT_CALL(*listener, onDestroy()); }