Complete rewrite to support arbitrary logic expressions

[dvtomas]

Hi, folks,
what do you think of this?

dvtomas@f0376dc

I know it's big, I got carried away, the filtering logic I need is different, so I generalized the original quite a bit...

I think the performance should be roughly the same as the original, but the possibilities to use it in creative ways and extend it (@przygienda , I see you are just adding a regex support, that should be an easy FilterSpec addition) are IMO just so much greater..

[przygienda]

I was thinking about design like that originally for a bit, problem is e'thing becomes a linear lookup rather than log & that's why I chose the HashSet design ...

…

On Thu, Sep 27, 2018 at 5:41 AM Tomas Dvorak ***@***.***> wrote: Hi, folks, what do you think of this? ***@***.*** <dvtomas@f0376dc> I know it's big, I got carried away, the filtering logic I need is different, so I generalized the original quite a bit... I think the performance should be roughly the same as the original, but the possibilities to use it in creative ways and extend it ***@***.*** <https://github.com/przygienda> , I see you are just adding a regex support, that should be an easy FilterSpec addition) are IMO just so much greater.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABo0C15F5ADI7qHD7UUuHnxVO3vxij_6ks5ufMeVgaJpZM4W8k2V> .

[dvtomas]

I guess we can have the cake and eat it too. The new design is quite flexible in ways it can be extended to support new functionality (say regex, or your HashSet).

I guess we could just add a new variant here: https://github.com/dvtomas/kvfilter/blob/7a1f21329b305c7c4015f68ecf2ee5a284c3c579/src/lib.rs#L56

Something like

MatchAllValues {key: String, values: HashSet<String>}

We get the performance of the original for your use case plus the flexibility of the new design. What do you think?

Out of curiosity, can you give me an example of your typical logging configuration? I'm a little bit surprised that you have a lot of key-values that configure the logging. The way I use logging is that usually I have just a LevelFilter set to Info, and only when a problem occurs, I temporarily enable debug or trace in the particular subsystem I'm interested in, thats usually one or two KV's. For this scenario I guess Vec would be faster than HashSet, it carries less overhead, no need to compute hash etc.. So I'm interested about the logging patterns others use, I'd be very grateful if you could give me an example of yours..

[przygienda]

hmm, ok, yes, as long we can revert to hashsets to not walk chains every time it's interesting, if you insert matchAllValues and MatchAny and RegeX along the lines of what I did I can live with the And/Or stuff and in fact it maybe even cute to cut a major that way ... my logging is very complex & stuff is proprietary so sorry, can't disclose stuff here. I may have 5-6 keys with possibly a dozen values on each that need matching since a single subsystem can completely swamp me generating thousands of messages per second if not carefully filtered towards a slice of what I need looking for (e.g. a key of a particular element of which 1000s are moved per second in the subsystem) ... I drive it either via atomic swaps or complex CLI options ...

…

On Sun, Sep 30, 2018 at 10:21 AM Tomas Dvorak ***@***.***> wrote: I guess we can have the cake and eat it too. The new design is quite flexible in ways it can be extended to support new functionality (say regex, or your HashSet). I guess we could just add a new variant here: https://github.com/dvtomas/kvfilter/blob/7a1f21329b305c7c4015f68ecf2ee5a284c3c579/src/lib.rs#L56 Something like MatchAllValues {key: String, values: HashSet<String>} We get the performance of the original for your use case plus the flexibility of the new design. What do you think? Out of curiosity, can you give me an example of your typical logging configuration? I'm a little bit surprised that you have a lot of key-values that configure the logging. The way I use logging is that usually I have just a LevelFilter set to Info, and only when a problem occurs, I temporarily enable debug or trace in the particular subsystem I'm interested in, thats usually one or two KV's. For this scenario I guess Vec would be faster than HashSet, it carries less overhead, no need to compute hash etc.. So I'm interested about the logging patterns others use, I'd be very grateful if you could give me an example of yours.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABo0C7a9Sw4wKqOZ1fgmu2KYefPzL2rAks5ugP2ZgaJpZM4W8k2V> .

[dvtomas]

All right, I think I can implement MatchAllValues with the type signature sketched above. Just to make sure: The proposed behavior is that a given key must have all the values in the HashSet, is that correct? If I give an example:

log!(log, "Message"; "key" => "value1", "key" => "value2")
would be accepted by
MatchAllValues {key: "key", values: ["value1", "value2"]] }
but rejected by
MatchAllValues {key: "key", values: ["value1", "value3"]] }
is that correct?

As for the regex, how would that work? Is something like MatchValueRegex {key: String, value: RegEx} what you want?

I'll be having vacation most of the next week, but I'll try to implement it as soon as i get to it.

[dvtomas]

@przygienda, please confirm I got the specification of what you need right. I'll implement it most probably during the weekend.

[dvtomas]

Implemented, see the relevant PR

[dvtomas]

The CI fails on Rust 1.20, the serde_regex crate doesn't compile because of match ergonomics. Rust 1.20 is over a year old now, the question is if there's any reason to support it?

[przygienda]

I thought I added minimum version after regex stuff and all CI passes. yes, regex forces 1.20+ and I think it's fine ...

…

On Fri, Oct 5, 2018 at 11:23 PM Tomas Dvorak ***@***.***> wrote: The CI fails on Rust 1.20, the serde_regex crate doesn't compile because of match ergonomics. Rust 1.20 is over a year old now, the question is if there's any reason to support it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABo0C8z_7UhRUMAIGcnT3EeJE3KdaU0zks5uiExbgaJpZM4W8k2V> .

[dvtomas]

OK, I've submitted an issue and a PR tailhook/serde-regex#2 to the serde_regex crate. Hopefully the author will publish the fixed version soon and we'll be able to go on.