Skip to content

Commit c3a6fcb

Browse files
tjuwangningtjuwangning
authored
Fix enhanced instance causes Spring Application startup failure (#762)
Root cause: SkyWalking's enhancement changes how Spring decides which proxy type to use. As mentioned in [#13221](apache/skywalking#13221 (comment)), SkyWalking modifies Spring's `hasNoUserSuppliedProxyInterfaces()` to avoid affecting the choice of proxy type. But once Spring Retry has added `Retryable` to the list of interfaces, `hasNoUserSuppliedProxyInterfaces()` will find a user-supplied interface and return false, which changes the proxy type and leads to a startup failure. Starting in 4.0.2.RELEASE, Spring adds a new hook method `evaluateProxyInterfaces()`, which allows proxy-related flags (like proxyTargetClass) to be determined earlier in the lifecycle. We also need to enhance `evaluateProxyInterfaces()` (and its related callbacks) so that SkyWalking's EnhancedInstance interface is always ignored.
1 parent 4bb2c2f commit c3a6fcb

File tree

9 files changed

+313
-0
lines changed

9 files changed

+313
-0
lines changed

‎CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ Release Notes.
1717
* Support for tracking in lettuce versions 6.5.x and above.
1818
* Upgrade byte-buddy version to 1.17.6.
1919
* Support gRPC 1.59.x and 1.70.x server interceptor trace
20+
* Fix the `CreateAopProxyInterceptor` in the Spring core-patch changes the AOP proxy type when a class is
21+
enhanced by both SkyWalking and Spring AOP.
2022

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

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.spring.patch;
20+
21+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
22+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
23+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
24+
25+
import java.lang.reflect.Method;
26+
27+
/**
28+
* <code>AutoProxyCreatorInterceptor</code> determines whether the given interface is {@link EnhancedInstance},
29+
* and therefore does not consider it a reasonable proxy interface.
30+
*/
31+
public class AutoProxyCreatorInterceptor implements InstanceMethodsAroundInterceptor {
32+
33+
@Override
34+
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
35+
MethodInterceptResult result) throws Throwable {
36+
37+
}
38+
39+
@Override
40+
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
41+
Object ret) throws Throwable {
42+
43+
Class<?> ifc = (Class<?>) allArguments[0];
44+
return ((boolean) ret) || ifc.getName().equals("org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance");
45+
}
46+
47+
@Override
48+
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
49+
Class<?>[] argumentsTypes, Throwable t) {
50+
51+
}
52+
53+
}

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.spring.patch;
20+
21+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
22+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
23+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
24+
25+
import java.lang.reflect.Method;
26+
27+
/**
28+
* <code>ProxyProcessorSupportInterceptor</code> determines whether the given interface is {@link EnhancedInstance},
29+
* and therefore does not consider it a reasonable proxy interface.
30+
*/
31+
public class ProxyProcessorSupportInterceptor implements InstanceMethodsAroundInterceptor {
32+
33+
@Override
34+
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
35+
MethodInterceptResult result) throws Throwable {
36+
37+
}
38+
39+
@Override
40+
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
41+
Object ret) throws Throwable {
42+
Class<?> ifc = (Class<?>) allArguments[0];
43+
return ((boolean) ret) || ifc.getName().equals("org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance");
44+
}
45+
46+
@Override
47+
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
48+
Class<?>[] argumentsTypes, Throwable t) {
49+
50+
}
51+
52+
}

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.spring.patch.define;
20+
21+
import net.bytebuddy.description.method.MethodDescription;
22+
import net.bytebuddy.matcher.ElementMatcher;
23+
import org.apache.skywalking.apm.agent.core.plugin.WitnessMethod;
24+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
25+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
26+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
27+
import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
28+
import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
29+
30+
import java.util.Collections;
31+
import java.util.List;
32+
33+
import static net.bytebuddy.matcher.ElementMatchers.named;
34+
35+
public class AutoProxyCreatorInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
36+
37+
private static final String ENHANCE_CLASS = "org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator";
38+
public static final String ENHANCE_METHOD = "isConfigurationCallbackInterface";
39+
public static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.spring.patch.AutoProxyCreatorInterceptor";
40+
41+
@Override
42+
public final ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
43+
return new ConstructorInterceptPoint[0];
44+
}
45+
46+
@Override
47+
public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
48+
return new InstanceMethodsInterceptPoint[] {
49+
new InstanceMethodsInterceptPoint() {
50+
@Override
51+
public ElementMatcher<MethodDescription> getMethodsMatcher() {
52+
return named(ENHANCE_METHOD);
53+
}
54+
55+
@Override
56+
public String getMethodsInterceptor() {
57+
return INTERCEPT_CLASS;
58+
}
59+
60+
@Override
61+
public boolean isOverrideArgs() {
62+
return false;
63+
}
64+
}
65+
};
66+
}
67+
68+
@Override
69+
protected ClassMatch enhanceClass() {
70+
return NameMatch.byName(ENHANCE_CLASS);
71+
}
72+
73+
@Override
74+
protected List<WitnessMethod> witnessMethods() {
75+
return Collections.singletonList(new WitnessMethod(ENHANCE_CLASS, named(ENHANCE_METHOD)));
76+
}
77+
78+
}

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.spring.patch.define;
20+
21+
import net.bytebuddy.description.method.MethodDescription;
22+
import net.bytebuddy.matcher.ElementMatcher;
23+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
24+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
25+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
26+
import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
27+
import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
28+
29+
import static net.bytebuddy.matcher.ElementMatchers.named;
30+
31+
public class ProxyProcessorSupportInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
32+
33+
private static final String ENHANCE_CLASS = "org.springframework.aop.framework.ProxyProcessorSupport";
34+
public static final String ENHANCE_METHOD = "isInternalLanguageInterface";
35+
public static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.spring.patch.ProxyProcessorSupportInterceptor";
36+
37+
@Override
38+
public final ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
39+
return new ConstructorInterceptPoint[0];
40+
}
41+
42+
@Override
43+
public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
44+
return new InstanceMethodsInterceptPoint[] {
45+
new InstanceMethodsInterceptPoint() {
46+
@Override
47+
public ElementMatcher<MethodDescription> getMethodsMatcher() {
48+
return named(ENHANCE_METHOD);
49+
}
50+
51+
@Override
52+
public String getMethodsInterceptor() {
53+
return INTERCEPT_CLASS;
54+
}
55+
56+
@Override
57+
public boolean isOverrideArgs() {
58+
return false;
59+
}
60+
}
61+
};
62+
}
63+
64+
@Override
65+
protected ClassMatch enhanceClass() {
66+
return NameMatch.byName(ENHANCE_CLASS);
67+
}
68+
69+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,5 @@ spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.Autowired
1919
spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AopExpressionMatchInstrumentation
2020
spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AspectJExpressionPointCutInstrumentation
2121
spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.BeanWrapperImplInstrumentation
22+
spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.ProxyProcessorSupportInstrumentation
23+
spring-core-patch=org.apache.skywalking.apm.plugin.spring.patch.define.AutoProxyCreatorInstrumentation

