From 4ccf579a79824f8f31b3cae805ee9914924bbd36 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 9 Jan 2025 15:32:10 -0800 Subject: [PATCH 1/3] WIP: go-redis v9 support --- go.mod | 4 +- go.sum | 19 ++++----- redis/db/redisbp/config.go | 6 +-- redis/db/redisbp/config_test.go | 18 ++++----- redis/db/redisbp/doc.go | 2 +- redis/db/redisbp/example_hooks_test.go | 8 ++-- .../redisbp/example_monitored_client_test.go | 2 +- redis/db/redisbp/hooks.go | 40 ++++++++++++++++++- redis/db/redisbp/hooks_test.go | 2 +- redis/db/redisbp/monitored_client.go | 2 +- redis/db/redisbp/monitored_client_test.go | 2 +- redis/db/redisbp/prometheus_test.go | 2 +- 12 files changed, 71 insertions(+), 36 deletions(-) diff --git a/go.mod b/go.mod index 9eb78ce30..e789a4b16 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/fsnotify/fsnotify v1.5.4 github.com/getsentry/sentry-go v0.11.0 github.com/go-kit/kit v0.9.0 - github.com/go-redis/redis/v8 v8.11.5 github.com/gofrs/uuid v3.2.0+incompatible github.com/google/go-cmp v0.6.0 github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 @@ -19,6 +18,7 @@ require ( github.com/opentracing/opentracing-go v1.2.0 github.com/prometheus/client_golang v1.19.0 github.com/prometheus/client_model v0.5.0 + github.com/redis/go-redis/v9 v9.7.0 github.com/sony/gobreaker v0.4.1 go.uber.org/automaxprocs v1.5.1 go.uber.org/zap v1.24.0 @@ -33,7 +33,7 @@ require ( github.com/VividCortex/gohistogram v1.0.0 // indirect github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/cespare/xxhash/v2 v2.2.0 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/chasex/redis-go-cluster v1.0.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect diff --git a/go.sum b/go.sum index 2d48c7c67..20a47bb45 100644 --- a/go.sum +++ b/go.sum @@ -30,9 +30,13 @@ github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLj github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs= +github.com/bsm/ginkgo/v2 v2.12.0/go.mod h1:SwYbGRRDovPVboqFv0tPTcG1sN61LM1Z4ARdbAV9g4c= +github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= +github.com/bsm/gomega v1.27.10/go.mod h1:JyEr/xRbxbtgWNi8tIEVPUYZ5Dzef52k01W3YH0H+O0= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= -github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= -github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chasex/redis-go-cluster v1.0.0 h1:eryAqclX9j1cX/BaR2mXZBQo4JdJdXSEZFWWgbl/7o8= github.com/chasex/redis-go-cluster v1.0.0/go.mod h1:hnZrM/dppeGCj1FS+cOHzQhKyQCBFga/FLXM7pZF5Yg= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= @@ -103,8 +107,6 @@ github.com/go-openapi/jsonreference v0.19.5 h1:1WJP/wi4OjB4iV8KVbH73rQaoialJrqv8 github.com/go-openapi/jsonreference v0.19.5/go.mod h1:RdybgQwPxbL4UEjuAruzK1x3nE69AqPYEJeo/TWfEeg= github.com/go-openapi/swag v0.19.14 h1:gm3vOOXfiuw5i9p5N9xJvfjvuofpyvLA9Wr6QfK5Fng= github.com/go-openapi/swag v0.19.14/go.mod h1:QYRuS/SOXUCsnplDa677K7+DxSOj6IPNl/eQntq43wQ= -github.com/go-redis/redis/v8 v8.11.5 h1:AcZZR7igkdvfVmQTPnu9WE37LRrO/YrBH5zWyjDC0oI= -github.com/go-redis/redis/v8 v8.11.5/go.mod h1:gREzHqY1hg6oD9ngVRbLStwAWKhA0FEgq8Jd4h5lpwo= github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee/go.mod h1:L0fX3K22YWvt/FAX9NnzrNzcI4wNYi9Yku4O0LKYflo= @@ -228,15 +230,9 @@ github.com/nats-io/jwt v0.3.0/go.mod h1:fRYCDE99xlTsqUzISS1Bi75UBJ6ljOJQOAAu5Vgl github.com/nats-io/nats.go v1.9.1/go.mod h1:ZjDU1L/7fJ09jvUSRVBR2e7+RnLiiIQyqyzEE/Zbp4w= github.com/nats-io/nkeys v0.1.0/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxziKVo7w= github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= -github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= -github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.10.3/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= -github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= -github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= -github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc= @@ -263,6 +259,8 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= +github.com/redis/go-redis/v9 v9.7.0 h1:HhLSs+B6O021gwzl+locl0zEDnyNkxMtf/Z3NNBMa9E= +github.com/redis/go-redis/v9 v9.7.0/go.mod h1:f6zhXITC7JUJIlPEiBOTXxJgPLdZcA93GewI7inzyWw= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= @@ -435,7 +433,6 @@ gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8 gopkg.in/go-playground/validator.v8 v8.18.2/go.mod h1:RX2a/7Ha8BgOhfk7j780h4/u/RRjR0eouCJSH80/M2Y= gopkg.in/ini.v1 v1.51.1/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA= -gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/redis/db/redisbp/config.go b/redis/db/redisbp/config.go index dec6b2db7..94009139e 100644 --- a/redis/db/redisbp/config.go +++ b/redis/db/redisbp/config.go @@ -4,7 +4,7 @@ import ( "fmt" "time" - "github.com/go-redis/redis/v8" + "github.com/redis/go-redis/v9" ) // ClientConfig can be used to configure a redis-go "Client". See the docs for @@ -150,7 +150,7 @@ func (opts PoolOptions) ApplyOptions(options *redis.Options) { options.MinIdleConns = opts.MinIdleConnections } if opts.MaxConnectionAge != 0 { - options.MaxConnAge = opts.MaxConnectionAge + options.ConnMaxLifetime = opts.MaxConnectionAge } if opts.Size != 0 { options.PoolSize = opts.Size @@ -166,7 +166,7 @@ func (opts PoolOptions) ApplyClusterOptions(options *redis.ClusterOptions) { options.MinIdleConns = opts.MinIdleConnections } if opts.MaxConnectionAge != 0 { - options.MaxConnAge = opts.MaxConnectionAge + options.ConnMaxLifetime = opts.MaxConnectionAge } if opts.Size != 0 { options.PoolSize = opts.Size diff --git a/redis/db/redisbp/config_test.go b/redis/db/redisbp/config_test.go index 6ef4a508e..8cd9deedb 100644 --- a/redis/db/redisbp/config_test.go +++ b/redis/db/redisbp/config_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - "github.com/go-redis/redis/v8" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/configbp" "github.com/reddit/baseplate.go/redis/db/redisbp" @@ -78,10 +78,10 @@ timeouts: Network: "tcp", Addr: "localhost:6379", - MinIdleConns: 5, - MaxConnAge: time.Minute, - PoolSize: 10, - PoolTimeout: time.Second * 10, + MinIdleConns: 5, + ConnMaxLifetime: time.Minute, + PoolSize: 10, + PoolTimeout: time.Second * 10, MaxRetries: 2, MinRetryBackoff: time.Millisecond, @@ -187,10 +187,10 @@ timeouts: options: &redis.ClusterOptions{ Addrs: []string{"localhost:6379", "localhost:6380"}, - MinIdleConns: 5, - MaxConnAge: time.Minute, - PoolSize: 10, - PoolTimeout: time.Second * 10, + MinIdleConns: 5, + ConnMaxLifetime: time.Minute, + PoolSize: 10, + PoolTimeout: time.Second * 10, MaxRetries: 2, MinRetryBackoff: time.Millisecond, diff --git a/redis/db/redisbp/doc.go b/redis/db/redisbp/doc.go index e8bca3aab..26a836255 100644 --- a/redis/db/redisbp/doc.go +++ b/redis/db/redisbp/doc.go @@ -1,6 +1,6 @@ // Package redisbp provides Baseplate integrations for go-redis. // -// See https://pkg.go.dev/github.com/go-redis/redis/v8 for documentation for +// See https://pkg.go.dev/github.com/redis/go-redis/v9 for documentation for // go-redis. // // It's recommended to be used in "use Redis as a DB" scenarios as it provides diff --git a/redis/db/redisbp/example_hooks_test.go b/redis/db/redisbp/example_hooks_test.go index e33803bca..4f50e3f48 100644 --- a/redis/db/redisbp/example_hooks_test.go +++ b/redis/db/redisbp/example_hooks_test.go @@ -3,8 +3,8 @@ package redisbp_test import ( "context" - "github.com/go-redis/redis/v8" "github.com/opentracing/opentracing-go" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/redis/db/redisbp" "github.com/reddit/baseplate.go/tracing" @@ -30,8 +30,8 @@ func ExampleSpanHook() { "test", tracing.SpanTypeOption{Type: tracing.SpanTypeServer}, ) - // Create a new client using the context for the server span - client := baseClient.WithContext(ctx) + //// Create a new client using the context for the server span + //client := baseClient.WithContext(ctx) // Commands should now be wrapped using Client Spans - client.Ping(ctx) + baseClient.Ping(ctx) } diff --git a/redis/db/redisbp/example_monitored_client_test.go b/redis/db/redisbp/example_monitored_client_test.go index d6eac7e7e..45f160379 100644 --- a/redis/db/redisbp/example_monitored_client_test.go +++ b/redis/db/redisbp/example_monitored_client_test.go @@ -3,8 +3,8 @@ package redisbp_test import ( "context" - "github.com/go-redis/redis/v8" "github.com/opentracing/opentracing-go" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/redis/db/redisbp" "github.com/reddit/baseplate.go/tracing" diff --git a/redis/db/redisbp/hooks.go b/redis/db/redisbp/hooks.go index bc3662344..c2e5d4281 100644 --- a/redis/db/redisbp/hooks.go +++ b/redis/db/redisbp/hooks.go @@ -4,11 +4,12 @@ import ( "context" "errors" "fmt" + "net" "time" - "github.com/go-redis/redis/v8" "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/internal/prometheusbpint" "github.com/reddit/baseplate.go/prometheusbp" @@ -41,6 +42,43 @@ type SpanHook struct { var _ redis.Hook = SpanHook{} +func (h SpanHook) DialHook(hook redis.DialHook) redis.DialHook { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + fmt.Printf("dialing %s %s\n", network, addr) + conn, err := hook(ctx, network, addr) + fmt.Printf("finished dialing %s %s\n", network, addr) + return conn, err + } +} + +func (h SpanHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook { + return func(ctx context.Context, cmd redis.Cmder) error { + fmt.Printf("starting processing: <%s>\n", cmd) + h.startChildSpan(ctx, cmd.Name()) + err := cmd.Err() + h.endChildSpan(ctx, err) + fmt.Printf("finished processing: <%s>\n", cmd) + return err + } +} + +func (h SpanHook) ProcessPipelineHook(hook redis.ProcessPipelineHook) redis.ProcessPipelineHook { + return func(ctx context.Context, cmds []redis.Cmder) error { + fmt.Printf("pipeline starting processing: %v\n", cmds) + h.startChildSpan(ctx, "pipeline") + errs := make([]error, 0, len(cmds)) + for _, cmd := range cmds { + if err := cmd.Err(); !errors.Is(err, redis.Nil) { + errs = append(errs, err) + } + } + + h.endChildSpan(ctx, errors.Join(errs...)) + fmt.Printf("pipeline finished processing: %v\n", cmds) + return nil + } +} + // BeforeProcess starts a client Span before processing a Redis command and // starts a timer to record how long the command took. func (h SpanHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { diff --git a/redis/db/redisbp/hooks_test.go b/redis/db/redisbp/hooks_test.go index fece802cb..7b1236ffd 100644 --- a/redis/db/redisbp/hooks_test.go +++ b/redis/db/redisbp/hooks_test.go @@ -4,9 +4,9 @@ import ( "context" "testing" - "github.com/go-redis/redis/v8" "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/prometheusbp/promtest" "github.com/reddit/baseplate.go/redis/internal/redisprom" diff --git a/redis/db/redisbp/monitored_client.go b/redis/db/redisbp/monitored_client.go index 32d540b39..173d3610f 100644 --- a/redis/db/redisbp/monitored_client.go +++ b/redis/db/redisbp/monitored_client.go @@ -8,7 +8,7 @@ import ( "strings" "time" - "github.com/go-redis/redis/v8" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/internal/prometheusbpint" "github.com/reddit/baseplate.go/metricsbp" diff --git a/redis/db/redisbp/monitored_client_test.go b/redis/db/redisbp/monitored_client_test.go index 2d0af6e99..9b6dad286 100644 --- a/redis/db/redisbp/monitored_client_test.go +++ b/redis/db/redisbp/monitored_client_test.go @@ -7,7 +7,7 @@ import ( "time" "github.com/alicebob/miniredis/v2" - "github.com/go-redis/redis/v8" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/mqsend" "github.com/reddit/baseplate.go/redis/db/redisbp" diff --git a/redis/db/redisbp/prometheus_test.go b/redis/db/redisbp/prometheus_test.go index ed7ae2606..740cdc597 100644 --- a/redis/db/redisbp/prometheus_test.go +++ b/redis/db/redisbp/prometheus_test.go @@ -4,8 +4,8 @@ import ( "sync" "testing" - "github.com/go-redis/redis/v8" "github.com/prometheus/client_golang/prometheus" + "github.com/redis/go-redis/v9" "github.com/reddit/baseplate.go/internal/prometheusbpint" ) From 0d2722a36f24ba61dd6744363f5a7bcee88ef6c1 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Fri, 10 Jan 2025 14:35:40 -0800 Subject: [PATCH 2/3] WIP -go-redis v9 monitoring client migration --- go.mod | 3 + go.sum | 4 ++ redis/db/redisbp/hooks.go | 88 +++++++++++------------ redis/db/redisbp/hooks_test.go | 62 +++++++--------- redis/db/redisbp/monitored_client_test.go | 5 +- 5 files changed, 76 insertions(+), 86 deletions(-) diff --git a/go.mod b/go.mod index e789a4b16..4131c3776 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/prometheus/client_model v0.5.0 github.com/redis/go-redis/v9 v9.7.0 github.com/sony/gobreaker v0.4.1 + github.com/stretchr/testify v1.8.1 go.uber.org/automaxprocs v1.5.1 go.uber.org/zap v1.24.0 golang.org/x/sys v0.18.0 @@ -54,6 +55,7 @@ require ( github.com/klauspost/compress v1.12.2 // indirect github.com/mediocregopher/radix.v2 v0.0.0-20181115013041-b67df6e626f9 // indirect github.com/pierrec/lz4 v2.6.0+incompatible // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/common v0.48.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect @@ -65,6 +67,7 @@ require ( golang.org/x/text v0.14.0 // indirect google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect google.golang.org/protobuf v1.33.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apimachinery v0.25.0 // indirect k8s.io/klog/v2 v2.80.1 // indirect ) diff --git a/go.sum b/go.sum index 20a47bb45..40c725596 100644 --- a/go.sum +++ b/go.sum @@ -281,11 +281,15 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= diff --git a/redis/db/redisbp/hooks.go b/redis/db/redisbp/hooks.go index c2e5d4281..1aaf272ae 100644 --- a/redis/db/redisbp/hooks.go +++ b/redis/db/redisbp/hooks.go @@ -44,80 +44,78 @@ var _ redis.Hook = SpanHook{} func (h SpanHook) DialHook(hook redis.DialHook) redis.DialHook { return func(ctx context.Context, network, addr string) (net.Conn, error) { - fmt.Printf("dialing %s %s\n", network, addr) conn, err := hook(ctx, network, addr) - fmt.Printf("finished dialing %s %s\n", network, addr) return conn, err } } func (h SpanHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook { return func(ctx context.Context, cmd redis.Cmder) error { - fmt.Printf("starting processing: <%s>\n", cmd) - h.startChildSpan(ctx, cmd.Name()) - err := cmd.Err() + ctx = h.startChildSpan(ctx, cmd.Name()) + err := hook(ctx, cmd) h.endChildSpan(ctx, err) - fmt.Printf("finished processing: <%s>\n", cmd) return err } } func (h SpanHook) ProcessPipelineHook(hook redis.ProcessPipelineHook) redis.ProcessPipelineHook { return func(ctx context.Context, cmds []redis.Cmder) error { - fmt.Printf("pipeline starting processing: %v\n", cmds) - h.startChildSpan(ctx, "pipeline") + ctx = h.startChildSpan(ctx, "pipeline") + err := hook(ctx, cmds) errs := make([]error, 0, len(cmds)) for _, cmd := range cmds { if err := cmd.Err(); !errors.Is(err, redis.Nil) { errs = append(errs, err) } } + if err != nil { + errs = append(errs, err) + } h.endChildSpan(ctx, errors.Join(errs...)) - fmt.Printf("pipeline finished processing: %v\n", cmds) return nil } } -// BeforeProcess starts a client Span before processing a Redis command and -// starts a timer to record how long the command took. -func (h SpanHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { - return h.startChildSpan(ctx, cmd.Name()), nil -} - -// AfterProcess ends the client Span started by BeforeProcess, publishes the -// time the Redis command took to complete, and a metric indicating whether the -// command was a "success" or "fail" -func (h SpanHook) AfterProcess(ctx context.Context, cmd redis.Cmder) error { - h.endChildSpan(ctx, cmd.Err()) - // NOTE: returning non-nil error from the hook changes the error the caller gets. - // for this particular case if we return cmd.Err(), it will not change the client error, - // but anyway it's not necessary - // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L60 - return nil -} +//// BeforeProcess starts a client Span before processing a Redis command and +//// starts a timer to record how long the command took. +//func (h SpanHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { +// return h.startChildSpan(ctx, cmd.Name()), nil +//} +// +//// AfterProcess ends the client Span started by BeforeProcess, publishes the +//// time the Redis command took to complete, and a metric indicating whether the +//// command was a "success" or "fail" +//func (h SpanHook) AfterProcess(ctx context.Context, cmd redis.Cmder) error { +// h.endChildSpan(ctx, cmd.Err()) +// // NOTE: returning non-nil error from the hook changes the error the caller gets. +// // for this particular case if we return cmd.Err(), it will not change the client error, +// // but anyway it's not necessary +// // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L60 +// return nil +//} // BeforeProcessPipeline starts a client span before processing a Redis pipeline // and starts a timer to record how long the pipeline took. -func (h SpanHook) BeforeProcessPipeline(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { - return h.startChildSpan(ctx, "pipeline"), nil -} - -// AfterProcessPipeline ends the client span started by BeforeProcessPipeline, -// publishes the time the Redis pipeline took to complete, and a metric -// indicating whether the pipeline was a "success" or "fail" -func (h SpanHook) AfterProcessPipeline(ctx context.Context, cmds []redis.Cmder) error { - errs := make([]error, 0, len(cmds)) - for _, cmd := range cmds { - if err := cmd.Err(); !errors.Is(err, redis.Nil) { - errs = append(errs, err) - } - } - h.endChildSpan(ctx, errors.Join(errs...)) - // NOTE: returning non-nil error from the hook changes the error the caller gets, and that's something we want to avoid. - // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L101 - return nil -} +//func (h SpanHook) BeforeProcessPipeline(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { +// return h.startChildSpan(ctx, "pipeline"), nil +//} +// +//// AfterProcessPipeline ends the client span started by BeforeProcessPipeline, +//// publishes the time the Redis pipeline took to complete, and a metric +//// indicating whether the pipeline was a "success" or "fail" +//func (h SpanHook) AfterProcessPipeline(ctx context.Context, cmds []redis.Cmder) error { +// errs := make([]error, 0, len(cmds)) +// for _, cmd := range cmds { +// if err := cmd.Err(); !errors.Is(err, redis.Nil) { +// errs = append(errs, err) +// } +// } +// h.endChildSpan(ctx, errors.Join(errs...)) +// // NOTE: returning non-nil error from the hook changes the error the caller gets, and that's something we want to avoid. +// // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L101 +// return nil +//} func (h SpanHook) startChildSpan(ctx context.Context, cmdName string) context.Context { name := fmt.Sprintf("%s.%s", h.ClientName, cmdName) diff --git a/redis/db/redisbp/hooks_test.go b/redis/db/redisbp/hooks_test.go index 7b1236ffd..4a6f0be44 100644 --- a/redis/db/redisbp/hooks_test.go +++ b/redis/db/redisbp/hooks_test.go @@ -2,6 +2,8 @@ package redisbp import ( "context" + "github.com/reddit/baseplate.go/internal/prometheusbpint" + "github.com/stretchr/testify/assert" "testing" "github.com/opentracing/opentracing-go" @@ -23,9 +25,26 @@ func TestSpanHook(t *testing.T) { Cluster: "cluster", } statusCmd := redis.NewStatusCmd(ctx, "ping") - stringCmd := redis.NewStringCmd(ctx, "get", "1") - stringCmd.SetErr(nil) - + t.Run( + "ProcessHook", + func(t *testing.T) { + hook := SpanHook{ + promActive: &prometheusbpint.HighWatermarkGauge{ + HighWatermarkValue: &prometheusbpint.HighWatermarkValue{}, + CurrGauge: redisprom.ActiveConnectionsDesc, + CurrGaugeLabelValues: []string{"test"}, + MaxGauge: redisprom.PeakActiveConnectionsDesc, + MaxGaugeLabelValues: []string{"test"}, + }, + } + ph := func(ctx context.Context, cmd redis.Cmder) error { return nil } + err := hook.ProcessHook(ph)(context.Background(), statusCmd) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + assert.Equal(t, int64(1), hook.promActive.Max()) + }, + ) t.Run( "Before/AfterProcess", func(t *testing.T) { @@ -46,10 +65,7 @@ func TestSpanHook(t *testing.T) { defer promtest.NewPrometheusMetricTest(t, "spec latency timer", redisprom.LatencySeconds, labels).CheckSampleCountDelta(1) defer promtest.NewPrometheusMetricTest(t, "spec requests total", redisprom.RequestsTotal, labels).CheckDelta(1) - ctx, err := hooks.BeforeProcess(ctx, statusCmd) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } + ctx := hooks.startChildSpan(ctx, statusCmd.Name()) activeSpan := opentracing.SpanFromContext(ctx) if activeSpan == nil { t.Fatalf("'activeSpan' is 'nil'") @@ -58,37 +74,7 @@ func TestSpanHook(t *testing.T) { t.Fatalf("Incorrect span name %q", name) } - if err = hooks.AfterProcess(ctx, statusCmd); err != nil { - t.Fatalf("Unexpected error: %s", err) - } - }, - ) - - t.Run( - "Before/AfterProcessPipeline", - func(t *testing.T) { - defer promtest.NewPrometheusMetricTest(t, "latency timer", latencyTimer, prometheus.Labels{ - nameLabel: "redis", - commandLabel: "pipeline", - successLabel: "true", - }).CheckSampleCountDelta(1) - - cmds := []redis.Cmder{statusCmd, stringCmd} - ctx, err := hooks.BeforeProcessPipeline(ctx, cmds) - if err != nil { - t.Fatalf("Unexpected error: %s", err) - } - activeSpan := opentracing.SpanFromContext(ctx) - if activeSpan == nil { - t.Fatalf("'activeSpan' is 'nil'") - } - if name := tracing.AsSpan(activeSpan).Name(); name != "redis.pipeline" { - t.Fatalf("Incorrect span name %q", name) - } - - if err = hooks.AfterProcessPipeline(ctx, cmds); err != nil { - t.Fatalf("Unexpected error: %s", err) - } + hooks.endChildSpan(ctx, nil) }, ) } diff --git a/redis/db/redisbp/monitored_client_test.go b/redis/db/redisbp/monitored_client_test.go index 9b6dad286..d69968396 100644 --- a/redis/db/redisbp/monitored_client_test.go +++ b/redis/db/redisbp/monitored_client_test.go @@ -46,13 +46,12 @@ func TestNewMonitoredClient(t *testing.T) { defer s.Close() client := redisbp.NewMonitoredClient("redis", &redis.Options{Addr: s.Addr()}) - ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) + defer cancel() if resp := client.Ping(ctx); resp.Err() != nil { t.Fatal(resp.Err()) } - ctx, cancel := context.WithTimeout(ctx, testTimeout) - defer cancel() msg, err := mmq.Receive(ctx) if err != nil { t.Fatal(err) From 23da3025cef04841d0fb1f1d59b378adf73ec07f Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Mon, 13 Jan 2025 09:01:06 -0800 Subject: [PATCH 3/3] Test cleanup --- redis/db/redisbp/hooks.go | 40 ---------------------------------- redis/db/redisbp/hooks_test.go | 36 +++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/redis/db/redisbp/hooks.go b/redis/db/redisbp/hooks.go index 1aaf272ae..2309fa4ca 100644 --- a/redis/db/redisbp/hooks.go +++ b/redis/db/redisbp/hooks.go @@ -77,46 +77,6 @@ func (h SpanHook) ProcessPipelineHook(hook redis.ProcessPipelineHook) redis.Proc } } -//// BeforeProcess starts a client Span before processing a Redis command and -//// starts a timer to record how long the command took. -//func (h SpanHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (context.Context, error) { -// return h.startChildSpan(ctx, cmd.Name()), nil -//} -// -//// AfterProcess ends the client Span started by BeforeProcess, publishes the -//// time the Redis command took to complete, and a metric indicating whether the -//// command was a "success" or "fail" -//func (h SpanHook) AfterProcess(ctx context.Context, cmd redis.Cmder) error { -// h.endChildSpan(ctx, cmd.Err()) -// // NOTE: returning non-nil error from the hook changes the error the caller gets. -// // for this particular case if we return cmd.Err(), it will not change the client error, -// // but anyway it's not necessary -// // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L60 -// return nil -//} - -// BeforeProcessPipeline starts a client span before processing a Redis pipeline -// and starts a timer to record how long the pipeline took. -//func (h SpanHook) BeforeProcessPipeline(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { -// return h.startChildSpan(ctx, "pipeline"), nil -//} -// -//// AfterProcessPipeline ends the client span started by BeforeProcessPipeline, -//// publishes the time the Redis pipeline took to complete, and a metric -//// indicating whether the pipeline was a "success" or "fail" -//func (h SpanHook) AfterProcessPipeline(ctx context.Context, cmds []redis.Cmder) error { -// errs := make([]error, 0, len(cmds)) -// for _, cmd := range cmds { -// if err := cmd.Err(); !errors.Is(err, redis.Nil) { -// errs = append(errs, err) -// } -// } -// h.endChildSpan(ctx, errors.Join(errs...)) -// // NOTE: returning non-nil error from the hook changes the error the caller gets, and that's something we want to avoid. -// // see: https://github.com/go-redis/redis/blob/v8.10.0/redis.go#L101 -// return nil -//} - func (h SpanHook) startChildSpan(ctx context.Context, cmdName string) context.Context { name := fmt.Sprintf("%s.%s", h.ClientName, cmdName) _, ctx = opentracing.StartSpanFromContext( diff --git a/redis/db/redisbp/hooks_test.go b/redis/db/redisbp/hooks_test.go index 4a6f0be44..f8786cc97 100644 --- a/redis/db/redisbp/hooks_test.go +++ b/redis/db/redisbp/hooks_test.go @@ -37,7 +37,13 @@ func TestSpanHook(t *testing.T) { MaxGaugeLabelValues: []string{"test"}, }, } - ph := func(ctx context.Context, cmd redis.Cmder) error { return nil } + ph := func(ctx context.Context, cmd redis.Cmder) error { + span := opentracing.SpanFromContext(ctx) + if span == nil { + t.Fatalf("Failed to get span from context\n") + } + return nil + } err := hook.ProcessHook(ph)(context.Background(), statusCmd) if err != nil { t.Fatalf("Unexpected error: %v", err) @@ -45,6 +51,34 @@ func TestSpanHook(t *testing.T) { assert.Equal(t, int64(1), hook.promActive.Max()) }, ) + t.Run( + "ProcessPipelineHook", + func(t *testing.T) { + hook := SpanHook{ + promActive: &prometheusbpint.HighWatermarkGauge{ + HighWatermarkValue: &prometheusbpint.HighWatermarkValue{}, + CurrGauge: redisprom.ActiveConnectionsDesc, + CurrGaugeLabelValues: []string{"test"}, + MaxGauge: redisprom.PeakActiveConnectionsDesc, + MaxGaugeLabelValues: []string{"test"}, + }, + } + infoCmd := redis.NewStatusCmd(ctx, "info") + ph := func(ctx context.Context, cmd []redis.Cmder) error { + span := opentracing.SpanFromContext(ctx) + if span == nil { + t.Fatalf("Failed to get span from context\n") + } + return nil + } + err := hook.ProcessPipelineHook(ph)(context.Background(), []redis.Cmder{statusCmd, infoCmd}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + assert.Equal(t, int64(1), hook.promActive.Max()) + }, + ) + t.Run( "Before/AfterProcess", func(t *testing.T) {