You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2021/04/02 02:04:26 UTC
[skywalking] branch master updated: Redis Lettuce span UI doesn't
show detailed Redis command parameters in 'db.statement' field (#6614)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new fc7d8dc Redis Lettuce span UI doesn't show detailed Redis command parameters in 'db.statement' field (#6614)
fc7d8dc is described below
commit fc7d8dcb5eda80fa25fbbad21c871d12cf939230
Author: Shengjun <14...@qq.com>
AuthorDate: Fri Apr 2 10:03:57 2021 +0800
Redis Lettuce span UI doesn't show detailed Redis command parameters in 'db.statement' field (#6614)
---
CHANGES.md | 1 +
.../apm/plugin/lettuce/v5/LettucePluginConfig.java | 41 ++++++++++++++++++++++
.../lettuce/v5/RedisChannelWriterInterceptor.java | 32 +++++++++++++++--
.../v5/RedisChannelWriterInterceptorTest.java | 26 ++++++++------
docs/en/setup/service-agent/java-agent/README.md | 2 ++
.../scenarios/lettuce-scenario/bin/startup.sh | 2 +-
.../lettuce-scenario/config/expectedData.yaml | 2 +-
7 files changed, 91 insertions(+), 15 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index f7dedb5..154104f 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -27,6 +27,7 @@ Release Notes.
* Replace hbase-1.x-plugin with hbase-1.x-2.x-plugin to adapt hbase client 2.x
* Remove the close_before_method and close_after_method parameters of custom-enhance-plugin to avoid memory leaks.
* Fix bug that springmvn-annotation-4.x-plugin, witness class does not exist in some versions.
+* Add Redis command parameters to 'db.statement' field on Lettuce span UI for displaying more info
* Fix NullPointerException with `ReactiveRequestHolder.getHeaders`.
#### OAP-Backend
diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/LettucePluginConfig.java b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/LettucePluginConfig.java
new file mode 100644
index 0000000..57cd760
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/LettucePluginConfig.java
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.lettuce.v5;
+
+import org.apache.skywalking.apm.agent.core.boot.PluginConfig;
+
+public class LettucePluginConfig {
+ public static class Plugin {
+ @PluginConfig(root = LettucePluginConfig.class)
+ public static class Lettuce {
+ /**
+ * If set to true, the parameters of the Redis command would be collected.
+ */
+ public static boolean TRACE_REDIS_PARAMETERS = false;
+ /**
+ * For the sake of performance, SkyWalking won't save Redis parameter string into the tag.
+ * If TRACE_REDIS_PARAMETERS is set to true, the first {@code REDIS_PARAMETER_MAX_LENGTH} parameter
+ * characters would be collected.
+ * <p>
+ * Set a negative number to save specified length of parameter string to the tag.
+ */
+ public static int REDIS_PARAMETER_MAX_LENGTH = 128;
+ }
+ }
+}
diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptor.java
index 0037c56..bb2ef12 100644
--- a/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptor.java
@@ -18,7 +18,10 @@
package org.apache.skywalking.apm.plugin.lettuce.v5;
-import io.lettuce.core.protocol.RedisCommand;
+import java.lang.reflect.Method;
+import java.util.Collection;
+
+import org.apache.skywalking.apm.agent.core.conf.Constants;
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;
@@ -28,12 +31,18 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceC
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
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.util.StringUtil;
-import java.lang.reflect.Method;
-import java.util.Collection;
+import io.lettuce.core.protocol.CommandArgs;
+import io.lettuce.core.protocol.RedisCommand;
public class RedisChannelWriterInterceptor implements InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor {
+ private static final String PASSWORD_MASK = "******";
+ private static final String ABBR = "...";
+ private static final String DELIMITER_SPACE = " ";
+ private static final String AUTH = "AUTH";
+
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
MethodInterceptResult result) throws Throwable {
@@ -47,6 +56,9 @@ public class RedisChannelWriterInterceptor implements InstanceMethodsAroundInter
String command = redisCommand.getType().name();
operationName = operationName + command;
dbStatement.append(command);
+ if (LettucePluginConfig.Plugin.Lettuce.TRACE_REDIS_PARAMETERS) {
+ dbStatement.append(DELIMITER_SPACE).append(getArgsStatement(redisCommand));
+ }
} else if (allArguments[0] instanceof Collection) {
@SuppressWarnings("unchecked") Collection<RedisCommand> redisCommands = (Collection<RedisCommand>) allArguments[0];
operationName = operationName + "BATCH_WRITE";
@@ -61,6 +73,20 @@ public class RedisChannelWriterInterceptor implements InstanceMethodsAroundInter
Tags.DB_STATEMENT.set(span, dbStatement.toString());
SpanLayer.asCache(span);
}
+
+ private String getArgsStatement(RedisCommand redisCommand) {
+ String statement;
+ if (AUTH.equalsIgnoreCase(redisCommand.getType().name())) {
+ statement = PASSWORD_MASK;
+ } else {
+ CommandArgs args = redisCommand.getArgs();
+ statement = (args != null) ? args.toCommandString() : Constants.EMPTY_STRING;
+ }
+ if (StringUtil.isNotEmpty(statement) && statement.length() > LettucePluginConfig.Plugin.Lettuce.REDIS_PARAMETER_MAX_LENGTH) {
+ statement = statement.substring(0, LettucePluginConfig.Plugin.Lettuce.REDIS_PARAMETER_MAX_LENGTH) + ABBR;
+ }
+ return statement;
+ }
@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptorTest.java
index 3613233..27e8c8c 100644
--- a/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptorTest.java
+++ b/apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/lettuce/v5/RedisChannelWriterInterceptorTest.java
@@ -18,10 +18,12 @@
package org.apache.skywalking.apm.plugin.lettuce.v5;
-import io.lettuce.core.RedisURI;
-import io.lettuce.core.protocol.Command;
-import io.lettuce.core.protocol.CommandType;
-import io.lettuce.core.protocol.RedisCommand;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import java.util.ArrayList;
+import java.util.List;
+
import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan;
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment;
@@ -46,11 +48,12 @@ import org.mockito.Mock;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.modules.junit4.PowerMockRunnerDelegate;
-import java.util.ArrayList;
-import java.util.List;
-
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
+import io.lettuce.core.RedisURI;
+import io.lettuce.core.codec.ByteArrayCodec;
+import io.lettuce.core.protocol.Command;
+import io.lettuce.core.protocol.CommandArgs;
+import io.lettuce.core.protocol.CommandType;
+import io.lettuce.core.protocol.RedisCommand;
@RunWith(PowerMockRunner.class)
@PowerMockRunnerDelegate(TracingSegmentRunner.class)
@@ -89,6 +92,7 @@ public class RedisChannelWriterInterceptorTest {
})
@Before
public void setUp() throws Exception {
+ LettucePluginConfig.Plugin.Lettuce.TRACE_REDIS_PARAMETERS = true;
mockRedisChannelWriterInstance = new MockInstance();
mockClientOptionsInstance = new MockInstance();
mockClientOptionsInstance.setSkyWalkingDynamicField("127.0.0.1:6379;127.0.0.1:6378;");
@@ -98,7 +102,8 @@ public class RedisChannelWriterInterceptorTest {
@Test
public void testInterceptor() throws Throwable {
interceptor.onConstruct(mockRedisChannelWriterInstance, new Object[] {mockClientOptionsInstance});
- RedisCommand redisCommand = new Command(CommandType.SET, null);
+ CommandArgs args = new CommandArgs(new ByteArrayCodec()).addKey("name".getBytes()).addValue("Tom".getBytes());
+ RedisCommand redisCommand = new Command(CommandType.SET, null, args);
interceptor.beforeMethod(mockRedisChannelWriterInstance, null, new Object[] {redisCommand}, null, null);
interceptor.afterMethod(mockRedisChannelWriterInstance, null, null, null, null);
MatcherAssert.assertThat((String) mockRedisChannelWriterInstance.getSkyWalkingDynamicField(), Is.is("127.0.0.1:6379;127.0.0.1:6378;"));
@@ -110,6 +115,7 @@ public class RedisChannelWriterInterceptorTest {
assertThat(SpanHelper.getComponentId(spans.get(0)), is(57));
List<TagValuePair> tags = SpanHelper.getTags(spans.get(0));
assertThat(tags.get(0).getValue(), is("Redis"));
+ assertThat(tags.get(1).getValue(), CoreMatchers.containsString("Tom"));
assertThat(SpanHelper.getLayer(spans.get(0)), CoreMatchers.is(SpanLayer.CACHE));
}
diff --git a/docs/en/setup/service-agent/java-agent/README.md b/docs/en/setup/service-agent/java-agent/README.md
index fd4481d..f3ffa01 100755
--- a/docs/en/setup/service-agent/java-agent/README.md
+++ b/docs/en/setup/service-agent/java-agent/README.md
@@ -166,6 +166,8 @@ property key | Description | Default |
`plugin.toolkit.log.grpc.reporter.server_port` | Specify which grpc server's port for log data to report to. | `11800` |
`plugin.toolkit.log.grpc.reporter.max_message_size` | Specify the maximum size of log data for grpc client to report to. | `10485760` |
`plugin.toolkit.log.grpc.reporter.upstream_timeout` | How long grpc client will timeout in sending data to upstream. Unit is second.|`30` seconds|
+`plugin.lettuce.trace_redis_parameters` | If set to true, the parameters of Redis commands would be collected by Lettuce agent.| `false` |
+`plugin.lettuce.redis_parameter_max_length` | If set to positive number and `plugin.lettuce.trace_redis_parameters` is set to `true`, Redis command parameters would be collected and truncated to this length.| `128` |
## Dynamic Configurations
All configurations above are static, if you need to change some agent settings at runtime, please read [CDS - Configuration Discovery Service document](configuration-discovery.md) for more details.
diff --git a/test/plugin/scenarios/lettuce-scenario/bin/startup.sh b/test/plugin/scenarios/lettuce-scenario/bin/startup.sh
index 32d2ba4..895ef74 100644
--- a/test/plugin/scenarios/lettuce-scenario/bin/startup.sh
+++ b/test/plugin/scenarios/lettuce-scenario/bin/startup.sh
@@ -18,4 +18,4 @@
home="$(cd "$(dirname $0)"; pwd)"
-java -Dredis.host=${REDIS_SERVERS} -jar ${agent_opts} ${home}/../libs/lettuce-scenario.jar &
\ No newline at end of file
+java -Dredis.host=${REDIS_SERVERS} -jar -Dskywalking.plugin.lettuce.trace_redis_parameters=true ${agent_opts} ${home}/../libs/lettuce-scenario.jar &
\ No newline at end of file
diff --git a/test/plugin/scenarios/lettuce-scenario/config/expectedData.yaml b/test/plugin/scenarios/lettuce-scenario/config/expectedData.yaml
index 94c2877..dac050f 100644
--- a/test/plugin/scenarios/lettuce-scenario/config/expectedData.yaml
+++ b/test/plugin/scenarios/lettuce-scenario/config/expectedData.yaml
@@ -32,7 +32,7 @@ segmentItems:
peer: not null
tags:
- {key: db.type, value: Redis}
- - {key: db.statement, value: GET}
+ - {key: db.statement, value: GET key<key>}
skipAnalysis: 'false'
- operationName: Lettuce/BATCH_WRITE
operationId: 0