deps: updates build to compile with LTS JDK and bumps deps #204
Conversation
This moves to the same build infra as the rest of Zipkin, notably to allow compiling on JDK 11, 17 and 21. This also bumps all the dependencies. This also migrates to junit5 which began with the helpful @TeamModerne tool, but needed a lot of fixing up due to custom rules. ``` ./mvnw org.openrewrite.maven:rewrite-maven-plugin:run \ -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:LATEST \ -Drewrite.activeRecipes=org.openrewrite.java.testing.junit5.JUnit4to5Migration ``` Signed-off-by: Adrian Cole <adrian@tetrate.io>
| public MockWebServer mockServer = new MockWebServer(); | ||
|
|
||
| // TODO: replace this with jupiter | ||
| @Rule | ||
| public EnvironmentVariables environmentVariables = new EnvironmentVariables(); |
There was a problem hiding this comment.
We've used https://github.com/webcompere/system-stubs in Micrometer for setting environment variables in tests with Jupiter.
|
@reta if you have free time, do you mind trying to progress this? While the project is zipkin-aws proceeding doesn't need an aws account. There is some work to do porting off junit 4 environment rule and also figuring out why the mock sqs setup I tried to port to jupiter doesn't work. It is fine to use a different sqs mock or even a docker testcontainer instead. Push any commits you like and feel free to do anything including force pushing or squashing. When you've moved as far as you can, please summarize in a comment, even if incomplete still. Cheers! |
Sure @codefromthecrypt , I will work on it |
|
nice to see some commits going 🙇♂️ |
...entation-aws-java-sdk-core/src/test/java/brave/instrumentation/aws/AwsClientTracingTest.java
Show resolved
Hide resolved
|
@codefromthecrypt it seems like we are in good shape :-) anything else you have in mind for this change? |
There was a problem hiding this comment.
Thanks very much @reta, you did it!
Next step is to merge this and then push a tag release-0.24.0 and once that's done make GH release from that tag in the same style as the latest on zipkin-gcp.
These notes are in RELEASE.md, but the jist is only this.
| HttpTracing httpTracing = HttpTracing.create(tracing); | ||
| environmentVariables.set("AWS_REGION", "us-east-1"); | ||
| variables.set("AWS_REGION", "us-east-1"); |
| try { | ||
| AggregatedHttpRequest signed = sign(ctx, aggReg); | ||
| return unwrap().execute(ctx, signed.toHttpRequest()); | ||
| final HttpRequest httpRequest = signed.toHttpRequest(); |
There was a problem hiding this comment.
not terrible drift considering the years passed!
| @@ -63,7 +63,7 @@ abstract class AbstractSender extends Sender { | |||
|
|
|||
| boolean isAscii(byte[] encodedSpans) { | |||
| for (int i = 0; i < encodedSpans.length; i++) { | |||
| if (encodedSpans[i] >= 0x80) { | |||
| if (Byte.toUnsignedInt(encodedSpans[i]) >= 0x80) { | |||
There was a problem hiding this comment.
so byte is signed [-128 to 127], and 0x80 is exactly -128, the comparison to 0x80 was actually lifting byte to int
There was a problem hiding this comment.
thanks, might want to github search the whole org and also openzipkin-contrib in case this bug is elsewhere via copy/pasta!
|
@reta so when we merge, we always squash and retain co-author tags, but tidy up the message so it isn't like a list of interim commits. Over to you for the release and looking forward to you pinging it on the zipkin gitter channel, too when done. 
|
🚀
|
@reta ps one thing in RELEASE.md it says for the first release of the year, make sure the pom.xml license headers are up to date. If they aren't the release will fail. so you want to do something like this to make sure they are bumped.. or check manually https://github.com/openzipkin/zipkin-aws/blob/master/RELEASE.md#first-release-of-the-year |
This moves to the same build infra as the rest of Zipkin, notably to allow compiling on JDK 11, 17 and 21. This also bumps all the dependencies and ports to junit jupiter (used by brave now).
This also migrates to junit5 which began with the helpful @TeamModerne tool, but needed a lot of fixing up due to custom rules.
I intentionally raised this on a shared branch as I'm out of time. If someone can finish up the rest of it, many thanks. Notably, I think I messed up porting the SQS extension (some tests pass some hang). Also, there's a need to replace the environment rule with something. Pinging @timetebeek, but really anyone who is good at jupiter could give a swing and let this project release again after merge!