Skip to content

Commit

Permalink
Polishing redis#920
Browse files Browse the repository at this point in the history
Add author and since tags. Add excludeCommandArgsFromSpanTags(…) builder method to reflect default nature of args inclusion in tracing.
  • Loading branch information
mp911de committed Nov 19, 2018
1 parent 600a923 commit 83ffcae
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 37 deletions.
1 change: 1 addition & 0 deletions src/main/java/io/lettuce/core/protocol/CommandHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
* @author Mark Paluch
* @author Jongyeol Choi
* @author Grzegorz Szpak
* @author Daniel Albuquerque
*/
public class CommandHandler extends ChannelDuplexHandler implements HasQueuedCommands {

Expand Down
13 changes: 11 additions & 2 deletions src/main/java/io/lettuce/core/tracing/BraveTracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
* If one of the context objects above is found, it's used to determine the parent context for the command {@link Span}.
*
* @author Mark Paluch
* @author Daniel Albuquerque
* @see brave.Tracer
* @see brave.Tracing#currentTracer()
* @see BraveTraceContextProvider
Expand Down Expand Up @@ -139,8 +140,16 @@ public Builder serviceName(String serviceName) {
}

/**
* Controls the inclusion of command arguments in {@link Span} tags.
* This is enabled by default.
* Excludes command arguments from {@link Span} tags. Enabled by default.
*
* @return {@code this} {@link Builder}.
*/
public Builder excludeCommandArgsFromSpanTags() {
return includeCommandArgsInSpanTags(false);
}

/**
* Controls the inclusion of command arguments in {@link Span} tags. 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}.
Expand Down
1 change: 1 addition & 0 deletions src/main/java/io/lettuce/core/tracing/NoOpTracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* No-Op {@link Tracing} support that does not trace at all.
*
* @author Mark Paluch
* @author Daniel Albuquerque
* @since 5.1
*/
enum NoOpTracing implements Tracing, TraceContextProvider, TracerProvider {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/io/lettuce/core/tracing/Tracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* value objects to represent traces, spans and metadata in an dependency-agnostic manner.
*
* @author Mark Paluch
* @author Daniel Albuquerque
* @since 5.1
* @see TracerProvider
* @see TraceContextProvider
Expand All @@ -45,12 +46,15 @@ public interface Tracing {
/**
* Returns {@literal true} if tracing is enabled.
*
* @return {@literal true} if tracing DefaultClientResourcesTestis enabled.
* @return {@literal true} if tracing is enabled.
*/
boolean isEnabled();

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,13 @@
import io.lettuce.core.resource.ClientResources;
import io.lettuce.core.resource.DefaultClientResources;
import io.lettuce.test.Wait;
import io.lettuce.test.resource.FastShutdown;

/**
* Integration tests for {@link BraveTracing}.
*
* @author Mark Paluch
* @author Daniel Albuquerque
*/
class BraveTracingIntegrationTests extends TestSupport {

Expand Down Expand Up @@ -123,6 +127,35 @@ void pingWithTraceShouldCatchErrors() {
assertThat(spans.get(2).name()).isEqualTo("foo");
}

@Test
void getAndSetWithTraceWithCommandArgsExcludedFromTags() {

ClientResources clientResources = ClientResources.builder()
.tracing(BraveTracing.builder().tracing(clientTracing).excludeCommandArgsFromSpanTags().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.sync().set("foo", "bar");
connect.sync().get("foo");

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");

FastShutdown.shutdown(client);
FastShutdown.shutdown(clientResources);
}

@Test
void reactivePing() {

Expand Down Expand Up @@ -179,40 +212,6 @@ void reactiveGetAndSetWithTrace() {
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");
}

@Test
void reactiveGetAndSetWithTraceProvider() {

Expand Down

0 comments on commit 83ffcae

Please sign in to comment.