Repository: ClusterLabs/ha_cluster_exporter
Pull request #123 (2020-02-05 09:46:00)

Refactor metrics
This PR addresses some of the feedback we got in prometheus/docs#1540, which is part of the submission of this package in the [curated list of third party exporters](https://prometheus.io/docs/instrumenting/exporters/).

- **BC Break**: The following metrics have been removed, because they were mostly redundant, and their name broke Prometheus conventions:
  - `ha_cluster_pacemaker_nodes_total`
  - `ha_cluster_pacemaker_resources_total`
  - `ha_cluster_sbd_devices_total`
- **BC Break**: The `ha_cluster_corosync_ring_errors_total` has been renamed to `ha_cluster_corosync_ring_errors` to comply with Prometheus naming conventions.
- **BC Break**: The `ha_cluster_sbd_device_status` has been renamed to `ha_cluster_sbd_devices`, which now works similarly to other metrics that represent entities in the cluster: there will be one line per SBD device valued `1`, and the status is a label.
- refactored the creation of the metric descriptors in a `setDescriptor()` function targeting the `DefaultCollector` struct. This got rid of some global state and a whole lot of repetition and ambiguity in the previous metric description instantiation.
- the metric subsystem prefix is now initialized just once, within the collector structs.

Any PromQL query that took advantage of the removed `*_total` metrics can be easily adapted by removing the `_total` suffix from the metric name and using the `count()` function.

While we understand that these changes involve some backwards compatibility breakages that may result annoying to our users, we feel that, as we approach our first 1.0 stable release, addressing the feedback we got from the Prometheus maintainers is very important for the project.


Patch:
diff --git a/README.md b/README.md
index 25963a0..114061a 100644
--- a/README.md
+++ b/README.md
@@ -48,7 +48,7 @@ go get github.com/ClusterLabs/ha_cluster_exporter
 ```
 
 ### RPM
-You can find the repositories for RPM based distributions in [SUSE's Open Build Service](https://build.opensuse.org/repositories/server:monitoring/prometheus-ha_cluster_exporter).  
+You can find the repositories for RPM based distributions in [SUSE's Open Build Service](https://build.opensuse.org/package/show/server:monitoring/prometheus-ha_cluster_exporter).  
 On openSUSE or SUSE Linux Enterprise you can just use the `zypper` system package manager:
 ```shell
 export DISTRO=SLE_15_SP1 # change as desired
diff --git a/corosync_metrics.go b/corosync_metrics.go
index 2e69c34..5cd321b 100644
--- a/corosync_metrics.go
+++ b/corosync_metrics.go
@@ -12,29 +12,24 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-var (
-	corosyncMetrics = metricDescriptors{
-		// the map key will function as an identifier of the metric throughout the rest of the code;
-		// it is arbitrary, but by convention we use the actual metric name
-		"quorate":           NewMetricDesc("corosync", "quorate", "Whether or not the cluster is quorate", nil),
-		"ring_errors_total": NewMetricDesc("corosync", "ring_errors_total", "Total number of corosync ring errors", nil),
-		"quorum_votes":      NewMetricDesc("corosync", "quorum_votes", "Cluster quorum votes; one line per type", []string{"type"}),
-	}
-)
-
 func NewCorosyncCollector(cfgToolPath string, quorumToolPath string) (*corosyncCollector, error) {
 	err := CheckExecutables(cfgToolPath, quorumToolPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not initialize Corosync collector")
 	}
 
-	return &corosyncCollector{
+	collector := &corosyncCollector{
 		DefaultCollector{
-			metrics: corosyncMetrics,
+			subsystem: "corosync",
 		},
 		cfgToolPath,
 		quorumToolPath,
-	}, nil
+	}
+	collector.setDescriptor("quorate", "Whether or not the cluster is quorate", nil)
+	collector.setDescriptor("ring_errors", "The number of corosync ring errors", nil)
+	collector.setDescriptor("quorum_votes", "Cluster quorum votes; one line per type", []string{"type"})
+
+	return collector, nil
 }
 
 type corosyncCollector struct {
@@ -72,7 +67,7 @@ func (c *corosyncCollector) collectRingErrorsTotal(ch chan<- prometheus.Metric)
 		return errors.Wrap(err, "cannot parse ring status")
 	}
 
