Issue:
When using a PostgreSQL testcontainer and using @ServiceConnection to instantiate the connection, there can exist a case where a circular dependency is introduced to to sent events.
How to reproduce: I've created a repository reproducing the issue: https://github.com/lukasnaske/spring-serviceconnection-issue-demo/tree/main
To summarize:
Customizing the Meter registry and adding a dedicated Gauge on the HealthEdnpoint class to sync this to prometheus, it is no longer possible to setup a testcontainer connectino to postgres using @ServiceConnection. A ciruclar depenendency is introduced and reported like this:
entityManagerFactory defined in class path resource [org/springframework/boot/autoconfigure/orm/jpa/HibernateJpaConfiguration.class]
β
dataSourceScriptDatabaseInitializer defined in class path resource [org/springframework/boot/autoconfigure/sql/init/DataSourceInitializationConfiguration.class]
βββββββ
| dataSource defined in class path resource [org/springframework/boot/autoconfigure/jdbc/DataSourceConfiguration$Hikari.class]
β β
| startupTimeMetrics defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/startup/StartupTimeMetricsListenerAutoConfiguration.class]
β β
| simpleMeterRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/export/simple/SimpleMetricsExportAutoConfiguration.class]
β β
| metricsHealths defined in class path resource [com/example/testcontainer/issue/demo/MetricsConfig.class]
β β
| healthEndpoint defined in class path resource [org/springframework/boot/actuate/autoconfigure/health/HealthEndpointConfiguration.class]
β β
| healthContributorRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/health/HealthEndpointConfiguration.class]
β β
| dbHealthContributor defined in class path resource [org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.class]
βββββββ
From my investigation it appears to be connected to the BeforeTestcontainerUsedEvent sent while initialising the datasource within org.springframework.boot.testcontainers.service.connection.ContainerConnectionDetailsFactory.ContainerConnectionDetails#getContainer, which then triggers further bean creations due to the application event published probably being initialised.
From what I have seen the event is used to start the container only when it's needed. Do you think there's a way to start the container without relying on the event publisher? But maybe my investigation is also incorrect.
Comment From: snicoll
Thanks for the report and the sample. The customizer forces the health endpoint to be created early. Even by only creating it within the scope of the customizer, rather than at the bean signature level, fails the same way.
This is the wrong callback to do that though. The MeterRegistryCustomizer is really meant to customize the registry before any metrics is contributed to it, not to register metrics. See the reference guide for more details. I've moved to a MeterBinder and your test works, i.e.:
@Bean
MeterBinder metricsHealths(HealthEndpoint healthEndpoint) {
return meterRegistry -> {
meterRegistry.gauge("health", healthEndpoint, this::getStatusCode);
};
}
I am not sure that we'll be able to fix your scenario, I'll flag for team attention to get more feedback.
Comment From: lukasnaske
Hi thanks for the hint. Seems like legacy code strikes again and I was trying to resolve the wrong issue.
In any case, I think if it's really the event being sent that triggers the circular dependency in this case is a bit unfortunate and it might not be easily resolvable in other cases. But, given we were using the metrics configuration incorrectly this is probably not a valid bug.
Comment From: wilkinsona
In addition to recommending MeterBinder for this, BeforeTestcontainerUsedEvent event has been deprecated in 3.4 as it's on longer needed following the reworking of the of dynamic property registration and the related container lifecycle. Unless we hear of an unavoidable problem caused by the event, I don't think there's anything more that we should do here.