Fix enhanced instance causes Spring Application startup failure by tjuwangning · Pull Request #762 · apache/skywalking-java

tjuwangning · 2025-07-10T01:51:12Z

Fix apache/skywalking#13221

Add a unit test to verify that the fix works.
Explain briefly why the bug exists and how to fix it.

After Spring Framework 4.0.2.RELEASE, if we use a Spring Advisor to add an interface to a class and that same class is also enhanced by SkyWalking, Spring will fail to start.

Reproduce (using spring-retry as an example):

Enable Spring Retry by annotating a configuration with @EnableRetry and the method with @Retryable, the Spring Retry's annotations cause the bean class to implement the Retryable interface at runtime.
If SkyWalking has also instrumented the same class, Spring's startup will abort with an exception.

Root cause:
SkyWalking's enhancement changes how Spring decides which proxy type to use. As mentioned in #13221, 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.

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [Bug] Use @Transcational annotation with enhanced instance causes Spring Application startup failure skywalking#13221
Update the CHANGES log.

wu-sheng · 2025-07-10T05:07:29Z

I am on vacation, can't review this until 26th.
Please make CI passed and polish codes as much as you can.

wu-sheng · 2025-07-26T11:44:19Z

@lujiajing1126 Is this a case you are familiar with?

Copilot

Pull Request Overview

This PR fixes a Spring Application startup failure caused by enhanced instances in the SkyWalking agent. The fix introduces interceptors to handle enhanced instances during Spring's proxy creation process, preventing startup failures when beans have been enhanced by the agent.

Key changes:

