openzipkin / brave

Java distributed tracing implementation compatible with Zipkin backend services.
Apache License 2.0
2.36k stars 713 forks source link

Support spring 6 #1418

Open codefromthecrypt opened 7 months ago

codefromthecrypt commented 7 months ago

Feature

Spring 6 requires Java 17+ and jakarta packages.

This makes things more complicated than now because we release with JDK11, so that old spring can compile to Java 6 bytecode.

Rationale

We can add jakarta-friendly instrumenation for spring-web and spring-webmvc

Example Scenario

Spring boot 3 code requires spring 6

codefromthecrypt commented 7 months ago

my thinking is this, knowing there's no new LTS until Sep 2025, and our Java 6,7 projects already default to Java 8 but use the lower version when the release profile is active...

This way, everything complicated is constrained to the release profile, which means dev/x normally is same as now (Except can no longer use JDK 11). If snapshot builds publish fine, and target the correct bytecode, we know a release will also work fine.

bonus points for a src/it test for the 6, 7 modules which.. when it is a release profile, fails if the bytecode of the classes is not the correct version!

cc @reta @anuraaga @shakuzen for input

reta commented 7 months ago

cc @reta @anuraaga @shakuzen for input

We've been tackling this issue in different projects, the way it seems to work (and not requiring too much maintenance overhead):

Not sure if that makes sense for Brave, but it seems to be viable option in general, wdyt @codefromthecrypt ?

codefromthecrypt commented 7 months ago

@reta everything works except for this below part as we shouldn't kill the <6,7 part just for spring

keep the byte code at 8 (or 11) level

codefromthecrypt commented 7 months ago

so I was thinking we use toolchain or a maven-compiler-plugin setting to make sure that in the release profile, we use an alternate JDK to compile the several modules we have which are pre-8. In their normal profile we claim they are bytecode 8, but basically they aren't at release time. Does this make sense?

https://github.com/openzipkin/brave/blob/master/brave/pom.xml#L110-L148

codefromthecrypt commented 7 months ago

seems like setup-java can already configure multiple JDKs.. https://github.com/actions/setup-java?tab=readme-ov-file#install-multiple-jdks

reta commented 7 months ago

In their normal profile we claim they are bytecode 8, but basically they aren't at release time. Does this make sense?

👍 I think it makes perfect sense, and yes, should be possible with GA