Document issues when using Spring FactoryBean and Spring Cloud OpenFeign Clients

[raul-avila-ph]

Bug description

We found a problem where Micrometer tracing propagation stops working when a Feign client is injected to a Spring FactoryBean.

Steps to reproduce:

1. Have Micrometer tracing enabled
2. Configure a Feign client
3. Create a service class that contains the Feign client as dependency
4. Instantiate the service class through a Spring FactoryBean, not using stereotype annotations or bean declaration. This FactoryBean will need to have a reference to the Feign client, because it will be necessary to instantiate the service class created in step 3

We would expect Micrometer traces to continue working. With the setup explained above it stopped working.

Sample

We have created a small sample project reproducing the issue, it can be found here: https://github.com/raul-avila-ph/tracing-issue/

The project contains a single test that passes only when tracing is working properly. The code in master has the test failing, while you can see there is a branch called without-factory-bean that basically removes the FactoryBean and instantiates the service with @Service. The test in this branch works, so tracing is not broken under these conditions.

Please note that this problem occurs when Feign client is a transitive dependency of the Spring FactoryBean as well, we have just injected it directly in the sample code for simplicity.

[OlgaMaciaszek]

@jonatan-ivanov, could you take a look at this issue?

[jonatan-ivanov]

Thank you for the issue and extra thanks for providing a reproducer!
I looked into it and I think the issue is not related to Spring Cloud OpenFeign. When the app uses the FactoryBean, you can see lots of WARN events in the logs. One of them is saying something like this:

'observationRegistry' is not eligible for getting processed by all BeanPostProcessors

Exact event:

2024-06-26T12:45:18.772-07:00  WARN 86139 --- [demo] [           main] [                                                 ] trationDelegate$BeanPostProcessorChecker : Bean 'observationRegistry' of type [io.micrometer.observation.SimpleObservationRegistry] 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 [meterRegistryPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

This is a problem since in Boot's auto-config ObservationRegistryPostProcessor is needed since it registers the handlers/filter/predicates/etc. to the registry using ObservationRegistryConfigurer.
In your case, since this is not happening, no ObservationHandler will be registered so nothing happens when an Observation is created (no metrics, no tracing, no context-propagation). You can check this with a debugger: either inject and ObservationRegistry to one of your components or put a breakpoint into MicrometerObservationCapability (OpenFeign's observability support), the registry has a config field which should have a list of handlers but that list is empty due to the post processor were not executed and register them. This issue might belong to Boot's or Framework's issue tracker.

I guess one workaround could be creating and configuring the registry on your own the same way Boot does (see ObservationAutoConfiguration) but without a post processor.

[OlgaMaciaszek]

Thanks @jonatan-ivanov. There was actually a bug in Spring Cloud Commons that caused bean postprocessing issues. @raul-avila-ph Please upgrade to Spring Cloud Commons 2023.0.2 and see if the issue has been resolved. If not, please follow the steps outlined by @jonatan-ivanov above.

[raul-avila-ph]

Thanks for taking a look.

We tried upgrading Spring Cloud Commons to version 2023.0.2, but the problem persists.

We then tried the solution proposed by @jonatan-ivanov. This can be found in a separate branch in our project (https://github.com/raul-avila-ph/tracing-issue/tree/fix-observation-registry). This solves the problem, however we're not entirely happy with the workaround, because we can't replicate what the autoconfiguration does entirely, as the class ObservationHandlerGrouping is package private (see comment: https://github.com/raul-avila-ph/tracing-issue/blob/fe41dcfb19c2162d7ebd7488fb29ced3d2504a45/src/main/java/com/example/tracingissue/ObservationConfiguration.java#L34). We could create our own config class in a package org.springframework.boot.actuate.autoconfigure.observation as a workaround, but we don't really like to do this.

[OlgaMaciaszek]

@jonatan-ivanov could you please take a look?

[jonatan-ivanov]

I guess the other thing you can do is copying that class (still not optimal) or asking the Boot team to make it public (still a hack/workaround).
I think the proper solution would be fixing what breaks the bean post processors but I don't think that issue lies in OpenFeign, probably somewhere else in Spring Cloud or Spring Framework?

[raul-avila-ph]

Thanks for your response. Should I create the issue somewhere else? It'd be great to have this problem solved at some point, but I don't want to create unnecessary noise in other Spring projects.

[OlgaMaciaszek]

Thanks a lot @raul-avila-ph, @jonatan-ivanov. I have done some debugging to verify why there's an issue with the FactoryBean scenario. What happens is that the observationRegistry bean is created earlier then it should, i.e. without the FactoryBean its creation is triggered from org.springframework.context.support.AbstractApplicationContext#finishBeanFactoryInitialization while in the one with the FactoryBean it's done from org.springframework.context.support.AbstractApplicationContext#registerBeanPostProcessors, so a few steps to early; That happens because during registerBeanPostProcessor, the Framework looks for beans of type ApplicationContext and that includes instantiating any FactoryBeans, including MyFactoryBean and that triggers the instantiation of MyFeignClient which is way too early for the Feign Client to be instantiated correctly; instantiating Feign clients includes a call to org.springframework.cloud.context.named.NamedContextFactory#createContext that first registers Feign FactoryBean definitions and then refreshes the context (which will result in a lot of beans being created way too early, including observationRegistry).

To summarise, with the current programming model, there's no way to correctly create Feign client beans declared as fields in FactoryBeans and the real issue is that it's not been documented earlier, so I'm converting this one into a documentation issue.