From 64bfb348d875fbd1e877a6c63371b5ed7d6b1717 Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Mon, 25 Nov 2024 23:52:17 +0530 Subject: [PATCH 1/6] Add support for custom attributes function --- extra/redisotel/config.go | 20 ++++++++++++++++---- extra/redisotel/metrics.go | 4 ++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/extra/redisotel/config.go b/extra/redisotel/config.go index c02ee0b31..1119780c1 100644 --- a/extra/redisotel/config.go +++ b/extra/redisotel/config.go @@ -1,6 +1,8 @@ package redisotel import ( + "context" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -11,8 +13,9 @@ import ( type config struct { // Common options. - dbSystem string - attrs []attribute.KeyValue + dbSystem string + attrs []attribute.KeyValue + attrsFunc func(context.Context) []attribute.KeyValue // Tracing options. @@ -51,8 +54,9 @@ func (fn option) metrics() {} func newConfig(opts ...baseOption) *config { conf := &config{ - dbSystem: "redis", - attrs: []attribute.KeyValue{}, + dbSystem: "redis", + attrs: []attribute.KeyValue{}, + attrsFunc: func(ctx context.Context) []attribute.KeyValue { return []attribute.KeyValue{} }, tp: otel.GetTracerProvider(), mp: otel.GetMeterProvider(), @@ -81,6 +85,14 @@ func WithAttributes(attrs ...attribute.KeyValue) Option { }) } +// WithAttributesFunc takes a function that returns additional attributes to be added using the context. +// This is executed only in ProcessPipelineHook and ProcessHook +func WithAttributesFunc(f func(context.Context) []attribute.KeyValue) Option { + return option(func(conf *config) { + conf.attrsFunc = f + }) +} + //------------------------------------------------------------------------------ type TracingOption interface { diff --git a/extra/redisotel/metrics.go b/extra/redisotel/metrics.go index 915838f34..1fb9e9b79 100644 --- a/extra/redisotel/metrics.go +++ b/extra/redisotel/metrics.go @@ -175,6 +175,7 @@ func addMetricsHook(rdb *redis.Client, conf *config) error { createTime: createTime, useTime: useTime, attrs: conf.attrs, + attrsFunc: conf.attrsFunc, }) return nil } @@ -183,6 +184,7 @@ type metricsHook struct { createTime metric.Float64Histogram useTime metric.Float64Histogram attrs []attribute.KeyValue + attrsFunc func(context.Context) []attribute.KeyValue } var _ redis.Hook = (*metricsHook)(nil) @@ -214,6 +216,7 @@ func (mh *metricsHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook { attrs := make([]attribute.KeyValue, 0, len(mh.attrs)+2) attrs = append(attrs, mh.attrs...) + attrs = append(attrs, mh.attrsFunc(ctx)...) attrs = append(attrs, attribute.String("type", "command")) attrs = append(attrs, statusAttr(err)) @@ -235,6 +238,7 @@ func (mh *metricsHook) ProcessPipelineHook( attrs := make([]attribute.KeyValue, 0, len(mh.attrs)+2) attrs = append(attrs, mh.attrs...) + attrs = append(attrs, mh.attrsFunc(ctx)...) attrs = append(attrs, attribute.String("type", "pipeline")) attrs = append(attrs, statusAttr(err)) From 103a07a35ea75145df5c668cd2dc6c00666ec598 Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Tue, 26 Nov 2024 00:00:05 +0530 Subject: [PATCH 2/6] Formatting --- extra/redisotel/metrics.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extra/redisotel/metrics.go b/extra/redisotel/metrics.go index 1fb9e9b79..6f5b9c8f4 100644 --- a/extra/redisotel/metrics.go +++ b/extra/redisotel/metrics.go @@ -175,7 +175,7 @@ func addMetricsHook(rdb *redis.Client, conf *config) error { createTime: createTime, useTime: useTime, attrs: conf.attrs, - attrsFunc: conf.attrsFunc, + attrsFunc: conf.attrsFunc, }) return nil } @@ -184,7 +184,7 @@ type metricsHook struct { createTime metric.Float64Histogram useTime metric.Float64Histogram attrs []attribute.KeyValue - attrsFunc func(context.Context) []attribute.KeyValue + attrsFunc func(context.Context) []attribute.KeyValue } var _ redis.Hook = (*metricsHook)(nil) From bfba6541759f8b6fc06c55abcf9536e0d4094fca Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Tue, 17 Dec 2024 00:55:39 +0530 Subject: [PATCH 3/6] Update ordering --- extra/redisotel/metrics.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extra/redisotel/metrics.go b/extra/redisotel/metrics.go index 6f5b9c8f4..5ec5128e9 100644 --- a/extra/redisotel/metrics.go +++ b/extra/redisotel/metrics.go @@ -215,8 +215,8 @@ func (mh *metricsHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook { dur := time.Since(start) attrs := make([]attribute.KeyValue, 0, len(mh.attrs)+2) - attrs = append(attrs, mh.attrs...) attrs = append(attrs, mh.attrsFunc(ctx)...) + attrs = append(attrs, mh.attrs...) attrs = append(attrs, attribute.String("type", "command")) attrs = append(attrs, statusAttr(err)) @@ -237,8 +237,8 @@ func (mh *metricsHook) ProcessPipelineHook( dur := time.Since(start) attrs := make([]attribute.KeyValue, 0, len(mh.attrs)+2) - attrs = append(attrs, mh.attrs...) attrs = append(attrs, mh.attrsFunc(ctx)...) + attrs = append(attrs, mh.attrs...) attrs = append(attrs, attribute.String("type", "pipeline")) attrs = append(attrs, statusAttr(err)) From dffbe613abc88f162b59fca2ac20db1dd56f6193 Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Tue, 17 Dec 2024 00:55:43 +0530 Subject: [PATCH 4/6] Add tests --- extra/redisotel/go.mod | 17 ++-- extra/redisotel/go.sum | 26 ++++-- extra/redisotel/metrics_test.go | 148 ++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 16 deletions(-) create mode 100644 extra/redisotel/metrics_test.go diff --git a/extra/redisotel/go.mod b/extra/redisotel/go.mod index b2e30b394..75897c346 100644 --- a/extra/redisotel/go.mod +++ b/extra/redisotel/go.mod @@ -9,20 +9,23 @@ replace github.com/redis/go-redis/extra/rediscmd/v9 => ../rediscmd require ( github.com/redis/go-redis/extra/rediscmd/v9 v9.6.2 github.com/redis/go-redis/v9 v9.6.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 + github.com/stretchr/testify v1.10.0 + go.opentelemetry.io/otel v1.23.0 + go.opentelemetry.io/otel/metric v1.23.0 + go.opentelemetry.io/otel/sdk v1.23.0 + go.opentelemetry.io/otel/sdk/metric v1.23.0 + go.opentelemetry.io/otel/trace v1.23.0 ) require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect + github.com/davecgh/go-spew v1.1.1 // 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/stdr v1.2.2 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/sys v0.16.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) -retract ( - v9.5.3 // This version was accidentally released. -) +retract v9.5.3 // This version was accidentally released. diff --git a/extra/redisotel/go.sum b/extra/redisotel/go.sum index 9eb9bcd4e..b689b6ff5 100644 --- a/extra/redisotel/go.sum +++ b/extra/redisotel/go.sum @@ -3,6 +3,7 @@ github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= 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/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= @@ -12,15 +13,22 @@ 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/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.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +go.opentelemetry.io/otel v1.23.0 h1:Df0pqjqExIywbMCMTxkAwzjLZtRf+bBKLbUcpxO2C9E= +go.opentelemetry.io/otel v1.23.0/go.mod h1:YCycw9ZeKhcJFrb34iVSkyT0iczq/zYDtZYFufObyB0= +go.opentelemetry.io/otel/metric v1.23.0 h1:pazkx7ss4LFVVYSxYew7L5I6qvLXHA0Ap2pwV+9Cnpo= +go.opentelemetry.io/otel/metric v1.23.0/go.mod h1:MqUW2X2a6Q8RN96E2/nqNoT+z9BSms20Jb7Bbp+HiTo= +go.opentelemetry.io/otel/sdk v1.23.0 h1:0KM9Zl2esnl+WSukEmlaAEjVY5HDZANOHferLq36BPc= +go.opentelemetry.io/otel/sdk v1.23.0/go.mod h1:wUscup7byToqyKJSilEtMf34FgdCAsFpFOjXnAwFfO0= +go.opentelemetry.io/otel/sdk/metric v1.23.0 h1:u81lMvmK6GMgN4Fty7K7S6cSKOZhMKJMK2TB+KaTs0I= +go.opentelemetry.io/otel/sdk/metric v1.23.0/go.mod h1:2LUOToN/FdX6wtfpHybOnCZjoZ6ViYajJYMiJ1LKDtQ= +go.opentelemetry.io/otel/trace v1.23.0 h1:37Ik5Ib7xfYVb4V1UtnT97T1jI+AoIYkJyPkuL4iJgI= +go.opentelemetry.io/otel/trace v1.23.0/go.mod h1:GSGTbIClEsuZrGIzoEHqsVfxgn5UkggkflQwDScNUsk= golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= 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_test.go b/extra/redisotel/metrics_test.go new file mode 100644 index 000000000..5ebc987e8 --- /dev/null +++ b/extra/redisotel/metrics_test.go @@ -0,0 +1,148 @@ +package redisotel + +import ( + "context" + "testing" + + "github.com/redis/go-redis/v9" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/sdk/instrumentation" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" + semconv "go.opentelemetry.io/otel/semconv/v1.4.0" +) + +var instrumentationScope = instrumentation.Scope{ + Name: instrumName, + Version: "semver:" + redis.Version(), +} + +func setupMetrics(conf *config) (*sdkmetric.ManualReader, *redis.Client) { + reader := sdkmetric.NewManualReader() + mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + otel.SetMeterProvider(mp) + + rdb := redis.NewClient(&redis.Options{ + Addr: ":6379", + }) + if conf.meter == nil { + conf.meter = conf.mp.Meter( + instrumName, + metric.WithInstrumentationVersion("semver:"+redis.Version()), + ) + } + addMetricsHook(rdb, conf) + return reader, rdb +} + +func TestMetrics(t *testing.T) { + reader, rdb := setupMetrics(newConfig()) + rdb.Get(context.Background(), "key") + + want := metricdata.ScopeMetrics{ + Scope: instrumentationScope, + Metrics: []metricdata.Metrics{ + { + Name: "db.client.connections.create_time", + Description: "The time it took to create a new connection.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attribute.NewSet( + semconv.DBSystemRedis, + attribute.String("status", "error"), + ), + }, + }, + }, + }, + { + Name: "db.client.connections.use_time", + Description: "The time between borrowing a connection and returning it to the pool.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attribute.NewSet( + semconv.DBSystemRedis, + attribute.String("type", "command"), + attribute.String("status", "error"), + ), + }, + }, + }, + }, + }, + } + rm := metricdata.ResourceMetrics{} + err := reader.Collect(context.Background(), &rm) + assert.NoError(t, err) + require.Len(t, rm.ScopeMetrics, 1) + metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) +} + +func TestCustomAttributes(t *testing.T) { + customAttrFn := func(ctx context.Context) []attribute.KeyValue { + return []attribute.KeyValue{ + attribute.String("custom", "value"), + } + } + config := newConfig(WithAttributesFunc(customAttrFn)) + reader, rdb := setupMetrics(config) + + rdb.Get(context.Background(), "key") + + want := metricdata.ScopeMetrics{ + Scope: instrumentationScope, + Metrics: []metricdata.Metrics{ + { + Name: "db.client.connections.create_time", + Description: "The time it took to create a new connection.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attribute.NewSet( + semconv.DBSystemRedis, + attribute.String("status", "error"), + ), + }, + }, + }, + }, + { + Name: "db.client.connections.use_time", + Description: "The time between borrowing a connection and returning it to the pool.", + Unit: "ms", + Data: metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attribute.NewSet( + semconv.DBSystemRedis, + attribute.String("type", "command"), + attribute.String("status", "error"), + attribute.String("custom", "value"), + ), + }, + }, + }, + }, + }, + } + rm := metricdata.ResourceMetrics{} + err := reader.Collect(context.Background(), &rm) + assert.NoError(t, err) + require.Len(t, rm.ScopeMetrics, 1) + metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) +} From f7d3f48e2e7c006c5e7436b607053a12745e0c9b Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Tue, 17 Dec 2024 19:03:13 +0530 Subject: [PATCH 5/6] Fix tests --- extra/redisotel/metrics_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extra/redisotel/metrics_test.go b/extra/redisotel/metrics_test.go index 5ebc987e8..0c8535ace 100644 --- a/extra/redisotel/metrics_test.go +++ b/extra/redisotel/metrics_test.go @@ -58,7 +58,7 @@ func TestMetrics(t *testing.T) { { Attributes: attribute.NewSet( semconv.DBSystemRedis, - attribute.String("status", "error"), + attribute.String("status", "ok"), ), }, }, @@ -75,7 +75,7 @@ func TestMetrics(t *testing.T) { Attributes: attribute.NewSet( semconv.DBSystemRedis, attribute.String("type", "command"), - attribute.String("status", "error"), + attribute.String("status", "ok"), ), }, }, @@ -114,7 +114,7 @@ func TestCustomAttributes(t *testing.T) { { Attributes: attribute.NewSet( semconv.DBSystemRedis, - attribute.String("status", "error"), + attribute.String("status", "ok"), ), }, }, @@ -131,7 +131,7 @@ func TestCustomAttributes(t *testing.T) { Attributes: attribute.NewSet( semconv.DBSystemRedis, attribute.String("type", "command"), - attribute.String("status", "error"), + attribute.String("status", "ok"), attribute.String("custom", "value"), ), }, From f376e01c43445fe84dca575346a8433a381323c7 Mon Sep 17 00:00:00 2001 From: Jatin Rungta <9137405+urdarinda@users.noreply.github.com> Date: Thu, 16 Jan 2025 20:00:16 +0530 Subject: [PATCH 6/6] Fix tests by using specific meter providers --- extra/redisotel/metrics_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/extra/redisotel/metrics_test.go b/extra/redisotel/metrics_test.go index 0c8535ace..aecf60da9 100644 --- a/extra/redisotel/metrics_test.go +++ b/extra/redisotel/metrics_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/sdk/instrumentation" @@ -26,8 +25,8 @@ var instrumentationScope = instrumentation.Scope{ func setupMetrics(conf *config) (*sdkmetric.ManualReader, *redis.Client) { reader := sdkmetric.NewManualReader() mp := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) - otel.SetMeterProvider(mp) - + conf.mp = mp + rdb := redis.NewClient(&redis.Options{ Addr: ":6379", }) @@ -43,7 +42,7 @@ func setupMetrics(conf *config) (*sdkmetric.ManualReader, *redis.Client) { func TestMetrics(t *testing.T) { reader, rdb := setupMetrics(newConfig()) - rdb.Get(context.Background(), "key") + rdb.Ping(context.Background()) want := metricdata.ScopeMetrics{ Scope: instrumentationScope, @@ -99,7 +98,7 @@ func TestCustomAttributes(t *testing.T) { config := newConfig(WithAttributesFunc(customAttrFn)) reader, rdb := setupMetrics(config) - rdb.Get(context.Background(), "key") + rdb.Ping(context.Background()) want := metricdata.ScopeMetrics{ Scope: instrumentationScope,