From 5c6914fc9eea78eb7285976060690e1a1a80010f Mon Sep 17 00:00:00 2001 From: jialong <419016694@qq.com> Date: Sat, 1 Feb 2020 16:18:22 +0800 Subject: [PATCH] Make MVC plugin support inherited annotations from interface (#4278) --- .../MethodInheritanceAnnotationMatcher.java | 101 ++++++++++++++++++ ...ethodInheritanceAnnotationMatcherTest.java | 87 +++++++++++++++ .../v3/define/ControllerInstrumentation.java | 3 +- .../AbstractControllerInstrumentation.java | 13 +-- .../AbstractControllerInstrumentation.java | 13 +-- .../spring/mvc/commons/ParsePathUtil.java | 69 ++++++++++++ .../RequestMappingMethodInterceptor.java | 34 +++--- .../RestMappingMethodInterceptor.java | 83 +++++++------- .../config/expectedData.yaml | 82 +++++++++++++- .../implinterface/TestCaseController.java | 38 +++++++ .../implinterface/TestCaseInterface.java | 33 ++++++ .../resttemplate/RestTemplateController.java | 8 ++ 12 files changed, 498 insertions(+), 66 deletions(-) create mode 100644 apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/match/MethodInheritanceAnnotationMatcher.java create mode 100644 apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/plugin/bytebuddy/MethodInheritanceAnnotationMatcherTest.java create mode 100644 apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/ParsePathUtil.java create mode 100644 test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseController.java create mode 100644 test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseInterface.java diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/match/MethodInheritanceAnnotationMatcher.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/match/MethodInheritanceAnnotationMatcher.java new file mode 100644 index 000000000..1b62350ce --- /dev/null +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/plugin/match/MethodInheritanceAnnotationMatcher.java @@ -0,0 +1,101 @@ +/* + * 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.agent.core.plugin.match; + +import net.bytebuddy.build.HashCodeAndEqualsPlugin; +import net.bytebuddy.description.annotation.AnnotationList; +import net.bytebuddy.description.annotation.AnnotationSource; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.method.MethodList; +import net.bytebuddy.description.method.ParameterList; +import net.bytebuddy.description.type.TypeDefinition; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.description.type.TypeList; +import net.bytebuddy.matcher.CollectionItemMatcher; +import net.bytebuddy.matcher.ElementMatcher; + +import java.util.Objects; + +import static net.bytebuddy.matcher.ElementMatchers.annotationType; + +/** + * Matching used to match method annotations, Can match annotations on interface methods + * @author jialong + */ +@HashCodeAndEqualsPlugin.Enhance +public class MethodInheritanceAnnotationMatcher extends ElementMatcher.Junction.AbstractBase { + /** + * The matcher to be applied to the provided annotation list. + */ + private final ElementMatcher matcher; + + /** + * Creates a new matcher for the annotations of an annotated element. + * + * @param matcher The matcher to be applied to the provided annotation list. + */ + public MethodInheritanceAnnotationMatcher(ElementMatcher matcher) { + this.matcher = matcher; + } + + @Override + public boolean matches(T target) { + if (matcher.matches(target.getDeclaredAnnotations())) { + return true; + } + String name = target.getName(); + ParameterList parameters = target.getParameters(); + + TypeDefinition declaringType = target.getDeclaringType(); + return recursiveMatches(declaringType, name, parameters); + } + + + private boolean recursiveMatches(TypeDefinition typeDefinition, String methodName, ParameterList parameters) { + TypeList.Generic interfaces = typeDefinition.getInterfaces(); + for (TypeDescription.Generic implInterface : interfaces) { + if (recursiveMatches(implInterface, methodName, parameters)) { + return true; + } + MethodList declaredMethods = implInterface.getDeclaredMethods(); + for (MethodDescription declaredMethod : declaredMethods) { + if (Objects.equals(declaredMethod.getName(), methodName) && parameterEquals(parameters, declaredMethod.getParameters())) { + return matcher.matches(declaredMethod.getDeclaredAnnotations()); + } + } + } + return false; + } + + + private boolean parameterEquals(ParameterList source, ParameterList impl) { + if (source.size() != impl.size()) { + return false; + } + for (int i = 0; i < source.size(); i++) { + if (!Objects.equals(source.get(i).getType(), impl.get(i).getType())) { + return false; + } + } + return true; + } + + public static ElementMatcher.Junction byMethodInheritanceAnnotationMatcher(ElementMatcher matcher) { + return new MethodInheritanceAnnotationMatcher(new CollectionItemMatcher<>(annotationType(matcher))); + } +} diff --git a/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/plugin/bytebuddy/MethodInheritanceAnnotationMatcherTest.java b/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/plugin/bytebuddy/MethodInheritanceAnnotationMatcherTest.java new file mode 100644 index 000000000..6555d1cd4 --- /dev/null +++ b/apm-sniffer/apm-agent-core/src/test/java/org/apache/skywalking/apm/agent/core/plugin/bytebuddy/MethodInheritanceAnnotationMatcherTest.java @@ -0,0 +1,87 @@ +/* + * 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.agent.core.plugin.bytebuddy; + +import net.bytebuddy.description.annotation.AnnotationSource; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.junit.Assert; +import org.junit.Test; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import static net.bytebuddy.matcher.ElementMatchers.named; +import static org.apache.skywalking.apm.agent.core.plugin.match.MethodInheritanceAnnotationMatcher.byMethodInheritanceAnnotationMatcher; + +/** + * @author jialong + */ +public class MethodInheritanceAnnotationMatcherTest { + + @Test + public void testMatch() throws Exception { + ElementMatcher.Junction matcher = byMethodInheritanceAnnotationMatcher(named("org.apache.skywalking.apm.agent.core.plugin.bytebuddy.MethodInheritanceAnnotationMatcherTest$TestAnnotaion")); + + Assert.assertTrue(matcher.matches(new MethodDescription.ForLoadedMethod(TestBean.class.getMethod("test1", String.class)))); + Assert.assertTrue(matcher.matches(new MethodDescription.ForLoadedMethod(TestBean.class.getMethod("test2", String.class)))); + Assert.assertTrue(matcher.matches(new MethodDescription.ForLoadedMethod(TestBean.class.getMethod("test3", String.class)))); + } + + private class TestBean implements TestInterface1, TestInterface3 { + @Override + public String test1(String test) { + return null; + } + + @Override + public String test2(String test) { + return null; + } + + @Override + public String test3(String test) { + return null; + } + } + + private interface TestInterface1 extends TestInterface2 { + @TestAnnotaion + String test1(String test); + } + + private interface TestInterface2 { + @TestAnnotaion + String test2(String test); + } + + private interface TestInterface3 { + @TestAnnotaion + String test3(String test); + } + + @Target({ElementType.METHOD}) + @Retention(RetentionPolicy.RUNTIME) + @Documented + public @interface TestAnnotaion { + } + +} diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v3/define/ControllerInstrumentation.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v3/define/ControllerInstrumentation.java index a63bfacd0..887f49d03 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v3/define/ControllerInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v3/define/ControllerInstrumentation.java @@ -28,6 +28,7 @@ import org.apache.skywalking.apm.agent.core.plugin.match.ClassAnnotationMatch; import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; import static net.bytebuddy.matcher.ElementMatchers.*; +import static org.apache.skywalking.apm.agent.core.plugin.match.MethodInheritanceAnnotationMatcher.byMethodInheritanceAnnotationMatcher; import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.REQUEST_MAPPING_METHOD_INTERCEPTOR; /** @@ -65,7 +66,7 @@ public class ControllerInstrumentation extends AbstractSpring3Instrumentation { new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return isAnnotatedWith(named(REQUEST_MAPPING_ENHANCE_ANNOTATION)); + return byMethodInheritanceAnnotationMatcher(named(REQUEST_MAPPING_ENHANCE_ANNOTATION)); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/define/AbstractControllerInstrumentation.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/define/AbstractControllerInstrumentation.java index 72fccc43d..95b6df720 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/define/AbstractControllerInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v4/define/AbstractControllerInstrumentation.java @@ -28,6 +28,7 @@ import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; import org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants; import static net.bytebuddy.matcher.ElementMatchers.*; +import static org.apache.skywalking.apm.agent.core.plugin.match.MethodInheritanceAnnotationMatcher.byMethodInheritanceAnnotationMatcher; /** * {@link ControllerInstrumentation} enhance all constructor and method annotated with @@ -68,7 +69,7 @@ public abstract class AbstractControllerInstrumentation extends AbstractSpring4I new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return isAnnotatedWith(named("org.springframework.web.bind.annotation.RequestMapping")); + return byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.RequestMapping")); } @Override @@ -84,11 +85,11 @@ public abstract class AbstractControllerInstrumentation extends AbstractSpring4I new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return isAnnotatedWith(named("org.springframework.web.bind.annotation.GetMapping")) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PostMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PutMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.DeleteMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PatchMapping"))); + return byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.GetMapping")) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PostMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PutMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.DeleteMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PatchMapping"))); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v5/define/AbstractControllerInstrumentation.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v5/define/AbstractControllerInstrumentation.java index 7f2372ad1..e4cee47ac 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v5/define/AbstractControllerInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/v5/define/AbstractControllerInstrumentation.java @@ -27,6 +27,7 @@ import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; import org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants; import static net.bytebuddy.matcher.ElementMatchers.*; +import static org.apache.skywalking.apm.agent.core.plugin.match.MethodInheritanceAnnotationMatcher.byMethodInheritanceAnnotationMatcher; /** * {@link ControllerInstrumentation} enhance all constructor and method annotated with @@ -67,7 +68,7 @@ public abstract class AbstractControllerInstrumentation extends AbstractSpring5I new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return isAnnotatedWith(named("org.springframework.web.bind.annotation.RequestMapping")); + return byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.RequestMapping")); } @Override @@ -83,11 +84,11 @@ public abstract class AbstractControllerInstrumentation extends AbstractSpring5I new DeclaredInstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return isAnnotatedWith(named("org.springframework.web.bind.annotation.GetMapping")) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PostMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PutMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.DeleteMapping"))) - .or(isAnnotatedWith(named("org.springframework.web.bind.annotation.PatchMapping"))); + return byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.GetMapping")) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PostMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PutMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.DeleteMapping"))) + .or(byMethodInheritanceAnnotationMatcher(named("org.springframework.web.bind.annotation.PatchMapping"))); } @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/ParsePathUtil.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/ParsePathUtil.java new file mode 100644 index 000000000..17737b7e5 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/ParsePathUtil.java @@ -0,0 +1,69 @@ +/* + * 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; + +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; + +/** + * Tools for parsing path from annotation + * @author jialong + */ +public class ParsePathUtil { + + public static String recursiveParseMethodAnnotaion(Method method, Function parseFunc) { + String result = parseFunc.apply(method); + if (Objects.isNull(result)) { + Class declaringClass = method.getDeclaringClass(); + result = recursiveMatches(declaringClass, method.getName(), method.getParameters(), parseFunc); + } + return Optional.ofNullable(result).orElse(""); + } + + private static String recursiveMatches(Class claz, String methodName, Parameter[] parameters, Function parseFunc) { + Class[] interfaces = claz.getInterfaces(); + for (Class implInterface : interfaces) { + String path = recursiveMatches(implInterface, methodName, parameters, parseFunc); + if (Objects.nonNull(path)) { + return path; + } + Method[] declaredMethods = implInterface.getDeclaredMethods(); + for (Method declaredMethod : declaredMethods) { + if (Objects.equals(declaredMethod.getName(), methodName) && parameterEquals(parameters, declaredMethod.getParameters())) { + return parseFunc.apply(declaredMethod); + } + } + } + return null; + } + + private static boolean parameterEquals(Parameter[] p1, Parameter[] p2) { + if (p1.length != p1.length) { + return false; + } + for (int i = 0; i < p1.length; i++) { + if (!Objects.equals(p1[i].getType().getName(), p2[i].getType().getName())) { + return false; + } + } + return true; + } +} 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 208800d27..70ab5fd93 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 @@ -18,6 +18,7 @@ package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor; +import org.apache.skywalking.apm.plugin.spring.mvc.commons.ParsePathUtil; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.web.bind.annotation.RequestMapping; @@ -32,21 +33,28 @@ import java.lang.reflect.Method; public class RequestMappingMethodInterceptor extends AbstractMethodInterceptor { @Override public String getRequestURL(Method method) { - String requestURL = ""; - RequestMapping methodRequestMapping = AnnotationUtils.getAnnotation(method, RequestMapping.class); - if (methodRequestMapping.value().length > 0) { - requestURL = methodRequestMapping.value()[0]; - } else if (methodRequestMapping.path().length > 0) { - requestURL = methodRequestMapping.path()[0]; - } - return requestURL; + return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> { + String requestURL = null; + RequestMapping methodRequestMapping = AnnotationUtils.getAnnotation(m, RequestMapping.class); + if (methodRequestMapping != null) { + if (methodRequestMapping.value().length > 0) { + requestURL = methodRequestMapping.value()[0]; + } else if (methodRequestMapping.path().length > 0) { + requestURL = methodRequestMapping.path()[0]; + } + } + return requestURL; + }); } @Override public String getAcceptedMethodTypes(Method method) { - RequestMapping methodRequestMapping = AnnotationUtils.getAnnotation(method, RequestMapping.class); - StringBuilder methodTypes = new StringBuilder(); - if (methodRequestMapping.method().length > 0) { + return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> { + RequestMapping methodRequestMapping = AnnotationUtils.getAnnotation(m, RequestMapping.class); + if (methodRequestMapping == null || methodRequestMapping.method().length == 0) { + return null; + } + StringBuilder methodTypes = new StringBuilder(); methodTypes.append("{"); for (int i = 0; i < methodRequestMapping.method().length; i++) { methodTypes.append(methodRequestMapping.method()[i].toString()); @@ -55,7 +63,7 @@ public class RequestMappingMethodInterceptor extends AbstractMethodInterceptor { } } methodTypes.append("}"); - } - return methodTypes.toString(); + return methodTypes.toString(); + }); } } 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 d122604dd..05066b3bb 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 @@ -18,8 +18,13 @@ package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor; +import org.apache.skywalking.apm.plugin.spring.mvc.commons.ParsePathUtil; import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.web.bind.annotation.*; +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; import java.lang.reflect.Method; @@ -34,44 +39,46 @@ import java.lang.reflect.Method; public class RestMappingMethodInterceptor extends AbstractMethodInterceptor { @Override public String getRequestURL(Method method) { - String requestURL = ""; - GetMapping getMapping = AnnotationUtils.getAnnotation(method, GetMapping.class); - PostMapping postMapping = AnnotationUtils.getAnnotation(method, PostMapping.class); - PutMapping putMapping = AnnotationUtils.getAnnotation(method, PutMapping.class); - DeleteMapping deleteMapping = AnnotationUtils.getAnnotation(method, DeleteMapping.class); - PatchMapping patchMapping = AnnotationUtils.getAnnotation(method, PatchMapping.class); - if (getMapping != null) { - if (getMapping.value().length > 0) { - requestURL = getMapping.value()[0]; - } else if (getMapping.path().length > 0) { - requestURL = getMapping.path()[0]; + return ParsePathUtil.recursiveParseMethodAnnotaion(method, m -> { + String requestURL = null; + GetMapping getMapping = AnnotationUtils.getAnnotation(m, GetMapping.class); + PostMapping postMapping = AnnotationUtils.getAnnotation(m, PostMapping.class); + PutMapping putMapping = AnnotationUtils.getAnnotation(m, PutMapping.class); + DeleteMapping deleteMapping = AnnotationUtils.getAnnotation(m, DeleteMapping.class); + PatchMapping patchMapping = AnnotationUtils.getAnnotation(m, PatchMapping.class); + if (getMapping != null) { + if (getMapping.value().length > 0) { + requestURL = getMapping.value()[0]; + } else if (getMapping.path().length > 0) { + requestURL = getMapping.path()[0]; + } + } else if (postMapping != null) { + if (postMapping.value().length > 0) { + requestURL = postMapping.value()[0]; + } else if (postMapping.path().length > 0) { + requestURL = postMapping.path()[0]; + } + } else if (putMapping != null) { + if (putMapping.value().length > 0) { + requestURL = putMapping.value()[0]; + } else if (putMapping.path().length > 0) { + requestURL = putMapping.path()[0]; + } + } else if (deleteMapping != null) { + if (deleteMapping.value().length > 0) { + requestURL = deleteMapping.value()[0]; + } else if (deleteMapping.path().length > 0) { + requestURL = deleteMapping.path()[0]; + } + } else if (patchMapping != null) { + if (patchMapping.value().length > 0) { + requestURL = patchMapping.value()[0]; + } else if (patchMapping.path().length > 0) { + requestURL = patchMapping.path()[0]; + } } - } else if (postMapping != null) { - if (postMapping.value().length > 0) { - requestURL = postMapping.value()[0]; - } else if (postMapping.path().length > 0) { - requestURL = postMapping.path()[0]; - } - } else if (putMapping != null) { - if (putMapping.value().length > 0) { - requestURL = putMapping.value()[0]; - } else if (putMapping.path().length > 0) { - requestURL = putMapping.path()[0]; - } - } else if (deleteMapping != null) { - if (deleteMapping.value().length > 0) { - requestURL = deleteMapping.value()[0]; - } else if (deleteMapping.path().length > 0) { - requestURL = deleteMapping.path()[0]; - } - } else if (patchMapping != null) { - if (patchMapping.value().length > 0) { - requestURL = patchMapping.value()[0]; - } else if (patchMapping.path().length > 0) { - requestURL = patchMapping.path()[0]; - } - } - return requestURL; + return requestURL; + }); } @Override diff --git a/test/plugin/scenarios/spring-4.3.x-scenario/config/expectedData.yaml b/test/plugin/scenarios/spring-4.3.x-scenario/config/expectedData.yaml index 982805a27..cc2ac080a 100644 --- a/test/plugin/scenarios/spring-4.3.x-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/spring-4.3.x-scenario/config/expectedData.yaml @@ -21,11 +21,11 @@ registryItems: operationNames: - spring-4.3.x-scenario: [/create/, '/delete/{id}', /inherit/child/test, /healthCheck, '/get/{id}', '/update/{id}', /case/resttemplate, - /case/spring3] + /case/spring3, /impl/requestmapping, /impl/restmapping] heartbeat: [] segmentItems: - applicationCode: spring-4.3.x-scenario - segmentSize: ge 8 + segmentSize: ge 10 segments: - segmentId: not null spans: @@ -222,6 +222,52 @@ segmentItems: entryEndpointId: 0, refType: CrossProcess, parentSpanId: 6, parentTraceSegmentId: not null, parentServiceInstanceId: 1, networkAddress: 'localhost:8080', entryEndpoint: /case/resttemplate, entryServiceInstanceId: 1} + - segmentId: not null + spans: + - operationName: /impl/requestmapping + operationId: 0 + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 14 + componentName: '' + isError: false + spanType: Entry + peer: '' + peerId: 0 + tags: + - {key: url, value: 'http://localhost:8080/spring-4.3.x-scenario/impl/requestmapping'} + - {key: http.method, value: GET} + refs: + - {parentEndpointId: 0, parentEndpoint: /case/resttemplate, networkAddressId: 0, + entryEndpointId: 0, refType: CrossProcess, parentSpanId: 7, parentTraceSegmentId: not null, + parentServiceInstanceId: 1, networkAddress: 'localhost:8080', entryEndpoint: /case/resttemplate, + entryServiceInstanceId: 1} + - segmentId: not null + spans: + - operationName: /impl/restmapping + operationId: 0 + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 14 + componentName: '' + isError: false + spanType: Entry + peer: '' + peerId: 0 + tags: + - {key: url, value: 'http://localhost:8080/spring-4.3.x-scenario/impl/restmapping'} + - {key: http.method, value: GET} + refs: + - {parentEndpointId: 0, parentEndpoint: /case/resttemplate, networkAddressId: 0, + entryEndpointId: 0, refType: CrossProcess, parentSpanId: 8, parentTraceSegmentId: not null, + parentServiceInstanceId: 1, networkAddress: 'localhost:8080', entryEndpoint: /case/resttemplate, + entryServiceInstanceId: 1} - segmentId: not null spans: - operationName: /spring-4.3.x-scenario/case/spring3/ @@ -320,6 +366,38 @@ segmentItems: tags: - {key: http.method, value: GET} - {key: url, value: 'http://localhost:8080/spring-4.3.x-scenario/inherit/child/test'} + - operationName: /spring-4.3.x-scenario/impl/requestmapping + operationId: 0 + parentSpanId: 0 + spanId: 7 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 12 + componentName: '' + isError: false + spanType: Exit + peer: localhost:8080 + peerId: 0 + tags: + - {key: http.method, value: GET} + - {key: url, value: 'http://localhost:8080/spring-4.3.x-scenario/impl/requestmapping'} + - operationName: /spring-4.3.x-scenario/impl/restmapping + operationId: 0 + parentSpanId: 0 + spanId: 8 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 12 + componentName: '' + isError: false + spanType: Exit + peer: localhost:8080 + peerId: 0 + tags: + - {key: http.method, value: GET} + - {key: url, value: 'http://localhost:8080/spring-4.3.x-scenario/impl/restmapping'} - operationName: /case/resttemplate operationId: 0 parentSpanId: -1 diff --git a/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseController.java b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseController.java new file mode 100644 index 000000000..62e1e0881 --- /dev/null +++ b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseController.java @@ -0,0 +1,38 @@ +/* + * + * 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 test.org.apache.skywalking.apm.testcase.implinterface; + +import org.springframework.web.bind.annotation.RestController; + +/** + * @author jialong + */ +@RestController +public class TestCaseController implements TestCaseInterface { + + @Override + public String implRequestMappingAnnotationTestCase() { + return "implRequestMappingAnnotationTestCase"; + } + + @Override + public String implRestAnnotationTestCase() { + return "implRestAnnotationTestCase"; + } +} diff --git a/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseInterface.java b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseInterface.java new file mode 100644 index 000000000..3cbe84faa --- /dev/null +++ b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/implinterface/TestCaseInterface.java @@ -0,0 +1,33 @@ +/* + * + * 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 test.org.apache.skywalking.apm.testcase.implinterface; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; + +/** + * @author jialong + */ +public interface TestCaseInterface { + @RequestMapping("/impl/requestmapping") + String implRequestMappingAnnotationTestCase(); + + @GetMapping("/impl/restmapping") + String implRestAnnotationTestCase(); +} diff --git a/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/resttemplate/RestTemplateController.java b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/resttemplate/RestTemplateController.java index bd5520328..9a830bbff 100644 --- a/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/resttemplate/RestTemplateController.java +++ b/test/plugin/scenarios/spring-4.3.x-scenario/src/main/java/test/org/apache/skywalking/apm/testcase/resttemplate/RestTemplateController.java @@ -66,6 +66,14 @@ public class RestTemplateController { response = new OkHttpClient().newCall(inheritRequest).execute(); logger.info(response.toString()); + Request implRequestMappingRequest = new Request.Builder().url(url + "/impl/requestmapping").build(); + response = new OkHttpClient().newCall(implRequestMappingRequest).execute(); + logger.info(response.toString()); + + Request implRestMappingRequest = new Request.Builder().url(url + "/impl/restmapping").build(); + response = new OkHttpClient().newCall(implRestMappingRequest).execute(); + logger.info(response.toString()); + return SUCCESS; }