kazusa
/
skywalking-java
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CreateAopProxyInterceptor in the Spring core-patch indeed changes the implementation of the Spring AOP proxy (#739)

This commit is contained in:
youjie23 2024-12-15 19:26:40 +08:00 committed by GitHub
parent 18e971c48e
commit 26600e76e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 164 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.md
View File

@ -15,6 +15,7 @@ Release Notes.
* Add empty judgment for constructorInterceptPoint
* Bump up gRPC to 1.68.1
* Bump up netty to 4.1.115.Final
* Fix the `CreateAopProxyInterceptor` in the Spring core-patch to prevent it from changing the implementation of the Spring AOP proxy
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/222?closed=1)

36
apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/CreateAopProxyInterceptor.java
View File

@ -18,12 +18,14 @@
package org.apache.skywalking.apm.plugin.spring.patch;
import java.lang.reflect.Method;
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.springframework.aop.SpringProxy;
import org.springframework.aop.framework.AdvisedSupport;
import java.lang.reflect.Method;
/**
* <code>CreateAopProxyInterceptor</code> check that the bean has been implement {@link EnhancedInstance}.
* if yes, true will be returned.
@ -32,25 +34,45 @@ public class CreateAopProxyInterceptor implements InstanceMethodsAroundIntercept
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
MethodInterceptResult result) throws Throwable {
MethodInterceptResult result) throws Throwable {
}
@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
Object ret) throws Throwable {
AdvisedSupport advisedSupport = (AdvisedSupport) allArguments[0];
Class targetClass = advisedSupport.getTargetClass();
if (targetClass != null && EnhancedInstance.class.isAssignableFrom(targetClass)) {
return true;
if (maybeHasUserSuppliedProxyInterfaces(ret)) {
Class targetClass = advisedSupport.getTargetClass();
if (targetClass != null) {
if (onlyImplementsEnhancedInstance(advisedSupport) || onlyImplementsEnhancedInstanceAndSpringProxy(advisedSupport)) {
return true;
}
}
}
return ret;
}
private boolean maybeHasUserSuppliedProxyInterfaces(Object ret) {
return !(Boolean) ret;
}
private boolean onlyImplementsEnhancedInstanceAndSpringProxy(AdvisedSupport advisedSupport) {
Class<?>[] ifcs = advisedSupport.getProxiedInterfaces();
Class targetClass = advisedSupport.getTargetClass();
return ifcs.length == 2 && EnhancedInstance.class.isAssignableFrom(targetClass) && SpringProxy.class.isAssignableFrom(targetClass);
}
private boolean onlyImplementsEnhancedInstance(AdvisedSupport advisedSupport) {
Class<?>[] ifcs = advisedSupport.getProxiedInterfaces();
Class targetClass = advisedSupport.getTargetClass();
return ifcs.length == 1 && EnhancedInstance.class.isAssignableFrom(targetClass);
}
@Override
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
Class<?>[] argumentsTypes, Throwable t) {
}
}

139
apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/test/java/org/apache/skywalking/apm/plugin/spring/patch/CreateAopProxyInterceptorTest.java
View File

@ -24,6 +24,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.aop.SpringProxy;
import org.springframework.aop.framework.AdvisedSupport;
import static org.hamcrest.core.Is.is;
@ -48,18 +49,69 @@ public class CreateAopProxyInterceptorTest {
}
@Test
public void testInterceptNormalObject() throws Throwable {
doReturn(Object.class).when(advisedSupport).getTargetClass();
public void testInterceptClassImplementsNoInterfaces() throws Throwable {
// doReturn(Object.class).when(advisedSupport).getTargetClass();
// doReturn(Object.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, true)));
}
@Test
public void testInterceptClassImplementsUserSuppliedInterface() throws Throwable {
doReturn(MockClassImplementsUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
@Test
public void testInterceptEnhanceInstanceObject() throws Throwable {
doReturn(MockClass.class).when(advisedSupport).getTargetClass();
public void testInterceptClassImplementsSpringProxy() throws Throwable {
// doReturn(MockClassImplementsSpringProxy.class).when(advisedSupport).getTargetClass();
// doReturn(MockClassImplementsSpringProxy.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, true)));
}
@Test
public void testInterceptClassImplementsEnhancedInstance() throws Throwable {
doReturn(MockClassImplementsEnhancedInstance.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsEnhancedInstance.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
private class MockClass implements EnhancedInstance {
@Test
public void testClassImplementsEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
@Test
public void testInterceptClassImplementsSpringProxyAndEnhancedInstance() throws Throwable {
doReturn(MockClassImplementsSpringProxyAndEnhancedInstance.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsSpringProxyAndEnhancedInstance.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
@Test
public void testInterceptClassImplementsSpringProxyAndUserSuppliedInterface() throws Throwable {
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
@Test
public void testInterceptClassImplementsEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
doReturn(MockClassImplementsEnhancedInstanceAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsEnhancedInstanceAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
@Test
public void testInterceptClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
doReturn(MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
doReturn(MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
}
private class MockClassImplementsEnhancedInstance implements EnhancedInstance {
@Override
public Object getSkyWalkingDynamicField() {
@ -72,4 +124,81 @@ public class CreateAopProxyInterceptorTest {
}
}
private class MockClassImplementsUserSuppliedInterface implements UserSuppliedInterface {
@Override
public void methodOfUserSuppliedInterface() {
}
}
private class MockClassImplementsSpringProxy implements SpringProxy {
}
private class MockClassImplementsSpringProxyAndEnhancedInstance implements EnhancedInstance, SpringProxy {
@Override
public Object getSkyWalkingDynamicField() {
return null;
}
@Override
public void setSkyWalkingDynamicField(Object value) {
}
}
private class MockClassImplementsEnhancedInstanceAndUserSuppliedInterface implements EnhancedInstance, UserSuppliedInterface {
@Override
public Object getSkyWalkingDynamicField() {
return null;
}
@Override
public void setSkyWalkingDynamicField(Object value) {
}
@Override
public void methodOfUserSuppliedInterface() {
}
}
private class MockClassImplementsSpringProxyAndUserSuppliedInterface implements SpringProxy, UserSuppliedInterface {
@Override
public void methodOfUserSuppliedInterface() {
}
}
private class MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface implements EnhancedInstance, SpringProxy, UserSuppliedInterface {
@Override
public Object getSkyWalkingDynamicField() {
return null;
}
@Override
public void setSkyWalkingDynamicField(Object value) {
}
@Override
public void methodOfUserSuppliedInterface() {
}
}
interface UserSuppliedInterface {
void methodOfUserSuppliedInterface();
}
}