From fc7d8dcb5eda80fa25fbbad21c871d12cf939230 Mon Sep 17 00:00:00 2001 From: Shengjun <1492705772@qq.com> Date: Fri, 2 Apr 2021 10:03:57 +0800 Subject: [PATCH] Redis Lettuce span UI doesn't show detailed Redis command parameters in 'db.statement' field (#6614) --- CHANGES.md | 1 + .../lettuce/v5/LettucePluginConfig.java | 41 +++++++++++++++++++ .../v5/RedisChannelWriterInterceptor.java | 32 +++++++++++++-- .../v5/RedisChannelWriterInterceptorTest.java | 26 +++++++----- .../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(-) create mode 100644 apm-sniffer/apm-sdk-plugin/lettuce-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/LettucePluginConfig.java diff --git a/CHANGES.md b/CHANGES.md index f7dedb545..154104f77 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 000000000..57cd760a8 --- /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. + *

+ * 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 0037c563e..bb2ef1291 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 redisCommands = (Collection) 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 36132331c..27e8c8caa 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 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 fd4481d02..f3ffa01b7 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 32d2ba464..895ef7496 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 94c287765..dac050fd7 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} skipAnalysis: 'false' - operationName: Lettuce/BATCH_WRITE operationId: 0