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 <danilo@FreeBSD.org>

collector/netstat_freebsd.go

@@ -37,19 +37,63 @@ import (
 import "C"
 
 var (
-	bsdNetstatTcpSendPacketsTotal = prometheus.NewDesc(
+	sysctlRaw    = unix.SysctlRaw
-		prometheus.BuildFQName(namespace, "netstat", "tcp_transmit_packets_total"),
+	tcpSendTotal = "bsdNetstatTcpSendPacketsTotal"
-		"TCP packets sent",
+	tcpRecvTotal = "bsdNetstatTcpRecvPacketsTotal"
-		nil, nil,
-	)
 
-	bsdNetstatTcpRecvPacketsTotal = prometheus.NewDesc(
+	counterMetrics = map[string]*prometheus.Desc{
-		prometheus.BuildFQName(namespace, "netstat", "tcp_receive_packets_total"),
+		tcpSendTotal: prometheus.NewDesc(
-		"TCP packets received",
+			prometheus.BuildFQName(namespace, "netstat", "tcp_transmit_packets_total"),
-		nil, nil,
+			"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 {
-
+	tcpStats, err := NewTCPStat().GetData()
-	tcpData, err := getData("net.inet.tcp.stats")
 	if err != nil {
 		return err
 	}
 
-	tcpStats := *(*C.struct_tcpstat)(unsafe.Pointer(&tcpData[0]))
+	allStats := make(map[string]float64)
 
-	ch <- prometheus.MustNewConstMetric(
+	for k, v := range tcpStats {
-		bsdNetstatTcpSendPacketsTotal,
+		allStats[k] = v
-		prometheus.CounterValue,
+	}
-		float64(tcpStats.tcps_sndtotal),
-	)
 
-	ch <- prometheus.MustNewConstMetric(
+	for metricKey, metricData := range counterMetrics {
-		bsdNetstatTcpRecvPacketsTotal,
+		ch <- prometheus.MustNewConstMetric(
-		prometheus.CounterValue,
+			metricData,
-		float64(tcpStats.tcps_rcvtotal),
+			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)
+}

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) {
+func TestGetTCPMetrics(t *testing.T) {
-	data, err := getData("net.inet.tcp.stats")
+	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 {
+	sndTotal := tcpData[tcpSendTotal]
-		t.Errorf("data length too small: want >= %d, got %d", want, got)
+	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
 	}
 }