stats: consolidate iteration via IterationCriteria#44296
Open
sxl613 wants to merge 1 commit intoenvoyproxy:mainfrom
Open
stats: consolidate iteration via IterationCriteria#44296sxl613 wants to merge 1 commit intoenvoyproxy:mainfrom
sxl613 wants to merge 1 commit intoenvoyproxy:mainfrom
Conversation
Ref: envoyproxy#44156 Reduces 8 virtual forEach methods to 4 by introducing IterationCriteria, a struct that carries iteration filters (currently sinked_only). This eliminates duplication between forEachCounter/forEachSinkedCounter and equivalents for Gauge, TextReadout, and Histogram. Signed-off-by: Szczepan Lukowski <szczepan.lukowski@gmail.com>
sxl613
had a problem deploying
to
external-contributors
GitHub Actions
Error
|
Hi @sxl613, 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. |
Contributor
|
Hi -- sorry you wasted some tokens generating this. This is actually going to massively conflict with my in-progress #44285 so I'd prefer not to pursue reviewing this PR. |
Member
|
/wait-any |
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
3 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.
Ref: #44156
Store previously had 8 virtual forEach methods:
forEachCounter,forEachGauge,forEachTextReadout,forEachHistogram, and a parallelforEachSinked*family for each.The sinked variants were functionally identical to the non-sinked variants except for a filter.
This PR collapses them into 4 methods by introducing an
IterationCriteriastruct.A struct over a naked bool since it reads clearly (with the help of designated initializers) and allows for more filter fields in the future. #44156 mentions one for
used_only, I've not added that one since I felt like a smaller change would be best to start with.There were three differences between the sinked and non-sinked variants in
Allocatorthat I preserved in the merged implementation:f_sizenullptr checks in the sinked variants; the merged implementation adds them in both caseshidden()filter applied when iterating sinked stats with no sink predicatessink_predicates_is not setRisk Level: Low
Testing: adapted existing tests only
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
Claude Code was used to help navigate the codebase and understand the existing implementation prior to making changes.