feat: add new metrics #22
Conversation
Add additional metrics for version 7.0.x and the upcoming 8.0.0 and correct small typos and compatability problems with the old 6.0.20
Add also the option to expose overall total metrics defined at the top-level as the sum of all the per-thread ones, avoiding more computation on the receiving side
aligned with the total number of new metrics
There was a problem hiding this comment.
Hey @edoardomich , thanks for your contribution. Mind checking out the comments and see if you agree with them and adapt the changes?
If you could add a suricata dump-counters output into testdata for 8.0.x with the configuration you used to develop this, that'd be awesome! Maybe also add a smoke test for the flow_end merics that seemed to be non-functional. The tests aren't the best, but hopefully sufficient for adding another smoke test.
Thanks!
main.go
Outdated
| @@ -83,7 +83,7 @@ var ( | |||
| newPerThreadGaugeMetric("capture", "afpacket_busy_loop_avg", "", "busy_loop_avg"), | |||
| // The following 4 are put into a single metrics afpacket_polls_total | |||
| // where the result is a labels. | |||
| // newPerThreadCounterMetric("capture", "afpacket_poll_total", "", "polls"), | |||
| newPerThreadCounterMetric("capture", "afpacket_poll_total", "", "polls"), | |||
There was a problem hiding this comment.
Mind reverting this one? The idea here was that summing the afpacket_poll_results_total counters should result in the total. If this is not the case anymore, is perThreadAFPacketPollResultEntries missing an entry for a different result?
This is from the current suricata dev version and it seems signal, timeout, data, error capture polls.
"capture": {
"kernel_packets": 18574,
"kernel_drops": 0,
"errors": 0,
"afpacket": {
"busy_loop_avg": 0,
"polls": 18422,
"poll_signal": 0,
"poll_timeout": 16464,
"poll_data": 1958,
"poll_errors": 0,
"send_errors": 0
}
main.go
Outdated
| // From: .thread.decoder.event.afpacket | ||
| // New in 8.0.0: d78f2c9a4e2b59f44daeddff098915084493d08d | ||
| perThreadDecoderEventAFPacketMetrics = []metricInfo{ | ||
| newPerThreadCounterMetric("decoder_event", "afpacket_trunc_pkt", "", "trunc_pkt").Optional(), |
There was a problem hiding this comment.
| newPerThreadCounterMetric("decoder_event", "afpacket_trunc_pkt", "", "trunc_pkt").Optional(), | |
| newPerThreadCounterMetric("decoder_event", "afpacket_truncated_packets_total", "", "trunc_pkt"), |
Mostly to align with the other decoder packet counter metrics above (udp_packets_total) and dropping Optional() as it seems if afpacket is within decoder.event, can expect trunc_pkt to be included.
Aside, seems a bit unfortunate the metric is not part of the capture.afpacket object as the event is raised in source-af-packet.c, but not sure it's worth an upstream ticket to move or also include it there.
@jlucovsky / @jasonish, thoughts? Nothing urgent or to do, just some feedback / food for thought, for the afpacket.trunc_pkt stat.
There was a problem hiding this comment.
Ok for the rename. I add Optional() to be compatible with previous versions (like version 7), since this metric was only included for version 8.0.0.
| @@ -255,6 +314,12 @@ var ( | |||
| newPerThreadCounterMetric("detect", "alerts_suppressed_total", "", "alerts_suppressed").Optional(), | |||
| } | |||
|
|
|||
| // From .thread.file_store | |||
| perThreadFileStoreMetrics = []metricInfo{ | |||
| newPerThreadCounterMetric("filestore", "open_files_max_hit", "", "open_files_max_hit").Optional(), | |||
There was a problem hiding this comment.
Running a default configuration, I only see this in dump-counters:
"file_store": {
"open_files": 0
},
Mind posting a section of your dump-counters that includes the filestore metrics?
The max_hit should probably also be a guage.
Enhance the coverage by including additional metrics introduced in version 7.0.x series, as well as new ones added in the recently released version 8.0.0. Additionally, I propose adding a flag to retrieve global metrics that represent the aggregated totals of the per-thread counters.
The tests were conducted using simple container images with Suricata v6.0.20, v7.0.11, and the newly released v8.0.0