-
Notifications
You must be signed in to change notification settings - Fork 5.3k
config: added gRPC-based filter to access_log #5682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
f7ed72d
Defined GrpcStatusFilter proto
crockeo 0923e1a
Added a function to convert from grpc name to Status::GrpcStatus
crockeo 4cec844
Implemented GrpcStatusFilter
crockeo c9b7c1c
Added validation to GrpcStatusFilter
crockeo 0f73b01
Implemented validation-related tests
crockeo 6486b4d
Added tests to check that the filter blocks the correct traffic
crockeo d8fc6d4
Added documentation to GrpcStatusFilter
crockeo 0de9b81
Finished testing on request headers, did heavy lifting to modify code…
crockeo 188b155
Added explicit hash to statuses_ to compile correctly on GCC 5.4.0
crockeo 2ebb7d2
Fixed issue with missing hole in HttpGrpcAccessLogTest frame
crockeo c06354d
Addressed PR comments, aside from testing request.
crockeo 93f4a2e
Added a test to ensure the filter infers the correct gRPC status from…
crockeo 8e5a3d1
Changed initializer list to uniform initialization, and changed tuple…
crockeo ce9a103
Addressed comment updates
crockeo e94e234
Added test to check that, when lacking a gRPC status or HTTP status, …
crockeo 58bac8c
Fixed s/respone/response/
crockeo cab43ed
Added test to check status Grpc::Utility::nameToGrpcStatus validity
crockeo 93eb881
Added option to exclude based on gRPC status code
crockeo a562461
Included check for response_headers
crockeo 631b44c
Break for optimization!
crockeo a247343
Added test for exclude: false
crockeo f4eee7d
Changed validated strings to enum.
crockeo a38443f
More directly linked GrpcStatusFilter enum test to Status::GrpcStatus…
crockeo 569a63a
Addressed comments
crockeo 52829ad
Updated comments
crockeo af976c5
Merge remote-tracking branch 'upstream/master' into dev
crockeo 33df38d
CI test post-merge without explicit hash
crockeo 02da790
Removed GrpcStatusHash
crockeo 571a226
Fixed the invalid code test to work correctly variably serialized errors
crockeo 1ccf23f
Merge remote-tracking branch 'upstream/master' into dev
crockeo 1cbe0be
Merge remote-tracking branch 'upstream/master' into dev
crockeo 6ba655e
Added status validation to GrpcStatusFilter protobuf
crockeo 033d899
Replaced custom expect fail with substring with EXPECT_FAIL_WITH_REGEX
crockeo 6c9a4a3
Set up ClangFormat on the new machine and formatted
crockeo 7589725
Merge remote-tracking branch 'upstream/master' into dev
crockeo c64da70
Removed unused imports
crockeo 3fb87e8
Changed stringstream to fmtlib
crockeo 5e0e44b
Merge remote-tracking branch 'upstream/master' into dev
crockeo 968b011
Removed unused sstream include
crockeo 0915542
Changed vector to array
crockeo 7625146
Added release note to version history
crockeo 99db72b
Merge branch 'master' of github.com:envoyproxy/envoy into dev
crockeo ad58a61
Fixed mac-specific error with -Wmissing-braces
crockeo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,27 +74,33 @@ FilterFactory::fromProto(const envoy::config::filter::accesslog::v2::AccessLogFi | |
| case envoy::config::filter::accesslog::v2::AccessLogFilter::kResponseFlagFilter: | ||
| MessageUtil::validate(config); | ||
| return FilterPtr{new ResponseFlagFilter(config.response_flag_filter())}; | ||
| case envoy::config::filter::accesslog::v2::AccessLogFilter::kGrpcStatusFilter: | ||
| MessageUtil::validate(config); | ||
| return FilterPtr{new GrpcStatusFilter(config.grpc_status_filter())}; | ||
| default: | ||
| NOT_REACHED_GCOVR_EXCL_LINE; | ||
| } | ||
| } | ||
|
|
||
| bool TraceableRequestFilter::evaluate(const StreamInfo::StreamInfo& info, | ||
| const Http::HeaderMap& request_headers) { | ||
| const Http::HeaderMap& request_headers, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| Tracing::Decision decision = Tracing::HttpTracerUtility::isTracing(info, request_headers); | ||
|
|
||
| return decision.traced && decision.reason == Tracing::Reason::ServiceForced; | ||
| } | ||
|
|
||
| bool StatusCodeFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&) { | ||
| bool StatusCodeFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| if (!info.responseCode()) { | ||
| return compareAgainstValue(0ULL); | ||
| } | ||
|
|
||
| return compareAgainstValue(info.responseCode().value()); | ||
| } | ||
|
|
||
| bool DurationFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&) { | ||
| bool DurationFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| absl::optional<std::chrono::nanoseconds> final = info.requestComplete(); | ||
| ASSERT(final); | ||
|
|
||
|
|
@@ -108,7 +114,8 @@ RuntimeFilter::RuntimeFilter(const envoy::config::filter::accesslog::v2::Runtime | |
| percent_(config.percent_sampled()), | ||
| use_independent_randomness_(config.use_independent_randomness()) {} | ||
|
|
||
| bool RuntimeFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMap& request_header) { | ||
| bool RuntimeFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMap& request_header, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| const Http::HeaderEntry* uuid = request_header.RequestId(); | ||
| uint64_t random_value; | ||
| if (use_independent_randomness_ || uuid == nullptr || | ||
|
|
@@ -139,11 +146,12 @@ AndFilter::AndFilter(const envoy::config::filter::accesslog::v2::AndFilter& conf | |
| Runtime::Loader& runtime, Runtime::RandomGenerator& random) | ||
| : OperatorFilter(config.filters(), runtime, random) {} | ||
|
|
||
| bool OrFilter::evaluate(const StreamInfo::StreamInfo& info, | ||
| const Http::HeaderMap& request_headers) { | ||
| bool OrFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap& request_headers, | ||
| const Http::HeaderMap& response_headers, | ||
| const Http::HeaderMap& response_trailers) { | ||
| bool result = false; | ||
| for (auto& filter : filters_) { | ||
| result |= filter->evaluate(info, request_headers); | ||
| result |= filter->evaluate(info, request_headers, response_headers, response_trailers); | ||
|
|
||
| if (result) { | ||
| break; | ||
|
|
@@ -153,11 +161,12 @@ bool OrFilter::evaluate(const StreamInfo::StreamInfo& info, | |
| return result; | ||
| } | ||
|
|
||
| bool AndFilter::evaluate(const StreamInfo::StreamInfo& info, | ||
| const Http::HeaderMap& request_headers) { | ||
| bool AndFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap& request_headers, | ||
| const Http::HeaderMap& response_headers, | ||
| const Http::HeaderMap& response_trailers) { | ||
| bool result = true; | ||
| for (auto& filter : filters_) { | ||
| result &= filter->evaluate(info, request_headers); | ||
| result &= filter->evaluate(info, request_headers, response_headers, response_trailers); | ||
|
|
||
| if (!result) { | ||
| break; | ||
|
|
@@ -167,15 +176,17 @@ bool AndFilter::evaluate(const StreamInfo::StreamInfo& info, | |
| return result; | ||
| } | ||
|
|
||
| bool NotHealthCheckFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&) { | ||
| bool NotHealthCheckFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| return !info.healthCheck(); | ||
| } | ||
|
|
||
| HeaderFilter::HeaderFilter(const envoy::config::filter::accesslog::v2::HeaderFilter& config) { | ||
| header_data_.push_back(Http::HeaderUtility::HeaderData(config.header())); | ||
| } | ||
|
|
||
| bool HeaderFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMap& request_headers) { | ||
| bool HeaderFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMap& request_headers, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| return Http::HeaderUtility::matchHeaders(request_headers, header_data_); | ||
| } | ||
|
|
||
|
|
@@ -190,13 +201,60 @@ ResponseFlagFilter::ResponseFlagFilter( | |
| } | ||
| } | ||
|
|
||
| bool ResponseFlagFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&) { | ||
| bool ResponseFlagFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&, | ||
| const Http::HeaderMap&, const Http::HeaderMap&) { | ||
| if (configured_flags_ != 0) { | ||
| return info.intersectResponseFlags(configured_flags_); | ||
| } | ||
| return info.hasAnyResponseFlag(); | ||
| } | ||
|
|
||
| GrpcStatusFilter::GrpcStatusFilter( | ||
| const envoy::config::filter::accesslog::v2::GrpcStatusFilter& config) { | ||
| for (int i = 0; i < config.statuses_size(); i++) { | ||
| statuses_.insert(protoToGrpcStatus(config.statuses(i))); | ||
| } | ||
|
|
||
| exclude_ = config.exclude(); | ||
| } | ||
|
|
||
| bool GrpcStatusFilter::evaluate(const StreamInfo::StreamInfo& info, const Http::HeaderMap&, | ||
| const Http::HeaderMap& response_headers, | ||
| const Http::HeaderMap& response_trailers) { | ||
| // The gRPC specification does not guarantee a gRPC status code will be returned from a gRPC | ||
| // request. When it is returned, it will be in the response trailers. With that said, Envoy will | ||
| // treat a trailers-only response as a headers-only response, so we have to check the following | ||
| // in order: | ||
| // 1. response_trailers gRPC status, if it exists. | ||
| // 2. response_headers gRPC status, if it exists. | ||
| // 3. Inferred from info HTTP status, if it exists. | ||
| // | ||
| // If none of those options exist, it will default to Grpc::Status::GrpcStatus::Unknown. | ||
| const std::array<absl::optional<Grpc::Status::GrpcStatus>, 3> optional_statuses = {{ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't repro compile errors locally, but it looked like it was expecting braces around each individual object in the array. To do so I needed to add the double brace around the entire initializer list. Let me know if there's a better way to do it. |
||
| {Grpc::Common::getGrpcStatus(response_trailers)}, | ||
| {Grpc::Common::getGrpcStatus(response_headers)}, | ||
| {info.responseCode() ? absl::optional<Grpc::Status::GrpcStatus>( | ||
| Grpc::Utility::httpToGrpcStatus(info.responseCode().value())) | ||
| : absl::nullopt}, | ||
| }}; | ||
|
|
||
| Grpc::Status::GrpcStatus status = Grpc::Status::GrpcStatus::Unknown; | ||
| for (const auto& optional_status : optional_statuses) { | ||
| if (optional_status.has_value()) { | ||
| status = optional_status.value(); | ||
crockeo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| break; | ||
| } | ||
| } | ||
|
|
||
| const bool found = statuses_.find(status) != statuses_.end(); | ||
| return exclude_ ? !found : found; | ||
| } | ||
|
|
||
| Grpc::Status::GrpcStatus GrpcStatusFilter::protoToGrpcStatus( | ||
| envoy::config::filter::accesslog::v2::GrpcStatusFilter_Status status) const { | ||
| return static_cast<Grpc::Status::GrpcStatus>(status); | ||
| } | ||
|
|
||
| InstanceSharedPtr | ||
| AccessLogFactory::fromProto(const envoy::config::filter::accesslog::v2::AccessLog& config, | ||
| Server::Configuration::FactoryContext& context) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.