-
Notifications
You must be signed in to change notification settings - Fork 5.5k
access log: add response flag filter #3536
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
Changes from 18 commits
1176b61
d379e04
05eb710
4cb95e0
0b4b631
d02b862
53697d5
e5ad2c8
4bb4afb
c421d95
aa5b20d
1c99b8a
bc18e7b
53662e8
3421855
64803c0
9ebe110
08bb66e
779af08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ Version history | |
|
|
||
| 1.8.0 (Pending) | ||
| =============== | ||
| * access log: added :ref:`response flag filter <envoy_api_msg_config.filter.accesslog.v2.ResponseFlagFilter>` to filter based on the presence of Envoy response flags. | ||
| to filter logs based on request headers. | ||
|
Member
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. duplicated text / please break on 100 chars |
||
| * admin: added :http:get:`/hystrix_event_stream` as an endpoint for monitoring envoy's statistics | ||
| through `Hystrix dashboard <https://github.com/Netflix-Skunkworks/hystrix-dashboard/wiki>`_. | ||
| * http: response filters not applied to early error paths such as http_parser generated 400s. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 hasResponseFlag() const PURE; | ||
|
Member
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. IMO this function name is a little strange given the other overload of same name Can we call it |
||
|
|
||
| /** | ||
| * @return upstream host description. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,12 @@ | |
|
|
||
| #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" | ||
|
|
||
| #include "common/http/header_utility.h" | ||
| #include "common/protobuf/protobuf.h" | ||
| #include "common/request_info/request_info_impl.h" | ||
|
Member
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. This header change can be reverted I believe here and above. |
||
|
|
||
| namespace Envoy { | ||
| namespace AccessLog { | ||
|
|
@@ -166,6 +166,21 @@ class HeaderFilter : public Filter { | |
| std::vector<Http::HeaderUtility::HeaderData> 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. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively I thought about adding an option to specify the flags you care about. But I think that response flags are important, and infrequent enough to have the filter filter on all of them. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way to achieve both settings is have an array field that will specify which response flags to log on and if that field is empty default to all response flags? (or have an enum that says all)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah +1 to what @ccaraman said. I would do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that is what I thought, will add.