Fix bugs in Redisson plugin (#693)

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)
 

apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml

@@ -31,7 +31,7 @@
     <name>redisson-3.x-plugin</name>
     <url>http://maven.apache.org</url>
     <properties>
-        <redisson.version>3.6.0</redisson.version>
+        <redisson.version>3.20.0</redisson.version>
     </properties>
 
     <dependencies>

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.InstanceM
 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 class ConnectionManagerInterceptor implements InstanceMethodsAroundInterc
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                               Object ret) throws Throwable {
         try {
-            ConnectionManager connectionManager = (ConnectionManager) objInst;
+            Config config = getConfig(objInst);
-            Config config = connectionManager.getCfg();
+            Object singleServerConfig = null;
-
+            Object sentinelServersConfig = null;
-            Object singleServerConfig = ClassUtil.getObjectField(config, "singleServerConfig");
+            Object masterSlaveServersConfig = null;
-            Object sentinelServersConfig = ClassUtil.getObjectField(config, "sentinelServersConfig");
+            Object clusterServersConfig = null;
-            Object masterSlaveServersConfig = ClassUtil.getObjectField(config, "masterSlaveServersConfig");
+            Object replicatedServersConfig = null;
-            Object clusterServersConfig = ClassUtil.getObjectField(config, "clusterServersConfig");
+            if (Objects.nonNull(config)) {
-            Object replicatedServersConfig = ClassUtil.getObjectField(config, "replicatedServersConfig");
+                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 class ConnectionManagerInterceptor implements InstanceMethodsAroundInterc
             }
             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 @@ public class ConnectionManagerInterceptor implements InstanceMethodsAroundInterc
         }
     }
 
+    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) {

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 class RedisConnectionMethodInterceptor implements InstanceMethodsAroundIn
         if (allArguments[0] instanceof CommandsData) {
             operationName = operationName + "BATCH_EXECUTE";
             command = "BATCH_EXECUTE";
+            if (RedissonPluginConfig.Plugin.Redisson.SHOW_BATCH_COMMANDS) {
+                command += ":" + showBatchCommands((CommandsData) allArguments[0]);
+            }
         } else if (allArguments[0] instanceof CommandData) {
             CommandData commandData = (CommandData) allArguments[0];
             command = commandData.getCommand().getName();
-            operationName = operationName + command;
+            if ("PING".equals(command) && !RedissonPluginConfig.Plugin.Redisson.SHOW_PING_COMMAND) {
-            arguments = commandData.getParams();
+                return;
+            } else {
+                operationName = operationName + command;
+                arguments = commandData.getParams();
+            }
         }
 
         AbstractSpan span = ContextManager.createExitSpan(operationName, peer);
@@ -143,4 +151,11 @@ public class RedisConnectionMethodInterceptor implements InstanceMethodsAroundIn
         }
         return Optional.empty();
     }
+
+    private String showBatchCommands(CommandsData commandsData) {
+        return commandsData.getCommands()
+                           .stream()
+                           .map(data -> data.getCommand().getName())
+                           .collect(Collectors.joining(";"));
+    }
 }

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 class RedissonPluginConfig {
              * 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 true, the PING command would be collected.
+             */
+            public static boolean SHOW_PING_COMMAND = false;
+            /**
+             * 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

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

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