formatter: removed legacy router formatter support#41039
formatter: removed legacy router formatter support#41039wbpcode merged 7 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: WangBaiping <wbphub@gmail.com>
|
/retest |
Signed-off-by: WangBaiping <wbphub@gmail.com>
Signed-off-by: WangBaiping <wbphub@gmail.com>
zuercher
left a comment
There was a problem hiding this comment.
Some minor things, but generally looks good.
| return Envoy::Formatter::FormatterImpl::create(final_header_value, true); | ||
| } | ||
|
|
||
| // Let the substitution formatter parse the final_header_value. |
There was a problem hiding this comment.
nit: just header_value in the comment.
| {"PER_REQUEST_STATE", | ||
| {CommandSyntaxChecker::PARAMS_REQUIRED, | ||
| [](absl::string_view format, absl::optional<size_t>) { | ||
| // PER_REQUEST_STATE(KEY) == FILTER_STATE(KEY:PLAIN) | ||
| // Mainly for backward compatibility. | ||
| return std::make_unique<FilterStateFormatter>(format, absl::nullopt, true); | ||
| }}}, |
There was a problem hiding this comment.
Since we log it was obsoleted. should this one be gated on a feature flag as well?
There was a problem hiding this comment.
I am a little not sure about this, actually. As you can see, to keep the PER_REQUEST_STATE is pretty cheap. It's a little different with the DYNAMIC_METADATA because the legacy DYNAMIC_METADATA has completely different format parsing logic.
So, I inclined to keep this (as a special undocumented support) and only deprecate/remove the legacy DYNAMIC_METADATA as the change log.
WDYT?
There was a problem hiding this comment.
I think we should eventually remove it, but I'm ok if you don't want to do it right now.
There was a problem hiding this comment.
then thing could be simpler. I will refactor the PR tomorrow
Signed-off-by: WangBaiping <wbphub@gmail.com>
Signed-off-by: WangBaiping <wbphub@gmail.com>
|
/retest |
|
friendly ping a new review cc @zuercher |
Commit Message: formatter: removed legacy router formatter support
Additional Description:
Three yeas ago, at #21932, we unified all the formatters to the substitution formatter. And we add a warn log for the legacy UPSTREAM_METADATA and DYNAMIC_METADATA.
Now, I think it's time to remove it finally. This PR Removed legacy header formatter support for
%DYNAMIC_METADATA(["namespace", "key", ...])%and%UPSTREAM_METADATA(["namespace", "key", ...])%. Please use%DYNAMIC_METADATA(namespace:key:...])%and%UPSTREAM_METADATA(namespace:key:...])%as alternatives.This change can be reverted temporarily by setting the runtime guard
envoy.reloadable_features.remove_legacy_route_formattertofalse.Risk Level: low.
Testing: unit.
Docs Changes: n/a.
Release Notes: added.