Add a "verbose" option to the data frame analytics stats endpoint (#59589)

Przemysław Witek · web-flow · commit dfbb47dcaa60 · 2020-07-15T15:59:56.000+02:00
diff --git a/docs/reference/ml/df-analytics/apis/get-dfanalytics-stats.asciidoc b/docs/reference/ml/df-analytics/apis/get-dfanalytics-stats.asciidoc
@@ -58,6 +58,10 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=from]
 (Optional, integer) 
 include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=size]
 
+`verbose`::
+(Optional, boolean)
+include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=verbose]
+
 [role="child_attributes"]
 [[ml-get-dfanalytics-stats-response-body]]
 ==== {api-response-body-title}
diff --git a/docs/reference/ml/ml-shared.asciidoc b/docs/reference/ml/ml-shared.asciidoc
@@ -1389,3 +1389,7 @@ tag::use-null[]
 Defines whether a new series is used as the null series when there is no value 
 for the by or partition fields. The default value is `false`.
 end::use-null[]
+
+tag::verbose[]
+Defines whether the stats response should be verbose. The default value is `false`.
+end::verbose[]
diff --git a/server/src/main/java/org/elasticsearch/common/Strings.java b/server/src/main/java/org/elasticsearch/common/Strings.java
@@ -761,6 +761,16 @@ public static String toString(ToXContent toXContent) {
         return toString(toXContent, false, false);
     }
 
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed.
+     * Allows to configure the params.
+     * The content is not pretty-printed nor human readable.
+     */
+    public static String toString(ToXContent toXContent, ToXContent.Params params) {
+        return toString(toXContent, params, false, false);
+    }
+
     /**
      * Returns a string representation of the builder (only applicable for text based xcontent).
      * @param xContentBuilder builder containing an object to converted to a string
@@ -776,12 +786,22 @@ public static String toString(XContentBuilder xContentBuilder) {
      *
      */
     public static String toString(ToXContent toXContent, boolean pretty, boolean human) {
+        return toString(toXContent, ToXContent.EMPTY_PARAMS, pretty, human);
+    }
+
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed.
+     * Allows to configure the params.
+     * Allows to control whether the outputted json needs to be pretty printed and human readable.
+     */
+    private static String toString(ToXContent toXContent, ToXContent.Params params, boolean pretty, boolean human) {
         try {
             XContentBuilder builder = createBuilder(pretty, human);
             if (toXContent.isFragment()) {
                 builder.startObject();
             }
-            toXContent.toXContent(builder, ToXContent.EMPTY_PARAMS);
+            toXContent.toXContent(builder, params);
             if (toXContent.isFragment()) {
                 builder.endObject();
             }
diff --git a/server/src/test/java/org/elasticsearch/common/StringsTests.java b/server/src/test/java/org/elasticsearch/common/StringsTests.java
@@ -24,6 +24,8 @@
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.test.ESTestCase;
 
+import java.util.Collections;
+
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
@@ -98,6 +100,16 @@ public void testToStringToXContent() {
         }
     }
 
+    public void testToStringToXContentWithOrWithoutParams() {
+        ToXContent toXContent = (builder, params) -> builder.field("color_from_param", params.param("color", "red"));
+        // Rely on the default value of "color" param when params are not passed
+        assertThat(Strings.toString(toXContent), containsString("\"color_from_param\":\"red\""));
+        // Pass "color" param explicitly
+        assertThat(
+            Strings.toString(toXContent, new ToXContent.MapParams(Collections.singletonMap("color", "blue"))),
+            containsString("\"color_from_param\":\"blue\""));
+    }
+
     public void testSplitStringToSet() {
         assertEquals(Strings.tokenizeByCommaToSet(null), Sets.newHashSet());
         assertEquals(Strings.tokenizeByCommaToSet(""), Sets.newHashSet());
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsAction.java
@@ -33,6 +33,7 @@
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.DataCounts;
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
 import org.elasticsearch.xpack.core.ml.utils.PhaseProgress;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -141,7 +142,9 @@ public boolean equals(Object obj) {
                 return false;
             }
             Request other = (Request) obj;
-            return Objects.equals(id, other.id) && allowNoMatch == other.allowNoMatch && Objects.equals(pageParams, other.pageParams);
+            return Objects.equals(id, other.id)
+                && allowNoMatch == other.allowNoMatch
+                && Objects.equals(pageParams, other.pageParams);
         }
     }
 
