Add flag to skip checkKeys if role == master

.github/workflows/tests.yml

@@ -49,7 +49,6 @@ jobs:
           LOG_LEVEL: "info"
         run: |
           sleep 15
-          make checks
           make test
 
 
@@ -102,6 +101,9 @@ jobs:
           version: v1.64
           args: "--tests=false"
 
+      - name: Run checks
+        run: make checks
+
 
   build-stuff:
     runs-on: ubuntu-latest

exporter/exporter.go

@@ -5,6 +5,7 @@
 	"net/http"
 	"net/url"
 	"runtime"
+	"runtime/debug"
 	"strconv"
 	"strings"
 	"sync"
@@ -85,6 +86,7 @@
 	BuildInfo                      BuildInfo
 	BasicAuthUsername              string
 	BasicAuthPassword              string
+	SkipCheckKeysForRoleMaster     bool
 }
 
 // NewRedisExporter returns a new exporter of Redis metrics.
@@ -719,31 +721,30 @@
 
 	log.Debugf("dbCount: %d", dbCount)
 
-	e.extractInfoMetrics(ch, infoAll, dbCount)
+	role := e.extractInfoMetrics(ch, infoAll, dbCount)
 
 	if !e.options.ExcludeLatencyHistogramMetrics {
 		e.extractLatencyMetrics(ch, infoAll, c)
 	}
 
