You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/09/22 12:47:01 UTC
[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #34: Fix spring mvc plugin
wu-sheng commented on a change in pull request #34:
URL: https://github.com/apache/skywalking-java/pull/34#discussion_r713904599
##########
File path: apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
##########
@@ -198,56 +198,58 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
Object request = runtimeContext.get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
if (request != null) {
- StackDepth stackDepth = (StackDepth) runtimeContext.get(CONTROLLER_METHOD_STACK_DEPTH);
- if (stackDepth == null) {
- throw new IllegalMethodStackDepthException();
- } else {
- stackDepth.decrement();
- }
-
- AbstractSpan span = ContextManager.activeSpan();
-
- if (stackDepth.depth() == 0) {
- Object response = runtimeContext.get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
- if (response == null) {
- throw new ServletResponseNotFoundException();
+ try {
+ StackDepth stackDepth = (StackDepth) runtimeContext.get(CONTROLLER_METHOD_STACK_DEPTH);
+ if (stackDepth == null) {
+ throw new IllegalMethodStackDepthException();
+ } else {
+ stackDepth.decrement();
}
- Integer statusCode = null;
+ AbstractSpan span = ContextManager.activeSpan();
- if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom(response.getClass())) {
- statusCode = ((HttpServletResponse) response).getStatus();
- } else if (ServerHttpResponse.class.isAssignableFrom(response.getClass())) {
- if (IS_SERVLET_GET_STATUS_METHOD_EXIST) {
- statusCode = ((ServerHttpResponse) response).getRawStatusCode();
+ if (stackDepth.depth() == 0) {
+ Object response = runtimeContext.get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+ if (response == null) {
+ throw new ServletResponseNotFoundException();
}
- Object context = runtimeContext.get(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
- if (context != null) {
- ((AbstractSpan[]) context)[0] = span.prepareForAsync();
+
+ Integer statusCode = null;
+
+ if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom(response.getClass())) {
+ statusCode = ((HttpServletResponse) response).getStatus();
+ } else if (ServerHttpResponse.class.isAssignableFrom(response.getClass())) {
+ if (IS_SERVLET_GET_STATUS_METHOD_EXIST) {
+ statusCode = ((ServerHttpResponse) response).getRawStatusCode();
+ }
+ Object context = runtimeContext.get(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
+ if (context != null) {
+ ((AbstractSpan[]) context)[0] = span.prepareForAsync();
+ }
}
- }
- if (statusCode != null && statusCode >= 400) {
- span.errorOccurred();
- Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
- }
+ if (statusCode != null && statusCode >= 400) {
+ span.errorOccurred();
+ Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
+ }
- runtimeContext.remove(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(CONTROLLER_METHOD_STACK_DEPTH);
- }
+ runtimeContext.remove(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(CONTROLLER_METHOD_STACK_DEPTH);
+ }
- // Active HTTP parameter collection automatically in the profiling context.
- if (!SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS && span.isProfiling()) {
- if (HttpServletRequest.class.isAssignableFrom(request.getClass())) {
- RequestUtil.collectHttpParam((HttpServletRequest) request, span);
- } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
- RequestUtil.collectHttpParam((ServerHttpRequest) request, span);
+ // Active HTTP parameter collection automatically in the profiling context.
+ if (!SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS && span.isProfiling()) {
+ if (HttpServletRequest.class.isAssignableFrom(request.getClass())) {
+ RequestUtil.collectHttpParam((HttpServletRequest) request, span);
+ } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
+ RequestUtil.collectHttpParam((ServerHttpRequest) request, span);
+ }
}
+ } finally {
+ ContextManager.stopSpan();
Review comment:
Are you only adding this `try/finally` to stop span? Could you share when this last step goes missed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org