[Bug] Use @Transcational annotation with enhanced instance causes Spring Application startup failure

[lujiajing1126]

Search before asking

• I had searched in the issues and found no similar issues.

Apache SkyWalking Component

Java Agent (apache/skywalking-java)

What happened

https://github.com/apache/skywalking-java/pull/739/files#r2068184158

What you expected to happen

The application can start normally.

How to reproduce

A minimal example to reproduce the issue, from the example code shown in this SO question.

@SpringBootApplication
@EnableTransactionManagement
public class StackoverflowApplication {

    @Autowired private SomeService service;

    public static void main(String[] args) {
        SpringApplication.run(StackoverflowApplication.class, args);
    }
}

@Service
class SomeService {
    
    @Override
    @Transactional
    public void handle() { }

    @Trace // Use Trace annotation in order to let SkyWalking Java Agent to enhance this class
    public void methodToBeTraced() {}
}

Anything else

No response

Are you willing to submit a pull request to fix on your own?

• Yes I am willing to submit a pull request on my own!

Code of Conduct

• I agree to follow this project's Code of Conduct

[wu-sheng]

Are you going o fix this?

[lujiajing1126]

Are you going o fix this?

Yes. We are investigating this issue. We hope to resolve soon after holidays.

[Ricehomesky]

As the result of #581 and #12858 , Skywalking add an interceptor on method hasNoUserSuppliedProxyInterfaces to prevent the proxy type from being incorrectly modified after enhancement by SkyWalking agent.
The key code of proxy type's chosen is as follows:
DefaultAopProxyFactory.java

public AopProxy createAopProxy(AdvisedSupport config) throws AopConfigException {                          
	if (config.isOptimize() || config.isProxyTargetClass() || hasNoUserSuppliedProxyInterfaces(config)) {  
		Class targetClass = config.getTargetClass();                                                       
		if (targetClass == null) {                                                                         
			throw new AopConfigException("TargetSource cannot determine target class: " +
					"Either an interface or a target is required for proxy creation.");                    
		}                                                                                                  
		if (targetClass.isInterface()) {                                                                   
			return new JdkDynamicAopProxy(config);                                                         
		}                                                                                                  
		return CglibProxyFactory.createCglibProxy(config);                                                 
	}                                                                                                      
	else { 
		return new JdkDynamicAopProxy(config);
	}                                                                                                      
}

private boolean hasNoUserSuppliedProxyInterfaces(AdvisedSupport config) {
	Class<?>[] ifcs = config.getProxiedInterfaces();
	return (ifcs.length == 0 || (ifcs.length == 1 && SpringProxy.class.isAssignableFrom(ifcs[0])));
}

However, the second condition that determines the proxy type in the createAopProxy method, config.isProxyTargetClass() could also be modified. As the following code:
ProxyProcessorSupport.java

protected void evaluateProxyInterfaces(Class<?> beanClass, ProxyFactory proxyFactory) {
	Class<?>[] targetInterfaces = ClassUtils.getAllInterfacesForClass(beanClass, getProxyClassLoader());
	boolean hasReasonableProxyInterface = false;

        // including skywalking's EnhancedInstance
	for (Class<?> ifc : targetInterfaces) {
		if (!isConfigurationCallbackInterface(ifc) && !isInternalLanguageInterface(ifc) &&
				ifc.getMethods().length > 0) {
			hasReasonableProxyInterface = true;
			break;
		}
	}
	if (hasReasonableProxyInterface) {
		// Must allow for introductions; can't just set interfaces to the target's interfaces only.
		for (Class<?> ifc : targetInterfaces) {
			proxyFactory.addInterface(ifc);
		}
	}
	else {
                 // cannot be set to true when the beanClass implement EnhancedInstance
		proxyFactory.setProxyTargetClass(true);
	}
}

Spring iterates all of the interfaces implemented by the given beanClass, including org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance,
The filed proxyTargetClass would not be set to true, which affects the proxy type returned by createAopProxy.

Maybe we could add a new condition just like isConfigurationCallbackInterface and isInternalLanguageInterface to filter out skywalkking's interface.

ProxyProcessorSupport.java

protected boolean isConfigurationCallbackInterface(Class<?> ifc) {
	return (InitializingBean.class == ifc || DisposableBean.class == ifc || Closeable.class == ifc ||
			AutoCloseable.class == ifc || ObjectUtils.containsElement(ifc.getInterfaces(), Aware.class));
}

protected boolean isInternalLanguageInterface(Class<?> ifc) {
	return (ifc.getName().equals("groovy.lang.GroovyObject") ||
			ifc.getName().endsWith(".cglib.proxy.Factory") ||
			ifc.getName().endsWith(".bytebuddy.MockAccess"));
}

In addition, ProxyProcessorSupport.java added after Spring 4.1.x, we need to investigate the earlier version's situation.

[wu-sheng]

We have to align with Spring logic...
That is the complexity.

[wu-sheng]

@lujiajing1126 Any progress on this?

[lujiajing1126]

@lujiajing1126 Any progress on this?

We've found some potential solution as described by @Ricehomesky above. But as you mentioned, we still have to investigate the Spring MVC core deeply to make it correct. So it may still need some more time.