Actuator healthcheck #1103

charlesmst · 2024-05-02T14:54:49Z

This is the initial work for #1091.

It is only implementing the check now, but watch will be added later by pushing metrics every 30s as @ST-DDT suggested.

I was wondering if this should be the default behavior when the actuator is present, so I added a property to enable this instead of the GRPC standard health endpoint.

ST-DDT

I'm currently buzy with work, so reviews will take a while.

grpc-server-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/config/HealthType.java

ST-DDT · 2024-05-08T20:28:57Z

...r-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/health/ActuatorGrpcHealth.java

+    public void check(HealthCheckRequest request, StreamObserver<HealthCheckResponse> responseObserver) {
+
+        if (!request.getService().isEmpty()) {
+            var health = healthEndpoint.healthForPath(request.getService());


The rest of our code doesn't use var.

ST-DDT · 2024-05-08T20:31:17Z

...r-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/health/ActuatorGrpcHealth.java

+        if (Objects.equals(org.springframework.boot.actuate.health.Status.UP.getCode(), status.getCode())) {
+            return HealthCheckResponse.ServingStatus.SERVING;
+        }
+        if (Objects.equals(org.springframework.boot.actuate.health.Status.DOWN.getCode(), status.getCode()) || Objects
+                .equals(org.springframework.boot.actuate.health.Status.OUT_OF_SERVICE.getCode(), status.getCode())) {
+            return HealthCheckResponse.ServingStatus.NOT_SERVING;
+        }
+        return HealthCheckResponse.ServingStatus.UNKNOWN;


IMO these checks can be simplified to:

org.springframework.boot.actuate.health.Status.UP.equals(status)

I changed to Status.UP.equals(status) in the UP condition, but I am not sure if you meant just replacing the object equals comparing code to use the class equals, or not having the DOWN and OUT_OF_SERVICE cases and just returning NOT_SERVING for everything else, can you clarify this part?

ST-DDT · 2024-05-08T20:32:13Z

grpc-server-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/config/HealthType.java

+    /**
+     * Use the standard GRPC health service from io.grpc.
+     */
+    GRPC,
+    /**
+     * Uses a bridge to the Spring Boot Actuator health service.
+     */
+    ACTUATOR,


Maybe add @see / {@link} links to the implementation classes.

ST-DDT · 2024-05-08T20:33:47Z

...spring-boot-starter/src/main/java/net/devh/boot/grpc/server/config/GrpcServerProperties.java

@@ -228,6 +228,14 @@ public class GrpcServerProperties {
     */
    private boolean healthServiceEnabled = true;


maybe deprecate this in favor of HealthType.NONE

ST-DDT · 2024-05-08T20:35:00Z

...spring-boot-starter/src/main/java/net/devh/boot/grpc/server/config/GrpcServerProperties.java

+     * @param healthServiceType The implementation of gRPC health service.
+     * @return GRPC or Actuator.
+     */
+    private HealthType healthServiceType = HealthType.GRPC;


Question: Should we create a nested object for this?
That would give the potential later update frequency a place for its property.
If we don't plan to add it for now, we can leave it as is for now.

That makes sense, maybe with Watch a new property will be necessary for the interval between checks. I will change to a nested object

docs/en/actuator.md

.../net/devh/boot/grpc/server/autoconfigure/GrpcHealthServiceTrueActuatorConfigurationTest.java

...ring-boot-starter/src/test/java/net/devh/boot/grpc/server/health/ActuatorGrpcHealthTest.java

ST-DDT · 2024-05-08T20:46:17Z

...r-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/health/ActuatorGrpcHealth.java

+            var health = healthEndpoint.healthForPath(request.getService());
+            if (health == null) {
+                responseObserver.onError(new StatusException(
+                        Status.NOT_FOUND.withDescription("unknown service " + request.getService())));


Status is only used once here and maybe that can be avoided with statically importing NOT_FOUND, then you could import org.springframework.boot.actuate.health.Status instead and make it easier to read.

(I wish Java would support import aliasing)

👍 Code changed accordingly

Co-authored-by: ST-DDT <ST-DDT@gmx.de>

…pc/server/autoconfigure/GrpcHealthServiceTrueActuatorConfigurationTest.java Co-authored-by: ST-DDT <ST-DDT@gmx.de>

…pc/server/health/ActuatorGrpcHealthTest.java Co-authored-by: ST-DDT <ST-DDT@gmx.de>

charlesmst added 7 commits April 30, 2024 11:00

health service for actuator

54b83e0

autoconfiguration for actuator

d253312

auto configuration

c7f8845

use enum for health type

e3ff1be

fix formatting

80bb4fc

docs

211bd43

fix formatting

b5975ea

charlesmst force-pushed the actuator-healthcheck branch from 337738f to b5975ea Compare May 2, 2024 14:55

charlesmst changed the title ~~Actuator healthcheck #1093~~ Actuator healthcheck #1091 May 2, 2024

charlesmst changed the title ~~Actuator healthcheck #1091~~ Actuator healthcheck May 2, 2024

ST-DDT reviewed May 2, 2024

View reviewed changes

grpc-server-spring-boot-starter/src/main/java/net/devh/boot/grpc/server/config/HealthType.java Outdated Show resolved Hide resolved

add none type in health type

18f3bff

ST-DDT reviewed May 8, 2024

View reviewed changes

ST-DDT added the enhancement A feature request or improvement label May 8, 2024

ST-DDT assigned charlesmst May 8, 2024

charlesmst and others added 7 commits May 10, 2024 14:30

Merge branch 'master' into actuator-healthcheck

6fb8115

Update docs/en/actuator.md

fd7b735

Co-authored-by: ST-DDT <ST-DDT@gmx.de>

Update docs/en/actuator.md

954a893

Co-authored-by: ST-DDT <ST-DDT@gmx.de>

add links to spring health indicator

4dbb3d5

Update docs/en/actuator.md

9fbbb8b

Co-authored-by: ST-DDT <ST-DDT@gmx.de>

Update grpc-server-spring-boot-starter/src/test/java/net/devh/boot/gr…

091159e

…pc/server/autoconfigure/GrpcHealthServiceTrueActuatorConfigurationTest.java Co-authored-by: ST-DDT <ST-DDT@gmx.de>

Update grpc-server-spring-boot-starter/src/test/java/net/devh/boot/gr…

134e004

…pc/server/health/ActuatorGrpcHealthTest.java Co-authored-by: ST-DDT <ST-DDT@gmx.de>

charlesmst force-pushed the actuator-healthcheck branch 2 times, most recently from 459c018 to dc3e521 Compare May 10, 2024 18:04

replace usage of var and status imports

183c22b

charlesmst force-pushed the actuator-healthcheck branch from dc3e521 to 183c22b Compare May 10, 2024 18:11

charlesmst added 2 commits May 13, 2024 17:10

only load health status manager if GRPC enabled

b91340c

avoid silently failing to create actuator healthcheck

cced054

charlesmst force-pushed the actuator-healthcheck branch from fd3070e to 259cec4 Compare May 13, 2024 20:57

add nested config object for health service

65e8a1b

charlesmst force-pushed the actuator-healthcheck branch from 259cec4 to 65e8a1b Compare May 13, 2024 20:59

add docs for health type none

c408461

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Actuator healthcheck #1103

Actuator healthcheck #1103

charlesmst commented May 2, 2024

ST-DDT left a comment

ST-DDT May 8, 2024

ST-DDT May 8, 2024

charlesmst May 10, 2024

ST-DDT May 8, 2024

ST-DDT May 8, 2024

ST-DDT May 8, 2024 •

edited

charlesmst May 10, 2024

ST-DDT May 8, 2024

charlesmst May 10, 2024

		@@ -228,6 +228,14 @@ public class GrpcServerProperties {
		*/
		private boolean healthServiceEnabled = true;

Actuator healthcheck #1103

Are you sure you want to change the base?

Actuator healthcheck #1103

Conversation

charlesmst commented May 2, 2024

ST-DDT left a 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

ST-DDT May 8, 2024 • edited

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

ST-DDT May 8, 2024 •

edited