-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
http conn man: stopped filters can add data from encodeTrailers #8404
http conn man: stopped filters can add data from encodeTrailers #8404
Conversation
Now filters can return StopIteration from encodeHeaders, and then call addEncodedData from encodeTrailers. This allows the JsonTranscoderFilter to properly transcode gRPC status in trailer headers into a JSON reply body. Risk Level: Low Testing: unit-tests, integration tests in gRPC JSON transcoder filter. Documentation: n/a Release notes: n/a Fixes envoyproxy#8402 Signed-off-by: Anatoly Scheglov <ascheglov@yandex-team.ru>
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.
Thanks for this change and all of the tests. This makes sense to me. Can you make a parallel change in the decode path as well? Thank you!
/wait
Signed-off-by: Anatoly Scheglov <ascheglov@yandex-team.ru>
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.
Thanks looks great. One additional test request. Thank you!
/wait
EXPECT_CALL(*decoder_filters_[0], decodeTrailers(_)) | ||
.WillOnce(InvokeWithoutArgs([&]() -> FilterTrailersStatus { | ||
decoder_filters_[0]->callbacks_->addDecodedData(trailers_data, false); | ||
return FilterTrailersStatus::Continue; |
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.
Can you add a similar parallel test in which the filter decode/encodeTrailers() callbacks continue to stop iteration and then do a continuation later outside of callback context?
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.
done
Signed-off-by: Anatoly Scheglov <ascheglov@yandex-team.ru>
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.
Awesome work, thank you!
…yproxy#8404) Now filters can return StopIteration from encodeHeaders, and then call addEncodedData from encodeTrailers. This allows the JsonTranscoderFilter to properly transcode gRPC status in trailer headers into a JSON reply body. Risk Level: Low Testing: unit-tests, integration tests in gRPC JSON transcoder filter. Documentation: n/a Release notes: n/a Fixes envoyproxy#8402 Signed-off-by: Anatoly Scheglov <ascheglov@yandex-team.ru>
Now filters can return StopIteration from encodeHeaders,
and then call addEncodedData from encodeTrailers.
This allows the JsonTranscoderFilter to properly transcode gRPC status
in trailer headers into a JSON reply body.
Risk Level: Low
Testing: manual testing, unit-tests, integration tests in gRPC JSON transcoder filter.
Documentation: n/a
Release notes: n/a
Fixes #8402