Netty 'spring.netty-leak-detection' default property value is always applied to ResourceLeakDetector

[russellyou]

Spring will override the Netty leak detection level set using -Dio.netty.leakDetection.level=advanced

The code below want to set leak detection if property was not null. But the property has default value as SIMPLE. This will get leak detection overridden as SIMPLE always if you set it other value like 'advanced' outside of Spring

public class NettyProperties {
	private LeakDetection leakDetection = LeakDetection.SIMPLE;

...
public class NettyAutoConfiguration {
	public NettyAutoConfiguration(NettyProperties properties) {
		if (properties.getLeakDetection() != null) {
			NettyProperties.LeakDetection leakDetection = properties.getLeakDetection();
			ResourceLeakDetector.setLevel(ResourceLeakDetector.Level.valueOf(leakDetection.name()));
		}
	}
}
...

changed to

if (properties.getLeakDetection() != LeakDetection.SIMPLE) {

will solve the problem as LeakDetection will be simple if not set.

Happy to raise a PR if this is

Version: current main branch

[philwebb]

This looks like a bug to me, but I think we should probably change the default value in NettyProperties.leakDetection to null rather than updating the NettyAutoConfiguration.

[snicoll]

I don't think I agree with that. There are many configuration properties that have a default value that can be overridden by a system property (all our own properties, for a start). I think we should be documenting what the default value is if we decide to offer support for that. See #14338

[bclozel]

I think we discussed the null value and decided against that in #26958 (comment)

[philwebb]

We'll have to discuss this one a bit more. We need to decide if -Dio.netty.leakDetection.level=advanced should override our property or not and if that should be always or just when the property has not been defined.

IMO it's a bit strange at the moment that running an app with -Dio.netty.leakDetection.level=advanced and an empty application.properties gets simple leak detection.

[bclozel]

I think I've changed my mind. Developers can configure this with:

• a static call ResourceLeakDetector.setLevel(ResourceLeakDetector.Level.PARANOID);
• -Dio.netty.leakDetection.level=paranoid
• the Spring Boot configuration property

The configuration property should only apply if a specific value has been set, so setting the default value to null might be a good compromise. With all 3 ways involved, we're always going to find a combination that leads to strange results. This is why we initially declined #14338.

Even with this solution, if ResourceLeakDetector.setLevel() is called within a bean, the configuration property will be overridden. This could be a breaking change if our configuration property currently shadows an existing system property - upgrading Spring Boot might change the resulting behavior.

[philwebb]

We're going to change the default value in NettyProperties to null and update the description to indicate that when not set the Netty default (which is SIMPLE) will be used.

[snicoll]

closing in favor of PR #32144