ext proc: fix upstream extproc grpc client stats scope - #47409
Merged
yanjunxiang-google merged 3 commits intoSep 15, 2026
Merged
Conversation
Signed-off-by: wbpcode <wbphub@gmail.com>
Signed-off-by: wbpcode <wbphub@gmail.com>
wbpcode
requested review from
mattklein123,
tyxia,
yanavlasov and
yanjunxiang-google
as code owners
September 13, 2026 15:35
Member
Author
|
I personally think it's unnecessary to add a runtime flag tor this minor behavior change/bug fix. But will leave this to @yanjunxiang-google to take a check. |
Contributor
There was a problem hiding this comment.
Is this comment still relevant as the scope is not passed in the lamda any more?
Contributor
There was a problem hiding this comment.
Should this be changed into: Stats::Scope& scope = extra_context.scopeOr(context);
| @@ -0,0 +1,5 @@ | |||
| Fixed the stats scope that is used to create the gRPC client of the external processing | |||
Contributor
There was a problem hiding this comment.
This seems to be a minor behavior change. Please move the release log to the minor_behavior_change section.
| scope](Http::FilterChainFactoryCallbacks& callbacks) { | ||
| auto client = | ||
| createExternalProcessorClient(context.clusterManager().grpcAsyncClientManager(), *scope); | ||
| return [filter_config = std::move(filter_config), |
Contributor
There was a problem hiding this comment.
Please add a unit test in config_test.cc to verify the change.
Contributor
|
Thanks for fixing this! LGTM overall. Adding some minor comments. |
Signed-off-by: wbpcode <wbphub@gmail.com>
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
2 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.
Commit Message: ext proc: fix upstream extproc grpc client stats scope
Additional Description:
For the upstream ext_proc filter, the upstream cluster's stats scope will be used to create the google grpc client's stats scope.
But note the upstream cluster's stats scope has a
cluster.<cluster_name>prefix. That result in the google grpc client's stats scope will also contains an unexpectedcluster.<cluster_name>prefix.It's unexpected because the grpc client is shared across listeners and clusters in a central cache. This unexpected prefix will make it's hard to determine the grpc client's stats.
This PR fix it by always propagating the server scope to the grpc client.
The fix will not affect the downstream ext_proc. But for the upstream ext_proc, there would be a very minor behavior change to the google grpc client's stats prefix.
Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: added.
Platform Specific Features: n/a.