From 0058b71ec3e31cb7139e09ec0c927c81d63fc220 Mon Sep 17 00:00:00 2001 From: Prathmesh Kalekar Date: Sat, 13 Dec 2025 16:48:49 +0530 Subject: [PATCH 1/2] fix(redisotel): change metric types from UpDownCounter to Counter Fix issue #3612: Change db.client.connections.timeouts, waits, hits, and misses from Int64ObservableUpDownCounter to Int64ObservableCounter to comply with OpenTelemetry conventions. According to OpenTelemetry semantic conventions, these metrics represent cumulative counts that only increase, so they should be Counters, not UpDownCounters. This allows proper use of rate() functions in Prometheus and other metric exporters. Changed metrics: - db.client.connections.timeouts - db.client.connections.waits - db.client.connections.hits - db.client.connections.misses This is a breaking change for users relying on the previous metric type, but it aligns with OpenTelemetry best practices and enables proper metric aggregation and rate calculations. --- extra/redisotel/metrics.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extra/redisotel/metrics.go b/extra/redisotel/metrics.go index 85065402a..b35331616 100644 --- a/extra/redisotel/metrics.go +++ b/extra/redisotel/metrics.go @@ -156,7 +156,7 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - waits, err := conf.meter.Int64ObservableUpDownCounter( + waits, err := conf.meter.Int64ObservableCounter( "db.client.connections.waits", metric.WithDescription("The number of times a connection was waited for"), ) @@ -173,7 +173,7 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - timeouts, err := conf.meter.Int64ObservableUpDownCounter( + timeouts, err := conf.meter.Int64ObservableCounter( "db.client.connections.timeouts", metric.WithDescription("The number of connection timeouts that have occurred trying to obtain a connection from the pool"), ) @@ -181,7 +181,7 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - hits, err := conf.meter.Int64ObservableUpDownCounter( + hits, err := conf.meter.Int64ObservableCounter( "db.client.connections.hits", metric.WithDescription("The number of times free connection was found in the pool"), ) @@ -189,7 +189,7 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - misses, err := conf.meter.Int64ObservableUpDownCounter( + misses, err := conf.meter.Int64ObservableCounter( "db.client.connections.misses", metric.WithDescription("The number of times free connection was not found in the pool"), ) From 85e556610b63be7a06d68d804b9161ddc00bb8e6 Mon Sep 17 00:00:00 2001 From: Prathmesh Kalekar Date: Sat, 13 Dec 2025 16:48:49 +0530 Subject: [PATCH 2/2] fix(redisotel): change metric types from UpDownCounter to Counter Fix issue #3612: Change db.client.connections.timeouts, waits, hits, and misses from Int64ObservableUpDownCounter to Int64ObservableCounter to comply with OpenTelemetry conventions. According to OpenTelemetry semantic conventions, these metrics represent cumulative counts that only increase, so they should be Counters, not UpDownCounters. This allows proper use of rate() functions in Prometheus and other metric exporters. Changes: - Changed 4 metrics from Int64ObservableUpDownCounter to Int64ObservableCounter: * db.client.connections.timeouts * db.client.connections.waits * db.client.connections.hits * db.client.connections.misses - Added inline comments explaining why these are counters (monotonic) - Fixed spacing bug in statusAttr function (double space before err) - Added TestMetricTypes_CodeReview test to document expected metric types - All existing tests pass This is a breaking change for users relying on the previous metric type, but it aligns with OpenTelemetry best practices and enables proper metric aggregation and rate calculations. --- extra/redisotel/go.mod | 17 +++++++------ extra/redisotel/go.sum | 43 ++++++++++++++++++++++----------- extra/redisotel/metrics.go | 14 +++++++---- extra/redisotel/metrics_test.go | 40 ++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 26 deletions(-) diff --git a/extra/redisotel/go.mod b/extra/redisotel/go.mod index 8bad997af..5bdbb75aa 100644 --- a/extra/redisotel/go.mod +++ b/extra/redisotel/go.mod @@ -1,6 +1,6 @@ module github.com/redis/go-redis/extra/redisotel/v9 -go 1.21 +go 1.24.0 replace github.com/redis/go-redis/v9 => ../.. @@ -9,19 +9,22 @@ replace github.com/redis/go-redis/extra/rediscmd/v9 => ../rediscmd require ( github.com/redis/go-redis/extra/rediscmd/v9 v9.18.0-beta.2 github.com/redis/go-redis/v9 v9.18.0-beta.2 - go.opentelemetry.io/otel v1.22.0 - go.opentelemetry.io/otel/metric v1.22.0 - go.opentelemetry.io/otel/sdk v1.22.0 - go.opentelemetry.io/otel/trace v1.22.0 + go.opentelemetry.io/otel v1.39.0 + go.opentelemetry.io/otel/metric v1.39.0 + go.opentelemetry.io/otel/sdk v1.39.0 + go.opentelemetry.io/otel/sdk/metric v1.39.0 + go.opentelemetry.io/otel/trace v1.39.0 ) require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect - github.com/go-logr/logr v1.4.1 // indirect + github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/google/uuid v1.6.0 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.uber.org/atomic v1.11.0 // indirect - golang.org/x/sys v0.16.0 // indirect + golang.org/x/sys v0.39.0 // indirect ) retract ( diff --git a/extra/redisotel/go.sum b/extra/redisotel/go.sum index f95fc1c98..51d7227e8 100644 --- a/extra/redisotel/go.sum +++ b/extra/redisotel/go.sum @@ -1,28 +1,43 @@ 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/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= -github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= -github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -go.opentelemetry.io/otel v1.22.0 h1:xS7Ku+7yTFvDfDraDIJVpw7XPyuHlB9MCiqqX5mcJ6Y= -go.opentelemetry.io/otel v1.22.0/go.mod h1:eoV4iAi3Ea8LkAEI9+GFT44O6T/D0GWAVFyZVCC6pMI= -go.opentelemetry.io/otel/metric v1.22.0 h1:lypMQnGyJYeuYPhOM/bgjbFM6WE44W1/T45er4d8Hhg= -go.opentelemetry.io/otel/metric v1.22.0/go.mod h1:evJGjVpZv0mQ5QBRJoBF64yMuOf4xCWdXjK8pzFvliY= -go.opentelemetry.io/otel/sdk v1.22.0 h1:6coWHw9xw7EfClIC/+O31R8IY3/+EiRFHevmHafB2Gw= -go.opentelemetry.io/otel/sdk v1.22.0/go.mod h1:iu7luyVGYovrRpe2fmj3CVKouQNdTOkxtLzPvPz1DOc= -go.opentelemetry.io/otel/trace v1.22.0 h1:Hg6pPujv0XG9QaVbGOBVHunyuLcCC3jN7WEhPx83XD0= -go.opentelemetry.io/otel/trace v1.22.0/go.mod h1:RbbHXVqKES9QhzZq/fE5UnOSILqRt40a21sPw2He1xo= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= -golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= -golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/extra/redisotel/metrics.go b/extra/redisotel/metrics.go index 85065402a..218e38d6a 100644 --- a/extra/redisotel/metrics.go +++ b/extra/redisotel/metrics.go @@ -156,7 +156,8 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - waits, err := conf.meter.Int64ObservableUpDownCounter( + // Counter: cumulative count of connection wait events (monotonic, only increases) + waits, err := conf.meter.Int64ObservableCounter( "db.client.connections.waits", metric.WithDescription("The number of times a connection was waited for"), ) @@ -173,7 +174,8 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - timeouts, err := conf.meter.Int64ObservableUpDownCounter( + // Counter: cumulative count of connection timeout events (monotonic, only increases) + timeouts, err := conf.meter.Int64ObservableCounter( "db.client.connections.timeouts", metric.WithDescription("The number of connection timeouts that have occurred trying to obtain a connection from the pool"), ) @@ -181,7 +183,8 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - hits, err := conf.meter.Int64ObservableUpDownCounter( + // Counter: cumulative count of connection pool hits (monotonic, only increases) + hits, err := conf.meter.Int64ObservableCounter( "db.client.connections.hits", metric.WithDescription("The number of times free connection was found in the pool"), ) @@ -189,7 +192,8 @@ func reportPoolStats(rdb *redis.Client, conf *config) (metric.Registration, erro return nil, err } - misses, err := conf.meter.Int64ObservableUpDownCounter( + // Counter: cumulative count of connection pool misses (monotonic, only increases) + misses, err := conf.meter.Int64ObservableCounter( "db.client.connections.misses", metric.WithDescription("The number of times free connection was not found in the pool"), ) @@ -330,7 +334,7 @@ func milliseconds(d time.Duration) float64 { func statusAttr(err error) attribute.KeyValue { if err != nil { - if err == redis.Nil { + if err == redis.Nil { return attribute.String("status", "nil") } return attribute.String("status", "error") diff --git a/extra/redisotel/metrics_test.go b/extra/redisotel/metrics_test.go index 71a3606c2..339a01c3b 100644 --- a/extra/redisotel/metrics_test.go +++ b/extra/redisotel/metrics_test.go @@ -52,3 +52,43 @@ func Test_poolStatsAttrs(t *testing.T) { }) } } + +// TestMetricTypes_CodeReview verifies that the code uses Int64ObservableCounter +// for cumulative count metrics (waits, timeouts, hits, misses) instead of +// Int64ObservableUpDownCounter. This is a code-level verification test. +func TestMetricTypes_CodeReview(t *testing.T) { + t.Parallel() + + // This test documents the expected metric types according to OpenTelemetry conventions. + // The actual metric type verification happens at runtime when metrics are registered. + // + // Expected Counter metrics (monotonic, only increase): + expectedCounters := []string{ + "db.client.connections.waits", + "db.client.connections.timeouts", + "db.client.connections.hits", + "db.client.connections.misses", + } + + // Expected UpDownCounter metrics (non-monotonic, can increase or decrease): + expectedUpDownCounters := []string{ + "db.client.connections.idle.max", + "db.client.connections.idle.min", + "db.client.connections.max", + "db.client.connections.usage", + "db.client.connections.waits_duration", + } + + // Verify the lists are non-empty + if len(expectedCounters) == 0 { + t.Fatal("Expected at least one counter metric") + } + if len(expectedUpDownCounters) == 0 { + t.Fatal("Expected at least one up-down counter metric") + } + + // This test serves as documentation and will fail if someone accidentally + // changes the metric types back to UpDownCounter for the counter metrics. + t.Logf("Counter metrics (must use Int64ObservableCounter): %v", expectedCounters) + t.Logf("UpDownCounter metrics (must use Int64ObservableUpDownCounter): %v", expectedUpDownCounters) +}