diff --git a/configs/access_log_format_helper.template.json b/configs/access_log_format_helper.template.json index 7c11c9a2eba36..710f8aa90912c 100644 --- a/configs/access_log_format_helper.template.json +++ b/configs/access_log_format_helper.template.json @@ -1,15 +1,15 @@ {% macro ingress_sampled_log() %} - "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\"\n" + "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\"\n" {% endmacro %} {% macro ingress_full() %} - "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%\" %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\"\n" + "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\"\n" {% endmacro %} {% macro egress_error_log() %} - "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\n" + "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\n" {% endmacro %} {% macro egress_error_amazon_service() %} - "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\" \"%RESP(X-AMZN-RequestId)%\"\n" + "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\" \"%RESP(X-AMZN-RequestId)%\"\n" {% endmacro %} diff --git a/docs/configuration/http_conn_man/access_log.rst b/docs/configuration/http_conn_man/access_log.rst index 185393411423c..6e6f2f3ad7d3b 100644 --- a/docs/configuration/http_conn_man/access_log.rst +++ b/docs/configuration/http_conn_man/access_log.rst @@ -71,8 +71,8 @@ The following command operators are supported: %DURATION% Total duration in milliseconds of the request from the start time to the last byte out. -%FAILURE_REASON% - Additional failure reason if any in addition to response code. Possible values are: +%RESPONSE_FLAGS% + Additional details about the response, if any. Possible values are: * **LH**: Local service failed :ref:`health check request ` in addition to 503 response code. * **UH**: No healthy upstream hosts in upstream cluster in addition to 503 response code. @@ -83,7 +83,7 @@ The following command operators are supported: * **UC**: Upstream connection termination in addition to 503 response code. * **UO**: Upstream overflow (:ref:`circuit breaking `) in addition to 503 response code. * **NR**: No :ref:`route configured ` for a given request in addition to 404 response code. - * **FI**: The request was aborted as a result of :ref:`fault injection `. + * **FI**: The request was aborted with an injected response code via :ref:`fault injection `. %UPSTREAM_HOST% Upstream host URL (e.g., tcp://ip:port for TCP connections). @@ -107,7 +107,7 @@ If custom format is not specified, Envoy uses the following default format: .. code-block:: none [%START_TIME%] "%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%" - %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% + %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% "%REQ(X-FORWARDED-FOR)%" "%REQ(USER-AGENT)%" "%REQ(X-REQUEST-ID)%" "%REQ(:AUTHORITY)%" "%UPSTREAM_HOST%"\n diff --git a/examples/grpc-bridge/config/s2s-python-envoy.json b/examples/grpc-bridge/config/s2s-python-envoy.json index 02430e45866d7..6bfeb15de5d02 100644 --- a/examples/grpc-bridge/config/s2s-python-envoy.json +++ b/examples/grpc-bridge/config/s2s-python-envoy.json @@ -13,7 +13,7 @@ "access_log": [ { "path": "/var/log/envoy/egress_http.log", - "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\n" + "format": "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH):256% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\n" } ], "stat_prefix": "egress_http", diff --git a/include/envoy/http/access_log.h b/include/envoy/http/access_log.h index 2f1477df257e8..68b5f29c4e8f2 100644 --- a/include/envoy/http/access_log.h +++ b/include/envoy/http/access_log.h @@ -10,29 +10,27 @@ namespace Http { namespace AccessLog { -enum class FailureReason { - // No failure. - None, +enum ResponseFlag { // Local server healthcheck failed. - FailedLocalHealthCheck, + FailedLocalHealthCheck = 0x1, // No healthy upstream. - NoHealthyUpstream, + NoHealthyUpstream = 0x2, // Request timeout on upstream. - UpstreamRequestTimeout, + UpstreamRequestTimeout = 0x4, // Local codec level reset was sent on the stream. - LocalReset, + LocalReset = 0x8, // Remote codec level reset was received on the stream. - UpstreamRemoteReset, + UpstreamRemoteReset = 0x10, // Local reset by a connection pool due to an initial connection failure. - UpstreamConnectionFailure, + UpstreamConnectionFailure = 0x20, // If the stream was locally reset due to connection termination. - UpstreamConnectionTermination, + UpstreamConnectionTermination = 0x40, // The stream was reset because of a resource overflow. - UpstreamOverflow, + UpstreamOverflow = 0x80, // No route found for a given request. - NoRouteFound, - // Used when faults (abort with error codes) are injected. - FaultInjected, + NoRouteFound = 0x100, + // Abort with error code is injected. + FaultInjected = 0x200 }; /** @@ -43,13 +41,12 @@ class RequestInfo { virtual ~RequestInfo() {} /** - * filter can trigger this callback on failed response to provide more details about - * failure. + * Each filter can set independent response flag, flags are accumulated. */ - virtual void onFailedResponse(FailureReason failure_reason) PURE; + virtual void setResponseFlag(ResponseFlag response_flag) PURE; /** - * filter can trigger this callback when an upstream host has been selected. + * Filter can trigger this callback when an upstream host has been selected. */ virtual void onUpstreamHostSelected(Upstream::HostDescriptionPtr host) PURE; @@ -89,9 +86,9 @@ class RequestInfo { virtual std::chrono::milliseconds duration() const PURE; /** - * @return the failure reason for richer log experience. + * @return whether response flag is set or not. */ - virtual FailureReason failureReason() const PURE; + virtual bool getResponseFlag(ResponseFlag response_flag) const PURE; /** * @return upstream host description. diff --git a/source/common/http/access_log/access_log_formatter.cc b/source/common/http/access_log/access_log_formatter.cc index 73fa92464b871..f6cbadacfe0ba 100644 --- a/source/common/http/access_log/access_log_formatter.cc +++ b/source/common/http/access_log/access_log_formatter.cc @@ -6,50 +6,86 @@ namespace Http { namespace AccessLog { -const std::string FilterReasonUtils::NONE = "-"; -const std::string FilterReasonUtils::FAILED_LOCAL_HEALTH_CHECK = "LH"; -const std::string FilterReasonUtils::NO_HEALTHY_UPSTREAM = "UH"; -const std::string FilterReasonUtils::UPSTREAM_REQUEST_TIMEOUT = "UT"; -const std::string FilterReasonUtils::LOCAL_RESET = "LR"; -const std::string FilterReasonUtils::UPSTREAM_REMOTE_RESET = "UR"; -const std::string FilterReasonUtils::UPSTREAM_CONNECTION_FAILURE = "UF"; -const std::string FilterReasonUtils::UPSTREAM_CONNECTION_TERMINATION = "UC"; -const std::string FilterReasonUtils::UPSTREAM_OVERFLOW = "UO"; -const std::string FilterReasonUtils::NO_ROUTE_FOUND = "NR"; -const std::string FilterReasonUtils::FAULT_INJECTED = "FI"; - -const std::string& FilterReasonUtils::toShortString(FailureReason failure_reason) { - switch (failure_reason) { - case FailureReason::None: - return NONE; - case FailureReason::FailedLocalHealthCheck: - return FAILED_LOCAL_HEALTH_CHECK; - case FailureReason::NoHealthyUpstream: - return NO_HEALTHY_UPSTREAM; - case FailureReason::UpstreamRequestTimeout: - return UPSTREAM_REQUEST_TIMEOUT; - case FailureReason::LocalReset: - return LOCAL_RESET; - case FailureReason::UpstreamRemoteReset: - return UPSTREAM_REMOTE_RESET; - case FailureReason::UpstreamConnectionFailure: - return UPSTREAM_CONNECTION_FAILURE; - case FailureReason::UpstreamConnectionTermination: - return UPSTREAM_CONNECTION_TERMINATION; - case FailureReason::UpstreamOverflow: - return UPSTREAM_OVERFLOW; - case FailureReason::NoRouteFound: - return NO_ROUTE_FOUND; - case FailureReason::FaultInjected: - return FAULT_INJECTED; +const std::string ResponseFlagUtils::NONE = "-"; +const std::string ResponseFlagUtils::FAILED_LOCAL_HEALTH_CHECK = "LH"; +const std::string ResponseFlagUtils::NO_HEALTHY_UPSTREAM = "UH"; +const std::string ResponseFlagUtils::UPSTREAM_REQUEST_TIMEOUT = "UT"; +const std::string ResponseFlagUtils::LOCAL_RESET = "LR"; +const std::string ResponseFlagUtils::UPSTREAM_REMOTE_RESET = "UR"; +const std::string ResponseFlagUtils::UPSTREAM_CONNECTION_FAILURE = "UF"; +const std::string ResponseFlagUtils::UPSTREAM_CONNECTION_TERMINATION = "UC"; +const std::string ResponseFlagUtils::UPSTREAM_OVERFLOW = "UO"; +const std::string ResponseFlagUtils::NO_ROUTE_FOUND = "NR"; +const std::string ResponseFlagUtils::FAULT_INJECTED = "FI"; + +void ResponseFlagUtils::appendString(std::string& result, const std::string& append) { + if (result.empty()) { + result = append; + } else { + result += "," + append; + } +} + +const std::string ResponseFlagUtils::toShortString(const RequestInfo& request_info) { + std::string result; + + if (request_info.getResponseFlag(ResponseFlag::FailedLocalHealthCheck)) { + appendString(result, FAILED_LOCAL_HEALTH_CHECK); + } + + if (request_info.getResponseFlag(ResponseFlag::NoHealthyUpstream)) { + appendString(result, NO_HEALTHY_UPSTREAM); + } + + if (request_info.getResponseFlag(ResponseFlag::UpstreamRequestTimeout)) { + appendString(result, UPSTREAM_REQUEST_TIMEOUT); + } + + if (request_info.getResponseFlag(ResponseFlag::LocalReset)) { + appendString(result, LOCAL_RESET); + } + + if (request_info.getResponseFlag(ResponseFlag::UpstreamRemoteReset)) { + appendString(result, UPSTREAM_REMOTE_RESET); } - throw std::invalid_argument("Unknown failure_reason"); + if (request_info.getResponseFlag(ResponseFlag::UpstreamConnectionFailure)) { + appendString(result, UPSTREAM_CONNECTION_FAILURE); + } + + if (request_info.getResponseFlag(ResponseFlag::UpstreamConnectionTermination)) { + appendString(result, UPSTREAM_CONNECTION_TERMINATION); + } + + if (request_info.getResponseFlag(ResponseFlag::UpstreamOverflow)) { + appendString(result, UPSTREAM_OVERFLOW); + } + + if (request_info.getResponseFlag(ResponseFlag::NoRouteFound)) { + appendString(result, NO_ROUTE_FOUND); + } + + if (request_info.getResponseFlag(ResponseFlag::FaultInjected)) { + appendString(result, FAULT_INJECTED); + } + + return result.empty() ? NONE : result; +} + +bool ResponseFlagUtils::isTraceableFailure(const Http::AccessLog::RequestInfo& request_info) { + return request_info.getResponseFlag(Http::AccessLog::ResponseFlag::NoHealthyUpstream) | + request_info.getResponseFlag(Http::AccessLog::ResponseFlag::UpstreamConnectionFailure) | + request_info.getResponseFlag(Http::AccessLog::ResponseFlag::UpstreamOverflow) | + request_info.getResponseFlag(Http::AccessLog::ResponseFlag::UpstreamRequestTimeout) | + request_info.getResponseFlag( + Http::AccessLog::ResponseFlag::UpstreamConnectionTermination) | + request_info.getResponseFlag(Http::AccessLog::ResponseFlag::NoRouteFound) | + request_info.getResponseFlag(Http::AccessLog::ResponseFlag::FaultInjected); } const std::string AccessLogFormatUtils::DEFAULT_FORMAT = "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%\" " - "%RESPONSE_CODE% %FAILURE_REASON% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% " + "%RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% " "%RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% " "\"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" " "\"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\n"; @@ -210,9 +246,9 @@ RequestInfoFormatter::RequestInfoFormatter(const std::string& field_name) { field_extractor_ = [](const RequestInfo& request_info) { return std::to_string(request_info.duration().count()); }; - } else if (field_name == "FAILURE_REASON") { + } else if (field_name == "RESPONSE_FLAGS") { field_extractor_ = [](const RequestInfo& request_info) { - return FilterReasonUtils::toShortString(request_info.failureReason()); + return ResponseFlagUtils::toShortString(request_info); }; } else if (field_name == "UPSTREAM_HOST") { field_extractor_ = [](const RequestInfo& request_info) { diff --git a/source/common/http/access_log/access_log_formatter.h b/source/common/http/access_log/access_log_formatter.h index b6a0ddb46f48b..abdef95ca5363 100644 --- a/source/common/http/access_log/access_log_formatter.h +++ b/source/common/http/access_log/access_log_formatter.h @@ -6,14 +6,16 @@ namespace Http { namespace AccessLog { /** - * Util class for FilterReason. + * Util class for ResponseFlags. */ -class FilterReasonUtils { +class ResponseFlagUtils { public: - static const std::string& toShortString(FailureReason failure_reason); + static const std::string toShortString(const RequestInfo& request_info); + static bool isTraceableFailure(const Http::AccessLog::RequestInfo& request_info); private: - FilterReasonUtils(){}; + ResponseFlagUtils(); + static void appendString(std::string& result, const std::string& append); const static std::string NONE; const static std::string FAILED_LOCAL_HEALTH_CHECK; diff --git a/source/common/http/access_log/request_info_impl.h b/source/common/http/access_log/request_info_impl.h index bed92f75416d3..967f1365960df 100644 --- a/source/common/http/access_log/request_info_impl.h +++ b/source/common/http/access_log/request_info_impl.h @@ -26,11 +26,11 @@ struct RequestInfoImpl : public RequestInfo { start_time_); } - void onFailedResponse(Http::AccessLog::FailureReason failure_reason) override { - failure_reason_ = failure_reason; + void setResponseFlag(Http::AccessLog::ResponseFlag response_flag) override { + response_flags_ |= response_flag; } - Http::AccessLog::FailureReason failureReason() const override { return failure_reason_; } + bool getResponseFlag(ResponseFlag flag) const override { return response_flags_ & flag; } void onUpstreamHostSelected(Upstream::HostDescriptionPtr host) override { upstream_host_ = host; } @@ -45,7 +45,7 @@ struct RequestInfoImpl : public RequestInfo { uint64_t bytes_received_{}; Optional response_code_; uint64_t bytes_sent_{}; - FailureReason failure_reason_{FailureReason::None}; + uint64_t response_flags_{}; Upstream::HostDescriptionPtr upstream_host_{}; bool hc_request_{}; }; diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 6168497161720..6038b7c00aad4 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -1,5 +1,6 @@ #include "conn_manager_utility.h" +#include "common/http/access_log/access_log_formatter.h" #include "common/http/headers.h" #include "common/http/utility.h" #include "common/network/utility.h" @@ -148,7 +149,7 @@ bool ConnectionManagerUtility::shouldTraceRequest( case Http::TracingType::All: return true; case Http::TracingType::UpstreamFailure: - return request_info.failureReason() != Http::AccessLog::FailureReason::None; + return Http::AccessLog::ResponseFlagUtils::isTraceableFailure(request_info); } // Compiler enforces switch above to cover all the cases and it's impossible to be here, diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 6f79750bd3b9b..e4cf54785abd2 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -27,6 +27,8 @@ class ConnectionManagerUtility { const Optional& config); private: + static bool isTraceableFailure(const Http::AccessLog::RequestInfo& request_info); + // NOTE: This is used for stable randomness in the case where the route table does not use any // runtime rules. If runtime rules are used, we use true randomness which is slower but // provides behavior that most consumers would expect. diff --git a/source/common/http/filter/fault_filter.cc b/source/common/http/filter/fault_filter.cc index 348abf505fcee..7f1ee24cad637 100644 --- a/source/common/http/filter/fault_filter.cc +++ b/source/common/http/filter/fault_filter.cc @@ -137,7 +137,7 @@ void FaultFilter::abortWithHTTPStatus() { "fault.http.abort.http_status", config_->abortCode()))}}}; callbacks_->encodeHeaders(std::move(response_headers), true); config_->stats().aborts_injected_.inc(); - callbacks_->requestInfo().onFailedResponse(Http::AccessLog::FailureReason::FaultInjected); + callbacks_->requestInfo().setResponseFlag(Http::AccessLog::ResponseFlag::FaultInjected); } void FaultFilter::resetTimerState() { diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 1183fb0de8c42..401c1af98850c 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -149,7 +149,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e config_.stats_.no_route_.inc(); stream_log_debug("no cluster match for URL '{}'", *callbacks_, headers.Path()->value().c_str()); - callbacks_->requestInfo().onFailedResponse(Http::AccessLog::FailureReason::NoRouteFound); + callbacks_->requestInfo().setResponseFlag(Http::AccessLog::ResponseFlag::NoRouteFound); Http::HeaderMapPtr response_headers{new Http::HeaderMapImpl{ {Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::NotFound))}}}; callbacks_->encodeHeaders(std::move(response_headers), true); @@ -176,7 +176,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e // See if we are supposed to immediately kill some percentage of this cluster's traffic. if (cluster_->maintenanceMode()) { - callbacks_->requestInfo().onFailedResponse(Http::AccessLog::FailureReason::UpstreamOverflow); + callbacks_->requestInfo().setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamOverflow); chargeUpstreamCode(Http::Code::ServiceUnavailable, nullptr); Http::Utility::sendLocalReply(*callbacks_, Http::Code::ServiceUnavailable, "maintenance mode"); return Http::FilterHeadersStatus::StopIteration; @@ -222,7 +222,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e } void Filter::sendNoHealthyUpstreamResponse() { - callbacks_->requestInfo().onFailedResponse(Http::AccessLog::FailureReason::NoHealthyUpstream); + callbacks_->requestInfo().setResponseFlag(Http::AccessLog::ResponseFlag::NoHealthyUpstream); chargeUpstreamCode(Http::Code::ServiceUnavailable, nullptr); Http::Utility::sendLocalReply(*callbacks_, Http::Code::ServiceUnavailable, "no healthy upstream"); } @@ -367,15 +367,15 @@ void Filter::onUpstreamReset(UpstreamResetType type, Http::Code code; const char* body; if (type == UpstreamResetType::GlobalTimeout || type == UpstreamResetType::PerTryTimeout) { - callbacks_->requestInfo().onFailedResponse( - Http::AccessLog::FailureReason::UpstreamRequestTimeout); + callbacks_->requestInfo().setResponseFlag( + Http::AccessLog::ResponseFlag::UpstreamRequestTimeout); code = Http::Code::GatewayTimeout; body = "upstream request timeout"; } else { - Http::AccessLog::FailureReason failure_reason = - streamResetReasonToFailureReason(reset_reason.value()); - callbacks_->requestInfo().onFailedResponse(failure_reason); + Http::AccessLog::ResponseFlag response_flags = + streamResetReasonToResponseFlag(reset_reason.value()); + callbacks_->requestInfo().setResponseFlag(response_flags); code = Http::Code::ServiceUnavailable; body = "upstream connect error or disconnect/reset before headers"; } @@ -385,21 +385,21 @@ void Filter::onUpstreamReset(UpstreamResetType type, } } -Http::AccessLog::FailureReason -Filter::streamResetReasonToFailureReason(Http::StreamResetReason reset_reason) { +Http::AccessLog::ResponseFlag +Filter::streamResetReasonToResponseFlag(Http::StreamResetReason reset_reason) { switch (reset_reason) { case Http::StreamResetReason::ConnectionFailure: - return Http::AccessLog::FailureReason::UpstreamConnectionFailure; + return Http::AccessLog::ResponseFlag::UpstreamConnectionFailure; case Http::StreamResetReason::ConnectionTermination: - return Http::AccessLog::FailureReason::UpstreamConnectionTermination; + return Http::AccessLog::ResponseFlag::UpstreamConnectionTermination; case Http::StreamResetReason::LocalReset: case Http::StreamResetReason::LocalRefusedStreamReset: - return Http::AccessLog::FailureReason::LocalReset; + return Http::AccessLog::ResponseFlag::LocalReset; case Http::StreamResetReason::Overflow: - return Http::AccessLog::FailureReason::UpstreamOverflow; + return Http::AccessLog::ResponseFlag::UpstreamOverflow; case Http::StreamResetReason::RemoteReset: case Http::StreamResetReason::RemoteRefusedStreamReset: - return Http::AccessLog::FailureReason::UpstreamRemoteReset; + return Http::AccessLog::ResponseFlag::UpstreamRemoteReset; } throw std::invalid_argument("Unknown reset_reason"); diff --git a/source/common/router/router.h b/source/common/router/router.h index 708d148beb0f9..c49c7bb1c6ebc 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -168,8 +168,8 @@ class Filter : Logger::Loggable, public Http::StreamDecoderF enum class UpstreamResetType { Reset, GlobalTimeout, PerTryTimeout }; - Http::AccessLog::FailureReason - streamResetReasonToFailureReason(Http::StreamResetReason reset_reason); + Http::AccessLog::ResponseFlag + streamResetReasonToResponseFlag(Http::StreamResetReason reset_reason); static const std::string& upstreamZone(Upstream::HostDescriptionPtr upstream_host); void chargeUpstreamCode(const Http::HeaderMap& response_headers, diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc index 8ff34a50159d3..abea122f2a598 100644 --- a/source/common/tracing/http_tracer_impl.cc +++ b/source/common/tracing/http_tracer_impl.cc @@ -264,10 +264,7 @@ void LightStepSink::flushTrace(const Http::HeaderMap& request_headers, const Htt span.SetTag("error", "true"); } - if (request_info.failureReason() != Http::AccessLog::FailureReason::None) { - span.SetTag("failure reason", - Http::AccessLog::FilterReasonUtils::toShortString(request_info.failureReason())); - } + span.SetTag("response flags", Http::AccessLog::ResponseFlagUtils::toShortString(request_info)); if (request_headers.ClientTraceId()) { span.SetTag("guid:x-client-trace-id", diff --git a/source/server/http/health_check.cc b/source/server/http/health_check.cc index 17a9ede98e4ab..9a81504e6daab 100644 --- a/source/server/http/health_check.cc +++ b/source/server/http/health_check.cc @@ -125,8 +125,8 @@ void HealthCheckFilter::onComplete() { ASSERT(handling_); Http::HeaderMapPtr headers; if (server_.healthCheckFailed()) { - callbacks_->requestInfo().onFailedResponse( - Http::AccessLog::FailureReason::FailedLocalHealthCheck); + callbacks_->requestInfo().setResponseFlag( + Http::AccessLog::ResponseFlag::FailedLocalHealthCheck); headers.reset(new Http::HeaderMapImpl{ {Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::ServiceUnavailable))}}); } else { @@ -136,8 +136,8 @@ void HealthCheckFilter::onComplete() { } if (!Http::CodeUtility::is2xx(enumToInt(final_status))) { - callbacks_->requestInfo().onFailedResponse( - Http::AccessLog::FailureReason::FailedLocalHealthCheck); + callbacks_->requestInfo().setResponseFlag( + Http::AccessLog::ResponseFlag::FailedLocalHealthCheck); } headers.reset(new Http::HeaderMapImpl{ diff --git a/test/common/http/access_log/access_log_formatter_test.cc b/test/common/http/access_log/access_log_formatter_test.cc index 65d1eab89e738..6898e98e0ec13 100644 --- a/test/common/http/access_log/access_log_formatter_test.cc +++ b/test/common/http/access_log/access_log_formatter_test.cc @@ -5,28 +5,48 @@ #include "test/mocks/http/mocks.h" #include "test/test_common/utility.h" +using testing::_; +using testing::NiceMock; using testing::Return; using testing::ReturnRef; namespace Http { namespace AccessLog { -TEST(FailureReasonUtilsTest, toShortStringConversion) { - std::vector> expected = { - std::make_pair(FailureReason::None, "-"), - std::make_pair(FailureReason::FailedLocalHealthCheck, "LH"), - std::make_pair(FailureReason::NoHealthyUpstream, "UH"), - std::make_pair(FailureReason::UpstreamRequestTimeout, "UT"), - std::make_pair(FailureReason::LocalReset, "LR"), - std::make_pair(FailureReason::UpstreamRemoteReset, "UR"), - std::make_pair(FailureReason::UpstreamConnectionFailure, "UF"), - std::make_pair(FailureReason::UpstreamConnectionTermination, "UC"), - std::make_pair(FailureReason::UpstreamOverflow, "UO"), - std::make_pair(FailureReason::NoRouteFound, "NR"), - std::make_pair(FailureReason::FaultInjected, "FI")}; +TEST(ResponseFlagUtilsTest, toShortStringConversion) { + std::vector> expected = { + std::make_pair(ResponseFlag::FailedLocalHealthCheck, "LH"), + std::make_pair(ResponseFlag::NoHealthyUpstream, "UH"), + std::make_pair(ResponseFlag::UpstreamRequestTimeout, "UT"), + std::make_pair(ResponseFlag::LocalReset, "LR"), + std::make_pair(ResponseFlag::UpstreamRemoteReset, "UR"), + std::make_pair(ResponseFlag::UpstreamConnectionFailure, "UF"), + std::make_pair(ResponseFlag::UpstreamConnectionTermination, "UC"), + std::make_pair(ResponseFlag::UpstreamOverflow, "UO"), + std::make_pair(ResponseFlag::NoRouteFound, "NR"), + std::make_pair(ResponseFlag::FaultInjected, "FI")}; for (const auto& testCase : expected) { - EXPECT_EQ(testCase.second, FilterReasonUtils::toShortString(testCase.first)); + NiceMock request_info; + ON_CALL(request_info, getResponseFlag(testCase.first)).WillByDefault(Return(true)); + EXPECT_EQ(testCase.second, ResponseFlagUtils::toShortString(request_info)); + } + + // No flag is set. + { + NiceMock request_info; + ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(false)); + EXPECT_EQ("-", ResponseFlagUtils::toShortString(request_info)); + } + + // Test combinations. + // These are not real use cases, but are used to cover multiple response flags case. + { + NiceMock request_info; + ON_CALL(request_info, getResponseFlag(ResponseFlag::FaultInjected)).WillByDefault(Return(true)); + ON_CALL(request_info, getResponseFlag(ResponseFlag::UpstreamRequestTimeout)) + .WillByDefault(Return(true)); + EXPECT_EQ("UT,FI", ResponseFlagUtils::toShortString(request_info)); } } @@ -47,7 +67,7 @@ TEST(AccessLogFormatterTest, plainStringFormatter) { TEST(AccessLogFormatterTest, requestInfoFormatter) { EXPECT_THROW(RequestInfoFormatter formatter("unknown_field"), EnvoyException); - MockRequestInfo requestInfo; + NiceMock requestInfo; TestHeaderMapImpl header{{":method", "GET"}, {":path", "/"}}; { @@ -98,10 +118,9 @@ TEST(AccessLogFormatterTest, requestInfoFormatter) { } { - RequestInfoFormatter failure_reason_format("FAILURE_REASON"); - FailureReason reason = FailureReason::LocalReset; - EXPECT_CALL(requestInfo, failureReason()).WillOnce(Return(reason)); - EXPECT_EQ("LR", failure_reason_format.format(header, header, requestInfo)); + RequestInfoFormatter response_flags_format("RESPONSE_FLAGS"); + ON_CALL(requestInfo, getResponseFlag(ResponseFlag::LocalReset)).WillByDefault(Return(true)); + EXPECT_EQ("LR", response_flags_format.format(header, header, requestInfo)); } { diff --git a/test/common/http/access_log/access_log_impl_test.cc b/test/common/http/access_log/access_log_impl_test.cc index 0d8eff2d692b8..fa63697cd29ed 100644 --- a/test/common/http/access_log/access_log_impl_test.cc +++ b/test/common/http/access_log/access_log_impl_test.cc @@ -43,8 +43,10 @@ class TestRequestInfo : public RequestInfo { std::chrono::milliseconds duration() const override { return std::chrono::milliseconds(duration_); } - Http::AccessLog::FailureReason failureReason() const override { return failure_reason_; } - void onFailedResponse(FailureReason failure_reason) override { failure_reason_ = failure_reason; } + bool getResponseFlag(ResponseFlag response_flag) const override { + return response_flags_ & response_flag; + } + void setResponseFlag(ResponseFlag response_flag) override { response_flags_ |= response_flag; } void onUpstreamHostSelected(Upstream::HostDescriptionPtr host) override { upstream_host_ = host; } Upstream::HostDescriptionPtr upstreamHost() const override { return upstream_host_; } bool healthCheck() const override { return hc_request_; } @@ -53,7 +55,7 @@ class TestRequestInfo : public RequestInfo { SystemTime start_time_; Protocol protocol_{Protocol::Http11}; Optional response_code_; - FailureReason failure_reason_{FailureReason::None}; + uint64_t response_flags_{}; uint64_t duration_{3}; Upstream::HostDescriptionPtr upstream_host_{}; bool hc_request_{}; @@ -86,7 +88,7 @@ TEST_F(AccessLogImplTest, LogMoreData) { InstancePtr log = InstanceImpl::fromJson(*loader, runtime_, log_manager_); EXPECT_CALL(*file_, write(_)); - request_info_.failure_reason_ = FailureReason::UpstreamConnectionFailure; + request_info_.response_flags_ = ResponseFlag::UpstreamConnectionFailure; request_headers_.addViaCopy(Http::Headers::get().UserAgent, "user-agent-set"); request_headers_.addViaCopy(Http::Headers::get().RequestId, "id"); request_headers_.addViaCopy(Http::Headers::get().Host, "host"); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index c7387bf5c3498..4dfde80bf2943 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -61,13 +61,38 @@ TEST_F(ConnectionManagerUtilityTest, ShouldTraceRequest) { NiceMock request_info; Optional tracing_failure( {"operation", Http::TracingType::UpstreamFailure}); - EXPECT_CALL(request_info, failureReason()) - .WillOnce(Return(Http::AccessLog::FailureReason::UpstreamConnectionFailure)); - EXPECT_TRUE(ConnectionManagerUtility::shouldTraceRequest(request_info, tracing_failure)); - EXPECT_CALL(request_info, failureReason()) - .WillOnce(Return(Http::AccessLog::FailureReason::None)); - EXPECT_FALSE(ConnectionManagerUtility::shouldTraceRequest(request_info, tracing_failure)); + std::vector upstream_failed_response_flag{ + Http::AccessLog::ResponseFlag::UpstreamConnectionFailure, + Http::AccessLog::ResponseFlag::UpstreamConnectionTermination, + Http::AccessLog::ResponseFlag::NoHealthyUpstream, + Http::AccessLog::ResponseFlag::UpstreamRequestTimeout, + Http::AccessLog::ResponseFlag::UpstreamOverflow, + Http::AccessLog::ResponseFlag::FaultInjected, Http::AccessLog::ResponseFlag::NoRouteFound}; + + for (Http::AccessLog::ResponseFlag flag : upstream_failed_response_flag) { + ON_CALL(request_info, getResponseFlag(flag)).WillByDefault(Return(true)); + EXPECT_TRUE(ConnectionManagerUtility::shouldTraceRequest(request_info, tracing_failure)); + + ON_CALL(request_info, getResponseFlag(flag)).WillByDefault(Return(false)); + EXPECT_FALSE(ConnectionManagerUtility::shouldTraceRequest(request_info, tracing_failure)); + } + } + + { + NiceMock request_info; + Optional tracing_failure( + {"operation", Http::TracingType::UpstreamFailure}); + + std::vector upstream_not_failed_response_flag{ + Http::AccessLog::ResponseFlag::FailedLocalHealthCheck, + Http::AccessLog::ResponseFlag::LocalReset, + Http::AccessLog::ResponseFlag::UpstreamRemoteReset}; + + for (Http::AccessLog::ResponseFlag flag : upstream_not_failed_response_flag) { + ON_CALL(request_info, getResponseFlag(flag)).WillByDefault(Return(true)); + EXPECT_FALSE(ConnectionManagerUtility::shouldTraceRequest(request_info, tracing_failure)); + } } } diff --git a/test/common/http/filter/fault_filter_test.cc b/test/common/http/filter/fault_filter_test.cc index 669c9d7a54f99..421bd48cf49b2 100644 --- a/test/common/http/filter/fault_filter_test.cc +++ b/test/common/http/filter/fault_filter_test.cc @@ -214,7 +214,7 @@ TEST_F(FaultFilterTest, AbortWithHttpStatus) { EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); EXPECT_CALL(filter_callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FaultInjected)); + setResponseFlag(Http::AccessLog::ResponseFlag::FaultInjected)); EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(data_, false)); @@ -244,7 +244,7 @@ TEST_F(FaultFilterTest, FixedDelayZeroDuration) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", _)).Times(0); EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_CALL(filter_callbacks_.request_info_, onFailedResponse(_)).Times(0); + EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(_)).Times(0); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); // Expect filter to continue execution when delay is 0ms @@ -281,7 +281,7 @@ TEST_F(FaultFilterTest, FixedDelayNonZeroDuration) { // Delay only case EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", _)).Times(0); EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_CALL(filter_callbacks_.request_info_, onFailedResponse(_)).Times(0); + EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(_)).Times(0); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(1); timer_->callback_(); @@ -322,7 +322,7 @@ TEST_F(FaultFilterTest, FixedDelayAndAbort) { EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); EXPECT_CALL(filter_callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FaultInjected)); + setResponseFlag(Http::AccessLog::ResponseFlag::FaultInjected)); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); @@ -366,7 +366,7 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortHeaderMatchSuccess) { Http::TestHeaderMapImpl response_headers{{":status", "503"}}; EXPECT_CALL(filter_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); EXPECT_CALL(filter_callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FaultInjected)); + setResponseFlag(Http::AccessLog::ResponseFlag::FaultInjected)); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); @@ -390,7 +390,7 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortHeaderMatchFail) { EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.abort.abort_percent", _)).Times(0); EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", _)).Times(0); EXPECT_CALL(filter_callbacks_, encodeHeaders_(_, _)).Times(0); - EXPECT_CALL(filter_callbacks_.request_info_, onFailedResponse(_)).Times(0); + EXPECT_CALL(filter_callbacks_.request_info_, setResponseFlag(_)).Times(0); EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0); // Expect filter to continue execution when headers don't match diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 1b633f89ef746..90c411d53b933 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -76,7 +76,7 @@ class RouterTest : public testing::Test { TEST_F(RouterTest, RouteNotFound) { EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::NoRouteFound)); + setResponseFlag(Http::AccessLog::ResponseFlag::NoRouteFound)); Http::TestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -105,7 +105,7 @@ TEST_F(RouterTest, PoolFailureWithPriority) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamConnectionFailure)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamConnectionFailure)); EXPECT_CALL(callbacks_.request_info_, onUpstreamHostSelected(_)) .WillOnce(Invoke([&](const Upstream::HostDescriptionPtr host) -> void { EXPECT_EQ(host_url_, host->url()); })); @@ -136,7 +136,7 @@ TEST_F(RouterTest, NoHost) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::NoHealthyUpstream)); + setResponseFlag(Http::AccessLog::ResponseFlag::NoHealthyUpstream)); Http::TestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -151,7 +151,7 @@ TEST_F(RouterTest, MaintenanceMode) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamOverflow)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamOverflow)); Http::TestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -203,7 +203,7 @@ TEST_F(RouterTest, UpstreamTimeout) { router_.decodeData(data, true); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamRequestTimeout)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamRequestTimeout)); EXPECT_CALL(encoder.stream_, resetStream(Http::StreamResetReason::LocalReset)); Http::TestHeaderMapImpl response_headers{ {":status", "504"}, {"content-length", "24"}, {"content-type", "text/plain"}}; @@ -243,7 +243,7 @@ TEST_F(RouterTest, UpstreamPerTryTimeout) { router_.decodeData(data, true); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamRequestTimeout)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamRequestTimeout)); EXPECT_CALL(encoder.stream_, resetStream(Http::StreamResetReason::LocalReset)); Http::TestHeaderMapImpl response_headers{ {":status", "504"}, {"content-length", "24"}, {"content-type", "text/plain"}}; @@ -269,7 +269,7 @@ TEST_F(RouterTest, RetryRequestNotComplete) { return nullptr; })); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamRemoteReset)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamRemoteReset)); EXPECT_CALL(callbacks_.request_info_, onUpstreamHostSelected(_)) .WillOnce(Invoke([&](const Upstream::HostDescriptionPtr host) -> void { EXPECT_EQ(host_url_, host->url()); })); @@ -313,7 +313,7 @@ TEST_F(RouterTest, RetryNoneHealthy) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::NoHealthyUpstream)); + setResponseFlag(Http::AccessLog::ResponseFlag::NoHealthyUpstream)); router_.retry_state_->callback_(); } @@ -488,7 +488,8 @@ TEST_F(RouterTest, RetryTimeoutDuringRetryDelay) { // Fire timeout. EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::UpstreamRequestTimeout)); + setResponseFlag(Http::AccessLog::ResponseFlag::UpstreamRequestTimeout)); + EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putResponseTime(_)).Times(0); Http::TestHeaderMapImpl response_headers{ {":status", "504"}, {"content-length", "24"}, {"content-type", "text/plain"}}; diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index 615b1e3f3678b..7e11509e48b0d 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -408,6 +408,7 @@ TEST_F(LightStepSinkTest, FlushSeveralSpans) { setupValidSink(); NiceMock request_info; + ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(true)); Http::MockAsyncClientRequest request(&cm_.async_client_); Http::AsyncClient::Callbacks* callback; const Optional timeout(std::chrono::seconds(5)); @@ -477,6 +478,7 @@ TEST_F(LightStepSinkTest, FlushSpansTimer) { setupValidSink(); NiceMock request_info; + ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(true)); const Optional timeout(std::chrono::seconds(5)); EXPECT_CALL(cm_.async_client_, send_(_, _, timeout)); @@ -512,6 +514,7 @@ TEST_F(LightStepSinkTest, FlushOneSpanGrpcFailure) { setupValidSink(); NiceMock request_info; + ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(true)); Http::MockAsyncClientRequest request(&cm_.async_client_); Http::AsyncClient::Callbacks* callback; const Optional timeout(std::chrono::seconds(5)); diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index 711e755dfe6f6..0786d26dcd6dd 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -33,7 +33,7 @@ class MockRequestInfo : public RequestInfo { ~MockRequestInfo(); // Http::AccessLog::RequestInfo - MOCK_METHOD1(onFailedResponse, void(FailureReason failure_reason)); + MOCK_METHOD1(setResponseFlag, void(ResponseFlag response_flag)); MOCK_METHOD1(onUpstreamHostSelected, void(Upstream::HostDescriptionPtr host)); MOCK_CONST_METHOD0(startTime, SystemTime()); MOCK_CONST_METHOD0(bytesReceived, uint64_t()); @@ -42,7 +42,7 @@ class MockRequestInfo : public RequestInfo { MOCK_CONST_METHOD0(responseCode, Optional&()); MOCK_CONST_METHOD0(bytesSent, uint64_t()); MOCK_CONST_METHOD0(duration, std::chrono::milliseconds()); - MOCK_CONST_METHOD0(failureReason, FailureReason()); + MOCK_CONST_METHOD1(getResponseFlag, bool(Http::AccessLog::ResponseFlag)); MOCK_CONST_METHOD0(upstreamHost, Upstream::HostDescriptionPtr()); MOCK_CONST_METHOD0(healthCheck, bool()); MOCK_METHOD1(healthCheck, void(bool is_hc)); diff --git a/test/server/http/health_check_test.cc b/test/server/http/health_check_test.cc index daca189202970..b70a7b59e2c07 100644 --- a/test/server/http/health_check_test.cc +++ b/test/server/http/health_check_test.cc @@ -83,7 +83,7 @@ TEST_F(HealthCheckFilterNoPassThroughTest, HealthCheckFailedCallbackCalled) { })); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FailedLocalHealthCheck)); + setResponseFlag(Http::AccessLog::ResponseFlag::FailedLocalHealthCheck)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false)); @@ -133,7 +133,7 @@ TEST_F(HealthCheckFilterCachingTest, CachedServiceUnavailableCallbackCalled) { })); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FailedLocalHealthCheck)); + setResponseFlag(Http::AccessLog::ResponseFlag::FailedLocalHealthCheck)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, true)); @@ -170,7 +170,7 @@ TEST_F(HealthCheckFilterCachingTest, All) { // Verify that the next request uses the cached value. prepareFilter(true); EXPECT_CALL(callbacks_.request_info_, - onFailedResponse(Http::AccessLog::FailureReason::FailedLocalHealthCheck)); + setResponseFlag(Http::AccessLog::ResponseFlag::FailedLocalHealthCheck)); EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)) .Times(1) .WillRepeatedly(Invoke([&](Http::HeaderMap& headers, bool end_stream) {