diff --git a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java index 1ba7156c2e..95157a45fa 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java +++ b/dd-java-agent/instrumentation/lettuce-5/src/main/java8/datadog/trace/instrumentation/lettuce/LettuceAsyncCommandsAdvice.java @@ -8,21 +8,18 @@ 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; public class LettuceAsyncCommandsAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static Scope startSpan(@Advice.Argument(0) final RedisCommand command) { - Map commandMap = LettuceInstrumentationUtil.getCommandInfo(command); - String commandName = commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_NAME); - String commandArgs = commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_ARGS); + String commandName = LettuceInstrumentationUtil.getCommandName(command); final Scope scope = GlobalTracer.get() .buildSpan(LettuceInstrumentationUtil.SERVICE_NAME + ".query") - .startActive(LettuceInstrumentationUtil.doFinishSpanEarly(commandMap)); + .startActive(LettuceInstrumentationUtil.doFinishSpanEarly(commandName)); final Span span = scope.span(); Tags.DB_TYPE.set(span, LettuceInstrumentationUtil.SERVICE_NAME); @@ -30,7 +27,6 @@ public class LettuceAsyncCommandsAdvice { Tags.COMPONENT.set(span, LettuceInstrumentationUtil.COMPONENT_NAME); span.setTag(DDTags.RESOURCE_NAME, commandName); - span.setTag("db.command.args", commandArgs); span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); span.setTag(DDTags.SPAN_TYPE, LettuceInstrumentationUtil.SERVICE_NAME); @@ -43,8 +39,8 @@ public class LettuceAsyncCommandsAdvice { @Advice.Thrown final Throwable throwable, @Advice.Return final AsyncCommand asyncCommand) { + 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)); span.finish(); @@ -53,7 +49,9 @@ public class LettuceAsyncCommandsAdvice { } // close spans on error or normal completion - asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(scope.span())); + if (!LettuceInstrumentationUtil.doFinishSpanEarly(span.getBaggageItem(DDTags.RESOURCE_NAME))) { + asyncCommand.handleAsync(new LettuceAsyncBiFunction<>(scope.span())); + } scope.close(); } } 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 5e89c36d96..4406c9ea82 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,31 +7,21 @@ public class LettuceInstrumentationUtil { public static final String SERVICE_NAME = "redis"; public static final String COMPONENT_NAME = SERVICE_NAME + "-client"; - public static final String MAP_KEY_CMD_NAME = "CMD_NAME"; - public static final String MAP_KEY_CMD_ARGS = "CMD_ARGS"; public static final String[] NON_INSTRUMENTING_COMMAND_WORDS = new String[] {"SHUTDOWN", "DEBUG", "OOM", "SEGFAULT"}; public static final Set nonInstrumentingCommands = new HashSet<>(Arrays.asList(NON_INSTRUMENTING_COMMAND_WORDS)); - public static boolean doFinishSpanEarly(Map commandMap) { - String cmdName = commandMap.get(MAP_KEY_CMD_NAME); - String cmdArgs = commandMap.get(MAP_KEY_CMD_ARGS); - - if (cmdName.equals("SHUTDOWN") - || (nonInstrumentingCommands.contains(cmdName) - && nonInstrumentingCommands.contains(cmdArgs))) { - return true; - } - return false; + public static boolean doFinishSpanEarly(String commandName) { + return nonInstrumentingCommands.contains(commandName); } - public static Map getCommandInfo(RedisCommand command) { + public static String getCommandName(RedisCommand command) { String commandName = "Redis Command"; - String commandArgs = null; - Map commandMap = new HashMap<>(); if (command != null) { + /* + // Disable command argument capturing for now to avoid leak of sensitive data // get the arguments passed into the redis command if (command.getArgs() != null) { // standardize to null instead of using empty string @@ -40,17 +30,18 @@ public class LettuceInstrumentationUtil { commandArgs = null; } } + */ // get the redis command name (i.e. GET, SET, HMSET, etc) if (command.getType() != null) { commandName = command.getType().name(); + /* // if it is an AUTH command, then remove the extracted command arguments since it is the password if ("AUTH".equals(commandName)) { commandArgs = null; } + */ } } - commandMap.put(MAP_KEY_CMD_NAME, commandName); - commandMap.put(MAP_KEY_CMD_ARGS, commandArgs); - return commandMap; + return 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 46409e0349..ad4015b3ef 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 @@ -2,7 +2,6 @@ package datadog.trace.instrumentation.lettuce.rx; import datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil; import io.lettuce.core.protocol.RedisCommand; -import java.util.Map; import java.util.function.Supplier; import net.bytebuddy.asm.Advice; import reactor.core.publisher.Flux; @@ -10,19 +9,19 @@ import reactor.core.publisher.Flux; public class LettuceFluxCreationAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Map extractCommand( + public static String extractCommandName( @Advice.Argument(0) final Supplier supplier) { - return LettuceInstrumentationUtil.getCommandInfo(supplier.get()); + return LettuceInstrumentationUtil.getCommandName(supplier.get()); } - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + // if there is an exception thrown, then don't make spans + @Advice.OnMethodExit(suppress = Throwable.class) public static void monitorSpan( - @Advice.Enter final Map commandMap, - @Advice.Return(readOnly = false) Flux publisher) { + @Advice.Enter final String commandName, @Advice.Return(readOnly = false) Flux publisher) { - boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandMap); + boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandName); LettuceFluxTerminationRunnable handler = - new LettuceFluxTerminationRunnable(commandMap, finishSpanOnClose); + new LettuceFluxTerminationRunnable(commandName, 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 // Mono (In here a flux is created first and then converted to Mono) 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 871d542fa8..f7c8884f84 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 @@ -7,7 +7,6 @@ import io.opentracing.Span; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.util.Collections; -import java.util.Map; import java.util.function.Consumer; import org.reactivestreams.Subscription; import org.slf4j.LoggerFactory; @@ -21,8 +20,8 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl private int numResults = 0; private FluxOnSubscribeConsumer onSubscribeConsumer = null; - public LettuceFluxTerminationRunnable(Map commandMap, boolean finishSpanOnClose) { - this.onSubscribeConsumer = new FluxOnSubscribeConsumer(this, commandMap, finishSpanOnClose); + public LettuceFluxTerminationRunnable(String commandName, boolean finishSpanOnClose) { + this.onSubscribeConsumer = new FluxOnSubscribeConsumer(this, commandName, finishSpanOnClose); } public FluxOnSubscribeConsumer getOnSubscribeConsumer() { @@ -73,15 +72,13 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl public static class FluxOnSubscribeConsumer implements Consumer { private final LettuceFluxTerminationRunnable owner; - private final Map commandMap; + private final String commandName; private final boolean finishSpanOnClose; public FluxOnSubscribeConsumer( - LettuceFluxTerminationRunnable owner, - Map commandMap, - boolean finishSpanOnClose) { + LettuceFluxTerminationRunnable owner, String commandName, boolean finishSpanOnClose) { this.owner = owner; - this.commandMap = commandMap; + this.commandName = commandName; this.finishSpanOnClose = finishSpanOnClose; } @@ -98,10 +95,7 @@ public class LettuceFluxTerminationRunnable implements Consumer, Runnabl Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); Tags.COMPONENT.set(span, LettuceInstrumentationUtil.COMPONENT_NAME); - span.setTag( - DDTags.RESOURCE_NAME, this.commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_NAME)); - span.setTag( - "db.command.args", this.commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_ARGS)); + span.setTag(DDTags.RESOURCE_NAME, commandName); span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); span.setTag(DDTags.SPAN_TYPE, LettuceInstrumentationUtil.SERVICE_NAME); scope.close(); 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 3190d6e92b..8e9828e0a3 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 @@ -2,7 +2,6 @@ package datadog.trace.instrumentation.lettuce.rx; import datadog.trace.instrumentation.lettuce.LettuceInstrumentationUtil; import io.lettuce.core.protocol.RedisCommand; -import java.util.Map; import java.util.function.Supplier; import net.bytebuddy.asm.Advice; import reactor.core.publisher.Mono; @@ -10,20 +9,19 @@ import reactor.core.publisher.Mono; public class LettuceMonoCreationAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static Map extractCommand( + public static String extractCommandName( @Advice.Argument(0) final Supplier supplier) { - return LettuceInstrumentationUtil.getCommandInfo(supplier.get()); + return LettuceInstrumentationUtil.getCommandName(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 Map commandMap, - @Advice.Return(readOnly = false) Mono publisher) { + @Advice.Enter final String commandName, @Advice.Return(readOnly = false) Mono publisher) { - boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandMap); - LettuceMonoDualConsumer mdc = new LettuceMonoDualConsumer(commandMap, finishSpanOnClose); + boolean finishSpanOnClose = LettuceInstrumentationUtil.doFinishSpanEarly(commandName); + LettuceMonoDualConsumer mdc = new LettuceMonoDualConsumer(commandName, 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 5e92156761..0f25eb4533 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 @@ -7,7 +7,6 @@ import io.opentracing.Span; import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.util.Collections; -import java.util.Map; import java.util.function.BiConsumer; import java.util.function.Consumer; import org.slf4j.LoggerFactory; @@ -17,11 +16,11 @@ public class LettuceMonoDualConsumer implements Consumer, BiConsumer { private Span span = null; - private final Map commandMap; + private final String commandName; private final boolean finishSpanOnClose; - public LettuceMonoDualConsumer(Map commandMap, boolean finishSpanOnClose) { - this.commandMap = commandMap; + public LettuceMonoDualConsumer(String commandName, boolean finishSpanOnClose) { + this.commandName = commandName; this.finishSpanOnClose = finishSpanOnClose; } @@ -53,10 +52,7 @@ public class LettuceMonoDualConsumer Tags.SPAN_KIND.set(this.span, Tags.SPAN_KIND_CLIENT); Tags.COMPONENT.set(this.span, LettuceInstrumentationUtil.COMPONENT_NAME); - this.span.setTag( - DDTags.RESOURCE_NAME, this.commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_NAME)); - this.span.setTag( - "db.command.args", this.commandMap.get(LettuceInstrumentationUtil.MAP_KEY_CMD_ARGS)); + this.span.setTag(DDTags.RESOURCE_NAME, this.commandName); this.span.setTag(DDTags.SERVICE_NAME, LettuceInstrumentationUtil.SERVICE_NAME); this.span.setTag(DDTags.SPAN_TYPE, LettuceInstrumentationUtil.SERVICE_NAME); 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 829e6ab4cb..b946943b30 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 @@ -167,7 +167,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key value" "span.kind" "client" "span.type" "redis" } @@ -207,7 +206,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -261,7 +259,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -359,7 +356,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key key value key value key value<53>" "span.kind" "client" "span.type" "redis" } @@ -377,7 +373,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -425,7 +420,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key key" errorTags(IllegalStateException, "TestException") "span.kind" "client" "span.type" "redis" @@ -470,7 +464,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key value<1> value<2>" "db.command.cancelled" true "span.kind" "client" "span.type" "redis" @@ -501,7 +494,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "SEGFAULT" "span.kind" "client" "span.type" "redis" } @@ -534,7 +526,6 @@ class LettuceAsyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "NOSAVE" "span.kind" "client" "span.type" "redis" } diff --git a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy index 213f9d57f8..77ee5c52f3 100644 --- a/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy +++ b/dd-java-agent/instrumentation/lettuce-5/src/test/groovy/LettuceReactiveClientTest.groovy @@ -80,7 +80,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key value" "span.kind" "client" "span.type" "redis" } @@ -111,7 +110,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -149,7 +147,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -278,7 +275,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "SEGFAULT" "span.kind" "client" "span.type" "redis" } @@ -310,7 +306,6 @@ class LettuceReactiveClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "NOSAVE" "span.kind" "client" "span.type" "redis" } 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 69ed46ce7e..32e56129e5 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 @@ -146,7 +146,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key value" "span.kind" "client" "span.type" "redis" } @@ -174,7 +173,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -202,7 +200,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -257,7 +254,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key value" "span.kind" "client" "span.type" "redis" } @@ -285,7 +281,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key key value key value key value<53>" "span.kind" "client" "span.type" "redis" } @@ -313,7 +308,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "key" "span.kind" "client" "span.type" "redis" } @@ -340,7 +334,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "SEGFAULT" "span.kind" "client" "span.type" "redis" } @@ -372,7 +365,6 @@ class LettuceSyncClientTest extends AgentTestRunner { defaultTags() "component" "redis-client" "db.type" "redis" - "db.command.args" "NOSAVE" "span.kind" "client" "span.type" "redis" }