-	ch <- c.makeGaugeMetric("ring_errors_total", float64(ringErrorsTotal))
+	ch <- c.makeGaugeMetric("ring_errors", float64(ringErrorsTotal))
 
 	return nil
 }
diff --git a/doc/metrics.md b/doc/metrics.md
index 989d140..25436a1 100644
--- a/doc/metrics.md
+++ b/doc/metrics.md
@@ -25,14 +25,12 @@ The Pacemaker subsystem collects an atomic snapshot of the HA cluster directly f
 
 0. [Sample](../test/pacemaker.metrics)
 1. [`ha_cluster_pacemaker_config_last_change`](#ha_cluster_pacemaker_config_last_change)
-3. [`ha_cluster_pacemaker_fail_count`](#ha_cluster_pacemaker_fail_count)
-2. [`ha_cluster_pacemaker_location_constraints`](#ha_cluster_pacemaker_location_constraints)
+2. [`ha_cluster_pacemaker_fail_count`](#ha_cluster_pacemaker_fail_count)
+3. [`ha_cluster_pacemaker_location_constraints`](#ha_cluster_pacemaker_location_constraints)
 4. [`ha_cluster_pacemaker_migration_threshold`](#ha_cluster_pacemaker_migration_threshold)
-5. [`ha_cluster_pacemaker_nodes_total`](#ha_cluster_pacemaker_nodes_total)
-6. [`ha_cluster_pacemaker_nodes`](#ha_cluster_pacemaker_nodes)
-7. [`ha_cluster_pacemaker_resources_total`](#ha_cluster_pacemaker_resources_total)
-8. [`ha_cluster_pacemaker_resources`](#ha_cluster_pacemaker_resources)
-9. [`ha_cluster_pacemaker_stonith_enabled`](#ha_cluster_pacemaker_stonith_enabled)
+5. [`ha_cluster_pacemaker_nodes`](#ha_cluster_pacemaker_nodes)
+6. [`ha_cluster_pacemaker_resources`](#ha_cluster_pacemaker_resources)
+7. [`ha_cluster_pacemaker_stonith_enabled`](#ha_cluster_pacemaker_stonith_enabled)
 
 
 ### `ha_cluster_pacemaker_config_last_change`
@@ -92,13 +90,6 @@ Either the value is `1`, or the line is absent altogether.
 The total number of lines for this metric will be the cardinality of `name` times the cardinality of `status`.
 
 
-### `ha_cluster_pacemaker_nodes_total` 
-
-#### Description
-
-The total number of *configured* nodes in the cluster. This value is mostly static and *does not* take into account the status of the nodes. It only changes when the Pacemaker configuration changes.
-
-
 ### `ha_cluster_pacemaker_resources` 
 
 #### Description
@@ -117,13 +108,6 @@ Either the value is `1`, or the line is absent altogether.
 The total number of lines for this metric will be the cardinality of `id` times the cardinality of `status`.
 
 
-### `ha_cluster_pacemaker_resources_total` 
-
-#### Description
-
-The total number of *configured* resources in the cluster. This value is mostly static and *does not* take into account the status of the resources. It only changes when the Pacemaker configuration changes.
-
-
 ### `ha_cluster_pacemaker_stonith_enabled`
 
 #### Description
@@ -139,7 +123,7 @@ The Corosync subsystem collects cluster quorum votes and ring status by parsing
 0. [Sample](../test/corosync.metrics)
 1. [`ha_cluster_corosync_quorate`](#ha_cluster_corosync_quorate)
 2. [`ha_cluster_corosync_quorum_votes`](#ha_cluster_corosync_quorum_votes)
-3. [`ha_cluster_corosync_ring_errors_total`](#ha_cluster_corosync_ring_errors_total)
+3. [`ha_cluster_corosync_ring_errors`](#ha_cluster_corosync_ring_errors)
 
 
 ### `ha_cluster_corosync_quorate`
@@ -161,44 +145,35 @@ Cluster quorum votes; one line per type.
 - `type`: one of `expected_votes|highest_expected|total_votes|quorum`
 
 
-### `ha_cluster_corosync_ring_errors_total`
+### `ha_cluster_corosync_ring_errors`
 
 #### Description
 
-Total number of corosync ring errors.
+The number of corosync ring errors.
 
 
 ## SBD
 
-The SBD subsystems collect devices stats by parsing its configuration the output of `sbd --dump`.
+The SBD subsystems collect devices stats by parsing its configuration and the output of `sbd --dump`.
 
 0. [Sample](../test/sbd.metrics)
-1. [`ha_cluster_sbd_device_status`](#ha_cluster_sbd_device_status)
-2. [`ha_cluster_sbd_devices_total`](#ha_cluster_sbd_devices_total)
+2. [`ha_cluster_sbd_devices`](#ha_cluster_sbd_devices)
 
-
-### `ha_cluster_sbd_device_status`
+### `ha_cluster_sbd_devices`
 
 #### Description
 
-Whether or not an SBD device is healthy. One line per `device`.  
-Value is either `1` or `0`.
+The SBD devices in the cluster; one line per device.  
+Either the value is `1`, or the line is absent altogether.
 
 #### Labels
 
-- `device`: the path of the device.
+- `device`: the path of the SBD device
+- `status`: one of `healthy|unhealthy`
 
 The total number of lines for this metric will be the cardinality of `device`.
 
 
-### `ha_cluster_sbd_devices_total`
-
-#### Description
-
-Total count of configured SBD devices.  
-Value is an integer greater than or equal to `0`.
-
-
 ## DRBD
 
 The DRBD subsystems collect devices stats by parsing its configuration the JSON output of `drbdsetup`.
diff --git a/drbd_metrics.go b/drbd_metrics.go
index d4f3fe6..c62005e 100644
--- a/drbd_metrics.go
+++ b/drbd_metrics.go
@@ -43,41 +43,37 @@ type drbdStatus struct {
 	} `json:"connections"`
 }
 
-var (
-	drbdMetrics = metricDescriptors{
-		// the map key will function as an identifier of the metric throughout the rest of the code;
-		// it is arbitrary, but by convention we use the actual metric name
-		"resources":            NewMetricDesc("drbd", "resources", "The DRBD resources; 1 line per name, per volume", []string{"resource", "role", "volume", "disk_state"}),
-		"written":              NewMetricDesc("drbd", "written", "KiB written to DRBD; 1 line per res, per volume", []string{"resource", "volume"}),
-		"read":                 NewMetricDesc("drbd", "read", "KiB read from DRBD; 1 line per res, per volume", []string{"resource", "volume"}),
-		"al_writes":            NewMetricDesc("drbd", "al_writes", "Writes to activity log; 1 line per res, per volume", []string{"resource", "volume"}),
-		"bm_writes":            NewMetricDesc("drbd", "bm_writes", "Writes to bitmap; 1 line per res, per volume", []string{"resource", "volume"}),
-		"upper_pending":        NewMetricDesc("drbd", "upper_pending", "Upper pending; 1 line per res, per volume", []string{"resource", "volume"}),
-		"lower_pending":        NewMetricDesc("drbd", "lower_pending", "Lower pending; 1 line per res, per volume", []string{"resource", "volume"}),
-		"quorum":               NewMetricDesc("drbd", "quorum", "Quorum status per resource and per volume", []string{"resource", "volume"}),
-		"connections":          NewMetricDesc("drbd", "connections", "The DRBD resource connections; 1 line per per resource, per peer_node_id", []string{"resource", "peer_node_id", "peer_role", "volume", "peer_disk_state"}),
-		"connections_sync":     NewMetricDesc("drbd", "connections_sync", "The in sync percentage value for DRBD resource connections", []string{"resource", "peer_node_id", "volume"}),
-		"connections_received": NewMetricDesc("drbd", "connections_received", "KiB received per connection", []string{"resource", "peer_node_id", "volume"}),
-		"connections_sent":     NewMetricDesc("drbd", "connections_sent", "KiB sent per connection", []string{"resource", "peer_node_id", "volume"}),
-		"connections_pending":  NewMetricDesc("drbd", "connections_pending", "Pending value per connection", []string{"resource", "peer_node_id", "volume"}),
-		"connections_unacked":  NewMetricDesc("drbd", "connections_unacked", "Unacked value per connection", []string{"resource", "peer_node_id", "volume"}),
-		"split_brain":          NewMetricDesc("drbd", "split_brain", "Whether a split brain has been detected; 1 line per resource, per volume.", []string{"resource", "volume"}),
-	}
-)
-
 func NewDrbdCollector(drbdSetupPath string, drbdSplitBrainPath string) (*drbdCollector, error) {
 	err := CheckExecutables(drbdSetupPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not initialize DRBD collector")
 	}
 
-	return &drbdCollector{
+	collector := &drbdCollector{
 		DefaultCollector{
-			metrics: drbdMetrics,
+			subsystem: "drbd",
 		},
 		drbdSetupPath,
 		drbdSplitBrainPath,
-	}, nil
+	}
+
+	collector.setDescriptor("resources", "The DRBD resources; 1 line per name, per volume", []string{"resource", "role", "volume", "disk_state"})
+	collector.setDescriptor("written", "KiB written to DRBD; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("read", "KiB read from DRBD; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("al_writes", "Writes to activity log; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("bm_writes", "Writes to bitmap; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("upper_pending", "Upper pending; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("lower_pending", "Lower pending; 1 line per res, per volume", []string{"resource", "volume"})
+	collector.setDescriptor("quorum", "Quorum status per resource and per volume", []string{"resource", "volume"})
+	collector.setDescriptor("connections", "The DRBD resource connections; 1 line per per resource, per peer_node_id", []string{"resource", "peer_node_id", "peer_role", "volume", "peer_disk_state"})
+	collector.setDescriptor("connections_sync", "The in sync percentage value for DRBD resource connections", []string{"resource", "peer_node_id", "volume"})
+	collector.setDescriptor("connections_received", "KiB received per connection", []string{"resource", "peer_node_id", "volume"})
+	collector.setDescriptor("connections_sent", "KiB sent per connection", []string{"resource", "peer_node_id", "volume"})
+	collector.setDescriptor("connections_pending", "Pending value per connection", []string{"resource", "peer_node_id", "volume"})
+	collector.setDescriptor("connections_unacked", "Unacked value per connection", []string{"resource", "peer_node_id", "volume"})
+	collector.setDescriptor("split_brain", "Whether a split brain has been detected; 1 line per resource, per volume.", []string{"resource", "volume"})
+
+	return collector, nil
 }
 
 type drbdCollector struct {
diff --git a/ha_cluster_exporter.go b/ha_cluster_exporter.go
index a07c8df..965c317 100644
--- a/ha_cluster_exporter.go
+++ b/ha_cluster_exporter.go
@@ -26,32 +26,48 @@ func (SystemClock) Now() time.Time {
 	return time.Now()
 }
 
-type metricDescriptors map[string]*prometheus.Desc
-
 type DefaultCollector struct {
-	metrics metricDescriptors
+	subsystem   string
+	descriptors map[string]*prometheus.Desc
+}
+
+func (c *DefaultCollector) getDescriptor(name string) *prometheus.Desc {
+	desc, ok := c.descriptors[name]
+	if !ok {
+		// we hard panic on this because it's most certainly a coding error
+		panic(errors.Errorf("undeclared metric '%s'", name))
+	}
+	return desc
+}
+
+// Convenience wrapper around prometheus.NewDesc constructor.
+// Stores a metric descriptor with a fully qualified name like `NAMESPACE_subsystem_name`.
+// `name` is the last and most relevant part of the metrics Full Qualified Name;
+// `help` is the message displayed in the HELP line
+// `variableLabels` is a list of labels to declare. Use `nil` to declare no labels.
+func (c *DefaultCollector) setDescriptor(name, help string, variableLabels []string) {
+	if c.descriptors == nil {
+		c.descriptors = make(map[string]*prometheus.Desc)
+	}
+	c.descriptors[name] = prometheus.NewDesc(prometheus.BuildFQName(NAMESPACE, c.subsystem, name), help, variableLabels, nil)
 }
 
 func (c *DefaultCollector) Describe(ch chan<- *prometheus.Desc) {
-	for _, metric := range c.metrics {
-		ch <- metric
+	for _, descriptor := range c.descriptors {
+		ch <- descriptor
 	}
 }
 
-func (c *DefaultCollector) makeGaugeMetric(metricKey string, value float64, labelValues ...string) prometheus.Metric {
-	return c.makeMetric(metricKey, value, prometheus.GaugeValue, labelValues...)
+func (c *DefaultCollector) makeGaugeMetric(name string, value float64, labelValues ...string) prometheus.Metric {
+	return c.makeMetric(name, value, prometheus.GaugeValue, labelValues...)
 }
 
-func (c *DefaultCollector) makeCounterMetric(metricKey string, value float64, labelValues ...string) prometheus.Metric {
-	return c.makeMetric(metricKey, value, prometheus.CounterValue, labelValues...)
+func (c *DefaultCollector) makeCounterMetric(name string, value float64, labelValues ...string) prometheus.Metric {
+	return c.makeMetric(name, value, prometheus.CounterValue, labelValues...)
 }
 
-func (c *DefaultCollector) makeMetric(metricKey string, value float64, valueType prometheus.ValueType, labelValues ...string) prometheus.Metric {
-	desc, ok := c.metrics[metricKey]
-	if !ok {
-		// we hard panic on this because it's most certainly a coding error
-		panic(errors.Errorf("undeclared metric '%s'", metricKey))
-	}
+func (c *DefaultCollector) makeMetric(name string, value float64, valueType prometheus.ValueType, labelValues ...string) prometheus.Metric {
+	desc := c.getDescriptor(name)
 	metric := prometheus.MustNewConstMetric(desc, valueType, value, labelValues...)
 	if config.GetBool("enable-timestamps") {
 		metric = prometheus.NewMetricWithTimestamp(clock.Now(), metric)
@@ -59,16 +75,6 @@ func (c *DefaultCollector) makeMetric(metricKey string, value float64, valueType
 	return metric
 }
 
-// Convenience wrapper around Prometheus constructors.
-// Produces a metric with name `NAMESPACE_subsystem_name`.
-// `NAMESPACE` is a global project constant;
-// `subsystem` is an arbitrary name used to group related metrics under the same name prefix;
-// `name` is the last and most relevant part of the metrics Full Qualified Name;
-// `variableLabels` is a list of labels to declare. Use `nil` to declare no labels.
-func NewMetricDesc(subsystem, name, help string, variableLabels []string) *prometheus.Desc {
-	return prometheus.NewDesc(prometheus.BuildFQName(NAMESPACE, subsystem, name), help, variableLabels, nil)
-}
-
 // check that all the given paths exist and are executable files
 func CheckExecutables(paths ...string) error {
 	for _, path := range paths {
diff --git a/pacemaker_metrics.go b/pacemaker_metrics.go
index 7364fbb..2f95b08 100644
--- a/pacemaker_metrics.go
+++ b/pacemaker_metrics.go
@@ -94,35 +94,26 @@ type CIB struct {
 	} `xml:"configuration"`
 }
 
-var (
-	pacemakerMetrics = metricDescriptors{
-		// the map key will function as an identifier of the metric throughout the rest of the code;
-		// it is arbitrary, but by convention we use the actual metric name
-		"nodes":                NewMetricDesc("pacemaker", "nodes", "The nodes in the cluster; one line per name, per status", []string{"node", "type", "status"}),
-		"nodes_total":          NewMetricDesc("pacemaker", "nodes_total", "Total number of nodes in the cluster", nil),
-		"resources":            NewMetricDesc("pacemaker", "resources", "The resources in the cluster; one line per id, per status", []string{"node", "resource", "role", "managed", "status"}),
-		"resources_total":      NewMetricDesc("pacemaker", "resources_total", "Total number of resources in the cluster", nil),
-		"stonith_enabled":      NewMetricDesc("pacemaker", "stonith_enabled", "Whether or not stonith is enabled", nil),
-		"fail_count":           NewMetricDesc("pacemaker", "fail_count", "The Fail count number per node and resource id", []string{"node", "resource"}),
-		"migration_threshold":  NewMetricDesc("pacemaker", "migration_threshold", "The migration_threshold number per node and resource id", []string{"node", "resource"}),
-		"config_last_change":   NewMetricDesc("pacemaker", "config_last_change", "The timestamp of the last change of the cluster configuration", nil),
-		"location_constraints": NewMetricDesc("pacemaker", "location_constraints", "Resource location constraints. The value indicates the score.", []string{"constraint", "node", "resource", "role"}),
-	}
-)
-
 func NewPacemakerCollector(crmMonPath string, cibAdminPath string) (*pacemakerCollector, error) {
 	err := CheckExecutables(crmMonPath, cibAdminPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not initialize Pacemaker collector")
 	}
 
-	return &pacemakerCollector{
-		DefaultCollector{
-			metrics: pacemakerMetrics,
-		},
+	collector := &pacemakerCollector{
+		DefaultCollector{subsystem: "pacemaker"},
 		crmMonPath,
 		cibAdminPath,
-	}, nil
+	}
+	collector.setDescriptor("nodes", "The nodes in the cluster; one line per name, per status", []string{"node", "type", "status"})
+	collector.setDescriptor("resources", "The resources in the cluster; one line per id, per status", []string{"node", "resource", "role", "managed", "status"})
+	collector.setDescriptor("stonith_enabled", "Whether or not stonith is enabled", nil)
+	collector.setDescriptor("fail_count", "The Fail count number per node and resource id", []string{"node", "resource"})
+	collector.setDescriptor("migration_threshold", "The migration_threshold number per node and resource id", []string{"node", "resource"})
+	collector.setDescriptor("config_last_change", "The timestamp of the last change of the cluster configuration", nil)
+	collector.setDescriptor("location_constraints", "Resource location constraints. The value indicates the score.", []string{"constraint", "node", "resource", "role"})
+
+	return collector, nil
 }
 
 type pacemakerCollector struct {
@@ -151,8 +142,6 @@ func (c *pacemakerCollector) Collect(ch chan<- prometheus.Metric) {
 		stonithEnabled = 1
 	}
 
-	ch <- c.makeGaugeMetric("nodes_total", float64(pacemakerStatus.Summary.Nodes.Number))
-	ch <- c.makeGaugeMetric("resources_total", float64(pacemakerStatus.Summary.Resources.Number))
 	ch <- c.makeGaugeMetric("stonith_enabled", stonithEnabled)
 
 	c.recordNodeMetrics(pacemakerStatus, ch)
diff --git a/sbd_metrics.go b/sbd_metrics.go
index 441189b..68fd3cd 100644
--- a/sbd_metrics.go
+++ b/sbd_metrics.go
@@ -13,17 +13,8 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-const SBD_STATUS_UNHEALTHY = 0
-const SBD_STATUS_HEALTHY = 1
-
-var (
-	sbdMetrics = metricDescriptors{
-		// the map key will function as an identifier of the metric throughout the rest of the code;
-		// it is arbitrary, but by convention we use the actual metric name
-		"device_status": NewMetricDesc("sbd", "device_status", "Whether or not an SBD device is healthy; one line per device", []string{"device"}),
-		"devices_total": NewMetricDesc("sbd", "devices_total", "Total count of configured SBD devices", nil),
-	}
-)
+const SBD_STATUS_UNHEALTHY = "unhealthy"
+const SBD_STATUS_HEALTHY = "healthy"
 
 func NewSbdCollector(sbdPath string, sbdConfigPath string) (*sbdCollector, error) {
 	err := CheckExecutables(sbdPath)
@@ -35,13 +26,17 @@ func NewSbdCollector(sbdPath string, sbdConfigPath string) (*sbdCollector, error
 		return nil, errors.Errorf("could not initialize SBD collector: '%s' does not exist", sbdConfigPath)
 	}
 
-	return &sbdCollector{
+	collector := &sbdCollector{
 		DefaultCollector{
-			metrics: sbdMetrics,
+			subsystem: "sbd",
 		},
 		sbdPath,
 		sbdConfigPath,
-	}, nil
+	}
+
+	collector.setDescriptor("devices", "SBD devices; one line per device", []string{"device", "status"})
+
+	return collector, nil
 }
 
 type sbdCollector struct {
@@ -60,11 +55,10 @@ func (c *sbdCollector) Collect(ch chan<- prometheus.Metric) {
 	}
 
 	sbdDevices := getSbdDevices(sbdConfiguration)
-	ch <- c.makeGaugeMetric("devices_total", float64(len(sbdDevices)))
 
 	sbdStatuses := c.getSbdDeviceStatuses(sbdDevices)
 	for sbdDev, sbdStatus := range sbdStatuses {
-		ch <- c.makeGaugeMetric("device_status", sbdStatus, sbdDev)
+		ch <- c.makeGaugeMetric("devices", 1, sbdDev, sbdStatus)
 	}
 }
 
@@ -107,8 +101,8 @@ func getSbdDevices(sbdConfigRaw []byte) []string {
 
 // this function takes a list of sbd devices and returns
 // a map of SBD device names with 1 if healthy, 0 if not
-func (c *sbdCollector) getSbdDeviceStatuses(sbdDevices []string) map[string]float64 {
-	sbdStatuses := make(map[string]float64)
+func (c *sbdCollector) getSbdDeviceStatuses(sbdDevices []string) map[string]string {
+	sbdStatuses := make(map[string]string)
 	for _, sbdDev := range sbdDevices {
 		_, err := exec.Command(c.sbdPath, "-d", sbdDev, "dump").Output()
 


Test patch:
diff --git a/ha_cluster_exporter_test.go b/ha_cluster_exporter_test.go
index 288e218..3ff1448 100644
--- a/ha_cluster_exporter_test.go
+++ b/ha_cluster_exporter_test.go
@@ -37,15 +37,12 @@ func expectMetrics(t *testing.T, c prometheus.Collector, fixture string) {
 }
 
 func TestMetricFactory(t *testing.T) {
-	SUT := &DefaultCollector{
-		metrics: metricDescriptors{
-			"test_metric": NewMetricDesc("test", "metric", "", nil),
-		},
-	}
+	SUT := &DefaultCollector{}
+	SUT.setDescriptor("test_metric", "", nil)
 
 	metric := SUT.makeGaugeMetric("test_metric", 1)
 
-	assert.Equal(t, SUT.metrics["test_metric"], metric.Desc())
+	assert.Equal(t, SUT.getDescriptor("test_metric"), metric.Desc())
 }
 
 func TestMetricFactoryWithTimestamp(t *testing.T) {
@@ -56,11 +53,8 @@ func TestMetricFactoryWithTimestamp(t *testing.T) {
 	}()
 
 	clock = StoppedClock{}
-	SUT := &DefaultCollector{
-		metrics: metricDescriptors{
-			"test_metric": NewMetricDesc("test", "metric", "", nil),
-		},
-	}
+	SUT := &DefaultCollector{}
+	SUT.setDescriptor("test_metric", "", nil)
 
 	metric := SUT.makeGaugeMetric("test_metric", 1)
 	metricDto := &dto.Metric{}
diff --git a/test/corosync.metrics b/test/corosync.metrics
index 5c765e2..c1e1224 100644
--- a/test/corosync.metrics
+++ b/test/corosync.metrics
@@ -7,6 +7,6 @@ ha_cluster_corosync_quorum_votes{type="expected_votes"} 2
 ha_cluster_corosync_quorum_votes{type="highest_expected"} 2
 ha_cluster_corosync_quorum_votes{type="quorum"} 1
 ha_cluster_corosync_quorum_votes{type="total_votes"} 2
-# HELP ha_cluster_corosync_ring_errors_total Total number of corosync ring errors
-# TYPE ha_cluster_corosync_ring_errors_total gauge
-ha_cluster_corosync_ring_errors_total 1
+# HELP ha_cluster_corosync_ring_errors The number of corosync ring errors
+# TYPE ha_cluster_corosync_ring_errors gauge
+ha_cluster_corosync_ring_errors 1
diff --git a/test/pacemaker.metrics b/test/pacemaker.metrics
index d51dcab..f95f0b1 100644
--- a/test/pacemaker.metrics
+++ b/test/pacemaker.metrics
@@ -30,9 +30,6 @@ ha_cluster_pacemaker_nodes{node="hana01",status="expected_up",type="member"} 1
 ha_cluster_pacemaker_nodes{node="hana01",status="online",type="member"} 1
 ha_cluster_pacemaker_nodes{node="hana02",status="expected_up",type="member"} 1
 ha_cluster_pacemaker_nodes{node="hana02",status="online",type="member"} 1
-# HELP ha_cluster_pacemaker_nodes_total Total number of nodes in the cluster
-# TYPE ha_cluster_pacemaker_nodes_total gauge
-ha_cluster_pacemaker_nodes_total 2
 # HELP ha_cluster_pacemaker_resources The resources in the cluster; one line per id, per status
 # TYPE ha_cluster_pacemaker_resources gauge
 ha_cluster_pacemaker_resources{managed="true",node="hana01",resource="rsc_ip_PRD_HDB00",role="started",status="active"} 1
@@ -41,9 +38,6 @@ ha_cluster_pacemaker_resources{managed="true",node="hana01",resource="rsc_SAPHan
 ha_cluster_pacemaker_resources{managed="true",node="hana01",resource="stonith-sbd",role="started",status="active"} 1
 ha_cluster_pacemaker_resources{managed="true",node="hana02",resource="rsc_SAPHana_PRD_HDB00",role="slave",status="active"} 1
 ha_cluster_pacemaker_resources{managed="true",node="hana02",resource="rsc_SAPHanaTopology_PRD_HDB00",role="started",status="active"} 1
-# HELP ha_cluster_pacemaker_resources_total Total number of resources in the cluster
-# TYPE ha_cluster_pacemaker_resources_total gauge
-ha_cluster_pacemaker_resources_total 6
 # HELP ha_cluster_pacemaker_stonith_enabled Whether or not stonith is enabled
 # TYPE ha_cluster_pacemaker_stonith_enabled gauge
 ha_cluster_pacemaker_stonith_enabled 1
diff --git a/test/sbd.metrics b/test/sbd.metrics
index d8a812d..a7895dc 100644
--- a/test/sbd.metrics
+++ b/test/sbd.metrics
@@ -1,7 +1,4 @@
-# HELP ha_cluster_sbd_device_status Whether or not an SBD device is healthy; one line per device
-# TYPE ha_cluster_sbd_device_status gauge
-ha_cluster_sbd_device_status{device="/dev/vdc"} 1
-ha_cluster_sbd_device_status{device="/dev/vdd"} 0
-# HELP ha_cluster_sbd_devices_total Total count of configured SBD devices
-# TYPE ha_cluster_sbd_devices_total gauge
-ha_cluster_sbd_devices_total 2
+# HELP ha_cluster_sbd_devices SBD devices; one line per device
+# TYPE ha_cluster_sbd_devices gauge
+ha_cluster_sbd_devices{device="/dev/vdc",status="healthy"} 1
+ha_cluster_sbd_devices{device="/dev/vdd",status="unhealthy"} 1