From cb9766ca85910f70a8f169bd9b902bf4e90119ae Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 22 May 2024 08:26:51 +0800 Subject: [PATCH 1/3] enhance redisson plugin. --- CHANGES.md | 3 ++ .../redisson-3.x-plugin/pom.xml | 2 +- .../v3/ConnectionManagerInterceptor.java | 43 ++++++++++++++----- .../v3/RedisConnectionMethodInterceptor.java | 20 ++++++++- .../redisson/v3/RedissonPluginConfig.java | 8 ++++ .../java-agent/Supported-list.md | 2 +- .../redisson-scenario/bin/startup.sh | 2 +- .../redisson-scenario/support-version.list | 7 +++ 8 files changed, 72 insertions(+), 15 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 66f717e538..b6b6724e40 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,9 @@ Release Notes. * Add support for `Derby`/`Sybase`/`SQLite`/`DB2`/`OceanBase` jdbc url format in `URLParser`. * Optimize spring-plugins:scheduled-annotation-plugin compatibility about Spring 6.1.x support. * Add a forceIgnoring mechanism in a CROSS_THREAD scenario. +* Fix NPE in Redisson plugin since Redisson 3.20.0. +* Support for showing batch command details and ignoring PING commands in Redisson plugin. +* Fix peer value of Master-Slave mode in Redisson plugin. All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/213?closed=1) diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml index 2e0d14219d..19f79cbc3e 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml @@ -31,7 +31,7 @@ redisson-3.x-plugin http://maven.apache.org - 3.6.0 + 3.20.0 diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/ConnectionManagerInterceptor.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/ConnectionManagerInterceptor.java index d4b352deea..8cdb79b9ff 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/ConnectionManagerInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/ConnectionManagerInterceptor.java @@ -18,6 +18,7 @@ package org.apache.skywalking.apm.plugin.redisson.v3; +import java.util.Objects; import org.apache.skywalking.apm.agent.core.context.util.PeerFormat; import org.apache.skywalking.apm.agent.core.logging.api.ILog; import org.apache.skywalking.apm.agent.core.logging.api.LogManager; @@ -26,11 +27,12 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; import org.apache.skywalking.apm.plugin.redisson.v3.util.ClassUtil; import org.redisson.config.Config; -import org.redisson.connection.ConnectionManager; import java.lang.reflect.Method; import java.net.URI; import java.util.Collection; +import org.redisson.connection.MasterSlaveConnectionManager; +import org.redisson.connection.ServiceManager; public class ConnectionManagerInterceptor implements InstanceMethodsAroundInterceptor { @@ -45,14 +47,19 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { try { - ConnectionManager connectionManager = (ConnectionManager) objInst; - Config config = connectionManager.getCfg(); - - Object singleServerConfig = ClassUtil.getObjectField(config, "singleServerConfig"); - Object sentinelServersConfig = ClassUtil.getObjectField(config, "sentinelServersConfig"); - Object masterSlaveServersConfig = ClassUtil.getObjectField(config, "masterSlaveServersConfig"); - Object clusterServersConfig = ClassUtil.getObjectField(config, "clusterServersConfig"); - Object replicatedServersConfig = ClassUtil.getObjectField(config, "replicatedServersConfig"); + Config config = getConfig(objInst); + Object singleServerConfig = null; + Object sentinelServersConfig = null; + Object masterSlaveServersConfig = null; + Object clusterServersConfig = null; + Object replicatedServersConfig = null; + if (Objects.nonNull(config)) { + singleServerConfig = ClassUtil.getObjectField(config, "singleServerConfig"); + sentinelServersConfig = ClassUtil.getObjectField(config, "sentinelServersConfig"); + masterSlaveServersConfig = ClassUtil.getObjectField(config, "masterSlaveServersConfig"); + clusterServersConfig = ClassUtil.getObjectField(config, "clusterServersConfig"); + replicatedServersConfig = ClassUtil.getObjectField(config, "replicatedServersConfig"); + } StringBuilder peer = new StringBuilder(); EnhancedInstance retInst = (EnhancedInstance) ret; @@ -70,7 +77,7 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA } if (masterSlaveServersConfig != null) { Object masterAddress = ClassUtil.getObjectField(masterSlaveServersConfig, "masterAddress"); - peer.append(getPeer(masterAddress)); + peer.append(getPeer(masterAddress)).append(";"); appendAddresses(peer, (Collection) ClassUtil.getObjectField(masterSlaveServersConfig, "slaveAddresses")); retInst.setSkyWalkingDynamicField(PeerFormat.shorten(peer.toString())); return ret; @@ -118,6 +125,22 @@ static String getPeer(Object obj) { } } + private Config getConfig(EnhancedInstance objInst) { + Config config = null; + MasterSlaveConnectionManager connectionManager = (MasterSlaveConnectionManager) objInst; + try { + config = (Config) ClassUtil.getObjectField(connectionManager, "cfg"); + } catch (NoSuchFieldException | IllegalAccessException ignore) { + try { + ServiceManager serviceManager = (ServiceManager) ClassUtil.getObjectField( + connectionManager, "serviceManager"); + config = serviceManager.getCfg(); + } catch (NoSuchFieldException | IllegalAccessException ignore2) { + } + } + return config; + } + @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Throwable t) { diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java index 60956c7a64..4a2c62a8c7 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java @@ -19,6 +19,7 @@ package org.apache.skywalking.apm.plugin.redisson.v3; import io.netty.channel.Channel; +import java.util.stream.Collectors; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; @@ -66,11 +67,18 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr if (allArguments[0] instanceof CommandsData) { operationName = operationName + "BATCH_EXECUTE"; command = "BATCH_EXECUTE"; + if (RedissonPluginConfig.Plugin.Redisson.SHOW_BATCH_COMMANDS) { + command += ":" + showBatchCommands(allArguments[0]); + } } else if (allArguments[0] instanceof CommandData) { CommandData commandData = (CommandData) allArguments[0]; command = commandData.getCommand().getName(); - operationName = operationName + command; - arguments = commandData.getParams(); + if ("PING".equals(command) && RedissonPluginConfig.Plugin.Redisson.PING_IGNORED) { + return; + } else { + operationName = operationName + command; + arguments = commandData.getParams(); + } } AbstractSpan span = ContextManager.createExitSpan(operationName, peer); @@ -143,4 +151,12 @@ private Optional parseOperation(String cmd) { } return Optional.empty(); } + + private String showBatchCommands(Object argument) { + CommandsData commandsData = (CommandsData) argument; + return commandsData.getCommands() + .stream() + .map(data -> data.getCommand().getName()) + .collect(Collectors.joining(";")); + } } diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java index 52dedcb10a..9feafe3703 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java @@ -40,6 +40,14 @@ public static class Redisson { * Set a negative number to save specified length of parameter string to the tag. */ public static int REDIS_PARAMETER_MAX_LENGTH = 128; + /** + * If set to false, the PING command would be collected. + */ + public static boolean PING_IGNORED = true; + /** + * If set to true, the detail of the Redis batch commands would be collected. + */ + public static boolean SHOW_BATCH_COMMANDS = false; /** * Operation represent a cache span is "write" or "read" action , and "op"(operation) is tagged with key "cache.op" usually diff --git a/docs/en/setup/service-agent/java-agent/Supported-list.md b/docs/en/setup/service-agent/java-agent/Supported-list.md index 1fa2672282..12b712bbf4 100644 --- a/docs/en/setup/service-agent/java-agent/Supported-list.md +++ b/docs/en/setup/service-agent/java-agent/Supported-list.md @@ -87,7 +87,7 @@ metrics based on the tracing data. * [aerospike](https://github.com/aerospike/aerospike-client-java) 3.x -> 6.x * Redis * [Jedis](https://github.com/xetorthio/jedis) 2.x-4.x - * [Redisson](https://github.com/redisson/redisson) Easy Java Redis client 3.5.2+ + * [Redisson](https://github.com/redisson/redisson) Easy Java Redis client 3.5.0 -> 3.30.0 * [Lettuce](https://github.com/lettuce-io/lettuce-core) 5.x * [MongoDB Java Driver](https://github.com/mongodb/mongo-java-driver) 2.13-2.14, 3.4.0-3.12.7, 4.0.0-4.1.0 * Memcached Client diff --git a/test/plugin/scenarios/redisson-scenario/bin/startup.sh b/test/plugin/scenarios/redisson-scenario/bin/startup.sh index 28a6efa87a..95e650d4b6 100644 --- a/test/plugin/scenarios/redisson-scenario/bin/startup.sh +++ b/test/plugin/scenarios/redisson-scenario/bin/startup.sh @@ -18,4 +18,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -Dredis.servers=${REDIS_SERVERS} -Dskywalking.plugin.redisson.trace_redis_parameters=true -jar ${agent_opts} ${home}/../libs/redisson-scenario.jar & \ No newline at end of file +java -Dredis.servers=${REDIS_SERVERS} -Dskywalking.plugin.redisson.trace_redis_parameters=true -Dskywalking.plugin.redisson.show_batch_commands=true -Dskywalking.plugin.redisson.ping_ignored=true -jar ${agent_opts} ${home}/../libs/redisson-scenario.jar & \ No newline at end of file diff --git a/test/plugin/scenarios/redisson-scenario/support-version.list b/test/plugin/scenarios/redisson-scenario/support-version.list index 78009656c1..aa0a5594df 100644 --- a/test/plugin/scenarios/redisson-scenario/support-version.list +++ b/test/plugin/scenarios/redisson-scenario/support-version.list @@ -14,6 +14,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +# 3.5.0-3.12.4, 3.26.1-3.30.0 have been tested, and 3.12.5-3.26.0 are also supported but not included in the test +3.30.0 +3.29.0 +3.28.0 +3.27.2 +3.26.1 +3.12.4 3.11.5 3.10.7 3.9.1 From ad7e320b195a6e46f74478ec0ed2c0f82f35ef39 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 22 May 2024 09:35:44 +0800 Subject: [PATCH 2/3] update startup.sh. --- test/plugin/scenarios/redisson-scenario/bin/startup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugin/scenarios/redisson-scenario/bin/startup.sh b/test/plugin/scenarios/redisson-scenario/bin/startup.sh index 95e650d4b6..28a6efa87a 100644 --- a/test/plugin/scenarios/redisson-scenario/bin/startup.sh +++ b/test/plugin/scenarios/redisson-scenario/bin/startup.sh @@ -18,4 +18,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -Dredis.servers=${REDIS_SERVERS} -Dskywalking.plugin.redisson.trace_redis_parameters=true -Dskywalking.plugin.redisson.show_batch_commands=true -Dskywalking.plugin.redisson.ping_ignored=true -jar ${agent_opts} ${home}/../libs/redisson-scenario.jar & \ No newline at end of file +java -Dredis.servers=${REDIS_SERVERS} -Dskywalking.plugin.redisson.trace_redis_parameters=true -jar ${agent_opts} ${home}/../libs/redisson-scenario.jar & \ No newline at end of file From 06368cb1c088a5c02721fe35dfee13fe3b60b4f0 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 22 May 2024 13:01:16 +0800 Subject: [PATCH 3/3] change ping_ignore to show_ping_command & update showBatchCommands() --- .../redisson/v3/RedisConnectionMethodInterceptor.java | 7 +++---- .../apm/plugin/redisson/v3/RedissonPluginConfig.java | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java index 4a2c62a8c7..f660ca1a15 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java @@ -68,12 +68,12 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr operationName = operationName + "BATCH_EXECUTE"; command = "BATCH_EXECUTE"; if (RedissonPluginConfig.Plugin.Redisson.SHOW_BATCH_COMMANDS) { - command += ":" + showBatchCommands(allArguments[0]); + command += ":" + showBatchCommands((CommandsData) allArguments[0]); } } else if (allArguments[0] instanceof CommandData) { CommandData commandData = (CommandData) allArguments[0]; command = commandData.getCommand().getName(); - if ("PING".equals(command) && RedissonPluginConfig.Plugin.Redisson.PING_IGNORED) { + if ("PING".equals(command) && !RedissonPluginConfig.Plugin.Redisson.SHOW_PING_COMMAND) { return; } else { operationName = operationName + command; @@ -152,8 +152,7 @@ private Optional parseOperation(String cmd) { return Optional.empty(); } - private String showBatchCommands(Object argument) { - CommandsData commandsData = (CommandsData) argument; + private String showBatchCommands(CommandsData commandsData) { return commandsData.getCommands() .stream() .map(data -> data.getCommand().getName()) diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java index 9feafe3703..aeb0a5f270 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java @@ -41,9 +41,9 @@ public static class Redisson { */ public static int REDIS_PARAMETER_MAX_LENGTH = 128; /** - * If set to false, the PING command would be collected. + * If set to true, the PING command would be collected. */ - public static boolean PING_IGNORED = true; + public static boolean SHOW_PING_COMMAND = false; /** * If set to true, the detail of the Redis batch commands would be collected. */