influxdata / telegraf

Agent for collecting, processing, aggregating, and writing metrics, logs, and other arbitrary data.
https://influxdata.com/telegraf
MIT License
14.11k stars 5.51k forks source link

inputs.sflow: Parse extended packet format (type 1001) #14098

Closed Hipska closed 1 month ago

Hipska commented 8 months ago

Use Case

We are receiving extended sflow packets but the current inputs.sflow implementation isn't able to parse them:

2023-10-12T11:00:30Z D! [inputs.sflow] Unknown flow format: 1001
2023-10-12T11:00:32Z D! [inputs.sflow] Unknown flow format: 1001
2023-10-12T11:00:36Z D! [inputs.sflow] Unknown flow format: 1001
2023-10-12T11:00:36Z D! [inputs.sflow] Unknown flow format: 1001
2023-10-12T11:00:37Z D! [inputs.sflow] Unknown sample type: 2
2023-10-12T11:00:37Z D! [inputs.sflow] Unknown sample type: 2

The current inputs.netflow plugin uses the netsampler/goflow2 module which should be able to do this.

Expected behavior

Telegraf to be able to parse extended packet formats as well.

Actual behavior

The current inputs.sflow is only able to parse/handle Raw packets: https://github.com/influxdata/telegraf/blob/0e1f3f8b865579ff4dc906cc33930ed167a4134e/plugins/inputs/sflow/packetdecoder.go#L248-L253

Additional info

No response

srebhan commented 8 months ago

@Hipska please use the inputs.netflow plugin with the sflow protocol type!

Hipska commented 8 months ago

Why? I was going to look into rewriting the inputs.sflow plugin to use the module I mentioned.

If inputs.sflow is considered to be deprecated and inputs.netflow should be used, then it should marked as deprecated..

srebhan commented 8 months ago

It can be marked deprecates as soon as we are sure the new plugin is equal in features... Which it is not as goflow2 does not support all protocols yet...

Hipska commented 8 months ago

Wait, now you are saying the Netflow plugin also doesn’t fit? (As that is currently using goflow2)

powersj commented 8 months ago

@Hipska,

We can deprecate the usage of sflow, only once we know that all the various extended types are supported by the netflow plugin (based on goflow).

Let's use this issue to check to see if the goflow library does supports the existing extended types.

Is this something you can confirm?

Thanks

Hipska commented 8 months ago

Yes, see DecodeFlowRecord, which can handle these specific cases.

But I don't see why you want to deprecate the sflow specific plugin? NetFlow and sFlow are not the same, but indeed very similar. IMHO it would make sense to deprecate sflow option for inputs.netflow and have the sflow specific plugin with more specific documentation and fine-grained metric details..

srebhan commented 8 months ago

@Hipska did you even look at the netflow input plugin? It uses the github.com/netsampler/goflow2 library which supports netflow v5, v9, IPFIX and sflow v5... The issue appears when it comes to raw-header samples received via SFlow. The goflow2 library does not extraction of raw-header information, while the sflow plugin does. I use the gopacket library to extend to implement raw-header parsing in inputs.netflow but it is not a superset of the ones inputs.sflow supports..

So why do you want to reimplement inputs.sflow with a library that is already used in inputs.netflow for doing exactly this?

Hipska commented 8 months ago

@Hipska did you even look at the netflow input plugin? It uses the github.com/netsampler/goflow2 library which supports netflow v5, v9, IPFIX and sflow v5...

Yes, see this specific line in my initial request:

The current inputs.netflow plugin uses the netsampler/goflow2 module which should be able to do this.

Coming to this point:

So why do you want to reimplement inputs.sflow with a library that is already used in inputs.netflow for doing exactly this?

I already said in my previous comment:

IMHO it would make sense to deprecate sflow option for inputs.netflow and have the sflow specific plugin with more specific documentation and fine-grained metric details..

srebhan commented 8 months ago

And I already said that inputs.netflow does not support all raw-header protocol inputs.sflow supports, so we cannot deprecate it. :-) So the correct thing would be to implement the missing pieces in inputs.netflow and THEN deprecate inputs.sflow...

Hipska commented 1 month ago

Whut?

srebhan commented 1 month ago

I rechecked the netflow plugin and it can do everything that the sflow plugin can do plus it can parse packet type 1001... So yes, this should be closed now and you should go with the netflow plugin. If this doesn't work, please open a new issue against the netflow plugin.