ext_proc: propagate detailed GRPC status for fail open#40808
Merged
botengyao merged 11 commits intoenvoyproxy:mainfrom Aug 28, 2025
Merged
ext_proc: propagate detailed GRPC status for fail open#40808botengyao merged 11 commits intoenvoyproxy:mainfrom
botengyao merged 11 commits intoenvoyproxy:mainfrom
Conversation
melginaldi
requested review from
mattklein123,
tyxia and
yanavlasov
as code owners
adisuissa
reviewed
Aug 20, 2025
Contributor
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks!
Overall LGTM.
Please add a release note (here].
Assigning @yanjunxiang-google as code-owner.
/assign @yanjunxiang-google
melginaldi
requested review from
KBaichoo,
RyanTheOptimist,
abeyad,
agrawroh,
danzh2010,
fredyw,
ggreenway and
phlax
as code owners
melginaldi
force-pushed
the
extproc-fail-open-stats
branch
from
3b68ca7 to
2580a12
Compare
adisuissa
removed request for
KBaichoo,
RyanTheOptimist,
abeyad,
agrawroh,
danzh2010,
fredyw,
ggreenway,
mattklein123,
phlax,
tyxia and
yanavlasov
Contributor
Author
Done. Release note is added |
Contributor
|
Thanks for working on this. Please fix the DCO and other CI errors |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
melginaldi
force-pushed
the
extproc-fail-open-stats
branch
from
b851831 to
d01a0be
Compare
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
melginaldi
force-pushed
the
extproc-fail-open-stats
branch
from
d01a0be to
55d351f
Compare
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…tats Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…tats Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Contributor
|
/retest |
Contributor
Author
I fixed the DCO error. The CI error is unrelated to my PR and its changes. I think #40860 is trying to fix the Envoy/Checks error |
…tats Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
Contributor
Author
|
The Envy checks are now passing. This PR is ready for review |
Contributor
|
LGTM |
yanjunxiang-google
approved these changes
Aug 27, 2025
Contributor
Author
|
/assign @yanavlasov |
adisuissa
reviewed
Aug 28, 2025
Contributor
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Small nit, but otherwise LGTM.
Thanks!
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
…tats Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
botengyao
approved these changes
Aug 28, 2025
| } | ||
|
|
||
| void Filter::onGrpcClose() { | ||
| void Filter::onGrpcClose() { onGrpcCloseWithStatus(Grpc::Status::Aborted); } |
Member
There was a problem hiding this comment.
I was thinking to also expend the status to onGrpcClose method, but this solution is better. Thanks for adding this support.
mmcloud
pushed a commit
to mmcloud/envoy
that referenced
this pull request
Aug 29, 2025
) ext_proc: propagate detailed GRPC status for fail open Commit Message: Propagate detailed GRPC status for fail open rather than always ABORT. In order to have a better understanding of what is causing a request to fail we should log the GRPC status even if fail open is being used. Additional Description: For backwards compatibility this CL uses optional parameters for call_status with a default value of Grpc::Status::Aborted. Risk Level: Low Testing: Added a unit test within ext_proc_integration_test.cc to validate the logged call_status changed accordingly. I did some additional unit testing within the Google codebase for products that use ext_proc. Docs Changes: N/A Release Notes: If failure_mode_allowed is true, save the grpc failure status code returned from the ext_proc server in the filter state. Previously all fail-open cases would return call_status Grpc::Status::Aborted. Platform Specific Features: N/A Signed-off-by: Melissa Ginaldi <meginaldi@google.com> --------- Signed-off-by: Melissa Ginaldi <mginaldi@google.com> Signed-off-by: Matthew McLeod <mail@matthewmcleod.co.uk>
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.
ext_proc: propagate detailed GRPC status for fail open
Commit Message: Propagate detailed GRPC status for fail open rather than always ABORT. In order to have a better understanding of what is causing a request to fail we should log the GRPC status even if fail open is being used.
Additional Description: For backwards compatibility this CL uses optional parameters for call_status with a default value of Grpc::Status::Aborted.
Risk Level: Low
Testing: Added a unit test within ext_proc_integration_test.cc to validate the logged call_status changed accordingly. I did some additional unit testing within the Google codebase for products that use ext_proc.
Docs Changes: N/A
Release Notes: If failure_mode_allowed is true, save the grpc failure status code returned from the ext_proc server in the filter state. Previously all fail-open cases would return call_status Grpc::Status::Aborted.
Platform Specific Features: N/A
Signed-off-by: Melissa Ginaldi meginaldi@google.com