Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepares for 3.0 by updating to Spring Boot 3 and floor Java 8 #3684

Merged
merged 19 commits into from
Jan 10, 2024

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Jan 10, 2024

We currently still have a problem as the latest version shows up as 3.0.0-rc0 due to an accidental release. Moreover, a request to delete that version hasn't happened. This switches to 3.x by dropping support for Java 6 and moving to Spring Boot 3.

As noted, recent Zipkin Reporter 3 and Brave 6 decouple instrumentation from this core jar. While it is still in use for other formats, the call sites using them are all minimum Java 8 anyway. By updating to floor Java 8, we can use later JDKs to release than 11, though this change doesn't include doing that, yet.

As Spring Boot requires JRE 17, we update the floor JDK to 17. However, zipkin-dependencies is currently stuck at 11, so until that's not the case, this leaves collector and storage modules at Java 11, even if the server can only run in 17+.

This will be mergable once I manually verify Spring Boot 3 doesn't break modularity with zipkin-aws and -gcp.

We currently still have a problem as the latest version shows up as
3.0.0-rc0 due to an accidental release. Moreover, a request to delete
that version hasn't happened. This switches to 3.x by dropping support
for Java 6 and moving to Spring Boot 3.

As noted, recent Zipkin Reporter 3 and Brave 6 decouple instrumentation
from this core jar. While it is still in use for other formats, the call
sites using them are all minimum Java 8 anyway. By updating to floor
Java 8, we can use later JDKs to release than 11, though this change
doesn't include doing that, yet.

This will be mergable once I manually verify Spring Boot 3 doesn't break
modularity with zipkin-aws and -gcp.

Signed-off-by: Adrian Cole <[email protected]>
Adrian Cole added 14 commits January 10, 2024 12:17
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
uh
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

used openrewrite to migrate tests and server code to 17, by doing this and reverting the opposite

https://docs.openrewrite.org/recipes/java/migrate/upgradetojava17

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.migrate.UpgradeToJava17

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

@timtebeek fyi, the last commit fixes some things that were mostly, but not completely done with the 17 update recipe. It seemed to dislike some things about how our linefeeds work as well unicode specifically 💩

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

@timtebeek just a hunch, but I think it is about multi-line literal conversion where the <17 has no trailing newline or unicode. Was easy to fix these, so no biggie!

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

oh wow it is green

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt codefromthecrypt marked this pull request as ready for review January 10, 2024 09:45
@codefromthecrypt
Copy link
Member Author

surprised this worked. here are the minimal changes for zipkin-gcp. I'll add more to the branch after using openrewrite to migrate some stuff https://github.com/openzipkin/zipkin-gcp/compare/zipkin3?expand=1

