Bump Spring and Jetty dependencies #5372

ikhoon · 2024-01-08T06:54:55Z

Motivation:

Spring 6 HTTP client is split into two parts.
#4838 (comment) It will be easier to review if Spring dependencies are upgraded before working on Spring HTTP 6 integration.

Dependencies:

Spring 6.0.13 -> 6.1.2
Spring Boot 2.7.16 -> 2.7.18, 3.1.4 -> 3.2.1
Jetty 12.0.5

Modifications:

Spring Boot Actuator
- @AutoConfigureMetrics has been removed in favor of @AutoConfigureObservability. Disable tracing in integration tests spring-projects/spring-boot#31308
- @EnableTestMetrics is added in Spring Boot 2 and 3 modules to share the same test code.
  - @EnableTestMetrics in boot2-actuator-autoconfigure inherits @AutoConfigureMetrics but @AutoConfigureObservability is inherited in boot3-actuator-autoconfigure.
Spring Boot WebFlux
- AbstractServerHttpRequestVersionSpecific is added to shim the API changes in Spring 6.1
- Some return types are wrapped in Mono in test REST APIs. Otherwise, the service method is invoked with a blocking executor of Spring WebFlux. spring-projects/spring-framework@b016f38#diff-8fd6b8e3408492ba1cabbbea613ac33b7c674db0a9287a2d479e83ec55c1544eR253-R254
Jetty 12
- Jetty version has been upgraded to 12 in Spring Boot 3.2. Upgrade to Jetty 12 spring-projects/spring-boot#36073
- Jetty core module is now independent of Servlet API. https://webtide.com/introducing-jetty-12/
  - Due to the change, the existing JettyService code can no longer be reused, so JettyService is newly implemented based on the Jetty 12 API.
- JettyServiceBuilder
  - JettyServiceBuilder.tlsReverseDnsLookup() is removed because Jetty ServletApiRequest.getRemoteHost() does not perform a reverse DNS lookup.
  - JettyServiceBuilder.handlerWrapper(HandlerWrapper) has been removed in favor of JettyServiceBuilder.insertHandler(Handler.Singleton)
    - HandlerWrapper does not exist in Jetty 12.
    - insertHandler(Handler.Singleton) is the most similar replacement.
  - JettyServiceBuilder.sessionIdManager(SessionIdManager) and sessionIdManagerFactory(Function) have been removed because Server.setSessionIdManager() does not exist.
    - Instead, SessionIdManager can be set via JettyServiceBuilder.bean() https://eclipse.dev/jetty/documentation/jetty-12/programming-guide/index.html#pg-server-session-idmgr
- JettyService
  - Jetty Request becomes immutable so Armeria request headers are indirectly set via HttpChannel.onRequest()
  - HttpStreamOverHTTP1 provides a request body instead of HttpChannelOverHttp.
  - A response is now written to HttpStreamOverHTTP1.send(...). Previously, HttpConnection.send(...) was used.
  - Failed to find the counterpart of jReq.setDispatcherType() and jReq.setAsyncSupported().
    - Without setAsyncSupported, async could support via Callback interface.
  - httpChannel.handle() has been replaced with httpChannel.onRequest(requestMetadata).run().

Motivation: Spring 6 HTTP client is split into two parts. line#4838 (comment) It will be easier to review if Spring dependencies is upgraded before working on Spring HTTP 6 integration. - Spring 6.0.13 -> 6.1.12 - Spring Boot 2.7.16 -> 2.7.18, 3.1.4 -> 3.2.1

github-actions · 2024-01-08T06:58:35Z

🔍 Build Scan® (commit: `b567dbf`)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/iab4xx6tefpeq
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/lcjceyrc7brow
build-self-hosted-unsafe-jdk-19-snapshot-blockhound	✅	https://ge.armeria.dev/s/aq5atkd2oqktw
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/e4qpfdoukcnec
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/ddnxryyph5le4
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/dzlhycoed5okc
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/njhb2a6cc44uq
build-macos-12-jdk-19	✅	https://ge.armeria.dev/s/sbdsoq5gjxt54

