Merge pull request #1506 from prometheus/mdl-drbd-cleanup

collector: clean up DRBD collector, less global state

collector/drbd_linux.go

@@ -31,12 +31,14 @@ type drbdNumericalMetric struct {
 	multiplier float64
 }
 
-func newDRBDNumericalMetric(name string, desc string, valueType prometheus.ValueType, multiplier float64) drbdNumericalMetric {
+func newDRBDNumericalMetric(name, desc string, valueType prometheus.ValueType, multiplier float64) drbdNumericalMetric {
 	return drbdNumericalMetric{
 		desc: prometheus.NewDesc(
 			prometheus.BuildFQName(namespace, "drbd", name),
 			desc,
-			[]string{"device"}, nil),
+			[]string{"device"},
+			nil,
+		),
 		valueType:  valueType,
 		multiplier: multiplier,
 	}
@@ -45,114 +47,136 @@ func newDRBDNumericalMetric(name string, desc string, valueType prometheus.Value
 // String pair metric provided by /proc/drbd.
 type drbdStringPairMetric struct {
 	desc    *prometheus.Desc
-	valueOkay string
+	valueOK string
 }
 
-func (metric *drbdStringPairMetric) isOkay(value string) float64 {
+func (m *drbdStringPairMetric) isOkay(v string) float64 {
-	if value == metric.valueOkay {
+	if v == m.valueOK {
 		return 1
 	}
+
 	return 0
 }
 
-func newDRBDStringPairMetric(name string, desc string, valueOkay string) drbdStringPairMetric {
+func newDRBDStringPairMetric(name, desc, valueOK string) drbdStringPairMetric {
 	return drbdStringPairMetric{
 		desc: prometheus.NewDesc(
 			prometheus.BuildFQName(namespace, "drbd", name),
 			desc,
-			[]string{"device", "node"}, nil),
+			[]string{"device", "node"},
-		valueOkay: valueOkay,
+			nil,
+		),
+		valueOK: valueOK,
 	}
 }
 
-var (
+type drbdCollector struct {
-	drbdNumericalMetrics = map[string]drbdNumericalMetric{
+	numerical  map[string]drbdNumericalMetric
-		"ns": newDRBDNumericalMetric(
+	stringPair map[string]drbdStringPairMetric
-			"network_sent_bytes_total",
+	connected  *prometheus.Desc
-			"Total number of bytes sent via the network.",
+}
-			prometheus.CounterValue,
-			1024),
-		"nr": newDRBDNumericalMetric(
-			"network_received_bytes_total",
-			"Total number of bytes received via the network.",
-			prometheus.CounterValue,
-			1),
-		"dw": newDRBDNumericalMetric(
-			"disk_written_bytes_total",
-			"Net data written on local hard disk; in bytes.",
-			prometheus.CounterValue,
-			1024),
-		"dr": newDRBDNumericalMetric(
-			"disk_read_bytes_total",
-			"Net data read from local hard disk; in bytes.",
-			prometheus.CounterValue,
-			1024),
-		"al": newDRBDNumericalMetric(
-			"activitylog_writes_total",
-			"Number of updates of the activity log area of the meta data.",
-			prometheus.CounterValue,
-			1),
-		"bm": newDRBDNumericalMetric(
-			"bitmap_writes_total",
-			"Number of updates of the bitmap area of the meta data.",
-			prometheus.CounterValue,
-			1),
-		"lo": newDRBDNumericalMetric(
-			"local_pending",
-			"Number of open requests to the local I/O sub-system.",
-			prometheus.GaugeValue,
-			1),
-		"pe": newDRBDNumericalMetric(
-			"remote_pending",
-			"Number of requests sent to the peer, but that have not yet been answered by the latter.",
-			prometheus.GaugeValue,
-			1),
-		"ua": newDRBDNumericalMetric(
-			"remote_unacknowledged",
-			"Number of requests received by the peer via the network connection, but that have not yet been answered.",
-			prometheus.GaugeValue,
-			1),
-		"ap": newDRBDNumericalMetric(
-			"application_pending",
-			"Number of block I/O requests forwarded to DRBD, but not yet answered by DRBD.",
-			prometheus.GaugeValue,
-			1),
-		"ep": newDRBDNumericalMetric(
-			"epochs",
-			"Number of Epochs currently on the fly.",
-			prometheus.GaugeValue,
-			1),
-		"oos": newDRBDNumericalMetric(
-			"out_of_sync_bytes",
-			"Amount of data known to be out of sync; in bytes.",
-			prometheus.GaugeValue,
-			1024),
-	}
-	drbdStringPairMetrics = map[string]drbdStringPairMetric{
-		"ro": newDRBDStringPairMetric(
-			"node_role_is_primary",
-			"Whether the role of the node is in the primary state.",
-			"Primary"),
-		"ds": newDRBDStringPairMetric(
-			"disk_state_is_up_to_date",
-			"Whether the disk of the node is up to date.",
-			"UpToDate"),
-	}
-
-	drbdConnected = prometheus.NewDesc(
-		prometheus.BuildFQName(namespace, "drbd", "connected"),
-		"Whether DRBD is connected to the peer.",
-		[]string{"device"}, nil)
-)
-
-type drbdCollector struct{}
 
 func init() {
 	registerCollector("drbd", defaultDisabled, newDRBDCollector)
 }
 
 func newDRBDCollector() (Collector, error) {
-	return &drbdCollector{}, nil
+	return &drbdCollector{
+		numerical: map[string]drbdNumericalMetric{
+			"ns": newDRBDNumericalMetric(
+				"network_sent_bytes_total",
+				"Total number of bytes sent via the network.",
+				prometheus.CounterValue,
+				1024,
+			),
+			"nr": newDRBDNumericalMetric(
+				"network_received_bytes_total",
+				"Total number of bytes received via the network.",
+				prometheus.CounterValue,
+				1,
+			),
+			"dw": newDRBDNumericalMetric(
+				"disk_written_bytes_total",
+				"Net data written on local hard disk; in bytes.",
+				prometheus.CounterValue,
+				1024,
+			),
+			"dr": newDRBDNumericalMetric(
+				"disk_read_bytes_total",
+				"Net data read from local hard disk; in bytes.",
+				prometheus.CounterValue,
+				1024,
+			),
+			"al": newDRBDNumericalMetric(
+				"activitylog_writes_total",
+				"Number of updates of the activity log area of the meta data.",
+				prometheus.CounterValue,
+				1,
+			),
+			"bm": newDRBDNumericalMetric(
+				"bitmap_writes_total",
+				"Number of updates of the bitmap area of the meta data.",
+				prometheus.CounterValue,
+				1,
+			),
+			"lo": newDRBDNumericalMetric(
+				"local_pending",
+				"Number of open requests to the local I/O sub-system.",
+				prometheus.GaugeValue,
+				1,
+			),
+			"pe": newDRBDNumericalMetric(
+				"remote_pending",
+				"Number of requests sent to the peer, but that have not yet been answered by the latter.",
+				prometheus.GaugeValue,
+				1,
+			),
+			"ua": newDRBDNumericalMetric(
+				"remote_unacknowledged",
+				"Number of requests received by the peer via the network connection, but that have not yet been answered.",
+				prometheus.GaugeValue,
+				1,
+			),
+			"ap": newDRBDNumericalMetric(
+				"application_pending",
+				"Number of block I/O requests forwarded to DRBD, but not yet answered by DRBD.",
+				prometheus.GaugeValue,
+				1,
+			),
+			"ep": newDRBDNumericalMetric(
+				"epochs",
+				"Number of Epochs currently on the fly.",
+				prometheus.GaugeValue,
+				1,
+			),
+			"oos": newDRBDNumericalMetric(
+				"out_of_sync_bytes",
+				"Amount of data known to be out of sync; in bytes.",
+				prometheus.GaugeValue,
+				1024,
+			),
+		},
+
+		stringPair: map[string]drbdStringPairMetric{
+			"ro": newDRBDStringPairMetric(
+				"node_role_is_primary",
+				"Whether the role of the node is in the primary state.",
+				"Primary",
+			),
+			"ds": newDRBDStringPairMetric(
+				"disk_state_is_up_to_date",
+				"Whether the disk of the node is up to date.",
+				"UpToDate",
+			),
+		},
+
+		connected: prometheus.NewDesc(
+			prometheus.BuildFQName(namespace, "drbd", "connected"),
+			"Whether DRBD is connected to the peer.",
+			[]string{"device"},
+			nil,
+		),
+	}, nil
 }
 
 func (c *drbdCollector) Update(ch chan<- prometheus.Metric) error {
@@ -160,9 +184,10 @@ func (c *drbdCollector) Update(ch chan<- prometheus.Metric) error {
 	file, err := os.Open(statsFile)
 	if err != nil {
 		if os.IsNotExist(err) {
-			log.Debugf("Not collecting DRBD statistics, as %s does not exist: %s", statsFile, err)
+			log.Debugf("drbd: %s does not exist, skipping: %s", statsFile, err)
 			return nil
 		}
+
 		return err
 	}
 	defer file.Close()
@@ -170,44 +195,80 @@ func (c *drbdCollector) Update(ch chan<- prometheus.Metric) error {
 	scanner := bufio.NewScanner(file)
 	scanner.Split(bufio.ScanWords)
 	device := "unknown"
+
 	for scanner.Scan() {
 		field := scanner.Text()
-		if kv := strings.Split(field, ":"); len(kv) == 2 {
+
+		kv := strings.Split(field, ":")
+		if len(kv) != 2 {
+			log.Debugf("drbd: skipping invalid key:value pair %q", field)
+			continue
+		}
+
 		if id, err := strconv.ParseUint(kv[0], 10, 64); err == nil && kv[1] == "" {
+			// New DRBD device encountered.
 			device = fmt.Sprintf("drbd%d", id)
-			} else if metric, ok := drbdNumericalMetrics[kv[0]]; ok {
+			continue
+		}
+
+		if m, ok := c.numerical[kv[0]]; ok {
 			// Numerical value.
-				value, err := strconv.ParseFloat(kv[1], 64)
+			v, err := strconv.ParseFloat(kv[1], 64)
 			if err != nil {
 				return err
 			}
+
 			ch <- prometheus.MustNewConstMetric(
-					metric.desc, metric.valueType,
+				m.desc,
-					value*metric.multiplier, device)
+				m.valueType,
-			} else if metric, ok := drbdStringPairMetrics[kv[0]]; ok {
+				v*m.multiplier,
+				device,
+			)
+
+			continue
+		}
+
+		if m, ok := c.stringPair[kv[0]]; ok {
 			// String pair value.
 			values := strings.Split(kv[1], "/")
 			ch <- prometheus.MustNewConstMetric(
-					metric.desc, prometheus.GaugeValue,
+				m.desc,
-					metric.isOkay(values[0]), device, "local")
+				prometheus.GaugeValue,
+				m.isOkay(values[0]),
+				device,
+				"local",
+			)
+
 			ch <- prometheus.MustNewConstMetric(
-					metric.desc, prometheus.GaugeValue,
+				m.desc,
-					metric.isOkay(values[1]), device, "remote")
+				prometheus.GaugeValue,
-			} else if kv[0] == "cs" {
+				m.isOkay(values[1]),
+				device,
+				"remote",
+			)
+
+			continue
+		}
+
+		if kv[0] == "cs" {
 			// Connection state.
 			var connected float64
 			if kv[1] == "Connected" {
 				connected = 1
 			}
+
 			ch <- prometheus.MustNewConstMetric(
-					drbdConnected, prometheus.GaugeValue,
+				c.connected,
-					connected, device)
+				prometheus.GaugeValue,
-			} else {
+				connected,
-				log.Debugf("Don't know how to process key-value pair [%s: %q]", kv[0], kv[1])
+				device,
-			}
+			)
-		} else {
+
-			log.Debugf("Don't know how to process string %q", field)
+			continue
 		}
+
+		log.Debugf("drbd: unhandled key-value pair: [%s: %q]", kv[0], kv[1])
 	}
+
 	return scanner.Err()
 }