Spring aop patch (#1118)

* spring aop patch #1114

* more strict check

apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/AopExpressionMatchInterceptor.java

+/*
+ * 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.patch;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.StaticMethodsAroundInterceptor;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * {@link AopExpressionMatchInterceptor} check if the method is match the enhanced method
+ * if yes,return false else return true;
+ *
+ * @author lican
+ */
+public class AopExpressionMatchInterceptor implements StaticMethodsAroundInterceptor {
+
+    private List<Method> methods = new ArrayList<Method>(2);
+
+    public AopExpressionMatchInterceptor() {
+        methods.addAll(Arrays.asList(EnhancedInstance.class.getDeclaredMethods()));
+    }
+
+    @Override
+    public void beforeMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes, MethodInterceptResult result) {
+
+    }
+
+    @Override
+    public Object afterMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes, Object ret) {
+        Method targetAopMethod = (Method) allArguments[1];
+        Class<?> targetAopClass = (Class<?>) allArguments[2];
+        if (EnhancedInstance.class.isAssignableFrom(targetAopClass) && isEnhancedMethod(targetAopMethod)) {
+            return false;
+        }
+        return ret;
+    }
+
+    @Override
+    public void handleMethodException(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes, Throwable t) {
+
+    }
+
+    private boolean isEnhancedMethod(Method targetMethod) {
+        for (Method method : methods) {
+            if (method.getName().equals(targetMethod.getName())
+                    && method.getReturnType().equals(targetMethod.getReturnType())
+                    && equalParamTypes(method.getParameterTypes(), targetMethod.getParameterTypes())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private boolean equalParamTypes(Class<?>[] params1, Class<?>[] params2) {
+        if (params1.length != params2.length) {
+            return false;
+        }
+        for (int i = 0; i < params1.length; i++) {
+            if (!params1[i].equals(params2[i])) {
+                return false;
+            }
+        }
+        return true;
+    }
+}

apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/define/AopExpressionMatchInstrumentation.java

+/*
+ * 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.patch.define;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassStaticMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+/**
+ * {@link AopExpressionMatchInstrumentation} indicates that exclude enhanced method in @{link EnhancedInstance} to prevent
+ * side effect when use intercept all method in controller to complete their business code
+ *
+ * @author lican
+ */
+public class AopExpressionMatchInstrumentation extends ClassStaticMethodsEnhancePluginDefine {
+
+    private  static final String ENHANCE_CLASS = "org.springframework.aop.support.MethodMatchers";
+    private  static final String ENHANCE_METHOD = "matches";
+    private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.spring.patch.AopExpressionMatchInterceptor";
+
+    @Override
+    protected final ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+        return new ConstructorInterceptPoint[0];
+    }
+
+    @Override
+    protected StaticMethodsInterceptPoint[] getStaticMethodsInterceptPoints() {
+        return new StaticMethodsInterceptPoint[]{new StaticMethodsInterceptPoint() {
+            @Override
+            public ElementMatcher<MethodDescription> getMethodsMatcher() {
+                return named(ENHANCE_METHOD);
+            }
+
+            @Override
+            public String getMethodsInterceptor() {
+                return INTERCEPT_CLASS;
+            }
+
+            @Override
+            public boolean isOverrideArgs() {
+                return false;
+            }
+        }
+        };
+    }
+
+    @Override
+    protected ClassMatch enhanceClass() {
+        return NameMatch.byName(ENHANCE_CLASS);
+    }
+
+}

apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/resources/skywalking-plugin.def

@@ -16,3 +16,4 @@
 
 spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AopProxyFactoryInstrumentation
 spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AutowiredAnnotationProcessorInstrumentation
+spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AopExpressionMatchInstrumentation

apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/test/java/org/apache/skywalking/apm/plugin/spring/patch/AopExpressionMatchInterceptorTest.java

+/*
+ * 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.patch;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.aop.MethodMatcher;
+import org.springframework.util.ReflectionUtils;
+
+/**
+ * @author lican
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class AopExpressionMatchInterceptorTest {
+
+
+    private AopExpressionMatchInterceptor interceptor;
+    @Mock
+    private MethodMatcher methodMatcher;
+
+    @Before
+    public void setUp() throws Exception {
+        interceptor = new AopExpressionMatchInterceptor();
+    }
+
+    @Test
+    public void beforeMethod() {
+    }
+
+    @Test
+    public void afterMethod() {
+        Object ret = interceptor.afterMethod(
+                Object.class,
+                null,
+                new Object[]{methodMatcher, ReflectionUtils.findMethod(MockClass.class, "getSkyWalkingDynamicField"), MockClass.class, false},
+                new Class[0],
+                true);
+        Assert.assertEquals(false, ret);
+    }
+
+    @Test
+    public void handleMethodException() {
+    }
+
+    private class MockClass implements EnhancedInstance {
+
+        @Override
+        public Object getSkyWalkingDynamicField() {
+            return null;
+        }
+
+        @Override
+        public void setSkyWalkingDynamicField(Object value) {
+
+        }
+    }
+}
\ No newline at end of file