filter_grep: respect all rules even when a match is found
Currently the grep filter will keep data when the first positive match for a rule is found, regardless of whether the data would be excluded by another rule later on. An example is given in #3259.
This leads to surprising behaviour when combining Regex rules with one another or when combining Regex rules with Exclude rules:
-
The order of
Regexrules matters, but the order ofExcluderules does not. -
Mixing
RegexandExcluderules will result in theExcluderules being ignored if any of theRegexrules match.
This commit changes the existing behaviour so that inputs must pass all rules.
I've also included new runtime tests for this behaviour.
This is a breaking change for users with multiple Regex rules in a single block that rely on the existing short-circuiting behaviour. However, I think the new behaviour is consistent with the existing documentation, so I'm not sure why anyone would be doing this.
Fixes #3259.
Enter [N/A] in the box, if an item is not applicable to your change.
Testing Before we can approve your change; please submit the following in a comment:
[N/A]Example configuration file for the change[N/A]Debug log output from testing the change[N/A]Attached Valgrind output that shows no leaks or memory corruption was found
Documentation
[N/A]Documentation required for this feature
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
Not stale, still fixes an active issue.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
Still an active issue.
@atheriel Thank you for contribution and sorry for late reply.
Can you add a configuration parameter to select current/this behavior ? I think it is better to be selectable by user.
I prefer the default is current behavior to prevent breaking change. However, this new behavior may be better as a default.. Hmm....
@nokute78 I'm happy to add a configuration value if needed, but I'm not sure I understand your comment:
However, this new behavior may be better as a default.. Hmm....
Do you want this to be the default or not? I'd prefer to avoid implementing the configuration option if it will not be used.
@atheriel In my opinion, the configuration parameter is needed to prevent breaking change, so default is current behavior. On the other hand, this new behavior is proper. I think a gradual transition is good.
- Add configuration parameter and warning message to notify user uses default value and it will be changed in the future.
- In future release (e.g. next major version) , change default of configuration parameter. Then, this new behavior (respecting all rules) will be default.
This PR is to support 1. How about this ?
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.