Restructure implementation without zfsMetricProvider.

collector/zfs.go

@@ -31,12 +31,22 @@ type zfsMetric struct {
 	sysctl    zfsSysctl        // The sysctl of the ZFS metric.
 }
 
-func (m *zfsMetric) BuildFQName() string {
-	return prometheus.BuildFQName(Namespace, string(m.subsystem), m.name)
+func (m *zfsMetric) ConstMetric(value zfsMetricValue) prometheus.Metric {
+	return prometheus.MustNewConstMetric(
+		prometheus.NewDesc(
+			prometheus.BuildFQName(Namespace, string(m.subsystem), m.name),
+			m.name,
+			nil,
+			nil,
+		),
+		prometheus.UntypedValue,
+		float64(value),
+	)
 }
 
-func (m *zfsMetric) HelpString() string {
-	return m.name
+type datasetMetric struct {
+	subsystem zfsSubsystemName
+	name      string
 }
 
 // Collector
@@ -65,10 +75,7 @@ func NewZFSCollector() (Collector, error) {
 
 func (c *zfsCollector) Update(ch chan<- prometheus.Metric) (err error) {
 
-	metricProvider := NewZFSMetricProvider()
-
-	log.Debug("Preparing metrics update")
-	err = metricProvider.PrepareUpdate()
+	err = c.PrepareUpdate()
 	switch {
 	case err == zfsNotAvailableError:
 		log.Debug(err)
@@ -77,56 +84,12 @@ func (c *zfsCollector) Update(ch chan<- prometheus.Metric) (err error) {
 		return err
 	}
 
-	log.Debugf("Fetching %d metrics", len(c.zfsMetrics))
-	for _, metric := range c.zfsMetrics {
-
-		value, err := metricProvider.Value(metric.sysctl)
-		if err != nil {
-			return err
-		}
-
-		ch <- prometheus.MustNewConstMetric(
-			prometheus.NewDesc(
-				metric.BuildFQName(),
-				metric.HelpString(),
-				nil,
-				nil,
-			),
-			prometheus.UntypedValue,
-			float64(value),
-		)
+	// Arcstats
 
+	err = c.updateArcstats(ch)
+	if err != nil {
+		return err
 	}
 
 	return err
 }
-
-// Metrics Provider
-// Platform-dependend parts implemented in zfs_${os} files.
-
-type zfsMetricProvider struct {
-	values map[zfsSysctl]zfsMetricValue
-}
-
-func NewZFSMetricProvider() zfsMetricProvider {
-	return zfsMetricProvider{
-		values: make(map[zfsSysctl]zfsMetricValue),
-	}
-
-}
-
-func (p *zfsMetricProvider) Value(s zfsSysctl) (value zfsMetricValue, err error) {
-
-	var ok bool
-	value = zfsErrorValue
-
-	value, ok = p.values[s]
-	if !ok {
-		value, err = p.handleMiss(s)
-		if err != nil {
-			return value, err
-		}
-	}
-
-	return value, err
-}

collector/zfs_freebsd.go

@@ -3,6 +3,8 @@ package collector
 import (
 	"fmt"
 	"unsafe"
+
+	"github.com/prometheus/client_golang/prometheus"
 )
 
 /*
@@ -36,24 +38,28 @@ int zfsModuleLoaded() {
 */
 import "C"
 
-func (c *zfsMetricProvider) PrepareUpdate() error {
+func (c *zfsCollector) PrepareUpdate() error {
 	if C.zfsModuleLoaded() == 0 {
 		return zfsNotAvailableError
 	}
 	return nil
 }
 
-func (p *zfsMetricProvider) handleMiss(s zfsSysctl) (zfsMetricValue, error) {
+func (c *zfsCollector) updateArcstats(ch chan<- prometheus.Metric) (err error) {
 
-	sysctlCString := C.CString(string(s))
-	defer C.free(unsafe.Pointer(sysctlCString))
+	for _, metric := range c.zfsMetrics {
 
-	value := int(C.zfsIntegerSysctl(sysctlCString))
+		sysctlCString := C.CString(string(metric.sysctl))
+		defer C.free(unsafe.Pointer(sysctlCString))
 
-	if value == -1 {
-		return zfsErrorValue, fmt.Errorf("Could not retrieve sysctl '%s'", s)
-	}
+		value := int(C.zfsIntegerSysctl(sysctlCString))
 
-	return zfsMetricValue(value), nil
+		if value == -1 {
+			return fmt.Errorf("Could not retrieve value for metric '%v'", metric)
+		}
+
+		ch <- metric.ConstMetric(zfsMetricValue(value))
+	}
 
+	return err
 }

collector/zfs_linux.go

@@ -9,38 +9,52 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/log"
 )
 
-const zfsArcstatsProcpath = "spl/kstat/zfs/arcstats"
-
-func (p *zfsMetricProvider) PrepareUpdate() (err error) {
+const (
+	zfsArcstatsProcpath = "spl/kstat/zfs/arcstats"
+)
 
-	err = p.prepareUpdateArcstats(zfsArcstatsProcpath)
+func (c *zfsCollector) PrepareUpdate() (err error) {
+	file, err := c.openArcstatsFile()
 	if err != nil {
-		return
+		file.Close()
 	}
-	return nil
+	return err
 }
 
-func (p *zfsMetricProvider) handleMiss(s zfsSysctl) (value zfsMetricValue, err error) {
-	// all values are fetched in PrepareUpdate().
-	return zfsErrorValue, fmt.Errorf("sysctl '%s' found")
+func (c *zfsCollector) openArcstatsFile() (file *os.File, err error) {
+	file, err = os.Open(procFilePath(zfsArcstatsProcpath))
+	if err != nil {
+		log.Debugf("Cannot open '%s' for reading.Is the kernel module loaded?", procFilePath(zfsArcstatsProcpath))
+		err = zfsNotAvailableError
+	}
+	return
 }
 
-func (p *zfsMetricProvider) prepareUpdateArcstats(zfsArcstatsProcpath string) (err error) {
+func (c *zfsCollector) updateArcstats(ch chan<- prometheus.Metric) (err error) {
 
-	file, err := os.Open(procFilePath(zfsArcstatsProcpath))
-	if err != nil {
-		log.Debugf("Cannot open ZFS arcstats procfs file for reading. " +
-			" Is the kernel module loaded?")
-		return zfsNotAvailableError
-	}
+	file, err := c.openArcstatsFile()
 	defer file.Close()
-	return p.parseArcstatsProcfsFile(file)
+
+	return c.parseArcstatsProcfsFile(file, func(s zfsSysctl, v zfsMetricValue) {
+		// TODO: Find corresponding metric in a more efficient way
+		for _, metric := range c.zfsMetrics {
+			if metric.subsystem != arc {
+				continue
+			}
+			if metric.sysctl != s {
+				continue
+			}
+			ch <- metric.ConstMetric(v)
+		}
+	})
+
 }
 
-func (p *zfsMetricProvider) parseArcstatsProcfsFile(reader io.Reader) (err error) {
+func (c *zfsCollector) parseArcstatsProcfsFile(reader io.Reader, handler func(zfsSysctl, zfsMetricValue)) (err error) {
 
 	scanner := bufio.NewScanner(reader)
 
@@ -65,7 +79,7 @@ func (p *zfsMetricProvider) parseArcstatsProcfsFile(reader io.Reader) (err error
 			return fmt.Errorf("could not parse expected integer value for '%s'", key)
 		}
 		log.Debugf("%s = %d", key, value)
-		p.values[zfsSysctl(key)] = zfsMetricValue(value)
+		handler(zfsSysctl(key), zfsMetricValue(value))
 	}
 	if !parseLine {
 		return errors.New("did not parse a single arcstat metrics")

collector/zfs_linux_test.go

@@ -13,21 +13,32 @@ func TestArcstatsParsing(t *testing.T) {
 	}
 	defer arcstatsFile.Close()
 
-	p := NewZFSMetricProvider()
-	err = p.parseArcstatsProcfsFile(arcstatsFile)
-
+	c := zfsCollector{}
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	value, err := p.Value(zfsSysctl("kstat.zfs.misc.arcstats.hits"))
+	handlerCalled := false
+	err = c.parseArcstatsProcfsFile(arcstatsFile, func(s zfsSysctl, v zfsMetricValue) {
+
+		if s != zfsSysctl("kstat.zfs.misc.arcstats.hits") {
+			return
+		}
+
+		handlerCalled = true
+
+		if v != zfsMetricValue(8772612) {
+			t.Fatalf("Incorrect value parsed from procfs data")
+		}
+
+	})
 
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if value != zfsMetricValue(8772612) {
-		t.Fatalf("Incorrect value parsed from procfs data")
+	if !handlerCalled {
+		t.Fatal("Arcstats parsing handler was not called for some expected sysctls")
 	}
 
 }