Add health indicator impact to HealthPeriodicLogger (#122390)

docs/changelog/122390.yaml

@@ -0,0 +1,5 @@
+pr: 122390
+summary: Add health indicator impact to `HealthPeriodicLogger`
+area: Health
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/health/HealthPeriodicLogger.java

@@ -52,10 +52,10 @@ import static org.elasticsearch.health.HealthStatus.GREEN;
 import static org.elasticsearch.health.HealthStatus.RED;
 
 /**
- * This class periodically logs the results of the Health API to the standard Elasticsearch server log file. It a lifecycle
- * aware component because it health depends on other lifecycle aware components. This means:
+ * This class periodically logs the results of the Health API to the standard Elasticsearch server log file. It is a lifecycle
+ * aware component because it depends on other lifecycle aware components. This means:
  * - We do not schedule any jobs until the lifecycle state is STARTED
- * - When the lifecycle state becomes STOPPED, do not schedule any more runs, but we do let the current one finish
+ * - When the lifecycle state becomes STOPPED, we do not schedule any more runs, but we do let the current one finish
  * - When the lifecycle state becomes CLOSED, we will interrupt the current run as well.
  */
 public class HealthPeriodicLogger extends AbstractLifecycleComponent implements ClusterStateListener, SchedulerEngine.Listener {
@@ -361,11 +361,24 @@ public class HealthPeriodicLogger extends AbstractLifecycleComponent implements
                 String.format(Locale.ROOT, "%s.%s.status", HEALTH_FIELD_PREFIX, indicatorResult.name()),
                 indicatorResult.status().xContentValue()
             );
-            if (GREEN.equals(indicatorResult.status()) == false && indicatorResult.details() != null) {
-                result.put(
-                    String.format(Locale.ROOT, "%s.%s.details", HEALTH_FIELD_PREFIX, indicatorResult.name()),
-                    Strings.toString(indicatorResult.details())
-                );
+            if (GREEN.equals(indicatorResult.status()) == false) {
+                // indicator details
+                if (indicatorResult.details() != null) {
+                    result.put(
+                        String.format(Locale.ROOT, "%s.%s.details", HEALTH_FIELD_PREFIX, indicatorResult.name()),
+                        Strings.toString(indicatorResult.details())
+                    );
+                }
+                // indicator impact
+                if (indicatorResult.impacts() != null) {
+                    indicatorResult.impacts()
+                        .forEach(
+                            impact -> result.put(
+                                String.format(Locale.ROOT, "%s.%s.%s.impacted", HEALTH_FIELD_PREFIX, indicatorResult.name(), impact.id()),
+                                true
+                            )
+                        );
+                }
             }
         });
 

server/src/main/java/org/elasticsearch/health/node/DiskHealthIndicatorService.java

