Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve bean post processor warnings #1361

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

hpoettker
Copy link
Contributor

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:

  • LoadBalancerRequestFactoryConfigurationTests
  • ReactorLoadBalancerClientAutoConfigurationTests

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:

  • By declaring the bean factory methods as static, the respective configuration class no longer needs to be instantiated before the methods are invoked.
  • The bean post processors do not work with the injected DeferringLoadBalancerInterceptor or DeferringLoadBalancerExchangeFilterFunction themselves but only pass on references to builders. Therefore their injection points can be declared to be lazy.

Copy link
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @hpoettker thanks for the PR. Looks good. Please add your full name and surname with the @author tag the javadocs of all the classes you've modified.

@hpoettker
Copy link
Contributor Author

@OlgaMaciaszek done

@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.5 milestone Jul 15, 2024
Copy link
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@OlgaMaciaszek OlgaMaciaszek merged commit 4e6540b into spring-cloud:main Jul 15, 2024
1 check passed
@hpoettker hpoettker deleted the resolve-warnings branch July 15, 2024 15:09
OlgaMaciaszek added a commit that referenced this pull request Jul 31, 2024
This reverts commit 4e6540b.

# Conflicts:
#	spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java
@OlgaMaciaszek
Copy link
Collaborator

@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.

OlgaMaciaszek added a commit that referenced this pull request Jul 31, 2024
@hpoettker
Copy link
Contributor Author

@OlgaMaciaszek Thanks for the notice.

I think we've run into spring-projects/spring-framework#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
Copy link
Collaborator

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants