[Spring Starter] Spring boot 4 support#15459
Conversation
0949e97 to
330cd89
Compare
|
Would that resolve the missing metrics going to mimir on spring boot 4 setup using the java agent @jaydeluca ? |
|
@George-C-Odes there is no ETA, im working on this as I can. |
|
my goal is to get this all completed before the next release 🤞. This is just the spring boot starter, there are still other javaagent instrumentations that need to be fixed too |
…entelemetry-java-instrumentation into spring-boot-4-autoconfigure
| @Bean | ||
| DefaultKafkaProducerFactoryCustomizer otelKafkaProducerFactoryCustomizer( | ||
| OpenTelemetry openTelemetry) { | ||
| KafkaTelemetry kafkaTelemetry = KafkaTelemetry.create(openTelemetry); | ||
| return producerFactory -> producerFactory.addPostProcessor(kafkaTelemetry::wrap); | ||
| } |
There was a problem hiding this comment.
actually only this bean is different for spring boot 4. Wondering whether it would make sense to move this bean into separate (maybe nested?) configuration class so that the other beans wouldn't need to be copy-pasted. Or is that too much effort?
| () -> getTelemetry(openTelemetryProvider, configProvider)); | ||
| } | ||
|
|
||
| @ConditionalOnClass(DefaultKafkaProducerFactoryCustomizer.class) |
There was a problem hiding this comment.
Do you also need to add @ConditionalOnEnabledInstrumentation(module = "kafka") here or is it somehow automatically inherited from the outer class?
Imo the way it is now pushing this into a nested configuration class doesn't make sense. If you'd remove DefaultKafkaProducerFactoryCustomizer condition from the outer class and only leave it on the inner class then the outer class could also be used for spring boot 4 and you'd only need to provide a spring boot 4 version of what the inner class does.
There was a problem hiding this comment.
is it somehow automatically inherited from the outer class?
yes
There was a problem hiding this comment.
Imo the way it is now pushing this into a nested configuration class doesn't make sense
agree - does it work if you inline the bean method?
| @@ -1,2 +1,2 @@ | |||
| Comparing source compatibility of opentelemetry-spring-boot-autoconfigure-2.23.0-SNAPSHOT.jar against opentelemetry-spring-boot-autoconfigure-2.22.0.jar | |||
| Comparing source compatibility of opentelemetry-spring-boot-autoconfigure-2.23.0-SNAPSHOT.jar against | |||
There was a problem hiding this comment.
i'm still trying to figure this out. the build is failing for this
Run # need to "git add" in case any generated files did not already exist
Diff detected - did you run './gradlew jApiCmp'?
docs/apidiffs/current_vs_latest/opentelemetry-spring-boot-autoconfigure.txt
diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-spring-boot-autoconfigure.txt b/docs/apidiffs/current_vs_latest/opentelemetry-spring-boot-autoconfigure.txt
index 7e15cee7..9dfa3287 100644
--- a/docs/apidiffs/current_vs_latest/opentelemetry-spring-boot-autoconfigure.txt
+++ b/docs/apidiffs/current_vs_latest/opentelemetry-spring-boot-autoconfigure.txt
@@ -1,2 +1,2 @@
-Comparing source compatibility of opentelemetry-spring-boot-autoconfigure-2.23.0-SNAPSHOT.jar against opentelemetry-spring-boot-autoconfigure-2.22.0.jar
+Comparing source compatibility of opentelemetry-spring-boot-autoconfigure-2.23.0-SNAPSHOT.jar against
No changes.
\ No newline at end of file
and when i run ./gradlew :instrumentation:spring:spring-boot-autoconfigure:jApiCmp or ./gradlew jApiCmp I get the same result.
There was a problem hiding this comment.
did you try deleting the file and rebasing against main?
| // Spring Boot 2 & 3 | ||
| "org.springframework.boot.autoconfigure.kafka.DefaultKafkaProducerFactoryCustomizer") | ||
| @Configuration | ||
| public class KafkaInstrumentationSpringBoot4AutoConfiguration { |
There was a problem hiding this comment.
is this class still needed? Tests pass without it.
| exclude("ch.qos.logback", "logback-classic") | ||
| } | ||
| // testSpring2: configure Spring Boot 3.x dependencies for latest dep testing | ||
| if (name == "testSpring2Implementation") { |
There was a problem hiding this comment.
since this does not involve any latest dep checks this will bump the version to latest spring 3 even in regular tests?

Related to #14906
Notes:
I tried to avoid as much duplication as I could, If anyone has suggestions on areas to revisit, I can try again