-	if e.options.IsCluster {
-		clusterClient, err := e.connectToRedisCluster()
-		if err != nil {
-			log.Errorf("Couldn't connect to redis cluster")
-			return err
+	// skip these metrics for master if SkipCheckKeysForRoleMaster is set
+	// (can help with reducing work load on the master node)
+	log.Infof("checkKeys metric collection for role: %s  flag: %#v", role, e.options.SkipCheckKeysForRoleMaster)
+	debug.PrintStack()
+	if role == InstanceRoleSlave || !e.options.SkipCheckKeysForRoleMaster {
+		if err := e.extractCheckKeyMetrics(ch, c); err != nil {
+			log.Errorf("extractCheckKeyMetrics() err: %s", err)
 		}
-		defer clusterClient.Close()
 
-		e.extractCheckKeyMetrics(ch, clusterClient)
+		e.extractCountKeysMetrics(ch, c)
+
+		e.extractStreamMetrics(ch, c)
 	} else {
-		e.extractCheckKeyMetrics(ch, c)
+		log.Infof("skipping checkKeys metrics, role: %s  flag: %#v", role, e.options.SkipCheckKeysForRoleMaster)
 	}
 
 	e.extractSlowLogMetrics(ch, c)
 
-	e.extractStreamMetrics(ch, c)
-
-	e.extractCountKeysMetrics(ch, c)
-
 	e.extractKeyGroupMetrics(ch, c, dbCount)
 
 	if strings.Contains(infoAll, "# Sentinel") {

exporter/exporter_test.go

@@ -23,22 +23,25 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-var (
-	keys            []string
-	keysExpiring    []string
-	listKeys        []string
-	singleStringKey string
-
+const (
 	dbNumStr        = "11"
 	altDBNumStr     = "12"
 	invalidDBNumStr = "16"
-	dbNumStrFull    = fmt.Sprintf("db%s", dbNumStr)
 )
 
-const (
-	TestKeysSetName    = "test-set"
-	TestKeysStreamName = "test-stream"
-	TestKeysHllName    = "test-hll"
+var (
+	keys         []string
+	keysExpiring []string
+	listKeys     []string
+
+	dbNumStrFull = fmt.Sprintf("db%s", dbNumStr)
+)
+
+var (
+	TestKeyNameSingleString = "" // initialized with a timestamp at runtime
+	TestKeyNameSet          = "test-set"
+	TestKeyNameStream       = "test-stream"
+	TestKeyNameHll          = "test-hll"
 )
 
 func getTestExporter() *Exporter {
@@ -95,43 +98,43 @@ func setupKeys(t *testing.T, c redis.Conn, dbNumStr string) error {
 		}
 	}
 
-	if _, err := c.Do("PFADD", TestKeysHllName, "val1"); err != nil {
+	if _, err := c.Do("PFADD", TestKeyNameHll, "val1"); err != nil {
 		t.Errorf("PFADD err: %s", err)
 		return err
 	}
-	if _, err := c.Do("PFADD", TestKeysHllName, "val22"); err != nil {
+	if _, err := c.Do("PFADD", TestKeyNameHll, "val22"); err != nil {
 		t.Errorf("PFADD err: %s", err)
 		return err
 	}
-	if _, err := c.Do("PFADD", TestKeysHllName, "val333"); err != nil {
+	if _, err := c.Do("PFADD", TestKeyNameHll, "val333"); err != nil {
 		t.Errorf("PFADD err: %s", err)
 		return err
 	}
 
-	if _, err := c.Do("SADD", TestKeysSetName, "test-val-1"); err != nil {
+	if _, err := c.Do("SADD", TestKeyNameSet, "test-val-1"); err != nil {
 		t.Errorf("SADD err: %s", err)
 		return err
 	}
-	if _, err := c.Do("SADD", TestKeysSetName, "test-val-2"); err != nil {
+	if _, err := c.Do("SADD", TestKeyNameSet, "test-val-2"); err != nil {
 		t.Errorf("SADD err: %s", err)
 		return err
 	}
 
-	if _, err := c.Do("SET", singleStringKey, "this-is-a-string"); err != nil {
+	if _, err := c.Do("SET", TestKeyNameSingleString, "this-is-a-string"); err != nil {
 		t.Errorf("PFADD err: %s", err)
 		return err
 	}
 
 	// Create test streams
-	c.Do("XGROUP", "CREATE", TestKeysStreamName, "test_group_1", "$", "MKSTREAM")
-	c.Do("XGROUP", "CREATE", TestKeysStreamName, "test_group_2", "$", "MKSTREAM")
-	c.Do("XADD", TestKeysStreamName, TestStreamTimestamps[0], "field_1", "str_1")
-	c.Do("XADD", TestKeysStreamName, TestStreamTimestamps[1], "field_2", "str_2")
+	c.Do("XGROUP", "CREATE", TestKeyNameStream, "test_group_1", "$", "MKSTREAM")
+	c.Do("XGROUP", "CREATE", TestKeyNameStream, "test_group_2", "$", "MKSTREAM")
+	c.Do("XADD", TestKeyNameStream, TestStreamTimestamps[0], "field_1", "str_1")
+	c.Do("XADD", TestKeyNameStream, TestStreamTimestamps[1], "field_2", "str_2")
 
 	// Process messages to assign Consumers to their groups
-	c.Do("XREADGROUP", "GROUP", "test_group_1", "test_consumer_1", "COUNT", "1", "STREAMS", TestKeysStreamName, ">")
-	c.Do("XREADGROUP", "GROUP", "test_group_1", "test_consumer_2", "COUNT", "1", "STREAMS", TestKeysStreamName, ">")
-	c.Do("XREADGROUP", "GROUP", "test_group_2", "test_consumer_1", "COUNT", "1", "STREAMS", TestKeysStreamName, "0")
+	c.Do("XREADGROUP", "GROUP", "test_group_1", "test_consumer_1", "COUNT", "1", "STREAMS", TestKeyNameStream, ">")
+	c.Do("XREADGROUP", "GROUP", "test_group_1", "test_consumer_2", "COUNT", "1", "STREAMS", TestKeyNameStream, ">")
+	c.Do("XREADGROUP", "GROUP", "test_group_2", "test_consumer_1", "COUNT", "1", "STREAMS", TestKeyNameStream, "0")
 
 	time.Sleep(time.Millisecond * 100)
 	return nil
@@ -155,10 +158,10 @@ func deleteKeys(c redis.Conn, dbNumStr string) {
 		c.Do("DEL", key)
 	}
 
-	c.Do("DEL", TestKeysHllName)
-	c.Do("DEL", TestKeysSetName)
-	c.Do("DEL", TestKeysStreamName)
-	c.Do("DEL", singleStringKey)
+	c.Do("DEL", TestKeyNameHll)
+	c.Do("DEL", TestKeyNameSet)
+	c.Do("DEL", TestKeyNameStream)
+	c.Do("DEL", TestKeyNameSingleString)
 }
 
 func setupDBKeys(t *testing.T, uri string) {
@@ -363,15 +366,9 @@ func TestKeysReset(t *testing.T) {
 	setupDBKeys(t, os.Getenv("TEST_REDIS_URI"))
 	defer deleteKeysFromDB(t, os.Getenv("TEST_REDIS_URI"))
 
-	chM := make(chan prometheus.Metric, 10000)
-	go func() {
-		e.Collect(chM)
-		close(chM)
-	}()
-
 	body := downloadURL(t, ts.URL+"/metrics")
 	if !strings.Contains(body, keys[0]) {
-		t.Errorf("Did not found key %q\n%s", keys[0], body)
+		t.Errorf("Did not find key %q\n%s", keys[0], body)
 	}
 
 	deleteKeysFromDB(t, os.Getenv("TEST_REDIS_URI"))
@@ -468,7 +465,7 @@ func init() {
 		keys = append(keys, fmt.Sprintf("key_%s_%d", n, testTimestamp))
 	}
 
-	singleStringKey = fmt.Sprintf("key_string_%d", testTimestamp)
+	TestKeyNameSingleString = fmt.Sprintf("key_string_%d", testTimestamp)
 
 	listKeys = append(listKeys, "beatles_list")
 

exporter/http_test.go

@@ -28,7 +28,7 @@ func TestHTTPScrapeMetricsEndpoints(t *testing.T) {
 	defer deleteKeysFromDB(t, os.Getenv("TEST_PWD_REDIS_URI"))
 
 	csk := dbNumStrFull + "=" + url.QueryEscape(keys[0]) // check-single-keys
-	css := dbNumStrFull + "=" + TestKeysStreamName       // check-single-streams
+	css := dbNumStrFull + "=" + TestKeyNameStream        // check-single-streams
 	cntk := dbNumStrFull + "=" + keys[0] + "*"           // count-keys
 
 	u, err := url.Parse(os.Getenv("TEST_REDIS_URI"))

exporter/info.go

@@ -33,8 +33,13 @@ var reMasterDirect = regexp.MustCompile(`^(master(_[0-9]+)?_(last_io_seconds_ago
 slave0:ip=10.254.11.1,port=6379,state=online,offset=1751844676,lag=0
 slave1:ip=10.254.11.2,port=6379,state=online,offset=1751844222,lag=0
 */
+
 var reSlave = regexp.MustCompile(`^slave\d+`)
 
+const (
+	InstanceRoleSlave = "slave"
+)
+
 func extractVal(s string) (val float64, err error) {
 	split := strings.Split(s, "=")
 	if len(split) != 2 {
@@ -60,7 +65,8 @@ func extractPercentileVal(s string) (percentile float64, val float64, err error)
 	return
 }
 
-func (e *Exporter) extractInfoMetrics(ch chan<- prometheus.Metric, info string, dbCount int) {
+// returns the role of the instance we're scraping (master or slave)
+func (e *Exporter) extractInfoMetrics(ch chan<- prometheus.Metric, info string, dbCount int) string {
 	keyValues := map[string]string{}
 	handledDBs := map[string]bool{}
 	cmdCount := map[string]uint64{}
@@ -161,8 +167,10 @@ func (e *Exporter) extractInfoMetrics(ch chan<- prometheus.Metric, info string,
 		}
 	}
 
+	instanceRole := keyValues["role"]
+
 	e.registerConstMetricGauge(ch, "instance_info", 1,
-		keyValues["role"],
+		instanceRole,
 		keyValues["redis_version"],
 		keyValues["redis_build_id"],
 		keyValues["redis_mode"],
@@ -174,12 +182,14 @@ func (e *Exporter) extractInfoMetrics(ch chan<- prometheus.Metric, info string,
 		keyValues["master_replid"],
 	)
 
-	if keyValues["role"] == "slave" {
+	if instanceRole == InstanceRoleSlave {
 		e.registerConstMetricGauge(ch, "slave_info", 1,
 			keyValues["master_host"],
 			keyValues["master_port"],
 			keyValues["slave_read_only"])
 	}
+
+	return instanceRole
 }
 
 func (e *Exporter) generateCommandLatencySummaries(ch chan<- prometheus.Metric, cmdLatencyMap map[string]map[float64]float64, cmdCount map[string]uint64, cmdSum map[string]float64) {

exporter/info_test.go

@@ -141,12 +141,6 @@ func TestClusterMaster(t *testing.T) {
 	ts := httptest.NewServer(e)
 	defer ts.Close()
 
-	chM := make(chan prometheus.Metric, 10000)
-	go func() {
-		e.Collect(chM)
-		close(chM)
-	}()
-
 	body := downloadURL(t, ts.URL+"/metrics")
 	log.Debugf("master - body: %s", body)
 	for _, want := range []string{
@@ -159,6 +153,54 @@ func TestClusterMaster(t *testing.T) {
 	}
 }
 
+func TestClusterSkipCheckKeysIfMaster(t *testing.T) {
+	uriMaster := os.Getenv("TEST_REDIS_CLUSTER_MASTER_URI")
+	uriSlave := os.Getenv("TEST_REDIS_CLUSTER_SLAVE_URI")
+	if uriMaster == "" || uriSlave == "" {
+		t.Skipf("TEST_REDIS_CLUSTER_MASTER_URI or slave not set - skipping")
+	}
+
+	setupDBKeysCluster(t, uriMaster)
+	defer deleteKeysFromDBCluster(uriMaster)
+
+	for _, uri := range []string{uriMaster, uriSlave} {
+		for _, skip := range []bool{true, false} {
+			e, _ := NewRedisExporter(
+				uri,
+				Options{Namespace: "test",
+					Registry:                   prometheus.NewRegistry(),
+					CheckKeys:                  TestKeyNameHll,
+					SkipCheckKeysForRoleMaster: skip,
+					IsCluster:                  true,
+				})
+			ts := httptest.NewServer(e)
+
+			body := downloadURL(t, ts.URL+"/metrics")
+
+			expectedMetricPresent := true
+			if skip && uri == uriMaster {
+				expectedMetricPresent = false
+			}
+			t.Logf("skip: %#v  uri: %s    uri == uriMaster: %#v", skip, uri, uri == uriMaster)
+			t.Logf("expectedMetricPresent: %#v", expectedMetricPresent)
+
+			want := `test_key_size{db="db0",key="test-hll"} 3`
+
+			if expectedMetricPresent {
+				if !strings.Contains(body, want) {
+					t.Fatalf("expectedMetricPresent but missing. metric: %s   body: %s\n", want, body)
+				}
+			} else {
+				if strings.Contains(body, want) {
+					t.Fatalf("should have skipped it but found it, body:\n%s", body)
+				}
+			}
+
+			ts.Close()
+		}
+	}
+}
+
 func TestClusterSlave(t *testing.T) {
 	if os.Getenv("TEST_REDIS_CLUSTER_SLAVE_URI") == "" {
 		t.Skipf("TEST_REDIS_CLUSTER_SLAVE_URI not set - skipping")
@@ -169,12 +211,6 @@ func TestClusterSlave(t *testing.T) {
 	ts := httptest.NewServer(e)
 	defer ts.Close()
 
-	chM := make(chan prometheus.Metric, 10000)
-	go func() {
-		e.Collect(chM)
-		close(chM)
-	}()
-
 	body := downloadURL(t, ts.URL+"/metrics")
 	log.Debugf("slave - body: %s", body)
 	for _, want := range []string{

exporter/keys.go

@@ -143,18 +143,28 @@
 	return info, err
 }
 
-func (e *Exporter) extractCheckKeyMetrics(ch chan<- prometheus.Metric, c redis.Conn) {
+func (e *Exporter) extractCheckKeyMetrics(ch chan<- prometheus.Metric, redisClient redis.Conn) error {
+	c := redisClient
+
+	if e.options.IsCluster {
+		cc, err := e.connectToRedisCluster()
+		if err != nil {
+			return fmt.Errorf("Couldn't connect to redis cluster, err: %s", err)
+		}
+		defer cc.Close()
+
+		c = cc
+	}
+
 	keys, err := parseKeyArg(e.options.CheckKeys)
 	if err != nil {
-		log.Errorf("Couldn't parse check-keys: %#v", err)
-		return
+		return fmt.Errorf("Couldn't parse check-keys: %w", err)
 	}
 	log.Debugf("keys: %#v", keys)
 
 	singleKeys, err := parseKeyArg(e.options.CheckSingleKeys)
 	if err != nil {
-		log.Errorf("Couldn't parse check-single-keys: %#v", err)
-		return
+		return fmt.Errorf("Couldn't parse check-single-keys: %w", err)
 	}
 	log.Debugf("e.singleKeys: %#v", singleKeys)
 
@@ -210,6 +220,7 @@
 			log.Error(err)
 		}
 	}
+	return nil
 }
 
 func (e *Exporter) extractCountKeysMetrics(ch chan<- prometheus.Metric, c redis.Conn) {