keep_matching in OnMatch to skip matched Matchers#117
Merged
Conversation
…it should not enforce the matcher's action and instead continue looking for another match if possible Signed-off-by: Brian Surber <[email protected]>
Signed-off-by: Brian Surber <[email protected]>
bsurber
force-pushed
the
keep_matching-in-xds
branch
from
March 13, 2025 16:01
2e66edd to
42edf51
Compare
Contributor
|
This LGTM, but I'll hold off on merging until we get consensus on envoyproxy/envoy#38726. |
Contributor
Author
Now that we have consensus on the desired behavior, are we clear to submit this? It'll help clean up the CI + dependency management in the dependent PR. |
Contributor
markdroth
approved these changes
May 1, 2025
Contributor
|
Please fix DCO. |
…t yet been implemented. Signed-off-by: Brian Surber <[email protected]>
bsurber
force-pushed
the
keep_matching-in-xds
branch
from
May 1, 2025 22:55
6ffc1e2 to
a43e4bf
Compare
ravenblackx
added a commit
to envoyproxy/envoy
that referenced
this pull request
May 28, 2025
…38726) Commit Message: - Add a `keep_matching` field to OnMatch to mark Matchers that should be recorded when matched but should not be the final matched + enforced action. - Implement support for `keep_matching` using MatcherTree re-entry. - Re-entry is implemented in PR #38564 as well for a separate review if this PR gets closed. - ListMatcher was chosen as the simplest example usecase / proof-of-concept for re-entry, but implementation in other MatcherTree children will follow in separate PRs (namely prefix_map_matcher). - Centralize recursion handling & re-entry logic into `evaluateMatch(...)`, leaving each MatchTree child class to implement the simplest possible `match(...)` logic (find a match and return it + optionally a reentrant). - Previously recursion handling was inconsistent across the MatchTree children, and this saves each child from having to re-implement recursion, `keep_handling` processing, etc. - TrieMatcher also required a reentrant implementation as part of moving recursion logic out of its `match(...)` logic. Additional Description: - The final goal is to implement preview / darklaunch Matchers whose actions are logged or added to metadata, but not enforced. - An initial implementation of preview matched actions just using re-entry was drafted for the rate_limit_quota filter in PR #38576, prompting the request to instead add this `keep_matching` feature to the Matcher API for re-usability. - This change will also depend on a duplicate, matcher proto change in [github.com/cnf/xds](cncf/xds#117). Risk Level: moderate - includes changes to widely-used libraries Testing: Unit + Integration testing, loadtesting WIP Docs Changes: Release Notes: Platform Specific Features: ~[Optional Runtime guard:]~ ~[Optional Fixes #Issue]~ ~[Optional Fixes commit #PR or SHA]~ ~[Optional Deprecated:]~ [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md): The new `keep_matching` field is not marked as WIP as its implementation should be tested for any breaking changes. --------- Signed-off-by: Brian Surber <[email protected]> Signed-off-by: Raven Black <[email protected]> Co-authored-by: Raven Black <[email protected]>
update-envoy Bot
added a commit
to envoyproxy/data-plane-api
that referenced
this pull request
Jul 2, 2025
…38726) Commit Message: - Add a `keep_matching` field to OnMatch to mark Matchers that should be recorded when matched but should not be the final matched + enforced action. - Implement support for `keep_matching` using MatcherTree re-entry. - Re-entry is implemented in PR #38564 as well for a separate review if this PR gets closed. - ListMatcher was chosen as the simplest example usecase / proof-of-concept for re-entry, but implementation in other MatcherTree children will follow in separate PRs (namely prefix_map_matcher). - Centralize recursion handling & re-entry logic into `evaluateMatch(...)`, leaving each MatchTree child class to implement the simplest possible `match(...)` logic (find a match and return it + optionally a reentrant). - Previously recursion handling was inconsistent across the MatchTree children, and this saves each child from having to re-implement recursion, `keep_handling` processing, etc. - TrieMatcher also required a reentrant implementation as part of moving recursion logic out of its `match(...)` logic. Additional Description: - The final goal is to implement preview / darklaunch Matchers whose actions are logged or added to metadata, but not enforced. - An initial implementation of preview matched actions just using re-entry was drafted for the rate_limit_quota filter in PR #38576, prompting the request to instead add this `keep_matching` feature to the Matcher API for re-usability. - This change will also depend on a duplicate, matcher proto change in [github.com/cnf/xds](cncf/xds#117). Risk Level: moderate - includes changes to widely-used libraries Testing: Unit + Integration testing, loadtesting WIP Docs Changes: Release Notes: Platform Specific Features: ~[Optional Runtime guard:]~ ~[Optional Fixes #Issue]~ ~[Optional Fixes commit #PR or SHA]~ ~[Optional Deprecated:]~ [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md): The new `keep_matching` field is not marked as WIP as its implementation should be tested for any breaking changes. --------- Signed-off-by: Brian Surber <[email protected]> Signed-off-by: Raven Black <[email protected]> Co-authored-by: Raven Black <[email protected]> Mirrored from https://github.com/envoyproxy/envoy @ 426cd861187368163b42fce910ab5828f7f0b392
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.
Add a keep_matching field to OnMatch which indicates to callers that it should not enforce the matcher's action and instead continue looking for another match, if possible.
If a match continuation isn't possible or does not find a further match, on_no_match action will be used.
CC @markdroth