spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
701 stars 697 forks source link

Resolve bean post processor warnings #1361

Closed hpoettker closed 1 month ago

hpoettker commented 1 month ago

Introduction

The PR resolves the remaining warnings about beans not being eligible for getting processed by all BeanPostProcessors. Similar warnings have been reported in #1315 and fixed in #1319.

The warnings do not indicate a severe issue. But they lead to confusion among users and bad getting started experiences. And as the size of the PR shows, the required change is rather small.

Reproducing the issue

The effectiveness of the change can be seen in the following tests:

Without the change, the test LoadBalancerRequestFactoryConfigurationTests logs the following warnings:

Bean 'org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration$DeferringLoadBalancerInterceptorConfig' of type [org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration$DeferringLoadBalancerInterceptorConfig] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). The currently created BeanPostProcessor [lbRestClientPostProcessor] is declared through a non-static factory method on that class; consider declaring it as static instead.
Bean 'deferringLoadBalancerInterceptor' of type [org.springframework.cloud.client.loadbalancer.DeferringLoadBalancerInterceptor] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [lbRestClientPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

Without the change, the test ReactorLoadBalancerClientAutoConfigurationTests logs the following warnings:

Bean 'loadBalancerBeanPostProcessorAutoConfiguration' of type [org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). The currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor] is declared through a non-static factory method on that class; consider declaring it as static instead.
Bean 'org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration$ReactorDeferringLoadBalancerFilterConfig' of type [org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration$ReactorDeferringLoadBalancerFilterConfig] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.
Bean 'reactorDeferringLoadBalancerExchangeFilterFunction' of type [org.springframework.cloud.client.loadbalancer.reactive.DeferringLoadBalancerExchangeFilterFunction] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

With the change, no such warnings appear.

How it works

The PR works by using two mechanisms:

hpoettker commented 1 month ago

@OlgaMaciaszek done

OlgaMaciaszek commented 3 weeks ago

@hpoettker Had to revert these changes as, unfortunately, they were messing up native images: https://ge.spring.io/s/elz5iwbaoj2me . Have tried using an ObjectProvider instead of @Lazy, but that also hasn't fixed the issue.

hpoettker commented 3 weeks ago

@OlgaMaciaszek Thanks for the notice.

I think we've run into https://github.com/spring-projects/spring-framework/issues/29309.

I could reproduce the problem locally. The ObjectProvider is a great idea and it actually fixes the issue for me. In the linked commit, you applied the change to LoadBalancerBeanPostProcessorAutoConfiguration. But to fix the AOT smoke test, it must be applied to LoadBalancerAutoConfiguration as the exception in the build log is about lbRestClientPostProcessor.

Are you interested in a PR with the ObjectProvider applied to both auto-configurations?

OlgaMaciaszek commented 3 weeks ago

@hpoettker - yes, that's right. This should fix it: https://github.com/spring-cloud/spring-cloud-commons/tree/use-object-provider-for-lb-restclient-postprocessor