Adds interceptors for Spring's ProxyProcessorSupport and AbstractAutoProxyCreator classes
Creates a test scenario for Spring Retry functionality to validate the fix
Updates the CI configuration to include the new test scenario

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/resources/skywalking-plugin.def`	Registers two new interceptors for Spring proxy creation
`apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/define/ProxyProcessorSupportInstrumentation.java`	Instrumentation for ProxyProcessorSupport class
`apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/define/AutoProxyCreatorInstrumentation.java`	Instrumentation for AbstractAutoProxyCreator class
`apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/ProxyProcessorSupportInterceptor.java`	Interceptor to handle EnhancedInstance in proxy processing
`apm-sniffer/apm-sdk-plugin/spring-plugins/core-patch/src/main/java/org/apache/skywalking/apm/plugin/spring/patch/AutoProxyCreatorInterceptor.java`	Interceptor to handle EnhancedInstance in auto proxy creation
`test/plugin/scenarios/spring-retry-scenario/`	Complete test scenario to validate the fix with Spring Retry
`.github/workflows/plugins-test.2.yaml`	Updates CI to include the new test scenario

...ain/java/org/apache/skywalking/apm/plugin/spring/patch/ProxyProcessorSupportInterceptor.java

...src/main/java/org/apache/skywalking/apm/plugin/spring/patch/AutoProxyCreatorInterceptor.java

...try-scenario/src/main/java/test/apache/skywalking/apm/testcase/spring/retry/Application.java

wu-sheng · 2025-07-27T02:33:32Z

Please update supported-list.d doc to add Spring retry support?

wu-sheng · 2025-07-27T14:35:19Z

test/plugin/scenarios/spring-retry-scenario/config/expectedData.yaml

+            spanType: Local
+            peer: ''
+            skipAnalysis: false
+          - operationName: HEAD:/case/healthCheck


This is not required from the expectation file.

wu-sheng · 2025-07-27T14:35:52Z

test/plugin/scenarios/spring-retry-scenario/config/expectedData.yaml

+    segments:
+      - segmentId: not null
+        spans:
+          - operationName: test.apache.skywalking.apm.testcase.spring.retry.service.CaseService.handle()


There is no client span and I can't see the retry working in the expectation file.

I think you should enhance your test scenario.

lujiajing1126 · 2025-07-28T01:24:52Z

@lujiajing1126 Is this a case you are familiar with?

@tjuwangning Ning Wang also works in our team. We've already discussed this patch offline.

I suppose he has to add some more detailed technical comments for this PR and explain better how the test case works.

wu-sheng · 2025-07-28T01:26:18Z

Yes, please. I am a little confused about reviewing this test scenario.

lujiajing1126 · 2025-07-28T01:33:26Z

Please update supported-list.d doc to add Spring retry support?

Actually, the original issue can be reproduced in any case that an additional interface is added for the original class. In the spring-retry case, it is the marker interface Retryable

wu-sheng · 2025-07-28T01:49:11Z

Actually, the original issue can be reproduced in any case that an additional interface is added for the original class. In the spring-retry case, it is the marker interface Retryable

Is this error caused by adding retry annotation, or retry actually happened?

tjuwangning · 2025-07-28T04:08:58Z

Actually, the original issue can be reproduced in any case that an additional interface is added for the original class. In the spring-retry case, it is the marker interface Retryable

Is this error caused by adding retry annotation, or retry actually happened?

Spring cannot startup when add the retry annotation.
I hava revised the PR description to explain the issue more clearly. Furthermore, the current name of the test scenario is somewhat ambiguous, I will rename it later.

wu-sheng · 2025-07-28T04:39:31Z

Then, I think you just need to add this retry annotation to existing scenario.

wu-sheng · 2025-07-28T12:17:33Z

...src/main/java/org/apache/skywalking/apm/plugin/spring/patch/AutoProxyCreatorInterceptor.java

+import java.lang.reflect.Method;
+
+/**
+ * <code>AutoProxyCreatorInterceptor</code> check that the bean has been implement {@link EnhancedInstance}.


I am clear about your new description, but a little confused about this comment word.
I think the original method is returning true/false, and we are just checking EnhancedInstance implementation when the original return is false.
When the original return is false? We should write this comment according to #isConfigurationCallbackInterface purpose, right?

Yes, I have updated it.

wu-sheng · 2025-07-28T12:31:59Z

And please update changes.md

wu-sheng · 2025-07-29T02:16:12Z

...x-scenario/src/main/java/test/apache/skywalking/apm/testcase/spring3/config/RetryConfig.java

@@ -0,0 +1,9 @@
+package test.apache.skywalking.apm.testcase.spring3.config;


You missed the APLv2 header.

wang added 8 commits May 27, 2025 09:35

fix parent class enhance issue

f4b0c0c

update CHANGES log

61343ce

set the getter/setter method name as a constant

a7d419a

fix spring aop proxy change

507b366

merge upstream

4f1b6c4

add license header

0735c03

rename package name

40ab21b

remove system print

b1f1768

wu-sheng added enhancement New feature or request plugin labels Jul 10, 2025

wu-sheng added this to the 9.5.0 milestone Jul 10, 2025

lujiajing1126 assigned tjuwangning Jul 10, 2025

wang and others added 2 commits July 10, 2025 14:24

update operationName in expectedData.yaml

8e54734

Merge branch 'main' into main

5bf120e

wu-sheng requested a review from Copilot July 26, 2025 11:44

Copilot AI reviewed Jul 26, 2025

View reviewed changes

wu-sheng linked an issue Jul 27, 2025 that may be closed by this pull request

[Bug] Use @Transcational annotation with enhanced instance causes Spring Application startup failure apache/skywalking#13221

Closed

3 tasks

wu-sheng mentioned this pull request Jul 27, 2025

Add support gRPC 1.59.x and 1.70.x server interceptor trace #764

Merged

wu-sheng reviewed Jul 27, 2025

View reviewed changes

update test scenario

6253528

Merge remote-tracking branch 'origin/main'

c8719e1

wu-sheng reviewed Jul 28, 2025

View reviewed changes

Merge branch 'main' into main

452d51f

update comment word

232ea16

wu-sheng reviewed Jul 29, 2025

View reviewed changes

add license header

547bff2

wu-sheng approved these changes Jul 29, 2025

View reviewed changes

wu-sheng merged commit c3a6fcb into apache:main Jul 29, 2025
198 checks passed

		@@ -0,0 +1,9 @@
		package test.apache.skywalking.apm.testcase.spring3.config;

Conversation

tjuwangning commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix apache/skywalking#13221

Uh oh!

wu-sheng commented Jul 10, 2025

Uh oh!

wu-sheng commented Jul 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wu-sheng commented Jul 27, 2025

Uh oh!

wu-sheng Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

wu-sheng Jul 27, 2025

Choose a reason for hiding this comment

Uh oh!

lujiajing1126 commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wu-sheng commented Jul 28, 2025

Uh oh!

lujiajing1126 commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wu-sheng commented Jul 28, 2025

Uh oh!

tjuwangning commented Jul 28, 2025

Uh oh!

wu-sheng commented Jul 28, 2025

Uh oh!

wu-sheng Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

tjuwangning Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

wu-sheng commented Jul 28, 2025

Uh oh!

wu-sheng Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tjuwangning commented Jul 10, 2025 •

edited

Loading

lujiajing1126 commented Jul 28, 2025 •

edited

Loading

lujiajing1126 commented Jul 28, 2025 •

edited

Loading