Skip to content

Commit

Permalink
Fix bugs in Redisson plugin (#693)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CzyerChen
CzyerChen authored May 22, 2024
1 parent b608d74 commit ffbd90c
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 14 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
43 changes: 33 additions & 10 deletions .../main/java/org/apache/skywalking/apm/plugin/redisson/v3/ConnectionManagerInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 17 additions & 2 deletions ...n/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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((CommandsData) 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.SHOW_PING_COMMAND) {
return;
} else {
operationName = operationName + command;
arguments = commandData.getParams();
}
}

AbstractSpan span = ContextManager.createExitSpan(operationName, peer);
Expand Down Expand Up @@ -143,4 +151,11 @@ private Optional<String> parseOperation(String cmd) {
}
return Optional.empty();
}

private String showBatchCommands(CommandsData commandsData) {
return commandsData.getCommands()
.stream()
.map(data -> data.getCommand().getName())
.collect(Collectors.joining(";"));
}
}
8 changes: 8 additions & 0 deletions ...ugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
Expand Down
2 changes: 1 addition & 1 deletion docs/en/setup/service-agent/java-agent/Supported-list.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/plugin/scenarios/redisson-scenario/support-version.list
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ffbd90c

Please sign in to comment.