From 5dd44bba1fc6c9d505c72fc23b0a5c4e6250ca4d Mon Sep 17 00:00:00 2001 From: mijiatian Date: Sun, 23 Jun 2024 23:40:23 +0800 Subject: [PATCH 1/5] fix redisson plugin's problem of exitSpan --- .../v3/RedisConnectionMethodInterceptor.java | 35 ++++++------ .../RedisConnectionInstrumentation.java | 54 +++++++++---------- 2 files changed, 47 insertions(+), 42 deletions(-) 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 f660ca1a15..12f1ca42bf 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 @@ -19,6 +19,8 @@ package org.apache.skywalking.apm.plugin.redisson.v3; import io.netty.channel.Channel; + +import java.util.Objects; import java.util.stream.Collectors; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; @@ -28,8 +30,8 @@ import org.apache.skywalking.apm.agent.core.logging.api.LogManager; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor; -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.agent.core.plugin.interceptor.enhance.v2.InstanceMethodsAroundInterceptorV2; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2.MethodInvocationContext; 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; @@ -42,17 +44,17 @@ import java.net.InetSocketAddress; import java.util.Optional; -public class RedisConnectionMethodInterceptor implements InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor { +public class RedisConnectionMethodInterceptor implements InstanceMethodsAroundInterceptorV2, InstanceConstructorInterceptor { private static final ILog LOGGER = LogManager.getLogger(RedisConnectionMethodInterceptor.class); private static final String ABBR = "..."; private static final String QUESTION_MARK = "?"; private static final String DELIMITER_SPACE = " "; + public static final Object STOP_SPAN_FLAG = new Object(); @Override - public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, - MethodInterceptResult result) throws Throwable { + public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInvocationContext context) throws Throwable { String peer = (String) objInst.getSkyWalkingDynamicField(); RedisConnection connection = (RedisConnection) objInst; @@ -81,6 +83,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr } } + context.setContext(STOP_SPAN_FLAG); AbstractSpan span = ContextManager.createExitSpan(operationName, peer); span.setComponent(ComponentsDefine.REDISSON); Tags.CACHE_TYPE.set(span, "Redis"); @@ -93,17 +96,19 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr } @Override - public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, - Object ret) throws Throwable { - ContextManager.stopSpan(); + public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret, MethodInvocationContext context) throws Throwable { + if (Objects.nonNull(context.getContext())) { + ContextManager.stopSpan(); + } return ret; } @Override - public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, - Class[] argumentsTypes, Throwable t) { - AbstractSpan span = ContextManager.activeSpan(); - span.log(t); + public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Throwable t, MethodInvocationContext context) { + if (ContextManager.isActive()) { + AbstractSpan span = ContextManager.activeSpan(); + span.log(t); + } } @Override @@ -154,8 +159,8 @@ private Optional parseOperation(String cmd) { private String showBatchCommands(CommandsData commandsData) { return commandsData.getCommands() - .stream() - .map(data -> data.getCommand().getName()) - .collect(Collectors.joining(";")); + .stream() + .map(data -> data.getCommand().getName()) + .collect(Collectors.joining(";")); } } diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java index 84bc82535f..bc1c35a3c1 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java @@ -21,15 +21,15 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint; -import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; -import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.v2.ClassInstanceMethodsEnhancePluginDefineV2; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.v2.InstanceMethodsInterceptV2Point; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; import static net.bytebuddy.matcher.ElementMatchers.named; import static org.apache.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType; import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; -public class RedisConnectionInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { +public class RedisConnectionInstrumentation extends ClassInstanceMethodsEnhancePluginDefineV2 { private static final String ENHANCE_CLASS = "org.redisson.client.RedisConnection"; @@ -38,39 +38,39 @@ public class RedisConnectionInstrumentation extends ClassInstanceMethodsEnhanceP @Override public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { return new ConstructorInterceptPoint[] { - new ConstructorInterceptPoint() { - @Override - public ElementMatcher getConstructorMatcher() { - return takesArgumentWithType(0, "org.redisson.client.RedisClient"); - } + new ConstructorInterceptPoint() { + @Override + public ElementMatcher getConstructorMatcher() { + return takesArgumentWithType(0, "org.redisson.client.RedisClient"); + } - @Override - public String getConstructorInterceptor() { - return REDISSON_METHOD_INTERCEPTOR_CLASS; + @Override + public String getConstructorInterceptor() { + return REDISSON_METHOD_INTERCEPTOR_CLASS; + } } - } }; } @Override - public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { - return new InstanceMethodsInterceptPoint[] { - new InstanceMethodsInterceptPoint() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("send"); - } + public InstanceMethodsInterceptV2Point[] getInstanceMethodsInterceptV2Points() { + return new InstanceMethodsInterceptV2Point[] { + new InstanceMethodsInterceptV2Point() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("send"); + } - @Override - public String getMethodsInterceptor() { - return REDISSON_METHOD_INTERCEPTOR_CLASS; - } + @Override + public String getMethodsInterceptorV2() { + return REDISSON_METHOD_INTERCEPTOR_CLASS; + } - @Override - public boolean isOverrideArgs() { - return false; + @Override + public boolean isOverrideArgs() { + return false; + } } - } }; } From 2577058a4f944508fd07756b02be4d70bddecc15 Mon Sep 17 00:00:00 2001 From: mijiatian Date: Sun, 23 Jun 2024 23:43:53 +0800 Subject: [PATCH 2/5] fix redisson plugin's problem of exitSpan --- .../RedisConnectionInstrumentation.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java index bc1c35a3c1..34678af6b7 100644 --- a/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/define/RedisConnectionInstrumentation.java @@ -38,39 +38,39 @@ public class RedisConnectionInstrumentation extends ClassInstanceMethodsEnhanceP @Override public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { return new ConstructorInterceptPoint[] { - new ConstructorInterceptPoint() { - @Override - public ElementMatcher getConstructorMatcher() { - return takesArgumentWithType(0, "org.redisson.client.RedisClient"); - } + new ConstructorInterceptPoint() { + @Override + public ElementMatcher getConstructorMatcher() { + return takesArgumentWithType(0, "org.redisson.client.RedisClient"); + } - @Override - public String getConstructorInterceptor() { - return REDISSON_METHOD_INTERCEPTOR_CLASS; - } + @Override + public String getConstructorInterceptor() { + return REDISSON_METHOD_INTERCEPTOR_CLASS; } + } }; } @Override public InstanceMethodsInterceptV2Point[] getInstanceMethodsInterceptV2Points() { return new InstanceMethodsInterceptV2Point[] { - new InstanceMethodsInterceptV2Point() { - @Override - public ElementMatcher getMethodsMatcher() { - return named("send"); - } + new InstanceMethodsInterceptV2Point() { + @Override + public ElementMatcher getMethodsMatcher() { + return named("send"); + } - @Override - public String getMethodsInterceptorV2() { - return REDISSON_METHOD_INTERCEPTOR_CLASS; - } + @Override + public String getMethodsInterceptorV2() { + return REDISSON_METHOD_INTERCEPTOR_CLASS; + } - @Override - public boolean isOverrideArgs() { - return false; - } + @Override + public boolean isOverrideArgs() { + return false; } + } }; } From 4d5b7346bb11d237037a58e9bb8a7300a4fd48f1 Mon Sep 17 00:00:00 2001 From: mijiatian Date: Sun, 23 Jun 2024 23:46:22 +0800 Subject: [PATCH 3/5] fix redisson plugin's problem of exitSpan --- .../redisson/v3/RedisConnectionMethodInterceptor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 12f1ca42bf..113066dc66 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 @@ -159,8 +159,8 @@ private Optional parseOperation(String cmd) { private String showBatchCommands(CommandsData commandsData) { return commandsData.getCommands() - .stream() - .map(data -> data.getCommand().getName()) - .collect(Collectors.joining(";")); + .stream() + .map(data -> data.getCommand().getName()) + .collect(Collectors.joining(";")); } } From 43b661c00b99d514ff599a97877aab8d15f5da10 Mon Sep 17 00:00:00 2001 From: mijiatian Date: Mon, 24 Jun 2024 00:33:00 +0800 Subject: [PATCH 4/5] fix problem discovering in code review --- .../redisson/v3/RedisConnectionMethodInterceptor.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 113066dc66..183612df29 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 @@ -105,7 +105,7 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Throwable t, MethodInvocationContext context) { - if (ContextManager.isActive()) { + if (Objects.nonNull(context.getContext())) { AbstractSpan span = ContextManager.activeSpan(); span.log(t); } @@ -159,8 +159,8 @@ private Optional parseOperation(String cmd) { private String showBatchCommands(CommandsData commandsData) { return commandsData.getCommands() - .stream() - .map(data -> data.getCommand().getName()) - .collect(Collectors.joining(";")); + .stream() + .map(data -> data.getCommand().getName()) + .collect(Collectors.joining(";")); } } From 2061c8a47674416708f4bab0f8e92543249c760a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= Date: Mon, 24 Jun 2024 07:52:18 +0800 Subject: [PATCH 5/5] Update apm-sniffer/apm-sdk-plugin/redisson-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/redisson/v3/RedisConnectionMethodInterceptor.java --- .../plugin/redisson/v3/RedisConnectionMethodInterceptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 183612df29..d475382c8f 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 @@ -83,8 +83,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr } } - context.setContext(STOP_SPAN_FLAG); AbstractSpan span = ContextManager.createExitSpan(operationName, peer); + context.setContext(STOP_SPAN_FLAG); span.setComponent(ComponentsDefine.REDISSON); Tags.CACHE_TYPE.set(span, "Redis"); Tags.CACHE_INSTANCE.set(span, dbInstance);