From f0dbecf0610c35efcded699250b57a48a1b69ca4 Mon Sep 17 00:00:00 2001 From: Danilo Egea Gondolfo Date: Fri, 25 Apr 2025 15:30:17 +0100 Subject: [PATCH] netstat_freebsd: refactoring Make the code testable and more maintainable: - Encapsulate all the CGO code so it can be tested - Fix the test file (it wasn't compiling) and improve tests - Mock the data returned by unix.SysctlRaw so the code can be tested without actually calling sysctl. No change in behavior intended Signed-off-by: Danilo Egea Gondolfo --- collector/netstat_freebsd.go | 118 +++++++++++++++++++++--------- collector/netstat_freebsd_test.go | 35 ++++++--- 2 files changed, 107 insertions(+), 46 deletions(-) diff --git a/collector/netstat_freebsd.go b/collector/netstat_freebsd.go index 3c34d8b3..61a5f26c 100644 --- a/collector/netstat_freebsd.go +++ b/collector/netstat_freebsd.go @@ -37,19 +37,63 @@ import ( import "C" var ( - bsdNetstatTcpSendPacketsTotal = prometheus.NewDesc( - prometheus.BuildFQName(namespace, "netstat", "tcp_transmit_packets_total"), - "TCP packets sent", - nil, nil, - ) + sysctlRaw = unix.SysctlRaw + tcpSendTotal = "bsdNetstatTcpSendPacketsTotal" + tcpRecvTotal = "bsdNetstatTcpRecvPacketsTotal" - bsdNetstatTcpRecvPacketsTotal = prometheus.NewDesc( - prometheus.BuildFQName(namespace, "netstat", "tcp_receive_packets_total"), - "TCP packets received", - nil, nil, - ) + counterMetrics = map[string]*prometheus.Desc{ + tcpSendTotal: prometheus.NewDesc( + prometheus.BuildFQName(namespace, "netstat", "tcp_transmit_packets_total"), + "TCP packets sent", nil, nil), + tcpRecvTotal: prometheus.NewDesc( + prometheus.BuildFQName(namespace, "netstat", "tcp_receive_packets_total"), + "TCP packets received", nil, nil), + } ) +type NetstatData struct { + structSize int + sysctl string +} + +type NetstatMetrics map[string]float64 + +type NetstatTCPData NetstatData + +func NewTCPStat() *NetstatTCPData { + return &NetstatTCPData{ + structSize: int(unsafe.Sizeof(C.struct_tcpstat{})), + sysctl: "net.inet.tcp.stats", + } +} + +func (netstatMetric *NetstatTCPData) GetData() (NetstatMetrics, error) { + data, err := getData(netstatMetric.sysctl, netstatMetric.structSize) + if err != nil { + return nil, err + } + + tcpStats := *(*C.struct_tcpstat)(unsafe.Pointer(&data[0])) + + return NetstatMetrics{ + tcpSendTotal: float64(tcpStats.tcps_sndtotal), + tcpRecvTotal: float64(tcpStats.tcps_rcvtotal), + }, nil +} + +func getData(queryString string, expectedSize int) ([]byte, error) { + data, err := sysctlRaw(queryString) + if err != nil { + fmt.Println("Error:", err) + return nil, err + } + + if len(data) < expectedSize { + return nil, errors.New("Data Size mismatch") + } + return data, nil +} + type netStatCollector struct { netStatMetric *prometheus.Desc } @@ -70,39 +114,41 @@ func (c *netStatCollector) Collect(ch chan<- prometheus.Metric) { _ = c.Update(ch) } -func getData(queryString string) ([]byte, error) { - data, err := unix.SysctlRaw(queryString) - if err != nil { - fmt.Println("Error:", err) - return nil, err - } - - if len(data) < int(unsafe.Sizeof(C.struct_tcpstat{})) { - return nil, errors.New("Data Size mismatch") - } - return data, nil -} - func (c *netStatCollector) Update(ch chan<- prometheus.Metric) error { - - tcpData, err := getData("net.inet.tcp.stats") + tcpStats, err := NewTCPStat().GetData() if err != nil { return err } - tcpStats := *(*C.struct_tcpstat)(unsafe.Pointer(&tcpData[0])) + allStats := make(map[string]float64) - ch <- prometheus.MustNewConstMetric( - bsdNetstatTcpSendPacketsTotal, - prometheus.CounterValue, - float64(tcpStats.tcps_sndtotal), - ) + for k, v := range tcpStats { + allStats[k] = v + } - ch <- prometheus.MustNewConstMetric( - bsdNetstatTcpRecvPacketsTotal, - prometheus.CounterValue, - float64(tcpStats.tcps_rcvtotal), - ) + for metricKey, metricData := range counterMetrics { + ch <- prometheus.MustNewConstMetric( + metricData, + prometheus.CounterValue, + allStats[metricKey], + ) + } return nil } + +// Used by tests to mock unix.SysctlRaw +func getFreeBSDDataMock(sysctl string) []byte { + + if sysctl == "net.inet.tcp.stats" { + tcpStats := C.struct_tcpstat{ + tcps_sndtotal: 1234, + tcps_rcvtotal: 4321, + } + size := int(unsafe.Sizeof(C.struct_tcpstat{})) + + return unsafe.Slice((*byte)(unsafe.Pointer(&tcpStats)), size) + } + + return make([]byte, 0, 0) +} diff --git a/collector/netstat_freebsd_test.go b/collector/netstat_freebsd_test.go index 3b3f8523..26ac371e 100644 --- a/collector/netstat_freebsd_test.go +++ b/collector/netstat_freebsd_test.go @@ -18,11 +18,16 @@ package collector import ( "github.com/prometheus/client_golang/prometheus" - "golang.org/x/sys/unix" "testing" - "unsafe" ) +func testSetup() { + sysctlRaw = func(name string, _ ...int) ([]byte, error) { + mockData := getFreeBSDDataMock(name) + return mockData, nil + } +} + func TestNetStatCollectorDescribe(t *testing.T) { ch := make(chan *prometheus.Desc, 1) collector := &netStatCollector{ @@ -31,24 +36,34 @@ func TestNetStatCollectorDescribe(t *testing.T) { collector.Describe(ch) desc := <-ch - if want, got := "dummy_metric", desc.String(); want != got { + expected := "Desc{fqName: \"dummy_metric\", help: \"dummy\", constLabels: {}, variableLabels: {}}" + if want, got := expected, desc.String(); want != got { t.Errorf("want %s, got %s", want, got) } } -func TestGetData(t *testing.T) { - data, err := getData("net.inet.tcp.stats") +func TestGetTCPMetrics(t *testing.T) { + testSetup() + + tcpData, err := NewTCPStat().GetData() if err != nil { t.Fatal("unexpected error:", err) } - if got, want := len(data), int(unsafe.Sizeof(unix.TCPStats{})); got < want { - t.Errorf("data length too small: want >= %d, got %d", want, got) + sndTotal := tcpData[tcpSendTotal] + rcvTotal := tcpData[tcpRecvTotal] + + if got, want := sndTotal, float64(1234); got != want { + t.Errorf("unexpected sndTotal value: want %f, got %f", want, got) + } + + if got, want := rcvTotal, float64(4321); got != want { + t.Errorf("unexpected rcvTotal value: want %f, got %f", want, got) } } func TestNetStatCollectorUpdate(t *testing.T) { - ch := make(chan prometheus.Metric, len(metrics)) + ch := make(chan prometheus.Metric, len(counterMetrics)) collector := &netStatCollector{ netStatMetric: prometheus.NewDesc("netstat_metric", "NetStat Metric", nil, nil), } @@ -57,11 +72,11 @@ func TestNetStatCollectorUpdate(t *testing.T) { t.Fatal("unexpected error:", err) } - if got, want := len(ch), len(metrics); got != want { + if got, want := len(ch), len(counterMetrics); got != want { t.Errorf("metric count mismatch: want %d, got %d", want, got) } - for range metrics { + for range len(counterMetrics) { <-ch } }