Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] Add a "verbose" option to the data frame analytics stats endpoint (#59589) #59621

Merged
merged 1 commit into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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}
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/ml/ml-shared.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1384,3 +1384,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[]
22 changes: 21 additions & 1 deletion server/src/main/java/org/elasticsearch/common/Strings.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
Expand Down
12 changes: 12 additions & 0 deletions server/src/test/java/org/elasticsearch/common/StringsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {

Expand All @@ -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);
Expand All @@ -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));
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand All @@ -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;
Expand All @@ -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));
}
}
Loading