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

Wiki article and example YAML/scripts for custom Zeek/Suricata/NetFlow #72

Merged
merged 37 commits into from
Jun 9, 2021

Conversation

philrz
Copy link
Contributor

@philrz philrz commented May 5, 2021

The motivation for this article is described in #64. Having written the initial version and tested it extensively, I'm inclined to wait on fixes for some of the bugs that were found along the way, as that way the final version will have fewer warts. Beyond the bugs, the end-to-end shown here also illustrates the need for some of the other enhancements waiting in our backlog. I'm starting this out as a draft PR and will add comments at the relevant spots.

Closes #64
Closes #8

@philrz philrz self-assigned this May 5, 2021
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
Comment on lines 271 to 275
1. The `type alert` defines the names, [data types](https://github.com/brimdata/zed/blob/main/docs/formats/zson.md#33-primitive-values),
and hierarchical locations of expected fields in the input records. Here we've
defined a single "wide" shape for _all_ alerts we've known Suricata to
generate, which is convenient because it allows Brim to easily display them in
columnar format.
Copy link
Contributor Author

@philrz philrz May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's been debated at times whether it's appropriate to use these wide schemas, particularly when it might result in records with excessive nulls because of alerts that don't use several fields. However, I've continued to err on the side of "wide"-ness because we've had community users report it as a bug when data they're accustomed to seeing in columnar layouts in Brim suddenly lose their column headers because additional schemas have appeared. We did discuss once as a group some UX we could add in Brim to call a user's attention to the multiple schemas to make it easy for the user to see the data under each schema separately (e.g. perhaps under a separate "tab" per schema), and when that exists, I'd be more open to letting through more diverse schemas.

docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
Comment on lines 423 to 425
Our pool is now ready to be queried in Brim.

![NetFlow Pool](media/NetFlow-Pool.png)
Copy link
Contributor Author

@philrz philrz May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the histogram is empty, which is unsurprising since I think we're still wired to only populate this if the field named _path is present. We've discussed in the past that it might be helpful to be able to specify a primary "segmentation axis" for the data in a pool (e.g. _path for Zeek) that could be used to make stacked bars when present, and in its absence, just give non-stacked "count" bars? (see brimdata/zui#1541)

@philrz
Copy link
Contributor Author

philrz commented May 5, 2021

The markdown link checker is an expected failure because the links in the docs are relative to where this will ultimately be published in the wiki.

Copy link
Member

@jameskerr jameskerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I was reading about the output files and globs, I wasn't sure what directory those files would end up in? The current directory?

Otherwise very clear and high quality. The ergonomics are not quite there in the tooling yet. There are a lot of workarounds.

Co-authored-by: James Kerr <[email protected]>
examples/zeek-wrapper.sh Outdated Show resolved Hide resolved
examples/suricata-wrapper.sh Outdated Show resolved Hide resolved
examples/suricata-wrapper.sh Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
examples/nfdump-wrapper.sh Outdated Show resolved Hide resolved
docs/Custom-brimcap-load-Config.md Outdated Show resolved Hide resolved
@philrz
Copy link
Contributor Author

philrz commented May 6, 2021

Thanks for the feedback, @jameskerr. Some thoughts:

As I was reading about the output files and globs, I wasn't sure what directory those files would end up in? The current directory?

I'm glad you spoke up about this. I was on the fence about whether to get into the topic in this article, but since you spotted its absence, I went ahead and added some text that mentions how it goes to a temporary directory by default, and then a new "Debug" section that describes how to use the workdir: option to force it to write to a specific location. Feel free to give it another read and let me know if you think it could use more improvements.

The ergonomics are not quite there in the tooling yet. There are a lot of workarounds.

That's exactly what I was trying to communicate, so thanks for saying that! 😄   Indeed, I'm glad to see that these things are feasible with a little elbow grease. But before I feel confident pointing users at it and implying that we're ready for them to go as far as they want with it, it'll indeed be good to see more bug fixes and UX improvements start to arrive. As mentioned at our team sync meeting, I'm keen to hold this PR open in a draft state and use it like a checklist as we march toward bundling the next release.

@philrz philrz marked this pull request as ready for review June 8, 2021 23:59
@philrz philrz merged commit b071368 into main Jun 9, 2021
@philrz philrz deleted the custom-yaml-article branch June 9, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants