ZFS Collector: Refactor to use maps/slices and fewer globals

Removed all global types that were unnecessary, and refactored to use constructor-created values and inline values instead of globals.

Signed-Off-By: Joe Handzik <joseph.t.handzik@hpe.com>

collector/zfs.go

@@ -24,32 +24,16 @@ import (
 	"github.com/prometheus/common/log"
 )
 
-type zfsMetricValue int
-
-const zfsErrorValue = zfsMetricValue(-1)
-
 var zfsNotAvailableError = errors.New("ZFS / ZFS statistics are not available")
 
 type zfsSysctl string
-type zfsSubsystemName string
-
-const (
-	arc            = zfsSubsystemName("zfsArc")
-	dmuTx          = zfsSubsystemName("zfsDmuTx")
-	fm             = zfsSubsystemName("zfsFm")
-	vdevCache      = zfsSubsystemName("zfsVdevCache")
-	xuio           = zfsSubsystemName("zfsXuio")
-	zfetch         = zfsSubsystemName("zfsFetch")
-	zil            = zfsSubsystemName("zfsZil")
-	zpoolSubsystem = zfsSubsystemName("zfsPool")
-)
 
 // Metrics
 
 type zfsMetric struct {
-	subsystem zfsSubsystemName // The Prometheus subsystem name.
+	subsystem string    // The Prometheus subsystem name.
-	name      string           // The Prometheus name of the metric.
+	name      string    // The Prometheus name of the metric.
-	sysctl    zfsSysctl        // The sysctl of the ZFS metric.
+	sysctl    zfsSysctl // The sysctl of the ZFS metric.
 }
 
 // Collector
@@ -59,58 +43,36 @@ func init() {
 }
 
 type zfsCollector struct {
-	zfsMetrics []zfsMetric
+	zfsMetrics        []zfsMetric
+	linuxProcpathBase string
+	linuxPathMap      map[string]string
 }
 
 func NewZFSCollector() (Collector, error) {
-	return &zfsCollector{}, nil
+	var z zfsCollector
+	z.linuxProcpathBase = "spl/kstat/zfs"
+	z.linuxPathMap = map[string]string{
+		"zfsArc":       "arcstats",
+		"zfsDmuTx":     "dmu_tx",
+		"zfsFm":        "fm",
+		"zfsFetch":     "zfetchstats",
+		"zfsVdevCache": "vdev_cache_stats",
+		"zfsXuio":      "xuio_stats",
+		"zfsZil":       "zil",
+	}
+	return &z, nil
 }
 
 func (c *zfsCollector) Update(ch chan<- prometheus.Metric) (err error) {
-	// Arcstats
+	for subsystem := range c.linuxPathMap {
-	err = c.updateArcstats(ch)
+		err = c.updateZfsStats(subsystem, ch)
-	switch {
+		switch {
-	case err == zfsNotAvailableError:
+		case err == zfsNotAvailableError:
-		log.Debug(err)
+			log.Debug(err)
-		return nil
+			return nil
-	case err != nil:
+		case err != nil:
-		return err
+			return err
-	}
+		}
-
-	// Zfetchstats
-	err = c.updateZfetchstats(ch)
-	if err != nil {
-		return err
-	}
-
-	// Zil
-	err = c.updateZil(ch)
-	if err != nil {
-		return err
-	}
-
-	// VdevCacheStats
-	err = c.updateVdevCacheStats(ch)
-	if err != nil {
-		return err
-	}
-
-	// XuioStats
-	err = c.updateXuioStats(ch)
-	if err != nil {
-		return err
-	}
-
-	// Fm
-	err = c.updateFm(ch)
-	if err != nil {
-		return err
-	}
-
-	// DmuTx
-	err = c.updateDmuTx(ch)
-	if err != nil {
-		return err
 	}
 
 	// Pool stats
@@ -122,7 +84,7 @@ func (s zfsSysctl) metricName() string {
 	return parts[len(parts)-1]
 }
 
-func (c *zfsCollector) constSysctlMetric(subsystem zfsSubsystemName, sysctl zfsSysctl, value zfsMetricValue) prometheus.Metric {
+func (c *zfsCollector) constSysctlMetric(subsystem string, sysctl zfsSysctl, value int) prometheus.Metric {
 	metricName := sysctl.metricName()
 
 	return prometheus.MustNewConstMetric(

collector/zfs_linux.go

@@ -26,17 +26,6 @@ import (
 	"github.com/prometheus/common/log"
 )
 
-const (
-	zfsProcpathBase      = "spl/kstat/zfs/"
-	zfsArcstatsExt       = "arcstats"
-	zfsDmuTxExt          = "dmu_tx"
-	zfsFmExt             = "fm"
-	zfsFetchstatsExt     = "zfetchstats"
-	zfsVdevCacheStatsExt = "vdev_cache_stats"
-	zfsXuioStatsExt      = "xuio_stats"
-	zfsZilExt            = "zil"
-)
-
 func (c *zfsCollector) openProcFile(path string) (file *os.File, err error) {
 	file, err = os.Open(procFilePath(path))
 	if err != nil {
@@ -46,91 +35,19 @@ func (c *zfsCollector) openProcFile(path string) (file *os.File, err error) {
 	return
 }
 
-func (c *zfsCollector) updateArcstats(ch chan<- prometheus.Metric) (err error) {
+func (c *zfsCollector) updateZfsStats(subsystem string, ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsArcstatsExt))
+	file, err := c.openProcFile(filepath.Join(c.linuxProcpathBase, c.linuxPathMap[subsystem]))
 	if err != nil {
 		return err
 	}
 	defer file.Close()
 
-	return c.parseProcfsFile(file, zfsArcstatsExt, func(s zfsSysctl, v zfsMetricValue) {
+	return c.parseProcfsFile(file, c.linuxPathMap[subsystem], func(s zfsSysctl, v int) {
-		ch <- c.constSysctlMetric(arc, s, v)
+		ch <- c.constSysctlMetric(subsystem, s, v)
 	})
 }
 
-func (c *zfsCollector) updateZfetchstats(ch chan<- prometheus.Metric) (err error) {
+func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler func(zfsSysctl, int)) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsFetchstatsExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsFetchstatsExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(zfetch, s, v)
-	})
-}
-
-func (c *zfsCollector) updateZil(ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsZilExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsZilExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(zil, s, v)
-	})
-}
-
-func (c *zfsCollector) updateVdevCacheStats(ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsVdevCacheStatsExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsVdevCacheStatsExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(vdevCache, s, v)
-	})
-}
-
-func (c *zfsCollector) updateXuioStats(ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsXuioStatsExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsXuioStatsExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(xuio, s, v)
-	})
-}
-
-func (c *zfsCollector) updateFm(ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsFmExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsFmExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(fm, s, v)
-	})
-}
-
-func (c *zfsCollector) updateDmuTx(ch chan<- prometheus.Metric) (err error) {
-	file, err := c.openProcFile(filepath.Join(zfsProcpathBase, zfsDmuTxExt))
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	return c.parseProcfsFile(file, zfsDmuTxExt, func(s zfsSysctl, v zfsMetricValue) {
-		ch <- c.constSysctlMetric(dmuTx, s, v)
-	})
-}
-
-func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler func(zfsSysctl, zfsMetricValue)) (err error) {
 	scanner := bufio.NewScanner(reader)
 
 	parseLine := false
@@ -154,7 +71,7 @@ func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler
 		if err != nil {
 			return fmt.Errorf("could not parse expected integer value for %q", key)
 		}
-		handler(zfsSysctl(key), zfsMetricValue(value))
+		handler(zfsSysctl(key), value)
 
 	}
 	if !parseLine {

collector/zfs_linux_test.go

@@ -31,7 +31,7 @@ func TestArcstatsParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(arcstatsFile, "arcstats", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(arcstatsFile, "arcstats", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.arcstats.hits") {
 			return
@@ -39,7 +39,7 @@ func TestArcstatsParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(8772612) {
+		if v != int(8772612) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -67,7 +67,7 @@ func TestZfetchstatsParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(zfetchstatsFile, "zfetchstats", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(zfetchstatsFile, "zfetchstats", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.zfetchstats.hits") {
 			return
@@ -75,7 +75,7 @@ func TestZfetchstatsParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(7067992) {
+		if v != int(7067992) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -103,7 +103,7 @@ func TestZilParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(zilFile, "zil", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(zilFile, "zil", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.zil.zil_commit_count") {
 			return
@@ -111,7 +111,7 @@ func TestZilParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(10) {
+		if v != int(10) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -139,7 +139,7 @@ func TestVdevCacheStatsParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(vdevCacheStatsFile, "vdev_cache_stats", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(vdevCacheStatsFile, "vdev_cache_stats", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.vdev_cache_stats.delegations") {
 			return
@@ -147,7 +147,7 @@ func TestVdevCacheStatsParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(40) {
+		if v != int(40) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -175,7 +175,7 @@ func TestXuioStatsParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(xuioStatsFile, "xuio_stats", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(xuioStatsFile, "xuio_stats", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.xuio_stats.onloan_read_buf") {
 			return
@@ -183,7 +183,7 @@ func TestXuioStatsParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(32) {
+		if v != int(32) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -211,7 +211,7 @@ func TestFmParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(fmFile, "fm", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(fmFile, "fm", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.fm.erpt-dropped") {
 			return
@@ -219,7 +219,7 @@ func TestFmParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(18) {
+		if v != int(18) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}
 
@@ -247,7 +247,7 @@ func TestDmuTxParsing(t *testing.T) {
 	}
 
 	handlerCalled := false
-	err = c.parseProcfsFile(dmuTxFile, "dmu_tx", func(s zfsSysctl, v zfsMetricValue) {
+	err = c.parseProcfsFile(dmuTxFile, "dmu_tx", func(s zfsSysctl, v int) {
 
 		if s != zfsSysctl("kstat.zfs.misc.dmu_tx.dmu_tx_assigned") {
 			return
@@ -255,7 +255,7 @@ func TestDmuTxParsing(t *testing.T) {
 
 		handlerCalled = true
 
-		if v != zfsMetricValue(3532844) {
+		if v != int(3532844) {
 			t.Fatalf("Incorrect value parsed from procfs data")
 		}