@@ -154,6 +157,9 @@ public RequestBuilder(ElasticsearchClient client, GetDataFrameAnalyticsStatsActi
 
     public static class Response extends BaseTasksResponse implements ToXContentObject {
 
+        /** Name of the response's REST param which is used to determine whether this response should be verbose. */
+        public static final String VERBOSE = "verbose";
+
         public static class Stats implements ToXContentObject, Writeable {
 
             private final String id;
@@ -295,12 +301,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                 // TODO: Have callers wrap the content with an object as they choose rather than forcing it upon them
                 builder.startObject();
                 {
-                    toUnwrappedXContent(builder);
+                    toUnwrappedXContent(builder, params);
                 }
                 return builder.endObject();
             }
 
-            public XContentBuilder toUnwrappedXContent(XContentBuilder builder) throws IOException {
+            private XContentBuilder toUnwrappedXContent(XContentBuilder builder, Params params) throws IOException {
                 builder.field(DataFrameAnalyticsConfig.ID.getPreferredName(), id);
                 builder.field("state", state.toString());
                 if (failureReason != null) {
@@ -313,7 +319,12 @@ public XContentBuilder toUnwrappedXContent(XContentBuilder builder) throws IOExc
                 builder.field("memory_usage", memoryUsage);
                 if (analysisStats != null) {
                     builder.startObject("analysis_stats");
-                    builder.field(analysisStats.getWriteableName(), analysisStats);
+                    builder.field(
+                        analysisStats.getWriteableName(),
+                        analysisStats,
+                        new MapParams(
+                            Collections.singletonMap(
+                                ToXContentParams.FOR_INTERNAL_STORAGE, Boolean.toString(params.paramAsBoolean(VERBOSE, false)))));
                     builder.endObject();
                 }
                 if (node != null) {
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ClassificationStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ClassificationStats.java
@@ -115,7 +115,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.field(ITERATION.getPreferredName(), iteration);
         builder.field(HYPERPARAMETERS.getPreferredName(), hyperparameters);
         builder.field(TIMING_STATS.getPreferredName(), timingStats);
-        builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss);
+        builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss, params);
         builder.endObject();
         return builder;
     }
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ValidationLoss.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ValidationLoss.java
@@ -14,6 +14,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.FoldValues;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.List;
@@ -62,7 +63,9 @@ public void writeTo(StreamOutput out) throws IOException {
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         builder.field(LOSS_TYPE.getPreferredName(), lossType);
-        builder.field(FOLD_VALUES.getPreferredName(), foldValues);
+        if (params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) {
+            builder.field(FOLD_VALUES.getPreferredName(), foldValues);
+        }
         builder.endObject();
         return builder;
     }
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/RegressionStats.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/RegressionStats.java
@@ -115,7 +115,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.field(ITERATION.getPreferredName(), iteration);
         builder.field(HYPERPARAMETERS.getPreferredName(), hyperparameters);
         builder.field(TIMING_STATS.getPreferredName(), timingStats);
-        builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss);
+        builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss, params);
         builder.endObject();
         return builder;
     }
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/ValidationLoss.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/ValidationLoss.java
@@ -14,6 +14,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.FoldValues;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 
 import java.io.IOException;
 import java.util.List;
