Skip to content

Commit 0631cf2

Browse files
committed
feat: add optional logger wherever possible
This commit introduces an optional logger parameter to various structs. This enhancement allows users to provide custom logging implementations.
1 parent 68d8c59 commit 0631cf2

File tree

15 files changed

+340
-172
lines changed

15 files changed

+340
-172
lines changed

internal/log.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,66 @@ func (l LogLevelT) InfoOrAbove() bool {
7777
func (l LogLevelT) DebugOrAbove() bool {
7878
return l >= LogLevelDebug
7979
}
80+
81+
// LoggerWithLevel is a logger interface with leveled logging methods.
82+
//
83+
// This interface can be implemented by custom loggers to provide leveled logging.
84+
type LoggerWithLevel interface {
85+
// Infof logs an info level message
86+
Infof(ctx context.Context, format string, v ...interface{})
87+
88+
// Warnf logs a warning level message
89+
Warnf(ctx context.Context, format string, v ...interface{})
90+
91+
// Debugf logs a debug level message
92+
Debugf(ctx context.Context, format string, v ...interface{})
93+
94+
// Errorf logs an error level message
95+
Errorf(ctx context.Context, format string, v ...interface{})
96+
97+
// Enabled reports whether the given log level is enabled in the logger
98+
Enabled(ctx context.Context, level LogLevelT) bool
99+
}
100+
101+
// legacyLoggerAdapter is a logger that implements LoggerWithLevel interface
102+
// using the global [Logger] and [LogLevel] variables.
103+
type legacyLoggerAdapter struct{}
104+
105+
func (l *legacyLoggerAdapter) Infof(ctx context.Context, format string, v ...interface{}) {
106+
if LogLevel.InfoOrAbove() {
107+
Logger.Printf(ctx, format, v...)
108+
}
109+
}
110+
111+
func (l *legacyLoggerAdapter) Warnf(ctx context.Context, format string, v ...interface{}) {
112+
if LogLevel.WarnOrAbove() {
113+
Logger.Printf(ctx, format, v...)
114+
}
115+
}
116+
117+
func (l *legacyLoggerAdapter) Debugf(ctx context.Context, format string, v ...interface{}) {
118+
if LogLevel.DebugOrAbove() {
119+
Logger.Printf(ctx, format, v...)
120+
}
121+
}
122+
123+
func (l legacyLoggerAdapter) Errorf(ctx context.Context, format string, v ...interface{}) {
124+
Logger.Printf(ctx, format, v...)
125+
}
126+
127+
func (l legacyLoggerAdapter) Enabled(_ context.Context, level LogLevelT) bool {
128+
switch level {
129+
case LogLevelWarn:
130+
return LogLevel.WarnOrAbove()
131+
case LogLevelInfo:
132+
return LogLevel.InfoOrAbove()
133+
case LogLevelDebug:
134+
return LogLevel.DebugOrAbove()
135+
case LogLevelError:
136+
fallthrough
137+
default:
138+
return true
139+
}
140+
}
141+
142+
var LegacyLoggerWithLevel LoggerWithLevel = &legacyLoggerAdapter{}

internal/pool/pool.go

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ type Options struct {
119119
// DialerRetryTimeout is the backoff duration between retry attempts.
120120
// Default: 100ms
121121
DialerRetryTimeout time.Duration
122+
123+
// Optional logger for connection pool operations.
124+
Logger internal.LoggerWithLevel
122125
}
123126

124127
type lastDialErrorWrap struct {
@@ -254,7 +257,7 @@ func (p *ConnPool) checkMinIdleConns() {
254257
p.idleConnsLen.Add(-1)
255258

256259
p.freeTurn()
257-
internal.Logger.Printf(context.Background(), "addIdleConn panic: %+v", err)
260+
p.logger().Errorf(context.Background(), "addIdleConn panic: %+v", err)
258261
}
259262
}()
260263

@@ -416,7 +419,7 @@ func (p *ConnPool) dialConn(ctx context.Context, pooled bool) (*Conn, error) {
416419
return cn, nil
417420
}
418421

419-
internal.Logger.Printf(ctx, "redis: connection pool: failed to dial after %d attempts: %v", attempt, lastErr)
422+
p.logger().Errorf(ctx, "redis: connection pool: failed to dial after %d attempts: %v", attempt, lastErr)
420423
// All retries failed - handle error tracking
421424
p.setLastDialError(lastErr)
422425
if atomic.AddUint32(&p.dialErrorsNum, 1) == uint32(p.cfg.PoolSize) {
@@ -510,10 +513,10 @@ func (p *ConnPool) getConn(ctx context.Context) (*Conn, error) {
510513
acceptConn, err := hookManager.ProcessOnGet(ctx, cn, false)
511514
if err != nil || !acceptConn {
512515
if err != nil {
513-
internal.Logger.Printf(ctx, "redis: connection pool: failed to process idle connection by hook: %v", err)
516+
p.logger().Errorf(ctx, "redis: connection pool: failed to process idle connection by hook: %v", err)
514517
_ = p.CloseConn(cn)
515518
} else {
516-
internal.Logger.Printf(ctx, "redis: connection pool: conn[%d] rejected by hook, returning to pool", cn.GetID())
519+
p.logger().Errorf(ctx, "redis: connection pool: conn[%d] rejected by hook, returning to pool", cn.GetID())
517520
// Return connection to pool without freeing the turn that this Get() call holds.
518521
// We use putConnWithoutTurn() to run all the Put hooks and logic without freeing a turn.
519522
p.putConnWithoutTurn(ctx, cn)
@@ -541,7 +544,7 @@ func (p *ConnPool) getConn(ctx context.Context) (*Conn, error) {
541544
// this should not happen with a new connection, but we handle it gracefully
542545
if err != nil || !acceptConn {
543546
// Failed to process connection, discard it
544-
internal.Logger.Printf(ctx, "redis: connection pool: failed to process new connection conn[%d] by hook: accept=%v, err=%v", newcn.GetID(), acceptConn, err)
547+
p.logger().Errorf(ctx, "redis: connection pool: failed to process new connection conn[%d] by hook: accept=%v, err=%v", newcn.GetID(), acceptConn, err)
545548
_ = p.CloseConn(newcn)
546549
return nil, err
547550
}
@@ -584,7 +587,7 @@ func (p *ConnPool) queuedNewConn(ctx context.Context) (*Conn, error) {
584587
if !freeTurnCalled {
585588
p.freeTurn()
586589
}
587-
internal.Logger.Printf(context.Background(), "queuedNewConn panic: %+v", err)
590+
p.logger().Errorf(ctx, "queuedNewConn panic: %+v", err)
588591
}
589592
}()
590593

@@ -728,7 +731,7 @@ func (p *ConnPool) popIdle() (*Conn, error) {
728731

729732
// If we exhausted all attempts without finding a usable connection, return nil
730733
if attempts > 1 && attempts >= maxAttempts && int32(attempts) >= p.poolSize.Load() {
731-
internal.Logger.Printf(context.Background(), "redis: connection pool: failed to get a usable connection after %d attempts", attempts)
734+
p.logger().Errorf(context.Background(), "redis: connection pool: failed to get a usable connection after %d attempts", attempts)
732735
return nil, nil
733736
}
734737

@@ -757,7 +760,7 @@ func (p *ConnPool) putConn(ctx context.Context, cn *Conn, freeTurn bool) {
757760
// Peek at the reply type to check if it's a push notification
758761
if replyType, err := cn.PeekReplyTypeSafe(); err != nil || replyType != proto.RespPush {
759762
// Not a push notification or error peeking, remove connection
760-
internal.Logger.Printf(ctx, "Conn has unread data (not push notification), removing it")
763+
p.logger().Errorf(ctx, "Conn has unread data (not push notification), removing it")
761764
p.removeConnInternal(ctx, cn, err, freeTurn)
762765
return
763766
}
@@ -770,7 +773,7 @@ func (p *ConnPool) putConn(ctx context.Context, cn *Conn, freeTurn bool) {
770773
if hookManager != nil {
771774
shouldPool, shouldRemove, err = hookManager.ProcessOnPut(ctx, cn)
772775
if err != nil {
773-
internal.Logger.Printf(ctx, "Connection hook error: %v", err)
776+
p.logger().Errorf(ctx, "Connection hook error: %v", err)
774777
p.removeConnInternal(ctx, cn, err, freeTurn)
775778
return
776779
}
@@ -803,12 +806,12 @@ func (p *ConnPool) putConn(ctx context.Context, cn *Conn, freeTurn bool) {
803806
case StateUnusable:
804807
// expected state, don't log it
805808
case StateClosed:
806-
internal.Logger.Printf(ctx, "Unexpected conn[%d] state changed by hook to %v, closing it", cn.GetID(), currentState)
809+
p.logger().Errorf(ctx, "Unexpected conn[%d] state changed by hook to %v, closing it", cn.GetID(), currentState)
807810
shouldCloseConn = true
808811
p.removeConnWithLock(cn)
809812
default:
810813
// Pool as-is
811-
internal.Logger.Printf(ctx, "Unexpected conn[%d] state changed by hook to %v, pooling as-is", cn.GetID(), currentState)
814+
p.logger().Warnf(ctx, "Unexpected conn[%d] state changed by hook to %v, pooling as-is", cn.GetID(), currentState)
812815
}
813816
}
814817

@@ -1022,7 +1025,7 @@ func (p *ConnPool) isHealthyConn(cn *Conn, nowNs int64) bool {
10221025
if replyType, err := cn.rd.PeekReplyType(); err == nil && replyType == proto.RespPush {
10231026
// For RESP3 connections with push notifications, we allow some buffered data
10241027
// The client will process these notifications before using the connection
1025-
internal.Logger.Printf(
1028+
p.logger().Infof(
10261029
context.Background(),
10271030
"push: conn[%d] has buffered data, likely push notifications - will be processed by client",
10281031
cn.GetID(),
@@ -1045,3 +1048,11 @@ func (p *ConnPool) isHealthyConn(cn *Conn, nowNs int64) bool {
10451048
cn.SetUsedAtNs(nowNs)
10461049
return true
10471050
}
1051+
1052+
func (p *ConnPool) logger() internal.LoggerWithLevel {
1053+
if p.cfg.Logger != nil {
1054+
return p.cfg.Logger
1055+
}
1056+
1057+
return internal.LegacyLoggerWithLevel
1058+
}

logging/logging.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,4 @@ func (l *filterLogger) Printf(ctx context.Context, format string, v ...interface
8989
return
9090
}
9191
}
92+

maintnotifications/circuit_breaker.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,7 @@ func (cb *CircuitBreaker) Execute(fn func() error) error {
102102
if cb.state.CompareAndSwap(int32(CircuitBreakerOpen), int32(CircuitBreakerHalfOpen)) {
103103
cb.requests.Store(0)
104104
cb.successes.Store(0)
105-
if internal.LogLevel.InfoOrAbove() {
106-
internal.Logger.Printf(context.Background(), logs.CircuitBreakerTransitioningToHalfOpen(cb.endpoint))
107-
}
105+
cb.logger().Infof(context.Background(), logs.CircuitBreakerTransitioningToHalfOpen(cb.endpoint))
108106
// Fall through to half-open logic
109107
} else {
110108
return ErrCircuitBreakerOpen
@@ -144,17 +142,13 @@ func (cb *CircuitBreaker) recordFailure() {
144142
case CircuitBreakerClosed:
145143
if failures >= int64(cb.failureThreshold) {
146144
if cb.state.CompareAndSwap(int32(CircuitBreakerClosed), int32(CircuitBreakerOpen)) {
147-
if internal.LogLevel.WarnOrAbove() {
148-
internal.Logger.Printf(context.Background(), logs.CircuitBreakerOpened(cb.endpoint, failures))
149-
}
145+
cb.logger().Warnf(context.Background(), logs.CircuitBreakerOpened(cb.endpoint, failures))
150146
}
151147
}
152148
case CircuitBreakerHalfOpen:
153149
// Any failure in half-open state immediately opens the circuit
154150
if cb.state.CompareAndSwap(int32(CircuitBreakerHalfOpen), int32(CircuitBreakerOpen)) {
155-
if internal.LogLevel.WarnOrAbove() {
156-
internal.Logger.Printf(context.Background(), logs.CircuitBreakerReopened(cb.endpoint))
157-
}
151+
cb.logger().Warnf(context.Background(), logs.CircuitBreakerReopened(cb.endpoint))
158152
}
159153
}
160154
}
@@ -176,9 +170,7 @@ func (cb *CircuitBreaker) recordSuccess() {
176170
if successes >= int64(cb.maxRequests) {
177171
if cb.state.CompareAndSwap(int32(CircuitBreakerHalfOpen), int32(CircuitBreakerClosed)) {
178172
cb.failures.Store(0)
179-
if internal.LogLevel.InfoOrAbove() {
180-
internal.Logger.Printf(context.Background(), logs.CircuitBreakerClosed(cb.endpoint, successes))
181-
}
173+
cb.logger().Infof(context.Background(), logs.CircuitBreakerClosed(cb.endpoint, successes))
182174
}
183175
}
184176
}
@@ -202,6 +194,13 @@ func (cb *CircuitBreaker) GetStats() CircuitBreakerStats {
202194
}
203195
}
204196

197+
func (cb *CircuitBreaker) logger() internal.LoggerWithLevel {
198+
if cb.config != nil && cb.config.Logger != nil {
199+
return cb.config.Logger
200+
}
201+
return internal.LegacyLoggerWithLevel
202+
}
203+
205204
// CircuitBreakerStats provides statistics about a circuit breaker
206205
type CircuitBreakerStats struct {
207206
Endpoint string
@@ -325,8 +324,8 @@ func (cbm *CircuitBreakerManager) cleanup() {
325324
}
326325

327326
// Log cleanup results
328-
if len(toDelete) > 0 && internal.LogLevel.InfoOrAbove() {
329-
internal.Logger.Printf(context.Background(), logs.CircuitBreakerCleanup(len(toDelete), count))
327+
if len(toDelete) > 0 {
328+
cbm.logger().Infof(context.Background(), logs.CircuitBreakerCleanup(len(toDelete), count))
330329
}
331330

332331
cbm.lastCleanup.Store(now.Unix())
@@ -351,3 +350,10 @@ func (cbm *CircuitBreakerManager) Reset() {
351350
return true
352351
})
353352
}
353+
354+
func (cbm *CircuitBreakerManager) logger() internal.LoggerWithLevel {
355+
if cbm.config != nil && cbm.config.Logger != nil {
356+
return cbm.config.Logger
357+
}
358+
return internal.LegacyLoggerWithLevel
359+
}

maintnotifications/config.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,9 @@ type Config struct {
128128
// After this many retries, the connection will be removed from the pool.
129129
// Default: 3
130130
MaxHandoffRetries int
131+
132+
// Logger is an optional custom logger for maintenance notifications.
133+
Logger internal.LoggerWithLevel
131134
}
132135

133136
func (c *Config) IsEnabled() bool {
@@ -312,10 +315,9 @@ func (c *Config) ApplyDefaultsWithPoolConfig(poolSize int, maxActiveConns int) *
312315
result.CircuitBreakerMaxRequests = c.CircuitBreakerMaxRequests
313316
}
314317

315-
if internal.LogLevel.DebugOrAbove() {
316-
internal.Logger.Printf(context.Background(), logs.DebugLoggingEnabled())
317-
internal.Logger.Printf(context.Background(), logs.ConfigDebug(result))
318-
}
318+
c.logger().Debugf(context.Background(), logs.DebugLoggingEnabled())
319+
c.logger().Debugf(context.Background(), logs.ConfigDebug(result))
320+
319321
return result
320322
}
321323

@@ -341,6 +343,8 @@ func (c *Config) Clone() *Config {
341343

342344
// Configuration fields
343345
MaxHandoffRetries: c.MaxHandoffRetries,
346+
347+
Logger: c.Logger,
344348
}
345349
}
346350

@@ -365,6 +369,13 @@ func (c *Config) applyWorkerDefaults(poolSize int) {
365369
}
366370
}
367371

372+
func (c *Config) logger() internal.LoggerWithLevel {
373+
if c.Logger != nil {
374+
return c.Logger
375+
}
376+
return internal.LegacyLoggerWithLevel
377+
}
378+
368379
// DetectEndpointType automatically detects the appropriate endpoint type
369380
// based on the connection address and TLS configuration.
370381
//

0 commit comments

Comments
 (0)