Metrics and health do not include non-default candidate beans

[mdaepp]

The documentation for Spring Boot 3.4 now recommends the @Qualifier("xyz") and @Bean(defaultCandidate = false) approach to define multiple DataSources: https://docs.spring.io/spring-boot/3.4/how-to/data-access.html#howto.data-access.configure-two-datasources

At first glance, this approach is neat because the first DataSource is set up by auto-configuration as if there were no second DataSource. However, the new approach has some probably unwanted side-effects that users should be made aware of.

With the previous approach (see https://docs.spring.io/spring-boot/3.3/how-to/data-access.html#howto.data-access.configure-two-datasources), the additional DataSources would still participate in other auto-configurations such as DataSourcePoolMetricsAutoConfiguration and DataSourceHealthContributorAutoConfiguration. That is no longer the case with the new approach, because an additional DataSource with defaultCandidate = false is neither present in Map<String, DataSource> nor is it available via ObjectProvider<DataSource>.

I leave it to the subject matter experts to decide what should be done:

1. Update documentation to mention the limitations of the new approach
2. Revert documentation to previous approach
3. Fix other auto-configurations that apply to a "list" of DataSources of some sort (such as ObjectProvider<DataSource>)—not sure if that is even possible or a good idea for that matter

[quaff]

Fix other auto-configurations that apply to a "list" of DataSources of some sort (such as ObjectProvider<DataSource>)

ObjectProvider<DataSource> filter out non-defaultCandidate beans, we could use beanFactory.getBeansOfType(DataSource.class) instead of injection of Map<String, DataSource>.

[quaff]

Fix other auto-configurations that apply to a "list" of DataSources of some sort (such as ObjectProvider<DataSource>)

ObjectProvider<DataSource> filter out non-defaultCandidate beans, we could use beanFactory.getBeansOfType(DataSource.class) instead of injection of Map<String, DataSource>.

I'd like to prepare an PR if the team accept this solution, see https://github.com/spring-projects/spring-boot/compare/main...quaff:patch-108?expand=1.

[philwebb]

I'm not a fan of using the BeanFactory directly, but I can't immediately think of a better option. @jhoeller Any thoughts on the best way to inject something that will include non-defaultCandidate beans? Perhaps we need some new methods on ObjectProvider or a new annotation to signal intent?

[quaff]

I'm not a fan of using the BeanFactory directly, but I can't immediately think of a better option. @jhoeller Any thoughts on the best way to inject something that will include non-defaultCandidate beans? Perhaps we need some new methods on ObjectProvider or a new annotation to signal intent?

I vote for new methods on ObjectProvider, It's not good to introduce new annotation, make things more complex hard to understand.

[jhoeller]

The intent was for such general container lookups to happen via beanFactory.getBeansOfType(DataSource.class) indeed, if necessary. defaultCandidate=false restricts user injection without qualifiers, and if framework components declare user-style injection points with those semantics, they get the same restricted view.

We could introduce new methods on ObjectProvider but it would blur the line with the user's view, so I would only consider that if it was a common need in user components. If defaultCandidate=false has unwanted side effects in a scenario, it might be the wrong choice. That said, for framework facilities, it's just a matter of using the appropriate lookup mechanism. In framework components, I don't consider direct interaction with an injected BeanFactory reference as inferior to ObjectProvider interaction.

[philwebb]

Holding to the new year as we consider options

[snicoll]

This issue is now blocked by spring-projects/spring-framework#34203. If implemented we'd hopefully be able to restore the previous behavior with such beans.

[mdaepp]

@wilkinsona Thanks for fixing this! I think you forgot to adapt HikariDataSourceMetricsConfiguration org.springframework.boot.actuate.autoconfigure.metrics.jdbc.DataSourcePoolMetricsAutoConfiguration. It is implemented slightly differently as it uses ObjectProvider<DataSource> dataSources. If I am not mistaken, ObjectProvider still doesn't see non-default candidates. As-is, MicrometerMetricsTrackerFactory will not be registered for DataSources with defaultCandidate=false.

[quaff]

@wilkinsona Thanks for fixing this! I think you forgot to adapt HikariDataSourceMetricsConfiguration org.springframework.boot.actuate.autoconfigure.metrics.jdbc.DataSourcePoolMetricsAutoConfiguration. It is implemented slightly differently as it uses ObjectProvider<DataSource> dataSources. If I am not mistaken, ObjectProvider still doesn't see non-default candidates. As-is, MicrometerMetricsTrackerFactory will not be registered for DataSources with defaultCandidate=false.

You are right, I'd like to create PR if @wilkinsona think https://github.com/spring-projects/spring-boot/compare/main...quaff:patch-108?expand=1 is correct fix.

[wilkinsona]

Well spotted, @mdaepp. Thank you.

@quaff, that change to the main code looks good to me, thank you. On the testing side, rather than adding a new test, I'd change TwoHikariDataSourcesConfiguration to this:

	@Configuration(proxyBeanMethods = false)
	static class MultipleHikariDataSourcesConfiguration {

		@Bean
		DataSource standardDataSource() {
			return createHikariDataSource("standardDataSource");
		}

		@Bean(defaultCandidate = false)
		DataSource nonDefault() {
			return createHikariDataSource("nonDefault");
		}

		@Bean(autowireCandidate = false)
		DataSource nonAutowire() {
			return createHikariDataSource("nonAutowire");
		}

	}

And then adapt the tests that used TwoHikariDataSourcesConfiguration to use MultipleHikariDataSourcesConfiguration instead and expect both standardDataSource and nonDefault.