From 82c7cfd1d15a2065aa924aa9ce95aff4164d20a2 Mon Sep 17 00:00:00 2001 From: Shannon Tan Date: Tue, 23 Feb 2021 10:29:55 -0800 Subject: [PATCH 1/7] Implement context extension --- ddtrace/opentracer/tracer.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index 8916f56034..8a1205657d 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -20,6 +20,7 @@ package opentracer import ( + "context" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/internal" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -35,6 +36,7 @@ func New(opts ...tracer.StartOption) opentracing.Tracer { } var _ opentracing.Tracer = (*opentracer)(nil) +var _ opentracing.TracerContextWithSpanExtension = (*opentracer)(nil) // opentracer implements opentracing.Tracer on top of ddtrace.Tracer. type opentracer struct{ ddtrace.Tracer } @@ -84,3 +86,12 @@ func (t *opentracer) Extract(format interface{}, carrier interface{}) (opentraci return nil, opentracing.ErrUnsupportedFormat } } + +func (t *opentracer) ContextWithSpanHook(ctx context.Context, openSpan opentracing.Span) context.Context { + ddSpan, ok := openSpan.(*span) + if !ok { + return ctx // unchanged + } + + return tracer.ContextWithSpan(ctx, ddSpan.Span) +} From bd764ae860527cfa0a5f6162d46573c2c3cd266f Mon Sep 17 00:00:00 2001 From: Shannon Tan Date: Tue, 23 Feb 2021 11:35:44 -0800 Subject: [PATCH 2/7] Implement context extension --- ddtrace/opentracer/tracer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index 8a1205657d..d46c4d0205 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -92,6 +92,6 @@ func (t *opentracer) ContextWithSpanHook(ctx context.Context, openSpan opentraci if !ok { return ctx // unchanged } - + return tracer.ContextWithSpan(ctx, ddSpan.Span) } From c5ea938bec6653bf98306328458c2bc403af50a5 Mon Sep 17 00:00:00 2001 From: Shannon Tan <45950434+shannontan-bolt@users.noreply.github.com> Date: Fri, 26 Feb 2021 10:52:24 -0800 Subject: [PATCH 3/7] Update ddtrace/opentracer/tracer.go Co-authored-by: Gabriel Aszalos --- ddtrace/opentracer/tracer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index d46c4d0205..ffa44a65ee 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -87,6 +87,7 @@ func (t *opentracer) Extract(format interface{}, carrier interface{}) (opentraci } } +// ContextWithSpan implements opentracing.TracerContextWithSpanExtension. func (t *opentracer) ContextWithSpanHook(ctx context.Context, openSpan opentracing.Span) context.Context { ddSpan, ok := openSpan.(*span) if !ok { From 0c95b4356645df5e771c96ccac8b4294756f28f1 Mon Sep 17 00:00:00 2001 From: Shannon Tan <45950434+shannontan-bolt@users.noreply.github.com> Date: Fri, 26 Feb 2021 10:52:30 -0800 Subject: [PATCH 4/7] Update ddtrace/opentracer/tracer.go Co-authored-by: Gabriel Aszalos --- ddtrace/opentracer/tracer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index ffa44a65ee..384939a8cd 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -91,7 +91,7 @@ func (t *opentracer) Extract(format interface{}, carrier interface{}) (opentraci func (t *opentracer) ContextWithSpanHook(ctx context.Context, openSpan opentracing.Span) context.Context { ddSpan, ok := openSpan.(*span) if !ok { - return ctx // unchanged + return ctx } return tracer.ContextWithSpan(ctx, ddSpan.Span) From f624bde752f050b5a498a72dcd4f882ffcbc2acb Mon Sep 17 00:00:00 2001 From: Shannon Tan Date: Fri, 26 Feb 2021 10:53:37 -0800 Subject: [PATCH 5/7] Fix for comment --- ddtrace/opentracer/tracer.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index 384939a8cd..888d358145 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -93,6 +93,5 @@ func (t *opentracer) ContextWithSpanHook(ctx context.Context, openSpan opentraci if !ok { return ctx } - return tracer.ContextWithSpan(ctx, ddSpan.Span) } From 047de4093b8496376fadbc5999574a5542701fc8 Mon Sep 17 00:00:00 2001 From: Shannon Tan Date: Mon, 1 Mar 2021 09:58:16 -0800 Subject: [PATCH 6/7] Fix --- ddtrace/opentracer/tracer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/opentracer/tracer.go b/ddtrace/opentracer/tracer.go index 888d358145..0214a35ffe 100644 --- a/ddtrace/opentracer/tracer.go +++ b/ddtrace/opentracer/tracer.go @@ -21,6 +21,7 @@ package opentracer import ( "context" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/internal" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" From 0ab271b70596be4715ef2b746763ea3b61e9f603 Mon Sep 17 00:00:00 2001 From: Shannon Tan Date: Fri, 8 Oct 2021 11:38:05 -0700 Subject: [PATCH 7/7] Add Option to Exclude Raw Command Tag --- contrib/go-redis/redis.v8/option.go | 13 ++++++- contrib/go-redis/redis.v8/redis.go | 19 +++++++--- contrib/go-redis/redis.v8/redis_test.go | 50 +++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/contrib/go-redis/redis.v8/option.go b/contrib/go-redis/redis.v8/option.go index 86f90d8b59..bbd6e6e3c4 100644 --- a/contrib/go-redis/redis.v8/option.go +++ b/contrib/go-redis/redis.v8/option.go @@ -12,8 +12,9 @@ import ( ) type clientConfig struct { - serviceName string - analyticsRate float64 + serviceName string + analyticsRate float64 + excludeRawCommand bool } // ClientOption represents an option that can be used to create or wrap a client. @@ -58,3 +59,11 @@ func WithAnalyticsRate(rate float64) ClientOption { } } } + +// WithExcludeRawCommand sets the option to exclude the raw command +// for when the command/cache might contain sensitive data +func WithExcludeRawCommand(exclude bool) ClientOption { + return func(cfg *clientConfig) { + cfg.excludeRawCommand = exclude + } +} diff --git a/contrib/go-redis/redis.v8/redis.go b/contrib/go-redis/redis.v8/redis.go index afa9728e20..8d11aaa586 100644 --- a/contrib/go-redis/redis.v8/redis.go +++ b/contrib/go-redis/redis.v8/redis.go @@ -102,14 +102,18 @@ 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 + var opts []ddtrace.StartSpanOption 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.excludeRawCommand { + 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 +138,21 @@ 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 + var opts []ddtrace.StartSpanOption 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.excludeRawCommand { + opts = append(opts, + tracer.Tag("redis.raw_command", raw), + tracer.Tag(ext.ResourceName, raw)) + } + opts = append(opts, ddh.additionalTags...) if !math.IsNaN(p.config.analyticsRate) { opts = append(opts, tracer.Tag(ext.EventSampleRate, p.config.analyticsRate)) diff --git a/contrib/go-redis/redis.v8/redis_test.go b/contrib/go-redis/redis.v8/redis_test.go index bd68d4bdf0..1837c66cbb 100644 --- a/contrib/go-redis/redis.v8/redis_test.go +++ b/contrib/go-redis/redis.v8/redis_test.go @@ -464,3 +464,53 @@ func TestWithContext(t *testing.T) { assert.Equal(span1.SpanID(), setSpan.ParentID()) assert.Equal(span2.SpanID(), getSpan.ParentID()) } + +func TestWithExcludeRawCommand(t *testing.T) { + execCommand := func(t *testing.T, mt mocktracer.Tracer, opts ...ClientOption) { + 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) + } + + t.Run("defaults", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + execCommand(t, mt) + + spans := mt.FinishedSpans() + assert.Len(t, spans, 2) + for _, s := range spans { + assert.Equal(t, "SET test_key test_value", s.Tag("redis.raw_command")) + } + }) + + t.Run("with exclude true", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + execCommand(t, mt, WithExcludeRawCommand(true)) + + spans := mt.FinishedSpans() + assert.Len(t, spans, 2) + for _, s := range spans { + assert.Equal(t, nil, s.Tag("redis.raw_command")) + } + }) + + t.Run("with exclude false", func(t *testing.T) { + mt := mocktracer.Start() + defer mt.Stop() + + execCommand(t, mt, WithExcludeRawCommand(false)) + + spans := mt.FinishedSpans() + assert.Len(t, spans, 2) + for _, s := range spans { + assert.Equal(t, "SET test_key test_value", s.Tag("redis.raw_command")) + } + }) +}