Skip to content

First Proposal to filter out specific commands that shouldn't be traced via OTEL #3481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions extra/redisotel/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ type config struct {
tp trace.TracerProvider
tracer trace.Tracer

dbStmtEnabled bool
callerEnabled bool
dbStmtEnabled bool
callerEnabled bool
commandExclusions []string

// Metrics options.

Expand Down Expand Up @@ -54,13 +55,13 @@ func (fn option) metrics() {}

func newConfig(opts ...baseOption) *config {
conf := &config{
dbSystem: "redis",
attrs: []attribute.KeyValue{},

tp: otel.GetTracerProvider(),
mp: otel.GetMeterProvider(),
dbStmtEnabled: true,
callerEnabled: true,
dbSystem: "redis",
attrs: []attribute.KeyValue{},
commandExclusions: []string{},
tp: otel.GetTracerProvider(),
mp: otel.GetMeterProvider(),
dbStmtEnabled: true,
callerEnabled: true,
}

for _, opt := range opts {
Expand Down Expand Up @@ -124,6 +125,13 @@ func WithCallerEnabled(on bool) TracingOption {
})
}

// WithCommandExclusions tells the tracing hook to exclude the specified redis commands.
func WithCommandExclusions(exclusions []string) TracingOption {
return tracingOption(func(conf *config) {
conf.commandExclusions = exclusions
})
}

//------------------------------------------------------------------------------

type MetricsOption interface {
Expand Down
9 changes: 8 additions & 1 deletion extra/redisotel/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"strconv"
"strings"

"slices"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
semconv "go.opentelemetry.io/otel/semconv/v1.24.0"
Expand Down Expand Up @@ -102,6 +104,12 @@ func (th *tracingHook) DialHook(hook redis.DialHook) redis.DialHook {
func (th *tracingHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook {
return func(ctx context.Context, cmd redis.Cmder) error {

cmdString := rediscmd.CmdString(cmd)

if slices.Contains(th.conf.commandExclusions, cmdString) {
return nil
}

attrs := make([]attribute.KeyValue, 0, 8)
if th.conf.callerEnabled {
fn, file, line := funcFileLine("github.com/redis/go-redis")
Expand All @@ -113,7 +121,6 @@ func (th *tracingHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook {
}

if th.conf.dbStmtEnabled {
cmdString := rediscmd.CmdString(cmd)
attrs = append(attrs, semconv.DBStatement(cmdString))
}

Expand Down
77 changes: 77 additions & 0 deletions extra/redisotel/tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,83 @@ func TestTracingHook_ProcessPipelineHook_LongCommands(t *testing.T) {
}
}

func TestTracingHook_ProcessHook_LongCommand_WithExlusions(t *testing.T) {
imsb := tracetest.NewInMemoryExporter()
provider := sdktrace.NewTracerProvider(sdktrace.WithSyncer(imsb))
hook := newTracingHook(
"redis://localhost:6379",
WithTracerProvider(provider),
)
longValue := strings.Repeat("a", 102400)

tests := []struct {
name string
cmd redis.Cmder
expected string
}{
{
name: "short command",
cmd: redis.NewCmd(context.Background(), "SET", "key", "value"),
expected: "SET key value",
},
{
name: "set command with long key",
cmd: redis.NewCmd(context.Background(), "SET", longValue, "value"),
expected: "SET " + longValue + " value",
},
{
name: "set command with long value",
cmd: redis.NewCmd(context.Background(), "SET", "key", longValue),
expected: "SET key " + longValue,
},
{
name: "set command with long key and value",
cmd: redis.NewCmd(context.Background(), "SET", longValue, longValue),
expected: "SET " + longValue + " " + longValue,
},
{
name: "short command with many arguments",
cmd: redis.NewCmd(context.Background(), "MSET", "key1", "value1", "key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5"),
expected: "MSET key1 value1 key2 value2 key3 value3 key4 value4 key5 value5",
},
{
name: "long command",
cmd: redis.NewCmd(context.Background(), longValue, "key", "value"),
expected: longValue + " key value",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
defer imsb.Reset()

processHook := hook.ProcessHook(func(ctx context.Context, cmd redis.Cmder) error {
return nil
})

if err := processHook(context.Background(), tt.cmd); err != nil {
t.Fatal(err)
}

assertEqual(t, 1, len(imsb.GetSpans()))

spanData := imsb.GetSpans()[0]

var dbStatement string
for _, attr := range spanData.Attributes {
if attr.Key == semconv.DBStatementKey {
dbStatement = attr.Value.AsString()
break
}
}

if dbStatement != tt.expected {
t.Errorf("Expected DB statement: %q\nGot: %q", tt.expected, dbStatement)
}
})
}
}

func assertEqual(t *testing.T, expected, actual interface{}) {
t.Helper()
if expected != actual {
Expand Down
Loading