access_log: support for dynamic metadata set as part of the request info#590
Merged
htuch merged 9 commits intoenvoyproxy:masterfrom Apr 9, 2018
Merged
access_log: support for dynamic metadata set as part of the request info#590htuch merged 9 commits intoenvoyproxy:masterfrom
htuch merged 9 commits intoenvoyproxy:masterfrom
Conversation
documentation for PR: envoyproxy/envoy#2895 Signed-off-by: Guy Lichtman <guy@lichtman.io>
documentation for PR: envoyproxy/envoy#2895 Signed-off-by: Guy Lichtman <guy@lichtman.io>
rodaine
reviewed
Mar 29, 2018
| * %DYNAMIC_METADATA(com.test.my_filter:test_key)% will log: ``"foo"`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter:test_object)% will log: ``{"inner_key": "bar"}`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key)% will log: ``"bar"`` | ||
|
|
Member
There was a problem hiding this comment.
Please add an example for an unknown key logging -.
Member
mattklein123
left a comment
There was a problem hiding this comment.
LGTM other than small comments. Thank you!
docs/root/intro/version_history.rst
Outdated
| * tracing: the sampling decision is now delegated to the tracers, allowing the tracer to decide when and if | ||
| to use it. For example, if the :ref:`x-b3-sampled <config_http_conn_man_headers_x-b3-sampled>` header | ||
| is supplied with the client request, its value will override any sampling decision made by the Envoy proxy. | ||
| * access log: added DYNAMIC_METADATA :ref:`access log formatter <config_access_log_format>`. |
documentation for PR: envoyproxy/envoy#2895 Signed-off-by: Guy Lichtman <guy@lichtman.io>
Signed-off-by: Guy Lichtman <guy@lichtman.io>
htuch
suggested changes
Apr 2, 2018
| where NAMESPACE is the the filter namespace used when setting the metadata, KEY is an optional | ||
| lookup up key in the namespace with the option of specifying nested keys separated by ':', | ||
| and Z is an optional parameter denoting string truncation up to Z characters long. Dynamic Metadata | ||
| can be set by filters using the RequestInfo api: *setDynamicMetadata*. The data will be logged |
Member
There was a problem hiding this comment.
Nit: s/api/API/
Add for RequestInfo with a :repo link, same as well for setDynamicMetadata.
Contributor
Author
There was a problem hiding this comment.
Added RequestInfo link. Will add a link to setDynamicMetadata once the code is merged.
| lookup up key in the namespace with the option of specifying nested keys separated by ':', | ||
| and Z is an optional parameter denoting string truncation up to Z characters long. Dynamic Metadata | ||
| can be set by filters using the RequestInfo api: *setDynamicMetadata*. The data will be logged | ||
| as a json string. For example, for the following dynamic metadata: |
| * %DYNAMIC_METADATA(com.test.my_filter:test_object)% will log: ``{"inner_key": "bar"}`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key)% will log: ``"bar"`` | ||
| * %DYNAMIC_METADATA(com.unknown_filter)% will log: ``-`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter:unknown_key)% will log: ``-`` |
Member
There was a problem hiding this comment.
Can you add an example with truncation?
Contributor
Author
There was a problem hiding this comment.
Added an example in latest commit
…text fixes Signed-off-by: Guy Lichtman <guy@lichtman.io>
Signed-off-by: Guy Lichtman <guy@lichtman.io>
ccaraman
reviewed
Apr 4, 2018
| * %DYNAMIC_METADATA(com.unknown_filter)% will log: ``-`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter:unknown_key)% will log: ``-`` | ||
| * %DYNAMIC_METADATA(com.test.my_filter):25% will log (truncation at 25 characters): ``{"test_key": "foo", "test`` | ||
|
|
Contributor
There was a problem hiding this comment.
Can you add a snippet for TCP this isn't implemented?
Contributor
Author
There was a problem hiding this comment.
I can add a note that for tcp it is not implemented. Currently tcp_proxy doesn't populate dynamic metadata, but it could use this in the future if desired.. The dynamic metadata object is part of request_info which is used by tcp_proxy.
Signed-off-by: Guy Lichtman <guy@lichtman.io>
Signed-off-by: Guy Lichtman <guy@lichtman.io>
mattklein123
reviewed
Apr 8, 2018
Member
mattklein123
left a comment
There was a problem hiding this comment.
Change looks good, but also looks like somehow the file permission masks got changed (100644 → 100755). Can you take a look at that and revert?
Signed-off-by: Guy Lichtman <guy@lichtman.io>
htuch
pushed a commit
to envoyproxy/envoy
that referenced
this pull request
Apr 9, 2018
…nfo (#2895) Relates to issue #1278. Added new dynamic Metadata object to request_info (accessed via request_info.dynamicMetadata()). Dynamic Metadata can be set by filters and consumed by either other filters or the access log. Added to the access_log an option to specify logging of dynamic metadata through the following access log format token: %DYNAMIC_METADATA(filter_namespace:key*):Z%. "Z" is an optional parameter denoting max length. It is possible to have a nested key path. Each key is separated by ':'. For example for the following dynamic metadata: com.test.my_filter: {"test_key": "foo", "test_object": {"inner_key": "bar"}} %DYNAMIC_METADATA(com.test.my_filter)% will log: {"test_key": "foo", "test_object": {"inner_key": "bar"}} %DYNAMIC_METADATA(com.test.my_filter:test_key)% will log: "foo" %DYNAMIC_METADATA(com.test.my_filter:test_object)% will log: {"inner_key": "bar"} %DYNAMIC_METADATA(com.test.my_filter:test_object:inner_key)% will log: "bar" Risk Level: Low Testing: unit tests added to access_log_formatter_test and request_info_impl_test. I've done manual testing with a custom "body parser" filter which parses the request body and stores the parsed result in the dynamic metadata of request_info. If this is of interest, I am ok with contributing this filter too. Docs Changes: envoyproxy/data-plane-api#590 Signed-off-by: Guy Lichtman <guy@lichtman.io>
htuch
approved these changes
Apr 9, 2018
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
documentation for PR: envoyproxy/envoy#2895
Signed-off-by: Guy Lichtman guy@lichtman.io