You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2019/04/29 03:42:03 UTC
[skywalking] branch master updated: Fix #2546 (#2551)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new 31cb96d Fix #2546 (#2551)
31cb96d is described below
commit 31cb96d99c4f89c741640a085627de6b7e7c98b2
Author: Xin,Zhang <nz...@gmail.com>
AuthorDate: Mon Apr 29 11:41:57 2019 +0800
Fix #2546 (#2551)
* Fix #2546
* Change the stackdepth type
* Fix bug
* Fix CI failed
---
.../plugin/spring/mvc/v4/SpringTestCaseHelper.java | 1 +
.../apm/plugin/spring/mvc/commons/Constants.java | 2 +
.../IllegalMethodStackDepthException.java | 24 +++++++
.../ServletResponseNotFoundException.java | 24 +++++++
.../interceptor/AbstractMethodInterceptor.java | 82 +++++++++++++++++-----
.../spring/mvc/commons/interceptor/StackDepth.java | 41 +++++++++++
.../webflux/v5/AbstractMethodInterceptor.java | 40 ++++++++---
7 files changed, 185 insertions(+), 29 deletions(-)
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/SpringTestCaseHelper.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/SpringTestCaseHelper.java
index 34b257a..ffcee2a 100644
--- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/SpringTestCaseHelper.java
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/SpringTestCaseHelper.java
@@ -32,6 +32,7 @@ public final class SpringTestCaseHelper {
ContextManager.getRuntimeContext().put(Constants.RESPONSE_KEY_IN_RUNTIME_CONTEXT, response);
a.handleCase();
ContextManager.stopSpan();
+ ContextManager.getRuntimeContext().remove(Constants.CONTROLLER_METHOD_STACK_DEPTH);
}
public interface CaseHandler {
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/Constants.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/Constants.java
index 114f8c2..42119d6 100644
--- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/Constants.java
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/Constants.java
@@ -39,4 +39,6 @@ public class Constants {
public static final String FORWARD_REQUEST_FLAG = "SW_FORWARD_REQUEST_FLAG";
public static final String WEBFLUX_REQUEST_KEY = "SW_WEBFLUX_REQUEST_KEY";
+
+ public static final String CONTROLLER_METHOD_STACK_DEPTH = "SW_CONTROLLER_METHOD_STACK_DEPTH";
}
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/IllegalMethodStackDepthException.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/IllegalMethodStackDepthException.java
new file mode 100644
index 0000000..eee2204
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/IllegalMethodStackDepthException.java
@@ -0,0 +1,24 @@
+/*
+ * 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.spring.mvc.commons.exception;
+
+public class IllegalMethodStackDepthException extends RuntimeException {
+ public IllegalMethodStackDepthException() {
+ super("Please submit an new issue to Apache Skywalking if this Exception was happened.");
+ }
+}
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/ServletResponseNotFoundException.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/ServletResponseNotFoundException.java
new file mode 100644
index 0000000..12d665c
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/exception/ServletResponseNotFoundException.java
@@ -0,0 +1,24 @@
+/*
+ * 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.spring.mvc.commons.exception;
+
+public class ServletResponseNotFoundException extends RuntimeException {
+ public ServletResponseNotFoundException() {
+ super("Please submit an new issue to Apache Skywalking if this Exception was happened.");
+ }
+}
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/AbstractMethodInterceptor.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
index 5c82ae2..1a98e7b 100644
--- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.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
@@ -34,7 +34,10 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInt
import org.apache.skywalking.apm.agent.core.util.MethodUtil;
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
import org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
+import org.apache.skywalking.apm.plugin.spring.mvc.commons.exception.IllegalMethodStackDepthException;
+import org.apache.skywalking.apm.plugin.spring.mvc.commons.exception.ServletResponseNotFoundException;
+import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.CONTROLLER_METHOD_STACK_DEPTH;
import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.FORWARD_REQUEST_FLAG;
import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.REQUEST_KEY_IN_RUNTIME_CONTEXT;
import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.RESPONSE_KEY_IN_RUNTIME_CONTEXT;
@@ -44,6 +47,7 @@ import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.RESP
*/
public abstract class AbstractMethodInterceptor implements InstanceMethodsAroundInterceptor {
public abstract String getRequestURL(Method method);
+
public abstract String getAcceptedMethodTypes(Method method);
@Override
@@ -75,21 +79,48 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround
HttpServletRequest request = (HttpServletRequest)ContextManager.getRuntimeContext().get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
if (request != null) {
- ContextCarrier contextCarrier = new ContextCarrier();
- CarrierItem next = contextCarrier.items();
- while (next.hasNext()) {
- next = next.next();
- next.setHeadValue(request.getHeader(next.getHeadKey()));
+ StackDepth stackDepth = (StackDepth)ContextManager.getRuntimeContext().get(CONTROLLER_METHOD_STACK_DEPTH);
+
+ if (stackDepth == null) {
+ ContextCarrier contextCarrier = new ContextCarrier();
+ CarrierItem next = contextCarrier.items();
+ while (next.hasNext()) {
+ next = next.next();
+ next.setHeadValue(request.getHeader(next.getHeadKey()));
+ }
+
+ AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
+ Tags.URL.set(span, request.getRequestURL().toString());
+ Tags.HTTP.METHOD.set(span, request.getMethod());
+ span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
+ SpanLayer.asHttp(span);
+
+ stackDepth = new StackDepth();
+ ContextManager.getRuntimeContext().put(CONTROLLER_METHOD_STACK_DEPTH, stackDepth);
+ } else {
+ AbstractSpan span =
+ ContextManager.createLocalSpan(buildOperationName(objInst, method));
+ span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
}
- AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
- Tags.URL.set(span, request.getRequestURL().toString());
- Tags.HTTP.METHOD.set(span, request.getMethod());
- span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
- SpanLayer.asHttp(span);
+ stackDepth.increment();
}
}
+ private String buildOperationName(Object invoker, Method method) {
+ StringBuilder operationName = new StringBuilder(invoker.getClass().getName())
+ .append(".").append(method.getName()).append("(");
+ for (Class<?> type : method.getParameterTypes()) {
+ operationName.append(type.getName()).append(",");
+ }
+
+ if (method.getParameterTypes().length > 0) {
+ operationName = operationName.deleteCharAt(operationName.length() - 1);
+ }
+
+ return operationName.append(")").toString();
+ }
+
@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
@@ -102,19 +133,34 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround
return ret;
}
- HttpServletResponse response = (HttpServletResponse)ContextManager.getRuntimeContext().get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
- try {
- if (response != null) {
- AbstractSpan span = ContextManager.activeSpan();
+ HttpServletRequest request = (HttpServletRequest)ContextManager.getRuntimeContext().get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
+
+ if (request != null) {
+ StackDepth stackDepth = (StackDepth)ContextManager.getRuntimeContext().get(CONTROLLER_METHOD_STACK_DEPTH);
+ if (stackDepth == null) {
+ throw new IllegalMethodStackDepthException();
+ } else {
+ stackDepth.decrement();
+ }
+
+ AbstractSpan span = ContextManager.activeSpan();
+
+ if (stackDepth.depth() == 0) {
+ HttpServletResponse response = (HttpServletResponse)ContextManager.getRuntimeContext().get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+ if (response == null) {
+ throw new ServletResponseNotFoundException();
+ }
+
if (response.getStatus() >= 400) {
span.errorOccurred();
Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus()));
+
+ ContextManager.getRuntimeContext().remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
+ ContextManager.getRuntimeContext().remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
}
- ContextManager.stopSpan();
}
- } finally {
- ContextManager.getRuntimeContext().remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
- ContextManager.getRuntimeContext().remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+
+ ContextManager.stopSpan();
}
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/StackDepth.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/StackDepth.java
new file mode 100644
index 0000000..118df51
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/StackDepth.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.spring.mvc.commons.interceptor;
+
+public class StackDepth {
+
+ private int stackDepth;
+
+ public StackDepth() {
+ this.stackDepth = 0;
+ }
+
+ public int depth() {
+ return this.stackDepth;
+ }
+
+ public StackDepth increment() {
+ this.stackDepth++;
+ return this;
+ }
+
+ public StackDepth decrement() {
+ this.stackDepth--;
+ return this;
+ }
+}
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/AbstractMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/AbstractMethodInterceptor.java
index 1efefc2..d67e775 100644
--- a/apm-sniffer/apm-sdk-plugin/spring-plugins/webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/AbstractMethodInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/AbstractMethodInterceptor.java
@@ -30,7 +30,9 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceM
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.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.StackDepth;
+import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.CONTROLLER_METHOD_STACK_DEPTH;
import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.FORWARD_REQUEST_FLAG;
import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.WEBFLUX_REQUEST_KEY;
@@ -39,6 +41,7 @@ import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.WEBF
*/
public abstract class AbstractMethodInterceptor implements InstanceMethodsAroundInterceptor {
public abstract String getRequestURL(Method method);
+
public abstract String getAcceptedMethodTypes(Method method);
@Override
@@ -64,18 +67,30 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround
HttpRequest request = (HttpRequest)ContextManager.getRuntimeContext().get(WEBFLUX_REQUEST_KEY);
if (request != null) {
- ContextCarrier contextCarrier = new ContextCarrier();
- CarrierItem next = contextCarrier.items();
- while (next.hasNext()) {
- next = next.next();
- next.setHeadValue(request.headers().get(next.getHeadKey()));
+
+ StackDepth stackDepth = (StackDepth)ContextManager.getRuntimeContext().get(CONTROLLER_METHOD_STACK_DEPTH);
+
+ if (stackDepth == null) {
+ ContextCarrier contextCarrier = new ContextCarrier();
+ CarrierItem next = contextCarrier.items();
+ while (next.hasNext()) {
+ next = next.next();
+ next.setHeadValue(request.headers().get(next.getHeadKey()));
+ }
+
+ AbstractSpan span = ContextManager.createEntrySpan(requestURL, contextCarrier);
+ Tags.URL.set(span, request.uri());
+ Tags.HTTP.METHOD.set(span, request.method().name());
+ span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
+ SpanLayer.asHttp(span);
+
+ stackDepth = new StackDepth();
+ } else {
+ AbstractSpan span = ContextManager.createLocalSpan(objInst.getClass().getName() + "/" + method.getName());
+ span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
}
- AbstractSpan span = ContextManager.createEntrySpan(requestURL, contextCarrier);
- Tags.URL.set(span, request.uri());
- Tags.HTTP.METHOD.set(span, request.method().name());
- span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION);
- SpanLayer.asHttp(span);
+ stackDepth.increment();
}
}
@@ -91,7 +106,10 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround
return ret;
}
- ContextManager.stopSpan();
+ HttpRequest request = (HttpRequest)ContextManager.getRuntimeContext().get(WEBFLUX_REQUEST_KEY);
+ if (request != null) {
+ ContextManager.stopSpan();
+ }
return ret;
}