Add length limit FileSettingsHealthIndicatorService.description (#121334)

* Add length limit FileSettingsHealthIndicatorService.description

* [CI] Auto commit changes from spotless

* Add javadocs explaining `fileSettings.descriptionLengthLimit` setting

---------

Co-authored-by: elasticsearchmachine <[email protected]>

Author: prdoyle

server/src/main/java/org/elasticsearch/node/NodeConstruction.java

@@ -1067,7 +1067,7 @@ public Map<String, String> searchFields() {
         actionModule.getReservedClusterStateService().installStateHandler(new ReservedRepositoryAction(repositoriesService));
         actionModule.getReservedClusterStateService().installStateHandler(new ReservedPipelineAction());
 
-        FileSettingsHealthIndicatorService fileSettingsHealthIndicatorService = new FileSettingsHealthIndicatorService();
+        FileSettingsHealthIndicatorService fileSettingsHealthIndicatorService = new FileSettingsHealthIndicatorService(settings);
         FileSettingsService fileSettingsService = new FileSettingsService(
             clusterService,
             actionModule.getReservedClusterStateService(),

server/src/main/java/org/elasticsearch/reservedstate/service/FileSettingsService.java

@@ -21,6 +21,8 @@
 import org.elasticsearch.cluster.metadata.ReservedStateMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.file.MasterNodeFileWatchingService;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.health.HealthIndicatorDetails;
 import org.elasticsearch.health.HealthIndicatorImpact;
@@ -212,7 +214,7 @@ protected void onProcessFileChangesException(Exception e) {
     }
 
     @Override
-    protected void processInitialFileMissing() throws ExecutionException, InterruptedException, IOException {
+    protected void processInitialFileMissing() throws ExecutionException, InterruptedException {
         PlainActionFuture<ActionResponse.Empty> completion = new PlainActionFuture<>();
         logger.info("setting file [{}] not found, initializing [{}] as empty", watchedFile(), NAMESPACE);
         stateService.initEmpty(NAMESPACE, completion);
@@ -236,11 +238,29 @@ public static class FileSettingsHealthIndicatorService implements HealthIndicato
             )
         );
 
+        /**
+         * We want a length limit so we don't blow past the indexing limit in the case of a long description string.
+         * This is an {@code OperatorDynamic} setting so that if the truncation hampers troubleshooting efforts,
+         * the operator could override it and retry the operation without necessarily restarting the cluster.
+         */
+        public static final String DESCRIPTION_LENGTH_LIMIT_KEY = "fileSettings.descriptionLengthLimit";
+        static final Setting<Integer> DESCRIPTION_LENGTH_LIMIT = Setting.intSetting(
+            DESCRIPTION_LENGTH_LIMIT_KEY,
+            100,
+            1, // Need room for the ellipsis
+            Setting.Property.OperatorDynamic
+        );
+
+        private final Settings settings;
         private boolean isActive = false;
         private long changeCount = 0;
         private long failureStreak = 0;
         private String mostRecentFailure = null;
 
+        public FileSettingsHealthIndicatorService(Settings settings) {
+            this.settings = settings;
+        }
+
         public synchronized void startOccurred() {
             isActive = true;
             failureStreak = 0;
@@ -262,7 +282,16 @@ public synchronized void successOccurred() {
 
         public synchronized void failureOccurred(String description) {
             ++failureStreak;
-            mostRecentFailure = description;
+            mostRecentFailure = limitLength(description);
+        }
+
+        private String limitLength(String description) {
+            int descriptionLengthLimit = DESCRIPTION_LENGTH_LIMIT.get(settings);
+            if (description.length() > descriptionLengthLimit) {
+                return description.substring(0, descriptionLengthLimit - 1) + "…";
+            } else {
+                return description;
+            }
         }
 
         @Override

server/src/test/java/org/elasticsearch/action/ingest/ReservedPipelineActionTests.java

@@ -138,7 +138,7 @@ public void setup() {
                 clusterService,
                 mock(ReservedClusterStateService.class),
                 newEnvironment(Settings.EMPTY),
-                new FileSettingsService.FileSettingsHealthIndicatorService()
+                new FileSettingsService.FileSettingsHealthIndicatorService(Settings.EMPTY)
             )
         );
     }

server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsHealthIndicatorServiceTests.java

@@ -9,6 +9,7 @@
 
 package org.elasticsearch.reservedstate.service;
 
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.health.HealthIndicatorDetails;
 import org.elasticsearch.health.HealthIndicatorResult;
 import org.elasticsearch.health.SimpleHealthIndicatorDetails;
@@ -21,6 +22,7 @@
 
 import static org.elasticsearch.health.HealthStatus.GREEN;
 import static org.elasticsearch.health.HealthStatus.YELLOW;
+import static org.elasticsearch.reservedstate.service.FileSettingsService.FileSettingsHealthIndicatorService.DESCRIPTION_LENGTH_LIMIT_KEY;
 import static org.elasticsearch.reservedstate.service.FileSettingsService.FileSettingsHealthIndicatorService.FAILURE_SYMPTOM;
 import static org.elasticsearch.reservedstate.service.FileSettingsService.FileSettingsHealthIndicatorService.INACTIVE_SYMPTOM;
 import static org.elasticsearch.reservedstate.service.FileSettingsService.FileSettingsHealthIndicatorService.NO_CHANGES_SYMPTOM;
@@ -37,7 +39,7 @@ public class FileSettingsHealthIndicatorServiceTests extends ESTestCase {
 
     @Before
     public void initialize() {
-        healthIndicatorService = new FileSettingsHealthIndicatorService();
+        healthIndicatorService = new FileSettingsHealthIndicatorService(Settings.EMPTY);
     }
 
     public void testInitiallyGreen() {}
@@ -101,4 +103,28 @@ public void testGreenYellowYellowGreen() {
             healthIndicatorService.calculate(false, null)
         );
     }
+
+    public void testDescriptionIsTruncated() {
+        checkTruncatedDescription(9, "123456789", "123456789");
+        checkTruncatedDescription(8, "123456789", "1234567…");
+        checkTruncatedDescription(1, "12", "…");
+    }
+
+    private void checkTruncatedDescription(int lengthLimit, String description, String expectedTruncatedDescription) {
+        var service = new FileSettingsHealthIndicatorService(Settings.builder().put(DESCRIPTION_LENGTH_LIMIT_KEY, lengthLimit).build());
+        service.startOccurred();
+        service.changeOccurred();
+        service.failureOccurred(description);
+        assertEquals(
+            new HealthIndicatorResult(
+                "file_settings",
+                YELLOW,
+                FAILURE_SYMPTOM,
+                new SimpleHealthIndicatorDetails(Map.of("failure_streak", 1L, "most_recent_failure", expectedTruncatedDescription)),
+                STALE_SETTINGS_IMPACT,
+                List.of()
+            ),
+            service.calculate(false, null)
+        );
+    }
 }

server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java

@@ -138,7 +138,7 @@ public void setUp() throws Exception {
                 List.of(new ReservedClusterSettingsAction(clusterSettings))
             )
         );
-        healthIndicatorService = spy(new FileSettingsHealthIndicatorService());
+        healthIndicatorService = spy(new FileSettingsHealthIndicatorService(Settings.EMPTY));
         fileSettingsService = spy(new FileSettingsService(clusterService, controller, env, healthIndicatorService));
     }