Change SLM stats format (#46991)

Using arrays of objects with embedded IDs is preferred for new APIs over
using entity IDs as JSON keys.  This commit changes the SLM stats API to
use the preferred format.

client/rest-high-level/src/main/java/org/elasticsearch/client/slm/SnapshotLifecycleStats.java

@@ -74,7 +74,7 @@ public class SnapshotLifecycleStats implements ToXContentObject {
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_FAILED);
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_TIMED_OUT);
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_TIME_MILLIS);
-        PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, n) -> SnapshotPolicyStats.parse(p, n), POLICY_STATS);
+        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), SnapshotPolicyStats.PARSER, POLICY_STATS);
     }
 
     // Package visible for testing
@@ -178,22 +178,25 @@ public class SnapshotLifecycleStats implements ToXContentObject {
         private final long snapshotsDeleted;
         private final long snapshotDeleteFailures;
 
+        public static final ParseField POLICY_ID = new ParseField("policy");
         static final ParseField SNAPSHOTS_TAKEN = new ParseField("snapshots_taken");
         static final ParseField SNAPSHOTS_FAILED = new ParseField("snapshots_failed");
         static final ParseField SNAPSHOTS_DELETED = new ParseField("snapshots_deleted");
         static final ParseField SNAPSHOT_DELETION_FAILURES = new ParseField("snapshot_deletion_failures");
 
-        private static final ConstructingObjectParser<SnapshotPolicyStats, String> PARSER =
+        private static final ConstructingObjectParser<SnapshotPolicyStats, Void> PARSER =
             new ConstructingObjectParser<>("snapshot_policy_stats", true,
-                (a, id) -> {
+                a -> {
-                    long taken = (long) a[0];
+                    String id = (String) a[0];
-                    long failed = (long) a[1];
+                    long taken = (long) a[1];
-                    long deleted = (long) a[2];
+                    long failed = (long) a[2];
-                    long deleteFailed = (long) a[3];
+                    long deleted = (long) a[3];
+                    long deleteFailed = (long) a[4];
                     return new SnapshotPolicyStats(id, taken, failed, deleted, deleteFailed);
                 });
 
         static {
+            PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_ID);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_TAKEN);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_FAILED);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_DELETED);
@@ -209,7 +212,11 @@ public class SnapshotLifecycleStats implements ToXContentObject {
         }
 
         public static SnapshotPolicyStats parse(XContentParser parser, String policyId) {
-            return PARSER.apply(parser, policyId);
+            return PARSER.apply(parser, null);
+        }
+
+        public String getPolicyId() {
+            return policyId;
         }
 
         public long getSnapshotsTaken() {

docs/reference/ilm/apis/slm-api.asciidoc

@@ -142,6 +142,7 @@ The output looks similar to the following:
       "retention": {}
     },
     "stats": {
+      "policy": "daily-snapshots",
       "snapshots_taken": 0,
       "snapshots_failed": 0,
       "snapshots_deleted": 0,
@@ -231,6 +232,7 @@ Which, in this case shows an error because the index did not exist:
       "retention": {}
     },
     "stats": {
+      "policy": "daily-snapshots",
       "snapshots_taken": 0,
       "snapshots_failed": 1,
       "snapshots_deleted": 0,
@@ -319,6 +321,7 @@ Which now includes the successful snapshot information:
       "retention": {}
     },
     "stats": {
+      "policy": "daily-snapshots",
       "snapshots_taken": 1,
       "snapshots_failed": 1,
       "snapshots_deleted": 0,
@@ -371,14 +374,15 @@ Which returns a response similar to:
   "retention_timed_out": 0,
   "retention_deletion_time": "1.4s",
   "retention_deletion_time_millis": 1404,
-  "policy_metrics": {
+  "policy_metrics": [
-    "daily-snapshots": {
+    {
+      "policy": "daily-snapshots",
       "snapshots_taken": 1,
       "snapshots_failed": 1,
       "snapshots_deleted": 0,
       "snapshot_deletion_failures": 0
     }
-  },
+  ],
   "total_snapshots_taken": 1,
   "total_snapshots_failed": 1,
   "total_snapshots_deleted": 0,

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleStats.java

@@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -71,7 +72,7 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_FAILED);
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_TIMED_OUT);
         PARSER.declareLong(ConstructingObjectParser.constructorArg(), RETENTION_TIME_MILLIS);