@@ -24,13 +24,13 @@ jobs:
uses: actions/setup-java@v4
with:
distribution: 'zulu' # zulu as it supports a wide version range
java-version: '11' # earliest LTS and last that can compile the 1.6 release profile.
java-version: '17' # earliest LTS and last that can compile the 1.6 release profile.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can't compile 1.6 profile

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah 11 was the last that could

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry.message = ""
+ "CgABq/sBMnzE048LAAMAAAAOZ2V0VHJhY2VzQnlJZHMKAATN0p+4EGfTdAoABav7ATJ8xNOPDwAGDAAAAAQKAAEABR/wq+2DeAsAAgAAAAJzcgwAAwgAAX8AAAEGAAIkwwsAAwAAAAx6aXBraW4tcXVlcnkAAAoAAQAFH/Cr7zj4CwACAAAIAGFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFh\n"
+ "YWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhDAADCAABfwAAAQYAAiTDCwADAAAADHppcGtpbi1xdWVyeQAACgABAAUf8KwLPyILAAIAAABOR2MoOSwwLlBTU2NhdmVuZ2UsMjAxNS0wOS0xNyAxMjozNzowMiArMDAwMCwzMDQubWlsbGlzZWNvbmRzKzc2Mi5taWNyb3NlY29uZHMpDAADCAABfwAAAQYAAiTDCwADAAAADHppcGtpbi1xdWVyeQAIAAQABKZ6AAoAAQAFH/CsDLfACwACAAAAAnNzDAADCAABfwAAAQYAAiTDCwADAAAADHppcGtpbi1xdWVyeQAADwAIDAAAAAULAAEAAAATc3J2L2ZpbmFnbGUudmVyc2lvbgsAAgAAAAY2LjI4LjAIAAMAAAAGDAAECAABfwAAAQYAAgAACwADAAAADHppcGtpbi1xdWVyeQAACwABAAAAD3Nydi9tdXgvZW5hYmxlZAsAAgAAAAEBCAADAAAAAAwABAgAAX8AAAEGAAIAAAsAAwAAAAx6aXBraW4tcXVlcnkAAAsAAQAAAAJzYQsAAgAAAAEBCAADAAAAAAwABAgAAX8AAAEGAAIkwwsAAwAAAAx6aXBraW4tcXVlcnkAAAsAAQAAAAJjYQsAAgAAAAEBCAADAAAAAAwABAgAAX8AAAEGAAL5YAsAAwAAAAx6aXBraW4tcXVlcnkAAAsAAQAAAAZudW1JZHMLAAIAAAAEAAAAAQgAAwAAAAMMAAQIAAF/AAABBgACJMMLAAMAAAAMemlwa2luLXF1ZXJ5AAACAAkAAA==\n";
entry.message = """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool

@codefromthecrypt
Copy link
Member Author

merging and will reference zipkin-aws and zipkin-gcp via the snapshot repository and link back PRs. Thanks for the review @anuraaga!

@codefromthecrypt codefromthecrypt merged commit e19058d into master Jan 10, 2024
13 checks passed
@codefromthecrypt codefromthecrypt deleted the updates-for-3.x branch January 10, 2024 11:28
codefromthecrypt pushed a commit to openzipkin/zipkin-gcp that referenced this pull request Jan 10, 2024
This updates server modules to Zipkin 3, Spring Boot 3 and JRE 17. It
leaves the client side modules at Java 8.

In order to compile the server module, we can no longer compile with
JDK 11.

See openzipkin/zipkin#3684

Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt pushed a commit to openzipkin/zipkin-gcp that referenced this pull request Jan 10, 2024
This updates server modules to Zipkin 3, Spring Boot 3 and JRE 17. It
leaves the client side modules at Java 8.

In order to compile the server module, we can no longer compile with
JDK 11.

See openzipkin/zipkin#3684

Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt pushed a commit to openzipkin/zipkin-aws that referenced this pull request Jan 10, 2024
This updates server modules to Zipkin 3, Spring Boot 3 and JRE 17. It
leaves the client side modules at Java 8.

In order to compile the server module, we can no longer compile with
JDK 11.

See openzipkin/zipkin#3684

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

and here's zipkin-aws openzipkin/zipkin-aws#209

@codefromthecrypt
Copy link
Member Author

here's zipkin-gcp openzipkin/zipkin-gcp#209

@codefromthecrypt
Copy link
Member Author

will cut 3.0 tomorrow morning unless screams!

codefromthecrypt added a commit to openzipkin/zipkin-gcp that referenced this pull request Jan 10, 2024
This updates server modules to Zipkin 3, Spring Boot 3 and JRE 17. It
leaves the client side modules at Java 8.

In order to compile the server module, we can no longer compile with
JDK 11.

See openzipkin/zipkin#3684

Signed-off-by: Adrian Cole <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
codefromthecrypt added a commit to openzipkin/zipkin-aws that referenced this pull request Jan 10, 2024
This updates server modules to Zipkin 3, Spring Boot 3 and JRE 17. It
leaves the client side modules at Java 8.

In order to compile the server module, we can no longer compile with
JDK 11.

See openzipkin/zipkin#3684

Signed-off-by: Adrian Cole <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
@shakuzen
Copy link
Member

I'm late but great work, @codefromthecrypt.

@@ -36,7 +36,7 @@ final class TestResponses {
"minimum_index_compatibility_version" : "5.0.0"
},
"tagline" : "You Know, for Search"
}\
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand these reverts better; I'd thought text block without trailing new line would be correctly converted with a test like this.

JShell seems to agree at first glance:

jshell> var old = "foo\n" + "bar";  
old ==> "foo\nbar"

jshell> old.equals("""
   ...> foo
   ...> bar\
   ...> """)
$2 ==> true

How did this then end up with something that needed to be reverted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not 100pct sure as thinking about other things, but we had a lot of stuff in this format style... which may be an uncommon pattern

      String json = "{\n"
        + "  \"traceId\": \"6b221d5bc9e6496c\",\n"
        + "  \"name\": \"get-traces\",\n"
        + "  \"id\": \"6b221d5bc9e6496c\",\n"
        + "  \"annotations\": [\n"
        + "    { \"timestamp\": 1472470996199000}\n"
        + "  ]\n"
        + "}";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants