Update health check strategy

Separte DataFeedLoop health check from the other components, set the global status to out of service only if DataFeedLoop is down. In this case, when calling healtch check endpoint, the resposne http code is 500, otherwise if any other components is down, set the global status to "WARNING", the http code of the response stays as 200. This will avoid unncessary BDK Bot restarting when other components are not healthy.
yinan-symphony · Feb 13, 2024 · e6394b6 · e6394b6
1 parent 59b78cb
commit e6394b6
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 78 deletions.
diff --git a/...ot-starter/src/main/java/com/symphony/bdk/app/spring/config/BdkHealthIndicatorConfig.java b/...ot-starter/src/main/java/com/symphony/bdk/app/spring/config/BdkHealthIndicatorConfig.java
@@ -1,11 +1,8 @@
 package com.symphony.bdk.app.spring.config;
 
 import com.symphony.bdk.app.spring.service.SymphonyBdkHealthIndicator;
-import com.symphony.bdk.app.spring.service.SymphonyBdkHealthIndicator.CustomStatusCodeMapper;
 import com.symphony.bdk.core.service.health.HealthService;
 
-import org.springframework.boot.actuate.health.HttpCodeStatusMapper;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
@@ -20,9 +17,4 @@ public SymphonyBdkHealthIndicator symphonyBdkHealthIndicator(HealthService healt
     return new SymphonyBdkHealthIndicator(healthService);
   }
 
-  @Bean
-  @ConditionalOnBean(name = "bot")
-  public HttpCodeStatusMapper customStatusCodeMapper() {
-    return new CustomStatusCodeMapper();
-  }
 }
diff --git a/...starter/src/main/java/com/symphony/bdk/app/spring/service/SymphonyBdkHealthIndicator.java b/...starter/src/main/java/com/symphony/bdk/app/spring/service/SymphonyBdkHealthIndicator.java
@@ -12,7 +12,6 @@
 import org.apiguardian.api.API;
 import org.springframework.boot.actuate.health.AbstractHealthIndicator;
 import org.springframework.boot.actuate.health.Health;
-import org.springframework.boot.actuate.health.HttpCodeStatusMapper;
 import org.springframework.boot.actuate.health.Status;
 
 import java.util.Map;
@@ -70,10 +69,14 @@ private void buildHealthDetail(Health.Builder builder, V3Health health) {
     V3HealthStatus agtStatus = users.get(AGT).getStatus();
     V3HealthStatus datafeedLoop = healthService.datafeedHealthCheck();
 
-    boolean global = podStatus == V3HealthStatus.UP && dfStatus == V3HealthStatus.UP && kmStatus == V3HealthStatus.UP
-        && agtStatus == V3HealthStatus.UP && datafeedLoop == V3HealthStatus.UP;
-
-    builder.status(global ? Status.UP.getCode() : WARNING)
+    if (datafeedLoop != V3HealthStatus.UP) {
+      builder.status(Status.OUT_OF_SERVICE);
+    } else {
+      boolean global = podStatus == V3HealthStatus.UP && dfStatus == V3HealthStatus.UP && kmStatus == V3HealthStatus.UP
+          && agtStatus == V3HealthStatus.UP;
+      builder.status(global ? Status.UP.getCode() : WARNING);
+    }
+    builder
         .withDetail(POD, services.get(POD))
         .withDetail(DF, services.get(DF))
         .withDetail(KM, services.get(KM))
@@ -91,35 +94,4 @@ private void buildHealthDownDetail(Health.Builder builder) {
         .withDetail(CE, DOWN)
         .withDetail(DFL, DOWN);
   }
-
-
-  /**
-   * A custom spring actuator health endpoint status code mapper.
-   *
-   * It will return 500 for all status except Status.UP
-   */
-  @API(status = API.Status.INTERNAL)
-  public static class CustomStatusCodeMapper implements HttpCodeStatusMapper {
-
-    @Override
-    public int getStatusCode(Status status) {
-      if (status == Status.DOWN) {
-        return 500;
-      }
-
-      if (status == Status.OUT_OF_SERVICE) {
-        return 503;
-      }
-
-      if (status == Status.UNKNOWN) {
-        return 500;
-      }
-
-      if (status.getCode().equals(WARNING)) {
-        return 500;
-      }
-
-      return 200;
-    }
-  }
 }
diff --git a/...tarter/src/test/java/com/symphony/bdk/app/spring/config/BdkHealthIndicatorConfigTest.java b/...tarter/src/test/java/com/symphony/bdk/app/spring/config/BdkHealthIndicatorConfigTest.java
@@ -4,7 +4,7 @@
 
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.Mockito.mock;
 
 class BdkHealthIndicatorConfigTest {
@@ -15,7 +15,5 @@ void createSymphonyBdkHealthIndicatorTest() {
     final HealthService healthService = mock(HealthService.class);
 
     assertNotNull(config.symphonyBdkHealthIndicator(healthService));
-
-    assertNotNull(config.customStatusCodeMapper());
   }
 }
diff --git a/...ter/src/test/java/com/symphony/bdk/app/spring/service/SymphonyBdkHealthIndicatorTest.java b/...ter/src/test/java/com/symphony/bdk/app/spring/service/SymphonyBdkHealthIndicatorTest.java
@@ -1,28 +1,25 @@
 package com.symphony.bdk.app.spring.service;
 
