fix replication metrics handling, fix tests (#246)

.drone.yml

@@ -34,6 +34,7 @@ steps:
   - name: tests
     image: "golang:1.12"
     environment:
+      LOG_LEVEL: "info"
       TEST_TILE38_URI: "tile38:9851"
       TEST_SECOND_REDIS_URI: "redis://moar-redis:6380"
       TEST_REDIS_CLUSTER_MASTER_URI: "redis-cluster:7000"
@@ -41,17 +42,19 @@ steps:
       COVERALLS_TOKEN:
         from_secret: coveralls-token
     commands:
+      - 'go build'
+      - "sleep 10"   # let the redis test instances all come up first
+      - 'go test -v -covermode=atomic -cover -race -coverprofile=coverage.txt ./exporter/...  --redis.addr=redis'
+      - 'echo "checking gofmt"'
       - 'echo " ! gofmt -d main.go       2>&1 | read " | bash'
       - 'echo " ! gofmt -d exporter/*.go 2>&1 | read " | bash'
+      - 'echo "checking gofmt - DONE"'
       - 'go vet ./exporter/...'
-      - 'go build'
-      - "sleep 10"
-      - 'go test -v -covermode=atomic -cover -race -coverprofile=coverage.txt ./exporter/...  --redis.addr=redis'
       - 'go get github.com/mattn/goveralls'
       - '/go/bin/goveralls -coverprofile=coverage.txt -service=drone.io'
     when:
       event:
-        - push
+        - pull_request
   - name: coverage-codecov
@@ -63,7 +66,7 @@ steps:
         - coverage.txt
     when:
       event:
-        - push
+        - pull_request
   - name: release-docker-image-scratch

exporter/redis.go

@@ -560,7 +560,7 @@ func (e *Exporter) handleMetricsCommandStats(addr string, alias string, fieldKey
 	e.metrics["commands_duration_seconds_total"].WithLabelValues(addr, alias, cmd).Set(usecTotal / 1e6)
 }
-func (e *Exporter) handleMetricsReplication(addr string, alias string, fieldKey string, fieldValue string) {
+func (e *Exporter) handleMetricsReplication(addr string, alias string, fieldKey string, fieldValue string) bool {
 	e.metricsMtx.RLock()
 	defer e.metricsMtx.RUnlock()
@@ -571,7 +571,7 @@ func (e *Exporter) handleMetricsReplication(addr string, alias string, fieldKey
 		} else {
 			e.metrics["master_link_up"].WithLabelValues(addr, alias).Set(0)
 		}
-		return
+		return true
 	}
 	// not a slave, try extracting master metrics
@@ -593,7 +593,10 @@ func (e *Exporter) handleMetricsReplication(addr string, alias string, fieldKey
 				slaveState,
 			).Set(lag)
 		}
+		return true
 	}
