spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
73.8k stars 40.36k forks source link

OtlpMetricsExportAutoConfiguration uses default OTLP metrics url when value comes from Dynamic Properties #41276

Open eddumelendez opened 2 weeks ago

eddumelendez commented 2 weeks ago

I am using grafana/otel-lgtm:0.6.0 image in order to test OpenTelemetry support (metrics, traces, logging) in Spring Boot 3.4.0-SNAPSHOT and it can not be used with @ServiceConnection("otel/opentelemetry-collector-contrib") because there is no Docker Compose/Testcontainers implementation for OtlpLoggingConnectionDetails. So, the values are contributed via Dynamic Properties as you can see in the reproduces linked below. Using @ServiceConnection("otel/opentelemetry-collector-contrib"), metrics and tracing works as expected.

Reproducer: https://github.com/eddumelendez/spring-boot-testcontainers-grafana-lgtm/tree/otlp_logs

./mvnw spring-boot:test-run
http :8080/actuator/configprops

Values for Tracing and Logging comes from Testcontainers but metrics was not contributed.

I tried to dig into it but I am missing something about the Export Metrics AutoConfiguration configuration order.

Also, next steps:

wilkinsona commented 2 weeks ago

Thanks, @eddumelendez.

As far as I can tell, there's a general lifecycle problem with @Bean methods that inject DynamicPropertyRegistry and expect other beans to pick up any properties that are added to the registry. If the bean that adds properties to the registry hasn't been created at the point when a consumer of those properties is being created, the properties won't be visible. That's what's happening in your example.

The OtlpProperties bean is required during bean post-processing as it's a transitive dependency of tracingAwareMeterObservationHandler that's being called by ObservationRegistryPostProcessor when it's asked to post-process the observationRegistry bean. This means that the OtlpProperties bean is created and properties bound to it quite early in the context's lifecycle and, crucially, before the lgtmContainer @Bean method has been called. TestcontainersLifecycleBeanPostProcessor is called before ObservationRegistryPostProcessor to post-process observationRegistry but the bean factory's configuration has not be frozen by this point so it is not an opportunity to start all Testcontainer beans.

@ServiceConnection avoids the problem as it sets up some dependency relationships that are enough for the lgtmContainer bean to be created before the OtlpProperties bean is being created. Another workaround is to declare a dependency using a bean factory post-processor:

@Bean
static BeanFactoryPostProcessor otlpPropertiesDependencies() {
    return (beanFactory) -> {
        BeanDefinition definition = beanFactory.getBeanDefinition("management.otlp.metrics.export-" + OtlpProperties.class.getCanonicalName());
        definition.setDependsOn("lgtmContainer");
    };
}

I'm not sure how to fix this properly so I'd like to discuss it with the team. There are some similarities with problems we've had in the past with meter registry post-processing. The fixes for those may give us some inspiration.

eddumelendez commented 2 weeks ago

Hi @wilkinsona, thanks for the explanation and the workaround! Looking forward for the team meeting's outcome