From a58b20c257e16b483842ad2032953b0f5afafebb Mon Sep 17 00:00:00 2001
From: Trey Dockendorf <tdockendorf@osc.edu>
Date: Thu, 20 Feb 2020 15:12:32 -0500
Subject: [PATCH] Fix error handling

---
 cgroup_exporter.go      | 21 +++++++++++++--------
 cgroup_exporter_test.go | 11 +++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/cgroup_exporter.go b/cgroup_exporter.go
index 3b338b8..a564f53 100644
--- a/cgroup_exporter.go
+++ b/cgroup_exporter.go
@@ -61,6 +61,7 @@ type CgroupMetric struct {
 	uid             string
 	username        string
 	jobid           string
+	err             bool
 }
 
 type Exporter struct {
@@ -237,11 +238,11 @@ func NewExporter(paths []string) *Exporter {
 		info: prometheus.NewDesc(prometheus.BuildFQName(namespace, "", "info"),
 			"User slice information", []string{"cgroup", "username", "uid", "jobid"}, nil),
 		collectError: prometheus.NewDesc(prometheus.BuildFQName(namespace, "exporter", "collect_error"),
-			"Indicates collection error, 0=no error, 1=error", []string{"path"}, nil),
+			"Indicates collection error, 0=no error, 1=error", []string{"cgroup"}, nil),
 	}
 }
 
-func (e *Exporter) collect(ch chan<- prometheus.Metric) ([]CgroupMetric, error) {
+func (e *Exporter) collect() ([]CgroupMetric, error) {
 	var names []string
 	var metrics []CgroupMetric
 	for _, path := range e.paths {
@@ -249,13 +250,15 @@ func (e *Exporter) collect(ch chan<- prometheus.Metric) ([]CgroupMetric, error)
 		control, err := cgroups.Load(subsystem, cgroups.StaticPath(path))
 		if err != nil {
 			log.Errorf("Error loading cgroup subsystem path %s: %s", path, err.Error())
-			ch <- prometheus.MustNewConstMetric(e.collectError, prometheus.GaugeValue, 1, path)
+			metric := CgroupMetric{name: path, err: true}
+			metrics = append(metrics, metric)
 			continue
 		}
 		processes, err := control.Processes(cgroups.Cpuacct, true)
 		if err != nil {
 			log.Errorf("Error loading cgroup processes for path %s: %s", path, err.Error())
-			ch <- prometheus.MustNewConstMetric(e.collectError, prometheus.GaugeValue, 1, path)
+			metric := CgroupMetric{name: path, err: true}
+			metrics = append(metrics, metric)
 			continue
 		}
 		log.Debugf("Found %d processes", len(processes))
@@ -276,7 +279,8 @@ func (e *Exporter) collect(ch chan<- prometheus.Metric) ([]CgroupMetric, error)
 			})
 			if err != nil {
 				log.Errorf("Failed to load cgroups for %s: %s", name, err.Error())
-				ch <- prometheus.MustNewConstMetric(e.collectError, prometheus.GaugeValue, 1, name)
+				metric.err = true
+				metrics = append(metrics, metric)
 				continue
 			}
 			stats, _ := ctrl.Stat(cgroups.IgnoreNotExist)
@@ -295,7 +299,6 @@ func (e *Exporter) collect(ch chan<- prometheus.Metric) ([]CgroupMetric, error)
 			getInfo(name, &metric)
 			metrics = append(metrics, metric)
 		}
-		ch <- prometheus.MustNewConstMetric(e.collectError, prometheus.GaugeValue, 0, path)
 	}
 
 	return metrics, nil
@@ -312,12 +315,14 @@ func (e *Exporter) Describe(ch chan<- *prometheus.Desc) {
 	ch <- e.swapUsed
 	ch <- e.swapTotal
 	ch <- e.swapFailCount
-	ch <- e.collectError
 }
 
 func (e *Exporter) Collect(ch chan<- prometheus.Metric) {
-	metrics, _ := e.collect(ch)
+	metrics, _ := e.collect()
 	for _, m := range metrics {
+		if m.err {
+			ch <- prometheus.MustNewConstMetric(e.collectError, prometheus.GaugeValue, 1, m.name)
+		}
 		ch <- prometheus.MustNewConstMetric(e.cpuUser, prometheus.GaugeValue, m.cpuUser, m.name)
 		ch <- prometheus.MustNewConstMetric(e.cpuSystem, prometheus.GaugeValue, m.cpuSystem, m.name)
 		ch <- prometheus.MustNewConstMetric(e.cpuTotal, prometheus.GaugeValue, m.cpuTotal, m.name)
diff --git a/cgroup_exporter_test.go b/cgroup_exporter_test.go
index beebc7c..e0a438d 100644
--- a/cgroup_exporter_test.go
+++ b/cgroup_exporter_test.go
@@ -14,7 +14,6 @@
 package main
 
 import (
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/common/log"
 	kingpin "gopkg.in/alecthomas/kingpin.v2"
 	"path/filepath"
@@ -22,10 +21,6 @@ import (
 	"testing"
 )
 
-var (
-	ch = make(chan prometheus.Metric)
-)
-
 func TestParseCpuSet(t *testing.T) {
 	if cpus, err := parseCpuSet("0-2"); err != nil {
 		t.Errorf("Unexpected error: %s", err.Error())
@@ -54,7 +49,7 @@ func TestCollectUserSlice(t *testing.T) {
 	cgroupRoot = &fixture
 
 	exporter := NewExporter([]string{"/user.slice"})
-	metrics, err := exporter.collect(ch)
+	metrics, err := exporter.collect()
 	if err != nil {
 		t.Errorf("Unexpected error: %s", err.Error())
 		return
@@ -109,7 +104,7 @@ func TestCollectSLURM(t *testing.T) {
 	cgroupRoot = &fixture
 
 	exporter := NewExporter([]string{"/slurm"})
-	metrics, err := exporter.collect(ch)
+	metrics, err := exporter.collect()
 	if err != nil {
 		t.Errorf("Unexpected error: %s", err.Error())
 		return
@@ -167,7 +162,7 @@ func TestCollectTorque(t *testing.T) {
 	cgroupRoot = &fixture
 
 	exporter := NewExporter([]string{"/torque"})
-	metrics, err := exporter.collect(ch)
+	metrics, err := exporter.collect()
 	if err != nil {
 		t.Errorf("Unexpected error: %s", err.Error())
 		return