+
+	return false
 }
 func (e *Exporter) handleMetricsServer(addr string, alias string, fieldKey string, fieldValue string) {
@@ -641,8 +644,9 @@ func (e *Exporter) extractInfoMetrics(info, addr string, alias string, scrapes c
 		switch fieldClass {
 		case "Replication":
-			e.handleMetricsReplication(addr, alias, fieldKey, fieldValue)
-			continue
+			if ok := e.handleMetricsReplication(addr, alias, fieldKey, fieldValue); ok {
+				continue
+			}
 		case "Server":
 			e.handleMetricsServer(addr, alias, fieldKey, fieldValue)

exporter/redis_test.go

@@ -9,7 +9,6 @@ package exporter
 */
 import (
-	"bytes"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -152,7 +151,7 @@ func resetSlowLog(t *testing.T, addr string) error {
 	return nil
 }
-func downloadUrl(t *testing.T, url string) []byte {
+func downloadUrl(t *testing.T, url string) string {
 	log.Debugf("downloadURL() %s", url)
 	resp, err := http.Get(url)
 	if err != nil {
@@ -163,7 +162,7 @@ func downloadUrl(t *testing.T, url string) []byte {
 	if err != nil {
 		t.Fatal(err)
 	}
-	return body
+	return string(body)
 }
 func TestLatencySpike(t *testing.T) {
@@ -334,10 +333,9 @@ func setupDBKeys(t *testing.T, addr string) error {
 	}
 	defer c.Close()
-	_, err = c.Do("SELECT", dbNumStr)
-	if err != nil {
-		t.Errorf("couldn't setup redis, err: %s ", err)
-		return err
+	if _, err := c.Do("SELECT", dbNumStr); err != nil {
+		log.Printf("setupDBKeys() - couldn't setup redis, err: %s ", err)
+		// not failing on this one - cluster doesn't allow for SELECT so we log and ignore the error
 	}
 	for _, key := range keys {
@@ -384,10 +382,9 @@ func deleteKeysFromDB(t *testing.T, addr string) error {
 	}
 	defer c.Close()
-	_, err = c.Do("SELECT", dbNumStr)
-	if err != nil {
-		t.Errorf("couldn't setup redis, err: %s ", err)
-		return err
+	if _, err := c.Do("SELECT", dbNumStr); err != nil {
+		log.Printf("deleteKeysFromDB() - couldn't setup redis, err: %s ", err)
+		// not failing on this one - cluster doesn't allow for SELECT so we log and ignore the error
 	}
 	for _, key := range keys {
@@ -1019,6 +1016,10 @@ func TestCommandStats(t *testing.T) {
 }
 func TestHTTPEndpoint(t *testing.T) {
+	r := prometheus.NewRegistry()
+	prometheus.DefaultGatherer = r
+	prometheus.DefaultRegisterer = r
+
 	ts := httptest.NewServer(promhttp.Handler())
 	defer ts.Close()
@@ -1044,7 +1045,7 @@ func TestHTTPEndpoint(t *testing.T) {
 		`cmd="get"`,
 	}
 	for _, test := range tests {
-		if !bytes.Contains(body, []byte(test)) {
+		if !strings.Contains(body, test) {
 			t.Errorf("want metrics to include %q, have:\n%s", test, body)
 		}
 	}
@@ -1206,6 +1207,10 @@ func TestSanitizeMetricName(t *testing.T) {
 }
 func TestKeysReset(t *testing.T) {
+	r := prometheus.NewRegistry()
+	prometheus.DefaultGatherer = r
+	prometheus.DefaultRegisterer = r
+
 	ts := httptest.NewServer(promhttp.Handler())
 	defer ts.Close()
@@ -1223,16 +1228,14 @@ func TestKeysReset(t *testing.T) {
 	}()
 	body := downloadUrl(t, ts.URL+"/metrics")
-
-	if !bytes.Contains(body, []byte(keys[0])) {
+	if !strings.Contains(body, keys[0]) {
 		t.Errorf("Did not found key %q\n%s", keys[0], body)
 	}
 	deleteKeysFromDB(t, defaultRedisHost.Addrs[0])
 	body = downloadUrl(t, ts.URL+"/metrics")
-
-	if bytes.Contains(body, []byte(keys[0])) {
+	if strings.Contains(body, keys[0]) {
 		t.Errorf("Metric is present in metrics list %q\n%s", keys[0], body)
 	}
 }
@@ -1244,15 +1247,18 @@ func TestClusterMaster(t *testing.T) {
 		return
 	}
+	r := prometheus.NewRegistry()
+	prometheus.DefaultGatherer = r
+	prometheus.DefaultRegisterer = r
+
 	ts := httptest.NewServer(promhttp.Handler())
 	defer ts.Close()
 	addr := "redis://" + os.Getenv("TEST_REDIS_CLUSTER_MASTER_URI")
 	host := RedisHost{Addrs: []string{addr}, Aliases: []string{"master"}}
+	log.Printf("master - using host cfg: %#v", host)
 	e, _ := NewRedisExporter(host, "test", "", "")
-	setupDBKeys(t, defaultRedisHost.Addrs[0])
-	defer deleteKeysFromDB(t, defaultRedisHost.Addrs[0])
 	prometheus.Register(e)
 	chM := make(chan prometheus.Metric, 10000)
@@ -1262,8 +1268,14 @@ func TestClusterMaster(t *testing.T) {
 	}()
 	body := downloadUrl(t, ts.URL+"/metrics")
-	if !bytes.Contains(body, []byte("test_instance_info")) {
-		t.Errorf("Did not found key %q\n%s", keys[0], body)
+	//	log.Printf("master - body: %s", body)
+	for _, want := range []string{
+		"test_instance_info{addr=\"redis://redis-cluster:7000\",alias=\"master\"",
+		"test_master_repl_offset",
+	} {
+		if !strings.Contains(body, want) {
+			t.Errorf("Did not find key [%s] \nbody: %s", want, body)
+		}
 	}
 }
@@ -1311,12 +1323,16 @@ func TestPasswordProtectedInstance(t *testing.T) {
 	body := downloadUrl(t, ts.URL+"/metrics")
-	if !bytes.Contains(body, []byte("test_up")) {
-		t.Errorf("error")
+	if !strings.Contains(body, "test_up") {
+		t.Errorf("error, missing test_up")
 	}
 }
 func TestPasswordInvalid(t *testing.T) {
+	r := prometheus.NewRegistry()
+	prometheus.DefaultGatherer = r
+	prometheus.DefaultRegisterer = r
+
 	ts := httptest.NewServer(promhttp.Handler())
 	defer ts.Close()
@@ -1359,7 +1375,7 @@ func TestPasswordInvalid(t *testing.T) {
 	}()
 	body := downloadUrl(t, ts.URL+"/metrics")
-	if !bytes.Contains(body, []byte("test_exporter_last_scrape_error 1")) {
+	if !strings.Contains(body, "test_exporter_last_scrape_error 1") {
 		t.Errorf(`error, expected string "test_exporter_last_scrape_error 1" in body`)
 	}
 }
@@ -1371,16 +1387,18 @@ func TestClusterSlave(t *testing.T) {
 		return
 	}
+	r := prometheus.NewRegistry()
+	prometheus.DefaultGatherer = r
+	prometheus.DefaultRegisterer = r
+
 	ts := httptest.NewServer(promhttp.Handler())
 	defer ts.Close()
 	addr := "redis://" + os.Getenv("TEST_REDIS_CLUSTER_SLAVE_URI")
 	host := RedisHost{Addrs: []string{addr}, Aliases: []string{"slave"}}
+	log.Printf("slave - using host cfg: %#v", host)
 	e, _ := NewRedisExporter(host, "test", "", "")
-	setupDBKeys(t, defaultRedisHost.Addrs[0])
-	defer deleteKeysFromDB(t, defaultRedisHost.Addrs[0])
-
 	prometheus.Register(e)
 	chM := make(chan prometheus.Metric, 10000)
@@ -1390,15 +1408,19 @@ func TestClusterSlave(t *testing.T) {
 	}()
 	body := downloadUrl(t, ts.URL+"/metrics")
-	if !bytes.Contains(body, []byte("test_instance_info")) {
-		t.Errorf("Did not found key %q\n%s", keys[0], body)
+	//	log.Printf("slave - body: %s", body)
+	for _, want := range []string{
+		"test_instance_info",
+		"test_master_last_io_seconds",
+		"test_slave_info{addr=\"redis://redis-cluster:7005\",alias=\"slave\",",
+	} {
+		if !strings.Contains(body, want) {
+			t.Errorf("Did not find key [%s] \nbody: %s", want, body)
+		}
 	}
 }
 func TestCheckKeys(t *testing.T) {
-	ts := httptest.NewServer(promhttp.Handler())
-	defer ts.Close()
-
 	for _, tst := range []struct {
 		SingleCheckKey string
 		CheckKeys      string