thrift_proxy: Add thrift header to metadata filter#18637
thrift_proxy: Add thrift header to metadata filter#18637mattklein123 merged 18 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: James Fish <jfish@pinterest.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
|
cc @rgs1 /wait-any |
rgs1
left a comment
rgs1
left a comment
There was a problem hiding this comment.
LGTM -- thanks!
cc: @envoyproxy/api-shepherds
|
Do you want to just add the filter at the same time? This seems fine to me. /wait-any |
Will do it later in the week as need a couple of changes from our internal filter. |
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: James Fish <jfish@pinterest.com>
fishcakez
changed the title
fishcakez
changed the title
Signed-off-by: James Fish <jfish@pinterest.com>
Signed-off-by: James Fish <jfish@pinterest.com>
|
looks like need add couple tests for two errors cases to get coverage up to scratch |
|
ASAN failure is due to publishing test results timeout: |
|
@rgs1 to do first pass review, thank you. |
| ConfigSharedPtr filter_config(std::make_shared<Config>(proto_config)); | ||
| return | ||
| [filter_config](ThriftProxy::ThriftFilters::FilterChainFactoryCallbacks& callbacks) -> void { | ||
| callbacks.addDecoderFilter(std::make_shared<HeaderToMetadataFilter>(filter_config)); |
There was a problem hiding this comment.
nit: we don't need a new shared ptr on every call, pass a ref to filter config?
...ensions/filters/network/thrift_proxy/filters/header_to_metadata/header_to_metadata_filter.cc
Show resolved
Hide resolved
Signed-off-by: James Fish <jfish@pinterest.com>
rgs1
left a comment
rgs1
left a comment
There was a problem hiding this comment.
LGTM - thanks!
Over to @zuercher or @mattklein123 for merging.
4 participants
Commit Message: thrift_proxy: Add thrift header to metadata filter
Additional Description: Transforming headers to metadata and then using that metadata for subset load balancing, logging or other custom functionality is fundamental part of envoy proxy capabilities. Therefore add header to metadata proto configuration that is exactly like the http equivalent.
Risk Level: low
Testing: N/A
Docs Changes: N/A
Release Notes: new feature
Platform Specific Features: N/A
Fixes #18578: Filter to support this feature.
API Considerations(https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md): Given this is a clone of the http header to metadata config (without cookie support and with the future one of promotion) I believe it should pass the checklist.