Enhance `boot3-actuator-autoconfigure` #4880

be-hase · 2023-05-17T15:17:18Z

Motivation:

#4874

Modifications:

It should behave the same as spring boot's WebEndpointAutoConfiguration.

Support management.endpoints.web.exposure.*
Support management.endpoints.web.path-mapping.*
Support management.endpoints.web.discovery.*
Support management.endpoint.health.status.http-mapping.*
Support application/vnd.spring-boot.actuator.v3+json, application/vnd.spring-boot.actuator.v2+json and application/json

Result:

Closes It seems that the management.endpoints.web.exposure setting in spring boot is not supported by armeria-spring-boot3-actuator-starter. #4874

be-hase · 2023-05-17T15:20:03Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

    @Bean
    @ConditionalOnMissingBean
-    EndpointMediaTypes endpointMediaTypes() {
-        return new EndpointMediaTypes(MEDIA_TYPES, MEDIA_TYPES);
+    public IncludeExcludeEndpointFilter<ExposableWebEndpoint> webExposeExcludePropertyEndpointFilter(


For Support management.endpoints.web.exposure.*

be-hase · 2023-05-17T15:20:34Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

+    // In case WebEndpointAutoConfiguration is excluded
+    @Bean
+    @ConditionalOnMissingBean
+    public PathMapper webEndpointPathMapper(WebEndpointProperties properties) {


For management.endpoints.web.path-mapping.*

It would be also useful to comment about management.endpoints.web.path-mapping.* on the method or Javadoc.

be-hase · 2023-05-17T15:21:06Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

@@ -263,7 +279,7 @@ private static void configureExposableWebEndpoint(ServerBuilder sb, @Nullable In
                                         predicate, path, ImmutableMap.of(), cors);
                 });

-        if (StringUtils.hasText(endpointMapping.getPath())) {
+        if (StringUtils.hasText(endpointMapping.getPath()) && properties.getDiscovery().isEnabled()) {


For management.endpoints.web.discovery.*

This is not your fault but we also need to check if the endpointMapping.getPath() is /.

When the management context path is set to /, the discovery page is disabled to prevent the possibility of a clash with other mappings.

Ah. The case seems to have already been taken care of.
It is normalized to "/" -> "" in WebEndpointProperties.
https://github.com/spring-projects/spring-boot/blob/e3aac5913ed3caf53b34eb7750138a4ed6839549/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/WebEndpointProperties.java#L69-L74

I didn't notice that. 😓 Thanks!

be-hase · 2023-05-17T15:22:02Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

@@ -273,7 +289,8 @@ private static void configureExposableWebEndpoint(ServerBuilder sb, @Nullable In
            final HttpService linksService = (ctx, req) -> {
                final Map<String, Link> links =
                        new EndpointLinksResolver(endpoints).resolveLinks(req.path());
-                return HttpResponse.ofJson(ACTUATOR_MEDIA_TYPE, ImmutableMap.of("_links", links));
+                final MediaType contentType = firstNonNull(ctx.negotiatedResponseMediaType(), MediaType.JSON);


For supporting application/vnd.spring-boot.actuator.v3+json, application/vnd.spring-boot.actuator.v2+json and application/json

be-hase · 2023-05-17T15:23:42Z

...oconfigure/src/main/java/com/linecorp/armeria/spring/actuate/SimpleHttpCodeStatusMapper.java

-import com.google.common.collect.ImmutableMap;
-
-/**
- * Copied from spring-boot-actuator 2.3.0 to avoid breaking compatibility.


I think spring boot 2.3.x is EOL, so we won't have to take care of breaking compatibility anymore.
(No more users in trouble between 2.2.x and 2.3.x)

Thanks for cleaning up. 🙇

be-hase · 2023-05-17T15:29:47Z

...va/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfigurationSecureTest.java

@@ -45,7 +46,7 @@
 @DirtiesContext
 @AutoConfigureMetrics
 @EnableAutoConfiguration
-@ImportAutoConfiguration(ArmeriaSpringActuatorAutoConfiguration.class)
+@ImportAutoConfiguration({ ArmeriaSpringActuatorAutoConfiguration.class, JmxEndpointAutoConfiguration.class })


Applying JmxEndpointAutoConfiguration, we can reproduce #4874 .
(Occurs when the Endpoint is registered in a bean.)

code hints:

https://github.com/be-hase/spring-boot/blob/c4de86c244acdcff69ed0aecacd254399be79ce2/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java#L130-L139

https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/OnAvailableEndpointCondition.java

minwoox

Left minor comments. Thanks a lot for fixing this! 👍

minwoox · 2023-05-18T03:25:07Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

@@ -263,7 +279,7 @@ private static void configureExposableWebEndpoint(ServerBuilder sb, @Nullable In
                                         predicate, path, ImmutableMap.of(), cors);
                 });

-        if (StringUtils.hasText(endpointMapping.getPath())) {
+        if (StringUtils.hasText(endpointMapping.getPath()) && properties.getDiscovery().isEnabled()) {


This is not your fault but we also need to check if the endpointMapping.getPath() is /.

When the management context path is set to /, the discovery page is disabled to prevent the possibility of a clash with other mappings.

...onfigure/src/main/java/com/linecorp/armeria/spring/actuate/MappingWebEndpointPathMapper.java

minwoox · 2023-05-18T03:29:31Z

...oconfigure/src/main/java/com/linecorp/armeria/spring/actuate/SimpleHttpCodeStatusMapper.java

-import com.google.common.collect.ImmutableMap;
-
-/**
- * Copied from spring-boot-actuator 2.3.0 to avoid breaking compatibility.


Thanks for cleaning up. 🙇

spring/boot3-actuator-autoconfigure/src/test/resources/application-autoConfTest.yml

minwoox

Looks great! Thanks a lot, @be-hase!

minwoox · 2023-05-18T06:42:01Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

@@ -263,7 +279,7 @@ private static void configureExposableWebEndpoint(ServerBuilder sb, @Nullable In
                                         predicate, path, ImmutableMap.of(), cors);
                 });

-        if (StringUtils.hasText(endpointMapping.getPath())) {
+        if (StringUtils.hasText(endpointMapping.getPath()) && properties.getDiscovery().isEnabled()) {


I didn't notice that. 😓 Thanks!

ikhoon

Overall, looks nice. Left only minor comments.

...onfigure/src/main/java/com/linecorp/armeria/spring/actuate/MappingWebEndpointPathMapper.java

ikhoon · 2023-05-22T08:31:51Z

...rc/main/java/com/linecorp/armeria/spring/actuate/ArmeriaSpringActuatorAutoConfiguration.java

+    // In case WebEndpointAutoConfiguration is excluded
+    @Bean
+    @ConditionalOnMissingBean
+    public PathMapper webEndpointPathMapper(WebEndpointProperties properties) {


It would be also useful to comment about management.endpoints.web.path-mapping.* on the method or Javadoc.

trustin

Nice work, @be-hase! Thanks for this PR that requires good understanding of Spring. 🙇

be-hase · 2023-05-22T16:13:14Z

It would be also useful to comment about management.endpoints.web.path-mapping.* on the method or Javadoc.

fixed: f477edd

ikhoon

Thanks, @be-hase! 💯🙏

jrhee17

Looks really nice 👍 Thanks @be-hase ! 🙇 👍 🚀

be-hase · 2023-05-24T04:21:22Z

Thanks for the review :) > armeria team

be-hase requested review from ikhoon, jrhee17, minwoox and trustin as code owners May 17, 2023 15:17

be-hase commented May 17, 2023

View reviewed changes

be-hase force-pushed the issue-4874 branch from 6a214d5 to b7dd1f1 Compare May 17, 2023 15:37

Enhance spring boot actuator support

10332b8

be-hase force-pushed the issue-4874 branch from b7dd1f1 to 10332b8 Compare May 17, 2023 15:54

be-hase changed the title ~~Enhance spring boot actuator support~~ Enhance boot3-actuator-autoconfigure May 17, 2023

minwoox reviewed May 18, 2023

View reviewed changes

be-hase added 2 commits May 18, 2023 15:29

add license

d4ed172

check exclude

1a50b17

minwoox approved these changes May 18, 2023

View reviewed changes

ikhoon added the improvement label May 22, 2023

ikhoon added this to the 1.24.0 milestone May 22, 2023

ikhoon reviewed May 22, 2023

View reviewed changes

trustin approved these changes May 22, 2023

View reviewed changes

be-hase added 2 commits May 23, 2023 01:07

add javadoc

f477edd

add comment on MappingWebEndpointPathMapper

322750a

fix checkstyle

e2a5725

ikhoon approved these changes May 23, 2023

View reviewed changes

jrhee17 approved these changes May 23, 2023

View reviewed changes

ikhoon merged commit d482f0c into line:main May 24, 2023

be-hase deleted the issue-4874 branch May 24, 2023 04:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance `boot3-actuator-autoconfigure` #4880

Enhance `boot3-actuator-autoconfigure` #4880

be-hase commented May 17, 2023 •

edited

Loading

be-hase May 17, 2023

be-hase May 17, 2023

ikhoon May 22, 2023

be-hase May 17, 2023

minwoox May 18, 2023

be-hase May 18, 2023

minwoox May 18, 2023

be-hase May 17, 2023

be-hase May 17, 2023 •

edited

Loading

minwoox May 18, 2023

be-hase May 17, 2023

minwoox left a comment

minwoox May 18, 2023

minwoox May 18, 2023

minwoox left a comment

minwoox May 18, 2023

ikhoon left a comment

ikhoon May 22, 2023

trustin left a comment

be-hase commented May 22, 2023

ikhoon left a comment

jrhee17 left a comment

be-hase commented May 24, 2023

Enhance boot3-actuator-autoconfigure #4880

Enhance boot3-actuator-autoconfigure #4880

Conversation

be-hase commented May 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

be-hase May 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikhoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trustin left a comment

Choose a reason for hiding this comment

be-hase commented May 22, 2023

ikhoon left a comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

be-hase commented May 24, 2023

Enhance `boot3-actuator-autoconfigure` #4880

Enhance `boot3-actuator-autoconfigure` #4880

be-hase commented May 17, 2023 •

edited

Loading

be-hase May 17, 2023 •

edited

Loading