diff --git a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/build.gradle b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/build.gradle index 4527f4ac710..4f638792feb 100644 --- a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/build.gradle +++ b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/build.gradle @@ -1,5 +1,6 @@ plugins { id 'dd-trace-java.module.instrumentation' + id 'dd-trace-java.jmh-conventions' } muzzle { @@ -20,6 +21,7 @@ addTestSuiteForDir('lettuce62Test', 'test') dependencies { compileOnly group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' + jmh group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' testImplementation group: 'com.redis.testcontainers', name: 'testcontainers-redis', version: '1.6.2' testImplementation libs.testcontainers diff --git a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/jmh/java/datadog/trace/instrumentation/lettuce5/LettuceCommandMatchingBenchmark.java b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/jmh/java/datadog/trace/instrumentation/lettuce5/LettuceCommandMatchingBenchmark.java new file mode 100644 index 00000000000..cb25f25e216 --- /dev/null +++ b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/jmh/java/datadog/trace/instrumentation/lettuce5/LettuceCommandMatchingBenchmark.java @@ -0,0 +1,186 @@ +package datadog.trace.instrumentation.lettuce5; + +import io.lettuce.core.output.CommandOutput; +import io.lettuce.core.protocol.CommandArgs; +import io.lettuce.core.protocol.CommandType; +import io.lettuce.core.protocol.ProtocolKeyword; +import io.lettuce.core.protocol.RedisCommand; +import io.netty.buffer.ByteBuf; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Compares the old {@code HashSet} lookup in {@link + * LettuceInstrumentationUtil#expectsResponse(RedisCommand)} against the new {@code + * EnumSet} lookup. The win is lookup strategy (bit-set test vs. string hash/equals), + * not avoided allocation -- these {@code CommandType} names have no whitespace, so {@code + * toString().trim()} never allocates. {@code oldExpectsResponse} reproduces the removed + * implementation for comparison. + * + *

Split into two traffic shapes: {@code Miss} (ordinary data commands, ~100% of real traffic) + * and {@code Hit} (DEBUG/SHUTDOWN, rare but exercises the matching path). + * + * 

+ *   ./gradlew :dd-java-agent:instrumentation:lettuce:lettuce-5.0:jmh   # add -prof gc
+ *
+ */ +@Fork(3) +@Warmup(iterations = 2, time = 5) +@Measurement(iterations = 5, time = 5) +@Threads(1) +public class LettuceCommandMatchingBenchmark { + + /** Byte-for-byte reproduction of the {@code Set}-based check this replaces. */ + private static final String[] NON_INSTRUMENTING_COMMAND_WORDS = + new String[] {"SHUTDOWN", "DEBUG", "OOM", "SEGFAULT"}; + + private static final Set NON_INSTRUMENTING_COMMANDS_OLD = + new HashSet<>(Arrays.asList(NON_INSTRUMENTING_COMMAND_WORDS)); + + private static boolean oldExpectsResponse(final RedisCommand command) { + String commandName = "Redis Command"; + if (command != null && command.getType() != null) { + commandName = command.getType().toString().trim(); + } + return !NON_INSTRUMENTING_COMMANDS_OLD.contains(commandName); + } + + /** Minimal {@link RedisCommand} stub -- only {@link #getType()} is ever exercised here. */ + private static final class FakeRedisCommand implements RedisCommand { + private final ProtocolKeyword type; + + FakeRedisCommand(final ProtocolKeyword type) { + this.type = type; + } + + @Override + public ProtocolKeyword getType() { + return type; + } + + @Override + public CommandOutput getOutput() { + throw new UnsupportedOperationException(); + } + + @Override + public void complete() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean completeExceptionally(final Throwable throwable) { + throw new UnsupportedOperationException(); + } + + @Override + public void cancel() { + throw new UnsupportedOperationException(); + } + + @Override + public CommandArgs getArgs() { + throw new UnsupportedOperationException(); + } + + @Override + public void encode(final ByteBuf buf) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isCancelled() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isDone() { + throw new UnsupportedOperationException(); + } + + @Override + public void setOutput(final CommandOutput output) { + throw new UnsupportedOperationException(); + } + } + + // Representative production traffic: ordinary data commands, none of which ever match + // NON_INSTRUMENTING_COMMANDS. + private static final RedisCommand[] MISS_COMMANDS = + Arrays.stream( + new CommandType[] { + CommandType.GET, + CommandType.SET, + CommandType.EXISTS, + CommandType.EXPIRE, + CommandType.HSET, + CommandType.LPUSH, + CommandType.INCR, + }) + .map(FakeRedisCommand::new) + .toArray(RedisCommand[]::new); + + // Rare admin commands that always match NON_INSTRUMENTING_COMMANDS. Not representative of real + // traffic volume -- included only to exercise the hit path. + private static final RedisCommand[] HIT_COMMANDS = + Arrays.stream(new CommandType[] {CommandType.DEBUG, CommandType.SHUTDOWN}) + .map(FakeRedisCommand::new) + .toArray(RedisCommand[]::new); + + private abstract static class Cursor { + int index = 0; + + abstract RedisCommand[] commands(); + + RedisCommand next() { + final RedisCommand[] commands = commands(); + final int i = index; + index = (i + 1) % commands.length; + return commands[i]; + } + } + + @State(Scope.Thread) + public static class MissCursor extends Cursor { + @Override + RedisCommand[] commands() { + return MISS_COMMANDS; + } + } + + @State(Scope.Thread) + public static class HitCursor extends Cursor { + @Override + RedisCommand[] commands() { + return HIT_COMMANDS; + } + } + + @Benchmark + public boolean missOld(final MissCursor cursor) { + return oldExpectsResponse(cursor.next()); + } + + @Benchmark + public boolean missNew(final MissCursor cursor) { + return LettuceInstrumentationUtil.expectsResponse(cursor.next()); + } + + @Benchmark + public boolean hitOld(final HitCursor cursor) { + return oldExpectsResponse(cursor.next()); + } + + @Benchmark + public boolean hitNew(final HitCursor cursor) { + return LettuceInstrumentationUtil.expectsResponse(cursor.next()); + } +} diff --git a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceClientDecorator.java b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceClientDecorator.java index e6a03dbc242..2107b74faa5 100644 --- a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceClientDecorator.java +++ b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceClientDecorator.java @@ -67,8 +67,7 @@ public void onConnection(final AgentSpan span, final RedisURI connection) { } public void onCommand(final AgentSpan span, final RedisCommand command) { - final String commandName = LettuceInstrumentationUtil.getCommandName(command); - span.setResourceName(LettuceInstrumentationUtil.getCommandResourceName(commandName)); + span.setResourceName(LettuceInstrumentationUtil.getCommandResourceName(command)); } public String resourceNameForConnection(final RedisURI redisURI) { diff --git a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceInstrumentationUtil.java b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceInstrumentationUtil.java index ec4cc28fa78..92d89d97ecc 100644 --- a/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceInstrumentationUtil.java +++ b/dd-java-agent/instrumentation/lettuce/lettuce-5.0/src/main/java/datadog/trace/instrumentation/lettuce5/LettuceInstrumentationUtil.java @@ -1,25 +1,44 @@ package datadog.trace.instrumentation.lettuce5; +import io.lettuce.core.protocol.CommandType; +import io.lettuce.core.protocol.ProtocolKeyword; import io.lettuce.core.protocol.RedisCommand; -import java.util.Arrays; +import java.util.EnumSet; import java.util.HashSet; import java.util.Set; public class LettuceInstrumentationUtil { - public static final String[] NON_INSTRUMENTING_COMMAND_WORDS = - new String[] {"SHUTDOWN", "DEBUG", "OOM", "SEGFAULT"}; + // DEBUG covers `DEBUG OOM`/`DEBUG SEGFAULT`: Lettuce sends those as command type DEBUG with + // "OOM"/SEGFAULT as an argument, not as the command type. + public static final Set NON_INSTRUMENTING_COMMANDS = + EnumSet.of(CommandType.SHUTDOWN, CommandType.DEBUG); - public static final String[] AGENT_CRASHING_COMMANDS_WORDS = - new String[] {"CLIENT", "CLUSTER", "COMMAND", "CONFIG", "DEBUG", "SCRIPT"}; + public static final Set AGENT_CRASHING_COMMANDS = + EnumSet.of( + CommandType.CLIENT, + CommandType.CLUSTER, + CommandType.COMMAND, + CommandType.CONFIG, + CommandType.DEBUG, + CommandType.SCRIPT); public static final String AGENT_CRASHING_COMMAND_PREFIX = "COMMAND-NAME:"; - public static final Set nonInstrumentingCommands = - new HashSet<>(Arrays.asList(NON_INSTRUMENTING_COMMAND_WORDS)); + // Fallback for custom (non-CommandType) ProtocolKeyword implementations. + private static final Set NON_INSTRUMENTING_COMMAND_NAMES = + commandNames(NON_INSTRUMENTING_COMMANDS); - public static final Set agentCrashingCommands = - new HashSet<>(Arrays.asList(AGENT_CRASHING_COMMANDS_WORDS)); + private static final Set AGENT_CRASHING_COMMAND_NAMES = + commandNames(AGENT_CRASHING_COMMANDS); + + private static Set commandNames(final Set commands) { + final Set names = new HashSet<>(); + for (final CommandType command : commands) { + names.add(command.toString()); + } + return names; + } /** * Determines whether a redis command should finish its relevant span early (as soon as tags are @@ -30,32 +49,41 @@ public class LettuceInstrumentationUtil { * @return false if the span should finish early (the command will not have a return value) */ public static boolean expectsResponse(final RedisCommand command) { - final String commandName = LettuceInstrumentationUtil.getCommandName(command); - return !nonInstrumentingCommands.contains(commandName); + if (command == null) { + return true; + } + final ProtocolKeyword type = command.getType(); + if (type == null) { + return true; + } + if (type instanceof CommandType) { + return !NON_INSTRUMENTING_COMMANDS.contains(type); + } + return !NON_INSTRUMENTING_COMMAND_NAMES.contains(type.toString().trim()); } - // 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 + * Workaround to keep trace agent from crashing Currently the commands in 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 actualCommandName the actual redis command + * @param command the lettuce RedisCommand object * @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; + public static String getCommandResourceName(final RedisCommand command) { + final String commandName = getCommandName(command); + final ProtocolKeyword type = command == null ? null : command.getType(); + final boolean crashesAgent = + type instanceof CommandType + ? AGENT_CRASHING_COMMANDS.contains(type) + : type != null && AGENT_CRASHING_COMMAND_NAMES.contains(commandName); + if (crashesAgent) { + return AGENT_CRASHING_COMMAND_PREFIX + commandName; } - return actualCommandName; + return commandName; } /**