-        PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, n) -> SnapshotPolicyStats.parse(p, n), POLICY_STATS);
+        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), SnapshotPolicyStats.PARSER, POLICY_STATS);
     }
 
     public SnapshotLifecycleStats() {
@@ -213,23 +214,25 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
         builder.field(RETENTION_TIME.getPreferredName(), retentionTime);
         builder.field(RETENTION_TIME_MILLIS.getPreferredName(), retentionTime.millis());
 
-        Map<String, SnapshotPolicyStats> metrics = getMetrics();
+        List<SnapshotPolicyStats> metrics = getMetrics().values().stream()
-        long totalTaken = metrics.values().stream().mapToLong(s -> s.snapshotsTaken.count()).sum();
+            .sorted(Comparator.comparing(SnapshotPolicyStats::getPolicyId)) // maintain a consistent order when serializing
-        long totalFailed = metrics.values().stream().mapToLong(s -> s.snapshotsFailed.count()).sum();
+            .collect(Collectors.toList());
-        long totalDeleted = metrics.values().stream().mapToLong(s -> s.snapshotsDeleted.count()).sum();
+        long totalTaken = metrics.stream().mapToLong(s -> s.snapshotsTaken.count()).sum();
-        long totalDeleteFailures = metrics.values().stream().mapToLong(s -> s.snapshotDeleteFailures.count()).sum();
+        long totalFailed = metrics.stream().mapToLong(s -> s.snapshotsFailed.count()).sum();
+        long totalDeleted = metrics.stream().mapToLong(s -> s.snapshotsDeleted.count()).sum();
+        long totalDeleteFailures = metrics.stream().mapToLong(s -> s.snapshotDeleteFailures.count()).sum();
         builder.field(TOTAL_TAKEN.getPreferredName(), totalTaken);
         builder.field(TOTAL_FAILED.getPreferredName(), totalFailed);
         builder.field(TOTAL_DELETIONS.getPreferredName(), totalDeleted);
         builder.field(TOTAL_DELETION_FAILURES.getPreferredName(), totalDeleteFailures);
-        builder.startObject(POLICY_STATS.getPreferredName());
+
-        for (Map.Entry<String, SnapshotPolicyStats> policy : metrics.entrySet()) {
+        builder.startArray(POLICY_STATS.getPreferredName());
-            SnapshotPolicyStats perPolicyMetrics = policy.getValue();
+        for (SnapshotPolicyStats stats : metrics) {
-            builder.startObject(perPolicyMetrics.policyId);
+            builder.startObject();
-            perPolicyMetrics.toXContent(builder, params);
+            stats.toXContent(builder, params);
             builder.endObject();
         }
-        builder.endObject();
+        builder.endArray();
         builder.endObject();
         return builder;
     }
@@ -268,22 +271,25 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
         private final CounterMetric snapshotsDeleted = new CounterMetric();
         private final CounterMetric snapshotDeleteFailures = new CounterMetric();
 
+        public static final ParseField POLICY_ID = new ParseField("policy");
         public static final ParseField SNAPSHOTS_TAKEN = new ParseField("snapshots_taken");
         public static final ParseField SNAPSHOTS_FAILED = new ParseField("snapshots_failed");
         public static final ParseField SNAPSHOTS_DELETED = new ParseField("snapshots_deleted");
         public static final ParseField SNAPSHOT_DELETION_FAILURES = new ParseField("snapshot_deletion_failures");
 
-        private static final ConstructingObjectParser<SnapshotPolicyStats, String> PARSER =
+        static final ConstructingObjectParser<SnapshotPolicyStats, Void> PARSER =
             new ConstructingObjectParser<>("snapshot_policy_stats", true,
-                (a, id) -> {
+                a -> {
-                    long taken = (long) a[0];
+                    String id = (String) a[0];
-                    long failed = (long) a[1];
+                    long taken = (long) a[1];
-                    long deleted = (long) a[2];
+                    long failed = (long) a[2];
-                    long deleteFailed = (long) a[3];
+                    long deleted = (long) a[3];
+                    long deleteFailed = (long) a[4];
                     return new SnapshotPolicyStats(id, taken, failed, deleted, deleteFailed);
                 });
 
         static {
+            PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_ID);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_TAKEN);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_FAILED);
             PARSER.declareLong(ConstructingObjectParser.constructorArg(), SNAPSHOTS_DELETED);
@@ -310,8 +316,8 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
             this.snapshotDeleteFailures.inc(in.readVLong());
         }
 