codecov · 2024-01-15T13:58:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (30c9718) 0.00% compared to head (b567dbf) 73.77%.
Report is 27 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5372       +/-   ##
===========================================
+ Coverage        0   73.77%   +73.77%     
- Complexity      0    20460    +20460     
===========================================
  Files           0     1776     +1776     
  Lines           0    75521    +75521     
  Branches        0     9628     +9628     
===========================================
+ Hits            0    55719    +55719     
- Misses          0    15181    +15181     
- Partials        0     4621     +4621

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ikhoon · 2024-01-15T15:07:47Z

...y/jetty12/src/test/java/com/linecorp/armeria/server/jetty/AsyncStreamingHandlerFunction.java

+                                               .getBytes(StandardCharsets.US_ASCII);
+
+    @Override
+    public boolean handle(Request req, Response res, Callback callback) throws Exception {


The logic is similar to AsyncStreamingHandlerFunction in Jetty 11 module but the super interface has been changed.

ikhoon · 2024-01-15T15:54:05Z

Sorry for the massive change in a single PR. 😅
Jetty 12 module was added in this PR to test compatibility with Spring Boot 3.2.

…ring-deps

jrhee17

Looks good overall! Left some minor comments 🙇

...nfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java

.../java/com/linecorp/armeria/spring/web/reactive/AbstractServerHttpRequestVersionSpecific.java

jetty/jetty12/src/main/java/com/linecorp/armeria/server/jetty/AbstractArmeriaEndPoint.java

jrhee17 · 2024-01-22T08:25:00Z

jetty/jetty12/src/main/java/com/linecorp/armeria/server/jetty/AbstractJettyServiceBuilder.java

+/**
+ * A skeletal builder implementation for {@link JettyServiceBuilder} in Jetty 9 and Jetty 10+ modules.
+ */
+public abstract class AbstractJettyServiceBuilder {


Question) What do you think of moving the deprecated methods to jetty11/JettyServiceBuilder and sharing the implementation of AbstractJettyServiceBuilder between jetty11 and jetty12?

JettyServiceBuilder exists in both jetty11 and jetty9. Let me move the deprecated methods to them.

minwoox

The changes look good to me. Thanks a lot! 🙇

codefromthecrypt

Can you also update micrometer deps as the latest patch of spring boot causes a conflict with what we have.

-micrometer = "1.11.5"
-micrometer-tracing = "1.1.6"
-micrometer-docs-generator = "1.0.2"
+micrometer = "1.12.2"
+micrometer-tracing = "1.2.2"
+micrometer-docs-generator = "1.0.3"

Noticed this in zipkin until I bumped micrometer, too

 java.lang.NoSuchMethodError: 'void io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics.<init>(java.util.concurrent.ExecutorService, java.lang.String, java.lang.String, java.lang.Iterable)'
	at com.linecorp.armeria.server.DefaultServerConfig.monitorBlockingTaskExecutor(DefaultServerConfig.java:331)

dependencies.toml

cj848 · 2024-01-24T01:26:58Z

I'm just waiting for this PR to be merged.

ikhoon · 2024-01-24T12:21:00Z

Can you also update micrometer deps as the latest patch of spring boot causes a conflict with what we have.

Sure, I will bump all other dependencies in a dependency upgrade PR.

Co-authored-by: Crypt Keeper <[email protected]>

jrhee17

Looks great! Thanks @ikhoon 🙇 👍 🙇 (bar the lint failures)

ikhoon · 2024-01-25T04:57:58Z

The lint failure is fixed in the main branch with e3ba6ec

codefromthecrypt · 2024-01-25T05:01:14Z

woot 🥇

Bump Spring dependencies

d046225

Motivation: Spring 6 HTTP client is split into two parts. line#4838 (comment) It will be easier to review if Spring dependencies is upgraded before working on Spring HTTP 6 integration. - Spring 6.0.13 -> 6.1.12 - Spring Boot 2.7.16 -> 2.7.18, 3.1.4 -> 3.2.1

ikhoon added the dependencies label Jan 8, 2024

ikhoon added this to the 1.27.0 milestone Jan 8, 2024

Merge branch 'main' into up-spring-deps

534d3e8

ikhoon added 9 commits January 8, 2024 15:58

Add comment

33d1024

fix typo

d2e535a

--wip-- [skip ci]

f712060

Add Jetty 12 module

7bdaaf3

Handle breaking changes in Spring actuator

83ce66c

clean up

6fb0819

Fix test failures

76f87fb

Mono wraps normal types

49607ca

Fix a wrong Jetty dependency

11c72ba

Merge branch 'main' into up-spring-deps

89e2112

ikhoon changed the title ~~Bump Spring dependencies~~ Bump Spring and Jetty dependencies Jan 15, 2024

ikhoon commented Jan 15, 2024

View reviewed changes

ikhoon marked this pull request as ready for review January 15, 2024 15:51

ikhoon requested review from jrhee17, minwoox and trustin as code owners January 15, 2024 15:51

ikhoon added 3 commits January 16, 2024 14:31

Remove cruft

5f00129

Merge branch 'up-spring-deps' of github.com:ikhoon/armeria into up-sp…

ea7b026

…ring-deps

Fix a compile error

5e163f3

jrhee17 reviewed Jan 22, 2024

View reviewed changes

minwoox approved these changes Jan 22, 2024

View reviewed changes

codefromthecrypt reviewed Jan 24, 2024

View reviewed changes

dependencies.toml Outdated Show resolved Hide resolved

dependencies.toml Outdated Show resolved Hide resolved

codefromthecrypt mentioned this pull request Jan 24, 2024

deps: bumps to latest library versions, notably slf4j openzipkin/zipkin#3704

Merged

Address comments by @jrhee17

052b758

ikhoon and others added 5 commits January 24, 2024 21:37

Fix a compile error

f7eea31

lint

85da3fb

lint 2

c711974

Update dependencies.toml

0a7d172

Co-authored-by: Crypt Keeper <[email protected]>

Update dependencies.toml

b567dbf

Co-authored-by: Crypt Keeper <[email protected]>

jrhee17 approved these changes Jan 25, 2024

View reviewed changes

ikhoon merged commit 16455d9 into line:main Jan 25, 2024
15 of 16 checks passed

ikhoon deleted the up-spring-deps branch January 25, 2024 04:58

ikhoon mentioned this pull request Jan 31, 2024

Compatibility with armeria and boot 3.2 versions is broken. #5316

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump Spring and Jetty dependencies #5372

Bump Spring and Jetty dependencies #5372

ikhoon commented Jan 8, 2024 •

edited

Loading

github-actions bot commented Jan 8, 2024 •

edited

Loading

codecov bot commented Jan 15, 2024 •

edited

Loading

ikhoon Jan 15, 2024

ikhoon commented Jan 15, 2024

jrhee17 left a comment

jrhee17 Jan 22, 2024

ikhoon Jan 24, 2024

minwoox left a comment

codefromthecrypt left a comment

cj848 commented Jan 24, 2024

ikhoon commented Jan 24, 2024

jrhee17 left a comment •

edited

Loading

ikhoon commented Jan 25, 2024

codefromthecrypt commented Jan 25, 2024

Bump Spring and Jetty dependencies #5372

Bump Spring and Jetty dependencies #5372

Conversation

ikhoon commented Jan 8, 2024 • edited Loading

github-actions bot commented Jan 8, 2024 • edited Loading

🔍 Build Scan® (commit: b567dbf)

codecov bot commented Jan 15, 2024 • edited Loading

Codecov Report

ikhoon Jan 15, 2024

Choose a reason for hiding this comment

ikhoon commented Jan 15, 2024

jrhee17 left a comment

Choose a reason for hiding this comment

jrhee17 Jan 22, 2024

Choose a reason for hiding this comment

ikhoon Jan 24, 2024

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

codefromthecrypt left a comment

Choose a reason for hiding this comment

cj848 commented Jan 24, 2024

ikhoon commented Jan 24, 2024

jrhee17 left a comment • edited Loading

Choose a reason for hiding this comment

ikhoon commented Jan 25, 2024

codefromthecrypt commented Jan 25, 2024

ikhoon commented Jan 8, 2024 •

edited

Loading

github-actions bot commented Jan 8, 2024 •

edited

Loading

🔍 Build Scan® (commit: `b567dbf`)

codecov bot commented Jan 15, 2024 •

edited

Loading

jrhee17 left a comment •

edited

Loading