Skip to content

Commit d8192e5

Browse files
authored
Merge branch 'main' into feature_support_gateway_filter_trace_id
2 parents 231d5ca + 26600e7 commit d8192e5

File tree

3 files changed

+164
-12
lines changed

3 files changed

+164
-12
lines changed

CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Release Notes.
1515
* Add empty judgment for constructorInterceptPoint
1616
* Bump up gRPC to 1.68.1
1717
* Bump up netty to 4.1.115.Final
18+
* Fix the `CreateAopProxyInterceptor` in the Spring core-patch to prevent it from changing the implementation of the Spring AOP proxy
1819
* Support Tracing for GlobalFilter and GatewayFilter in Spring Gateway
1920

2021
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/222?closed=1)

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

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818

1919
package org.apache.skywalking.apm.plugin.spring.patch;
2020

21-
import java.lang.reflect.Method;
2221
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
2322
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
2423
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
24+
import org.springframework.aop.SpringProxy;
2525
import org.springframework.aop.framework.AdvisedSupport;
2626

27+
import java.lang.reflect.Method;
28+
2729
/**
2830
* <code>CreateAopProxyInterceptor</code> check that the bean has been implement {@link EnhancedInstance}.
2931
* if yes, true will be returned.
@@ -32,25 +34,45 @@ public class CreateAopProxyInterceptor implements InstanceMethodsAroundIntercept
3234

3335
@Override
3436
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
35-
MethodInterceptResult result) throws Throwable {
37+
MethodInterceptResult result) throws Throwable {
3638

3739
}
3840

3941
@Override
4042
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
41-
Object ret) throws Throwable {
43+
Object ret) throws Throwable {
4244
AdvisedSupport advisedSupport = (AdvisedSupport) allArguments[0];
4345

44-
Class targetClass = advisedSupport.getTargetClass();
45-
if (targetClass != null && EnhancedInstance.class.isAssignableFrom(targetClass)) {
46-
return true;
46+
if (maybeHasUserSuppliedProxyInterfaces(ret)) {
47+
Class targetClass = advisedSupport.getTargetClass();
48+
if (targetClass != null) {
49+
if (onlyImplementsEnhancedInstance(advisedSupport) || onlyImplementsEnhancedInstanceAndSpringProxy(advisedSupport)) {
50+
return true;
51+
}
52+
}
4753
}
4854
return ret;
4955
}
5056

57+
private boolean maybeHasUserSuppliedProxyInterfaces(Object ret) {
58+
return !(Boolean) ret;
59+
}
60+
61+
private boolean onlyImplementsEnhancedInstanceAndSpringProxy(AdvisedSupport advisedSupport) {
62+
Class<?>[] ifcs = advisedSupport.getProxiedInterfaces();
63+
Class targetClass = advisedSupport.getTargetClass();
64+
return ifcs.length == 2 && EnhancedInstance.class.isAssignableFrom(targetClass) && SpringProxy.class.isAssignableFrom(targetClass);
65+
}
66+
67+
private boolean onlyImplementsEnhancedInstance(AdvisedSupport advisedSupport) {
68+
Class<?>[] ifcs = advisedSupport.getProxiedInterfaces();
69+
Class targetClass = advisedSupport.getTargetClass();
70+
return ifcs.length == 1 && EnhancedInstance.class.isAssignableFrom(targetClass);
71+
}
72+
5173
@Override
5274
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
53-
Class<?>[] argumentsTypes, Throwable t) {
75+
Class<?>[] argumentsTypes, Throwable t) {
5476

5577
}
5678
}

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

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.junit.runner.RunWith;
2525
import org.mockito.Mock;
2626
import org.mockito.junit.MockitoJUnitRunner;
27+
import org.springframework.aop.SpringProxy;
2728
import org.springframework.aop.framework.AdvisedSupport;
2829

2930
import static org.hamcrest.core.Is.is;
@@ -48,18 +49,69 @@ public void setUp() {
4849
}
4950

5051
@Test
51-
public void testInterceptNormalObject() throws Throwable {
52-
doReturn(Object.class).when(advisedSupport).getTargetClass();
52+
public void testInterceptClassImplementsNoInterfaces() throws Throwable {
53+
// doReturn(Object.class).when(advisedSupport).getTargetClass();
54+
// doReturn(Object.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
55+
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, true)));
56+
}
57+
58+
@Test
59+
public void testInterceptClassImplementsUserSuppliedInterface() throws Throwable {
60+
doReturn(MockClassImplementsUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
61+
doReturn(MockClassImplementsUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
5362
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
5463
}
5564

5665
@Test
57-
public void testInterceptEnhanceInstanceObject() throws Throwable {
58-
doReturn(MockClass.class).when(advisedSupport).getTargetClass();
66+
public void testInterceptClassImplementsSpringProxy() throws Throwable {
67+
// doReturn(MockClassImplementsSpringProxy.class).when(advisedSupport).getTargetClass();
68+
// doReturn(MockClassImplementsSpringProxy.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
69+
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, true)));
70+
}
71+
72+
@Test
73+
public void testInterceptClassImplementsEnhancedInstance() throws Throwable {
74+
doReturn(MockClassImplementsEnhancedInstance.class).when(advisedSupport).getTargetClass();
75+
doReturn(MockClassImplementsEnhancedInstance.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
5976
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
6077
}
6178

62-
private class MockClass implements EnhancedInstance {
79+
@Test
80+
public void testClassImplementsEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
81+
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
82+
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
83+
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
84+
}
85+
86+
@Test
87+
public void testInterceptClassImplementsSpringProxyAndEnhancedInstance() throws Throwable {
88+
doReturn(MockClassImplementsSpringProxyAndEnhancedInstance.class).when(advisedSupport).getTargetClass();
89+
doReturn(MockClassImplementsSpringProxyAndEnhancedInstance.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
90+
assertThat(true, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
91+
}
92+
93+
@Test
94+
public void testInterceptClassImplementsSpringProxyAndUserSuppliedInterface() throws Throwable {
95+
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
96+
doReturn(MockClassImplementsSpringProxyAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
97+
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
98+
}
99+
100+
@Test
101+
public void testInterceptClassImplementsEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
102+
doReturn(MockClassImplementsEnhancedInstanceAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
103+
doReturn(MockClassImplementsEnhancedInstanceAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
104+
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
105+
}
106+
107+
@Test
108+
public void testInterceptClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface() throws Throwable {
109+
doReturn(MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface.class).when(advisedSupport).getTargetClass();
110+
doReturn(MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface.class.getInterfaces()).when(advisedSupport).getProxiedInterfaces();
111+
assertThat(false, is(interceptor.afterMethod(enhancedInstance, null, new Object[] {advisedSupport}, new Class[] {Object.class}, false)));
112+
}
113+
114+
private class MockClassImplementsEnhancedInstance implements EnhancedInstance {
63115

64116
@Override
65117
public Object getSkyWalkingDynamicField() {
@@ -72,4 +124,81 @@ public void setSkyWalkingDynamicField(Object value) {
72124
}
73125
}
74126

127+
private class MockClassImplementsUserSuppliedInterface implements UserSuppliedInterface {
128+
129+
@Override
130+
public void methodOfUserSuppliedInterface() {
131+
132+
}
133+
134+
}
135+
136+
private class MockClassImplementsSpringProxy implements SpringProxy {
137+
138+
}
139+
140+
private class MockClassImplementsSpringProxyAndEnhancedInstance implements EnhancedInstance, SpringProxy {
141+
142+
@Override
143+
public Object getSkyWalkingDynamicField() {
144+
return null;
145+
}
146+
147+
@Override
148+
public void setSkyWalkingDynamicField(Object value) {
149+
150+
}
151+
152+
}
153+
154+
private class MockClassImplementsEnhancedInstanceAndUserSuppliedInterface implements EnhancedInstance, UserSuppliedInterface {
155+
156+
@Override
157+
public Object getSkyWalkingDynamicField() {
158+
return null;
159+
}
160+
161+
@Override
162+
public void setSkyWalkingDynamicField(Object value) {
163+
164+
}
165+
166+
@Override
167+
public void methodOfUserSuppliedInterface() {
168+
}
169+
170+
}
171+
172+
private class MockClassImplementsSpringProxyAndUserSuppliedInterface implements SpringProxy, UserSuppliedInterface {
173+
174+
@Override
175+
public void methodOfUserSuppliedInterface() {
176+
}
177+
178+
}
179+
180+
private class MockClassImplementsSpringProxyAndEnhancedInstanceAndUserSuppliedInterface implements EnhancedInstance, SpringProxy, UserSuppliedInterface {
181+
182+
@Override
183+
public Object getSkyWalkingDynamicField() {
184+
return null;
185+
}
186+
187+
@Override
188+
public void setSkyWalkingDynamicField(Object value) {
189+
190+
}
191+
192+
@Override
193+
public void methodOfUserSuppliedInterface() {
194+
}
195+
196+
}
197+
198+
interface UserSuppliedInterface {
199+
200+
void methodOfUserSuppliedInterface();
201+
202+
}
203+
75204
}

0 commit comments

Comments
 (0)