-
Notifications
You must be signed in to change notification settings - Fork 207
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
[Proposal]Support JDK17 and springboot3 in Dapr Java SDK #971
Comments
Hii @skyao Thanks In Advance |
Great!! @Tejasker you're welcome! The proposal is basically ready, I'm fighting with CI and nearing completion. There will be a lot of legacy issues after this upgrade work that will need to be followed up with incremental improvements (as I marked in the proposal "out of scope") and you can pick what you like. |
The proposal looks good. It describes of immediate as well as long term plan and ensures code does not break when released. Looking forward for this release... |
Thanks @skyao for this great proposal. I think all the points you made here make sense to me. I agree with all the short and long-term directions. This is a great migration plan. Thanks! |
LGTM (non-binding). cc @artursouza @mukundansundar |
There are some important matters that need to be decided together and I will give multiple options in my proposal and then I will give my recommendation. But these decisions definitely need to be taken together. I will list the most important ones:
|
Share some of my thoughts.
If I remember correctly from the proposal, supportting java8 as the minimum version doesn't require much effort right. So I think we can keep it.
Can we post a vote to the community just to collect more data from users on this one?
I think jdk17 + springboot3.0 should be enough for examples, too many redundant samples may cause confusion to users.
I prefer to split it, it will be easy for others to understand it and we can easily add more similar tests in the future if we split them into sub-projects. It's easier to maintain as well. |
@mukundansundar @artursouza @yaron2 I need your input to make decision list above. In my proposal, I have considered multiple solutions for each decision, and I have also technically verified the feasibility of those options, also I have given my recommendations. At this point all we need to do is to make agreement about which options to adopt as soon as possible. |
@mukundansundar @artursouza @yaron2 I need your input to make decision. |
Let me answer these:
Great proposal, BTW. Thanks! |
I am new to dapr and dapr’s java-sdk, but have been working with Spring Boot since v1.0. I have read through this issue’s proposal and would like to give some feedback from a Spring Boot perspective, i.e., without much understanding of dapr’s java-sdk at this time. Based on my experiences with Spring Boot, I have a few high-level recommendations:
I hope this helps to some extent; once I have learned more about dapr and its java-sdk, I hope to be able to contribute more. |
@artursouza @mukundansundar I have updated the proposal and the PR code according to our discussion and agreement.
And then there will be a problem that we can't use jdk11 to test sprintboot 2.x in sdk-tests subproject: Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project dapr-sdk-examples: Fatal error compiling: error: release version 17 not supported -> [Help 1] The root cause is that the answers of question2/3/4 conflict. The simplest solution is to change the answer of question2, let us use jdk17 to test springboot2.x in sdk-tests subproject. |
Overview
This document is a design proposal to support JDK17 and springboot 3.0 in Dapr Java SDK.
This proposal will allow customers to use the Dapr Java SDK with JDK17 and springboot3.0 while trying to maintain compatibility with existing implementations: continue to support JDK8/JDK11, continue to support springboot2.x.
Background
Current Versions Overview
In this proposal, I will refer to the following four subprojects by "SDK Core" for convenience:
The following picture shows the overview of current JDK versions and springboot versions in the Dapr Java SDK:
I will describe the details of this overview picture.
JDK versions in java-sdk now
Currently, the Dapr Java SDK is compiled with JDK11, but for Java8 compatibility, set both the compiler source and target to 8:
The examples and sdk-tests subprojects have no need for Java8 compatibility because they do not need to be published to customers, so their source and target are set to 11:
Therefore, the current Java versions supported in Dapr Java SDK are Java8 and Java11:
Springboot versions in java-sdk now
Currently, Dapr's springboot support is integrated with springboot 2.x. The specific version is 2.7.8, which is set in the pom.xml:
Theoretically, there should be no springboot dependencies in the SDK Core, but in fact, springboot is introduced in the sdk-actors and sdk sub-projects for testing purposes. The good thing is that these dependencies are test dependencies and will not passed on to the users:
More specifically, in the sdk-tests subproject, in order to test the compatibility of multiple springboot versions, several other 2.x versions will be tested in addition to the 2.7.8 version:
These are sub-versions of springboot 2.x. The JDK version requirements are basically the same as 2.7.8 and will not affect the solution of this proposal.
This is a summary of the spring versions in Dapr java-sdk now:
Summary
JDK versions and springboot versions in Dapr Java SDK:
New requirements from customers
The new requirement from customers is to add JDK17 and springboot 3.0 support in Dapr Java SDK.
During implementation, we need to refine this requirement:
SDK Core
Required: SDK Core must add support for Java17 and keep support for JDK11.
Special Reminder: After discussion, we decided that Java8 is no longer supported in Dapr Java SDK after this proposal, the minimal Java version in Dapr Java SDK will be Java 11.
sdk-springboot (2.x)
Required: Springboot integration for springboot 2.x versions must keep support for JDK11.
Special Reminder: After discussion, we decided that we don't support sprintboot2.x + JDK17 in the compatibility list of Dapr Java SDK (springboot version matrix), it means that in sdk-tests subproject we will only cover JDK11 + springboot 2.x and JDK 17 + springboot 3.x.
sdk-springboot3 (3.0)
Required: The support of springboot 3.0 versions must be added.
Springboot 3.0 (also related spring framework 6.0) requires JDK17; it doesn't support JDK8 and JDK11 anymore.
In this proposal, only the compatibility support for springboot 3.0 is considered; how to utilize springboot 3.0-specific features is not in the scope of this proposal. However, this proposal will prepare the solution to support springboot3.0-specific features in the future.
examples(springboot 3.0)
Required: upgrade current examples to run with springboot 3.0
sdk-tests(both springboot 2.x and springboot 3.0)
Required: sdk-tests must add support for springboot 3.0 and keep support for springboot 2.x.
Special Reminder: After discussion, we decided that we will use ONLY springboot 3.0 in examples subproject.
In summary, JDK versions and springboot versions in Dapr Java SDK should be updated to:
Motivation
Goals
Scope
The scope of this proposal will be limited to add support for JDK17 and springboot3.0.
Related Items
Related issues
Implementation Details
Support JDK17
Considering that the Dapr Java SDK is currently compatible with Java8 and Java11, adding support for Java17 should theoretically be natural.
Assuming JDK8/JDK11/JDK17 have perfect compatibility. Each JDK upgrade only adds new features, and the old ones continue to be supported. And our Dapr Java SDK code is fully compatible with JDK8/JDK11/JDK17 because it only uses some of the features of Java8. As shown in the figure below:
In reality, the JDK upgrade is not perfect, but there are some BROKEN changes like renaming or removing.
https://github.com/johanjanssen/JavaUpgrades
Referring to the instructions on this site, the JDK broken changes include:
-XX:+AggressiveOpts
and-Xoptimize
The challenge of upgrading JDK17, therefore, lies in whether these broken changes affect our Dapr Java SDK code. If not, then we can upgrade smoothly. If they exist, they must be found and fixed.
For the issues introduced by each Java version and detailed solutions, please refer to this section, "Issues and solutions per Java version" of the above site.
Fortunately, after detailed examination and verification, I found that the Dapr Java SDK avoids most of the broken changes. JDK17 will be supported with only a few changes, which are listed below.
Upgrade jacoco plugin version
When building an existing Dapr Java SDK project(master branch) using JDK17, it fails with a large number of similar errors:
To fix this, upgrade jacoco-maven-plugin to the latest version:
Upgrade spotbugs-maven-plugin version
Similarly, spotbugs-maven-plugin will report errors under JDK17:
Refer to this issue from spotbugs: Fails with OpenJDK 17-ea: Unsupported class file major version 61 · Issue #1570 · spotbugs/spotbugs (github.com)
To fix this, upgrade spotbugs-maven-plugin to the latest version 4.8.2.0:
After version updated, spotbugs-maven-plugin reports a lot of EI_EXPOSE_REP/CT_CONSTRUCTOR_THROW/SS_SHOULD_BE_STATIC issues in various subprojects.
These code issues are not in the scope of this proposal, so this proposal will temporarily ignore these issues by setting up to exclude these rules in the configuration of spotbugs. These issues are recorded in detail in the appendix section of this proposal, and we can resolve them later: update the source code to meet the rule requirements or exclude them as inapplicable.
Other maven plugins version upgrade
There are also some other maven plugins that have been upgraded to new version. While some of these upgrades may not be necessary, they have all been updated along with above plugins to be safe.
Broken changes must be handled
In JDK11, some of the Java features and API are deprecated, but for the time being, they can continue to be used. After upgrading to JDK17, these features and API are dropped, so we must update our code to handle these broken changes.
For the items of Java removed, please reference to:
According to my testing, the main things that affect our Dapr Java sdk include (but are not limited to, it's possible I didn't check it thoroughly):
In Dapr Java SDK, we used two javax API:
In JDK17, these javax API are replaced with:
javax.annotation
In our Dapr Java SDK,
javax.annotation
used are:We have two options for these javax.annotation:
replace
Update the source code to replace javax.annotation with jakarta.annotation. To support Java8/Java11, add jakarta.annotation-api as dependency.
Keep
Continue to use javax**.annotation. To support JDK17, add javax.**annotation-api as denpendency.
These annotations are mainly used as "markup", and as long as they are compiled, there is no need to add the javax annotation-api even in JDK17 (where javax has been replaced by jakarta) if you don't actively manipulate them.
For example,
javax.annotation.Generated
are mainly used in grpc-generated code, such as in sdk-autogen subproject:Because we set sdk-autogen's source and target to JDK8, so there won't be a compilation problem here.
There is a special case that requires additional treatment. In examples subproject, if we change examples subproject to use JDK17, this will result in an error because
javax.annotation
cannot be found.This needs to be improved with gRPC and protocolBuffer to generate the code with jakarta.annotation. But so far this has not been supported, see: grpc/grpc-java#9179
To compile these auto-generated source code, we have no choice but to add
javax.annotation
as a dependency in examples subproject:In this proposal, I propose to keep these javax.annotation unchanged, and in the appendix section I will record the javax.annotations that found in Dapr Java SDK, so that we can change them in the future, if necessary.
javax.servlet
"javax.servlet" API is not a simple markup, they are used in runtime, so we can not only consider compiling. The common approach is to modify the source code to replace javax.servlet API to the corresponding jarkarta.servlet API.
Fortunately, we don't use javax.servlet in SDK Core, so this issue doesn't affect SDK Core. In our Dapr Java SDK,
javax.servlet
is used in examples and sdk-tests subproject.If we change to use JDK17 and springboot3.0, then we need to update the source code to use jakarta.servlet, like:
For examples subproject, if we decide to develop examples based on JDK17 and springboot3.0 only, then we only need to support JDK17 in the examples sub-project, we can udpate the source code to use jakarta.servlet.
challenges in sdk-tests subproject
For sdk-tests subproject, things are much more complicated.
In order to verify the compatibility of different springboot versions, the sdk-tests sub-project uses multiple springboot versions for testing, for example, the current verified versions (see springboot version metrics in appendicse) are: 2.7 / 2.6 / 2.5 / 2.4 / 2.3. After adding JDK17 and springboot 3.0 support, this springboot version metrics will have both springboot 3.0 and several springboot 2.x versions.
In the sdk-tests subproject we need to test for compatibility with multiple springboot versions at the same time, our testing code has to be considered to run on both JDK17 and JDK11. The Design to reuse the same testing code to cover different springboot versions runs into a very big challenge:
A typical example is the following OpenTelemetryInterceptor, which implements the HandlerInterceptor interface provided by the spring framework:
In springboot 3.0/spring6.0, it used jakarta.servlet:
In springboot 2.x / spring5, it used javax.servlet:
The best way to meet the requirements of the interface definition is to write two copies of the code for OpenTelemetryInterceptor, using javax.servlet and jakarta.servlet. This also means that we need to split the sdk-tests subproject into multiple projects.
Another tricky way is to use both javax.servlet and jakarta.servlet in the code implementation:
In this way, a single copy of the OpenTelemetryInterceptor code (which actually comes with both implementations) can work with both JDK17 and JDK8/jdk11.
Considering long-term planning, it is definitely more reasonable to split it into multiple sub-projects. The special code treatment above can only be a temporary solution. But this is the only special case in the entire sdk-tests sub-project (and really the entire Dapr Java SDK project). Splitting the subproject only for this one class is a suspicion of over-design, so I suggest to use the temporary solution in this proposal first, and then consider splitting sdk-tests into multiple subprojects if there are more similar scenarios.
Conclusion & Upgrade Plan
For the core subprojects (sdk / sdk-autogen / sdk-actros / sdk-workflow), since they were previously restricted to be compatible with Java8, adding support for JDK17 looks simple at the moment, requiring only upgrading individual maven plugins to the latest version / handing some javax annotation, with no other additional work required, especially almost no changes to existing Java code.
For examples and sdk-tests subproject, things will be a little more complicated, but the good news is that there is still a suitable solution.
Through above tests and verify, we got two good news:
Special Reminder: After discussion, we decided to remove Java8 support and don't support Springboot 2.x + Java17 in Dapr Java SDK.
Nice to havesupportedUpgrade Plan: the solution to add support for JDK17 would be very similar to the current solution that supports both Java8/Java11. JDK17 will replace the previous role of JDK11.
Support Springboot 3.0
After completing the JDK17 support, we went on to add springboot 3.0 support to the Dapr Java SDK.
Similar to the Java upgrade, the compatibility between springboot 3.0 and springboot 2.x is not perfect, and there are a lot of broken changes. A detailed description can be found in:
https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.0-Migration-Guide
However, since there are only a few essential functions used in the Dapr Java SDK, we are again lucky to avoid all the broken changes. Through my testing and validation, the code in the Dapr Java SDK is perfectly compatible with springboot 2.x and springboot 3.0, and we don't need to change any of the existing Java code.
From this point of view, it's OK to let customers using jdk17 and springboot3 continue to use the current sdk-springboot.
However, considering that we may use new features of springboot 3.0 later or encounter problems that must be handled differently in springboot2 or springboot3, it is still necessary to add a sdk-springboot30 subproject for springboot 3.0 support.
In order to make the responsibilities of the sub-projects clear, it is better to split another sdk-springboot-common sub-project to store the shared code, and sdk-springboot/sdk-springboot30 to store only the code or configurations that are specific to springboot2.x and springboot3.0.
In the future, if there is a need to support springboot3.1 and jdk21, then we can continue to follow this design by adding the sdk-springboot31 sub-project.
Therefore, we currently have three solutions to choose from, as shown below:
Short-term Solution 1
Do not change anything, and let springboot 3.0 users use the existing sdk-springboot directly. as shown above on the left.
The main disadvantage of the short-term solution 1 is that when we have to split the subproject, users of the springboot3.0 will have to modify their dependency from sdk-springboot to sdk-springboot30.
Short-term Solution 2
Add a new sdk-springboot30 sub-project for springboot 3.0 users, as shown in the middle of the image above.
The steps of the implementation are as follows:
At the moment, this sub-project is almost empty, but the advantage of introducing it is that springboot 3.0 users can use this dependency from now on and will not need to modify it in the future.
One thing to keep in mind during implementation is to avoid version conflicts of spring/springboot. The sdk-springboot30 subproject itself depends on springboot3.0/spring6, and since it depends on the sdk-springboot subproject, it also indirectly depends on springboot2.x/spring5.
There are two options:
Override the value of the springboot version property
When setting up the dependency on sdk-springboot, exclude springboot and spring
To be safe, both options can be implemented at the same time.
Long-term Solution
In addition to the short-term solution 2, add a sdk-springboot-common sub-project to store the shared code.
The steps of the implementation are as follows:
In the future, to support springboot3.1 and jdk21:
In this scenario, sdk-springboot-common's dependency on springboot2.x will be set to [provided scope](Maven – Introduction to the Dependency Mechanism (apache.org)), so dependencies on springboot2.x and spring5 will not be passed to the sdk-springboot / sdk-springboot30 / sdk- springboot31 subprojects.
The adoption of this long-term solution depends on the code and configuration specific to springboot 2.x / springboot 3.0 (and springboot 3.1 in the long term).
Conclusion & Upgrade Plan
Currently we only need to be compatible with springboot 3.0, will not use springboot3.0-specific features for the time being, so my suggestion is to adopt the Short-term solution 2.
In the future, if necessary, then migrate to the long-term solution.
Upgrade Plan:
Update examples subproject
For the examples subproject, I recommend upgrade the springboot version to 3.0, and let examples subproject depend on the sdk-springboot30 subproject. The upgrade is shown below:
The main reason for the upgrade is: it is not recommended to write duplicate examples for springboot 2.x and springboot 3.0.
Compatibility with springboot 2.x will be handled by the sdk-tests subproject.
From the practice of upgrading, the source code of the examples sub-projects can run directly with JDK17 and springboot 3.0, except for the broken changes related to the renaming of javax to jakarta.
For more implementation details, see PR: https://github.com/dapr/java-sdk/pull/975/files
Update sdk-tests subproject
In the previous section, "challenges in sdk-tests subproject", we discussed the fact that in order to perform compatibility tests on multiple springboot versions, the sdk-tests subproject needs to support both springboot 2.x and springboot 3.0.
Since springboot 2.x and springboot 3.0 use javax.x and jakarta.x respectively, the sdk-tests subproject should theoretically be split into 3 subprojects:
However, in the practice of upgrading, it was found that only one test case needed to be split. Almost all other test cases are compatible and should remain in the sdk-tests sub-project. And we found a somewhat strange but working solution to get the test code to use both javax.x and jakarta.x (see previous section, "challenges in sdk-tests subproject").
I suggest to use the temporary solution in this proposal first, and then consider splitting sdk-tests into multiple subprojects later if there are more similar scenarios:
Overview after Update
After completing the above update to add JDK17 and springboot 3.0 support, the version of JDK and springboot in the Dapr Java SDK project will be updated from:
to:
and ready for long-term evolution:
Appendices
Removing dependency of springboot from SDK Core
As mentioned earlier, springboot is introduced in the sdk-actors and sdk sub-projects for testing purposes.
There may be an optimization strategy here to move test code depends on springboot to the sdk-tests sub-project, thus removing the dependency of springboot from SDK Core:
After this improvement is made, the dependencies of the SDK Core will be clearer:
Note: This improvement is not in the scope of this proposal.
Springboot version matrix in sdk-tests
In the sdk-tests subproject, in order to test the compatibility of multiple springboot versions, several other 2.x versions will be tested in addition to the 2.7.8 version:
These springboot versions are defined in
.github/workflow/build.xml
asmatrix.spring-boot-version
:Then set to env
PRODUCT_SPRING_BOOT_VERSION
:And got by maven to override the default
spring-boot.version
property :So the versions of springboot that are verified in the sdk-tests project are:
After adding springboot 3.0 support, we need to update the springboot version matrix to add springboot 3.0.
In addition, after sdk-tests is compiled and run with JDK17, springboot versions 2.3 and 2.4 can't be tested because they don't support jdk17. So there are two options:
I personally recommend option 1: springboot 2.3 and 2.4 are very old.
This is the springboot version matrix after the upgrade:
Spotbugs bug pattern
After updating version of spotbugs-maven-plugin to support JDK17 , spotbugs-maven-plugin reports a lot of EI_EXPOSE_REP/CT_CONSTRUCTOR_THROW/SS_SHOULD_BE_STATIC issues in various subprojects.
Since no Java source code was changed during the JDK17 upgrade, these issues should have existed before and just been discovered this time.
Below are some of the issue reports collected:
To reproduce these errors, modify the "spotbugs-exclude.xml" file to remove the following bug patterns:
I would recommend reviewing these source code and spotbugs bug patterns. But that is not in the scope of this proposal, we can schedule the review later.
Update opentelemetry implementation
The opentelemetry currently used in the Dapr Java SDK is a very old version 0.14.0.
The result of running
mvn dependency:tree
is as follows:After upgrade to JDK and springboot 3.0, some versions of opentelemetry will change to a very new version 1.19.0:
And there are very significant differences and incompatibilities between these two versions that cause existing code to fail to compile.
To resolve this issue, this proposal temporarily enforces the use of opentelemetry version 0.14.0.
I would recommend upgrading the version of opentelemetry to 1.19.0, but that is not in the scope of this proposal. We can schedule the upgrade later.
The text was updated successfully, but these errors were encountered: