-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Support Java 25 for build, test, and Docker publishing #13671
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
Conversation
|
You need to resolve conflicts. |
|
Look like one UT fails. Could you verify locally? |
Thanks for the heads-up. |
I verified this locally. The unit test failure was indeed due to the old Mockito 4.x / ByteBuddy 1.14 dependencies being incompatible with JDK 25. |
|
Update of JDK 25 Support Fixes: I have successfully verified this build locally on both JDK 21 and JDK 25. To achieve compatibility without disrupting the existing project baseline, I implemented the following:
|
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <version>5.11.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move all dependency version update into bom. Each module should not have its specific version.
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>net.bytebuddy</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for bytebuddy. You should update bom, dependency management.
| Whitebox.setInternalState(MetricsStreamProcessor.class, "PROCESSOR", | ||
| Mockito.spy(MetricsStreamProcessor.getInstance()) | ||
| ); | ||
| // Fix for JDK 25 / Mockito 5: Prevent double-spying on the singleton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About comments, please align with the code indentation.
oap-server/server-core/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> | ||
| <version>5.11.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same version management required, same as previous.
| <build> | ||
| <pluginManagement> | ||
| <plugins> | ||
| <plugin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is added to enable Lombok annotation processing explicitly. Without it, the E2E Java test service fails to compile under JDK 25 (missing Lombok-generated methods like builder()), while older JDKs work implicitly. This keeps the behavior consistent across JDK versions.
We prefer consistent and unified version in the project level. Please update by following this principal.
|
|
And there is a mock error in the CI. please recheck it. |
…zerTest indentation
…legrafMetricsTest
Thanks for the review. I’ve updated the PR to:
I’ll recheck and follow up if anything else fails. |
|
There are some test errors of e2e codes. There are some mock app in Java as well. When you changed the JDK to compile the whole thing, it fails. https://github.com/apache/skywalking/blob/master/test/e2e-v2/java-test-service/pom.xml |
|
And UT(MeterProcessorTest) fails according to CI logs. |
Thanks for pointing this out. |
…annotation processing
|
What is the issue actually? I think when you run the compiled codes, it is eventually in JDK25. |
MeterProcessorTest uses Groovy, and Groovy parses / analyzes classes using ASM during semantic analysis. When running on JDK 25, the Java compiler emits class file major version 69, but the Groovy version we currently use doesn’t yet recognize that class version. As a result, Groovy fails with Unsupported class file major version 69 before the test logic executes, even though the JVM itself can run the code fine on JDK 25. So this isn’t a runtime incompatibility with JDK 25, but a tooling limitation in Groovy’s bytecode reader. That’s why the test passes unchanged on JDK 11 / 17 / 21, but crashes during Groovy’s semantic analysis phase on 25. I avoided upgrading Groovy in this PR since that would be a broader dependency change with higher risk. Once Groovy officially supports Java 25 bytecode, the skip can be safely removed. |
|
This is a pre-task because we planned to run the whole OAP in the JDK dedicatedly, instead of compiling in JDK11 level byte codes. Because we want to try virtual thread. |
|
https://groovy-lang.org/releasenotes/groovy-5.0.html I did some research. And groovy v5 seems out for months. Maybe you could start that in a new PR first, and then back to this? An important NOTICE, Groovy 5 requires JDK17+ to build and JDK11 is the minimum version of the JRE that we support. Groovy 5 has been tested on JDK versions 11 through 25. |
Thanks for the research and the clarification — that makes sense. Agreed that upgrading Groovy is a broader change and should be handled separately. I’ll start a new PR to evaluate upgrading to Groovy 5, taking into account the JDK 17 build requirement and continued JDK 11 runtime support. Once that’s clearer, I’ll come back to this JDK 25 PR accordingly. |
…13679) ### Evaluation Purpose This PR is an extension to another PR ( #13671 ) and evaluates upgrading the Groovy engine from **4.0.x to 5.0.0 (earliest GA)** with a focus on validating **SkyWalking’s JDK 11 runtime compatibility guarantee**. Groovy is used as a runtime dependency in several OAP backend modules, so this change is evaluated strictly from a **compatibility and policy** perspective. ### Verification Results Local evaluation was performed using **JDK 21 for build** and **JDK 11 for runtime**, targeting the primary Groovy consumers (`log-analyzer` and `meter-analyzer`). - **Build Status:** SUCCESS (built with JDK 21) - **Runtime Compatibility:** Verified that the **OAP distribution starts successfully on JDK 11** with Groovy 5.0.0. - *Test:* Built full distribution (`-Pdist`) on JDK 21, then launched OAP via `oapService.bat` using JDK 11. - *Result:* Server started successfully and bound ports without any `UnsupportedClassVersionError` or JVM compatibility failures. - **Tests Passed:** - `oap-server/analyzer/log-analyzer` (LAL script parsing) - `oap-server/analyzer/meter-analyzer` (MAL script execution) ### Key Findings - No compilation issues observed in backend modules. - No Groovy/Spock incompatibilities detected in the existing test suite. - Groovy 5.0.0 does not introduce runtime incompatibilities with JDK 11 based on the evaluated scope. ### Scope Notes This PR is intended solely as a **compatibility evaluation** and does not include test refactors, additional Groovy 5.x upgrades, or behavioral changes.
|
Resuming this PR as discussed. I’ve merged the latest master (which includes the Groovy 5.0.3 upgrade from PR #13679 ) into this branch. Happy to adjust based on the results. |
|
It seems e2e-service-provider compile failure under E2E test 25 |
Thanks for confirming. Since the failure occurs during maven-compiler-plugin:3.11.0:compile for e2e-service-provider under JDK 25, I’ll first try upgrading the compiler plugin for this module to check JDK 25 compatibility before touching dependencies. |
Co-authored-by: Wan Kai <wankai123@foxmail.com>
|
The E2E Java services fail to compile on JDK 25 due to Lombok ≤ 1.18.36 relying on removed javac internals (com.sun.tools.javac.code.TypeTag), which triggers ExceptionInInitializerError during annotation processing. To address this without impacting LTS JDKs, I added a JDK-25-only Maven profile that:
This keeps the E2E services buildable on JDK 25 while preserving the standard build process for all supported LTS versions. |
|
E2E test 25 failed According to the CI, java-agent used the same JDK version with OAP. skywalking/test/e2e-v2/script/env Line 16 in 4418152
Java agent repo:https://github.com/apache/skywalking-java |
Updated SW_AGENT_JAVA_COMMIT to 2a61027e5eb74ed1258c764ae2ffeabd499416a6 as suggested to align Java agent image with JDK 25. |
|
Thanks for rerunning — all checks are green now. Please let me know if anything else is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for building, testing, and publishing SkyWalking with Java 25 across the entire project infrastructure. The changes upgrade key dependencies (Lombok, Mockito, Byte Buddy) to versions compatible with JDK 25, fix test code to work with newer Mockito APIs, update CI/CD workflows to test with Java 25, and add Docker image publishing for Java 25.
Changes:
- Upgraded core dependencies (Lombok to 1.18.40, Mockito to 5.11.0, Byte Buddy to 1.17.0) and configured Maven compiler plugin with explicit annotation processor paths
- Updated CI workflow matrices to include Java 25 for unit tests, integration tests, and E2E tests
- Added Java 25 Docker image build and publish steps, and updated E2E test agent commit reference
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Upgraded lombok, mockito-core, and byte-buddy versions; removed mockito-inline dependency; configured maven-compiler-plugin with annotationProcessorPaths for JDK 25 compatibility |
| test/e2e-v2/java-test-service/pom.xml | Upgraded lombok to 1.18.36 and maven-compiler-plugin; added JDK 25 profile with delombok configuration; reformatted repository section |
| oap-server/server-receiver-plugin/skywalking-telegraf-receiver-plugin/src/test/java/org/apache/skywalking/oap/server/receiver/telegraf/TelegrafMetricsTest.java | Fixed Mockito usage for JDK 25 compatibility by removing spy wrapper on MetricsStreamProcessor and changing spy to mock for CoreModule |
| oap-server/analyzer/meter-analyzer/src/test/java/org/apache/skywalking/oap/meter/analyzer/dsl/AnalyzerTest.java | Added conditional spy logic to prevent double-spying on MetricsStreamProcessor singleton |
| oap-server/analyzer/agent-analyzer/src/test/java/org/apache/skywalking/oap/server/analyzer/provider/meter/process/MeterProcessorTest.java | Changed to use mock instead of spy for MetricsStreamProcessor; reorganized imports |
| .github/workflows/skywalking.yaml | Added Java 25 to test matrices for unit tests, integration tests, E2E tests, and Docker builds |
| .github/workflows/publish-docker.yaml | Added Java 25 Docker image build and push step |
| docs/en/guides/How-to-build.md | Updated documentation to include JDK 25 in supported versions list |
| docs/en/changes/changes.md | Added changelog entry for Java 25 support |
| test/e2e-v2/script/env | Updated SW_AGENT_JAVA_COMMIT to version supporting Java 25 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
wankai123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Feature Description
This PR introduces comprehensive support for JDK 25 across the build system, CI, and Docker release workflows, while maintaining backward compatibility for JDK 11-21.
Changes:
Build Configuration (
pom.xml):lombokto1.18.40to support JDK 25.maven-compiler-pluginto explicitly defineannotationProcessorPaths. This resolves compilation issues where the annotation processor was not correctly discovered on newer JDKs.maven.compiler.sourceandtargetremain at11to ensure the project can still be built and run on older LTS versions (11, 17, 21).CI Matrices (
skywalking.yaml):25to thejava-versionmatrix for Unit Tests, Integration Tests, and E2E Tests.Docker Release (
publish-docker.yaml):eclipse-temurin:25-jre.Documentation:
How-to-build.mdto include JDK 25 in the supported versions list.changes/changes.md.Testing:
mvn clean packagepasses locally.mvn clean packagepasses locally (backward compatibility confirmed).Checklist