-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add normalization for IPFIX events #9616
base: main
Are you sure you want to change the base?
Add normalization for IPFIX events #9616
Conversation
Uses IPFIX information elements to normalize IPFIX records.
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
Hi @somedatapacket, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
@karmi sorry about that, done. |
Jenkins, please test this |
I added two additional translations/normalizations. There are some things that require more complicated logic, though, for this to be feature complete. |
Thanks @somedatapacket . @jsvd who would be a good person to review this? |
@somedatapacket what things are missing to complete the picture? |
@andrewvc hopefully not much! but in my case I can't validate implementation of ipVersion normalization or flow direction, as these attributes are not included in the flow template I'm testing with. |
I am trying to figure how we could practically test this. The problem is that we do not have any tests which can validate the configuration file and prevent regressions especially for the filters logic. This is obviously a larger & separate issue than this PR purpose here but I'd like to see if we could come up with something practical that could help us validate these changes and help us move forward with figuring the way to add tests. One idea that came up while discussin this internally was to use a tool like nfdump to capture and replay netflow data. We could generate a config that preserves the filters and replace the input with a file or stdin input and similarly replace the output with a file or stdout output plus json codec. That way we could isolate the filter testing by replaying netflow data by sending it to stdin and validate the json output with what is expected, or something like that. As a first step @somedatapacket would you be open in manually trying this methodology? If we could come up with a set of netflow data which tests the new parsing that way, this would validate the idea and then we could use that to bootstrap the testing framework in another issue. WDYT? |
@colinsurprenant internally, I have been testing this code with data from routers in our lab environment, but my methodology is probably insufficiently rigorous. Would something like this have utility? https://github.com/logstash-plugins/logstash-codec-netflow/blob/master/spec/codecs/ipfix_stress.py |
@somedatapacket good question - let's ping @jorritfolmer to see what his thoughts are on that since in essence anything done in that direction for the module will be usable for the codec testing too I guess. |
@colinsurprenant @somedatapacket The logstash-input-udp rspec script would be a good place start. It actually can send a message through the udp socket as a client, and verifies the resulting Logstash event. Replace this message with one of the samples from our netflow packet library, and verify the result to be the expected normalised fields. On a separate note: Isn't this file based on the upstream Elastiflow? It seems support for normalising IPFIX fields has already been added some time ago: https://github.com/robcowart/elastiflow/commits/master/logstash/elastiflow/conf.d/20_filter_30_ipfix.logstash.conf. It seems to me accepting PRs here would lead to duplication of effort that would be much better focussed on improving the state of art over there. |
It sure looks similar! Elastiflow is new to me. |
@colinsurprenant is this native plugin based on elastiflow? sorry for my ignorance here if so... |
It is indeed! #7715 Until recently both were in sync but now Elastiflow changed from the apache 2 license. We want to continue to provide an out of the box netflow experience through the netflow module that logstash ships with, but it is normal that these two projects will continue to deviate. |
@jsvd @colinsurprenant what next steps could we take on this? |
@somedatapacket since adding automated tests is a project in itself I'd be ok to move forward with this PR given your own experience with it and create a separate issue for the testing framework. @jsvd WDYT? |
@colinsurprenant @jsvd i'm definitely in favor! |
Uses IPFIX information elements to normalize IPFIX records.