diff --git a/CHANGES.md b/CHANGES.md index c7b7e6353c..bd8dd18ef4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,7 @@ Release Notes. * Put `Agent-Version` property reading in the premain stage to avoid deadlock when using `jarsigner`. * Add a config `agent.enable`(default: true) to support disabling the agent through system property `-Dskywalking.agent.disable=false` or system environment variable setting `SW_AGENT_ENABLE=false`. +* Enhance redisson plugin to adopt uniform tags. #### Documentation diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java index 2cce4ba9e3..df6988c5eb 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java @@ -130,6 +130,11 @@ public static final class HTTP { */ public static final StringTag CACHE_KEY = new StringTag(18, "cache.key"); + /** + * CACHE_INSTANCE records the cache instance + */ + public static final StringTag CACHE_INSTANCE = new StringTag(20, "cache.instance"); + public static final String VAL_LOCAL_SPAN_AS_LOGIC_ENDPOINT = "{\"logic-span\":true}"; public static final StringTag SQL_PARAMETERS = new StringTag(19, "db.sql.parameters"); 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 5a62afe325..60956c7a64 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 @@ -18,7 +18,6 @@ package org.apache.skywalking.apm.plugin.redisson.v3; -import io.netty.buffer.ByteBuf; import io.netty.channel.Channel; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; @@ -32,6 +31,7 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; import org.apache.skywalking.apm.plugin.redisson.v3.util.ClassUtil; +import org.apache.skywalking.apm.util.StringUtil; import org.redisson.client.RedisClient; import org.redisson.client.RedisConnection; import org.redisson.client.protocol.CommandData; @@ -39,6 +39,7 @@ import java.lang.reflect.Method; import java.net.InetSocketAddress; +import java.util.Optional; public class RedisConnectionMethodInterceptor implements InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor { @@ -58,43 +59,29 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr InetSocketAddress remoteAddress = (InetSocketAddress) channel.remoteAddress(); String dbInstance = remoteAddress.getAddress().getHostAddress() + ":" + remoteAddress.getPort(); - StringBuilder dbStatement = new StringBuilder(); String operationName = "Redisson/"; + String command = ""; + Object[] arguments = new Object[0]; if (allArguments[0] instanceof CommandsData) { operationName = operationName + "BATCH_EXECUTE"; - CommandsData commands = (CommandsData) allArguments[0]; - for (CommandData commandData : commands.getCommands()) { - addCommandData(dbStatement, commandData); - dbStatement.append(";"); - } + command = "BATCH_EXECUTE"; } else if (allArguments[0] instanceof CommandData) { CommandData commandData = (CommandData) allArguments[0]; - String command = commandData.getCommand().getName(); + command = commandData.getCommand().getName(); operationName = operationName + command; - addCommandData(dbStatement, commandData); + arguments = commandData.getParams(); } AbstractSpan span = ContextManager.createExitSpan(operationName, peer); span.setComponent(ComponentsDefine.REDISSON); - Tags.DB_TYPE.set(span, "Redis"); - Tags.DB_INSTANCE.set(span, dbInstance); - Tags.DB_STATEMENT.set(span, dbStatement.toString()); - SpanLayer.asCache(span); - } + Tags.CACHE_TYPE.set(span, "Redis"); + Tags.CACHE_INSTANCE.set(span, dbInstance); + Tags.CACHE_CMD.set(span, command); - private void addCommandData(StringBuilder dbStatement, CommandData commandData) { - dbStatement.append(commandData.getCommand().getName()); - if (RedissonPluginConfig.Plugin.Redisson.TRACE_REDIS_PARAMETERS && commandData.getParams() != null) { - for (Object param : commandData.getParams()) { - dbStatement.append(DELIMITER_SPACE); - String paramStr = param instanceof ByteBuf ? QUESTION_MARK : String.valueOf(param.toString()); - if (paramStr.length() > RedissonPluginConfig.Plugin.Redisson.REDIS_PARAMETER_MAX_LENGTH) { - paramStr = paramStr.substring(0, RedissonPluginConfig.Plugin.Redisson.REDIS_PARAMETER_MAX_LENGTH) + ABBR; - } - dbStatement.append(paramStr); - } - } + getKey(arguments).ifPresent(key -> Tags.CACHE_KEY.set(span, key)); + parseOperation(command.toLowerCase()).ifPresent(op -> Tags.CACHE_OP.set(span, op)); + SpanLayer.asCache(span); } @Override @@ -131,4 +118,29 @@ public void onConstruct(EnhancedInstance objInst, Object[] allArguments) { } objInst.setSkyWalkingDynamicField(peer); } + + private Optional getKey(Object[] allArguments) { + if (!RedissonPluginConfig.Plugin.Redisson.TRACE_REDIS_PARAMETERS) { + return Optional.empty(); + } + if (allArguments.length == 0) { + return Optional.empty(); + } + Object argument = allArguments[0]; + // include null + if (!(argument instanceof String)) { + return Optional.empty(); + } + return Optional.of(StringUtil.cut((String) argument, RedissonPluginConfig.Plugin.Redisson.REDIS_PARAMETER_MAX_LENGTH)); + } + + private Optional parseOperation(String cmd) { + if (RedissonPluginConfig.Plugin.Redisson.OPERATION_MAPPING_READ.contains(cmd)) { + return Optional.of("read"); + } + if (RedissonPluginConfig.Plugin.Redisson.OPERATION_MAPPING_WRITE.contains(cmd)) { + return Optional.of("write"); + } + return Optional.empty(); + } } 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 f97c4a715b..52dedcb10a 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 @@ -20,6 +20,10 @@ import org.apache.skywalking.apm.agent.core.boot.PluginConfig; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + public class RedissonPluginConfig { public static class Plugin { @PluginConfig(root = RedissonPluginConfig.class) @@ -36,6 +40,118 @@ 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; + + /** + * Operation represent a cache span is "write" or "read" action , and "op"(operation) is tagged with key "cache.op" usually + * This config term define which command should be converted to write Operation . + * + * @see org.apache.skywalking.apm.agent.core.context.tag.Tags#CACHE_OP + * @see RedisConnectionMethodInterceptor#parseOperation(String) + */ + public static Set OPERATION_MAPPING_WRITE = new HashSet<>(Arrays.asList( + "getset", + "set", + "setbit", + "setex ", + "setnx ", + "setrange", + "strlen ", + "mset", + "msetnx ", + "psetex", + "incr ", + "incrby ", + "incrbyfloat", + "decr ", + "decrby ", + "append ", + "hmset", + "hset", + "hsetnx ", + "hincrby", + "hincrbyfloat", + "hdel", + "rpoplpush", + "rpush", + "rpushx", + "lpush", + "lpushx", + "lrem", + "ltrim", + "lset", + "brpoplpush", + "linsert", + "sadd", + "sdiff", + "sdiffstore", + "sinterstore", + "sismember", + "srem", + "sunion", + "sunionstore", + "sinter", + "zadd", + "zincrby", + "zinterstore", + "zrange", + "zrangebylex", + "zrangebyscore", + "zrank", + "zrem", + "zremrangebylex", + "zremrangebyrank", + "zremrangebyscore", + "zrevrange", + "zrevrangebyscore", + "zrevrank", + "zunionstore", + "xadd", + "xdel", + "del", + "xtrim" + )); + /** + * Operation represent a cache span is "write" or "read" action , and "op"(operation) is tagged with key "cache.op" usually + * This config term define which command should be converted to write Operation . + * + * @see org.apache.skywalking.apm.agent.core.context.tag.Tags#CACHE_OP + * @see RedisConnectionMethodInterceptor#parseOperation(String) + */ + public static Set OPERATION_MAPPING_READ = new HashSet<>(Arrays.asList( + "getrange", + "getbit ", + "mget", + "hvals", + "hkeys", + "hlen", + "hexists", + "hget", + "hgetall", + "hmget", + "blpop", + "brpop", + "lindex", + "llen", + "lpop", + "lrange", + "rpop", + "scard", + "srandmember", + "spop", + "sscan", + "smove", + "zlexcount", + "zscore", + "zscan", + "zcard", + "zcount", + "xget", + "get", + "xread", + "xlen", + "xrange", + "xrevrange" + )); } } } diff --git a/apm-sniffer/config/agent.config b/apm-sniffer/config/agent.config index 7124e2e8a0..2e7bc49160 100755 --- a/apm-sniffer/config/agent.config +++ b/apm-sniffer/config/agent.config @@ -305,3 +305,7 @@ plugin.jedis.operation_mapping_read=${SW_PLUGIN_JEDIS_OPERATION_MAPPING_READ:get plugin.redisson.trace_redis_parameters=${SW_PLUGIN_REDISSON_TRACE_REDIS_PARAMETERS:false} # If set to positive number and plugin.redisson.trace_redis_parameters is set to true, Redis command parameters would be collected and truncated to this length. plugin.redisson.redis_parameter_max_length=${SW_PLUGIN_REDISSON_REDIS_PARAMETER_MAX_LENGTH:128} +# Specify which command should be converted to write operation +plugin.redisson.operation_mapping_write=${SW_PLUGIN_REDISSON_OPERATION_MAPPING_WRITE:getset,set,setbit,setex,setnx,setrange,strlen,mset,msetnx,psetex,incr,incrby,incrbyfloat,decr,decrby,append,hmset,hset,hsetnx,hincrby,hincrbyfloat,hdel,rpoplpush,rpush,rpushx,lpush,lpushx,lrem,ltrim,lset,brpoplpush,linsert,sadd,sdiff,sdiffstore,sinterstore,sismember,srem,sunion,sunionstore,sinter,zadd,zincrby,zinterstore,zrange,zrangebylex,zrangebyscore,zrank,zrem,zremrangebylex,zremrangebyrank,zremrangebyscore,zrevrange,zrevrangebyscore,zrevrank,zunionstore,xadd,xdel,del,xtrim} +# Specify which command should be converted to read operation +plugin.redisson.operation_mapping_read=${SW_PLUGIN_REDISSON_OPERATION_MAPPING_READ:getrange,getbit,mget,hvals,hkeys,hlen,hexists,hget,hgetall,hmget,blpop,brpop,lindex,llen,lpop,lrange,rpop,scard,srandmember,spop,sscan,smove,zlexcount,zscore,zscan,zcard,zcount,xget,get,xread,xlen,xrange,xrevrange} \ No newline at end of file diff --git a/docs/en/setup/service-agent/java-agent/configurations.md b/docs/en/setup/service-agent/java-agent/configurations.md index c79c525d40..9669e9398f 100644 --- a/docs/en/setup/service-agent/java-agent/configurations.md +++ b/docs/en/setup/service-agent/java-agent/configurations.md @@ -112,6 +112,8 @@ This is the properties list supported in `agent/config/agent.config`. | `plugin.jedis.operation_mapping_read ` | Specify which command should be converted to `read` operation | SW_PLUGIN_JEDIS_OPERATION_MAPPING_READ | Referenc [Jedis-4.x-plugin](https://github.com/apache/skywalking-java/blob/main/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/JedisPluginConfig.java) [jedis-2.x-3.x-plugin](https://github.com/apache/skywalking-java/blob/main/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-2.x-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v3/JedisPluginConfig.java) | | `plugin.redisson.trace_redis_parameters` | If set to true, the parameters of Redis commands would be collected by Redisson agent. | SW_PLUGIN_REDISSON_TRACE_REDIS_PARAMETERS | `false` | | `plugin.redisson.redis_parameter_max_length` | If set to positive number and `plugin.redisson.trace_redis_parameters` is set to `true`, Redis command parameters would be collected and truncated to this length. | SW_PLUGIN_REDISSON_REDIS_PARAMETER_MAX_LENGTH | `128` | +| `plugin.redisson.operation_mapping_write` | Specify which command should be converted to `write` operation | SW_PLUGIN_REDISSON_OPERATION_MAPPING_WRITE | | +| `plugin.redisson.operation_mapping_read ` | Specify which command should be converted to `read` operation | SW_PLUGIN_REDISSON_OPERATION_MAPPING_READ | Referenc [Redisson-3.x-plugin](https://github.com/apache/skywalking-java/blob/main/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedissonPluginConfig.java) | | `plugin.neo4j.trace_cypher_parameters` | If set to true, the parameters of the cypher would be collected. | SW_PLUGIN_NEO4J_TRACE_CYPHER_PARAMETERS | `false` | | `plugin.neo4j.cypher_parameters_max_length` | If set to positive number, the `db.cypher.parameters` would be truncated to this length, otherwise it would be completely saved, which may cause performance problem. | SW_PLUGIN_NEO4J_CYPHER_PARAMETERS_MAX_LENGTH | `512` | | `plugin.neo4j.cypher_body_max_length` | If set to positive number, the `db.statement` would be truncated to this length, otherwise it would be completely saved, which may cause performance problem. | SW_PLUGIN_NEO4J_CYPHER_BODY_MAX_LENGTH | `2048` | diff --git a/test/plugin/scenarios/redisson-scenario/config/expectedData.yaml b/test/plugin/scenarios/redisson-scenario/config/expectedData.yaml index 73b06f1c2e..2b54ed3b81 100644 --- a/test/plugin/scenarios/redisson-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/redisson-scenario/config/expectedData.yaml @@ -30,9 +30,11 @@ segmentItems: spanType: Exit peer: not null tags: - - {key: db.type, value: Redis} - - {key: db.instance, value: not null} - - {key: db.statement, value: 'SET key_a ?'} + - {key: cache.type, value: Redis} + - {key: cache.instance, value: not null} + - {key: cache.cmd, value: SET} + - {key: cache.key, value: key_a} + - {key: cache.op, value: write} skipAnalysis: 'false' - operationName: Redisson/BATCH_EXECUTE parentSpanId: 0 @@ -45,10 +47,9 @@ segmentItems: spanType: Exit peer: not null tags: - - {key: db.type, value: Redis} - - {key: db.instance, value: not null} - - {key: db.statement, value: 'SET batch_k_a ?;SET batch_k_b ?;PEXPIRE batch_k_b - 20000;'} + - {key: cache.type, value: Redis} + - {key: cache.instance, value: not null} + - {key: cache.cmd, value: BATCH_EXECUTE} skipAnalysis: 'false' - operationName: GET:/case/redisson-case parentSpanId: -1