‎test/plugin/scenarios/spring-4.1.x-scenario/pom.xml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
<maven-compiler-plugin.version>3.8.1</maven-compiler-plugin.version>
3434
<test.framework.version>4.1.0.RELEASE</test.framework.version>
3535
<test.framework>spring</test.framework>
36+
<apm-toolkit-trace.version>9.4.0</apm-toolkit-trace.version>
3637
</properties>
3738

3839
<name>skywalking-spring-4.1.x-scenario</name>
@@ -86,6 +87,26 @@
8687
<artifactId>log4j-core</artifactId>
8788
<version>2.8.1</version>
8889
</dependency>
90+
<dependency>
91+
<groupId>org.springframework</groupId>
92+
<artifactId>spring-aop</artifactId>
93+
<version>${test.framework.version}</version>
94+
</dependency>
95+
<dependency>
96+
<groupId>org.springframework</groupId>
97+
<artifactId>spring-aspects</artifactId>
98+
<version>${test.framework.version}</version>
99+
</dependency>
100+
<dependency>
101+
<groupId>org.springframework.retry</groupId>
102+
<artifactId>spring-retry</artifactId>
103+
<version>1.2.5.RELEASE</version>
104+
</dependency>
105+
<dependency>
106+
<groupId>org.apache.skywalking</groupId>
107+
<artifactId>apm-toolkit-trace</artifactId>
108+
<version>${apm-toolkit-trace.version}</version>
109+
</dependency>
89110
</dependencies>
90111

91112
<build>

‎test/plugin/scenarios/spring-4.1.x-scenario/src/main/java/test/apache/skywalking/apm/testcase/spring3/config/RetryConfig.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package test.apache.skywalking.apm.testcase.spring3.config;
20+
21+
import org.springframework.context.annotation.Configuration;
22+
import org.springframework.retry.annotation.EnableRetry;
23+
24+
@Configuration
25+
@EnableRetry
26+
public class RetryConfig {
27+
}

‎test/plugin/scenarios/spring-4.1.x-scenario/src/main/java/test/apache/skywalking/apm/testcase/spring3/service/TestServiceBean.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818

1919
package test.apache.skywalking.apm.testcase.spring3.service;
2020

21+
import org.apache.skywalking.apm.toolkit.trace.Trace;
2122
import org.springframework.beans.factory.annotation.Autowired;
23+
import org.springframework.retry.annotation.Backoff;
24+
import org.springframework.retry.annotation.Retryable;
2225
import org.springframework.stereotype.Service;
2326
import test.apache.skywalking.apm.testcase.spring3.component.TestComponentBean;
2427
import test.apache.skywalking.apm.testcase.spring3.dao.TestRepositoryBean;
@@ -35,4 +38,10 @@ public void doSomeBusiness(String name) {
3538
componentBean.componentMethod(name);
3639
repositoryBean.doSomeStuff(name);
3740
}
41+
42+
// Test the class is enhanced by both SkyWalking and Spring AOP
43+
@Retryable(value = Exception.class, backoff = @Backoff(delay = 1000, multiplier = 2))
44+
@Trace
45+
private void doRetry() {
46+
}
3847
}

0 commit comments

Comments
 (0)