@@ -66,7 +66,8 @@ public class DiskHealthIndicatorService implements HealthIndicatorService {
 
     private static final Logger logger = LogManager.getLogger(DiskHealthIndicatorService.class);
 
-    private static final String IMPACT_INGEST_UNAVAILABLE_ID = "ingest_capability_unavailable";
+    // VisibleForTesting
+    public static final String IMPACT_INGEST_UNAVAILABLE_ID = "ingest_capability_unavailable";
     private static final String IMPACT_INGEST_AT_RISK_ID = "ingest_capability_at_risk";
     private static final String IMPACT_CLUSTER_STABILITY_AT_RISK_ID = "cluster_stability_at_risk";
     private static final String IMPACT_CLUSTER_FUNCTIONALITY_UNAVAILABLE_ID = "cluster_functionality_unavailable";

server/src/test/java/org/elasticsearch/health/HealthPeriodicLoggerTests.java

@@ -19,6 +19,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodeRole;
 import org.elasticsearch.cluster.node.DiscoveryNodeUtils;
+import org.elasticsearch.cluster.routing.allocation.shards.ShardsAvailabilityHealthIndicatorService;
 import org.elasticsearch.cluster.routing.allocation.shards.ShardsAvailabilityHealthIndicatorServiceTests;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
@@ -28,6 +29,7 @@ import org.elasticsearch.common.scheduler.SchedulerEngine;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.health.node.DiskHealthIndicatorService;
 import org.elasticsearch.telemetry.TelemetryProvider;
 import org.elasticsearch.telemetry.metric.LongGaugeMetric;
 import org.elasticsearch.telemetry.metric.MeterRegistry;
@@ -51,9 +53,12 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 
+import static org.elasticsearch.cluster.routing.allocation.shards.ShardsAvailabilityHealthIndicatorService.PRIMARY_UNASSIGNED_IMPACT_ID;
+import static org.elasticsearch.cluster.routing.allocation.shards.ShardsAvailabilityHealthIndicatorService.REPLICA_UNASSIGNED_IMPACT_ID;
 import static org.elasticsearch.health.HealthStatus.GREEN;
 import static org.elasticsearch.health.HealthStatus.RED;
 import static org.elasticsearch.health.HealthStatus.YELLOW;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.IMPACT_INGEST_UNAVAILABLE_ID;
 import static org.elasticsearch.test.ClusterServiceUtils.createClusterService;
 import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.ArgumentMatchers.any;
@@ -125,9 +130,9 @@ public class HealthPeriodicLoggerTests extends ESTestCase {
 
         Map<String, Object> loggerResults = HealthPeriodicLogger.convertToLoggedFields(results);
 
-        // verify that the number of fields is the number of indicators + 4
-        // (for overall and for message, plus details for the two yellow indicators)
-        assertThat(loggerResults.size(), equalTo(results.size() + 4));
+        // verify that the number of fields is the number of indicators + 7
+        // (for overall and for message, plus details for the two yellow indicators, plus three impact)
+        assertThat(loggerResults.size(), equalTo(results.size() + 7));
 
         // test indicator status
         assertThat(loggerResults.get(makeHealthStatusString("master_is_stable")), equalTo("green"));
@@ -165,6 +170,17 @@ public class HealthPeriodicLoggerTests extends ESTestCase {
             equalTo(String.format(Locale.ROOT, "health=%s [disk,shards_availability]", overallStatus.xContentValue()))
         );
 
+        // test impact
+        assertThat(loggerResults.get(makeHealthImpactString(DiskHealthIndicatorService.NAME, IMPACT_INGEST_UNAVAILABLE_ID)), equalTo(true));
+        assertThat(
+            loggerResults.get(makeHealthImpactString(ShardsAvailabilityHealthIndicatorService.NAME, PRIMARY_UNASSIGNED_IMPACT_ID)),
+            equalTo(true)
+        );
+        assertThat(
+            loggerResults.get(makeHealthImpactString(ShardsAvailabilityHealthIndicatorService.NAME, REPLICA_UNASSIGNED_IMPACT_ID)),
+            equalTo(true)
+        );
+
         // test empty results
         {
             List<HealthIndicatorResult> empty = new ArrayList<>();
@@ -793,7 +809,15 @@ public class HealthPeriodicLoggerTests extends ESTestCase {
                     1
                 )
             ),
-            null,
+            List.of(
+                new HealthIndicatorImpact(
+                    DiskHealthIndicatorService.NAME,
+                    IMPACT_INGEST_UNAVAILABLE_ID,
+                    2,
+                    "description",
+                    List.of(ImpactArea.INGEST)
+                )
+            ),
             null
         );
         var shardsAvailable = new HealthIndicatorResult(
@@ -801,7 +825,22 @@ public class HealthPeriodicLoggerTests extends ESTestCase {
             YELLOW,
             null,
             new SimpleHealthIndicatorDetails(ShardsAvailabilityHealthIndicatorServiceTests.addDefaults(Map.of())),
-            null,
+            List.of(
+                new HealthIndicatorImpact(
+                    ShardsAvailabilityHealthIndicatorService.NAME,
+                    PRIMARY_UNASSIGNED_IMPACT_ID,
+                    2,
+                    "description",
+                    List.of(ImpactArea.SEARCH)
+                ),
+                new HealthIndicatorImpact(
+                    ShardsAvailabilityHealthIndicatorService.NAME,
+                    REPLICA_UNASSIGNED_IMPACT_ID,
+                    2,
+                    "description",
+                    List.of(ImpactArea.SEARCH)
+                )
+            ),
             null
         );
 
@@ -846,6 +885,10 @@ public class HealthPeriodicLoggerTests extends ESTestCase {
         return String.format(Locale.ROOT, "%s.%s.details", HealthPeriodicLogger.HEALTH_FIELD_PREFIX, key);
     }
 
+    private String makeHealthImpactString(String indicatorName, String impact) {
+        return String.format(Locale.ROOT, "%s.%s.%s.impacted", HealthPeriodicLogger.HEALTH_FIELD_PREFIX, indicatorName, impact);
+    }
+
     private HealthPeriodicLogger createAndInitHealthPeriodicLogger(
         ClusterService clusterService,
         HealthService testHealthService,