From 1b00b8f50d526be785c469357e637ef4ac66a082 Mon Sep 17 00:00:00 2001 From: dingjiefei <1264677051@qq.com> Date: Fri, 19 Jan 2024 15:39:25 +0800 Subject: [PATCH 1/2] Resolve Memory leak in redisson instrumentation --- .../redisson/RedissonRequest.java | 29 ++++++------------- .../AbstractRedissonAsyncClientTest.java | 4 +-- .../redisson/AbstractRedissonClientTest.java | 9 +++--- 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java b/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java index 6b34e59e19c5..17289a9a1543 100644 --- a/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java +++ b/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java @@ -5,9 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.redisson; -import static java.util.Collections.emptyList; -import static java.util.Collections.singletonList; - import com.google.auto.value.AutoValue; import io.netty.buffer.ByteBuf; import io.opentelemetry.instrumentation.api.incubator.semconv.db.RedisCommandSanitizer; @@ -20,7 +17,6 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; -import java.util.stream.Collectors; import javax.annotation.Nullable; import org.redisson.client.protocol.CommandData; import org.redisson.client.protocol.CommandsData; @@ -48,6 +44,8 @@ public String getOperation() { CommandsData commandsData = (CommandsData) command; if (commandsData.getCommands().size() == 1) { return commandsData.getCommands().get(0).getCommand().getName(); + } else { + return "BATCH EXECUTE"; } } return null; @@ -55,30 +53,21 @@ public String getOperation() { @Nullable public String getStatement() { - List sanitizedStatements = sanitizeStatement(); - switch (sanitizedStatements.size()) { - case 0: - return null; - // optimize for the most common case - case 1: - return sanitizedStatements.get(0); - default: - return String.join(";", sanitizedStatements); - } + return sanitizeStatement(); } - private List sanitizeStatement() { + private String sanitizeStatement() { Object command = getCommand(); // get command if (command instanceof CommandsData) { List> commands = ((CommandsData) command).getCommands(); - return commands.stream() - .map(RedissonRequest::normalizeSingleCommand) - .collect(Collectors.toList()); + if (commands.size() == 1) { + return normalizeSingleCommand(commands.get(0)); + } } else if (command instanceof CommandData) { - return singletonList(normalizeSingleCommand((CommandData) command)); + return normalizeSingleCommand((CommandData) command); } - return emptyList(); + return null; } private static String normalizeSingleCommand(CommandData command) { diff --git a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index 40a3955f591a..da1c0886bea8 100644 --- a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -213,14 +213,14 @@ void atomicBatchCommand() throws ExecutionException, InterruptedException, Timeo trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasKind(INTERNAL).hasNoParent(), span -> - span.hasName("DB Query") + span.hasName("BATCH EXECUTE") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(SemanticAttributes.NETWORK_TYPE, "ipv4"), equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"), equalTo(NetworkAttributes.NETWORK_PEER_PORT, (long) port), equalTo(SemanticAttributes.DB_SYSTEM, "redis"), - equalTo(SemanticAttributes.DB_STATEMENT, "MULTI;SET batch1 ?")) + equalTo(SemanticAttributes.DB_OPERATION, "BATCH EXECUTE")) .hasParent(trace.getSpan(0)), span -> span.hasName("SET") diff --git a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 43c81ecd7226..35a1d290de29 100644 --- a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -147,15 +147,14 @@ void batchCommand() trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("DB Query") + span.hasName("BATCH EXECUTE") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(SemanticAttributes.NETWORK_TYPE, "ipv4"), equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"), equalTo(NetworkAttributes.NETWORK_PEER_PORT, (long) port), equalTo(SemanticAttributes.DB_SYSTEM, "redis"), - equalTo( - SemanticAttributes.DB_STATEMENT, "SET batch1 ?;SET batch2 ?")))); + equalTo(SemanticAttributes.DB_OPERATION, "BATCH EXECUTE")))); } private static void invokeExecute(RBatch batch) @@ -188,14 +187,14 @@ void atomicBatchCommand() { trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasNoParent().hasKind(INTERNAL), span -> - span.hasName("DB Query") + span.hasName("BATCH EXECUTE") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(SemanticAttributes.NETWORK_TYPE, "ipv4"), equalTo(NetworkAttributes.NETWORK_PEER_ADDRESS, "127.0.0.1"), equalTo(NetworkAttributes.NETWORK_PEER_PORT, (long) port), equalTo(SemanticAttributes.DB_SYSTEM, "redis"), - equalTo(SemanticAttributes.DB_STATEMENT, "MULTI;SET batch1 ?")) + equalTo(SemanticAttributes.DB_OPERATION, "BATCH EXECUTE")) .hasParent(trace.getSpan(0)), span -> span.hasName("SET") From 4e2d6d61296e657d5e4ae10065f1797eb1f7a829 Mon Sep 17 00:00:00 2001 From: dingjiefei <1264677051@qq.com> Date: Tue, 29 Jul 2025 23:28:54 +0800 Subject: [PATCH 2/2] draft --- .../redisson/RedissonRequest.java | 29 ++++++++++++++----- .../AbstractRedissonAsyncClientTest.java | 4 +-- .../redisson/AbstractRedissonClientTest.java | 8 ++--- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java b/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java index caf092044681..084bd9e026e6 100644 --- a/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java +++ b/instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java @@ -5,6 +5,9 @@ package io.opentelemetry.javaagent.instrumentation.redisson; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + import com.google.auto.value.AutoValue; import io.netty.buffer.ByteBuf; import io.opentelemetry.instrumentation.api.incubator.semconv.db.RedisCommandSanitizer; @@ -17,6 +20,7 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.redisson.client.protocol.CommandData; import org.redisson.client.protocol.CommandsData; @@ -46,7 +50,7 @@ public String getOperation() { if (commandsData.getCommands().size() == 1) { return commandsData.getCommands().get(0).getCommand().getName(); } else { - return "BATCH EXECUTE"; + return "BATCH"; } } return null; @@ -54,21 +58,30 @@ public String getOperation() { @Nullable public String getStatement() { - return sanitizeStatement(); + List sanitizedStatements = sanitizeStatement(); + switch (sanitizedStatements.size()) { + case 0: + return null; + // optimize for the most common case + case 1: + return sanitizedStatements.get(0); + default: + return String.join(";", sanitizedStatements); + } } - private String sanitizeStatement() { + private List sanitizeStatement() { Object command = getCommand(); // get command if (command instanceof CommandsData) { List> commands = ((CommandsData) command).getCommands(); - if (commands.size() == 1) { - return normalizeSingleCommand(commands.get(0)); - } + return commands.stream() + .map(RedissonRequest::normalizeSingleCommand) + .collect(Collectors.toList()); } else if (command instanceof CommandData) { - return normalizeSingleCommand((CommandData) command); + return singletonList(normalizeSingleCommand((CommandData) command)); } - return null; + return emptyList(); } private static String normalizeSingleCommand(CommandData command) { diff --git a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index b49c7ac560ed..9803d4f09bd8 100644 --- a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -225,14 +225,14 @@ void atomicBatchCommand() throws ExecutionException, InterruptedException, Timeo trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasKind(INTERNAL).hasNoParent(), span -> - span.hasName("BATCH EXECUTE") + span.hasName("BATCH") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(NETWORK_TYPE, "ipv4"), equalTo(NETWORK_PEER_ADDRESS, ip), equalTo(NETWORK_PEER_PORT, (long) port), equalTo(maybeStable(DB_SYSTEM), "redis"), - equalTo(maybeStable(DB_STATEMENT), "BATCH EXECUTE")) + equalTo(maybeStable(DB_STATEMENT), "MULTI;SET batch1 ?")) .hasParent(trace.getSpan(0)), span -> span.hasName("SET") diff --git a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 035973eeba3a..5fdae881b47d 100644 --- a/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -166,14 +166,14 @@ void batchCommand() trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("BATCH EXECUTE") + span.hasName("BATCH") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(NETWORK_TYPE, "ipv4"), equalTo(NETWORK_PEER_ADDRESS, ip), equalTo(NETWORK_PEER_PORT, (long) port), equalTo(maybeStable(DB_SYSTEM), "redis"), - equalTo(maybeStable(DB_STATEMENT), "BATCH EXECUTE")))); + equalTo(maybeStable(DB_STATEMENT), "SET batch1 ?;SET batch2 ?")))); } private static void invokeExecute(RBatch batch) @@ -206,14 +206,14 @@ void atomicBatchCommand() { trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasNoParent().hasKind(INTERNAL), span -> - span.hasName("BATCH EXECUTE") + span.hasName("BATCH") .hasKind(CLIENT) .hasAttributesSatisfyingExactly( equalTo(NETWORK_TYPE, "ipv4"), equalTo(NETWORK_PEER_ADDRESS, ip), equalTo(NETWORK_PEER_PORT, (long) port), equalTo(maybeStable(DB_SYSTEM), "redis"), - equalTo(maybeStable(DB_STATEMENT), "BATCH EXECUTE")) + equalTo(maybeStable(DB_STATEMENT), "MULTI;SET batch1 ?")) .hasParent(trace.getSpan(0)), span -> span.hasName("SET")