@@ -62,7 +63,9 @@ public void writeTo(StreamOutput out) throws IOException {
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         builder.field(LOSS_TYPE.getPreferredName(), lossType);
-        builder.field(FOLD_VALUES.getPreferredName(), foldValues);
+        if (params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) {
+            builder.field(FOLD_VALUES.getPreferredName(), foldValues);
+        }
         builder.endObject();
         return builder;
     }
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsActionRequestTests.java
@@ -9,6 +9,7 @@
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 
 public class GetDataFrameAnalyticsStatsActionRequestTests extends ESTestCase {
@@ -31,4 +32,11 @@ public void testSetId_GivenString() {
 
         assertThat(request.getId(), equalTo("foo"));
     }
+
+    public void testSetAllowNoMatch() {
+        GetDataFrameAnalyticsStatsAction.Request request = new GetDataFrameAnalyticsStatsAction.Request();
+        assertThat(request.isAllowNoMatch(), is(true));
+        request.setAllowNoMatch(false);
+        assertThat(request.isAllowNoMatch(), is(false));
+    }
 }
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetDataFrameAnalyticsStatsActionResponseTests.java
@@ -5,15 +5,18 @@
  */
 package org.elasticsearch.xpack.core.ml.action;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
 import org.elasticsearch.xpack.core.action.util.QueryPage;
 import org.elasticsearch.xpack.core.ml.action.GetDataFrameAnalyticsStatsAction.Response;
 import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsConfigTests;
 import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsState;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.AnalysisStats;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.AnalysisStatsNamedWriteablesProvider;
