cleanup the logging a bit

Author: ndyakov

async_handoff_integration_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/redis/go-redis/v9/hitless"
 	"github.com/redis/go-redis/v9/internal/pool"
+	"github.com/redis/go-redis/v9/logging"
 )
 
 // mockNetConn implements net.Conn for testing
@@ -348,5 +349,5 @@ func TestEventDrivenHandoffIntegration(t *testing.T) {
 }
 
 func init() {
-	SetLogger(&VoidLogger{})
+	logging.Disable()
 }

hitless/config.go

@@ -12,17 +12,6 @@ import (
 	"github.com/redis/go-redis/v9/logging"
 )
 
-// LogLevel represents the logging level for hitless upgrades
-type LogLevel = logging.LogLevel
-
-// Log level constants for hitless upgrades
-const (
-	LogLevelError LogLevel = logging.LogLevelError // 0 - errors only
-	LogLevelWarn  LogLevel = logging.LogLevelWarn  // 1 - warnings and errors
-	LogLevelInfo  LogLevel = logging.LogLevelInfo  // 2 - info, warnings, and errors
-	LogLevelDebug LogLevel = logging.LogLevelDebug // 3 - debug, info, warnings, and errors
-)
-
 // MaintNotificationsMode represents the maintenance notifications mode
 type MaintNotificationsMode string
 
@@ -124,8 +113,8 @@ type Config struct {
 
 	// LogLevel controls the verbosity of hitless upgrade logging.
 	// LogLevelError (0) = errors only, LogLevelWarn (1) = warnings, LogLevelInfo (2) = info, LogLevelDebug (3) = debug
-	// Default: LogLevelError(0)
-	LogLevel LogLevel
+	// Default: logging.LogLevelError(0)
+	LogLevel logging.LogLevel
 
 	// MaxHandoffRetries is the maximum number of times to retry a failed handoff.
 	// After this many retries, the connection will be removed from the pool.
@@ -291,7 +280,7 @@ func (c *Config) ApplyDefaultsWithPoolConfig(poolSize int, maxActiveConns int) *
 		result.MaxHandoffRetries = c.MaxHandoffRetries
 	}
 
