Making optional the creation of health indicators for routing datasources

[juliojgd]

spring-boot/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java

Line 108 in 8436fa9

builder.unknown().withDetail("routing", true);

Hi, does it make sense to add a HealthIndicator with a fixed outcome? (always UNKNOWN).
I mean, would'n it be better to allow the user to choose to not create a HealthIndicator for this type of Datasources (AbstractRoutingDataSource)?
I feel the important information is if any of the routed datasources is Ok or not. If you configured the datasources as beans there will be HealthIndicator's for them, making useless a fixed status value from the routing one.

I am thinking of a new property allowing to exclude the creation of HeatlhIndicator's for AbstractRoutingDataSource ones.

Do you think it would be useful? Thanks in advance for reading.

[wilkinsona]

Another option may be to reflectively examine the routing data source's resolvedDataSources or for Framework to provide an accessor that we can use.

[juliojgd]

I agree, it's an option to examine the resolvedDatasources but in that case is important to avoid to check the resolved datasources twice in each check cycle (one on its own and another one through the routing one). If this is controlled, might be a good alternative. I prefer the accesor option over the reflective access option, anyway.

[wilkinsona]

We discussed this one today and decided to add a property to allow routing data sources to be ignored.

[juliojgd]

I can contribute a PR for that

[wilkinsona]

Yes please, @juliojgd. That'd be great. I think the property should start with management.health.db, something like management.health.db.ignore-routing-datasources. Please let us know if you have any questions while you're working on it and we'll do our best to help.

[juliojgd]

I have a doubt.
There are several cases to cover divided by the number of routingDatasources in the context:
1.- No RoutingDatasource - OK
2.-Let "M" the number of non routing datsources and "N" the number of routing datasources. OK, I filter the datasources to avoid creating HealthIndicator's for the "N" routing ones.
3.- Only one RoutingDatasource in the context and no other datasource at all. (this case is depicted in the tests)

I think cases (1) and (2) are covered if I do something like this:

        @Bean
	@ConditionalOnMissingBean(name = { "dbHealthIndicator", "dbHealthContributor" })
	public HealthContributor dbHealthContributor(Map<String, DataSource> dataSources) {
		if (ignoreRoutingDataSources) {
			Map<String, DataSource> filteredDatasources = dataSources.entrySet().stream()
					.filter(e -> !(e.getValue() instanceof AbstractRoutingDataSource))
					.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
			return createContributor(filteredDatasources);
		}
		return createContributor(dataSources);
	}

in org.springframework.boot.actuate.autoconfigure.jdbc.DataSourceHealthContributorAutoConfiguration (please consider this implementation only an initial approximation).

But case (3) is not covered by this because and empty map is not allowed in the createContributor method.

Would it be better to keep unmodified the aforementioned method and make the change in the HealthContributor registration process?

Perhaps a ContributorRegistry filtering mechanism would be useful (meaning the capability to filter by some condition which contributor gets registered and which ones don't).
Does it make sense to do the filtering in the HealthContributor registering (providing a new mechanism to ease that filtering)?

Thanks in advance for your guidance

[wilkinsona]

Thanks for taking a look. I think that’s the right place for the filtering to be performed.

IMO, it is OK for a failure to occur if someone opts in to routing data sources being ignored and that results in there being no data sources. In that situation they can either opt back out again or they can disable db health altogether.

[juliojgd]

I agree with you. I implemented that way, if there are only routing datasources, a failure occurs when loading the context. I documented the new property but I wonder if it would be neccesary to have additional documentation warning about the failure when the property is set and there are only routing datasources in context. If it is needed, which would be the right place to write it down?