-import com.symphony.bdk.app.spring.service.SymphonyBdkHealthIndicator.CustomStatusCodeMapper;
 import com.symphony.bdk.core.service.health.HealthService;
 import com.symphony.bdk.gen.api.model.V3Health;
 import com.symphony.bdk.gen.api.model.V3HealthComponent;
 import com.symphony.bdk.gen.api.model.V3HealthStatus;
 import com.symphony.bdk.http.api.ApiException;
 import com.symphony.bdk.http.api.ApiRuntimeException;
 
-import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.boot.actuate.health.Health;
-import org.springframework.boot.actuate.health.HttpCodeStatusMapper;
 import org.springframework.boot.actuate.health.Status;
 
 import java.util.Collections;
+import java.util.Map;
 import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -34,20 +31,29 @@ class SymphonyBdkHealthIndicatorTest {
   @Mock HealthService healthService;
   @InjectMocks SymphonyBdkHealthIndicator healthIndicator;
 
-  @Test
-  void doHealthCheck_successful() throws Exception {
+  @ParameterizedTest
+  @MethodSource("datafeedHealthTestArguments")
+  void doHealthCheck_datafeedStatus_expectedStatusCode(V3HealthStatus dfl, V3HealthStatus df, String globalCode) throws Exception {
     V3Health health = new V3Health();
     health.putServicesItem("pod", new V3HealthComponent().status(V3HealthStatus.UP));
-    health.putServicesItem("datafeed", new V3HealthComponent().status(V3HealthStatus.UP));
+    health.putServicesItem("datafeed", new V3HealthComponent().status(df));
     health.putServicesItem("key_manager", new V3HealthComponent().status(V3HealthStatus.UP));
     health.putUsersItem("agentservice", new V3HealthComponent().status(V3HealthStatus.UP));
     health.putUsersItem("ceservice", new V3HealthComponent().status(V3HealthStatus.UP));
     when(healthService.healthCheckExtended()).thenReturn(health);
-    when(healthService.datafeedHealthCheck()).thenReturn(V3HealthStatus.UP);
+    when(healthService.datafeedHealthCheck()).thenReturn(dfl);
     Health.Builder builder = new Health.Builder();
     healthIndicator.doHealthCheck(builder);
     Health build = builder.build();
-    assertThat(build.getStatus().getCode()).isEqualTo("UP");
+    assertThat(build.getStatus().getCode()).isEqualTo(globalCode);
+  }
+
+  private static Stream<Arguments> datafeedHealthTestArguments() {
+    return Stream.of(
+        Arguments.of(V3HealthStatus.UP, V3HealthStatus.UP, Status.UP.getCode()),
+        Arguments.of(V3HealthStatus.DOWN, V3HealthStatus.UP, Status.OUT_OF_SERVICE.getCode()),
+        Arguments.of(V3HealthStatus.UP, V3HealthStatus.DOWN, "WARNING")
+    );
   }
 
   @Test
@@ -78,7 +84,7 @@ void doHealthCheck_exception() throws Exception {
             + "      \"message\": \"Ceservice authentication credentials missing or misconfigured\"\n" + "    }\n"
             + "  }\n" + "}";
 
-    doThrow(new ApiRuntimeException(new ApiException(503, "message", Collections.EMPTY_MAP, body))).when(healthService)
+    doThrow(new ApiRuntimeException(new ApiException(503, "message", Map.of(), body))).when(healthService)
         .healthCheckExtended();
     when(healthService.datafeedHealthCheck()).thenReturn(V3HealthStatus.UP);
     Health.Builder builder = new Health.Builder();
@@ -92,32 +98,12 @@ void doHealthCheck_badGw_exception() throws Exception {
     final String body = "<html>\n" + "<head><title>502 Bad Gateway</title></head>\n" + "<body>\n"
         + "<center><h1>502 Bad Gateway</h1></center>\n" + "</body>\n" + "</html>";
 
-    doThrow(new ApiRuntimeException(new ApiException(502, "message", Collections.EMPTY_MAP, body))).when(healthService)
+    doThrow(new ApiRuntimeException(new ApiException(502, "message", Map.of(), body))).when(healthService)
         .healthCheckExtended();
     Health.Builder builder = new Health.Builder();
     healthIndicator.doHealthCheck(builder);
     Health build = builder.build();
     assertThat(build.getStatus().getCode()).isEqualTo("DOWN");
   }
 
-  @Nested
-  class CustomStatusCodeMapperTest {
-
-    HttpCodeStatusMapper mapper = new CustomStatusCodeMapper();
-
-    @ParameterizedTest
-    @MethodSource("getStatusArguments")
-    void getStatusCode_status_code(Status status, int code) {
-      assertThat(mapper.getStatusCode(status)).isEqualTo(code);
-    }
-
-    private static Stream<Arguments> getStatusArguments() {
-      return Stream.of(
-          Arguments.of(Status.UP, 200),
-          Arguments.of(Status.DOWN, 500),
-          Arguments.of(new Status("WARNING"), 500),
-          Arguments.of(Status.OUT_OF_SERVICE, 503),
-          Arguments.of(Status.UNKNOWN, 500));
-    }
-  }
 }