Remove OpenTelemetry from the code (but leave redisotel as is) (#1782)

Author: vmihailenco

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`,

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
 )

internal/pool/conn.go

@@ -65,38 +65,29 @@ 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)
-	}
-	if err := fn(cn.rd); err != nil {
-		return internal.RecordError(ctx, span, err)
+		return 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 {
 		cn.bw.Reset(cn.netConn)
 	}
 
 	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)

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
-}

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,

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)
 	}()