Skip to content

Commit

Permalink
Add flag to disable reporting of span tags redis#920
Browse files Browse the repository at this point in the history
  • Loading branch information
worldtiki authored and mp911de committed Nov 19, 2018
1 parent ce632ff commit 600a923
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/main/java/io/lettuce/core/protocol/CommandHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public class CommandHandler extends ChannelDuplexHandler implements HasQueuedCom
private final boolean debugEnabled = logger.isDebugEnabled();
private final boolean latencyMetricsEnabled;
private final boolean tracingEnabled;
private final boolean includeCommandArgsInSpanTags;
private final boolean boundedQueues;
private final BackpressureSource backpressureSource = new BackpressureSource();

Expand Down Expand Up @@ -111,6 +112,7 @@ public CommandHandler(ClientOptions clientOptions, ClientResources clientResourc
Tracing tracing = clientResources.tracing();

this.tracingEnabled = tracing.isEnabled();
this.includeCommandArgsInSpanTags = tracing.includeCommandArgsInSpanTags();
}

public Queue<RedisCommand<?, ?, ?>> getStack() {
Expand Down Expand Up @@ -385,7 +387,7 @@ private void writeSingleCommand(ChannelHandlerContext ctx, RedisCommand<?, ?, ?>
Tracer.Span span = tracer.nextSpan(context);
span.name(command.getType().name());

if (command.getArgs() != null) {
if (includeCommandArgsInSpanTags && command.getArgs() != null) {
span.tag("redis.args", command.getArgs().toCommandString());
}

Expand Down
21 changes: 21 additions & 0 deletions src/main/java/io/lettuce/core/tracing/BraveTracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class BraveTracing implements Tracing {

private final BraveTracer tracer;
private final BraveTracingOptions tracingOptions;
private final boolean includeCommandArgsInSpanTags;

/**
* Create a new {@link BraveTracing} instance.
Expand All @@ -68,6 +69,7 @@ private BraveTracing(Builder builder) {

this.tracingOptions = new BraveTracingOptions(builder.serviceName, builder.endpointCustomizer, builder.spanCustomizer);
this.tracer = new BraveTracer(builder.tracing, this.tracingOptions);
this.includeCommandArgsInSpanTags = builder.includeCommandArgsInSpanTags;
}

/**
Expand Down Expand Up @@ -103,6 +105,7 @@ public static class Builder {
};
private Consumer<brave.Span> spanCustomizer = it -> {
};
private boolean includeCommandArgsInSpanTags = true;

private Builder() {
}
Expand Down Expand Up @@ -135,6 +138,19 @@ public Builder serviceName(String serviceName) {
return this;
}

/**
* Controls the inclusion of command arguments in {@link Span} tags.
* This is enabled by default.
*
* @param includeCommandArgsInSpanTags the flag to enable or disable the inclusion of command args in {@link Span} tags.
* @return {@code this} {@link Builder}.
*/
public Builder includeCommandArgsInSpanTags(boolean includeCommandArgsInSpanTags) {

this.includeCommandArgsInSpanTags = includeCommandArgsInSpanTags;
return this;
}

/**
* Sets an {@link zipkin2.Endpoint} customizer to customize the {@link zipkin2.Endpoint} through its
* {@link zipkin2.Endpoint.Builder}. The customizer is invoked before {@link zipkin2.Endpoint.Builder#build() building}
Expand Down Expand Up @@ -182,6 +198,11 @@ public boolean isEnabled() {
return true;
}

@Override
public boolean includeCommandArgsInSpanTags() {
return includeCommandArgsInSpanTags;
}

@Override
public TracerProvider getTracerProvider() {
return () -> tracer;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/io/lettuce/core/tracing/NoOpTracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public boolean isEnabled() {
return false;
}

@Override
public boolean includeCommandArgsInSpanTags() {
return false;
}

@Override
public Endpoint createEndpoint(SocketAddress socketAddress) {
return NOOP_ENDPOINT;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/io/lettuce/core/tracing/Tracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public interface Tracing {
*/
boolean isEnabled();

/**
* @return {@literal true} if tags for {@link Tracer.Span}s should include the command arguments.
*/
boolean includeCommandArgsInSpanTags();

/**
* Create an {@link Endpoint} given {@link SocketAddress}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,43 @@ void reactiveGetAndSetWithTrace() {
List<Span> spans = new ArrayList<>(BraveTracingIntegrationTests.spans);

assertThat(spans.get(0).name()).isEqualTo("set");
assertThat(spans.get(0).tags()).containsEntry("redis.args", "key<foo> value<bar>");
assertThat(spans.get(1).name()).isEqualTo("get");
assertThat(spans.get(1).tags()).containsEntry("redis.args", "key<foo>");
assertThat(spans.get(2).name()).isEqualTo("foo");
}

@Test
void reactiveGetAndSetWithTraceWithCommandArgsExcludedFromTags() {

DefaultClientResources clientResources = DefaultClientResources.builder()
.tracing(BraveTracing.builder()
.tracing(clientTracing)
.includeCommandArgsInSpanTags(false)
.build()
)
.build();
RedisClient client = RedisClient.create(clientResources, RedisURI.Builder.redis(host, port).build());

ScopedSpan trace = clientTracing.tracer().startScopedSpan("foo");

StatefulRedisConnection<String, String> connect = client.connect();
connect.reactive().set("foo", "bar") //
.then(connect.reactive().get("foo")) //
.subscriberContext(it -> it.put(TraceContext.class, trace.context())) //
.as(StepVerifier::create) //
.expectNext("bar").verifyComplete();

Wait.untilEquals(2, spans::size).waitOrTimeout();

trace.finish();

List<Span> spans = new ArrayList<>(BraveTracingIntegrationTests.spans);

assertThat(spans.get(0).name()).isEqualTo("set");
assertThat(spans.get(0).tags()).doesNotContainKey("redis.args");
assertThat(spans.get(1).name()).isEqualTo("get");
assertThat(spans.get(1).tags()).doesNotContainKey("redis.args");
assertThat(spans.get(2).name()).isEqualTo("foo");
}

Expand Down

0 comments on commit 600a923

Please sign in to comment.