search

netsampler / goflow2

High performance sFlow/IPFIX/NetFlow Collector
BSD 3-Clause "New" or "Revised" License
431 stars 99 forks source link

Add support for CSV format #162

Closed jbemmel closed 10 months ago

jbemmel commented 1 year ago

I am looking to replace a limited sflowtool with goflow2, and to do so it would have to mimic the output

The output looks like this:

FLOW,193.242.111.152,2,21,0013136f2fc0,0010a52f261f,0x0800,10,10,94.1.115.114,80.1.2.222,6,0x00,124,1863,750,0x18,179,165,1024

(see https://github.com/jbemmel/IXP-Manager/blob/master/tools/runtime/sflow/sflow-detect-ixp-bgp-sessions#L109)

This is a variation on the 'text' format, leaving out field names and inserting a custom first value "FLOW"

lspgn commented 1 year ago

Hi @jbemmel, Thank you for this PR and for the work you put in. Unfortunately, adding a new format to the GoFlow2 upstream adds more maintenance into the project and unless the feature is heavily used I would rather avoid merging at the moment. I think the solution is to maintain a fork.

Furthermore, CSV has a few drawbacks when it comes to schema management. It is not easily readable by a human unless in a relational database/spreadsheet. New columns must be added at the end and delimiters can vary per implementation. Also unless it's properly configured, output may not match with sflowtool. In regards to IXP manager, I would suggest to integrate a JSON decoder.

Finally, I am developing a v2 of this tool and the current version will not have new features. This turned around how the messages are formatted by the library. It also provides access to the sFlow counter samples which from my quick code exploration seems supported in sflowtool.

Let me know if you have questions

jbemmel commented 1 year ago

Hi @lspgn, thanks for your feedback

The CSV output would have a different purpose than JSON or protobuf, and would need to be configured correctly with the subset of fields. It would support quick integration for rapid prototyping and proof-of-concepts, perhaps as a prelude to a more robust integration

I would be happy to retarget the PR at your v2 branch

jbemmel commented 1 year ago

See https://github.com/netsampler/goflow2/pull/164

lspgn commented 10 months ago

Doing a bit of housekeeping. Unfortunately, I don't think I will add CSV at the moment. May revisit later. Thank you for your suggestion and PR.