+import org.elasticsearch.xpack.core.ml.dataframe.stats.classification.ValidationLoss;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.MemoryUsage;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.MemoryUsageTests;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.classification.ClassificationStatsTests;
@@ -26,9 +29,12 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Supplier;
 import java.util.stream.IntStream;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
 
 public class GetDataFrameAnalyticsStatsActionResponseTests extends AbstractWireSerializingTestCase<Response> {
 
@@ -40,6 +46,17 @@ protected NamedWriteableRegistry getNamedWriteableRegistry() {
     }
 
     public static Response randomResponse(int listSize) {
+        return randomResponse(
+            listSize,
+            () -> randomBoolean()
+                ? null
+                : randomFrom(
+                    ClassificationStatsTests.createRandom(),
+                    OutlierDetectionStatsTests.createRandom(),
+                    RegressionStatsTests.createRandom()));
+    }
+
+    private static Response randomResponse(int listSize, Supplier<AnalysisStats> analysisStatsSupplier) {
         List<Response.Stats> analytics = new ArrayList<>(listSize);
         for (int j = 0; j < listSize; j++) {
             String failureReason = randomBoolean() ? null : randomAlphaOfLength(10);
@@ -49,12 +66,7 @@ public static Response randomResponse(int listSize) {
                 new PhaseProgress(randomAlphaOfLength(10), randomIntBetween(0, 100))));
             DataCounts dataCounts = randomBoolean() ? null : DataCountsTests.createRandom();
             MemoryUsage memoryUsage = randomBoolean() ? null : MemoryUsageTests.createRandom();
-            AnalysisStats analysisStats = randomBoolean() ? null :
-                randomFrom(
-                    ClassificationStatsTests.createRandom(),
-                    OutlierDetectionStatsTests.createRandom(),
-                    RegressionStatsTests.createRandom()
-                );
+            AnalysisStats analysisStats = analysisStatsSupplier.get();
             Response.Stats stats = new Response.Stats(DataFrameAnalyticsConfigTests.randomValidId(),
                 randomFrom(DataFrameAnalyticsState.values()), failureReason, progress, dataCounts, memoryUsage, analysisStats, null,
                 randomAlphaOfLength(20));
@@ -88,4 +100,24 @@ public void testStats_GivenNulls() {
         assertThat(stats.getDataCounts(), equalTo(new DataCounts(stats.getId())));
         assertThat(stats.getMemoryUsage(), equalTo(new MemoryUsage(stats.getId())));
     }
+
+    public void testVerbose() {
+        String foldValuesFieldName = ValidationLoss.FOLD_VALUES.getPreferredName();
+        // Create response for supervised analysis that is certain to contain fold_values field
+        Response response =
+            randomResponse(1, () -> randomFrom(ClassificationStatsTests.createRandom(), RegressionStatsTests.createRandom()));
+
+        // VERBOSE param defaults to "false", fold values *not* outputted
+        assertThat(Strings.toString(response), not(containsString(foldValuesFieldName)));
+
+        // VERBOSE param explicitly set to "false", fold values *not* outputted
+        assertThat(
+            Strings.toString(response, new ToXContent.MapParams(Collections.singletonMap(Response.VERBOSE, "false"))),
+            not(containsString(foldValuesFieldName)));
+
+        // VERBOSE param explicitly set to "true", fold values outputted
+        assertThat(
+            Strings.toString(response, new ToXContent.MapParams(Collections.singletonMap(Response.VERBOSE, "true"))),
+            containsString(foldValuesFieldName));
+    }
 }
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ValidationLossTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/stats/classification/ValidationLossTests.java
@@ -6,13 +6,20 @@
 package org.elasticsearch.xpack.core.ml.dataframe.stats.classification;
 
 import org.elasticsearch.Version;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.xpack.core.ml.AbstractBWCSerializationTestCase;
 import org.elasticsearch.xpack.core.ml.dataframe.stats.common.FoldValuesTests;
+import org.elasticsearch.xpack.core.ml.utils.ToXContentParams;
 import org.junit.Before;
 
 import java.io.IOException;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 
 public class ValidationLossTests extends AbstractBWCSerializationTestCase<ValidationLoss> {
 
@@ -28,6 +35,11 @@ protected ValidationLoss doParseInstance(XContentParser parser) throws IOExcepti
         return ValidationLoss.fromXContent(parser, lenient);
     }
 
+    @Override
+    protected ToXContent.Params getToXContentParams() {
+        return new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true"));
+    }
+
     @Override
     protected Writeable.Reader<ValidationLoss> instanceReader() {
         return ValidationLoss::new;
@@ -41,12 +53,34 @@ protected ValidationLoss createTestInstance() {
     public static ValidationLoss createRandom() {
         return new ValidationLoss(
             randomAlphaOfLength(10),
-            randomList(5, () -> FoldValuesTests.createRandom())
+            randomList(5, FoldValuesTests::createRandom)
         );
     }
 
     @Override
     protected ValidationLoss mutateInstanceForVersion(ValidationLoss instance, Version version) {
         return instance;
     }
+
+    public void testValidationLossForStats() {
+        String foldValuesFieldName = ValidationLoss.FOLD_VALUES.getPreferredName();
+        ValidationLoss validationLoss = createTestInstance();
+
+        // FOR_INTERNAL_STORAGE param defaults to "false", fold values *not* outputted
+        assertThat(Strings.toString(validationLoss), not(containsString(foldValuesFieldName)));
+
+        // FOR_INTERNAL_STORAGE param explicitly set to "false", fold values *not* outputted
+        assertThat(
+            Strings.toString(
+                validationLoss,
+                new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "false"))),
+            not(containsString(foldValuesFieldName)));
+
+        // FOR_INTERNAL_STORAGE param explicitly set to "true", fold values are outputted
+        assertThat(
+            Strings.toString(
+                validationLoss,
+                new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true"))),
+            containsString(foldValuesFieldName));
+    }
 }
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/ValidationLossTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/stats/regression/ValidationLossTests.java
diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/dataframe/RestGetDataFrameAnalyticsStatsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/dataframe/RestGetDataFrameAnalyticsStatsAction.java
diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/api/ml.get_data_frame_analytics_stats.json b/x-pack/plugin/src/test/resources/rest-api-spec/api/ml.get_data_frame_analytics_stats.json
diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml

Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws`
`115`	`115`	`builder.field(ITERATION.getPreferredName(), iteration);`
`116`	`116`	`builder.field(HYPERPARAMETERS.getPreferredName(), hyperparameters);`
`117`	`117`	`builder.field(TIMING_STATS.getPreferredName(), timingStats);`
`118`		`- builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss);`
	`118`	`+ builder.field(VALIDATION_LOSS.getPreferredName(), validationLoss, params);`
`119`	`119`	`builder.endObject();`
`120`	`120`	`return builder;`
`121`	`121`	`}`