Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Negation of <check>? #6

Open
DopeforHope opened this issue Jan 7, 2022 · 8 comments · May be fixed by #8
Open

[Feature Request] Negation of <check>? #6

DopeforHope opened this issue Jan 7, 2022 · 8 comments · May be fixed by #8
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@DopeforHope
Copy link

Hello,

nice work with this plugin. Makes working with Fluentd and JSON much easier. May it be possible to get a <exclude> (just like on the grep filter plugin) which just does the opposite of <check>?

Thank you very much

@iamazeem
Copy link
Owner

iamazeem commented Jan 7, 2022

Hi @DopeforHope,

Glad the plugin was helpful!
Thank you for the feature request!
Yes, the exclusion/negation could be implemented.

But, did you try to negate via regex with negative lookahead (?!) and inversion (^)?

(?!pat) - Negative lookahead assertion: ensures that the following characters do not match pat, but doesn't include those characters in the matched text

For example:

  <check>
    pointer   /log/user     # point to { "log": { "user": "test", ... } }
    pattern   /^(?!test)/   # check it against the value of username except `test`
  </check>

Apart from that, please add some test examples for exclusion/negations.
This is a nice feature to have for a straightforward negation instead of fiddling with regex a lot.
Thought about it a little and it seems like that a flag e.g. negate or invert would be much better and concise instead of having a new tag.

Example:

  <check>
    pointer   /log/user
    pattern   /test/i
    negate    true        # default: false
  </check>

@iamazeem iamazeem self-assigned this Jan 7, 2022
@DopeforHope
Copy link
Author

Yeah, the negative lookahead and inversion should suffice for most cases as well as for me.

The flag seems convenient. I will try creating some test cases but I'm no ruby expert.

@iamazeem iamazeem linked a pull request Jan 9, 2022 that will close this issue
@iamazeem
Copy link
Owner

iamazeem commented Jan 9, 2022

@DopeforHope : Would you be able to review and test the negation support (#8) on your side?

@DopeforHope
Copy link
Author

Test have passed and seems fine.

@iamazeem iamazeem added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 13, 2022
@nmaupu
Copy link

nmaupu commented Sep 8, 2022

Hi, it would be very nice to be able to negate the entire filter configuration. I mean, instead of letting an event pass if all checks match, I would like my event to NOT pass if all checks pass. Does that make sense ?

Now I don't seem to be able to do it because there is no support for or operator.
Because:

a && b && c == !a || !b || !c

So either I have to negate the entire filter (!(a && b && c)) or support or operator and negate each check (either by using the negate function - not merged - or negate all regex myself). I can't do that with the actual code if I am not mistaken.

What do you think ?

@iamazeem
Copy link
Owner

iamazeem commented Sep 8, 2022

Hi @nmaupu,

IIUC, by "negating the entire filter configuration", you meant to introduce a global flag e.g. negate_all true (default: false).
Is that correct?

Apart from that, if the existing PR #8 is merged then that'll serve your purpose too, right?
But, in that case, you'll have to negate each check which might be cumbersome to do if it's a frequent occurrence. 🤔

@nmaupu
Copy link

nmaupu commented Sep 8, 2022

Yes, that is correct
The PR #8 is not ok for me because negating every check is not sufficient, I would also need to be able to ORing them instead of ANDing them.
By the way, I succeeded in doing what I needed with the regexp filter with something like this:

<filter test>
  @type grep
  <and>
    <exclude>
      key $.properties.log.verb
      pattern ^get$
    </exclude>
    <exclude>
      key $.properties.log.kind
      pattern ^Event$
    </exclude>
    <exclude>
      key $['properties'].['log'].['annotations'].['authorization.k8s.io/reason']
      pattern ^$
    </exclude>
  </and>
</filter>

@iamazeem
Copy link
Owner

iamazeem commented Sep 8, 2022

Good to hear that, @nmaupu! Looks good! 👍
A little bit of fiddling with the existing plugins solves almost all the cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants