Skip to content

Fix instrumentation for WebFlux with Spring 6.x - #2936

Merged
eyalkoren merged 25 commits into
elastic:mainfrom
eyalkoren:WebFlux-with-spring-web-6
Jan 23, 2023
Merged

Fix instrumentation for WebFlux with Spring 6.x#2936
eyalkoren merged 25 commits into
elastic:mainfrom
eyalkoren:WebFlux-with-spring-web-6

Conversation

@eyalkoren

@eyalkoren eyalkoren commented Dec 25, 2022

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #2935
I locally managed to successfully run the entire WebFlux instrumentation plugin tests with spring-web 6.0.3 and spring-boot 3.0.1 with only this small change in the instrumentation, so I think it may be enough.

Adding the tests require some more efforts and we should probably add a dedicated spring-boot 3.x test and possibly upgrade the spring-boot we test with in other modules, so if this fix is verified, tests will be included in a separate PR.

Checklist

@eyalkoren eyalkoren added the bug Bugs label Dec 25, 2022
@eyalkoren

eyalkoren commented Dec 25, 2022

Copy link
Copy Markdown
Contributor Author

And a few more minutes thinking about it - it is insufficient to change the source, the bytecode would still contain the wrong method signature 🤦‍♂️
I'll add a separate module that compiles with spring-web 6 and only abstract out the int getStatusCode() functionality

@ghost

ghost commented Dec 25, 2022

Copy link
Copy Markdown

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-23T16:05:09.139+0000

  • Duration: 90 min 48 sec

Test stats 🧪

Test Results
Failed 0
Passed 8787
Skipped 113
Total 8900

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark tests.

  • run jdk compatibility tests : Run the JDK Compatibility tests.

  • run integration tests : Run the Agent Integration tests.

  • run end-to-end tests : Run the APM-ITs.

  • run windows tests : Build & tests on windows.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@eyalkoren

eyalkoren commented Dec 29, 2022

Copy link
Copy Markdown
Contributor Author

I used this fix to set up the stage to switch to the following state:

  • build and test on JDK 17 by default
  • enforce Java 11 syntax compatibility by default
  • build and test weekly on JDK 11
  • build and test weekly on JDK 19 (requires upgrade of jacoco and closes Building & testing project with JDK 18 #2328)
  • have an infrastructure that allows modules that must be compiled with Java 17

The latter was achieved through maven profile, as follows:

  • Java-17-dependent code will reside under a top-level module called apm-agent-java-17
  • The apm-agent-java-17 module is not meant to contain code, only to serve as a parent to other modules
  • One example for a module that requires compilation with Java 17 is apm-agent-spring-6, a child of apm-agent-java-17
  • We use the non-java-11 profile to exclude the apm-agent-java-17 module entirely from build when not running on Java 11. NOTE: this is a restriction as it assumes that the only pre-Java-17 we build with is 11
  • We use the non-java-11 profile also to include the modules with actual code, such as apm-agent-spring-6, when packaging

@eyalkoren eyalkoren added the ci:jdk-compatibility Enables JDK compatibility tests in build pipeline label Jan 3, 2023
@eyalkoren eyalkoren changed the title Avoid dependency on return type incompatible with spring-web 6.x Fix instrumentation for WebFlux with Spring 6.x Jan 4, 2023
@eyalkoren
eyalkoren requested a review from SylvainJuge January 12, 2023 08:13
@eyalkoren
eyalkoren marked this pull request as ready for review January 12, 2023 08:13
@github-actions

Copy link
Copy Markdown

/test

@eyalkoren

Copy link
Copy Markdown
Contributor Author

For now I treated this as a bug fix, postponing the documentation of it as supported to #2942

@eyalkoren
eyalkoren enabled auto-merge (squash) January 23, 2023 15:11
@eyalkoren
eyalkoren merged commit 9575d83 into elastic:main Jan 23, 2023
@eyalkoren
eyalkoren deleted the WebFlux-with-spring-web-6 branch January 24, 2023 06:13
v1v added a commit that referenced this pull request Jan 25, 2023
…actions

* upstream/main: (76 commits)
  Bump version.byte-buddy from 1.12.20 to 1.12.21 (#2953)
  Fix instrumentation for WebFlux with Spring 6.x (#2936)
  Fix SQS instrumentation within messaging transactions (#2979)
  Add test for unsampled Transaction (#2950)
  clarify about dropped requests for sampling in documentation (#2957)
  Doc add first stack trace detail (#2978)
  SpanBuilder.addLink is supported, so delete wrong comment (#2975)
  [doc] document log sending (#2929)
  Fix agent log sending (#2974)
  Bump version.aws.sdk from 2.19.8 to 2.19.18 in /apm-agent-plugins (#2973)
  test: synchronizing gherkin specs (#2965)
  Bump aws-java-sdk from 1.12.353 to 1.12.376 in /apm-agent-plugins (#2944)
  Fixed AgentOverheadMetricsTest flakyness (#2918)
  Fix AWS instrumentation destination.service.resource handling (#2947)
  [doc] restructure logs doc (#2916)
  Junit5 test with dependencies runner (#2962)
  Set service target for non-sampled ES client spans (#2959)
  Send logs via intake API (#2694)
  [doc] setup with docker image (#2956)
  Increase CI build timeout (#2955)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-java bug Bugs ci:jdk-compatibility Enables JDK compatibility tests in build pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix support for Spring boot 3.x

4 participants