-
Notifications
You must be signed in to change notification settings - Fork 11
add enabled_logs variable which can be redefed to limit the set of logs that gets JSON streaming logs created for it #12
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
Conversation
scripts/main.zeek
Outdated
| else if ( filt?$path_func ) | ||
| filt$path = "json_streaming_" + filt$path_func(stream, "", []); | ||
|
|
||
| # Skip this filter if it's not in the enabled set (unless enabled_logs is empty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question here Seth — are the individual filters really the granularity you're after? I ask because the parsing is a bit complex and your example of set("http","files") suggests so. If you're actually after log streams, not their attached filters, we could do
const JSONStreaming::enabled_logs: set[Log::ID] = set() &redef;
instead. Then the set could be e.g. set(HTTP::LOG, Files::LOG) etc, and we'd check higher up as we iterate over Log::active_streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right. I'll adjust my PR next week after the holiday. Thanks for the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ckreibich Thanks for the suggestion I've updated the PR with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks Seth! Github has been unwilling to kick off that CI run for a whopping 8h now, so I'll hold out a little longer. 🤞
|
Hrrrm, looks like I no longer have maintainer access to the repo — I'd like to enable workflows for your PR. I'll need to ask around re. that one. |
|
I just can't get Github to actually run this package's workflows. It tests fine over here locally, so I'll merge now. Thanks Seth! |
This PR adds the ability to do the above. By default
enabled_logsis an empty set, meaning "do all of the logs" (the current default behavior). But by settingenabled_logsto a set containing one or more log names (e.g.,conn,http,dns,files, etc.) then only thosejson_streaming_logs get created. The rest of the logic (for rotating, etc.) is untouched.I also added
testing/tests/logs-filtered.zeekto verify.