diff --git a/api/bazel/repositories.bzl b/api/bazel/repositories.bzl index 840b2c6625c42..b1550bf7851ac 100644 --- a/api/bazel/repositories.bzl +++ b/api/bazel/repositories.bzl @@ -3,7 +3,7 @@ GOGOPROTO_SHA = "1adfc126b41513cc696b209667c8656ea7aac67c" # v1.0.0 PROMETHEUS_SHA = "99fa1f4be8e564e8a6b613da7fa6f46c9edafc6c" # Nov 17, 2017 OPENCENSUS_SHA = "ab82e5fdec8267dc2a726544b10af97675970847" # May 23, 2018 -PGV_GIT_SHA = "345b6b478ef955ad31382955d21fb504e95f38c7" +PGV_GIT_SHA = "f9d2b11e44149635b23a002693b76512b01ae515" load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") diff --git a/api/envoy/config/filter/accesslog/v2/accesslog.proto b/api/envoy/config/filter/accesslog/v2/accesslog.proto index af78123e36091..6642560694eaf 100644 --- a/api/envoy/config/filter/accesslog/v2/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v2/accesslog.proto @@ -61,6 +61,9 @@ message AccessLogFilter { // Header filter. HeaderFilter header_filter = 8; + + // Response flag filter. + ResponseFlagFilter response_flag_filter = 9; } } @@ -150,3 +153,15 @@ message HeaderFilter { // check. envoy.api.v2.route.HeaderMatcher header = 1 [(validate.rules).message.required = true]; } + +// Filters requests that received responses with an Envoy response flag set. +// A list of the response flags can be found +// in the access log formatter :ref:`documentation`. +message ResponseFlagFilter { + // Only responses with the any of the flags listed in this field will be logged. + // This field is optional. If it is not specified, then any response flag will pass + // the filter check. + repeated string flags = 1 [(validate.rules).repeated .items.string = { + in: ["LH", "UH", "UT", "LR", "UR", "UF", "UC", "UO", "NR", "DI", "FI", "RL", "UAEX"] + }]; +} diff --git a/docs/root/configuration/access_log.rst b/docs/root/configuration/access_log.rst index 88946fa75f669..c85bd554aa104 100644 --- a/docs/root/configuration/access_log.rst +++ b/docs/root/configuration/access_log.rst @@ -102,6 +102,8 @@ The following command operators are supported: TCP Total duration in milliseconds of the downstream connection. +.. _config_access_log_format_response_flags: + %RESPONSE_FLAGS% Additional details about the response or connection, if any. For TCP connections, the response codes mentioned in the descriptions do not apply. Possible values are: diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index c0e81b600d072..ebc3c78fdfa51 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,6 +3,8 @@ Version history 1.8.0 (Pending) =============== +* access log: added :ref:`response flag filter ` + to filter based on the presence of Envoy response flags. * admin: added :http:get:`/hystrix_event_stream` as an endpoint for monitoring envoy's statistics through `Hystrix dashboard `_. * http: response filters not applied to early error paths such as http_parser generated 400s. diff --git a/include/envoy/request_info/request_info.h b/include/envoy/request_info/request_info.h index b3bb71248e721..ee21a3ae7e8b1 100644 --- a/include/envoy/request_info/request_info.h +++ b/include/envoy/request_info/request_info.h @@ -64,6 +64,13 @@ class RequestInfo { */ virtual void setResponseFlag(ResponseFlag response_flag) PURE; + /** + * @param response_flags the response_flags to intersect with. + * @return true if the intersection of the response_flags argument and the currently set response + * flags is non-empty. + */ + virtual bool intersectResponseFlags(uint64_t response_flags) const PURE; + /** * @param host the selected upstream host for the request. */ @@ -216,7 +223,12 @@ class RequestInfo { /** * @return whether response flag is set or not. */ - virtual bool getResponseFlag(ResponseFlag response_flag) const PURE; + virtual bool hasResponseFlag(ResponseFlag response_flag) const PURE; + + /** + * @return whether any response flag is set or not. + */ + virtual bool hasAnyResponseFlag() const PURE; /** * @return upstream host description. diff --git a/source/common/access_log/BUILD b/source/common/access_log/BUILD index 40896979eae9d..afbca0e51e2cf 100644 --- a/source/common/access_log/BUILD +++ b/source/common/access_log/BUILD @@ -51,7 +51,10 @@ envoy_cc_library( "//source/common/http:header_utility_lib", "//source/common/http:headers_lib", "//source/common/http:utility_lib", + "//source/common/protobuf:utility_lib", + "//source/common/request_info:request_info_lib", "//source/common/runtime:uuid_util_lib", "//source/common/tracing:http_tracer_lib", + "@envoy_api//envoy/config/filter/accesslog/v2:accesslog_cc", ], ) diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc index 74f0f5b677fdb..3fa7fbbc061f5 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -4,6 +4,7 @@ #include #include "envoy/common/time.h" +#include "envoy/config/filter/accesslog/v2/accesslog.pb.validate.h" #include "envoy/filesystem/filesystem.h" #include "envoy/http/header_map.h" #include "envoy/runtime/runtime.h" @@ -17,6 +18,8 @@ #include "common/http/header_utility.h" #include "common/http/headers.h" #include "common/http/utility.h" +#include "common/protobuf/utility.h" +#include "common/request_info/utility.h" #include "common/runtime/uuid_util.h" #include "common/tracing/http_tracer_impl.h" @@ -68,6 +71,9 @@ FilterFactory::fromProto(const envoy::config::filter::accesslog::v2::AccessLogFi return FilterPtr{new OrFilter(config.or_filter(), runtime, random)}; case envoy::config::filter::accesslog::v2::AccessLogFilter::kHeaderFilter: return FilterPtr{new HeaderFilter(config.header_filter())}; + case envoy::config::filter::accesslog::v2::AccessLogFilter::kResponseFlagFilter: + MessageUtil::validate(config); + return FilterPtr{new ResponseFlagFilter(config.response_flag_filter())}; default: NOT_REACHED; } @@ -174,6 +180,24 @@ bool HeaderFilter::evaluate(const RequestInfo::RequestInfo&, return Http::HeaderUtility::matchHeaders(request_headers, header_data_); } +ResponseFlagFilter::ResponseFlagFilter( + const envoy::config::filter::accesslog::v2::ResponseFlagFilter& config) { + for (int i = 0; i < config.flags_size(); i++) { + absl::optional response_flag = + RequestInfo::ResponseFlagUtils::toResponseFlag(config.flags(i)); + // The config has been validated. Therefore, every flag in the config will have a mapping. + ASSERT(response_flag.has_value()); + configured_flags_ |= response_flag.value(); + } +} + +bool ResponseFlagFilter::evaluate(const RequestInfo::RequestInfo& info, const Http::HeaderMap&) { + if (configured_flags_ != 0) { + return info.intersectResponseFlags(configured_flags_); + } + return info.hasAnyResponseFlag(); +} + InstanceSharedPtr AccessLogFactory::fromProto(const envoy::config::filter::accesslog::v2::AccessLog& config, Server::Configuration::FactoryContext& context) { diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h index 63c22735f996a..cad54fd633012 100644 --- a/source/common/access_log/access_log_impl.h +++ b/source/common/access_log/access_log_impl.h @@ -6,7 +6,6 @@ #include "envoy/access_log/access_log.h" #include "envoy/config/filter/accesslog/v2/accesslog.pb.h" -#include "envoy/request_info/request_info.h" #include "envoy/runtime/runtime.h" #include "envoy/server/access_log_config.h" @@ -166,6 +165,21 @@ class HeaderFilter : public Filter { std::vector header_data_; }; +/** + * Filter requests that had a response with an Envoy response flag set. + */ +class ResponseFlagFilter : public Filter { +public: + ResponseFlagFilter(const envoy::config::filter::accesslog::v2::ResponseFlagFilter& config); + + // AccessLog::Filter + bool evaluate(const RequestInfo::RequestInfo& info, + const Http::HeaderMap& request_headers) override; + +private: + uint64_t configured_flags_{}; +}; + /** * Access log factory that reads the configuration from proto. */ diff --git a/source/common/request_info/request_info_impl.h b/source/common/request_info/request_info_impl.h index 6adced2357e91..f56f34ebc4e52 100644 --- a/source/common/request_info/request_info_impl.h +++ b/source/common/request_info/request_info_impl.h @@ -125,7 +125,13 @@ struct RequestInfoImpl : public RequestInfo { void setResponseFlag(ResponseFlag response_flag) override { response_flags_ |= response_flag; } - bool getResponseFlag(ResponseFlag flag) const override { return response_flags_ & flag; } + bool intersectResponseFlags(uint64_t response_flags) const override { + return (response_flags_ & response_flags) != 0; + } + + bool hasResponseFlag(ResponseFlag flag) const override { return response_flags_ & flag; } + + bool hasAnyResponseFlag() const override { return response_flags_ != 0; } void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override { upstream_host_ = host; diff --git a/source/common/request_info/utility.cc b/source/common/request_info/utility.cc index b8138833068b2..27a01a6b24453 100644 --- a/source/common/request_info/utility.cc +++ b/source/common/request_info/utility.cc @@ -33,61 +33,85 @@ const std::string ResponseFlagUtils::toShortString(const RequestInfo& request_in static_assert(ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code."); - if (request_info.getResponseFlag(ResponseFlag::FailedLocalHealthCheck)) { + if (request_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) { appendString(result, FAILED_LOCAL_HEALTH_CHECK); } - if (request_info.getResponseFlag(ResponseFlag::NoHealthyUpstream)) { + if (request_info.hasResponseFlag(ResponseFlag::NoHealthyUpstream)) { appendString(result, NO_HEALTHY_UPSTREAM); } - if (request_info.getResponseFlag(ResponseFlag::UpstreamRequestTimeout)) { + if (request_info.hasResponseFlag(ResponseFlag::UpstreamRequestTimeout)) { appendString(result, UPSTREAM_REQUEST_TIMEOUT); } - if (request_info.getResponseFlag(ResponseFlag::LocalReset)) { + if (request_info.hasResponseFlag(ResponseFlag::LocalReset)) { appendString(result, LOCAL_RESET); } - if (request_info.getResponseFlag(ResponseFlag::UpstreamRemoteReset)) { + if (request_info.hasResponseFlag(ResponseFlag::UpstreamRemoteReset)) { appendString(result, UPSTREAM_REMOTE_RESET); } - if (request_info.getResponseFlag(ResponseFlag::UpstreamConnectionFailure)) { + if (request_info.hasResponseFlag(ResponseFlag::UpstreamConnectionFailure)) { appendString(result, UPSTREAM_CONNECTION_FAILURE); } - if (request_info.getResponseFlag(ResponseFlag::UpstreamConnectionTermination)) { + if (request_info.hasResponseFlag(ResponseFlag::UpstreamConnectionTermination)) { appendString(result, UPSTREAM_CONNECTION_TERMINATION); } - if (request_info.getResponseFlag(ResponseFlag::UpstreamOverflow)) { + if (request_info.hasResponseFlag(ResponseFlag::UpstreamOverflow)) { appendString(result, UPSTREAM_OVERFLOW); } - if (request_info.getResponseFlag(ResponseFlag::NoRouteFound)) { + if (request_info.hasResponseFlag(ResponseFlag::NoRouteFound)) { appendString(result, NO_ROUTE_FOUND); } - if (request_info.getResponseFlag(ResponseFlag::DelayInjected)) { + if (request_info.hasResponseFlag(ResponseFlag::DelayInjected)) { appendString(result, DELAY_INJECTED); } - if (request_info.getResponseFlag(ResponseFlag::FaultInjected)) { + if (request_info.hasResponseFlag(ResponseFlag::FaultInjected)) { appendString(result, FAULT_INJECTED); } - if (request_info.getResponseFlag(ResponseFlag::RateLimited)) { + if (request_info.hasResponseFlag(ResponseFlag::RateLimited)) { appendString(result, RATE_LIMITED); } - if (request_info.getResponseFlag(ResponseFlag::UnauthorizedExternalService)) { + if (request_info.hasResponseFlag(ResponseFlag::UnauthorizedExternalService)) { appendString(result, UNAUTHORIZED_EXTERNAL_SERVICE); } return result.empty() ? NONE : result; } +absl::optional ResponseFlagUtils::toResponseFlag(const std::string& flag) { + static const std::map map = { + {ResponseFlagUtils::FAILED_LOCAL_HEALTH_CHECK, ResponseFlag::FailedLocalHealthCheck}, + {ResponseFlagUtils::NO_HEALTHY_UPSTREAM, ResponseFlag::NoHealthyUpstream}, + {ResponseFlagUtils::UPSTREAM_REQUEST_TIMEOUT, ResponseFlag::UpstreamRequestTimeout}, + {ResponseFlagUtils::LOCAL_RESET, ResponseFlag::LocalReset}, + {ResponseFlagUtils::UPSTREAM_REMOTE_RESET, ResponseFlag::UpstreamRemoteReset}, + {ResponseFlagUtils::UPSTREAM_CONNECTION_FAILURE, ResponseFlag::UpstreamConnectionFailure}, + {ResponseFlagUtils::UPSTREAM_CONNECTION_TERMINATION, + ResponseFlag::UpstreamConnectionTermination}, + {ResponseFlagUtils::UPSTREAM_OVERFLOW, ResponseFlag::UpstreamOverflow}, + {ResponseFlagUtils::NO_ROUTE_FOUND, ResponseFlag::NoRouteFound}, + {ResponseFlagUtils::DELAY_INJECTED, ResponseFlag::DelayInjected}, + {ResponseFlagUtils::FAULT_INJECTED, ResponseFlag::FaultInjected}, + {ResponseFlagUtils::RATE_LIMITED, ResponseFlag::RateLimited}, + {ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE, ResponseFlag::UnauthorizedExternalService}, + }; + const auto& it = map.find(flag); + if (it != map.end()) { + return absl::make_optional(it->second); + } + return absl::nullopt; +} + const std::string& Utility::formatDownstreamAddressNoPort(const Network::Address::Instance& address) { if (address.type() == Network::Address::Type::Ip) { diff --git a/source/common/request_info/utility.h b/source/common/request_info/utility.h index 15b34fb83c42a..879bccd196cff 100644 --- a/source/common/request_info/utility.h +++ b/source/common/request_info/utility.h @@ -14,6 +14,7 @@ namespace RequestInfo { class ResponseFlagUtils { public: static const std::string toShortString(const RequestInfo& request_info); + static absl::optional toResponseFlag(const std::string& response_flag); private: ResponseFlagUtils(); diff --git a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc index b90bbfc352dbf..6804cdd9bd617 100644 --- a/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc +++ b/source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc @@ -88,55 +88,55 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags( static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code."); - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::FailedLocalHealthCheck)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::FailedLocalHealthCheck)) { common_access_log.mutable_response_flags()->set_failed_local_healthcheck(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::NoHealthyUpstream)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::NoHealthyUpstream)) { common_access_log.mutable_response_flags()->set_no_healthy_upstream(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UpstreamRequestTimeout)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UpstreamRequestTimeout)) { common_access_log.mutable_response_flags()->set_upstream_request_timeout(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::LocalReset)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::LocalReset)) { common_access_log.mutable_response_flags()->set_local_reset(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UpstreamRemoteReset)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UpstreamRemoteReset)) { common_access_log.mutable_response_flags()->set_upstream_remote_reset(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UpstreamConnectionFailure)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UpstreamConnectionFailure)) { common_access_log.mutable_response_flags()->set_upstream_connection_failure(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UpstreamConnectionTermination)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UpstreamConnectionTermination)) { common_access_log.mutable_response_flags()->set_upstream_connection_termination(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UpstreamOverflow)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UpstreamOverflow)) { common_access_log.mutable_response_flags()->set_upstream_overflow(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::NoRouteFound)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::NoRouteFound)) { common_access_log.mutable_response_flags()->set_no_route_found(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::DelayInjected)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::DelayInjected)) { common_access_log.mutable_response_flags()->set_delay_injected(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::FaultInjected)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::FaultInjected)) { common_access_log.mutable_response_flags()->set_fault_injected(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::RateLimited)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::RateLimited)) { common_access_log.mutable_response_flags()->set_rate_limited(true); } - if (request_info.getResponseFlag(RequestInfo::ResponseFlag::UnauthorizedExternalService)) { + if (request_info.hasResponseFlag(RequestInfo::ResponseFlag::UnauthorizedExternalService)) { common_access_log.mutable_response_flags()->mutable_unauthorized_details()->set_reason( envoy::data::accesslog::v2::ResponseFlags_Unauthorized_Reason:: ResponseFlags_Unauthorized_Reason_EXTERNAL_SERVICE); diff --git a/test/common/access_log/access_log_formatter_test.cc b/test/common/access_log/access_log_formatter_test.cc index 6cc11f9234643..fe711bf4f2b1e 100644 --- a/test/common/access_log/access_log_formatter_test.cc +++ b/test/common/access_log/access_log_formatter_test.cc @@ -115,7 +115,7 @@ TEST(AccessLogFormatterTest, requestInfoFormatter) { { RequestInfoFormatter response_flags_format("RESPONSE_FLAGS"); - ON_CALL(request_info, getResponseFlag(RequestInfo::ResponseFlag::LocalReset)) + ON_CALL(request_info, hasResponseFlag(RequestInfo::ResponseFlag::LocalReset)) .WillByDefault(Return(true)); EXPECT_EQ("LR", response_flags_format.format(header, header, header, request_info)); } diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index 8b51a6b3d0a41..cf9b939d825cd 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -52,8 +52,8 @@ class AccessLogImplTest : public testing::Test { public: AccessLogImplTest() : file_(new Filesystem::MockFile()) { ON_CALL(context_, runtime()).WillByDefault(ReturnRef(runtime_)); - EXPECT_CALL(context_, accessLogManager()).WillOnce(ReturnRef(log_manager_)); - EXPECT_CALL(log_manager_, createAccessLog(_)).WillOnce(Return(file_)); + ON_CALL(context_, accessLogManager()).WillByDefault(ReturnRef(log_manager_)); + ON_CALL(log_manager_, createAccessLog(_)).WillByDefault(Return(file_)); ON_CALL(*file_, write(_)).WillByDefault(SaveArg<0>(&output_)); } @@ -724,6 +724,149 @@ name: envoy.file_access_log log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); } +TEST_F(AccessLogImplTest, ResponseFlagFilterAnyFlag) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: {} +config: + path: /dev/null + )EOF"; + + InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); + + EXPECT_CALL(*file_, write(_)).Times(0); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); + + request_info_.setResponseFlag(RequestInfo::ResponseFlag::NoRouteFound); + EXPECT_CALL(*file_, write(_)); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); +} + +TEST_F(AccessLogImplTest, ResponseFlagFilterSpecificFlag) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: + flags: + - UO +config: + path: /dev/null + )EOF"; + + InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); + + EXPECT_CALL(*file_, write(_)).Times(0); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); + + request_info_.setResponseFlag(RequestInfo::ResponseFlag::NoRouteFound); + EXPECT_CALL(*file_, write(_)).Times(0); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); + + request_info_.setResponseFlag(RequestInfo::ResponseFlag::UpstreamOverflow); + EXPECT_CALL(*file_, write(_)); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); +} + +TEST_F(AccessLogImplTest, ResponseFlagFilterSeveralFlags) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: + flags: + - UO + - RL +config: + path: /dev/null + )EOF"; + + InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); + + EXPECT_CALL(*file_, write(_)).Times(0); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); + + request_info_.setResponseFlag(RequestInfo::ResponseFlag::NoRouteFound); + EXPECT_CALL(*file_, write(_)).Times(0); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); + + request_info_.setResponseFlag(RequestInfo::ResponseFlag::UpstreamOverflow); + EXPECT_CALL(*file_, write(_)); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_); +} + +TEST_F(AccessLogImplTest, ResponseFlagFilterAllFlagsInPGV) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: + flags: + - LH + - UH + - UT + - LR + - UR + - UF + - UC + - UO + - NR + - DI + - FI + - RL + - UAEX +config: + path: /dev/null + )EOF"; + + static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1000, + "A flag has been added. Fix this code."); + + std::vector all_response_flags = { + RequestInfo::ResponseFlag::FailedLocalHealthCheck, + RequestInfo::ResponseFlag::NoHealthyUpstream, + RequestInfo::ResponseFlag::UpstreamRequestTimeout, + RequestInfo::ResponseFlag::LocalReset, + RequestInfo::ResponseFlag::UpstreamRemoteReset, + RequestInfo::ResponseFlag::UpstreamConnectionFailure, + RequestInfo::ResponseFlag::UpstreamConnectionTermination, + RequestInfo::ResponseFlag::UpstreamOverflow, + RequestInfo::ResponseFlag::NoRouteFound, + RequestInfo::ResponseFlag::DelayInjected, + RequestInfo::ResponseFlag::FaultInjected, + RequestInfo::ResponseFlag::RateLimited, + RequestInfo::ResponseFlag::UnauthorizedExternalService, + }; + + InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); + + for (const auto response_flag : all_response_flags) { + TestRequestInfo request_info; + request_info.setResponseFlag(response_flag); + EXPECT_CALL(*file_, write(_)); + log->log(&request_headers_, &response_headers_, &response_trailers_, request_info); + } +} + +TEST_F(AccessLogImplTest, ResponseFlagFilterUnsupportedFlag) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: + flags: + - UnsupportedFlag +config: + path: /dev/null + )EOF"; + + EXPECT_THROW_WITH_MESSAGE( + AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_), + ProtoValidationException, + "Proto constraint validation failed (AccessLogFilterValidationError.ResponseFlagFilter: " + "[\"embedded message failed validation\"] | caused by " + "ResponseFlagFilterValidationError.Flags[i]: [\"value must be in list \" [\"LH\" \"UH\" " + "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\"]]): " + "response_flag_filter {\n flags: \"UnsupportedFlag\"\n}\n"); +} + } // namespace } // namespace AccessLog } // namespace Envoy diff --git a/test/common/access_log/test_util.h b/test/common/access_log/test_util.h index b6de049ee64ed..42bc015aebd6f 100644 --- a/test/common/access_log/test_util.h +++ b/test/common/access_log/test_util.h @@ -28,10 +28,13 @@ class TestRequestInfo : public RequestInfo::RequestInfo { absl::optional responseCode() const override { return response_code_; } void addBytesSent(uint64_t) override { NOT_IMPLEMENTED; } uint64_t bytesSent() const override { return 2; } - - bool getResponseFlag(Envoy::RequestInfo::ResponseFlag response_flag) const override { + bool intersectResponseFlags(uint64_t response_flags) const override { + return (response_flags_ & response_flags) != 0; + } + bool hasResponseFlag(Envoy::RequestInfo::ResponseFlag response_flag) const override { return response_flags_ & response_flag; } + bool hasAnyResponseFlag() const override { return response_flags_ != 0; } void setResponseFlag(Envoy::RequestInfo::ResponseFlag response_flag) override { response_flags_ |= response_flag; } diff --git a/test/common/request_info/request_info_impl_test.cc b/test/common/request_info/request_info_impl_test.cc index 1dd67ac3250c0..687a6264a3c86 100644 --- a/test/common/request_info/request_info_impl_test.cc +++ b/test/common/request_info/request_info_impl_test.cc @@ -96,14 +96,22 @@ TEST(RequestInfoImplTest, ResponseFlagTest) { RateLimited}; RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_FALSE(request_info.hasAnyResponseFlag()); + EXPECT_FALSE(request_info.intersectResponseFlags(0)); for (ResponseFlag flag : responseFlags) { // Test cumulative setting of response flags. - EXPECT_FALSE(request_info.getResponseFlag(flag)) + EXPECT_FALSE(request_info.hasResponseFlag(flag)) << fmt::format("Flag: {} was already set", flag); request_info.setResponseFlag(flag); - EXPECT_TRUE(request_info.getResponseFlag(flag)) + EXPECT_TRUE(request_info.hasResponseFlag(flag)) << fmt::format("Flag: {} was expected to be set", flag); } + EXPECT_TRUE(request_info.hasAnyResponseFlag()); + + RequestInfoImpl request_info2(Http::Protocol::Http2); + request_info2.setResponseFlag(FailedLocalHealthCheck); + + EXPECT_TRUE(request_info2.intersectResponseFlags(FailedLocalHealthCheck)); } TEST(RequestInfoImplTest, MiscSettersAndGetters) { diff --git a/test/common/request_info/utility_test.cc b/test/common/request_info/utility_test.cc index 47dba21103991..e9058a1f1ed5a 100644 --- a/test/common/request_info/utility_test.cc +++ b/test/common/request_info/utility_test.cc @@ -34,14 +34,14 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { for (const auto& test_case : expected) { NiceMock request_info; - ON_CALL(request_info, getResponseFlag(test_case.first)).WillByDefault(Return(true)); + ON_CALL(request_info, hasResponseFlag(test_case.first)).WillByDefault(Return(true)); EXPECT_EQ(test_case.second, ResponseFlagUtils::toShortString(request_info)); } // No flag is set. { NiceMock request_info; - ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(false)); + ON_CALL(request_info, hasResponseFlag(_)).WillByDefault(Return(false)); EXPECT_EQ("-", ResponseFlagUtils::toShortString(request_info)); } @@ -49,14 +49,42 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { // These are not real use cases, but are used to cover multiple response flags case. { NiceMock request_info; - ON_CALL(request_info, getResponseFlag(ResponseFlag::DelayInjected)).WillByDefault(Return(true)); - ON_CALL(request_info, getResponseFlag(ResponseFlag::FaultInjected)).WillByDefault(Return(true)); - ON_CALL(request_info, getResponseFlag(ResponseFlag::UpstreamRequestTimeout)) + ON_CALL(request_info, hasResponseFlag(ResponseFlag::DelayInjected)).WillByDefault(Return(true)); + ON_CALL(request_info, hasResponseFlag(ResponseFlag::FaultInjected)).WillByDefault(Return(true)); + ON_CALL(request_info, hasResponseFlag(ResponseFlag::UpstreamRequestTimeout)) .WillByDefault(Return(true)); EXPECT_EQ("UT,DI,FI", ResponseFlagUtils::toShortString(request_info)); } } +TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) { + static_assert(ResponseFlag::LastFlag == 0x1000, "A flag has been added. Fix this code."); + + std::vector> expected = { + std::make_pair("LH", ResponseFlag::FailedLocalHealthCheck), + std::make_pair("UH", ResponseFlag::NoHealthyUpstream), + std::make_pair("UT", ResponseFlag::UpstreamRequestTimeout), + std::make_pair("LR", ResponseFlag::LocalReset), + std::make_pair("UR", ResponseFlag::UpstreamRemoteReset), + std::make_pair("UF", ResponseFlag::UpstreamConnectionFailure), + std::make_pair("UC", ResponseFlag::UpstreamConnectionTermination), + std::make_pair("UO", ResponseFlag::UpstreamOverflow), + std::make_pair("NR", ResponseFlag::NoRouteFound), + std::make_pair("DI", ResponseFlag::DelayInjected), + std::make_pair("FI", ResponseFlag::FaultInjected), + std::make_pair("RL", ResponseFlag::RateLimited), + std::make_pair("UAEX", ResponseFlag::UnauthorizedExternalService), + }; + + EXPECT_FALSE(ResponseFlagUtils::toResponseFlag("NonExistentFlag").has_value()); + + for (const auto& test_case : expected) { + absl::optional response_flag = ResponseFlagUtils::toResponseFlag(test_case.first); + EXPECT_TRUE(response_flag.has_value()); + EXPECT_EQ(test_case.second, response_flag.value()); + } +} + TEST(UtilityTest, formatDownstreamAddressNoPort) { EXPECT_EQ("1.2.3.4", Utility::formatDownstreamAddressNoPort(Network::Address::Ipv4Instance("1.2.3.4"))); diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index 82d3516575f3c..1b6e2e61c6707 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -263,7 +263,7 @@ TEST(HttpConnManFinalizerImpl, SpanPopulatedFailureResponse) { absl::optional response_code(503); EXPECT_CALL(request_info, responseCode()).WillRepeatedly(ReturnPointee(&response_code)); EXPECT_CALL(request_info, bytesSent()).WillOnce(Return(100)); - ON_CALL(request_info, getResponseFlag(RequestInfo::ResponseFlag::UpstreamRequestTimeout)) + ON_CALL(request_info, hasResponseFlag(RequestInfo::ResponseFlag::UpstreamRequestTimeout)) .WillByDefault(Return(true)); EXPECT_CALL(request_info, upstreamHost()).WillOnce(Return(nullptr)); diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc index e365cff61b448..bfbb62bc7eeb6 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_impl_test.cc @@ -219,7 +219,7 @@ TEST_F(HttpGrpcAccessLogTest, Marshalling) { request_info.addBytesReceived(10); request_info.addBytesSent(20); request_info.response_code_ = 200; - ON_CALL(request_info, getResponseFlag(RequestInfo::ResponseFlag::FaultInjected)) + ON_CALL(request_info, hasResponseFlag(RequestInfo::ResponseFlag::FaultInjected)) .WillByDefault(Return(true)); Http::TestHeaderMapImpl request_headers{ @@ -414,7 +414,7 @@ TEST_F(HttpGrpcAccessLogTest, MarshallingAdditionalHeaders) { TEST(responseFlagsToAccessLogResponseFlagsTest, All) { NiceMock request_info; - ON_CALL(request_info, getResponseFlag(_)).WillByDefault(Return(true)); + ON_CALL(request_info, hasResponseFlag(_)).WillByDefault(Return(true)); envoy::data::accesslog::v2::AccessLogCommon common_access_log; HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags(common_access_log, request_info); diff --git a/test/mocks/request_info/mocks.h b/test/mocks/request_info/mocks.h index 1df9251000078..cafe6815e5bcc 100644 --- a/test/mocks/request_info/mocks.h +++ b/test/mocks/request_info/mocks.h @@ -16,6 +16,7 @@ class MockRequestInfo : public RequestInfo { // RequestInfo::RequestInfo MOCK_METHOD1(setResponseFlag, void(ResponseFlag response_flag)); + MOCK_CONST_METHOD1(intersectResponseFlags, bool(uint64_t)); MOCK_METHOD1(onUpstreamHostSelected, void(Upstream::HostDescriptionConstSharedPtr host)); MOCK_CONST_METHOD0(startTime, SystemTime()); MOCK_CONST_METHOD0(startTimeMonotonic, MonotonicTime()); @@ -43,7 +44,8 @@ class MockRequestInfo : public RequestInfo { MOCK_CONST_METHOD0(responseCode, absl::optional()); MOCK_METHOD1(addBytesSent, void(uint64_t)); MOCK_CONST_METHOD0(bytesSent, uint64_t()); - MOCK_CONST_METHOD1(getResponseFlag, bool(ResponseFlag)); + MOCK_CONST_METHOD1(hasResponseFlag, bool(ResponseFlag)); + MOCK_CONST_METHOD0(hasAnyResponseFlag, bool()); MOCK_CONST_METHOD0(upstreamHost, Upstream::HostDescriptionConstSharedPtr()); MOCK_METHOD1(setUpstreamLocalAddress, void(const Network::Address::InstanceConstSharedPtr&)); MOCK_CONST_METHOD0(upstreamLocalAddress, const Network::Address::InstanceConstSharedPtr&());