From b939b82f0bdab18a8eb5caf6701356b58cbc8964 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Mon, 23 Sep 2019 16:13:21 -0400 Subject: [PATCH 1/7] [ML] Add new (actual|typical)_geo field for lat_lon results --- .../common/xcontent/XContentBuilder.java | 2 +- .../persistence/ElasticsearchMappings.java | 13 ++++ .../core/ml/job/results/AnomalyCause.java | 20 ++++++ .../core/ml/job/results/AnomalyRecord.java | 26 ++++++- .../ml/job/results/ReservedFieldNames.java | 4 ++ .../ElasticsearchMappingsTests.java | 4 +- .../ml/job/results/AnomalyRecordTests.java | 68 +++++++++++++++++++ .../job/persistence/JobResultsPersister.java | 7 +- 8 files changed, 138 insertions(+), 6 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index 51a4f86a0d3b2..ae535449fc3a2 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -850,7 +850,7 @@ private XContentBuilder value(ToXContent value) throws IOException { return value(value, ToXContent.EMPTY_PARAMS); } - private XContentBuilder value(ToXContent value, ToXContent.Params params) throws IOException { + public XContentBuilder value(ToXContent value, ToXContent.Params params) throws IOException { if (value == null) { return nullValue(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 804e9c8dcda69..7213c268bc042 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -129,6 +129,7 @@ public class ElasticsearchMappings { public static final String BOOLEAN = "boolean"; public static final String DATE = "date"; public static final String DOUBLE = "double"; + public static final String GEO_POINT = "geo_point"; public static final String INTEGER = "integer"; public static final String KEYWORD = "keyword"; public static final String LONG = "long"; @@ -761,9 +762,15 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .startObject(AnomalyRecord.ACTUAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() + .startObject(AnomalyRecord.ACTUAL_GEO.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() .startObject(AnomalyRecord.TYPICAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() + .startObject(AnomalyRecord.TYPICAL_GEO.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() .startObject(AnomalyRecord.PROBABILITY.getPreferredName()) .field(TYPE, DOUBLE) .endObject() @@ -812,9 +819,15 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .startObject(AnomalyCause.ACTUAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() + .startObject(AnomalyCause.ACTUAL_GEO.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() .startObject(AnomalyCause.TYPICAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() + .startObject(AnomalyCause.TYPICAL_GEO.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() .startObject(AnomalyCause.PROBABILITY.getPreferredName()) .field(TYPE, DOUBLE) .endObject() diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java index 50efe24ab0f6f..aa1d550f3377b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java @@ -12,6 +12,8 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; +import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; import java.io.IOException; import java.util.List; @@ -37,7 +39,9 @@ public class AnomalyCause implements ToXContentObject, Writeable { public static final ParseField FUNCTION = new ParseField("function"); public static final ParseField FUNCTION_DESCRIPTION = new ParseField("function_description"); public static final ParseField TYPICAL = new ParseField("typical"); + public static final ParseField TYPICAL_GEO = new ParseField("typical_geo"); public static final ParseField ACTUAL = new ParseField("actual"); + public static final ParseField ACTUAL_GEO = new ParseField("actual_geo"); public static final ParseField INFLUENCERS = new ParseField("influencers"); /** @@ -173,9 +177,25 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } if (typical != null) { builder.field(TYPICAL.getPreferredName(), typical); + if (DetectorFunction.LAT_LONG.getFullName().equals(function) && + params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { + //TODO should we fail if `typical` is not an array of length 2? + assert typical.size() == 2 : "lat_lon function typical result has invalid format " + typical; + if (typical.size() == 2) { + builder.field(TYPICAL_GEO.getPreferredName(), typical.get(0) + "," + typical.get(1)); + } + } } if (actual != null) { builder.field(ACTUAL.getPreferredName(), actual); + if (DetectorFunction.LAT_LONG.getFullName().equals(function) && + params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { + //TODO should we fail if `actual` is not an array of length 2? + assert actual.size() == 2 : "lat_lon function actual result has invalid format " + actual; + if (actual.size() == 2) { + builder.field(ACTUAL_GEO.getPreferredName(), actual.get(0) + "," + actual.get(1)); + } + } } if (fieldName != null) { builder.field(FIELD_NAME.getPreferredName(), fieldName); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index ab32373644d19..9f39bace54328 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -15,9 +15,11 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.xpack.core.ml.job.config.Detector; +import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.core.common.time.TimeUtils; +import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; import java.io.IOException; import java.util.ArrayList; @@ -52,7 +54,9 @@ public class AnomalyRecord implements ToXContentObject, Writeable { public static final ParseField FUNCTION = new ParseField("function"); public static final ParseField FUNCTION_DESCRIPTION = new ParseField("function_description"); public static final ParseField TYPICAL = new ParseField("typical"); + public static final ParseField TYPICAL_GEO = new ParseField("typical_geo"); public static final ParseField ACTUAL = new ParseField("actual"); + public static final ParseField ACTUAL_GEO = new ParseField("actual_geo"); public static final ParseField INFLUENCERS = new ParseField("influencers"); public static final ParseField BUCKET_SPAN = new ParseField("bucket_span"); @@ -276,9 +280,25 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I } if (typical != null) { builder.field(TYPICAL.getPreferredName(), typical); + if (DetectorFunction.LAT_LONG.getFullName().equals(function) && + params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { + //TODO should we fail if `typical` is not an array of length 2? + assert typical.size() == 2 : "lat_lon function typical result has invalid format " + typical; + if (typical.size() == 2) { + builder.field(TYPICAL_GEO.getPreferredName(), typical.get(0) + "," + typical.get(1)); + } + } } if (actual != null) { builder.field(ACTUAL.getPreferredName(), actual); + if (DetectorFunction.LAT_LONG.getFullName().equals(function) && + params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { + //TODO should we fail if `actual` is not an array of length 2? + assert actual.size() == 2 : "lat_lon function actual result has invalid format " + actual; + if (actual.size() == 2) { + builder.field(ACTUAL_GEO.getPreferredName(), actual.get(0) + "," + actual.get(1)); + } + } } if (fieldName != null) { builder.field(FIELD_NAME.getPreferredName(), fieldName); @@ -290,7 +310,11 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I builder.field(OVER_FIELD_VALUE.getPreferredName(), overFieldValue); } if (causes != null) { - builder.field(CAUSES.getPreferredName(), causes); + builder.startArray(CAUSES.getPreferredName()); + for (AnomalyCause cause : causes) { + builder.value(cause, params); + } + builder.endArray(); } if (influences != null) { builder.field(INFLUENCERS.getPreferredName(), influences); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index 97f1fce796351..088501156044e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -71,7 +71,9 @@ public final class ReservedFieldNames { AnomalyCause.FUNCTION.getPreferredName(), AnomalyCause.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyCause.TYPICAL.getPreferredName(), + AnomalyCause.TYPICAL_GEO.getPreferredName(), AnomalyCause.ACTUAL.getPreferredName(), + AnomalyCause.ACTUAL_GEO.getPreferredName(), AnomalyCause.INFLUENCERS.getPreferredName(), AnomalyCause.FIELD_NAME.getPreferredName(), @@ -85,7 +87,9 @@ public final class ReservedFieldNames { AnomalyRecord.FUNCTION.getPreferredName(), AnomalyRecord.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyRecord.TYPICAL.getPreferredName(), + AnomalyRecord.TYPICAL_GEO.getPreferredName(), AnomalyRecord.ACTUAL.getPreferredName(), + AnomalyRecord.ACTUAL_GEO.getPreferredName(), AnomalyRecord.INFLUENCERS.getPreferredName(), AnomalyRecord.FIELD_NAME.getPreferredName(), AnomalyRecord.OVER_FIELD_NAME.getPreferredName(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappingsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappingsTests.java index 421e04d814f8e..0ff3d720aa8c2 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappingsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappingsTests.java @@ -88,7 +88,7 @@ public class ElasticsearchMappingsTests extends ESTestCase { GetResult._INDEX ); - public void testResultsMapppingReservedFields() throws Exception { + public void testResultsMappingReservedFields() throws Exception { Set overridden = new HashSet<>(KEYWORDS); // These are not reserved because they're data types, not field names @@ -108,7 +108,7 @@ public void testResultsMapppingReservedFields() throws Exception { compareFields(expected, ReservedFieldNames.RESERVED_RESULT_FIELD_NAMES); } - public void testConfigMapppingReservedFields() throws Exception { + public void testConfigMappingReservedFields() throws Exception { Set overridden = new HashSet<>(KEYWORDS); // These are not reserved because they're data types, not field names diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java index 882a46f3cbe20..89a23391f69cc 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java @@ -5,14 +5,19 @@ */ package org.elasticsearch.xpack.core.ml.job.results; +import org.elasticsearch.client.ml.job.config.DetectorFunction; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; import java.io.IOException; import java.util.ArrayList; @@ -24,6 +29,7 @@ import java.util.Objects; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; public class AnomalyRecordTests extends AbstractSerializingTestCase { @@ -216,4 +222,66 @@ public void testLenientParser() throws IOException { AnomalyRecord.LENIENT_PARSER.apply(parser, null); } } + + public void testActualTypicalValuesGeoSerialization() throws IOException { + AnomalyRecord anomalyRecord = new AnomalyRecord("test-job-with-lat-lon", new Date(1000), 60L); + anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); + List actual = Arrays.asList(34.93873, -82.22706); + List typical = Arrays.asList(29.4241, -98.4936); + anomalyRecord.setActual(actual); + anomalyRecord.setTypical(typical); + + String reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), + false).utf8ToString(); + assertThat(reference, containsString("\"actual_geo\":\"34.93873,-82.22706\"")); + assertThat(reference, containsString("\"typical_geo\":\"29.4241,-98.4936\"")); + + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + false).utf8ToString(); + assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); + assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + + anomalyRecord.setFunction(DetectorFunction.AVG.getFullName()); + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), + false).utf8ToString(); + assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); + assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + } + + public void testActualTypicalCauseValuesGeoSerialization() throws IOException { + AnomalyRecord anomalyRecord = new AnomalyRecord("test-job-with-lat-lon", new Date(1000), 60L); + AnomalyCause cause = new AnomalyCause(); + List actual = Arrays.asList(34.93873, -82.22706); + List typical = Arrays.asList(29.4241, -98.4936); + cause.setActual(actual); + cause.setTypical(typical); + cause.setFunction(DetectorFunction.LAT_LONG.getFullName()); + anomalyRecord.setCauses(Collections.singletonList(cause)); + + String reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), + false).utf8ToString(); + assertThat(reference, containsString("\"actual_geo\":\"34.93873,-82.22706\"")); + assertThat(reference, containsString("\"typical_geo\":\"29.4241,-98.4936\"")); + + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + false).utf8ToString(); + assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); + assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + + cause.setFunction(DetectorFunction.AVG.getFullName()); + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), + false).utf8ToString(); + assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); + assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java index 783706259a17b..d1c9b06edff8d 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java @@ -69,6 +69,9 @@ public class JobResultsPersister { private static final Logger logger = LogManager.getLogger(JobResultsPersister.class); + private static final ToXContent.Params FOR_INTERNAL_STORAGE = + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")); + private final Client client; public JobResultsPersister(Client client) { @@ -134,7 +137,7 @@ public Builder persistTimingStats(TimingStats timingStats) { indexResult( TimingStats.documentId(timingStats.getJobId()), timingStats, - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), + FOR_INTERNAL_STORAGE, TimingStats.TYPE.getPreferredName()); return this; } @@ -148,7 +151,7 @@ public Builder persistTimingStats(TimingStats timingStats) { public Builder persistRecords(List records) { for (AnomalyRecord record : records) { logger.trace("[{}] ES BULK ACTION: index record to index [{}] with ID [{}]", jobId, indexName, record.getId()); - indexResult(record.getId(), record, "record"); + indexResult(record.getId(), record, FOR_INTERNAL_STORAGE, "record"); } return this; From b05ba94ef7cc8ced00ee4a3c8d0182a50cc1d84d Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Wed, 25 Sep 2019 06:38:03 -0400 Subject: [PATCH 2/7] Update AnomalyRecordTests.java --- .../xpack/core/ml/job/results/AnomalyRecordTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java index 89a23391f69cc..c3466f679d9cd 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java @@ -9,8 +9,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; From 1598c1f0599d67e1dd46c306c8bc77ed94bfe7dc Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 25 Oct 2019 09:00:42 -0400 Subject: [PATCH 3/7] changing from _geo, to geo.* format --- .../client/ml/job/results/AnomalyCause.java | 25 ++++++++++ .../client/ml/job/results/AnomalyRecord.java | 25 ++++++++++ .../ml/job/results/AnomalyCauseTests.java | 44 ++++++++++++++++ .../ml/job/results/AnomalyRecordTests.java | 43 ++++++++++++++++ .../apis/resultsresource.asciidoc | 14 +++++- .../persistence/ElasticsearchMappings.java | 32 +++++++----- .../core/ml/job/results/AnomalyCause.java | 36 +++++++------ .../core/ml/job/results/AnomalyRecord.java | 37 +++++++------- .../ml/job/results/ReservedFieldNames.java | 6 +-- .../ml/job/results/AnomalyCauseTests.java | 30 +++++++++-- .../ml/job/results/AnomalyRecordTests.java | 50 +++++++++++++------ 11 files changed, 267 insertions(+), 75 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyCause.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyCause.java index 4fbe5ac1ff381..1c7cb1fcdb06c 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyCause.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyCause.java @@ -18,7 +18,10 @@ */ package org.elasticsearch.client.ml.job.results; +import org.elasticsearch.client.ml.job.config.DetectorFunction; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -256,6 +259,28 @@ void setInfluencers(List influencers) { this.influencers = Collections.unmodifiableList(influencers); } + @Nullable + public GeoPoint getTypicalGeoPoint() { + if (DetectorFunction.LAT_LONG.getFullName().equals(function) == false || typical == null) { + return null; + } + if (typical.size() == 2) { + return new GeoPoint(typical.get(0), typical.get(1)); + } + return null; + } + + @Nullable + public GeoPoint getActualGeoPoint() { + if (DetectorFunction.LAT_LONG.getFullName().equals(function) == false || actual == null) { + return null; + } + if (actual.size() == 2) { + return new GeoPoint(actual.get(0), actual.get(1)); + } + return null; + } + @Override public int hashCode() { return Objects.hash(probability, actual, typical, byFieldName, byFieldValue, correlatedByFieldValue, fieldName, function, diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyRecord.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyRecord.java index 3c52aad74d0a8..e7789d22c5021 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyRecord.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/results/AnomalyRecord.java @@ -19,8 +19,11 @@ package org.elasticsearch.client.ml.job.results; import org.elasticsearch.client.common.TimeUtil; +import org.elasticsearch.client.ml.job.config.DetectorFunction; import org.elasticsearch.client.ml.job.config.Job; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -388,6 +391,28 @@ void setInfluencers(List influencers) { this.influences = Collections.unmodifiableList(influencers); } + @Nullable + public GeoPoint getTypicalGeoPoint() { + if (DetectorFunction.LAT_LONG.getFullName().equals(function) == false || typical == null) { + return null; + } + if (typical.size() == 2) { + return new GeoPoint(typical.get(0), typical.get(1)); + } + return null; + } + + @Nullable + public GeoPoint getActualGeoPoint() { + if (DetectorFunction.LAT_LONG.getFullName().equals(function) == false || actual == null) { + return null; + } + if (actual.size() == 2) { + return new GeoPoint(actual.get(0), actual.get(1)); + } + return null; + } + @Override public int hashCode() { return Objects.hash(jobId, detectorIndex, bucketSpan, probability, multiBucketImpact, recordScore, diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyCauseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyCauseTests.java index 3ac6a0b6ec4db..f6cae338eb2bc 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyCauseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyCauseTests.java @@ -18,12 +18,20 @@ */ package org.elasticsearch.client.ml.job.results; +import org.elasticsearch.client.ml.job.config.DetectorFunction; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractXContentTestCase; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.core.Is.is; + public class AnomalyCauseTests extends AbstractXContentTestCase { @Override @@ -103,4 +111,40 @@ protected AnomalyCause doParseInstance(XContentParser parser) { protected boolean supportsUnknownFields() { return true; } + + public void testActualAsGeoPoint() { + AnomalyCause anomalyCause = new AnomalyCause(); + + assertThat(anomalyCause.getActualGeoPoint(), is(nullValue())); + + anomalyCause.setFunction(DetectorFunction.LAT_LONG.getFullName()); + assertThat(anomalyCause.getActualGeoPoint(), is(nullValue())); + + anomalyCause.setActual(Collections.singletonList(80.0)); + assertThat(anomalyCause.getActualGeoPoint(), is(nullValue())); + + anomalyCause.setActual(Arrays.asList(90.0, 80.0)); + assertThat(anomalyCause.getActualGeoPoint(), equalTo(new GeoPoint(90.0, 80.0))); + + anomalyCause.setActual(Arrays.asList(10.0, 100.0, 90.0)); + assertThat(anomalyCause.getActualGeoPoint(), is(nullValue())); + } + + public void testTypicalAsGeoPoint() { + AnomalyCause anomalyCause = new AnomalyCause(); + + assertThat(anomalyCause.getTypicalGeoPoint(), is(nullValue())); + + anomalyCause.setFunction(DetectorFunction.LAT_LONG.getFullName()); + assertThat(anomalyCause.getTypicalGeoPoint(), is(nullValue())); + + anomalyCause.setTypical(Collections.singletonList(80.0)); + assertThat(anomalyCause.getTypicalGeoPoint(), is(nullValue())); + + anomalyCause.setTypical(Arrays.asList(90.0, 80.0)); + assertThat(anomalyCause.getTypicalGeoPoint(), equalTo(new GeoPoint(90.0, 80.0))); + + anomalyCause.setTypical(Arrays.asList(10.0, 100.0, 90.0)); + assertThat(anomalyCause.getTypicalGeoPoint(), is(nullValue())); + } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyRecordTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyRecordTests.java index 39bfff3a7e8f9..923e25aa2b45f 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyRecordTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/results/AnomalyRecordTests.java @@ -18,14 +18,21 @@ */ package org.elasticsearch.client.ml.job.results; +import org.elasticsearch.client.ml.job.config.DetectorFunction; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractXContentTestCase; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.core.Is.is; + public class AnomalyRecordTests extends AbstractXContentTestCase { @Override @@ -93,4 +100,40 @@ protected AnomalyRecord doParseInstance(XContentParser parser) { protected boolean supportsUnknownFields() { return true; } + + public void testActualAsGeoPoint() { + AnomalyRecord anomalyRecord = new AnomalyRecord(randomAlphaOfLength(10), new Date(), randomNonNegativeLong()); + + assertThat(anomalyRecord.getActualGeoPoint(), is(nullValue())); + + anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); + assertThat(anomalyRecord.getActualGeoPoint(), is(nullValue())); + + anomalyRecord.setActual(Collections.singletonList(80.0)); + assertThat(anomalyRecord.getActualGeoPoint(), is(nullValue())); + + anomalyRecord.setActual(Arrays.asList(90.0, 80.0)); + assertThat(anomalyRecord.getActualGeoPoint(), equalTo(new GeoPoint(90.0, 80.0))); + + anomalyRecord.setActual(Arrays.asList(10.0, 100.0, 90.0)); + assertThat(anomalyRecord.getActualGeoPoint(), is(nullValue())); + } + + public void testTypicalAsGeoPoint() { + AnomalyRecord anomalyRecord = new AnomalyRecord(randomAlphaOfLength(10), new Date(), randomNonNegativeLong()); + + assertThat(anomalyRecord.getTypicalGeoPoint(), is(nullValue())); + + anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); + assertThat(anomalyRecord.getTypicalGeoPoint(), is(nullValue())); + + anomalyRecord.setTypical(Collections.singletonList(80.0)); + assertThat(anomalyRecord.getTypicalGeoPoint(), is(nullValue())); + + anomalyRecord.setTypical(Arrays.asList(90.0, 80.0)); + assertThat(anomalyRecord.getTypicalGeoPoint(), equalTo(new GeoPoint(90.0, 80.0))); + + anomalyRecord.setTypical(Arrays.asList(10.0, 100.0, 90.0)); + assertThat(anomalyRecord.getTypicalGeoPoint(), is(nullValue())); + } } diff --git a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc index ce6a8a90015a3..731dac90dd46d 100644 --- a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc +++ b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc @@ -303,8 +303,8 @@ A record object has the following properties: part of the core analytical modeling, these low-level anomaly records are aggregated for their parent over field record. The causes resource contains similar elements to the record resource, namely `actual`, `typical`, - `*_field_name` and `*_field_value`. Probability and scores are not applicable - to causes. + `geo.actual`, `geo.typical`, `*_field_name` and `*_field_value`. + Probability and scores are not applicable to causes. `detector_index`:: (number) A unique identifier for the detector. @@ -383,6 +383,16 @@ A record object has the following properties: `typical`:: (array) The typical value for the bucket, according to analytical modeling. +`geo.actual`:: + (string) The actual value for the bucket formatted as a `geo_point`. + If the detector function is `lat_long`, this is a comma delimited string + of the latitude and longitude. + +`geo.typical`:: + (string) The typical value for the bucket formatted as a `geo_point`. + If the detector function is `lat_long`, this is a comma delimited string + of the latitude and longitude. + NOTE: Additional record properties are added, depending on the fields being analyzed. For example, if it's analyzing `hostname` as a _by field_, then a field `hostname` is added to the result document. This information enables you to diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 6b646e9f958f8..739cb5c978d41 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -795,15 +795,9 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .startObject(AnomalyRecord.ACTUAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() - .startObject(AnomalyRecord.ACTUAL_GEO.getPreferredName()) - .field(TYPE, GEO_POINT) - .endObject() .startObject(AnomalyRecord.TYPICAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() - .startObject(AnomalyRecord.TYPICAL_GEO.getPreferredName()) - .field(TYPE, GEO_POINT) - .endObject() .startObject(AnomalyRecord.PROBABILITY.getPreferredName()) .field(TYPE, DOUBLE) .endObject() @@ -852,15 +846,9 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .startObject(AnomalyCause.ACTUAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() - .startObject(AnomalyCause.ACTUAL_GEO.getPreferredName()) - .field(TYPE, GEO_POINT) - .endObject() .startObject(AnomalyCause.TYPICAL.getPreferredName()) .field(TYPE, DOUBLE) .endObject() - .startObject(AnomalyCause.TYPICAL_GEO.getPreferredName()) - .field(TYPE, GEO_POINT) - .endObject() .startObject(AnomalyCause.PROBABILITY.getPreferredName()) .field(TYPE, DOUBLE) .endObject() @@ -898,6 +886,16 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .field(TYPE, KEYWORD) .field(COPY_TO, ALL_FIELD_VALUES) .endObject() + .startObject(AnomalyCause.GEO_FIELDS.getPreferredName()) + .startObject(PROPERTIES) + .startObject(AnomalyCause.ACTUAL.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() + .startObject(AnomalyCause.TYPICAL.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() + .endObject() + .endObject() .endObject() .endObject() .startObject(AnomalyRecord.INFLUENCERS.getPreferredName()) @@ -912,6 +910,16 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .field(COPY_TO, ALL_FIELD_VALUES) .endObject() .endObject() + .endObject() + .startObject(AnomalyRecord.GEO_FIELDS.getPreferredName()) + .startObject(PROPERTIES) + .startObject(AnomalyRecord.ACTUAL.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() + .startObject(AnomalyRecord.TYPICAL.getPreferredName()) + .field(TYPE, GEO_POINT) + .endObject() + .endObject() .endObject(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java index aa1d550f3377b..671880b989317 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; -import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; import java.io.IOException; import java.util.List; @@ -39,10 +38,9 @@ public class AnomalyCause implements ToXContentObject, Writeable { public static final ParseField FUNCTION = new ParseField("function"); public static final ParseField FUNCTION_DESCRIPTION = new ParseField("function_description"); public static final ParseField TYPICAL = new ParseField("typical"); - public static final ParseField TYPICAL_GEO = new ParseField("typical_geo"); public static final ParseField ACTUAL = new ParseField("actual"); - public static final ParseField ACTUAL_GEO = new ParseField("actual_geo"); public static final ParseField INFLUENCERS = new ParseField("influencers"); + public static final ParseField GEO_FIELDS = new ParseField("geo"); /** * Metric Results @@ -177,25 +175,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } if (typical != null) { builder.field(TYPICAL.getPreferredName(), typical); - if (DetectorFunction.LAT_LONG.getFullName().equals(function) && - params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { - //TODO should we fail if `typical` is not an array of length 2? - assert typical.size() == 2 : "lat_lon function typical result has invalid format " + typical; - if (typical.size() == 2) { - builder.field(TYPICAL_GEO.getPreferredName(), typical.get(0) + "," + typical.get(1)); - } - } } if (actual != null) { builder.field(ACTUAL.getPreferredName(), actual); - if (DetectorFunction.LAT_LONG.getFullName().equals(function) && - params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { - //TODO should we fail if `actual` is not an array of length 2? - assert actual.size() == 2 : "lat_lon function actual result has invalid format " + actual; - if (actual.size() == 2) { - builder.field(ACTUAL_GEO.getPreferredName(), actual.get(0) + "," + actual.get(1)); - } - } } if (fieldName != null) { builder.field(FIELD_NAME.getPreferredName(), fieldName); @@ -209,10 +191,26 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (influencers != null) { builder.field(INFLUENCERS.getPreferredName(), influencers); } + if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { + builder.startObject(GEO_FIELDS.getPreferredName()); + writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); + writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); + builder.endObject(); + } builder.endObject(); return builder; } + private void writeDoublesAsGeoPoint(XContentBuilder builder, String fieldName, List doubles) throws IOException { + if (doubles != null) { + //TODO should we fail if `doubles` is not an array of length 2? + assert doubles.size() == 2 : "for lat_lon function [" + fieldName + "] result has invalid format " + doubles; + if (doubles.size() == 2) { + builder.field(fieldName, doubles.get(0) + "," + doubles.get(1)); + } + } + } + public double getProbability() { return probability; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index 9f39bace54328..01a95ea04a670 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -19,7 +19,6 @@ import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.core.common.time.TimeUtils; -import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; import java.io.IOException; import java.util.ArrayList; @@ -54,11 +53,10 @@ public class AnomalyRecord implements ToXContentObject, Writeable { public static final ParseField FUNCTION = new ParseField("function"); public static final ParseField FUNCTION_DESCRIPTION = new ParseField("function_description"); public static final ParseField TYPICAL = new ParseField("typical"); - public static final ParseField TYPICAL_GEO = new ParseField("typical_geo"); public static final ParseField ACTUAL = new ParseField("actual"); - public static final ParseField ACTUAL_GEO = new ParseField("actual_geo"); public static final ParseField INFLUENCERS = new ParseField("influencers"); public static final ParseField BUCKET_SPAN = new ParseField("bucket_span"); + public static final ParseField GEO_FIELDS = new ParseField("geo"); // Used for QueryPage public static final ParseField RESULTS_FIELD = new ParseField("records"); @@ -280,25 +278,9 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I } if (typical != null) { builder.field(TYPICAL.getPreferredName(), typical); - if (DetectorFunction.LAT_LONG.getFullName().equals(function) && - params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { - //TODO should we fail if `typical` is not an array of length 2? - assert typical.size() == 2 : "lat_lon function typical result has invalid format " + typical; - if (typical.size() == 2) { - builder.field(TYPICAL_GEO.getPreferredName(), typical.get(0) + "," + typical.get(1)); - } - } } if (actual != null) { builder.field(ACTUAL.getPreferredName(), actual); - if (DetectorFunction.LAT_LONG.getFullName().equals(function) && - params.paramAsBoolean(ToXContentParams.FOR_INTERNAL_STORAGE, false)) { - //TODO should we fail if `actual` is not an array of length 2? - assert actual.size() == 2 : "lat_lon function actual result has invalid format " + actual; - if (actual.size() == 2) { - builder.field(ACTUAL_GEO.getPreferredName(), actual.get(0) + "," + actual.get(1)); - } - } } if (fieldName != null) { builder.field(FIELD_NAME.getPreferredName(), fieldName); @@ -319,6 +301,12 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I if (influences != null) { builder.field(INFLUENCERS.getPreferredName(), influences); } + if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { + builder.startObject(GEO_FIELDS.getPreferredName()); + writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); + writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); + builder.endObject(); + } Map> inputFields = inputFieldMap(); for (String fieldName : inputFields.keySet()) { @@ -327,6 +315,17 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I return builder; } + + private void writeDoublesAsGeoPoint(XContentBuilder builder, String fieldName, List doubles) throws IOException { + if (doubles != null) { + //TODO should we fail if `doubles` is not an array of length 2? + assert doubles.size() == 2 : "for lat_lon function [" + fieldName + "] result has invalid format " + doubles; + if (doubles.size() == 2) { + builder.field(fieldName, doubles.get(0) + "," + doubles.get(1)); + } + } + } + private Map> inputFieldMap() { // LinkedHashSet preserves insertion order when iterating entries Map> result = new HashMap<>(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index 9880c0bafb26d..62c48b7cf4db7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -73,9 +73,8 @@ public final class ReservedFieldNames { AnomalyCause.FUNCTION.getPreferredName(), AnomalyCause.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyCause.TYPICAL.getPreferredName(), - AnomalyCause.TYPICAL_GEO.getPreferredName(), AnomalyCause.ACTUAL.getPreferredName(), - AnomalyCause.ACTUAL_GEO.getPreferredName(), + AnomalyCause.GEO_FIELDS.getPreferredName(), AnomalyCause.INFLUENCERS.getPreferredName(), AnomalyCause.FIELD_NAME.getPreferredName(), @@ -89,9 +88,8 @@ public final class ReservedFieldNames { AnomalyRecord.FUNCTION.getPreferredName(), AnomalyRecord.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyRecord.TYPICAL.getPreferredName(), - AnomalyRecord.TYPICAL_GEO.getPreferredName(), AnomalyRecord.ACTUAL.getPreferredName(), - AnomalyRecord.ACTUAL_GEO.getPreferredName(), + AnomalyRecord.GEO_FIELDS.getPreferredName(), AnomalyRecord.INFLUENCERS.getPreferredName(), AnomalyRecord.FIELD_NAME.getPreferredName(), AnomalyRecord.OVER_FIELD_NAME.getPreferredName(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java index 033392eddce27..8fa97a65ab1ee 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java @@ -9,17 +9,26 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; +import org.junit.Before; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static org.hamcrest.Matchers.containsString; public class AnomalyCauseTests extends AbstractSerializingTestCase { - @Override - protected AnomalyCause createTestInstance() { + private boolean lenient; + + @Before + public void setLenient() { + lenient = randomBoolean(); + } + + protected AnomalyCause createTestInstance(boolean lenient) { AnomalyCause anomalyCause = new AnomalyCause(); if (randomBoolean()) { int size = randomInt(10); @@ -59,7 +68,15 @@ protected AnomalyCause createTestInstance() { anomalyCause.setPartitionFieldValue(randomAlphaOfLengthBetween(1, 20)); } if (randomBoolean()) { - anomalyCause.setFunction(randomAlphaOfLengthBetween(1, 20)); + if (randomBoolean() && lenient) { + anomalyCause.setFunction(DetectorFunction.LAT_LONG.getFullName()); + anomalyCause.setActual(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), + randomDoubleBetween(-90.0, 90.0, true))); + anomalyCause.setTypical(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), + randomDoubleBetween(-90.0, 90.0, true))); + } else { + anomalyCause.setFunction(randomAlphaOfLengthBetween(5, 25)); + } } if (randomBoolean()) { anomalyCause.setFunctionDescription(randomAlphaOfLengthBetween(1, 20)); @@ -86,6 +103,11 @@ protected AnomalyCause createTestInstance() { return anomalyCause; } + @Override + protected AnomalyCause createTestInstance() { + return createTestInstance(lenient); + } + @Override protected Reader instanceReader() { return AnomalyCause::new; @@ -93,7 +115,7 @@ protected Reader instanceReader() { @Override protected AnomalyCause doParseInstance(XContentParser parser) { - return AnomalyCause.STRICT_PARSER.apply(parser, null); + return lenient ? AnomalyCause.LENIENT_PARSER.apply(parser, null) : AnomalyCause.STRICT_PARSER.apply(parser, null); } public void testStrictParser() throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java index c3466f679d9cd..6b79ffb15898c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; +import org.junit.Before; import java.io.IOException; import java.util.ArrayList; @@ -31,6 +32,13 @@ public class AnomalyRecordTests extends AbstractSerializingTestCase { + private boolean lenient; + + @Before + public void setLenient() { + lenient = randomBoolean(); + } + @Override protected AnomalyRecord createTestInstance() { return createTestInstance("foo"); @@ -62,7 +70,15 @@ public AnomalyRecord createTestInstance(String jobId) { anomalyRecord.setOverFieldName(randomAlphaOfLength(12)); anomalyRecord.setOverFieldValue(randomAlphaOfLength(12)); } - anomalyRecord.setFunction(randomAlphaOfLengthBetween(5, 20)); + if (randomBoolean() && lenient) { + anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); + anomalyRecord.setActual(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), + randomDoubleBetween(-90.0, 90.0, true))); + anomalyRecord.setTypical(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), + randomDoubleBetween(-90.0, 90.0, true))); + } else { + anomalyRecord.setFunction(randomAlphaOfLengthBetween(5, 25)); + } anomalyRecord.setFunctionDescription(randomAlphaOfLengthBetween(5, 20)); if (randomBoolean()) { anomalyRecord.setCorrelatedByFieldValue(randomAlphaOfLength(16)); @@ -79,7 +95,7 @@ public AnomalyRecord createTestInstance(String jobId) { int count = randomIntBetween(0, 9); List causes = new ArrayList<>(); for (int i=0; i instanceReader() { @Override protected AnomalyRecord doParseInstance(XContentParser parser) { - return AnomalyRecord.STRICT_PARSER.apply(parser, null); + return lenient ? AnomalyRecord.LENIENT_PARSER.apply(parser, null) : AnomalyRecord.STRICT_PARSER.apply(parser, null); } @SuppressWarnings("unchecked") @@ -233,22 +249,24 @@ public void testActualTypicalValuesGeoSerialization() throws IOException { XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, containsString("\"actual_geo\":\"34.93873,-82.22706\"")); - assertThat(reference, containsString("\"typical_geo\":\"29.4241,-98.4936\"")); + assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); - assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); anomalyRecord.setFunction(DetectorFunction.AVG.getFullName()); + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + false).utf8ToString(); + assertThat(reference, not(containsString("\"geo\""))); + reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); - assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + assertThat(reference, not(containsString("\"geo\""))); } public void testActualTypicalCauseValuesGeoSerialization() throws IOException { @@ -265,21 +283,23 @@ public void testActualTypicalCauseValuesGeoSerialization() throws IOException { XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, containsString("\"actual_geo\":\"34.93873,-82.22706\"")); - assertThat(reference, containsString("\"typical_geo\":\"29.4241,-98.4936\"")); + assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); - assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); cause.setFunction(DetectorFunction.AVG.getFullName()); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, not(containsString("\"actual_geo\":\"34.93873,-82.22706\""))); - assertThat(reference, not(containsString("\"typical_geo\":\"29.4241,-98.4936\""))); + assertThat(reference, not(containsString("\"geo\""))); + + reference = XContentHelper.toXContent(anomalyRecord, + XContentType.JSON, + false).utf8ToString(); + assertThat(reference, not(containsString("\"geo\""))); } } From 3db79b8daec0fc445fa0bf542d0e66928113215c Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 25 Oct 2019 11:42:25 -0400 Subject: [PATCH 4/7] changing geo field to `geo_results` --- .../apis/resultsresource.asciidoc | 6 +++--- .../xpack/core/ml/job/results/AnomalyCause.java | 2 +- .../xpack/core/ml/job/results/AnomalyRecord.java | 2 +- .../core/ml/job/results/AnomalyRecordTests.java | 16 ++++++++-------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc index 731dac90dd46d..cb9b00e4fe882 100644 --- a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc +++ b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc @@ -303,7 +303,7 @@ A record object has the following properties: part of the core analytical modeling, these low-level anomaly records are aggregated for their parent over field record. The causes resource contains similar elements to the record resource, namely `actual`, `typical`, - `geo.actual`, `geo.typical`, `*_field_name` and `*_field_value`. + `geo_results.actual`, `geo_results.typical`, `*_field_name` and `*_field_value`. Probability and scores are not applicable to causes. `detector_index`:: @@ -383,12 +383,12 @@ A record object has the following properties: `typical`:: (array) The typical value for the bucket, according to analytical modeling. -`geo.actual`:: +`geo_results.actual`:: (string) The actual value for the bucket formatted as a `geo_point`. If the detector function is `lat_long`, this is a comma delimited string of the latitude and longitude. -`geo.typical`:: +`geo_results.typical`:: (string) The typical value for the bucket formatted as a `geo_point`. If the detector function is `lat_long`, this is a comma delimited string of the latitude and longitude. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java index 671880b989317..486bd67a98374 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java @@ -40,7 +40,7 @@ public class AnomalyCause implements ToXContentObject, Writeable { public static final ParseField TYPICAL = new ParseField("typical"); public static final ParseField ACTUAL = new ParseField("actual"); public static final ParseField INFLUENCERS = new ParseField("influencers"); - public static final ParseField GEO_FIELDS = new ParseField("geo"); + public static final ParseField GEO_FIELDS = new ParseField("geo_results"); /** * Metric Results diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index 01a95ea04a670..1f3ea1b9fd466 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -56,7 +56,7 @@ public class AnomalyRecord implements ToXContentObject, Writeable { public static final ParseField ACTUAL = new ParseField("actual"); public static final ParseField INFLUENCERS = new ParseField("influencers"); public static final ParseField BUCKET_SPAN = new ParseField("bucket_span"); - public static final ParseField GEO_FIELDS = new ParseField("geo"); + public static final ParseField GEO_FIELDS = new ParseField("geo_results"); // Used for QueryPage public static final ParseField RESULTS_FIELD = new ParseField("records"); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java index 6b79ffb15898c..4bd56347feac1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java @@ -249,24 +249,24 @@ public void testActualTypicalValuesGeoSerialization() throws IOException { XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); + assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); + assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); anomalyRecord.setFunction(DetectorFunction.AVG.getFullName()); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, not(containsString("\"geo\""))); + assertThat(reference, not(containsString("\"geo_results\""))); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, not(containsString("\"geo\""))); + assertThat(reference, not(containsString("\"geo_results\""))); } public void testActualTypicalCauseValuesGeoSerialization() throws IOException { @@ -283,23 +283,23 @@ public void testActualTypicalCauseValuesGeoSerialization() throws IOException { XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); + assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, containsString("\"geo\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); + assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); cause.setFunction(DetectorFunction.AVG.getFullName()); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), false).utf8ToString(); - assertThat(reference, not(containsString("\"geo\""))); + assertThat(reference, not(containsString("\"geo_results\""))); reference = XContentHelper.toXContent(anomalyRecord, XContentType.JSON, false).utf8ToString(); - assertThat(reference, not(containsString("\"geo\""))); + assertThat(reference, not(containsString("\"geo_results\""))); } } From 5fbba226106d723cd07c212789a5bebc4e46991d Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 25 Oct 2019 11:42:57 -0400 Subject: [PATCH 5/7] finish refactor --- .../xpack/core/ml/job/persistence/ElasticsearchMappings.java | 4 ++-- .../elasticsearch/xpack/core/ml/job/results/AnomalyCause.java | 4 ++-- .../xpack/core/ml/job/results/AnomalyRecord.java | 4 ++-- .../xpack/core/ml/job/results/ReservedFieldNames.java | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 739cb5c978d41..060bb3778a291 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -886,7 +886,7 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .field(TYPE, KEYWORD) .field(COPY_TO, ALL_FIELD_VALUES) .endObject() - .startObject(AnomalyCause.GEO_FIELDS.getPreferredName()) + .startObject(AnomalyCause.GEO_RESULTS.getPreferredName()) .startObject(PROPERTIES) .startObject(AnomalyCause.ACTUAL.getPreferredName()) .field(TYPE, GEO_POINT) @@ -911,7 +911,7 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .endObject() .endObject() .endObject() - .startObject(AnomalyRecord.GEO_FIELDS.getPreferredName()) + .startObject(AnomalyRecord.GEO_RESULTS.getPreferredName()) .startObject(PROPERTIES) .startObject(AnomalyRecord.ACTUAL.getPreferredName()) .field(TYPE, GEO_POINT) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java index 486bd67a98374..37be89c799ea1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java @@ -40,7 +40,7 @@ public class AnomalyCause implements ToXContentObject, Writeable { public static final ParseField TYPICAL = new ParseField("typical"); public static final ParseField ACTUAL = new ParseField("actual"); public static final ParseField INFLUENCERS = new ParseField("influencers"); - public static final ParseField GEO_FIELDS = new ParseField("geo_results"); + public static final ParseField GEO_RESULTS = new ParseField("geo_results"); /** * Metric Results @@ -192,7 +192,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(INFLUENCERS.getPreferredName(), influencers); } if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { - builder.startObject(GEO_FIELDS.getPreferredName()); + builder.startObject(GEO_RESULTS.getPreferredName()); writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); builder.endObject(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index 1f3ea1b9fd466..b1272824eb619 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -56,7 +56,7 @@ public class AnomalyRecord implements ToXContentObject, Writeable { public static final ParseField ACTUAL = new ParseField("actual"); public static final ParseField INFLUENCERS = new ParseField("influencers"); public static final ParseField BUCKET_SPAN = new ParseField("bucket_span"); - public static final ParseField GEO_FIELDS = new ParseField("geo_results"); + public static final ParseField GEO_RESULTS = new ParseField("geo_results"); // Used for QueryPage public static final ParseField RESULTS_FIELD = new ParseField("records"); @@ -302,7 +302,7 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I builder.field(INFLUENCERS.getPreferredName(), influences); } if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { - builder.startObject(GEO_FIELDS.getPreferredName()); + builder.startObject(GEO_RESULTS.getPreferredName()); writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); builder.endObject(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index 62c48b7cf4db7..d1ebab9390d79 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -74,7 +74,7 @@ public final class ReservedFieldNames { AnomalyCause.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyCause.TYPICAL.getPreferredName(), AnomalyCause.ACTUAL.getPreferredName(), - AnomalyCause.GEO_FIELDS.getPreferredName(), + AnomalyCause.GEO_RESULTS.getPreferredName(), AnomalyCause.INFLUENCERS.getPreferredName(), AnomalyCause.FIELD_NAME.getPreferredName(), @@ -89,7 +89,7 @@ public final class ReservedFieldNames { AnomalyRecord.FUNCTION_DESCRIPTION.getPreferredName(), AnomalyRecord.TYPICAL.getPreferredName(), AnomalyRecord.ACTUAL.getPreferredName(), - AnomalyRecord.GEO_FIELDS.getPreferredName(), + AnomalyRecord.GEO_RESULTS.getPreferredName(), AnomalyRecord.INFLUENCERS.getPreferredName(), AnomalyRecord.FIELD_NAME.getPreferredName(), AnomalyRecord.OVER_FIELD_NAME.getPreferredName(), From 62b04c06f2b4763edda1bc97a483689801f3741e Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Tue, 5 Nov 2019 08:26:23 -0500 Subject: [PATCH 6/7] moving to serializing from the C++ side --- .../apis/resultsresource.asciidoc | 7 +- .../persistence/ElasticsearchMappings.java | 9 +- .../core/ml/job/results/AnomalyCause.java | 47 ++++---- .../core/ml/job/results/AnomalyRecord.java | 46 ++++---- .../xpack/core/ml/job/results/GeoResults.java | 101 ++++++++++++++++++ .../ml/job/results/ReservedFieldNames.java | 3 + .../ml/job/results/AnomalyCauseTests.java | 32 ++---- .../ml/job/results/AnomalyRecordTests.java | 88 +-------------- .../core/ml/job/results/GeoResultsTests.java | 71 ++++++++++++ 9 files changed, 250 insertions(+), 154 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/GeoResults.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/GeoResultsTests.java diff --git a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc index cb9b00e4fe882..b35100c24e6ae 100644 --- a/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc +++ b/docs/reference/ml/anomaly-detection/apis/resultsresource.asciidoc @@ -303,7 +303,8 @@ A record object has the following properties: part of the core analytical modeling, these low-level anomaly records are aggregated for their parent over field record. The causes resource contains similar elements to the record resource, namely `actual`, `typical`, - `geo_results.actual`, `geo_results.typical`, `*_field_name` and `*_field_value`. + `geo_results.actual_point`, `geo_results.typical_point`, + `*_field_name` and `*_field_value`. Probability and scores are not applicable to causes. `detector_index`:: @@ -383,12 +384,12 @@ A record object has the following properties: `typical`:: (array) The typical value for the bucket, according to analytical modeling. -`geo_results.actual`:: +`geo_results.actual_point`:: (string) The actual value for the bucket formatted as a `geo_point`. If the detector function is `lat_long`, this is a comma delimited string of the latitude and longitude. -`geo_results.typical`:: +`geo_results.typical_point`:: (string) The typical value for the bucket formatted as a `geo_point`. If the detector function is `lat_long`, this is a comma delimited string of the latitude and longitude. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java index 060bb3778a291..a11afff794aca 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java @@ -55,6 +55,7 @@ import org.elasticsearch.xpack.core.ml.job.results.CategoryDefinition; import org.elasticsearch.xpack.core.ml.job.results.Forecast; import org.elasticsearch.xpack.core.ml.job.results.ForecastRequestStats; +import org.elasticsearch.xpack.core.ml.job.results.GeoResults; import org.elasticsearch.xpack.core.ml.job.results.Influence; import org.elasticsearch.xpack.core.ml.job.results.Influencer; import org.elasticsearch.xpack.core.ml.job.results.ModelPlot; @@ -888,10 +889,10 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .endObject() .startObject(AnomalyCause.GEO_RESULTS.getPreferredName()) .startObject(PROPERTIES) - .startObject(AnomalyCause.ACTUAL.getPreferredName()) + .startObject(GeoResults.ACTUAL_POINT.getPreferredName()) .field(TYPE, GEO_POINT) .endObject() - .startObject(AnomalyCause.TYPICAL.getPreferredName()) + .startObject(GeoResults.TYPICAL_POINT.getPreferredName()) .field(TYPE, GEO_POINT) .endObject() .endObject() @@ -913,10 +914,10 @@ private static void addAnomalyRecordFieldsToMapping(XContentBuilder builder) thr .endObject() .startObject(AnomalyRecord.GEO_RESULTS.getPreferredName()) .startObject(PROPERTIES) - .startObject(AnomalyRecord.ACTUAL.getPreferredName()) + .startObject(GeoResults.ACTUAL_POINT.getPreferredName()) .field(TYPE, GEO_POINT) .endObject() - .startObject(AnomalyRecord.TYPICAL.getPreferredName()) + .startObject(GeoResults.TYPICAL_POINT.getPreferredName()) .field(TYPE, GEO_POINT) .endObject() .endObject() diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java index 37be89c799ea1..1f66a6a575a41 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCause.java @@ -5,14 +5,15 @@ */ package org.elasticsearch.xpack.core.ml.job.results; +import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; import java.io.IOException; import java.util.List; @@ -69,6 +70,9 @@ private static ObjectParser createParser(boolean ignoreUnkno parser.declareString(AnomalyCause::setOverFieldValue, OVER_FIELD_VALUE); parser.declareObjectArray(AnomalyCause::setInfluencers, ignoreUnknownFields ? Influence.LENIENT_PARSER : Influence.STRICT_PARSER, INFLUENCERS); + parser.declareObject(AnomalyCause::setGeoResults, + ignoreUnknownFields ? GeoResults.LENIENT_PARSER : GeoResults.STRICT_PARSER, + GEO_RESULTS); return parser; } @@ -83,6 +87,7 @@ private static ObjectParser createParser(boolean ignoreUnkno private String functionDescription; private List typical; private List actual; + private GeoResults geoResults; private String fieldName; @@ -116,6 +121,9 @@ public AnomalyCause(StreamInput in) throws IOException { if (in.readBoolean()) { influencers = in.readList(Influence::new); } + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + geoResults = in.readOptionalWriteable(GeoResults::new); + } } @Override @@ -146,6 +154,9 @@ public void writeTo(StreamOutput out) throws IOException { if (hasInfluencers) { out.writeList(influencers); } + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeOptionalWriteable(geoResults); + } } @Override @@ -191,27 +202,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (influencers != null) { builder.field(INFLUENCERS.getPreferredName(), influencers); } - if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { - builder.startObject(GEO_RESULTS.getPreferredName()); - writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); - writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); - builder.endObject(); + if (geoResults != null) { + builder.field(GEO_RESULTS.getPreferredName(), geoResults); } builder.endObject(); return builder; } - private void writeDoublesAsGeoPoint(XContentBuilder builder, String fieldName, List doubles) throws IOException { - if (doubles != null) { - //TODO should we fail if `doubles` is not an array of length 2? - assert doubles.size() == 2 : "for lat_lon function [" + fieldName + "] result has invalid format " + doubles; - if (doubles.size() == 2) { - builder.field(fieldName, doubles.get(0) + "," + doubles.get(1)); - } - } - } - - public double getProbability() { return probability; } @@ -325,6 +322,14 @@ public void setInfluencers(List influencers) { this.influencers = influencers; } + public GeoResults getGeoResults() { + return geoResults; + } + + public void setGeoResults(GeoResults geoResults) { + this.geoResults = geoResults; + } + @Override public int hashCode() { return Objects.hash(probability, @@ -340,7 +345,8 @@ public int hashCode() { overFieldValue, partitionFieldName, partitionFieldValue, - influencers); + influencers, + geoResults); } @Override @@ -368,8 +374,13 @@ public boolean equals(Object other) { Objects.equals(this.partitionFieldValue, that.partitionFieldValue) && Objects.equals(this.overFieldName, that.overFieldName) && Objects.equals(this.overFieldValue, that.overFieldValue) && + Objects.equals(this.geoResults, that.geoResults) && Objects.equals(this.influencers, that.influencers); } + @Override + public String toString() { + return Strings.toString(this, true, true); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index b1272824eb619..5770ec095f0ba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.ml.job.results; +import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -15,7 +16,6 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.xpack.core.ml.job.config.Detector; -import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import org.elasticsearch.xpack.core.common.time.TimeUtils; @@ -116,6 +116,9 @@ private static ConstructingObjectParser createParser(boolea CAUSES); parser.declareObjectArray(AnomalyRecord::setInfluencers, ignoreUnknownFields ? Influence.LENIENT_PARSER : Influence.STRICT_PARSER, INFLUENCERS); + parser.declareObject(AnomalyRecord::setGeoResults, + ignoreUnknownFields ? GeoResults.LENIENT_PARSER : GeoResults.STRICT_PARSER, + GEO_RESULTS); return parser; } @@ -134,6 +137,7 @@ private static ConstructingObjectParser createParser(boolea private List typical; private List actual; private boolean isInterim; + private GeoResults geoResults; private String fieldName; @@ -189,6 +193,9 @@ public AnomalyRecord(StreamInput in) throws IOException { if (in.readBoolean()) { influences = in.readList(Influence::new); } + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + geoResults = in.readOptionalWriteable(GeoResults::new); + } } @Override @@ -232,6 +239,9 @@ public void writeTo(StreamOutput out) throws IOException { if (hasInfluencers) { out.writeList(influences); } + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeOptionalWriteable(geoResults); + } } @Override @@ -301,11 +311,8 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I if (influences != null) { builder.field(INFLUENCERS.getPreferredName(), influences); } - if (DetectorFunction.LAT_LONG.getFullName().equals(function)) { - builder.startObject(GEO_RESULTS.getPreferredName()); - writeDoublesAsGeoPoint(builder, ACTUAL.getPreferredName(), actual); - writeDoublesAsGeoPoint(builder, TYPICAL.getPreferredName(), typical); - builder.endObject(); + if (geoResults != null) { + builder.field(GEO_RESULTS.getPreferredName(), geoResults); } Map> inputFields = inputFieldMap(); @@ -315,17 +322,6 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I return builder; } - - private void writeDoublesAsGeoPoint(XContentBuilder builder, String fieldName, List doubles) throws IOException { - if (doubles != null) { - //TODO should we fail if `doubles` is not an array of length 2? - assert doubles.size() == 2 : "for lat_lon function [" + fieldName + "] result has invalid format " + doubles; - if (doubles.size() == 2) { - builder.field(fieldName, doubles.get(0) + "," + doubles.get(1)); - } - } - } - private Map> inputFieldMap() { // LinkedHashSet preserves insertion order when iterating entries Map> result = new HashMap<>(); @@ -547,6 +543,13 @@ public void setInfluencers(List influencers) { this.influences = influencers; } + public GeoResults getGeoResults() { + return geoResults; + } + + public void setGeoResults(GeoResults geoResults) { + this.geoResults = geoResults; + } @Override public int hashCode() { @@ -554,10 +557,9 @@ public int hashCode() { initialRecordScore, typical, actual,function, functionDescription, fieldName, byFieldName, byFieldValue, correlatedByFieldValue, partitionFieldName, partitionFieldValue, overFieldName, overFieldValue, timestamp, isInterim, - causes, influences, jobId); + causes, influences, jobId, geoResults); } - @Override public boolean equals(Object other) { if (this == other) { @@ -592,6 +594,12 @@ public boolean equals(Object other) { && Objects.equals(this.timestamp, that.timestamp) && Objects.equals(this.isInterim, that.isInterim) && Objects.equals(this.causes, that.causes) + && Objects.equals(this.geoResults, that.geoResults) && Objects.equals(this.influences, that.influences); } + + @Override + public String toString() { + return Strings.toString(this, true, true); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/GeoResults.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/GeoResults.java new file mode 100644 index 0000000000000..93168095bb738 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/GeoResults.java @@ -0,0 +1,101 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.results; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Objects; + +public class GeoResults implements ToXContentObject, Writeable { + + public static final ParseField GEO_RESULTS = new ParseField("geo_results"); + + public static final ParseField TYPICAL_POINT = new ParseField("typical_point"); + public static final ParseField ACTUAL_POINT = new ParseField("actual_point"); + + public static final ObjectParser STRICT_PARSER = createParser(false); + public static final ObjectParser LENIENT_PARSER = createParser(true); + + private static ObjectParser createParser(boolean ignoreUnknownFields) { + ObjectParser parser = new ObjectParser<>(GEO_RESULTS.getPreferredName(), ignoreUnknownFields, + GeoResults::new); + parser.declareString(GeoResults::setActualPoint, ACTUAL_POINT); + parser.declareString(GeoResults::setTypicalPoint, TYPICAL_POINT); + return parser; + } + + private String actualPoint; + private String typicalPoint; + + public GeoResults() {} + + public GeoResults(StreamInput in) throws IOException { + this.actualPoint = in.readOptionalString(); + this.typicalPoint = in.readOptionalString(); + } + + public String getActualPoint() { + return actualPoint; + } + + public void setActualPoint(String actualPoint) { + this.actualPoint = actualPoint; + } + + public String getTypicalPoint() { + return typicalPoint; + } + + public void setTypicalPoint(String typicalPoint) { + this.typicalPoint = typicalPoint; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeOptionalString(actualPoint); + out.writeOptionalString(typicalPoint); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + if (typicalPoint != null) { + builder.field(TYPICAL_POINT.getPreferredName(), typicalPoint); + } + if (actualPoint != null) { + builder.field(ACTUAL_POINT.getPreferredName(), actualPoint); + } + builder.endObject(); + return builder; + } + + @Override + public int hashCode() { + return Objects.hash(typicalPoint, actualPoint); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + GeoResults that = (GeoResults) other; + return Objects.equals(this.typicalPoint, that.typicalPoint) && Objects.equals(this.actualPoint, that.actualPoint); + } + +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index d1ebab9390d79..e0ae183d17a0f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -99,6 +99,9 @@ public final class ReservedFieldNames { AnomalyRecord.INITIAL_RECORD_SCORE.getPreferredName(), AnomalyRecord.BUCKET_SPAN.getPreferredName(), + GeoResults.TYPICAL_POINT.getPreferredName(), + GeoResults.ACTUAL_POINT.getPreferredName(), + Bucket.ANOMALY_SCORE.getPreferredName(), Bucket.BUCKET_INFLUENCERS.getPreferredName(), Bucket.BUCKET_SPAN.getPreferredName(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java index 8fa97a65ab1ee..dbcad6444dd5e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyCauseTests.java @@ -5,30 +5,22 @@ */ package org.elasticsearch.xpack.core.ml.job.results; +import org.elasticsearch.client.ml.job.config.DetectorFunction; import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.ml.job.config.DetectorFunction; -import org.junit.Before; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import static org.hamcrest.Matchers.containsString; public class AnomalyCauseTests extends AbstractSerializingTestCase { - private boolean lenient; - - @Before - public void setLenient() { - lenient = randomBoolean(); - } - - protected AnomalyCause createTestInstance(boolean lenient) { + @Override + protected AnomalyCause createTestInstance() { AnomalyCause anomalyCause = new AnomalyCause(); if (randomBoolean()) { int size = randomInt(10); @@ -68,15 +60,8 @@ protected AnomalyCause createTestInstance(boolean lenient) { anomalyCause.setPartitionFieldValue(randomAlphaOfLengthBetween(1, 20)); } if (randomBoolean()) { - if (randomBoolean() && lenient) { - anomalyCause.setFunction(DetectorFunction.LAT_LONG.getFullName()); - anomalyCause.setActual(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), - randomDoubleBetween(-90.0, 90.0, true))); - anomalyCause.setTypical(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), - randomDoubleBetween(-90.0, 90.0, true))); - } else { - anomalyCause.setFunction(randomAlphaOfLengthBetween(5, 25)); - } + anomalyCause.setFunction(DetectorFunction.LAT_LONG.getFullName()); + anomalyCause.setGeoResults(GeoResultsTests.createTestGeoResults()); } if (randomBoolean()) { anomalyCause.setFunctionDescription(randomAlphaOfLengthBetween(1, 20)); @@ -103,11 +88,6 @@ protected AnomalyCause createTestInstance(boolean lenient) { return anomalyCause; } - @Override - protected AnomalyCause createTestInstance() { - return createTestInstance(lenient); - } - @Override protected Reader instanceReader() { return AnomalyCause::new; @@ -115,7 +95,7 @@ protected Reader instanceReader() { @Override protected AnomalyCause doParseInstance(XContentParser parser) { - return lenient ? AnomalyCause.LENIENT_PARSER.apply(parser, null) : AnomalyCause.STRICT_PARSER.apply(parser, null); + return AnomalyCause.STRICT_PARSER.apply(parser, null); } public void testStrictParser() throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java index 4bd56347feac1..7c94e6721435c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecordTests.java @@ -8,15 +8,12 @@ import org.elasticsearch.client.ml.job.config.DetectorFunction; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.ml.utils.ToXContentParams; -import org.junit.Before; import java.io.IOException; import java.util.ArrayList; @@ -28,17 +25,9 @@ import java.util.Objects; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; public class AnomalyRecordTests extends AbstractSerializingTestCase { - private boolean lenient; - - @Before - public void setLenient() { - lenient = randomBoolean(); - } - @Override protected AnomalyRecord createTestInstance() { return createTestInstance("foo"); @@ -70,12 +59,9 @@ public AnomalyRecord createTestInstance(String jobId) { anomalyRecord.setOverFieldName(randomAlphaOfLength(12)); anomalyRecord.setOverFieldValue(randomAlphaOfLength(12)); } - if (randomBoolean() && lenient) { + if (randomBoolean()) { anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); - anomalyRecord.setActual(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), - randomDoubleBetween(-90.0, 90.0, true))); - anomalyRecord.setTypical(Arrays.asList(randomDoubleBetween(-90.0, 90.0, true), - randomDoubleBetween(-90.0, 90.0, true))); + anomalyRecord.setGeoResults(GeoResultsTests.createTestGeoResults()); } else { anomalyRecord.setFunction(randomAlphaOfLengthBetween(5, 25)); } @@ -95,7 +81,7 @@ public AnomalyRecord createTestInstance(String jobId) { int count = randomIntBetween(0, 9); List causes = new ArrayList<>(); for (int i=0; i instanceReader() { @Override protected AnomalyRecord doParseInstance(XContentParser parser) { - return lenient ? AnomalyRecord.LENIENT_PARSER.apply(parser, null) : AnomalyRecord.STRICT_PARSER.apply(parser, null); + return AnomalyRecord.STRICT_PARSER.apply(parser, null); } @SuppressWarnings("unchecked") @@ -236,70 +222,4 @@ public void testLenientParser() throws IOException { AnomalyRecord.LENIENT_PARSER.apply(parser, null); } } - - public void testActualTypicalValuesGeoSerialization() throws IOException { - AnomalyRecord anomalyRecord = new AnomalyRecord("test-job-with-lat-lon", new Date(1000), 60L); - anomalyRecord.setFunction(DetectorFunction.LAT_LONG.getFullName()); - List actual = Arrays.asList(34.93873, -82.22706); - List typical = Arrays.asList(29.4241, -98.4936); - anomalyRecord.setActual(actual); - anomalyRecord.setTypical(typical); - - String reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), - false).utf8ToString(); - assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); - - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - false).utf8ToString(); - assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); - - anomalyRecord.setFunction(DetectorFunction.AVG.getFullName()); - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - false).utf8ToString(); - assertThat(reference, not(containsString("\"geo_results\""))); - - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), - false).utf8ToString(); - assertThat(reference, not(containsString("\"geo_results\""))); - } - - public void testActualTypicalCauseValuesGeoSerialization() throws IOException { - AnomalyRecord anomalyRecord = new AnomalyRecord("test-job-with-lat-lon", new Date(1000), 60L); - AnomalyCause cause = new AnomalyCause(); - List actual = Arrays.asList(34.93873, -82.22706); - List typical = Arrays.asList(29.4241, -98.4936); - cause.setActual(actual); - cause.setTypical(typical); - cause.setFunction(DetectorFunction.LAT_LONG.getFullName()); - anomalyRecord.setCauses(Collections.singletonList(cause)); - - String reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), - false).utf8ToString(); - assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); - - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - false).utf8ToString(); - assertThat(reference, containsString("\"geo_results\":{\"actual\":\"34.93873,-82.22706\",\"typical\":\"29.4241,-98.4936\"}")); - - cause.setFunction(DetectorFunction.AVG.getFullName()); - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), - false).utf8ToString(); - assertThat(reference, not(containsString("\"geo_results\""))); - - reference = XContentHelper.toXContent(anomalyRecord, - XContentType.JSON, - false).utf8ToString(); - assertThat(reference, not(containsString("\"geo_results\""))); - } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/GeoResultsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/GeoResultsTests.java new file mode 100644 index 0000000000000..f35c2e01c757b --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/results/GeoResultsTests.java @@ -0,0 +1,71 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.results; + +import org.elasticsearch.common.io.stream.Writeable.Reader; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.junit.Before; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; + +public class GeoResultsTests extends AbstractSerializingTestCase { + + private boolean lenient; + + @Before + public void setLenient() { + lenient = randomBoolean(); + } + + static GeoResults createTestGeoResults() { + GeoResults geoResults = new GeoResults(); + if (randomBoolean()) { + geoResults.setActualPoint(randomDoubleBetween(-90.0, 90.0, true) + "," + + randomDoubleBetween(-90.0, 90.0, true)); + } + if (randomBoolean()) { + geoResults.setTypicalPoint(randomDoubleBetween(-90.0, 90.0, true) + "," + + randomDoubleBetween(-90.0, 90.0, true)); + } + return geoResults; + } + + @Override + protected GeoResults createTestInstance() { + return createTestGeoResults(); + } + + @Override + protected Reader instanceReader() { + return GeoResults::new; + } + + @Override + protected GeoResults doParseInstance(XContentParser parser) { + return lenient ? GeoResults.LENIENT_PARSER.apply(parser, null) : GeoResults.STRICT_PARSER.apply(parser, null); + } + + public void testStrictParser() throws IOException { + String json = "{\"foo\":\"bar\"}"; + try (XContentParser parser = createParser(JsonXContent.jsonXContent, json)) { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> GeoResults.STRICT_PARSER.apply(parser, null)); + + assertThat(e.getMessage(), containsString("unknown field [foo]")); + } + } + + public void testLenientParser() throws IOException { + String json = "{\"foo\":\"bar\"}"; + try (XContentParser parser = createParser(JsonXContent.jsonXContent, json)) { + GeoResults.LENIENT_PARSER.apply(parser, null); + } + } +} From 6752b61387a40c09883da2e3a4ae85b227298b9f Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Mon, 11 Nov 2019 08:50:23 -0500 Subject: [PATCH 7/7] fixing toxcontent interactions --- .../org/elasticsearch/common/xcontent/XContentBuilder.java | 2 +- .../xpack/core/ml/job/results/AnomalyRecord.java | 6 +----- .../xpack/ml/job/persistence/JobResultsPersister.java | 7 ++----- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java index ae535449fc3a2..51a4f86a0d3b2 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java @@ -850,7 +850,7 @@ private XContentBuilder value(ToXContent value) throws IOException { return value(value, ToXContent.EMPTY_PARAMS); } - public XContentBuilder value(ToXContent value, ToXContent.Params params) throws IOException { + private XContentBuilder value(ToXContent value, ToXContent.Params params) throws IOException { if (value == null) { return nullValue(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java index 5770ec095f0ba..d50ee9e01d57a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/AnomalyRecord.java @@ -302,11 +302,7 @@ XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws I builder.field(OVER_FIELD_VALUE.getPreferredName(), overFieldValue); } if (causes != null) { - builder.startArray(CAUSES.getPreferredName()); - for (AnomalyCause cause : causes) { - builder.value(cause, params); - } - builder.endArray(); + builder.field(CAUSES.getPreferredName(), causes); } if (influences != null) { builder.field(INFLUENCERS.getPreferredName(), influences); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java index d1c9b06edff8d..783706259a17b 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsPersister.java @@ -69,9 +69,6 @@ public class JobResultsPersister { private static final Logger logger = LogManager.getLogger(JobResultsPersister.class); - private static final ToXContent.Params FOR_INTERNAL_STORAGE = - new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")); - private final Client client; public JobResultsPersister(Client client) { @@ -137,7 +134,7 @@ public Builder persistTimingStats(TimingStats timingStats) { indexResult( TimingStats.documentId(timingStats.getJobId()), timingStats, - FOR_INTERNAL_STORAGE, + new ToXContent.MapParams(Collections.singletonMap(ToXContentParams.FOR_INTERNAL_STORAGE, "true")), TimingStats.TYPE.getPreferredName()); return this; } @@ -151,7 +148,7 @@ public Builder persistTimingStats(TimingStats timingStats) { public Builder persistRecords(List records) { for (AnomalyRecord record : records) { logger.trace("[{}] ES BULK ACTION: index record to index [{}] with ID [{}]", jobId, indexName, record.getId()); - indexResult(record.getId(), record, FOR_INTERNAL_STORAGE, "record"); + indexResult(record.getId(), record, "record"); } return this;