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.

Sign up for GitHub

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

Jump to bottom

#3439 - Handle removal of ClientResponse#rawStatusCode and bump to Spring Boot to 3.2 #3440

Merged
merged 13 commits into from
Nov 29, 2023
Merged

#3439 - Handle removal of ClientResponse#rawStatusCode and bump to Spring Boot to 3.2 #3440

merged 13 commits into from
Nov 29, 2023

Conversation

NicklasWallgren
Copy link
Contributor

@NicklasWallgren NicklasWallgren commented Nov 24, 2023 •
edited
Loading

Handle removal of ClientResponse#rawStatusCode and bump to Spring Boot to 3.2

What does this PR do?

Handle removal of ClientResponse#rawStatusCode in Spring Framework 6.1

Checklist

  • This is an enhancement of existing features, or a new feature in existing plugins
    • I have updated CHANGELOG.asciidoc
    • I have added tests that prove my fix is effective or that my feature works
    • Added an API method or config option? Document in which version this will be introduced
    • I have made corresponding changes to the documentation
  • This is a bugfix
  • This is a new plugin
    • I have updated CHANGELOG.asciidoc
    • My code follows the style guidelines of this project
    • I have made corresponding changes to the documentation
    • I have added tests that prove my fix is effective or that my feature works
    • New and existing unit tests pass locally with my changes
    • I have updated supported-technologies.asciidoc
    • Added an API method or config option? Document in which version this will be introduced
    • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.
  • This is something else

@cla-checker-service CLA Checker Service
Copy link

cla-checker-service bot commented Nov 24, 2023 •
edited
Loading

💚 CLA has been signed

@github-actions github-actions bot added agent-java community Issues and PRs created by the community triage labels Nov 24, 2023
@github-actions GitHub Actions
Copy link

👋 @NicklasWallgren Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

@NicklasWallgren NicklasWallgren force-pushed the 3439-handle-removal-of-raw-status-code branch from bfcc8ee to ddc4a5b Compare November 24, 2023 11:14
@NicklasWallgren NicklasWallgren changed the title #3439 - Handle removal of ClientResponse#rawStatusCode and bump to Spring Boot to 3.5 #3439 - Handle removal of ClientResponse#rawStatusCode and bump to Spring Boot to 3.2 Nov 24, 2023
@JonasKunz
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@KaVeKa KaVeKa mentioned this pull request Nov 29, 2023
Closed
@JonasKunz
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@JonasKunz
Copy link
Contributor

@NicklasWallgren I hope you don't mind that I continued the work on your PR. Unfortunately due to backwards compatibility requirements with Spring 5, the fix wasn't quiet as simple.

JonasKunz
JonasKunz reviewed Nov 29, 2023
View reviewed changes
...webclient-plugin/src/main/java/co/elastic/apm/agent/springwebclient/WebClientSubscriber.java
@@ -192,5 +193,19 @@ private void cancelSpan() {
debugTrace(false, "cancelSpan", span);
}
}
@SuppressWarnings("unchecked")
Copy link
Contributor

Choose a reason for hiding this comment

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

[For Reviewer] This is suddenly required because onNext doesn't declare checked exceptions anymore. For backwards compatibility reasons, I figured it would be best to still continue throwing unwrapped exceptions.

Copy link
Member

Choose a reason for hiding this comment

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

May be worth adding a comment to make that a bit more explicit and document which version introduced it.

@JonasKunz JonasKunz requested a review from a team November 29, 2023 08:15
@JonasKunz JonasKunz linked an issue Nov 29, 2023 that may be closed by this pull request
Spring Boot 3.2 - WebClientSubscriber - java.lang.NoSuchMethodError #3439
Closed
SylvainJuge
SylvainJuge previously approved these changes Nov 29, 2023
View reviewed changes
Copy link
Member

@SylvainJuge SylvainJuge left a comment

Choose a reason for hiding this comment

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

LGTM, only a few minor details.

...bitmq/apm-rabbitmq-spring/src/test/java/co/elastic/apm/agent/rabbitmq/SpringAmqpBatchIT.java Outdated Show resolved Hide resolved
...webclient-plugin/src/main/java/co/elastic/apm/agent/springwebclient/WebClientSubscriber.java
@@ -192,5 +193,19 @@ private void cancelSpan() {
debugTrace(false, "cancelSpan", span);
}
}
@SuppressWarnings("unchecked")
Copy link
Member

Choose a reason for hiding this comment

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

May be worth adding a comment to make that a bit more explicit and document which version introduced it.

...webclient-plugin/src/main/java/co/elastic/apm/agent/springwebclient/WebClientSubscriber.java Outdated Show resolved Hide resolved
...m-spring-webflux-spring5/src/main/java/co/elastic/apm/agent/springwebflux/WebfluxHelper.java Outdated Show resolved Hide resolved
@JonasKunz
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@NicklasWallgren
Copy link
Contributor Author

@NicklasWallgren I hope you don't mind that I continued the work on your PR. Unfortunately due to backwards compatibility requirements with Spring 5, the fix wasn't quiet as simple.

No worries, I just wanted to get things rolling 🙂

Thanks for your work!

@JonasKunz JonasKunz merged commit c5a8373 into elastic:main Nov 29, 2023
20 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonasKunz JonasKunz JonasKunz left review comments

@SylvainJuge SylvainJuge SylvainJuge approved these changes

Assignees
No one assigned
Labels
agent-java community Issues and PRs created by the community ready-to-merge triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spring Boot 3.2 - WebClientSubscriber - java.lang.NoSuchMethodError
3 participants
@NicklasWallgren @JonasKunz @SylvainJuge