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

Bump supported Spring Boot baseline to version 3.2.x #1050

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

ThomasVitale
Copy link
Contributor

Description

Spring Boot < 3.2.x reached end of life and it's not maintained anymore. This PR bumps the minimum supported version to 3.2.x, supporting both the 3.2.x and 3.3.x release trains.

  • Bump dependencies in sdk-springboot module from Spring Boot 2.x to 3.2.6
  • Update examples module to use new Spring Boot support with Java 17 baseline
  • Fix wrong sdkman and maven wrapper setup that failed the local setup
  • Update GHA workflow to stop testing for Spring Boot versions < 3.2.x

Issue reference

Fixes gh-1039

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@salaboy
Copy link
Contributor

salaboy commented Jun 15, 2024

@artursouza I just checked this PR.. and this is what we need to do to upgrade to spring 3.2.x .. We will start refactoring the spring boot support, trying not to break existing users.

@salaboy
Copy link
Contributor

salaboy commented Jun 15, 2024

@artursouza can you please review?

@cicoyle
Copy link
Contributor

cicoyle commented Jun 19, 2024

Looks like CI is failing the license/synk check, but I can't view it. I'm fine with proceeding with this change given if there are vulnerabilities in Spring Boot < 3.2.x it won't be fixed and users have had some time to upgrade by now.

@ThomasVitale
Copy link
Contributor Author

@cicoyle thanks for the review. I can't see the Snyk report either. @salaboy do you have access? I'm not sure which dependency is triggering the failure.

@salaboy
Copy link
Contributor

salaboy commented Jun 23, 2024

@cicoyle when you have time can you please validate this one? now that the HTTP client is removed .. we need to upgrade the spring boot version.

@salaboy
Copy link
Contributor

salaboy commented Jun 26, 2024

@cicoyle thanks for the review. I can't see the Snyk report either. @salaboy do you have access? I'm not sure which dependency is triggering the failure.

Let me ask for permission

@salaboy
Copy link
Contributor

salaboy commented Jun 26, 2024

Screenshot 2024-06-26 at 15 35 52

@salaboy
Copy link
Contributor

salaboy commented Jun 26, 2024

@ThomasVitale I am working out on the licence stuff.. but it seems that dapr-sdk-tests is still trying to get Could not find artifact io.dapr:dapr-sdk-springboot3:jar:1.12.0-SNAPSHOT which might be because you forgot to update that dependency. Here: https://github.com/dapr/java-sdk/blob/master/sdk-tests/pom.xml#L132

@artursouza
Copy link
Member

@ThomasVitale I am working out on the licence stuff.. but it seems that dapr-sdk-tests is still trying to get Could not find artifact io.dapr:dapr-sdk-springboot3:jar:1.12.0-SNAPSHOT which might be because you forgot to update that dependency. Here: https://github.com/dapr/java-sdk/blob/master/sdk-tests/pom.xml#L132

I fixed it in FOSSA. Ignore Snyk's report on license, I disabled that going forward. Snyk is used for vulnerability reports only.

@ThomasVitale
Copy link
Contributor Author

ThomasVitale commented Jun 26, 2024

@salaboy thanks, I had missed that sdk-tests was excluded from the multi-module Maven project, so the tests didn't actually run as part of the local workflow. I tried to set them up locally based on the pipeline content, but I didn't manage to do it fully, I'm afraid. I pushed a change to update the dependencies in dk-tests, and remove sdk-springboot3. Can you re-run the PR pipeline?

@artursouza
Copy link
Member

@salaboy thanks, I had missed that sdk-tests was excluded from the multi-module Maven project, so the tests didn't actually run as part of the local workflow. I tried to set them up locally based on the pipeline content, but I didn't manage to do it fully, I'm afraid. I pushed a change to update the dependencies in dk-tests, and remove sdk-springboot3. Can you re-run the PR pipeline?

This is a breaking change but it might make sense for the Java community to simply move away from Springboot 2.

@salaboy
Copy link
Contributor

