Misc bug fixes (#1085)

* Fix panic in client-output-buffer-limit parsing when value length is not a multiple of 4

* Fix panic in extractTile38Metrics when info slice has odd length

* Fix panic in extractModulesMetrics when module fields lack '=' delimiter

* Fix panic in extractLatencyHistogramMetrics when reply or bucketInfo has odd length

* Fix panic in extractSlowLogMetrics from bare type assertions on Redis values

* Fix getStreamEntryId: bounds check before access, remove dead code and redundant type assertion

* Remove expired_time_cap_reached_count from gauge map (it is a counter)

* Fix extractTotalUsecForCommand matching unrelated commands via HasPrefix (e.g. get matching getex)

* Fix non-pipelined path to emit key_size=0 for missing keys, matching pipelined behavior

* Deduplicate cluster connection: create once and reuse for key checks and key groups

* Pre-compile regexes in parseClientListString and parseSentinelMasterString

Author: oliver006

exporter/clients.go

@@ -11,6 +11,8 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+var reClientListId = regexp.MustCompile(`^id=\d+ addr=\S+`)
+
 type ClientInfo struct {
 	Id,
 	Name,
@@ -41,7 +43,7 @@ id=14 addr=127.0.0.1:64958 fd=9 name= age=5 idle=0 flags=N db=0 sub=0 psub=0 mul
 id=40253233 addr=fd40:1481:21:dbe0:7021:300:a03:1a06:44426 fd=19 name= age=782 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=26 qbuf-free=32742 argv-mem=10 obl=0 oll=0 omem=0 tot-mem=61466 ow=0 owmem=0 events=r cmd=client user=default lib-name=redis-py lib-ver=5.0.1 numops=9
 */
 func parseClientListString(clientInfo string) (*ClientInfo, bool) {
-	if matched, _ := regexp.MatchString(`^id=\d+ addr=\S+`, clientInfo); !matched {
+	if !reClientListId.MatchString(clientInfo) {
 		return nil, false
 	}
 	connectedClient := ClientInfo{}

exporter/exporter.go

@@ -222,9 +222,6 @@ func NewRedisExporter(uri string, opts Options) (*Exporter, error) {
 			"active_defrag_key_hits":   "defrag_key_hits",
 			"active_defrag_key_misses": "defrag_key_misses",
 
-			// https://github.com/antirez/redis/blob/0af467d18f9d12b137af3b709c0af579c29d8414/src/expire.c#L297-L299
-			"expired_time_cap_reached_count": "expired_time_cap_reached_total",
-
 			// # Persistence
 			"loading":                      "loading_dump_file",
 			"async_loading":                "async_loading", // Added in Redis 7.0
@@ -730,7 +727,7 @@ func (e *Exporter) extractConfigMetrics(ch chan<- prometheus.Metric, config []in
 		if strKey == "client-output-buffer-limit" {
 			// client-output-buffer-limit "normal 0 0 0 slave 1610612736 1610612736 0 pubsub 33554432 8388608 60"
 			splitVal := strings.Split(strVal, " ")
-			for i := 0; i < len(splitVal); i += 4 {
+			for i := 0; i+3 < len(splitVal); i += 4 {
 				class := splitVal[i]
 				if val, err := strconv.ParseFloat(splitVal[i+1], 64); err == nil {
 					e.registerConstMetricGauge(ch, "config_client_output_buffer_limit_bytes", val, class, "hard")
@@ -747,15 +744,6 @@ func (e *Exporter) extractConfigMetrics(ch chan<- prometheus.Metric, config []in
 	return
 }
 
-// getKeyOperationConnection returns the appropriate Redis connection for key-based operations.
-// For cluster mode, it returns a cluster connection; otherwise, it returns the provided connection.
-func (e *Exporter) getKeyOperationConnection(defaultConn redis.Conn) (redis.Conn, error) {
-	if e.options.IsCluster {
-		return e.connectToRedisCluster()
-	}
-	return defaultConn, nil
-}
-
 func (e *Exporter) scrapeRedisHost(ch chan<- prometheus.Metric) error {
 	defer log.Debugf("scrapeRedisHost() done")
 
@@ -849,27 +837,38 @@ func (e *Exporter) scrapeRedisHost(ch chan<- prometheus.Metric) error {
 		e.extractLatencyMetrics(ch, infoAll, c)
 	}
 
+	var keyConn redis.Conn
+	if e.options.IsCluster {
+		//
+		// in cluster mode we need to create a new, cluster-aware connection
+		// to properly handle cluster-redirects
+		//
+		k, keyConnErr := e.connectToRedisCluster()
+		if keyConnErr != nil {
+			log.Errorf("failed to get key operation connection: %s", keyConnErr)
+		} else {
+			//
+			// The "defer keyConn.Close()" ensures this temporary cluster connection gets
+			// cleaned up after key metrics extraction, while avoiding closing the original
+			// connection "c" which is still needed.
+			//
+			defer k.Close()
+			keyConn = k
+		}
+	} else {
+		// not in cluster mode - we can use our existing connection for retrieving keys and such
+		keyConn = c
+	}
+
 	// skip these metrics for master if SkipCheckKeysForRoleMaster is set
 	// (can help with reducing workload on the master node)
 	log.Debugf("checkKeys metric collection for role: %s  SkipCheckKeysForRoleMaster flag: %#v", role, e.options.SkipCheckKeysForRoleMaster)
 	if role == InstanceRoleSlave || !e.options.SkipCheckKeysForRoleMaster {
-		// For key-based operations, use cluster connection if in cluster mode
-		keyConn, err := e.getKeyOperationConnection(c)
-		if err != nil {
-			log.Errorf("failed to get key operation connection: %s", err)
-		} else {
-			defer func() {
-				if keyConn != c {
-					keyConn.Close()
-				}
-			}()
-
+		if keyConn != nil {
 			if err := e.extractCheckKeyMetrics(ch, keyConn); err != nil {
 				log.Errorf("extractCheckKeyMetrics() err: %s", err)
 			}
-
 			e.extractCountKeysMetrics(ch, keyConn)
-
 			e.extractStreamMetrics(ch, keyConn)
 		}
 	} else {
@@ -878,17 +877,8 @@ func (e *Exporter) scrapeRedisHost(ch chan<- prometheus.Metric) error {
 
 	e.extractSlowLogMetrics(ch, c)
 
-	// Key groups also need cluster connection for key operations
-	keyGroupConn, err := e.getKeyOperationConnection(c)
-	if err != nil {
-		log.Errorf("failed to get key operation connection for key groups: %s", err)
-	} else {
-		defer func() {
-			if keyGroupConn != c {
-				keyGroupConn.Close()
-			}
-		}()
-		e.extractKeyGroupMetrics(ch, keyGroupConn, dbCount)
+	if keyConn != nil {
+		e.extractKeyGroupMetrics(ch, keyConn, dbCount)
 	}
 
 	if strings.Contains(infoAll, "# Sentinel") {

exporter/keys.go

@@ -44,8 +44,7 @@ func (e *Exporter) getKeyInfo(ch chan<- prometheus.Metric, c redis.Conn, dbLabel
 
 	switch keyType {
 	case "none":
-		log.Debugf("Key '%s' not found when trying to get type and size: using default '0.0'", keyName)
-		e.registerConstMetricGauge(ch, "key_size", 0.0, dbLabel, keyName)
+		log.Debugf("Key '%s' not found, skipping", keyName)
 		return
 
 	case "string":

exporter/latency.go

@@ -4,7 +4,6 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-
 	"sync"
 
 	"github.com/gomodule/redigo/redis"
@@ -63,7 +62,7 @@ func (e *Exporter) extractLatencyHistogramMetrics(outChan chan<- prometheus.Metr
 		return
 	}
 
-	for i := 0; i < len(reply); i += 2 {
+	for i := 0; i+1 < len(reply); i += 2 {
 		cmd, _ := redis.String(reply[i], nil)
 		details, _ := redis.Values(reply[i+1], nil)
 
@@ -76,7 +75,7 @@ func (e *Exporter) extractLatencyHistogramMetrics(outChan chan<- prometheus.Metr
 
 		buckets := map[float64]uint64{}
 
-		for j := 0; j < len(bucketInfo); j += 2 {
+		for j := 0; j+1 < len(bucketInfo); j += 2 {
 			usec := float64(bucketInfo[j])
 			count := bucketInfo[j+1]
 			buckets[usec] = count
@@ -94,7 +93,7 @@ func extractTotalUsecForCommand(infoAll string, cmd string) uint64 {
 
 	matches := extractUsecRegexp.FindAllStringSubmatch(infoAll, -1)
 	for _, match := range matches {
-		if !strings.HasPrefix(match[1], cmd) {
+		if match[1] != cmd && !strings.HasPrefix(match[1], cmd+"|") {
 			continue
 		}
 

exporter/modules.go

@@ -30,13 +30,20 @@ func (e *Exporter) extractModulesMetrics(ch chan<- prometheus.Metric, c redis.Co
 			if len(module) != 7 {
 				continue
 			}
+			extractModuleVal := func(s string) string {
+				parts := strings.SplitN(s, "=", 2)
+				if len(parts) != 2 {
+					return ""
+				}
+				return parts[1]
+			}
 			e.registerConstMetricGauge(ch, "module_info", 1,
-				strings.Split(module[0], "=")[1],
-				strings.Split(module[1], "=")[1],
-				strings.Split(module[2], "=")[1],
-				strings.Split(module[3], "=")[1],
-				strings.Split(module[4], "=")[1],
-				strings.Split(module[5], "=")[1],
+				extractModuleVal(module[0]),
+				extractModuleVal(module[1]),
+				extractModuleVal(module[2]),
+				extractModuleVal(module[3]),
+				extractModuleVal(module[4]),
+				extractModuleVal(module[5]),
 			)
 			continue
 		}

exporter/sentinels.go

@@ -10,6 +10,8 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+var reSentinelMaster = regexp.MustCompile(`^master\d+`)
+
 func (e *Exporter) handleMetricsSentinel(ch chan<- prometheus.Metric, fieldKey string, fieldValue string) {
 	switch fieldKey {
 	case
@@ -197,7 +199,7 @@ valid examples:
 */
 func parseSentinelMasterString(master string, masterInfo string) (masterName string, masterStatus string, masterAddr string, masterSlaves float64, masterSentinels float64, ok bool) {
 	ok = false
-	if matched, _ := regexp.MatchString(`^master\d+`, master); !matched {
+	if !reSentinelMaster.MatchString(master) {
 		return
 	}
 	matchedMasterInfo := make(map[string]string)

exporter/slowlog.go

@@ -20,9 +20,13 @@ func (e *Exporter) extractSlowLogMetrics(ch chan<- prometheus.Metric, c redis.Co
 
 	if len(values) > 0 {
 		if values, err = redis.Values(values[0], err); err == nil && len(values) > 0 {
-			slowlogLastID = values[0].(int64)
+			if id, ok := values[0].(int64); ok {
+				slowlogLastID = id
+			}
 			if len(values) > 2 {
-				lastSlowExecutionDurationSeconds = float64(values[2].(int64)) / 1e6
+				if dur, ok := values[2].(int64); ok {
+					lastSlowExecutionDurationSeconds = float64(dur) / 1e6
+				}
 			}
 		}
 	}

exporter/streams.go

@@ -75,17 +75,18 @@ func getStreamInfo(c redis.Conn, key string) (*streamInfo, error) {
 }
 
 func getStreamEntryId(redisValue []interface{}, index int) string {
-	if values, ok := redisValue[index].([]interface{}); !ok || len(values) < 2 {
+	if index >= len(redisValue) || redisValue[index] == nil {
 		log.Debugf("Failed to parse StreamEntryId")
 		return ""
 	}
 
-	if len(redisValue) < index || redisValue[index] == nil {
+	values, ok := redisValue[index].([]interface{})
+	if !ok || len(values) < 1 {
 		log.Debugf("Failed to parse StreamEntryId")
 		return ""
 	}
 
-	entryId, ok := redisValue[index].([]interface{})[0].([]byte)
+	entryId, ok := values[0].([]byte)
 	if !ok {
 		log.Debugf("Failed to parse StreamEntryId")
 		return ""

exporter/tile38.go

@@ -15,7 +15,7 @@ func (e *Exporter) extractTile38Metrics(ch chan<- prometheus.Metric, c redis.Con
 		return
 	}
 
-	for i := 0; i < len(info); i += 2 {
+	for i := 0; i+1 < len(info); i += 2 {
 		fieldKey := info[i]
 		if !strings.HasPrefix(fieldKey, "tile38_") {
 			fieldKey = "tile38_" + fieldKey