contrib/go-redis/redis.v8: add WithSkipRawCommand option and fix reso…

…urce (#1091)

This change adds a new option called `WithSkippedRawCommand` which
determines the instrumentation code to skip adding the
`redis.raw_command` tag to spans. This is useful in cases when the
Datadog Agent Redis obfuscation is disabled (which it is, by default)
and commands may contain sensitive information.

Additionally, duplicate entries and an incosistency was found when
setting the resource name for Redis spans, which was fixed as part of
this PR.

Closes #1022

contrib/go-redis/redis.v8/option.go

@@ -14,6 +14,7 @@ import (
 type clientConfig struct {
 	serviceName   string
 	analyticsRate float64
+	skipRaw       bool
 }
 
 // ClientOption represents an option that can be used to create or wrap a client.
@@ -29,6 +30,15 @@ func defaults(cfg *clientConfig) {
 	}
 }
 
+// WithSkipRawCommand reports whether to skip setting the "redis.raw_command" tag
+// on instrumenation spans. This may be useful if the Datadog Agent is not
+// set up to obfuscate this value and it could contain sensitive information.
+func WithSkipRawCommand(skip bool) ClientOption {
+	return func(cfg *clientConfig) {
+		cfg.skipRaw = skip
+	}
+}
+
 // WithServiceName sets the given service name for the client.
 func WithServiceName(name string) ClientOption {
 	return func(cfg *clientConfig) {

contrib/go-redis/redis.v8/redis.go

@@ -102,14 +102,16 @@ func (ddh *datadogHook) BeforeProcess(ctx context.Context, cmd redis.Cmder) (con
 	raw := cmd.String()
 	length := strings.Count(raw, " ")
 	p := ddh.params
-	opts := make([]ddtrace.StartSpanOption, 0, 5+len(ddh.additionalTags)+1) // 5 options below + for additionalTags + for analyticsRate
+	opts := make([]ddtrace.StartSpanOption, 0, 4+1+len(ddh.additionalTags)+1) // 4 options below + redis.raw_command + ddh.additionalTags + analyticsRate
 	opts = append(opts,
 		tracer.SpanType(ext.SpanTypeRedis),
 		tracer.ServiceName(p.config.serviceName),
 		tracer.ResourceName(raw[:strings.IndexByte(raw, ' ')]),
-		tracer.Tag("redis.raw_command", raw),
 		tracer.Tag("redis.args_length", strconv.Itoa(length)),
 	)
+	if !p.config.skipRaw {
+		opts = append(opts, tracer.Tag("redis.raw_command", raw))
+	}
 	opts = append(opts, ddh.additionalTags...)
 	if !math.IsNaN(p.config.analyticsRate) {
 		opts = append(opts, tracer.Tag(ext.EventSampleRate, p.config.analyticsRate))
@@ -134,16 +136,17 @@ func (ddh *datadogHook) BeforeProcessPipeline(ctx context.Context, cmds []redis.
 	raw := commandsToString(cmds)
 	length := strings.Count(raw, " ")
 	p := ddh.params
-	opts := make([]ddtrace.StartSpanOption, 0, 7+len(ddh.additionalTags)+1) // 7 options below + for additionalTags + for analyticsRate
+	opts := make([]ddtrace.StartSpanOption, 0, 5+1+len(ddh.additionalTags)+1) // 5 options below + redis.raw_command + ddh.additionalTags + analyticsRate
 	opts = append(opts,
 		tracer.SpanType(ext.SpanTypeRedis),
 		tracer.ServiceName(p.config.serviceName),
 		tracer.ResourceName(raw[:strings.IndexByte(raw, ' ')]),
-		tracer.Tag("redis.raw_command", raw),
 		tracer.Tag("redis.args_length", strconv.Itoa(length)),
-		tracer.Tag(ext.ResourceName, raw),
 		tracer.Tag("redis.pipeline_length", strconv.Itoa(len(cmds))),
 	)
+	if !p.config.skipRaw {
+		opts = append(opts, tracer.Tag("redis.raw_command", raw))
+	}
 	opts = append(opts, ddh.additionalTags...)
 	if !math.IsNaN(p.config.analyticsRate) {
 		opts = append(opts, tracer.Tag(ext.EventSampleRate, p.config.analyticsRate))

contrib/go-redis/redis.v8/redis_test.go

@@ -36,6 +36,51 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
+func TestSkipRaw(t *testing.T) {
+	runCmds := func(t *testing.T, opts ...ClientOption) []mocktracer.Span {
+		mt := mocktracer.Start()
+		defer mt.Stop()
+		ctx := context.Background()
+		client := NewClient(&redis.Options{Addr: "127.0.0.1:6379"}, opts...)
+		client.Set(ctx, "test_key", "test_value", 0)
+		pipeline := client.Pipeline()
+		pipeline.Expire(ctx, "pipeline_counter", time.Hour)
+		pipeline.Exec(ctx)
+		spans := mt.FinishedSpans()
+		assert.Len(t, spans, 2)
+		return spans
+	}
+
+	t.Run("true", func(t *testing.T) {
+		spans := runCmds(t, WithSkipRawCommand(true))
+		for _, span := range spans {
+			raw, ok := span.Tags()["redis.raw_command"]
+			assert.False(t, ok)
+			assert.Empty(t, raw)
+		}
+	})
+
+	t.Run("default", func(t *testing.T) {
+		spans := runCmds(t)
+		raw, ok := spans[0].Tags()["redis.raw_command"]
+		assert.True(t, ok)
+		assert.Equal(t, "set test_key test_value: ", raw)
+		raw, ok = spans[1].Tags()["redis.raw_command"]
+		assert.True(t, ok)
+		assert.Equal(t, "expire pipeline_counter 3600: false\n", raw)
+	})
+
+	t.Run("false", func(t *testing.T) {
+		spans := runCmds(t, WithSkipRawCommand(false))
+		raw, ok := spans[0].Tags()["redis.raw_command"]
+		assert.True(t, ok)
+		assert.Equal(t, "set test_key test_value: ", raw)
+		raw, ok = spans[1].Tags()["redis.raw_command"]
+		assert.True(t, ok)
+		assert.Equal(t, "expire pipeline_counter 3600: false\n", raw)
+	})
+}
+
 func TestClientEvalSha(t *testing.T) {
 	ctx := context.Background()
 	opts := &redis.Options{Addr: "127.0.0.1:6379"}
@@ -225,7 +270,7 @@ func TestPipeline(t *testing.T) {
 	assert.Equal("redis.command", span.OperationName())
 	assert.Equal(ext.SpanTypeRedis, span.Tag(ext.SpanType))
 	assert.Equal("my-redis", span.Tag(ext.ServiceName))
-	assert.Equal("expire pipeline_counter 3600: false\n", span.Tag(ext.ResourceName))
+	assert.Equal("expire", span.Tag(ext.ResourceName))
 	assert.Equal("127.0.0.1", span.Tag(ext.TargetHost))
 	assert.Equal("6379", span.Tag(ext.TargetPort))
 	assert.Equal("1", span.Tag("redis.pipeline_length"))
@@ -244,7 +289,7 @@ func TestPipeline(t *testing.T) {
 	assert.Equal("redis.command", span.OperationName())
 	assert.Equal(ext.SpanTypeRedis, span.Tag(ext.SpanType))
 	assert.Equal("my-redis", span.Tag(ext.ServiceName))
-	assert.Equal("expire pipeline_counter 3600: false\nexpire pipeline_counter_1 60: false\n", span.Tag(ext.ResourceName))
+	assert.Equal("expire", span.Tag(ext.ResourceName))
 	assert.Equal("2", span.Tag("redis.pipeline_length"))
 }