-	if result.LogLevel >= LogLevelDebug {
+	if result.LogLevel.DebugOrAbove() {
 		internal.Logger.Printf(context.Background(), "hitless: debug logging enabled")
 		internal.Logger.Printf(context.Background(), "hitless: config: %+v", result)
 	}

hitless/hitless_manager.go

@@ -148,12 +148,12 @@ func (hm *HitlessManager) TrackMovingOperationWithConnID(ctx context.Context, ne
 	// Use LoadOrStore for atomic check-and-set operation
 	if _, loaded := hm.activeMovingOps.LoadOrStore(key, movingOp); loaded {
 		// Duplicate MOVING notification, ignore
-		if hm.config.LogLevel >= LogLevelDebug { // Debug level
+		if hm.config.LogLevel.DebugOrAbove() { // Debug level
 			internal.Logger.Printf(context.Background(), "hitless: conn[%d] seqID[%d] Duplicate MOVING operation ignored: %s", connID, seqID, key.String())
 		}
 		return nil
 	}
-	if hm.config.LogLevel >= LogLevelDebug { // Debug level
+	if hm.config.LogLevel.DebugOrAbove() { // Debug level
 		internal.Logger.Printf(context.Background(), "hitless: conn[%d] seqID[%d] Tracking MOVING operation: %s", connID, seqID, key.String())
 	}
 
@@ -173,13 +173,13 @@ func (hm *HitlessManager) UntrackOperationWithConnID(seqID int64, connID uint64)
 
 	// Remove from active operations atomically
 	if _, loaded := hm.activeMovingOps.LoadAndDelete(key); loaded {
-		if hm.config.LogLevel >= LogLevelDebug { // Debug level
+		if hm.config.LogLevel.DebugOrAbove() { // Debug level
 			internal.Logger.Printf(context.Background(), "hitless: conn[%d] seqID[%d] Untracking MOVING operation: %s", connID, seqID, key.String())
 		}
 		// Decrement active operation count only if operation existed
 		hm.activeOperationCount.Add(-1)
 	} else {
-		if hm.config.LogLevel >= LogLevelDebug { // Debug level
+		if hm.config.LogLevel.DebugOrAbove() { // Debug level
 			internal.Logger.Printf(context.Background(), "hitless: conn[%d] seqID[%d] Operation not found for untracking: %s", connID, seqID, key.String())
 		}
 	}

hitless/hooks.go

@@ -16,7 +16,7 @@ type LoggingHook struct {
 
 // PreHook logs the notification before processing and allows modification.
 func (lh *LoggingHook) PreHook(ctx context.Context, notificationCtx push.NotificationHandlerContext, notificationType string, notification []interface{}) ([]interface{}, bool) {
-	if lh.LogLevel >= logging.LogLevelInfo { // Info level
+	if lh.LogLevel.InfoOrAbove() { // Info level
 		// Log the notification type and content
 		connID := uint64(0)
 		if conn, ok := notificationCtx.Conn.(*pool.Conn); ok {
@@ -33,9 +33,9 @@ func (lh *LoggingHook) PostHook(ctx context.Context, notificationCtx push.Notifi
 	if conn, ok := notificationCtx.Conn.(*pool.Conn); ok {
 		connID = conn.GetID()
 	}
-	if result != nil && lh.LogLevel >= logging.LogLevelWarn { // Warning level
+	if result != nil && lh.LogLevel.WarnOrAbove() { // Warning level
 		internal.Logger.Printf(ctx, "hitless: conn[%d] %s notification processing failed: %v - %v", connID, notificationType, result, notification)
-	} else if lh.LogLevel >= logging.LogLevelDebug { // Debug level
+	} else if lh.LogLevel.DebugOrAbove() { // Debug level
 		internal.Logger.Printf(ctx, "hitless: conn[%d] %s notification processed successfully", connID, notificationType)
 	}
 }

hitless/pool_hook.go

@@ -207,7 +207,7 @@ func (ph *PoolHook) onDemandWorker() {
 			return
 		case <-time.After(ph.workerTimeout):
 			// Worker has been idle for too long, exit to save resources
-			if ph.config != nil && ph.config.LogLevel >= LogLevelDebug { // Debug level
+			if ph.config != nil && ph.config.LogLevel.InfoOrAbove() { // Debug level
 				internal.Logger.Printf(context.Background(),
 					"hitless: worker exiting due to inactivity timeout (%v)", ph.workerTimeout)
 			}
@@ -302,14 +302,14 @@ func (ph *PoolHook) closeConnFromRequest(ctx context.Context, request HandoffReq
 	conn := request.Conn
 	if pooler != nil {
 		pooler.Remove(ctx, conn, err)
-		if ph.config != nil && ph.config.LogLevel >= LogLevelWarn { // Warning level
+		if ph.config != nil && ph.config.LogLevel.WarnOrAbove() { // Warning level
 			internal.Logger.Printf(ctx,
 				"hitless: removed conn[%d] from pool due to max handoff retries reached: %v",
 				conn.GetID(), err)
 		}
 	} else {
 		conn.Close()
-		if ph.config != nil && ph.config.LogLevel >= LogLevelWarn { // Warning level
+		if ph.config != nil && ph.config.LogLevel.WarnOrAbove() { // Warning level
 			internal.Logger.Printf(ctx,
 				"hitless: no pool provided for conn[%d], cannot remove due to handoff initialization failure: %v",
 				conn.GetID(), err)
@@ -357,11 +357,11 @@ func (ph *PoolHook) queueHandoff(conn *pool.Conn) error {
 				// Queue is full - log and attempt scaling
 				queueLen := len(ph.handoffQueue)
 				queueCap := cap(ph.handoffQueue)
-				if ph.config != nil && ph.config.LogLevel >= LogLevelWarn { // Warning level
+				if ph.config != nil && ph.config.LogLevel.WarnOrAbove() { // Warning level
 					internal.Logger.Printf(context.Background(),
 						"hitless: handoff queue is full (%d/%d), cant queue handoff request for conn[%d] seqID[%d]",
 						queueLen, queueCap, request.ConnID, request.SeqID)
-					if ph.config.LogLevel >= LogLevelDebug { // Debug level
+					if ph.config.LogLevel.DebugOrAbove() { // Debug level
 						ph.pending.Range(func(k, v interface{}) bool {
 							internal.Logger.Printf(context.Background(), "hitless: pending handoff for conn[%d] seqID[%d]", k, v)
 							return true
@@ -396,7 +396,7 @@ func (ph *PoolHook) performConnectionHandoff(ctx context.Context, conn *pool.Con
 	}
 
 	if retries > maxRetries {
-		if ph.config != nil && ph.config.LogLevel >= LogLevelWarn { // Warning level
+		if ph.config != nil && ph.config.LogLevel.WarnOrAbove() { // Warning level
 			internal.Logger.Printf(ctx,
 				"hitless: reached max retries (%d) for handoff of conn[%d] to %s",
 				maxRetries, conn.GetID(), conn.GetHandoffEndpoint())
@@ -430,7 +430,7 @@ func (ph *PoolHook) performConnectionHandoff(ctx context.Context, conn *pool.Con
 		deadline := time.Now().Add(ph.config.PostHandoffRelaxedDuration)
 		conn.SetRelaxedTimeoutWithDeadline(relaxedTimeout, relaxedTimeout, deadline)
 
-		if ph.config.LogLevel >= 2 { // Info level
+		if ph.config.LogLevel.InfoOrAbove() {
 			internal.Logger.Printf(context.Background(),
 				"hitless: conn[%d] applied post-handoff relaxed timeout (%v) until %v",
 				connID, relaxedTimeout, deadline.Format("15:04:05.000"))

hitless/push_notification_handler.go

@@ -115,7 +115,7 @@ func (snh *NotificationHandler) handleMoving(ctx context.Context, handlerCtx pus
 	deadline := time.Now().Add(time.Duration(timeS) * time.Second)
 	// If newEndpoint is empty, we should schedule a handoff to the current endpoint in timeS/2 seconds
 	if newEndpoint == "" || newEndpoint == internal.RedisNull {
-		if snh.manager.config.LogLevel >= LogLevelDebug { // Debug level
+		if snh.manager.config.LogLevel.DebugOrAbove() { // Debug level
 			internal.Logger.Printf(ctx, "hitless: conn[%d] scheduling handoff to current endpoint in %v seconds",
 				poolConn.GetID(), timeS/2)
 		}
@@ -179,7 +179,7 @@ func (snh *NotificationHandler) handleMigrating(ctx context.Context, handlerCtx
 	}
 
 	// Apply relaxed timeout to this specific connection
-	if snh.manager.config.LogLevel >= LogLevelDebug { // Debug level
+	if snh.manager.config.LogLevel.InfoOrAbove() { // Debug level
 		internal.Logger.Printf(ctx, "hitless: conn[%d] applying relaxed timeout (%v) for MIGRATING notification",
 			conn.GetID(),
 			snh.manager.config.RelaxedTimeout)
@@ -209,7 +209,7 @@ func (snh *NotificationHandler) handleMigrated(ctx context.Context, handlerCtx p
 	}
 
 	// Clear relaxed timeout for this specific connection
-	if snh.manager.config.LogLevel >= LogLevelDebug { // Debug level
+	if snh.manager.config.LogLevel.InfoOrAbove() { // Debug level
 		connID := conn.GetID()
 		internal.Logger.Printf(ctx, "hitless: conn[%d] clearing relaxed timeout for MIGRATED notification", connID)
 	}
@@ -238,7 +238,7 @@ func (snh *NotificationHandler) handleFailingOver(ctx context.Context, handlerCt
 	}
 
 	// Apply relaxed timeout to this specific connection
-	if snh.manager.config.LogLevel >= LogLevelDebug { // Debug level
+	if snh.manager.config.LogLevel.InfoOrAbove() { // Debug level
 		connID := conn.GetID()
 		internal.Logger.Printf(ctx, "hitless: conn[%d] applying relaxed timeout (%v) for FAILING_OVER notification", connID, snh.manager.config.RelaxedTimeout)
 	}
@@ -267,7 +267,7 @@ func (snh *NotificationHandler) handleFailedOver(ctx context.Context, handlerCtx
 	}
 
 	// Clear relaxed timeout for this specific connection
-	if snh.manager.config.LogLevel >= LogLevelDebug { // Debug level
+	if snh.manager.config.LogLevel.InfoOrAbove() { // Debug level
 		connID := conn.GetID()
 		internal.Logger.Printf(ctx, "hitless: conn[%d] clearing relaxed timeout for FAILED_OVER notification", connID)
 	}

internal/log.go

@@ -5,20 +5,6 @@ import (
 	"fmt"
 	"log"
 	"os"
-	"strings"
-
-	"github.com/redis/go-redis/v9/logging"
-)
-
-// LogLevel is an alias to the public logging.LogLevel for internal use
-type LogLevel = logging.LogLevel
-
-// Log level constants for internal use
-const (
-	LogLevelError = logging.LogLevelError
-	LogLevelWarn  = logging.LogLevelWarn
-	LogLevelInfo  = logging.LogLevelInfo
-	LogLevelDebug = logging.LogLevelDebug
 )
 
 // TODO (ned): Revisit logging
@@ -28,41 +14,20 @@ type Logging interface {
 	Printf(ctx context.Context, format string, v ...interface{})
 }
 
-type logger struct {
+type DefaultLogger struct {
 	log *log.Logger
 }
 
-func (l *logger) Printf(ctx context.Context, format string, v ...interface{}) {
+func (l *DefaultLogger) Printf(ctx context.Context, format string, v ...interface{}) {
 	_ = l.log.Output(2, fmt.Sprintf(format, v...))
 }
 
-// Logger calls Output to print to the stderr.
-// Arguments are handled in the manner of fmt.Print.
-var Logger Logging = &logger{
-	log: log.New(os.Stderr, "redis: ", log.LstdFlags|log.Lshortfile),
-}
-
-func NewFilterLogger(substr []string) Logging {
-	l := &logger{
+func NewDefaultLogger() Logging {
+	return &DefaultLogger{
 		log: log.New(os.Stderr, "redis: ", log.LstdFlags|log.Lshortfile),
 	}
-	return &filterLogger{logger: l, substr: substr}
 }
 
-type filterLogger struct {
-	logger Logging
-	substr []string
-}
-
-func (l *filterLogger) Printf(ctx context.Context, format string, v ...interface{}) {
-	msg := fmt.Sprintf(format, v...)
-	for _, substr := range l.substr {
-		if strings.Contains(msg, substr) {
-			return
-		}
-	}
-	if l.logger != nil {
-		l.logger.Printf(ctx, format, v...)
-		return
-	}
-}
+// Logger calls Output to print to the stderr.
+// Arguments are handled in the manner of fmt.Print.
+var Logger Logging = NewDefaultLogger()

internal/pool/pool_test.go

@@ -12,6 +12,7 @@ import (
 	. "github.com/bsm/gomega"
 	"github.com/redis/go-redis/v9"
 	"github.com/redis/go-redis/v9/internal/pool"
+	"github.com/redis/go-redis/v9/logging"
 )
 
 var _ = Describe("ConnPool", func() {
@@ -438,5 +439,5 @@ var _ = Describe("race", func() {
 })
 
 func init() {
-	redis.SetLogger(&redis.VoidLogger{})
+	logging.Disable()
 }

logging/logging.go

@@ -2,6 +2,14 @@
 // This package centralizes logging configuration to ensure consistency across all components.
 package logging
 
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/redis/go-redis/v9/internal"
+)
+
 // LogLevel represents the logging level
 type LogLevel int
 
@@ -33,3 +41,81 @@ func (l LogLevel) String() string {
 func (l LogLevel) IsValid() bool {
 	return l >= LogLevelError && l <= LogLevelDebug
 }
+
+func (l LogLevel) WarnOrAbove() bool {
+	return l >= LogLevelWarn
+}
+
+func (l LogLevel) InfoOrAbove() bool {
+	return l >= LogLevelInfo
+}
+
+func (l LogLevel) DebugOrAbove() bool {
+	return l >= LogLevelDebug
+}
+
+// VoidLogger is a logger that does nothing.
+// Used to disable logging and thus speed up the library.
+type VoidLogger struct{}
+
+func (v *VoidLogger) Printf(_ context.Context, _ string, _ ...interface{}) {
+	// do nothing
+}
+
+// Disable disables logging by setting the internal logger to a void logger.
+// This can be used to speed up the library if logging is not needed.
+// It will override any custom logger that was set before and set the VoidLogger.
+func Disable() {
+	internal.Logger = &VoidLogger{}
+}
+
+// Enable enables logging by setting the internal logger to the default logger.
+// This is the default behavior.
+// You can use redis.SetLogger to set a custom logger.
+//
+// NOTE: This function is not thread-safe.
+// It will override any custom logger that was set before and set the DefaultLogger.
+func Enable() {
+	internal.Logger = internal.NewDefaultLogger()
+}
+
+// NewBlacklistLogger returns a new logger that filters out messages containing any of the substrings.
+// This can be used to filter out messages containing sensitive information.
+func NewBlacklistLogger(substr []string) internal.Logging {
+	l := internal.NewDefaultLogger()
+	return &filterLogger{logger: l, substr: substr, blacklist: true}
+}
+
+// NewWhitelistLogger returns a new logger that only logs messages containing any of the substrings.
+// This can be used to only log messages related to specific commands or patterns.
+func NewWhitelistLogger(substr []string) internal.Logging {
+	l := internal.NewDefaultLogger()
+	return &filterLogger{logger: l, substr: substr, blacklist: false}
+}
+
+type filterLogger struct {
+	logger    internal.Logging
+	blacklist bool
+	substr    []string
+}
+
+func (l *filterLogger) Printf(ctx context.Context, format string, v ...interface{}) {
+	msg := fmt.Sprintf(format, v...)
+	found := false
+	for _, substr := range l.substr {
+		if strings.Contains(msg, substr) {
+			found = true
+			if l.blacklist {
+				return
+			}
+		}
+	}
+	// whitelist, only log if one of the substrings is present
+	if !l.blacklist && !found {
+		return
+	}
+	if l.logger != nil {
+		l.logger.Printf(ctx, format, v...)
+		return
+	}
+}