From 3f8bc96e8dc58e9e39c2f913a4df4835af8665a2 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Thu, 23 Apr 2020 17:19:21 +0100 Subject: [PATCH 1/8] get lettuce 4.0 sync and async working --- .../lettuce-4.0/lettuce-4.0.gradle | 34 ++ .../LettuceAsyncCommandsInstrumentation.java | 45 ++ .../lettuce/LettuceClientInstrumentation.java | 40 ++ .../lettuce/LettuceAsyncBiFunction.java | 39 ++ .../lettuce/LettuceAsyncCommandsAdvice.java | 45 ++ .../lettuce/LettuceClientDecorator.java | 76 +++ .../lettuce/LettuceInstrumentationUtil.java | 71 +++ .../lettuce/RedisConnectionAdvice.java | 35 ++ .../test/groovy/LettuceAsyncClientTest.groovy | 542 ++++++++++++++++++ .../test/groovy/LettuceSyncClientTest.groovy | 384 +++++++++++++ settings.gradle | 1 + 11 files changed, 1312 insertions(+) create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle new file mode 100644 index 00000000000..2b60b3b5810 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -0,0 +1,34 @@ +// Set properties before any plugins get loaded +ext { + minJavaVersionForTests = JavaVersion.VERSION_1_8 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 +} + +muzzle { + pass { + group = "biz.paluch.redis" + module = "lettuce" + versions = "[4.0.Final,4.5.0.Final]" + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +dependencies { + compileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' + main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.5.0.Final' + + testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' + testCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' + + latestDepTestCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.+' +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java new file mode 100644 index 00000000000..45c0bb1584e --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.lettuce; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public class LettuceAsyncCommandsInstrumentation extends Instrumenter.Default { + + public LettuceAsyncCommandsInstrumentation() { + super("lettuce", "lettuce-4-async"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("com.lambdaworks.redis.AbstractRedisAsyncCommands"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LettuceClientDecorator", + packageName + ".LettuceAsyncBiFunction", + packageName + ".LettuceInstrumentationUtil" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod() + .and(named("dispatch")) + .and(takesArgument(0, named("com.lambdaworks.redis.protocol.RedisCommand"))), + // Cannot reference class directly here because it would lead to class load failure on Java7 + packageName + ".LettuceAsyncCommandsAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java new file mode 100644 index 00000000000..aeb2c3f8be7 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -0,0 +1,40 @@ +package datadog.trace.instrumentation.lettuce; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class LettuceClientInstrumentation extends Instrumenter.Default { + + public LettuceClientInstrumentation() { + super("lettuce"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("com.lambdaworks.redis.RedisClient"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil" + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(named("connectStandalone")), + // Cannot reference class directly here because it would lead to class load failure on Java7 + packageName + ".RedisConnectionAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java new file mode 100644 index 00000000000..ea60863bee7 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java @@ -0,0 +1,39 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; + +import java.util.concurrent.CancellationException; +import java.util.function.BiFunction; + +/** + * Callback class to close the span on an error or a success in the RedisFuture returned by the + * lettuce async API + * + * @param the normal completion result + * @param the error + * @param the return type, should be null since nothing else should happen from tracing + * standpoint after the span is closed + */ +public class LettuceAsyncBiFunction + implements BiFunction { + + private final AgentSpan span; + + public LettuceAsyncBiFunction(final AgentSpan span) { + this.span = span; + } + + @Override + public R apply(final T t, final Throwable throwable) { + if (throwable instanceof CancellationException) { + span.setTag("db.command.cancelled", true); + } else { + DECORATE.onError(span, throwable); + } + DECORATE.beforeFinish(span); + span.finish(); + return null; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java new file mode 100644 index 00000000000..b205cf28087 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; + +import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +public class LettuceAsyncCommandsAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { + final AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + return activateSpan(span, doFinishSpanEarly(command)); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Argument(0) final RedisCommand command, + @Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable, + @Advice.Return final AsyncCommand asyncCommand) { + final AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + scope.close(); + return; + } + + // close spans on error or normal completion + if (!doFinishSpanEarly(command)) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + } + scope.close(); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java new file mode 100644 index 00000000000..e8315e78a42 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -0,0 +1,76 @@ +package datadog.trace.instrumentation.lettuce; + +import com.lambdaworks.redis.RedisURI; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.bootstrap.instrumentation.decorator.DatabaseClientDecorator; + +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 null; + } + + @Override + public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { + if (connection != null) { + span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); + span.setTag(Tags.PEER_PORT, connection.getPort()); + + span.setTag("db.redis.dbIndex", connection.getDatabase()); + span.setTag( + DDTags.RESOURCE_NAME, + "CONNECT:" + + connection.getHost() + + ":" + + connection.getPort() + + "/" + + connection.getDatabase()); + } + return super.onConnection(span, connection); + } + + @SuppressWarnings("rawtypes") + public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { + String commandName = LettuceInstrumentationUtil.getCommandName(command); + span.setTag( + DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(commandName)); + return span; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java new file mode 100644 index 00000000000..75024c44ad4 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.lettuce; + +import com.lambdaworks.redis.protocol.RedisCommand; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +public class LettuceInstrumentationUtil { + + public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; + + public static final Set nonInstrumentingCommands = + new HashSet<>(Arrays.asList("SHUTDOWN", "DEBUG", "OOM", "SEGFAULT")); + + public static final Set agentCrashingCommands = + new HashSet<>(Arrays.asList("CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT")); + + /** + * Determines whether a redis command should finish its relevant span early (as soon as tags are + * 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 command + * @return true if finish the span early (the command will not have a return value) + */ + public static boolean doFinishSpanEarly(final RedisCommand command) { + final String commandName = LettuceInstrumentationUtil.getCommandName(command); + return nonInstrumentingCommands.contains(commandName); + } + + // Workaround to keep trace agent from crashing + // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and + // traces with these commands as the resource name will not be processed by the trace agent + // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has + // list of commands that will currently fail at the trace agent level. + + /** + * Workaround to keep trace agent from crashing Currently the commands in + * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * resource name will not be processed by the trace agent + * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of + * commands that will currently fail at the trace agent level. + * + * @param actualCommandName the actual redis command + * @return the redis command with a prefix if it is a command that will crash the trace agent, + * otherwise, the original command is returned. + */ + public static String getCommandResourceName(final String actualCommandName) { + if (agentCrashingCommands.contains(actualCommandName)) { + return AGENT_CRASHING_COMMAND_PREFIX + actualCommandName; + } + return actualCommandName; + } + + /** + * Retrieves the actual redis command name from a RedisCommand object + * + * @param command the lettuce RedisCommand object + * @return the redis command as a string + */ + public static String getCommandName(final RedisCommand command) { + String commandName = "Redis Command"; + if (command != null) { + // get the redis command name (i.e. GET, SET, HMSET, etc) + if (command.getType() != null) { + commandName = command.getType().name().trim(); + } + } + return commandName; + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java new file mode 100644 index 00000000000..f616ce940f8 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -0,0 +1,35 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; + +import com.lambdaworks.redis.RedisURI; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +public class RedisConnectionAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onConnection(span, redisURI); + return activateSpan(span, false); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { + AgentSpan span = scope.span(); + try { + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } + } finally { + span.finish(); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy new file mode 100644 index 00000000000..e4683ffc25f --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy @@ -0,0 +1,542 @@ +import com.lambdaworks.redis.ClientOptions +import com.lambdaworks.redis.RedisClient +import com.lambdaworks.redis.RedisFuture +import com.lambdaworks.redis.RedisURI +import com.lambdaworks.redis.api.StatefulConnection +import com.lambdaworks.redis.api.async.RedisAsyncCommands +import com.lambdaworks.redis.api.sync.RedisCommands +import com.lambdaworks.redis.protocol.AsyncCommand +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.bootstrap.instrumentation.api.Tags + +import redis.embedded.RedisServer +import spock.lang.Shared +import spock.util.concurrent.AsyncConditions +import com.lambdaworks.redis.codec.Utf8StringCodec + +import java.util.concurrent.CancellationException +import java.util.concurrent.TimeUnit +import java.util.function.BiConsumer +import java.util.function.BiFunction +import java.util.function.Consumer +import java.util.function.Function + +import com.lambdaworks.redis.RedisConnectionException + +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX + +class LettuceAsyncClientTest extends AgentTestRunner { + public static final String HOST = "127.0.0.1" + public static final int DB_INDEX = 0 + // Disable autoreconnect so we do not get stray traces popping up on server shutdown + public static final ClientOptions CLIENT_OPTIONS = new ClientOptions.Builder().autoReconnect(false).build() + + @Shared + int port + @Shared + int incorrectPort + @Shared + String dbAddr + @Shared + String dbAddrNonExistent + @Shared + String dbUriNonExistent + @Shared + String embeddedDbUri + + @Shared + RedisServer redisServer + + @Shared + Map testHashMap = [ + firstname: "John", + lastname : "Doe", + age : "53" + ] + + RedisClient redisClient + StatefulConnection connection + RedisAsyncCommands asyncCommands + RedisCommands syncCommands + + def setupSpec() { + port = PortUtils.randomOpenPort() + incorrectPort = PortUtils.randomOpenPort() + dbAddr = HOST + ":" + port + "/" + DB_INDEX + dbAddrNonExistent = HOST + ":" + incorrectPort + "/" + DB_INDEX + dbUriNonExistent = "redis://" + dbAddrNonExistent + embeddedDbUri = "redis://" + dbAddr + + redisServer = RedisServer.builder() + // bind to localhost to avoid firewall popup + .setting("bind " + HOST) + // set max memory to avoid problems in CI + .setting("maxmemory 128M") + .port(port).build() + } + + def setup() { + redisClient = RedisClient.create(embeddedDbUri) + + println "Using redis: $redisServer.args" + redisServer.start() + redisClient.setOptions(CLIENT_OPTIONS) + + connection = redisClient.connect() + asyncCommands = connection.async() + syncCommands = connection.sync() + + syncCommands.set("TESTKEY", "TESTVAL") + + // 1 set + 1 connect trace + TEST_WRITER.waitForTraces(2) + TEST_WRITER.clear() + } + + def cleanup() { + connection.close() + redisServer.stop() + } + + def "connect using get on ConnectionFuture"() { + setup: + RedisClient testConnectionClient = RedisClient.create(embeddedDbUri) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect(new Utf8StringCodec(), + new RedisURI(HOST, port, 3, TimeUnit.SECONDS)) + + then: + connection != null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddr + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" port + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + defaultTags() + } + } + } + } + + cleanup: + connection.close() + } + + def "connect exception inside the connection future"() { + setup: + RedisClient testConnectionClient = RedisClient.create(dbUriNonExistent) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect(new Utf8StringCodec(), + new RedisURI(HOST, incorrectPort, 3, TimeUnit.SECONDS)) + + then: + connection == null + thrown RedisConnectionException + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddrNonExistent + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" incorrectPort + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + errorTags RedisConnectionException, String + defaultTags() + } + } + } + } + } + + def "set command using Future get with timeout"() { + setup: + RedisFuture redisFuture = asyncCommands.set("TESTSETKEY", "TESTSETVAL") + String res = redisFuture.get(3, TimeUnit.SECONDS) + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get command chained with thenAccept"() { + setup: + def conds = new AsyncConditions() + Consumer consumer = new Consumer() { + @Override + void accept(String res) { + conds.evaluate { + assert res == "TESTVAL" + } + } + } + + when: + RedisFuture redisFuture = asyncCommands.get("TESTKEY") + redisFuture.thenAccept(consumer) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + // to make sure instrumentation's chained completion stages won't interfere with user's, while still + // recording metrics + def "get non existent key command with handleAsync and chained with thenApply"() { + setup: + def conds = new AsyncConditions() + final String successStr = "KEY MISSING" + BiFunction firstStage = new BiFunction() { + @Override + String apply(String res, Throwable throwable) { + conds.evaluate { + assert res == null + assert throwable == null + } + return (res == null ? successStr : res) + } + } + Function secondStage = new Function() { + @Override + Object apply(String input) { + conds.evaluate { + assert input == successStr + } + return null + } + } + + when: + RedisFuture redisFuture = asyncCommands.get("NON_EXISTENT_KEY") + redisFuture.handleAsync(firstStage).thenApply(secondStage) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command with no arguments using a biconsumer"() { + setup: + def conds = new AsyncConditions() + BiConsumer biConsumer = new BiConsumer() { + @Override + void accept(String keyRetrieved, Throwable throwable) { + conds.evaluate { + assert keyRetrieved != null + } + } + } + + when: + RedisFuture redisFuture = asyncCommands.randomkey() + redisFuture.whenCompleteAsync(biConsumer) + + then: + conds.await() + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "RANDOMKEY" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash set and then nest apply to hash getall"() { + setup: + def conds = new AsyncConditions() + + when: + RedisFuture hmsetFuture = asyncCommands.hmset("TESTHM", testHashMap) + hmsetFuture.thenApplyAsync(new Function() { + @Override + Object apply(String setResult) { + TEST_WRITER.waitForTraces(1) // Wait for 'hmset' trace to get written + conds.evaluate { + assert setResult == "OK" + } + RedisFuture> hmGetAllFuture = asyncCommands.hgetall("TESTHM") + hmGetAllFuture.exceptionally(new Function>() { + @Override + Map apply(Throwable throwable) { + println("unexpected:" + throwable.toString()) + throwable.printStackTrace() + assert false + return null + } + }) + hmGetAllFuture.thenAccept(new Consumer>() { + @Override + void accept(Map hmGetAllResult) { + conds.evaluate { + assert testHashMap == hmGetAllResult + } + } + }) + return null + } + }) + + then: + conds.await() + assertTraces(2) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HMSET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + trace(1, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HGETALL" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command completes exceptionally"() { + setup: + // turn off auto flush to complete the command exceptionally manually + asyncCommands.setAutoFlushCommands(false) + def conds = new AsyncConditions() + RedisFuture redisFuture = asyncCommands.del("key1", "key2") + boolean completedExceptionally = ((AsyncCommand) redisFuture).completeExceptionally(new IllegalStateException("TestException")) + redisFuture.exceptionally({ + throwable -> + conds.evaluate { + assert throwable != null + assert throwable instanceof IllegalStateException + assert throwable.getMessage() == "TestException" + } + throw throwable + }) + + when: + // now flush and execute the command + asyncCommands.flushCommands() + redisFuture.get() + + then: + conds.await() + completedExceptionally == true + thrown Exception + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "DEL" + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + errorTags(IllegalStateException, "TestException") + defaultTags() + } + } + } + } + } + + def "cancel command before it finishes"() { + setup: + asyncCommands.setAutoFlushCommands(false) + def conds = new AsyncConditions() + RedisFuture redisFuture = asyncCommands.sadd("SKEY", "1", "2") + redisFuture.whenCompleteAsync({ + res, throwable -> + conds.evaluate { + assert throwable != null + assert throwable instanceof CancellationException + } + }) + + when: + boolean cancelSuccess = redisFuture.cancel(true) + asyncCommands.flushCommands() + + then: + conds.await() + cancelSuccess == true + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SADD" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + "db.command.cancelled" true + defaultTags() + } + } + } + } + } + + def "debug segfault command (returns void) with no argument should produce span"() { + setup: + asyncCommands.debugSegfault() + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName AGENT_CRASHING_COMMAND_PREFIX + "DEBUG" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + + def "shutdown command (returns void) should produce a span"() { + setup: + asyncCommands.shutdown(false) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SHUTDOWN" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy new file mode 100644 index 00000000000..f1cdf0b8813 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy @@ -0,0 +1,384 @@ +import com.lambdaworks.redis.ClientOptions +import com.lambdaworks.redis.RedisClient +import com.lambdaworks.redis.RedisConnectionException +import com.lambdaworks.redis.api.StatefulConnection +import com.lambdaworks.redis.api.sync.RedisCommands +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.PortUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.bootstrap.instrumentation.api.Tags +import redis.embedded.RedisServer +import spock.lang.Shared + +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX + +class LettuceSyncClientTest extends AgentTestRunner { + public static final String HOST = "127.0.0.1" + public static final int DB_INDEX = 0 + // Disable autoreconnect so we do not get stray traces popping up on server shutdown + public static final ClientOptions CLIENT_OPTIONS = new ClientOptions.Builder().autoReconnect(false).build() + + @Shared + int port + @Shared + int incorrectPort + @Shared + String dbAddr + @Shared + String dbAddrNonExistent + @Shared + String dbUriNonExistent + @Shared + String embeddedDbUri + + @Shared + RedisServer redisServer + + @Shared + Map testHashMap = [ + firstname: "John", + lastname : "Doe", + age : "53" + ] + + RedisClient redisClient + StatefulConnection connection + RedisCommands syncCommands + + def setupSpec() { + port = PortUtils.randomOpenPort() + incorrectPort = PortUtils.randomOpenPort() + dbAddr = HOST + ":" + port + "/" + DB_INDEX + dbAddrNonExistent = HOST + ":" + incorrectPort + "/" + DB_INDEX + dbUriNonExistent = "redis://" + dbAddrNonExistent + embeddedDbUri = "redis://" + dbAddr + + redisServer = RedisServer.builder() + // bind to localhost to avoid firewall popup + .setting("bind " + HOST) + // set max memory to avoid problems in CI + .setting("maxmemory 128M") + .port(port).build() + } + + def setup() { + redisClient = RedisClient.create(embeddedDbUri) + + redisServer.start() + connection = redisClient.connect() + syncCommands = connection.sync() + + syncCommands.set("TESTKEY", "TESTVAL") + syncCommands.hmset("TESTHM", testHashMap) + + // 2 sets + 1 connect trace + TEST_WRITER.waitForTraces(3) + TEST_WRITER.clear() + } + + def cleanup() { + connection.close() + redisServer.stop() + } + + def "connect"() { + setup: + RedisClient testConnectionClient = RedisClient.create(embeddedDbUri) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + StatefulConnection connection = testConnectionClient.connect() + + then: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddr + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" port + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + defaultTags() + } + } + } + } + + cleanup: + connection.close() + } + + def "connect exception"() { + setup: + RedisClient testConnectionClient = RedisClient.create(dbUriNonExistent) + testConnectionClient.setOptions(CLIENT_OPTIONS) + + when: + testConnectionClient.connect() + + then: + thrown RedisConnectionException + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "CONNECT:" + dbAddrNonExistent + errored true + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" HOST + "$Tags.PEER_PORT" incorrectPort + "$Tags.DB_TYPE" "redis" + "db.redis.dbIndex" 0 + errorTags RedisConnectionException, String + defaultTags() + } + } + } + } + } + + def "set command"() { + setup: + String res = syncCommands.set("TESTSETKEY", "TESTSETVAL") + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get command"() { + setup: + String res = syncCommands.get("TESTKEY") + + expect: + res == "TESTVAL" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "get non existent key command"() { + setup: + String res = syncCommands.get("NON_EXISTENT_KEY") + + expect: + res == null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "GET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "command with no arguments"() { + setup: + def keyRetrieved = syncCommands.randomkey() + + expect: + keyRetrieved != null + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "RANDOMKEY" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "list command"() { + setup: + long res = syncCommands.lpush("TESTLIST", "TESTLIST ELEMENT") + + expect: + res == 1 + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "LPUSH" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash set command"() { + setup: + def res = syncCommands.hmset("user", testHashMap) + + expect: + res == "OK" + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HMSET" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "hash getall command"() { + setup: + Map res = syncCommands.hgetall("TESTHM") + + expect: + res == testHashMap + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "HGETALL" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "debug segfault command (returns void) with no argument should produce span"() { + setup: + syncCommands.debugSegfault() + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName AGENT_CRASHING_COMMAND_PREFIX + "DEBUG" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } + + def "shutdown command (returns void) should produce a span"() { + setup: + syncCommands.shutdown(false) + + expect: + assertTraces(1) { + trace(0, 1) { + span(0) { + serviceName "redis" + operationName "redis.query" + spanType DDSpanTypes.REDIS + resourceName "SHUTDOWN" + errored false + + tags { + "$Tags.COMPONENT" "redis-client" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" "redis" + defaultTags() + } + } + } + } + } +} diff --git a/settings.gradle b/settings.gradle index 70e04f45663..c154297a42d 100644 --- a/settings.gradle +++ b/settings.gradle @@ -107,6 +107,7 @@ include ':dd-java-agent:instrumentation:jms' include ':dd-java-agent:instrumentation:jsp-2.3' include ':dd-java-agent:instrumentation:kafka-clients-0.11' include ':dd-java-agent:instrumentation:kafka-streams-0.11' +include ':dd-java-agent:instrumentation:lettuce-4.0' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:log4j1' include ':dd-java-agent:instrumentation:log4j2' From 8ef57915bef1c71a7ac16a605dcc680941f156fd Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 15:04:06 +0100 Subject: [PATCH 2/8] minimise impact on instrumentation point bytecode size --- .../LettuceAsyncCommandsInstrumentation.java | 3 +- .../lettuce/LettuceClientInstrumentation.java | 4 +- .../lettuce/InstrumentationPoints.java | 56 +++++++++++++++++++ .../lettuce/LettuceAsyncCommandsAdvice.java | 33 +++-------- .../lettuce/RedisConnectionAdvice.java | 21 +------ 5 files changed, 70 insertions(+), 47 deletions(-) create mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java index 45c0bb1584e..815ba64a9a0 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -29,7 +29,8 @@ public String[] helperClassNames() { return new String[] { packageName + ".LettuceClientDecorator", packageName + ".LettuceAsyncBiFunction", - packageName + ".LettuceInstrumentationUtil" + packageName + ".LettuceInstrumentationUtil", + packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index aeb2c3f8be7..adfbf9a29a6 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -26,7 +26,9 @@ public ElementMatcher typeMatcher() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil" + packageName + ".LettuceClientDecorator", + packageName + ".LettuceInstrumentationUtil", + packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java new file mode 100644 index 00000000000..e13e81e8bc0 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -0,0 +1,56 @@ +package datadog.trace.instrumentation.lettuce; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; + +import com.lambdaworks.redis.RedisURI; +import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.RedisCommand; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +@SuppressWarnings("rawtypes") +public class InstrumentationPoints { + + public static AgentScope onEnter(RedisCommand command) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onCommand(span, command); + return activateSpan(span, doFinishSpanEarly(command)); + } + + public static void onReturn(RedisCommand command, + AgentScope scope, + Throwable throwable, + AsyncCommand asyncCommand) { + AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + span.finish(); + } else if (!doFinishSpanEarly(command)) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + } + scope.close(); + } + + public static AgentScope onEnter(RedisURI redisURI) { + AgentSpan span = startSpan("redis.query"); + DECORATE.afterStart(span); + DECORATE.onConnection(span, redisURI); + return activateSpan(span, false); + } + + public static void onReturn(AgentScope scope, Throwable throwable) { + AgentSpan span = scope.span(); + if (throwable != null) { + DECORATE.onError(span, throwable); + DECORATE.beforeFinish(span); + } + span.finish(); + scope.close(); + } +} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java index b205cf28087..95725d0fbd3 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -1,45 +1,26 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; public class LettuceAsyncCommandsAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { - final AgentSpan span = startSpan("redis.query"); - DECORATE.afterStart(span); - DECORATE.onCommand(span, command); - return activateSpan(span, doFinishSpanEarly(command)); + return InstrumentationPoints.onEnter(command); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Argument(0) final RedisCommand command, - @Advice.Enter final AgentScope scope, - @Advice.Thrown final Throwable throwable, - @Advice.Return final AsyncCommand asyncCommand) { - final AgentSpan span = scope.span(); - if (throwable != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - span.finish(); - scope.close(); - return; - } - - // close spans on error or normal completion - if (!doFinishSpanEarly(command)) { - asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); - } - scope.close(); + @Advice.Argument(0) RedisCommand command, + @Advice.Enter AgentScope scope, + @Advice.Thrown Throwable throwable, + @Advice.Return AsyncCommand asyncCommand) { + InstrumentationPoints.onReturn(command, scope, throwable, asyncCommand); } + } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java index f616ce940f8..58f49eb6ac3 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -1,35 +1,18 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; - import com.lambdaworks.redis.RedisURI; import datadog.trace.bootstrap.instrumentation.api.AgentScope; -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import net.bytebuddy.asm.Advice; public class RedisConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { - AgentSpan span = startSpan("redis.query"); - DECORATE.afterStart(span); - DECORATE.onConnection(span, redisURI); - return activateSpan(span, false); + return InstrumentationPoints.onEnter(redisURI); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { - AgentSpan span = scope.span(); - try { - if (throwable != null) { - DECORATE.onError(span, throwable); - DECORATE.beforeFinish(span); - } - } finally { - span.finish(); - scope.close(); - } + InstrumentationPoints.onReturn(scope, throwable); } } From d1feb819edae1eadf299969b58bc103b20a0023b Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 16:36:47 +0100 Subject: [PATCH 3/8] use enumsets for keyword checking --- .../lettuce/LettuceClientInstrumentation.java | 3 +- .../lettuce/InstrumentationPoints.java | 9 ++- .../lettuce/LettuceClientDecorator.java | 8 +-- .../lettuce/LettuceInstrumentationUtil.java | 65 ++++++++++--------- 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index adfbf9a29a6..2637707eb32 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -28,7 +28,8 @@ public String[] helperClassNames() { return new String[] { packageName + ".LettuceClientDecorator", packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints" + packageName + ".InstrumentationPoints", + packageName + ".LettuceAsyncBiFunction" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index e13e81e8bc0..8205fcf5f18 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -3,23 +3,22 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.doFinishSpanEarly; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.finishSpanEarly; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -import net.bytebuddy.asm.Advice; @SuppressWarnings("rawtypes") -public class InstrumentationPoints { +public final class InstrumentationPoints { public static AgentScope onEnter(RedisCommand command) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onCommand(span, command); - return activateSpan(span, doFinishSpanEarly(command)); + return activateSpan(span, finishSpanEarly(command)); } public static void onReturn(RedisCommand command, @@ -31,7 +30,7 @@ public static void onReturn(RedisCommand command, DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); span.finish(); - } else if (!doFinishSpanEarly(command)) { + } else if (!finishSpanEarly(command)) { asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); } scope.close(); diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java index e8315e78a42..2309405c015 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.lettuce; +import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.getCommandResourceName; + import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.api.DDSpanTypes; @@ -52,7 +54,6 @@ public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { if (connection != null) { span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); span.setTag(Tags.PEER_PORT, connection.getPort()); - span.setTag("db.redis.dbIndex", connection.getDatabase()); span.setTag( DDTags.RESOURCE_NAME, @@ -68,9 +69,8 @@ public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { @SuppressWarnings("rawtypes") public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { - String commandName = LettuceInstrumentationUtil.getCommandName(command); - span.setTag( - DDTags.RESOURCE_NAME, LettuceInstrumentationUtil.getCommandResourceName(commandName)); + span.setTag(DDTags.RESOURCE_NAME, + null == command ? "Redis Command" : getCommandResourceName(command.getType())); return span; } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java index 75024c44ad4..4ec0b3bba80 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java @@ -1,19 +1,31 @@ package datadog.trace.instrumentation.lettuce; +import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; +import static com.lambdaworks.redis.protocol.CommandType.CLIENT; +import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; +import static com.lambdaworks.redis.protocol.CommandType.COMMAND; +import static com.lambdaworks.redis.protocol.CommandType.CONFIG; +import static com.lambdaworks.redis.protocol.CommandType.DEBUG; +import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; +import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; + +import com.lambdaworks.redis.protocol.CommandKeyword; +import com.lambdaworks.redis.protocol.CommandType; +import com.lambdaworks.redis.protocol.ProtocolKeyword; import com.lambdaworks.redis.protocol.RedisCommand; -import java.util.Arrays; -import java.util.HashSet; + +import java.util.EnumSet; import java.util.Set; public class LettuceInstrumentationUtil { public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; - public static final Set nonInstrumentingCommands = - new HashSet<>(Arrays.asList("SHUTDOWN", "DEBUG", "OOM", "SEGFAULT")); + public static final EnumSet NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); - public static final Set agentCrashingCommands = - new HashSet<>(Arrays.asList("CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT")); + public static final EnumSet NON_INSTRUMENTING_KEYWORDS = EnumSet.of(SEGFAULT); + + public static final Set AGENT_CRASHING_COMMANDS = EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); /** * Determines whether a redis command should finish its relevant span early (as soon as tags are @@ -23,9 +35,17 @@ public class LettuceInstrumentationUtil { * @param command * @return true if finish the span early (the command will not have a return value) */ - public static boolean doFinishSpanEarly(final RedisCommand command) { - final String commandName = LettuceInstrumentationUtil.getCommandName(command); - return nonInstrumentingCommands.contains(commandName); + public static boolean finishSpanEarly(final RedisCommand command) { + ProtocolKeyword keyword = command.getType(); + return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); + } + + private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { + return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); + } + + private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { + return keyword instanceof CommandKeyword && NON_INSTRUMENTING_KEYWORDS.contains(keyword); } // Workaround to keep trace agent from crashing @@ -41,31 +61,14 @@ public static boolean doFinishSpanEarly(final RedisCommand command) { * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of * commands that will currently fail at the trace agent level. * - * @param actualCommandName the actual redis command + * @param keyword the actual redis command * @return the redis command with a prefix if it is a command that will crash the trace agent, * otherwise, the original command is returned. */ - public static String getCommandResourceName(final String actualCommandName) { - if (agentCrashingCommands.contains(actualCommandName)) { - return AGENT_CRASHING_COMMAND_PREFIX + actualCommandName; - } - return actualCommandName; - } - - /** - * Retrieves the actual redis command name from a RedisCommand object - * - * @param command the lettuce RedisCommand object - * @return the redis command as a string - */ - public static String getCommandName(final RedisCommand command) { - String commandName = "Redis Command"; - if (command != null) { - // get the redis command name (i.e. GET, SET, HMSET, etc) - if (command.getType() != null) { - commandName = command.getType().name().trim(); - } + public static String getCommandResourceName(ProtocolKeyword keyword) { + if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { + return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); } - return commandName; + return keyword.name(); } } From 6273880d5e53482fdef95b5b0981a32b153f2b7b Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 16:47:14 +0100 Subject: [PATCH 4/8] address versions comments --- dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle index 2b60b3b5810..aa17ca152c5 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -8,7 +8,7 @@ muzzle { pass { group = "biz.paluch.redis" module = "lettuce" - versions = "[4.0.Final,4.5.0.Final]" + versions = "[4.0.Final,)" assertInverse = true } } @@ -25,7 +25,7 @@ testSets { dependencies { compileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' - main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.5.0.Final' + main_java8CompileOnly group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' testCompile group: 'biz.paluch.redis', name: 'lettuce', version: '4.0.Final' From 5271952930aa0fad3aa57534a97fcef5fac88c12 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 17:35:46 +0100 Subject: [PATCH 5/8] remove upper bound on Java version --- dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle index aa17ca152c5..0556359443d 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle +++ b/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle @@ -1,7 +1,6 @@ // Set properties before any plugins get loaded ext { minJavaVersionForTests = JavaVersion.VERSION_1_8 - maxJavaVersionForTests = JavaVersion.VERSION_1_8 } muzzle { From d158590ab781e75cb9098ee9d25dc062c00a52ae Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Fri, 24 Apr 2020 22:17:03 +0100 Subject: [PATCH 6/8] code cleanup --- .../LettuceAsyncCommandsInstrumentation.java | 5 +- .../lettuce/LettuceClientInstrumentation.java | 5 +- .../lettuce/InstrumentationPoints.java | 93 +++++++++++++++++-- .../lettuce/LettuceAsyncBiFunction.java | 39 -------- .../lettuce/LettuceAsyncCommandsAdvice.java | 18 ++-- .../lettuce/LettuceClientDecorator.java | 31 ++++--- .../lettuce/LettuceInstrumentationUtil.java | 74 --------------- .../lettuce/RedisConnectionAdvice.java | 9 +- .../test/groovy/LettuceAsyncClientTest.groovy | 2 +- .../test/groovy/LettuceSyncClientTest.groovy | 2 +- 10 files changed, 116 insertions(+), 162 deletions(-) delete mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java delete mode 100644 dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java index 815ba64a9a0..e618650194a 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java @@ -27,10 +27,7 @@ public ElementMatcher typeMatcher() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", - packageName + ".LettuceAsyncBiFunction", - packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints" + packageName + ".LettuceClientDecorator", packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java index 2637707eb32..bb1d00119f8 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java @@ -26,10 +26,7 @@ public ElementMatcher typeMatcher() { @Override public String[] helperClassNames() { return new String[] { - packageName + ".LettuceClientDecorator", - packageName + ".LettuceInstrumentationUtil", - packageName + ".InstrumentationPoints", - packageName + ".LettuceAsyncBiFunction" + packageName + ".LettuceClientDecorator", packageName + ".InstrumentationPoints" }; } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index 8205fcf5f18..2195cee9747 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -1,49 +1,77 @@ package datadog.trace.instrumentation.lettuce; +import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; +import static com.lambdaworks.redis.protocol.CommandType.CLIENT; +import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; +import static com.lambdaworks.redis.protocol.CommandType.COMMAND; +import static com.lambdaworks.redis.protocol.CommandType.CONFIG; +import static com.lambdaworks.redis.protocol.CommandType.DEBUG; +import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; +import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.finishSpanEarly; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.AsyncCommand; +import com.lambdaworks.redis.protocol.CommandType; +import com.lambdaworks.redis.protocol.ProtocolKeyword; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; -@SuppressWarnings("rawtypes") +import java.util.EnumSet; +import java.util.Set; +import java.util.concurrent.CancellationException; + public final class InstrumentationPoints { - public static AgentScope onEnter(RedisCommand command) { + private static final Set NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); + + private static final Set AGENT_CRASHING_COMMANDS = + EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); + + public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; + + public static AgentScope beforeCommand(RedisCommand command) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onCommand(span, command); return activateSpan(span, finishSpanEarly(command)); } - public static void onReturn(RedisCommand command, - AgentScope scope, - Throwable throwable, - AsyncCommand asyncCommand) { + public static void afterCommand(RedisCommand command, + AgentScope scope, + Throwable throwable, + AsyncCommand asyncCommand) { AgentSpan span = scope.span(); if (throwable != null) { DECORATE.onError(span, throwable); DECORATE.beforeFinish(span); span.finish(); } else if (!finishSpanEarly(command)) { - asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(span)); + asyncCommand.handleAsync((value, ex) -> { + if (ex instanceof CancellationException) { + span.setTag("db.command.cancelled", true); + } else { + DECORATE.onError(span, ex); + } + DECORATE.beforeFinish(span); + span.finish(); + return null; + }); } scope.close(); } - public static AgentScope onEnter(RedisURI redisURI) { + public static AgentScope beforeConnect(RedisURI redisURI) { AgentSpan span = startSpan("redis.query"); DECORATE.afterStart(span); DECORATE.onConnection(span, redisURI); return activateSpan(span, false); } - public static void onReturn(AgentScope scope, Throwable throwable) { + public static void afterConnect(AgentScope scope, Throwable throwable) { AgentSpan span = scope.span(); if (throwable != null) { DECORATE.onError(span, throwable); @@ -52,4 +80,49 @@ public static void onReturn(AgentScope scope, Throwable throwable) { span.finish(); scope.close(); } + + /** + * Determines whether a redis command should finish its relevant span early (as soon as tags are + * 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 command + * @return true if finish the span early (the command will not have a return value) + */ + public static boolean finishSpanEarly(RedisCommand command) { + ProtocolKeyword keyword = command.getType(); + return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); + } + + private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { + return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); + } + + private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { + return keyword == SEGFAULT; + } + + // Workaround to keep trace agent from crashing + // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and + // traces with these commands as the resource name will not be processed by the trace agent + // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has + // list of commands that will currently fail at the trace agent level. + + /** + * Workaround to keep trace agent from crashing Currently the commands in + * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * resource name will not be processed by the trace agent + * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of + * commands that will currently fail at the trace agent level. + * + * @param keyword the actual redis command + * @return the redis command with a prefix if it is a command that will crash the trace agent, + * otherwise, the original command is returned. + */ + public static String getCommandResourceName(ProtocolKeyword keyword) { + if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { + return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); + } + return keyword.name(); + } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java deleted file mode 100644 index ea60863bee7..00000000000 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncBiFunction.java +++ /dev/null @@ -1,39 +0,0 @@ -package datadog.trace.instrumentation.lettuce; - -import static datadog.trace.instrumentation.lettuce.LettuceClientDecorator.DECORATE; - -import datadog.trace.bootstrap.instrumentation.api.AgentSpan; - -import java.util.concurrent.CancellationException; -import java.util.function.BiFunction; - -/** - * Callback class to close the span on an error or a success in the RedisFuture returned by the - * lettuce async API - * - * @param the normal completion result - * @param the error - * @param the return type, should be null since nothing else should happen from tracing - * standpoint after the span is closed - */ -public class LettuceAsyncBiFunction - implements BiFunction { - - private final AgentSpan span; - - public LettuceAsyncBiFunction(final AgentSpan span) { - this.span = span; - } - - @Override - public R apply(final T t, final Throwable throwable) { - if (throwable instanceof CancellationException) { - span.setTag("db.command.cancelled", true); - } else { - DECORATE.onError(span, throwable); - } - DECORATE.beforeFinish(span); - span.finish(); - return null; - } -} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java index 95725d0fbd3..68002ccf99d 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -1,7 +1,5 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; - import com.lambdaworks.redis.protocol.AsyncCommand; import com.lambdaworks.redis.protocol.RedisCommand; import datadog.trace.bootstrap.instrumentation.api.AgentScope; @@ -10,17 +8,17 @@ public class LettuceAsyncCommandsAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { - return InstrumentationPoints.onEnter(command); + public static AgentScope onEnter(@Advice.Argument(0) final RedisCommand command) { + return InstrumentationPoints.beforeCommand(command); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopSpan( - @Advice.Argument(0) RedisCommand command, - @Advice.Enter AgentScope scope, - @Advice.Thrown Throwable throwable, - @Advice.Return AsyncCommand asyncCommand) { - InstrumentationPoints.onReturn(command, scope, throwable, asyncCommand); + public static void onExit( + @Advice.Argument(0) final RedisCommand command, + @Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable, + @Advice.Return final AsyncCommand asyncCommand) { + InstrumentationPoints.afterCommand(command, scope, throwable, asyncCommand); } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java index 2309405c015..c9084db3d8d 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java @@ -1,6 +1,6 @@ package datadog.trace.instrumentation.lettuce; -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.getCommandResourceName; +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.getCommandResourceName; import com.lambdaworks.redis.RedisURI; import com.lambdaworks.redis.protocol.RedisCommand; @@ -40,37 +40,38 @@ protected String dbType() { } @Override - protected String dbUser(final RedisURI connection) { + protected String dbUser(RedisURI connection) { return null; } @Override - protected String dbInstance(final RedisURI connection) { + protected String dbInstance(RedisURI connection) { return null; } @Override - public AgentSpan onConnection(final AgentSpan span, final RedisURI connection) { + public AgentSpan onConnection(AgentSpan span, RedisURI connection) { if (connection != null) { span.setTag(Tags.PEER_HOSTNAME, connection.getHost()); span.setTag(Tags.PEER_PORT, connection.getPort()); span.setTag("db.redis.dbIndex", connection.getDatabase()); - span.setTag( - DDTags.RESOURCE_NAME, - "CONNECT:" - + connection.getHost() - + ":" - + connection.getPort() - + "/" - + connection.getDatabase()); + span.setTag(DDTags.RESOURCE_NAME, resourceName(connection)); } return super.onConnection(span, connection); } - @SuppressWarnings("rawtypes") - public AgentSpan onCommand(final AgentSpan span, final RedisCommand command) { - span.setTag(DDTags.RESOURCE_NAME, + public AgentSpan onCommand(AgentSpan span, RedisCommand command) { + span.setTag(DDTags.RESOURCE_NAME, null == command ? "Redis Command" : getCommandResourceName(command.getType())); return span; } + + private static String resourceName(RedisURI connection) { + return "CONNECT:" + + connection.getHost() + + ":" + + connection.getPort() + + "/" + + connection.getDatabase(); + } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java deleted file mode 100644 index 4ec0b3bba80..00000000000 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceInstrumentationUtil.java +++ /dev/null @@ -1,74 +0,0 @@ -package datadog.trace.instrumentation.lettuce; - -import static com.lambdaworks.redis.protocol.CommandKeyword.SEGFAULT; -import static com.lambdaworks.redis.protocol.CommandType.CLIENT; -import static com.lambdaworks.redis.protocol.CommandType.CLUSTER; -import static com.lambdaworks.redis.protocol.CommandType.COMMAND; -import static com.lambdaworks.redis.protocol.CommandType.CONFIG; -import static com.lambdaworks.redis.protocol.CommandType.DEBUG; -import static com.lambdaworks.redis.protocol.CommandType.SCRIPT; -import static com.lambdaworks.redis.protocol.CommandType.SHUTDOWN; - -import com.lambdaworks.redis.protocol.CommandKeyword; -import com.lambdaworks.redis.protocol.CommandType; -import com.lambdaworks.redis.protocol.ProtocolKeyword; -import com.lambdaworks.redis.protocol.RedisCommand; - -import java.util.EnumSet; -import java.util.Set; - -public class LettuceInstrumentationUtil { - - public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; - - public static final EnumSet NON_INSTRUMENTING_COMMANDS = EnumSet.of(SHUTDOWN, DEBUG); - - public static final EnumSet NON_INSTRUMENTING_KEYWORDS = EnumSet.of(SEGFAULT); - - public static final Set AGENT_CRASHING_COMMANDS = EnumSet.of(CLIENT, CLUSTER, COMMAND, CONFIG, DEBUG, SCRIPT); - - /** - * Determines whether a redis command should finish its relevant span early (as soon as tags are - * 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 command - * @return true if finish the span early (the command will not have a return value) - */ - public static boolean finishSpanEarly(final RedisCommand command) { - ProtocolKeyword keyword = command.getType(); - return isNonInstrumentingCommand(keyword) || isNonInstrumentingKeyword(keyword); - } - - private static boolean isNonInstrumentingCommand(ProtocolKeyword keyword) { - return keyword instanceof CommandType && NON_INSTRUMENTING_COMMANDS.contains(keyword); - } - - private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { - return keyword instanceof CommandKeyword && NON_INSTRUMENTING_KEYWORDS.contains(keyword); - } - - // Workaround to keep trace agent from crashing - // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and - // traces with these commands as the resource name will not be processed by the trace agent - // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has - // list of commands that will currently fail at the trace agent level. - - /** - * Workaround to keep trace agent from crashing Currently the commands in - * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the - * resource name will not be processed by the trace agent - * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of - * commands that will currently fail at the trace agent level. - * - * @param keyword the actual redis command - * @return the redis command with a prefix if it is a command that will crash the trace agent, - * otherwise, the original command is returned. - */ - public static String getCommandResourceName(ProtocolKeyword keyword) { - if (keyword instanceof CommandType && AGENT_CRASHING_COMMANDS.contains(keyword)) { - return AGENT_CRASHING_COMMAND_PREFIX + keyword.name(); - } - return keyword.name(); - } -} diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java index 58f49eb6ac3..e2cf6bd0123 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java @@ -7,12 +7,13 @@ public class RedisConnectionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope onEnter(@Advice.Argument(1) RedisURI redisURI) { - return InstrumentationPoints.onEnter(redisURI); + public static AgentScope onEnter(@Advice.Argument(1) final RedisURI redisURI) { + return InstrumentationPoints.beforeConnect(redisURI); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void onReturn(@Advice.Enter AgentScope scope, @Advice.Thrown Throwable throwable) { - InstrumentationPoints.onReturn(scope, throwable); + public static void onExit(@Advice.Enter final AgentScope scope, + @Advice.Thrown final Throwable throwable) { + InstrumentationPoints.afterConnect(scope, throwable); } } diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy index e4683ffc25f..d1ce0c72615 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy @@ -25,7 +25,7 @@ import java.util.function.Function import com.lambdaworks.redis.RedisConnectionException -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.AGENT_CRASHING_COMMAND_PREFIX class LettuceAsyncClientTest extends AgentTestRunner { public static final String HOST = "127.0.0.1" diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy index f1cdf0b8813..b05d2eb23e8 100644 --- a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy @@ -10,7 +10,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags import redis.embedded.RedisServer import spock.lang.Shared -import static datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil.AGENT_CRASHING_COMMAND_PREFIX +import static datadog.trace.instrumentation.lettuce.InstrumentationPoints.AGENT_CRASHING_COMMAND_PREFIX class LettuceSyncClientTest extends AgentTestRunner { public static final String HOST = "127.0.0.1" From a8e79c438537dcc7a33146bbd0cb39ddcba369e5 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Mon, 27 Apr 2020 15:45:34 +0100 Subject: [PATCH 7/8] rename module to lettuce-4 --- .../lettuce-4.0.gradle => lettuce-4/lettuce-4.gradle} | 0 .../lettuce/LettuceAsyncCommandsInstrumentation.java | 0 .../instrumentation/lettuce/LettuceClientInstrumentation.java | 0 .../trace/instrumentation/lettuce/InstrumentationPoints.java | 0 .../instrumentation/lettuce/LettuceAsyncCommandsAdvice.java | 0 .../trace/instrumentation/lettuce/LettuceClientDecorator.java | 0 .../trace/instrumentation/lettuce/RedisConnectionAdvice.java | 0 .../src/test/groovy/LettuceAsyncClientTest.groovy | 0 .../src/test/groovy/LettuceSyncClientTest.groovy | 0 settings.gradle | 2 +- 10 files changed, 1 insertion(+), 1 deletion(-) rename dd-java-agent/instrumentation/{lettuce-4.0/lettuce-4.0.gradle => lettuce-4/lettuce-4.gradle} (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/test/groovy/LettuceAsyncClientTest.groovy (100%) rename dd-java-agent/instrumentation/{lettuce-4.0 => lettuce-4}/src/test/groovy/LettuceSyncClientTest.groovy (100%) diff --git a/dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle b/dd-java-agent/instrumentation/lettuce-4/lettuce-4.gradle similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/lettuce-4.0.gradle rename to dd-java-agent/instrumentation/lettuce-4/lettuce-4.gradle diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsInstrumentation.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java/datadog/trace/instrumentation/lettuce/LettuceClientInstrumentation.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceClientDecorator.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java rename to dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/RedisConnectionAdvice.java diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceAsyncClientTest.groovy similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceAsyncClientTest.groovy rename to dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceAsyncClientTest.groovy diff --git a/dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy b/dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceSyncClientTest.groovy similarity index 100% rename from dd-java-agent/instrumentation/lettuce-4.0/src/test/groovy/LettuceSyncClientTest.groovy rename to dd-java-agent/instrumentation/lettuce-4/src/test/groovy/LettuceSyncClientTest.groovy diff --git a/settings.gradle b/settings.gradle index c154297a42d..e3be71f98c4 100644 --- a/settings.gradle +++ b/settings.gradle @@ -107,7 +107,7 @@ include ':dd-java-agent:instrumentation:jms' include ':dd-java-agent:instrumentation:jsp-2.3' include ':dd-java-agent:instrumentation:kafka-clients-0.11' include ':dd-java-agent:instrumentation:kafka-streams-0.11' -include ':dd-java-agent:instrumentation:lettuce-4.0' +include ':dd-java-agent:instrumentation:lettuce-4' include ':dd-java-agent:instrumentation:lettuce-5' include ':dd-java-agent:instrumentation:log4j1' include ':dd-java-agent:instrumentation:log4j2' From 158ba0952017baf3310cacb1d2ae14c632eec22e Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Mon, 27 Apr 2020 15:47:24 +0100 Subject: [PATCH 8/8] remove misleading comments --- .../instrumentation/lettuce/InstrumentationPoints.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java index 2195cee9747..5bf9d5a3ddb 100644 --- a/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java +++ b/dd-java-agent/instrumentation/lettuce-4/src/main/java8/datadog/trace/instrumentation/lettuce/InstrumentationPoints.java @@ -102,18 +102,10 @@ private static boolean isNonInstrumentingKeyword(ProtocolKeyword keyword) { return keyword == SEGFAULT; } - // Workaround to keep trace agent from crashing - // Currently the commands in AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and - // traces with these commands as the resource name will not be processed by the trace agent - // https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has - // list of commands that will currently fail at the trace agent level. - /** * Workaround to keep trace agent from crashing Currently the commands in - * AGENT_CRASHING_COMMANDS_WORDS will crash the trace agent and traces with these commands as the + * AGENT_CRASHING_COMMANDS will crash the trace agent and traces with these commands as the * resource name will not be processed by the trace agent - * https://github.com/DataDog/datadog-trace-agent/blob/master/quantizer/redis.go#L18 has list of - * commands that will currently fail at the trace agent level. * * @param keyword the actual redis command * @return the redis command with a prefix if it is a command that will crash the trace agent,