You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@skywalking.apache.org by GitBox <gi...@apache.org> on 2018/01/13 07:09:49 UTC
[GitHub] ascrutae closed pull request #738: Concurrency conflicts in Spring plugin #735
ascrutae closed pull request #738: Concurrency conflicts in Spring plugin #735
URL: https://github.com/apache/incubator-skywalking/pull/738
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 914882eed..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
@@ -18,25 +18,26 @@
package org.apache.skywalking.apm.plugin.spring.mvc.commons;
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletResponse;
import org.springframework.web.context.request.NativeWebRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Method;
+
public class EnhanceRequireObjectCache {
private PathMappingCache pathMappingCache;
- private NativeWebRequest nativeWebRequest;
- private HttpServletResponse httpResponse;
+ private ThreadLocal<NativeWebRequest> nativeWebRequest = new ThreadLocal<NativeWebRequest>();
+ private ThreadLocal<HttpServletResponse> httpResponse = new ThreadLocal<HttpServletResponse>();
public void setPathMappingCache(PathMappingCache pathMappingCache) {
this.pathMappingCache = pathMappingCache;
}
public HttpServletResponse getHttpServletResponse() {
- return httpResponse == null ? (HttpServletResponse)nativeWebRequest.getNativeResponse() : httpResponse;
+ return httpResponse.get() == null ? (HttpServletResponse) nativeWebRequest.get().getNativeResponse() : httpResponse.get();
}
public void setNativeWebRequest(NativeWebRequest nativeWebRequest) {
- this.nativeWebRequest = nativeWebRequest;
+ this.nativeWebRequest.set(nativeWebRequest);
}
public String findPathMapping(Method method) {
@@ -52,10 +53,12 @@ public PathMappingCache getPathMappingCache() {
}
public void setHttpResponse(HttpServletResponse httpResponse) {
- this.httpResponse = httpResponse;
+ this.httpResponse.set(httpResponse);
}
- public HttpServletResponse getHttpResponse() {
- return 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 ca7d97597..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
@@ -19,27 +19,28 @@
package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
+import org.apache.skywalking.apm.agent.core.context.CarrierItem;
import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
+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;
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
-import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
-import org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
-import org.apache.skywalking.apm.agent.core.context.CarrierItem;
-import org.apache.skywalking.apm.agent.core.context.ContextManager;
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.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+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
@@ -72,15 +73,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 {
- 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();
- 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 7a502d21f..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
@@ -18,13 +18,14 @@
package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletResponse;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
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.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Method;
+
public class InvokeHandlerMethodInterceptor implements InstanceMethodsAroundInterceptor {
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
@@ -42,6 +43,5 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
@Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
-
}
}
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 <code>RequestMappingMethodInterceptor</code> only use the first mapping value.
* it will inteceptor with <code>@RequestMapping</code>
*
* @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 <code>RestMappingMethodInterceptor</code> only use the first mapping value.
@@ -34,7 +31,7 @@
*
* @author clevertension
*/
-public class RestMappingMethodInterceptor extends AbstractMethodInteceptor {
+public class RestMappingMethodInterceptor extends AbstractMethodInterceptor {
@Override
public String getRequestURL(Method method) {
String requestURL = "";
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services