From d842612697c811abe592b6d8878fd792a92aee86 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 26 Feb 2019 13:36:08 -0800 Subject: [PATCH] Migrate Jedis/Lettuce instrumentation to Decorator --- .../jedis/JedisClientDecorator.java | 44 ++++++ .../jedis/JedisInstrumentation.java | 37 ++--- .../src/test/groovy/JedisClientTest.groovy | 130 ++++++++++++------ .../LettuceAsyncCommandsInstrumentation.java | 12 +- .../lettuce/LettuceClientInstrumentation.java | 20 ++- ...ettuceReactiveCommandsInstrumentation.java | 23 ++-- .../lettuce/ConnectionFutureAdvice.java | 38 ++--- .../lettuce/LettuceAsyncBiFunction.java | 22 ++- .../lettuce/LettuceAsyncCommandsAdvice.java | 32 ++--- .../lettuce/LettuceClientDecorator.java | 67 +++++++++ .../lettuce/LettuceInstrumentationUtil.java | 10 +- .../lettuce/rx/LettuceFluxCreationAdvice.java | 16 ++- .../rx/LettuceFluxTerminationRunnable.java | 65 ++++----- .../lettuce/rx/LettuceMonoCreationAdvice.java | 12 +- .../lettuce/rx/LettuceMonoDualConsumer.java | 53 +++---- .../test/groovy/LettuceAsyncClientTest.groovy | 4 +- .../test/groovy/LettuceSyncClientTest.groovy | 4 +- 17 files changed, 333 insertions(+), 256 deletions(-) create mode 100644 dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisClientDecorator.java create mode 100644 dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java diff --git a/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisClientDecorator.java b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisClientDecorator.java new file mode 100644 index 0000000000..7090de35d1 --- /dev/null +++ b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisClientDecorator.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.jedis; + +import datadog.trace.agent.decorator.DatabaseClientDecorator; +import datadog.trace.api.DDSpanTypes; +import redis.clients.jedis.Protocol; + +public class JedisClientDecorator extends DatabaseClientDecorator { + public static final JedisClientDecorator DECORATE = new JedisClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"jedis", "redis"}; + } + + @Override + protected String service() { + return "redis"; + } + + @Override + protected String component() { + return "redis-command"; + } + + @Override + protected String spanType() { + return DDSpanTypes.REDIS; + } + + @Override + protected String dbType() { + return "redis"; + } + + @Override + protected String dbUser(final Protocol.Command session) { + return null; + } + + @Override + protected String dbInstance(final Protocol.Command session) { + return null; + } +} diff --git a/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java index 24a4d70082..8b3c7112df 100644 --- a/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java +++ b/dd-java-agent/instrumentation/jedis-1.4/src/main/java/datadog/trace/instrumentation/jedis/JedisInstrumentation.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.jedis; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.jedis.JedisClientDecorator.DECORATE; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -9,13 +9,8 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -30,7 +25,7 @@ public final class JedisInstrumentation extends Instrumenter.Default { private static final String COMPONENT_NAME = SERVICE_NAME + "-command"; public JedisInstrumentation() { - super(SERVICE_NAME); + super("jedis", "redis"); } @Override @@ -40,7 +35,12 @@ public final class JedisInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { - return new String[] {}; + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".JedisClientDecorator", + }; } @Override @@ -51,35 +51,24 @@ public final class JedisInstrumentation extends Instrumenter.Default { .and(named("sendCommand")) .and(takesArgument(1, named("redis.clients.jedis.Protocol$Command"))), JedisAdvice.class.getName()); + // FIXME: This instrumentation only incorporates sending the command, not processing the result. } public static class JedisAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan(@Advice.Argument(1) final Command command) { - final Scope scope = GlobalTracer.get().buildSpan("redis.command").startActive(true); - - final Span span = scope.span(); - Tags.DB_TYPE.set(span, SERVICE_NAME); - Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(span, COMPONENT_NAME); - - span.setTag(DDTags.RESOURCE_NAME, command.name()); - span.setTag(DDTags.SERVICE_NAME, SERVICE_NAME); - span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.REDIS); - + DECORATE.afterStart(scope.span()); + DECORATE.onStatement(scope.span(), command.name()); return scope; } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { - if (throwable != null) { - final Span span = scope.span(); - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } + DECORATE.onError(scope.span(), throwable); + DECORATE.beforeFinish(scope.span()); scope.close(); } } diff --git a/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy b/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy index e5ca40cd80..3acffa2537 100644 --- a/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy +++ b/dd-java-agent/instrumentation/jedis-1.4/src/test/groovy/JedisClientTest.groovy @@ -1,8 +1,6 @@ -import datadog.opentracing.DDSpan import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDSpanTypes import datadog.trace.api.DDTags -import datadog.trace.instrumentation.jedis.JedisInstrumentation import io.opentracing.tag.Tags import redis.clients.jedis.Jedis import redis.embedded.RedisServer @@ -38,60 +36,110 @@ class JedisClientTest extends AgentTestRunner { } def "set command"() { + when: jedis.set("foo", "bar") - expect: - TEST_WRITER.size() == 1 - def trace = TEST_WRITER.firstTrace() - trace.size() == 1 - final DDSpan setTrace = trace.get(0) - setTrace.getServiceName() == JedisInstrumentation.SERVICE_NAME - setTrace.getOperationName() == "redis.query" - setTrace.getResourceName() == "SET" - setTrace.getSpanType() == DDSpanTypes.REDIS - setTrace.getTags().get(Tags.COMPONENT.getKey()) == JedisInstrumentation.COMPONENT_NAME - setTrace.getTags().get(Tags.DB_TYPE.getKey()) == JedisInstrumentation.SERVICE_NAME - setTrace.getTags().get(Tags.SPAN_KIND.getKey()) == Tags.SPAN_KIND_CLIENT - setTrace.getTags().get(DDTags.SPAN_TYPE) == JedisInstrumentation.SERVICE_NAME + then: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "SET" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT.key" "redis-command" + "$Tags.DB_TYPE.key" "redis" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.REDIS + defaultTags() + } + } + } + } } def "get command"() { + when: jedis.set("foo", "bar") def value = jedis.get("foo") - expect: + then: value == "bar" - TEST_WRITER.size() == 2 - def trace = TEST_WRITER.get(1) - trace.size() == 1 - final DDSpan getSpan = trace.get(0) - getSpan.getServiceName() == JedisInstrumentation.SERVICE_NAME - getSpan.getOperationName() == "redis.query" - getSpan.getResourceName() == "GET" - getSpan.getSpanType() == DDSpanTypes.REDIS - getSpan.getTags().get(Tags.COMPONENT.getKey()) == JedisInstrumentation.COMPONENT_NAME - getSpan.getTags().get(Tags.DB_TYPE.getKey()) == JedisInstrumentation.SERVICE_NAME - getSpan.getTags().get(Tags.SPAN_KIND.getKey()) == Tags.SPAN_KIND_CLIENT - getSpan.getTags().get(DDTags.SPAN_TYPE) == JedisInstrumentation.SERVICE_NAME + + assertTraces(2) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "SET" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT.key" "redis-command" + "$Tags.DB_TYPE.key" "redis" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.REDIS + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "GET" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT.key" "redis-command" + "$Tags.DB_TYPE.key" "redis" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.REDIS + defaultTags() + } + } + } + } } def "command with no arguments"() { + when: jedis.set("foo", "bar") def value = jedis.randomKey() - expect: + then: value == "foo" - TEST_WRITER.size() == 2 - def trace = TEST_WRITER.get(1) - trace.size() == 1 - final DDSpan randomKeySpan = trace.get(0) - randomKeySpan.getServiceName() == JedisInstrumentation.SERVICE_NAME - randomKeySpan.getOperationName() == "redis.query" - randomKeySpan.getResourceName() == "RANDOMKEY" - randomKeySpan.getSpanType() == DDSpanTypes.REDIS - randomKeySpan.getTags().get(Tags.COMPONENT.getKey()) == JedisInstrumentation.COMPONENT_NAME - randomKeySpan.getTags().get(Tags.DB_TYPE.getKey()) == JedisInstrumentation.SERVICE_NAME - randomKeySpan.getTags().get(Tags.SPAN_KIND.getKey()) == Tags.SPAN_KIND_CLIENT - randomKeySpan.getTags().get(DDTags.SPAN_TYPE) == JedisInstrumentation.SERVICE_NAME + + assertTraces(2) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "SET" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT.key" "redis-command" + "$Tags.DB_TYPE.key" "redis" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.REDIS + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + resourceName "RANDOMKEY" + spanType DDSpanTypes.REDIS + tags { + "$Tags.COMPONENT.key" "redis-command" + "$Tags.DB_TYPE.key" "redis" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.REDIS + defaultTags() + } + } + } + } } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java index 265d7b5f95..b0c15e6e52 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -15,9 +15,6 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { - public static final String PACKAGE = - LettuceAsyncCommandsInstrumentation.class.getPackage().getName(); - public LettuceAsyncCommandsInstrumentation() { super("lettuce", "lettuce-5-async"); } @@ -30,7 +27,12 @@ public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - PACKAGE + ".LettuceAsyncBiFunction", PACKAGE + ".LettuceInstrumentationUtil" + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".LettuceClientDecorator", + packageName + ".LettuceAsyncBiFunction", + packageName + ".LettuceInstrumentationUtil" }; } @@ -41,6 +43,6 @@ public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { .and(named("dispatch")) .and(takesArgument(0, named("io.lettuce.core.protocol.RedisCommand"))), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".LettuceAsyncCommandsAdvice"); + packageName + ".LettuceAsyncCommandsAdvice"); } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index 6103b94437..036fe2a5e6 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -19,15 +19,6 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class LettuceClientInstrumentation extends Instrumenter.Default { - public static final String PACKAGE = LettuceClientInstrumentation.class.getPackage().getName(); - - private static final String[] HELPER_CLASS_NAMES = - new String[] { - LettuceReactiveCommandsInstrumentation.class.getPackage().getName() - + ".LettuceInstrumentationUtil", - PACKAGE + ".LettuceAsyncBiFunction" - }; - public LettuceClientInstrumentation() { super("lettuce"); } @@ -39,7 +30,14 @@ public final class LettuceClientInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { - return HELPER_CLASS_NAMES; + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".LettuceClientDecorator", + packageName + ".LettuceInstrumentationUtil", + packageName + ".LettuceAsyncBiFunction" + }; } @Override @@ -52,6 +50,6 @@ public final class LettuceClientInstrumentation extends Instrumenter.Default { .and(nameEndsWith("Async")) .and(takesArgument(1, named("io.lettuce.core.RedisURI"))), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".ConnectionFutureAdvice"); + packageName + ".ConnectionFutureAdvice"); } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceReactiveCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceReactiveCommandsInstrumentation.java index 72e91c3df6..66aa57104c 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceReactiveCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java/datadog/trace/instrumentation/lettuce/LettuceReactiveCommandsInstrumentation.java @@ -18,9 +18,6 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public class LettuceReactiveCommandsInstrumentation extends Instrumenter.Default { - public static final String PACKAGE = - LettuceReactiveCommandsInstrumentation.class.getPackage().getName(); - public LettuceReactiveCommandsInstrumentation() { super("lettuce", "lettuce-5-rx"); } @@ -33,12 +30,16 @@ public class LettuceReactiveCommandsInstrumentation extends Instrumenter.Default @Override public String[] helperClassNames() { return new String[] { - PACKAGE + ".LettuceInstrumentationUtil", - PACKAGE + ".rx.LettuceMonoCreationAdvice", - PACKAGE + ".rx.LettuceMonoDualConsumer", - PACKAGE + ".rx.LettuceFluxCreationAdvice", - PACKAGE + ".rx.LettuceFluxTerminationRunnable", - PACKAGE + ".rx.LettuceFluxTerminationRunnable$FluxOnSubscribeConsumer" + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".LettuceClientDecorator", + packageName + ".LettuceInstrumentationUtil", + packageName + ".rx.LettuceMonoCreationAdvice", + packageName + ".rx.LettuceMonoDualConsumer", + packageName + ".rx.LettuceFluxCreationAdvice", + packageName + ".rx.LettuceFluxTerminationRunnable", + packageName + ".rx.LettuceFluxTerminationRunnable$FluxOnSubscribeConsumer" }; } @@ -51,7 +52,7 @@ public class LettuceReactiveCommandsInstrumentation extends Instrumenter.Default .and(takesArgument(0, named("java.util.function.Supplier"))) .and(returns(named("reactor.core.publisher.Mono"))), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".rx.LettuceMonoCreationAdvice"); + packageName + ".rx.LettuceMonoCreationAdvice"); transformers.put( isMethod() .and(nameStartsWith("create")) @@ -59,7 +60,7 @@ public class LettuceReactiveCommandsInstrumentation extends Instrumenter.Default .and(takesArgument(0, named("java.util.function.Supplier"))) .and(returns(named("reactor.core.publisher.Flux"))), // Cannot reference class directly here because it would lead to class load failure on Java7 - PACKAGE + ".rx.LettuceFluxCreationAdvice"); + packageName + ".rx.LettuceFluxCreationAdvice"); return transformers; } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/ConnectionFutureAdvice.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/ConnectionFutureAdvice.java index 2c29275d5f..d405010de7 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/ConnectionFutureAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/ConnectionFutureAdvice.java @@ -1,16 +1,12 @@ package datadog.trace.instrumentation.lettuce; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import io.lettuce.core.ConnectionFuture; import io.lettuce.core.RedisURI; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import net.bytebuddy.asm.Advice; public class ConnectionFutureAdvice { @@ -18,24 +14,9 @@ public class ConnectionFutureAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan(@Advice.Argument(1) final RedisURI redisURI) { final Scope scope = GlobalTracer.get().buildSpan("redis.query").startActive(false); - final Span span = scope.span(); - Tags.DB_TYPE.set(span, LettuceInstrumentationUtil.SERVICE_NAME); - Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(span, LettuceInstrumentationUtil.COMPONENT_NAME); - - final int redisPort = redisURI.getPort(); - Tags.PEER_PORT.set(span, redisPort); - final String redisHost = redisURI.getHost(); - Tags.PEER_HOSTNAME.set(span, redisHost); - - final String url = redisHost + ":" + redisPort + "/" + redisURI.getDatabase(); - span.setTag("db.redis.url", url); - span.setTag("db.redis.dbIndex", redisURI.getDatabase()); - span.setTag(DDTags.RESOURCE_NAME, "CONNECT:" + url); - span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); - span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.REDIS); - + DECORATE.afterStart(span); + DECORATE.onConnection(span, redisURI); return scope; } @@ -43,17 +24,16 @@ public class ConnectionFutureAdvice { public static void stopSpan( @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable, - @Advice.Return final ConnectionFuture connectionFuture) { + @Advice.Return final ConnectionFuture connectionFuture) { + final Span span = scope.span(); if (throwable != null) { - final Span span = scope.span(); - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); scope.close(); return; } - - // close spans on error or normal completion - connectionFuture.handleAsync(new LettuceAsyncBiFunction<>(scope.span())); + connectionFuture.handleAsync(new LettuceAsyncBiFunction<>(span)); scope.close(); } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java index a48868b240..e9b940ba65 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java @@ -1,10 +1,8 @@ package datadog.trace.instrumentation.lettuce; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; import io.opentracing.Span; -import io.opentracing.tag.Tags; -import java.util.Collections; import java.util.concurrent.CancellationException; import java.util.function.BiFunction; @@ -22,21 +20,19 @@ public class LettuceAsyncBiFunction(scope.span())); + if (!doFinishSpanEarly(command)) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); } scope.close(); } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java new file mode 100644 index 0000000000..491bdb6448 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -0,0 +1,67 @@ +package datadog.trace.instrumentation.lettuce; + +import datadog.trace.agent.decorator.DatabaseClientDecorator; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import io.lettuce.core.RedisURI; +import io.lettuce.core.protocol.RedisCommand; +import io.opentracing.Span; +import io.opentracing.tag.Tags; + +public class LettuceClientDecorator extends DatabaseClientDecorator { + public static final LettuceClientDecorator DECORATE = new LettuceClientDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"lettuce"}; + } + + @Override + protected String service() { + return "redis"; + } + + @Override + protected String component() { + return "redis-client"; + } + + @Override + protected String spanType() { + return DDSpanTypes.REDIS; + } + + @Override + protected String dbType() { + return "redis"; + } + + @Override + protected String dbUser(final RedisURI connection) { + return null; + } + + @Override + protected String dbInstance(final RedisURI connection) { + return connection.getHost() + ":" + connection.getPort() + "/" + connection.getDatabase(); + } + + @Override + public Span onConnection(final Span span, final RedisURI connection) { + if (connection != null) { + Tags.PEER_HOSTNAME.set(span, connection.getHost()); + Tags.PEER_PORT.set(span, connection.getPort()); + + span.setTag("db.redis.dbIndex", connection.getDatabase()); + span.setTag(DDTags.RESOURCE_NAME, "CONNECT:" + dbInstance(connection)); + } + return super.onConnection(span, connection); + } + + public Span onCommand(final Span span, final RedisCommand command) { + final String commandName = LettuceInstrumentationUtil.getCommandName(command); + span.setTag( + DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(commandName)); + return span; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java index 4e6b83b9de..6d95607985 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java @@ -7,16 +7,13 @@ import java.util.Set; public class LettuceInstrumentationUtil { - public static final String SERVICE_NAME = "redis"; - public static final String COMPONENT_NAME = SERVICE_NAME + "-client"; - public static final String[] NON_INSTRUMENTING_COMMAND_WORDS = new String[] {"SHUTDOWN", "DEBUG", "OOM", "SEGFAULT"}; public static final String[] AGENT_CRASHING_COMMANDS_WORDS = new String[] {"CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT"}; - public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAMND-NAME:"; + public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; public static final Set nonInstrumentingCommands = new HashSet<>(Arrays.asList(NON_INSTRUMENTING_COMMAND_WORDS)); @@ -29,10 +26,11 @@ public class LettuceInstrumentationUtil { * added and the command is executed) because these commands have no return values/call backs, so * we must close the span early in order to provide info for the users * - * @param commandName a redis command, without any prefixes + * @param command * @return true if finish the span early (the command will not have a return value) */ - public static boolean doFinishSpanEarly(final String commandName) { + public static boolean doFinishSpanEarly(final RedisCommand command) { + final String commandName = LettuceInstrumentationUtil.getCommandName(command); return nonInstrumentingCommands.contains(commandName); } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxCreationAdvice.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxCreationAdvice.java index f1335ef090..354e8830bf 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxCreationAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxCreationAdvice.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.lettuce.rx; -import datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; + import io.lettuce.core.protocol.RedisCommand; import java.util.function.Supplier; import net.bytebuddy.asm.Advice; @@ -9,19 +10,20 @@ import reactor.core.publisher.Flux; public class LettuceFluxCreationAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static String extractCommandName( + public static RedisCommand extractCommandName( @Advice.Argument(0) final Supplier supplier) { - return LettuceInstrumentationUtil.getCommandName(supplier.get()); + return supplier.get(); } // if there is an exception thrown, then don't make spans @Advice.OnMethodExit(suppress = Throwable.class) public static void monitorSpan( - @Advice.Enter final String commandName, @Advice.Return(readOnly = false) Flux publisher) { + @Advice.Enter final RedisCommand command, + @Advice.Return(readOnly = false) Flux publisher) { - boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandName); - LettuceFluxTerminationRunnable handler = - new LettuceFluxTerminationRunnable(commandName, finishSpanOnClose); + final boolean finishSpanOnClose = doFinishSpanEarly(command); + final LettuceFluxTerminationRunnable handler = + new LettuceFluxTerminationRunnable(command, finishSpanOnClose); publisher = publisher.doOnSubscribe(handler.getOnSubscribeConsumer()); // don't register extra callbacks to finish the spans if the command being instrumented is one // of those that return diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxTerminationRunnable.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxTerminationRunnable.java index 3d776e625a..7e70bdf99d 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxTerminationRunnable.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceFluxTerminationRunnable.java @@ -1,15 +1,10 @@ package datadog.trace.instrumentation.lettuce.rx; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; -import datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil; -import io.opentracing.Scope; +import io.lettuce.core.protocol.RedisCommand; import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.function.Consumer; import org.reactivestreams.Subscription; import org.slf4j.LoggerFactory; @@ -23,8 +18,9 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl private int numResults = 0; private FluxOnSubscribeConsumer onSubscribeConsumer = null; - public LettuceFluxTerminationRunnable(final String commandName, final boolean finishSpanOnClose) { - this.onSubscribeConsumer = new FluxOnSubscribeConsumer(this, commandName, finishSpanOnClose); + public LettuceFluxTerminationRunnable( + final RedisCommand command, final boolean finishSpanOnClose) { + onSubscribeConsumer = new FluxOnSubscribeConsumer(this, command, finishSpanOnClose); } public FluxOnSubscribeConsumer getOnSubscribeConsumer() { @@ -32,16 +28,14 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl } private void finishSpan(final boolean isCommandCancelled, final Throwable throwable) { - if (this.span != null) { - this.span.setTag("db.command.results.count", this.numResults); + if (span != null) { + span.setTag("db.command.results.count", numResults); if (isCommandCancelled) { - this.span.setTag("db.command.cancelled", true); + span.setTag("db.command.cancelled", true); } - if (throwable != null) { - Tags.ERROR.set(this.span, true); - this.span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } - this.span.finish(); + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); } else { LoggerFactory.getLogger(Flux.class) .error( @@ -56,13 +50,13 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl || SignalType.ON_ERROR.equals(signal.getType())) { finishSpan(false, signal.getThrowable()); } else if (SignalType.ON_NEXT.equals(signal.getType())) { - ++this.numResults; + ++numResults; } } @Override public void run() { - if (this.span != null) { + if (span != null) { finishSpan(true, null); } else { LoggerFactory.getLogger(Flux.class) @@ -75,39 +69,28 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl public static class FluxOnSubscribeConsumer implements Consumer { private final LettuceFluxTerminationRunnable owner; - private final String commandName; + private final RedisCommand command; private final boolean finishSpanOnClose; public FluxOnSubscribeConsumer( final LettuceFluxTerminationRunnable owner, - final String commandName, + final RedisCommand command, final boolean finishSpanOnClose) { this.owner = owner; - this.commandName = commandName; + this.command = command; this.finishSpanOnClose = finishSpanOnClose; } @Override public void accept(final Subscription subscription) { - final Scope scope = - GlobalTracer.get() - .buildSpan(LettuceInstrumentationUtil.SERVICE_NAME + ".query") - .startActive(finishSpanOnClose); - final Span span = scope.span(); - this.owner.span = span; - - Tags.DB_TYPE.set(span, LettuceInstrumentationUtil.SERVICE_NAME); - Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(span, LettuceInstrumentationUtil.COMPONENT_NAME); - - // should be command name only, but use workaround to prepend string to agent crashing - // commands - span.setTag( - DDTags.RESOURCE_NAME, - LettuceInstrumentationUtil.getCommandResourceName(this.commandName)); - span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); - span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.REDIS); - scope.close(); + final Span span = GlobalTracer.get().buildSpan("redis.query").start(); + owner.span = span; + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + if (finishSpanOnClose) { + DECORATE.beforeFinish(span); + span.finish(); + } } } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoCreationAdvice.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoCreationAdvice.java index 36c6d56292..aa17be20ad 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoCreationAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoCreationAdvice.java @@ -9,19 +9,19 @@ import reactor.core.publisher.Mono; public class LettuceMonoCreationAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static String extractCommandName( + public static RedisCommand extractCommandName( @Advice.Argument(0) final Supplier supplier) { - return LettuceInstrumentationUtil.getCommandName(supplier.get()); + return supplier.get(); } // throwables wouldn't matter here, because no spans have been started due to redis command not // being run until the user subscribes to the Mono publisher @Advice.OnMethodExit(suppress = Throwable.class) public static void monitorSpan( - @Advice.Enter final String commandName, @Advice.Return(readOnly = false) Mono publisher) { - - boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandName); - LettuceMonoDualConsumer mdc = new LettuceMonoDualConsumer(commandName, finishSpanOnClose); + @Advice.Enter final RedisCommand command, + @Advice.Return(readOnly = false) Mono publisher) { + final boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(command); + final LettuceMonoDualConsumer mdc = new LettuceMonoDualConsumer(command, finishSpanOnClose); publisher = publisher.doOnSubscribe(mdc); // register the call back to close the span only if necessary if (!finishSpanOnClose) { diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoDualConsumer.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoDualConsumer.java index c69658d67c..4750f30806 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoDualConsumer.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/rx/LettuceMonoDualConsumer.java @@ -1,15 +1,11 @@ package datadog.trace.instrumentation.lettuce.rx; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; -import datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil; +import io.lettuce.core.protocol.RedisCommand; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; -import java.util.Collections; import java.util.function.BiConsumer; import java.util.function.Consumer; import org.slf4j.LoggerFactory; @@ -19,22 +15,29 @@ public class LettuceMonoDualConsumer implements Consumer, BiConsumer { private Span span = null; - private final String commandName; + private final RedisCommand command; private final boolean finishSpanOnClose; - public LettuceMonoDualConsumer(final String commandName, final boolean finishSpanOnClose) { - this.commandName = commandName; + public LettuceMonoDualConsumer(final RedisCommand command, final boolean finishSpanOnClose) { + this.command = command; this.finishSpanOnClose = finishSpanOnClose; } + @Override + public void accept(final R r) { + final Scope scope = GlobalTracer.get().buildSpan("redis.query").startActive(finishSpanOnClose); + span = scope.span(); + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + scope.close(); + } + @Override public void accept(final T t, final Throwable throwable) { - if (this.span != null) { - if (throwable != null) { - Tags.ERROR.set(this.span, true); - this.span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } - this.span.finish(); + if (span != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); } else { LoggerFactory.getLogger(Mono.class) .error( @@ -42,24 +45,4 @@ public class LettuceMonoDualConsumer + "it probably wasn't started."); } } - - @Override - public void accept(final R r) { - final Scope scope = - GlobalTracer.get() - .buildSpan(LettuceInstrumentationUtil.SERVICE_NAME + ".query") - .startActive(finishSpanOnClose); - this.span = scope.span(); - - Tags.DB_TYPE.set(this.span, LettuceInstrumentationUtil.SERVICE_NAME); - Tags.SPAN_KIND.set(this.span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(this.span, LettuceInstrumentationUtil.COMPONENT_NAME); - - // should be command name only, but use workaround to prepend string to agent crashing commands - this.span.setTag( - DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(this.commandName)); - this.span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); - this.span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.REDIS); - scope.close(); - } } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy index f44821d9d3..095391e399 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceAsyncClientTest.groovy @@ -124,7 +124,7 @@ class LettuceAsyncClientTest extends AgentTestRunner { tags { defaultTags() "component" "redis-client" - "db.redis.url" dbAddr + "db.instance" dbAddr "db.redis.dbIndex" 0 "db.type" "redis" "peer.hostname" HOST @@ -165,7 +165,7 @@ class LettuceAsyncClientTest extends AgentTestRunner { tags { defaultTags() "component" "redis-client" - "db.redis.url" dbAddrNonExistent + "db.instance" dbAddrNonExistent "db.redis.dbIndex" 0 "db.type" "redis" errorTags CompletionException, String diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy index 51bcf96edf..169caefef5 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceSyncClientTest.groovy @@ -104,7 +104,7 @@ class LettuceSyncClientTest extends AgentTestRunner { tags { defaultTags() "component" "redis-client" - "db.redis.url" dbAddr + "db.instance" dbAddr "db.redis.dbIndex" 0 "db.type" "redis" "peer.hostname" HOST @@ -142,7 +142,7 @@ class LettuceSyncClientTest extends AgentTestRunner { tags { defaultTags() "component" "redis-client" - "db.redis.url" dbAddrNonExistent + "db.instance" dbAddrNonExistent "db.redis.dbIndex" 0 "db.type" "redis" errorTags CompletionException, String