tracing: envoy.reloadable_features.update_grpc_response_error_tag deprecation#23098
tracing: envoy.reloadable_features.update_grpc_response_error_tag deprecation#23098adisuissa merged 2 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Jiayu Wu <jiayu1.wu@intel.com>
Signed-off-by: Jiayu Wu <jiayu1.wu@intel.com>
|
/assign-from @envoyproxy/first-pass-reviewers |
|
@envoyproxy/first-pass-reviewers assignee is @tonya11en |
| envoy_package() | ||
|
|
||
| envoy_cc_test( | ||
| name = "http_tracer_impl_legacy_test", |
There was a problem hiding this comment.
What is the difference between http_tracer_impl_test and http_tracer_impl_legacy_test?
There was a problem hiding this comment.
runtime feature is enabled in http_tracer_impl_test while is not enabled in http_tracer_impl_legacy_test. Since runtime guard is now removed, I think we should also remove the legacy test.
| // Set error tag when Grpc status code represents an upstream error. See | ||
| // https://github.com/envoyproxy/envoy/issues/18877. | ||
| absl::optional<Grpc::Status::GrpcStatus> grpc_status_code = Grpc::Common::getGrpcStatus(headers); | ||
| if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.update_grpc_response_error_tag")) { |
There was a problem hiding this comment.
The old logic is that
if feature
if
else
Now we have enabled the feature, still need the else on the top level?
There was a problem hiding this comment.
I think we don't need else any more, because the feature is always enabled.
There was a problem hiding this comment.
True. The logic in else is actually the old code path which we don't need anymore.
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
LGTM, thanks for cleaning this!
6 participants
#20090 introduced a runtime guarded feature. It has been 6 months since the new code has been exercised by default, so it's time to remove the old code path.
Signed-off-by: Jiayu Wu jiayu1.wu@intel.com
Commit Message:
Additional Description:
Risk Level: low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]