salaboy commented Jun 26, 2024 via email

@artur-ciocanu
Copy link
Contributor

artur-ciocanu commented Jun 27, 2024

@salaboy and @ThomasVitale I have spent some time looking at the build failures. According to this document: https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#parameter-name-retention things have changed in Spring Framework 6.x when it comes to parameter name resolution. We have to either use Java Compiler with -parameters flag or ensure that @PathVariable and @RequestPathParam are explicitly named. Here is the IT related file that has to be adjusted:

.

I have run an experiment using explicit naming that can be found here: https://github.com/artur-ciocanu/java-sdk/actions/runs/9696067245/job/26757186160. The GitHub Action succeeded, so we can at least try to fix these issues in the near term.

I am still looking into other build failures. I'll keep you posted.

CC: @artursouza @cicoyle

@artur-ciocanu
Copy link
Contributor

@salaboy and @ThomasVitale I have spent a few hours figuring out why the tests are failing. Here are my findings:

  • "parameter name resolution" - the are changes in SB 2.x and SB 3.x it is due to Spring Framework 6.x changes. The fixes are trivial I have suggested them in the previous comment.
  • OpenTelemetry version mismatch - it seems that as part of SB 3.x upgrade we get a new version of Zipkin reporter and this conflicts with OTEL version that is currently used in sdk-tests. Here is the error that I managed to reproduce:
Error:  Errors: 
Error:    TracingIT.testInvoke:60 » NoSuchMethod 'zipkin2.Call zipkin2.reporter.Sender.sendSpans(java.util.List)'
Error:    TracingIT.testInvoke:58 » NoSuchMethod 'zipkin2.Call zipkin2.reporter.Sender.sendSpans(java.util.List)'

My suggestion for the second issue would be to upgrade OTEL to latest library, but this requires some significant changes in sdk-tests since currently it uses 0.14.0 while the latest is 1.39.0.

@artursouza and @cicoyle what are your thoughts.

@salaboy
Copy link
Contributor

salaboy commented Jun 27, 2024

@artur-ciocanu I wonder if we can rely on the OTEL stuff that comes with Spring Boot 3.x instead of adding our own dependency

@artur-ciocanu
Copy link
Contributor

artur-ciocanu commented Jun 27, 2024

@artur-ciocanu I wonder if we can rely on the OTEL stuff that comes with Spring Boot 3.x instead of adding our own dependency

@salaboy I was thinking about it ... I guess for Spring Boot based IT tests we can, but I would have to defer to @artursouza and others, since I don't know why we had to use OTEL in its "raw" form. I am assuming because of GRPC tests, but I am not sure.

In any case if we upgrade OTEL the IT tests have to be adjusted.

@artur-ciocanu
Copy link
Contributor

@cicoyle @artursouza or @ThomasVitale could you please try to merge the "master" and see if we still have any issues with Spring Boot upgrade.

@ThomasVitale
Copy link
Contributor Author

@artur-ciocanu thanks, I'll try that later today

@ThomasVitale ThomasVitale force-pushed the gh-1039 branch 2 times, most recently from 2ae46fc to 53a4c74 Compare July 4, 2024 15:47
@ThomasVitale
Copy link
Contributor Author

ThomasVitale commented Jul 4, 2024

@artur-ciocanu I have rebased this PR on the current "master" and pushed the changes

@artur-ciocanu
Copy link
Contributor

@artursouza and @cicoyle it seems that we need maintainers approval. The build workflows are blocked right now.

Whenever you have a chance, could you please help? Thank you.

@artur-ciocanu
Copy link
Contributor

@ThomasVitale we are almost there 😀. It seems that there is still one IT test that is failing. It is related to SDK resiliency. I will take a look on Monday and see if I can reproduce it and come up with a fix.

@salaboy
Copy link
Contributor

salaboy commented Jul 6, 2024

Thanks so much for the help @artur-ciocanu 💪💪

@artur-ciocanu
Copy link
Contributor

