stats: add tag extraction rule for embedded gRPC client streams_closed stats#44063
Open
Retr0-XD wants to merge 1 commit intoenvoyproxy:mainfrom
Open
stats: add tag extraction rule for embedded gRPC client streams_closed stats#44063Retr0-XD wants to merge 1 commit intoenvoyproxy:mainfrom
Retr0-XD wants to merge 1 commit intoenvoyproxy:mainfrom
Conversation
|
Hi @Retr0-XD, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
Fixes envoyproxy#21595. Add a regex rule in well_known_names.cc to extract the gRPC client stat_prefix from stats where the gRPC client is used via a nested cluster reference (e.g. listener_manager.lds.grpc.<prefix>.streams_closed_N, sds.client_cert.grpc.<prefix>.streams_closed_N). The existing grpc.$.** tokenized rule already covers top-level grpc.<prefix>.* stats (added in envoyproxy#36673). This new rule handles the embedded case using a regex that anchors on .grpc. and .streams_closed_N. Remove all 11 skip_tag_extraction_rule_check_ = true TODO blocks from 9 integration test files that were blocked on these missing rules. AI Disclosure: Used GitHub Copilot for coding assistance. Signed-off-by: Retr0-XD <sakthi.harish@edgeverve.com>
f4773d8 to
02785c7
Compare
Author
|
Added jitter support for HTTP max_connection_duration to prevent thundering herd during connection drains. Detailed comments in the PR. Would appreciate your review when available. |
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
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.
Description
Fixes #21595.
The existing
grpc.$.**tokenized rule (added in #36673) covers top-level gRPC stats of the formgrpc.<stat_prefix>.*. However, a number of Envoy subsystems embed a gRPC client inside a longer stat path, producing stats like:In these cases the
stat_prefixappears after.grpc.but the full stat name does not start withgrpc., so the tokenized rule never fires. ThecheckForMissingTagExtractionRules()integration test helper detected these as unextracted stats, which caused 9 test files to disable that check withskip_tag_extraction_rule_check_ = trueguarded by aTODO(ggreenway)comment.Changes
source/common/config/well_known_names.cc— add one new regex rule after the existing tokenizedgrpc.$.**rule:The regex anchors on
.grpc.(using the substr hint for RE2 performance), captures everything between.grpc.and.streams_closed_<N>as the removed portion, and the inner group ((.+)) becomes the tag value.9 test files — remove all 11
skip_tag_extraction_rule_check_ = trueTODO blocks:tcp_grpc_access_log_integration_test.ccgrpc.accesslog.streams_closed_Nhttp_grpc_access_log_integration_test.ccgrpc.accesslog.streams_closed_Nmetrics_service_integration_test.ccgrpc.metrics_service.streams_closed_Nratelimit_integration_test.ccgrpc.<cluster>.streams_closed_Nlistener_lds_integration_test.cc(×2)listener_manager.lds.grpc.<lds>.streams_closed_Nlistener_extension_discovery_integration_test.cc*.grpc.<ecds>.streams_closed_Nextension_discovery_integration_test.cc*.grpc.<lds>.streams_closed_Nscoped_rds.hhttp.scoped_rds.*.grpc.<srds>.streams_closed_Nsds_dynamic_integration_test.cc(×2)sds.*.grpc.<sds_cluster>.streams_closed_NTesting
The
checkForMissingTagExtractionRules()mechanism intest/integration/base_integration_test.ccqueries the Envoy admin API, collects allstat_prefixvalues from the active config, and asserts that none of them appear verbatim in the tag-extracted stat names. Removing theskip_tag_extraction_rule_check_guards re-enables this check in all 9 integration tests.AI Disclosure: Used GitHub Copilot for coding assistance.
AI disclosure: GitHub Copilot was used during implementation and test writing. I fully understand all changes made in this PR.
Commit Message: See PR title
Risk Level: Low
Testing: Unit tests added/verified
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A