-        public static SnapshotPolicyStats parse(XContentParser parser, String policyId) {
+        public static SnapshotPolicyStats parse(XContentParser parser) {
-            return PARSER.apply(parser, policyId);
+            return PARSER.apply(parser, null);
         }
 
         public SnapshotPolicyStats merge(SnapshotPolicyStats other) {
@@ -339,6 +345,10 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
             snapshotDeleteFailures.inc();
         }
 
+        public String getPolicyId() {
+            return policyId;
+        }
+
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             out.writeString(policyId);
@@ -372,6 +382,7 @@ public class SnapshotLifecycleStats implements Writeable, ToXContentObject {
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+            builder.field(SnapshotPolicyStats.POLICY_ID.getPreferredName(), policyId);
             builder.field(SnapshotPolicyStats.SNAPSHOTS_TAKEN.getPreferredName(), snapshotsTaken.count());
             builder.field(SnapshotPolicyStats.SNAPSHOTS_FAILED.getPreferredName(), snapshotsFailed.count());
             builder.field(SnapshotPolicyStats.SNAPSHOTS_DELETED.getPreferredName(), snapshotsDeleted.count());

x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java

@@ -41,6 +41,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.xpack.core.slm.history.SnapshotHistoryItem.CREATE_OPERATION;
@@ -131,7 +133,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
             assertHistoryIsPresent(policyName, true, repoId, CREATE_OPERATION);
 
             Map<String, Object> stats = getSLMStats();
-            Map<String, Object> policyStats = (Map<String, Object>) stats.get(SnapshotLifecycleStats.POLICY_STATS.getPreferredName());
+            Map<String, Object> policyStats = policyStatsAsMap(stats);
             Map<String, Object> policyIdStats = (Map<String, Object>) policyStats.get(policyName);
             int snapsTaken = (int) policyIdStats.get(SnapshotLifecycleStats.SnapshotPolicyStats.SNAPSHOTS_TAKEN.getPreferredName());
             int totalTaken = (int) stats.get(SnapshotLifecycleStats.TOTAL_TAKEN.getPreferredName());
@@ -180,7 +182,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
             assertHistoryIsPresent(policyName, false, repoName, CREATE_OPERATION);
 
             Map<String, Object> stats = getSLMStats();
-            Map<String, Object> policyStats = (Map<String, Object>) stats.get(SnapshotLifecycleStats.POLICY_STATS.getPreferredName());
+            Map<String, Object> policyStats = policyStatsAsMap(stats);
             Map<String, Object> policyIdStats = (Map<String, Object>) policyStats.get(policyName);
             int snapsFailed = (int) policyIdStats.get(SnapshotLifecycleStats.SnapshotPolicyStats.SNAPSHOTS_FAILED.getPreferredName());
             int totalFailed = (int) stats.get(SnapshotLifecycleStats.TOTAL_FAILED.getPreferredName());
@@ -229,7 +231,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
             }
 
             Map<String, Object> stats = getSLMStats();
-            Map<String, Object> policyStats = (Map<String, Object>) stats.get(SnapshotLifecycleStats.POLICY_STATS.getPreferredName());
+            Map<String, Object> policyStats = policyStatsAsMap(stats);
             Map<String, Object> policyIdStats = (Map<String, Object>) policyStats.get(policyName);
             int snapsTaken = (int) policyIdStats.get(SnapshotLifecycleStats.SnapshotPolicyStats.SNAPSHOTS_TAKEN.getPreferredName());
             int totalTaken = (int) stats.get(SnapshotLifecycleStats.TOTAL_TAKEN.getPreferredName());
@@ -301,7 +303,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
                 assertHistoryIsPresent(policyName, true, repoId, DELETE_OPERATION);
 
                 Map<String, Object> stats = getSLMStats();
-                Map<String, Object> policyStats = (Map<String, Object>) stats.get(SnapshotLifecycleStats.POLICY_STATS.getPreferredName());
+                Map<String, Object> policyStats = policyStatsAsMap(stats);
                 Map<String, Object> policyIdStats = (Map<String, Object>) policyStats.get(policyName);
                 int snapsTaken = (int) policyIdStats.get(SnapshotLifecycleStats.SnapshotPolicyStats.SNAPSHOTS_TAKEN.getPreferredName());
                 int snapsDeleted = (int) policyIdStats.get(SnapshotLifecycleStats.SnapshotPolicyStats.SNAPSHOTS_DELETED.getPreferredName());
@@ -482,4 +484,13 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
         request.setJsonEntity(Strings.toString(document));
         assertOK(client.performRequest(request));
     }
+
+    @SuppressWarnings("unchecked")
+    private static Map<String, Object> policyStatsAsMap(Map<String, Object> stats) {
+        return ((List<Map<String, Object>>) stats.get(SnapshotLifecycleStats.POLICY_STATS.getPreferredName()))
+            .stream()
+            .collect(Collectors.toMap(
+                m -> (String) m.get(SnapshotLifecycleStats.SnapshotPolicyStats.POLICY_ID.getPreferredName()),
+                Function.identity()));
+    }
 }