From 44a5526c78770578bc9ced9911ba32c95e923f70 Mon Sep 17 00:00:00 2001 From: YunaiV Date: Thu, 11 Jan 2018 20:51:19 +0800 Subject: [PATCH] fixes #735 Concurrency conflicts in Spring plugin * EnhanceRequireObjectCache add clearRequestAndResponse method * invoke EnhanceRequireObjectCache clearRequestAndResponse At AbstractMethodInterceptor --- .../commons/EnhanceRequireObjectCache.java | 5 +++++ ...or.java => AbstractMethodInterceptor.java} | 22 ++++++++++--------- .../InvokeHandlerMethodInterceptor.java | 3 --- .../RequestMappingMethodInterceptor.java | 5 +++-- .../RestMappingMethodInterceptor.java | 9 +++----- 5 files changed, 23 insertions(+), 21 deletions(-) rename apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/{AbstractMethodInteceptor.java => AbstractMethodInterceptor.java} (85%) diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/EnhanceRequireObjectCache.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/EnhanceRequireObjectCache.java index 7623582f2..cbadbfdab 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/EnhanceRequireObjectCache.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/EnhanceRequireObjectCache.java @@ -56,4 +56,9 @@ public class EnhanceRequireObjectCache { this.httpResponse.set(httpResponse); } + public void clearRequestAndResponse() { + setNativeWebRequest(null); + setHttpResponse(null); + } + } diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInteceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java similarity index 85% rename from apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInteceptor.java rename to apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java index 8941cd10b..798a2216e 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInteceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java @@ -40,7 +40,7 @@ import java.lang.reflect.Method; /** * the abstract method inteceptor */ -public abstract class AbstractMethodInteceptor implements InstanceMethodsAroundInterceptor { +public abstract class AbstractMethodInterceptor implements InstanceMethodsAroundInterceptor { public abstract String getRequestURL(Method method); @Override @@ -73,17 +73,19 @@ public abstract class AbstractMethodInteceptor implements InstanceMethodsAroundI @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { - HttpServletResponse response = ((EnhanceRequireObjectCache)objInst.getSkyWalkingDynamicField()).getHttpServletResponse(); + try { + HttpServletResponse response = ((EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()).getHttpServletResponse(); - AbstractSpan span = ContextManager.activeSpan(); - if (response.getStatus() >= 400) { - span.errorOccurred(); - Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus())); + AbstractSpan span = ContextManager.activeSpan(); + if (response.getStatus() >= 400) { + span.errorOccurred(); + Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus())); + } + ContextManager.stopSpan(); + return ret; + } finally { + ((EnhanceRequireObjectCache)objInst.getSkyWalkingDynamicField()).clearRequestAndResponse(); } - ContextManager.stopSpan(); - - ((EnhanceRequireObjectCache)objInst.getSkyWalkingDynamicField()).setNativeWebRequest(null); - return ret; } @Override diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/InvokeHandlerMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/InvokeHandlerMethodInterceptor.java index 061d505d4..c2d555608 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/InvokeHandlerMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/InvokeHandlerMethodInterceptor.java @@ -38,9 +38,6 @@ public class InvokeHandlerMethodInterceptor implements InstanceMethodsAroundInte @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { - if (allArguments[2] instanceof EnhancedInstance) { - ((EnhanceRequireObjectCache)((EnhancedInstance)allArguments[2]).getSkyWalkingDynamicField()).setHttpResponse(null); - } return ret; } diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java index 58c4bcf05..1dd6b7748 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java @@ -19,16 +19,17 @@ package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor; -import java.lang.reflect.Method; import org.springframework.web.bind.annotation.RequestMapping; +import java.lang.reflect.Method; + /** * The RequestMappingMethodInterceptor only use the first mapping value. * it will inteceptor with @RequestMapping * * @author clevertension */ -public class RequestMappingMethodInterceptor extends AbstractMethodInteceptor { +public class RequestMappingMethodInterceptor extends AbstractMethodInterceptor { @Override public String getRequestURL(Method method) { String requestURL = ""; diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java index faee5b4cb..5cea58d0d 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java @@ -19,12 +19,9 @@ package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor; +import org.springframework.web.bind.annotation.*; + import java.lang.reflect.Method; -import org.springframework.web.bind.annotation.DeleteMapping; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PatchMapping; -import org.springframework.web.bind.annotation.PostMapping; -import org.springframework.web.bind.annotation.PutMapping; /** * The RestMappingMethodInterceptor only use the first mapping value. @@ -34,7 +31,7 @@ import org.springframework.web.bind.annotation.PutMapping; * * @author clevertension */ -public class RestMappingMethodInterceptor extends AbstractMethodInteceptor { +public class RestMappingMethodInterceptor extends AbstractMethodInterceptor { @Override public String getRequestURL(Method method) { String requestURL = "";