Add slice-based filtering with order-aware rule processing#189
Open
goodtune wants to merge 4 commits into
Open
Add slice-based filtering with order-aware rule processing#189goodtune wants to merge 4 commits into
goodtune wants to merge 4 commits into
Conversation
…processing Implement new `--systemd.collector.slice-include` and `--systemd.collector.slice-exclude` flags to filter systemd units by their slice membership. The filtering system uses order-aware rule processing where later rules override earlier ones, allowing flexible combinations of slice and unit filters. Slice filtering uses literal string matching and supports slice names with or without the `.slice` suffix. The implementation includes a thread-safe cache for slice lookups using double-checked locking to minimize dbus calls during metric collection. Add comprehensive unit tests covering order precedence, edge cases, cache functionality, and backward compatibility with existing unit-based filtering. Add comprehensive documentation for the new slice filtering flags in the README, explaining the three filtering modes (include-only, exclude-only, and mixed) and how order-aware rule processing works. Update the CHANGELOG to track this feature addition. Fix incorrect flag name in the Kubernetes daemonset example and add commented examples showing slice-based filtering usage. Signed-off-by: Gary Reynolds <gary@touch.asn.au>
593d275 to
e3323de
Compare
262e10c to
1cb5d4f
Compare
The order-aware filtering system was only parsing slice filters from os.Args, completely ignoring `--systemd.collector.unit-include` and `--systemd.collector.unit-exclude` flags. This caused unit filters to have no effect when combined with slice filters, breaking the expected behavior where later rules should extend or override earlier ones. Add parsing for unit-include and unit-exclude flags in `buildFilterRules()`, compiling them as regex patterns and adding them to the ordered filter rules. Add tests to verify `buildFilterRules()` correctly parses all four filter types and to test the scenario where slice-include captures units from a slice while unit-include extends the capture to additional specific units. Signed-off-by: Gary Reynolds <gary@touch.asn.au>
a675478 to
ed819ff
Compare
Add two new test cases covering order-aware filtering with three rules: - `TestFilterUnits_SliceIncludeUnitIncludeUnitExclude`: Tests slice-include followed by unit-include to extend capture, then unit-exclude to remove a specific unit from the included slice. - `TestFilterUnits_SliceIncludeUnitIncludeSliceExclude`: Tests slice-include followed by broad unit-include pattern, then slice-exclude to override the unit-include for units in a specific slice. These tests verify that later rules correctly override earlier ones when combining slice and unit filters in various orders. Signed-off-by: Gary Reynolds <gary@touch.asn.au>
Change the condition for selecting order-aware vs legacy filtering to check specifically for slice filter rules, not just any filter rules. This prevents a regression where using only `--unit-include` or `--unit-exclude` flags (without slice filters) would incorrectly use the order-aware path, breaking the legacy AND-based filtering behavior. With this fix: - Unit filters only: uses legacy filtering (must match include AND not match exclude) - Slice filters present: uses order-aware filtering (rules processed in CLI order) Signed-off-by: Gary Reynolds <gary@touch.asn.au>
fd93e51 to
034ac72
Compare
Author
|
This PR has been open for a few months now without any review. I’d love to get some feedback on the approach. @SuperQ - as the most active maintainer, would you be able to take a look when you get a chance? This adds slice-based filtering which complements the existing unit filtering and includes tests and docs. @lucacome @jpds - as recent contributors, I’d also welcome your perspective if you have time. The filtering logic touches the collector in a similar area to the code you’ve worked on. |
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.
Implement new
--systemd.collector.slice-includeand--systemd.collector.slice-excludeflags to filter systemd units by their slice membership. The filtering system uses order-aware rule processing where later rules override earlier ones, allowing flexible combinations of slice and unit filters. Slice filtering uses literal string matching and supports slice names with or without the.slicesuffix.The implementation includes a thread-safe cache for slice lookups using double-checked locking to minimize dbus calls during metric collection. Added unit tests covering order precedence, edge cases, cache functionality, and backward compatibility with existing unit-based filtering.
Added documentation for the new slice filtering flags in the
README, explaining the three filtering modes (include-only, exclude-only, and mixed) and how order-aware rule processing works.