@ThomasVitale and @salaboy I have opened #1070 to try to fix the SDK resiliency IT failure. Let's wait until it is merged and see if it will fix the issue.

@artur-ciocanu
Copy link
Contributor

@ThomasVitale the SDK resiliency IT fix PR has been merged. Could you please merge "master" and see if things are looking good.

* Bump dependencies in sdk-springboot module from Spring Boot 2.x to 3.2.6
* Update examples module to use new Spring Boot support with Java 17 baseline
* Fix wrong sdkman and mavne wrapper setup that failed the local setup
* Update GHA workflow to stop testing for Spring Boot versions < 3.2.x

Fixes gh-1039

Signed-off-by: Thomas Vitale <ThomasVitale@users.noreply.github.com>
@ThomasVitale
Copy link
Contributor Author

@artur-ciocanu thanks! I've just rebased the PR

@artur-ciocanu
Copy link
Contributor

@artursouza or @cicoyle it seems that we need maintainers approval to kick off the GitHub workflows. Could you please approve and if everything looks merge the PR.

Thank you! 🙇‍♀️

@yaron2
Copy link
Member

yaron2 commented Jul 11, 2024

Need to check why the build is failing

@artursouza
Copy link
Member

Build is still failing. I have no problem merging this once validation passes.

@artur-ciocanu
Copy link
Contributor

Let me check tomorrow one more time to see what might be the issue. Thanks for checking @artursouza 🙇

@ThomasVitale
Copy link
Contributor Author

Thanks! I would like to help debug the integration tests, but I didn't manage to run them locally. I have suggested an improvement for the docs on how to establish a local setup for running the sdk-tests project. #1075

@artur-ciocanu
Copy link
Contributor

@artursouza and @yaron2 I have opened a tiny PR #1077 to add failure message for the failed IT test. Also I have tried to build #1050 from my fork and the GitHub workflow completed successfully. Here are is the build run: https://github.com/artur-ciocanu/java-sdk/actions/runs/9904795676.

If you could review PR #1077 and merge it to master then we could at least see what is the underlying problem.

Thank you!

@artur-ciocanu
Copy link
Contributor

@artursouza sorry for this back and forth, but you could you please merge master and retrigger the GitHub workflows.

Much appreciated.

@artur-ciocanu
Copy link
Contributor

@artursouza sorry for this back and forth, but could you please merge master and retrigger the GitHub workflows.

Thank you, much appreciated. 🙇‍♂️

@artur-ciocanu
Copy link
Contributor

artur-ciocanu commented Jul 12, 2024

@artursouza as far as I can see the Spring Boot 3.x build was successful. The other Spring Boot builds like 2.5, 2.6, 2.7, 3.0 are marked as Expected — Waiting for status to be reported. However these have been removed from the test matrix in the GitHub workflow. Also if I go to https://github.com/dapr/java-sdk/pull/1050/checks everything seems to be 🟢 .

Do you think it is safe to merge this PR that the SB 3.2 and SB 3.3 builds are successful?

@artur-ciocanu
Copy link
Contributor

@artursouza and @cicoyle whenever you have a few minutes could you please take a final look, approve and merge. Thank you.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>
@salaboy
Copy link
Contributor

salaboy commented Jul 16, 2024

@cicoyle can you please review this?

@artursouza artursouza added this to the v1.13 milestone Jul 16, 2024
@artursouza
Copy link
Member

OK. This is going to release 1.12.

@artur-ciocanu
Copy link
Contributor

@artursouza it seems to be 🟢 !!!

@artursouza artursouza merged commit 6e0f267 into dapr:master Jul 16, 2024
7 checks passed
@marcduiker
Copy link
Contributor

@holopin-bot @ThomasVitale Thank you Thomas!

Copy link

holopin-bot bot commented Aug 15, 2024

Congratulations @ThomasVitale, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/clzvaaix033460cih33mhkpwg

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

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

Successfully merging this pull request may close these issues.

Drop Spring Boot 2.x support as it is not longer supported by the spring community
7 participants