From 6e4eb2e3acad9575fb1e2c8690a3c1decf2e59e5 Mon Sep 17 00:00:00 2001 From: Vladimir Mihailenco Date: Thu, 3 Jun 2021 14:01:51 +0300 Subject: [PATCH] Remove OpenTelemetry from the code (but leave redisotel as is) (#1782) --- CHANGELOG.md | 13 +++++++++++++ go.mod | 2 -- internal/pool/conn.go | 19 +++++-------------- internal/util.go | 24 ------------------------ options.go | 18 +----------------- redis.go | 13 ------------- 6 files changed, 19 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b540a4..42d89b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,19 @@ > :heart: > [**Uptrace.dev** - All-in-one tool to optimize performance and monitor errors & logs](https://uptrace.dev) +## v8.10 + +- Removed extra OpenTelemetry spans from go-redis core. Now go-redis instrumentation only adds a + single span with a Redis command (instead of 4 spans). There are multiple reasons behind this + decision: + + - Traces become smaller and less noisy. + - It may be costly to process those 3 extra spans for each query. + - go-redis no longer depends on OpenTelemetry. + + Eventually we hope to replace the information that we no longer collect with OpenTelemetry + Metrics. + ## v8.9 - Changed `PubSub.Channel` to only rely on `Ping` result. You can now use `WithChannelSize`, diff --git a/go.mod b/go.mod index bfe9148..aa13d2e 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,5 @@ require ( github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f github.com/onsi/ginkgo v1.15.0 github.com/onsi/gomega v1.10.5 - go.opentelemetry.io/otel v0.20.0 go.opentelemetry.io/otel/metric v0.20.0 - go.opentelemetry.io/otel/trace v0.20.0 ) diff --git a/internal/pool/conn.go b/internal/pool/conn.go index ee064c9..1ce29ed 100644 --- a/internal/pool/conn.go +++ b/internal/pool/conn.go @@ -65,26 +65,17 @@ func (cn *Conn) RemoteAddr() net.Addr { } func (cn *Conn) WithReader(ctx context.Context, timeout time.Duration, fn func(rd *proto.Reader) error) error { - ctx, span := internal.StartSpan(ctx, "redis.with_reader") - defer span.End() - if err := cn.netConn.SetReadDeadline(cn.deadline(ctx, timeout)); err != nil { - return internal.RecordError(ctx, span, err) + return err } - if err := fn(cn.rd); err != nil { - return internal.RecordError(ctx, span, err) - } - return nil + return fn(cn.rd) } func (cn *Conn) WithWriter( ctx context.Context, timeout time.Duration, fn func(wr *proto.Writer) error, ) error { - ctx, span := internal.StartSpan(ctx, "redis.with_writer") - defer span.End() - if err := cn.netConn.SetWriteDeadline(cn.deadline(ctx, timeout)); err != nil { - return internal.RecordError(ctx, span, err) + return err } if cn.bw.Buffered() > 0 { @@ -92,11 +83,11 @@ func (cn *Conn) WithWriter( } if err := fn(cn.wr); err != nil { - return internal.RecordError(ctx, span, err) + return err } if err := cn.bw.Flush(); err != nil { - return internal.RecordError(ctx, span, err) + return err } internal.WritesCounter.Add(ctx, 1) diff --git a/internal/util.go b/internal/util.go index 1a648fe..e34a7f0 100644 --- a/internal/util.go +++ b/internal/util.go @@ -4,16 +4,10 @@ import ( "context" "time" - "github.com/go-redis/redis/v8/internal/proto" "github.com/go-redis/redis/v8/internal/util" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/trace" ) func Sleep(ctx context.Context, dur time.Duration) error { - _, span := StartSpan(ctx, "time.Sleep") - defer span.End() - t := time.NewTimer(dur) defer t.Stop() @@ -50,21 +44,3 @@ func isLower(s string) bool { } return true } - -//------------------------------------------------------------------------------ - -var tracer = otel.Tracer("github.com/go-redis/redis") - -func StartSpan(ctx context.Context, name string) (context.Context, trace.Span) { - if span := trace.SpanFromContext(ctx); !span.IsRecording() { - return ctx, span - } - return tracer.Start(ctx, name) -} - -func RecordError(ctx context.Context, span trace.Span, err error) error { - if err != proto.Nil { - span.RecordError(err) - } - return err -} diff --git a/options.go b/options.go index 0fd8e88..7cf1bc1 100644 --- a/options.go +++ b/options.go @@ -12,9 +12,7 @@ import ( "strings" "time" - "github.com/go-redis/redis/v8/internal" "github.com/go-redis/redis/v8/internal/pool" - "go.opentelemetry.io/otel/attribute" ) // Limiter is the interface of a rate limiter or a circuit breaker. @@ -291,21 +289,7 @@ func getUserPassword(u *url.URL) (string, string) { func newConnPool(opt *Options) *pool.ConnPool { return pool.NewConnPool(&pool.Options{ Dialer: func(ctx context.Context) (net.Conn, error) { - ctx, span := internal.StartSpan(ctx, "redis.dial") - defer span.End() - - if span.IsRecording() { - span.SetAttributes( - attribute.String("db.connection_string", opt.Addr), - ) - } - - cn, err := opt.Dialer(ctx, opt.Network, opt.Addr) - if err != nil { - return nil, internal.RecordError(ctx, span, err) - } - - return cn, nil + return opt.Dialer(ctx, opt.Network, opt.Addr) }, PoolSize: opt.PoolSize, MinIdleConns: opt.MinIdleConns, diff --git a/redis.go b/redis.go index 7995c43..98d6034 100644 --- a/redis.go +++ b/redis.go @@ -10,7 +10,6 @@ import ( "github.com/go-redis/redis/v8/internal" "github.com/go-redis/redis/v8/internal/pool" "github.com/go-redis/redis/v8/internal/proto" - "go.opentelemetry.io/otel/attribute" ) // Nil reply returned by Redis when key does not exist. @@ -237,9 +236,6 @@ func (c *baseClient) initConn(ctx context.Context, cn *pool.Conn) error { return nil } - ctx, span := internal.StartSpan(ctx, "redis.init_conn") - defer span.End() - connPool := pool.NewSingleConnPool(c.connPool, cn) conn := newConn(ctx, c.opt, connPool) @@ -287,20 +283,11 @@ func (c *baseClient) releaseConn(ctx context.Context, cn *pool.Conn, err error) func (c *baseClient) withConn( ctx context.Context, fn func(context.Context, *pool.Conn) error, ) error { - ctx, span := internal.StartSpan(ctx, "redis.with_conn") - defer span.End() - cn, err := c.getConn(ctx) if err != nil { return err } - if span.IsRecording() { - if remoteAddr := cn.RemoteAddr(); remoteAddr != nil { - span.SetAttributes(attribute.String("net.peer.ip", remoteAddr.String())) - } - } - defer func() { c.releaseConn(ctx, cn, err) }()