Skip to content

Commit

Permalink
Introduce ModelPlotConfig.annotationsEnabled setting
Browse files Browse the repository at this point in the history
  • Loading branch information
przemekwitek committed Jun 3, 2020
1 parent 5b45400 commit 36c9cf4
Show file tree
Hide file tree
Showing 21 changed files with 115 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,27 @@ public class ModelPlotConfig implements ToXContentObject {

private static final ParseField TYPE_FIELD = new ParseField("model_plot_config");
private static final ParseField ENABLED_FIELD = new ParseField("enabled");
public static final ParseField TERMS_FIELD = new ParseField("terms");
private static final ParseField TERMS_FIELD = new ParseField("terms");
private static final ParseField ANNOTATIONS_ENABLED_FIELD = new ParseField("annotations_enabled");

public static final ConstructingObjectParser<ModelPlotConfig, Void> PARSER =
new ConstructingObjectParser<>(TYPE_FIELD.getPreferredName(), true, a -> new ModelPlotConfig((boolean) a[0], (String) a[1]));
new ConstructingObjectParser<>(
TYPE_FIELD.getPreferredName(), true, a -> new ModelPlotConfig((boolean) a[0], (String) a[1], (Boolean) a[2]));

static {
PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED_FIELD);
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), TERMS_FIELD);
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), ANNOTATIONS_ENABLED_FIELD);
}

private final boolean enabled;
private final String terms;
private final boolean annotationsEnabled;

public ModelPlotConfig(boolean enabled, String terms) {
public ModelPlotConfig(boolean enabled, String terms, Boolean annotationsEnabled) {
this.enabled = enabled;
this.terms = terms;
this.annotationsEnabled = Boolean.TRUE.equals(annotationsEnabled);
}

@Override
Expand All @@ -55,6 +60,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (terms != null) {
builder.field(TERMS_FIELD.getPreferredName(), terms);
}
builder.field(ANNOTATIONS_ENABLED_FIELD.getPreferredName(), annotationsEnabled);
builder.endObject();
return builder;
}
Expand All @@ -67,6 +73,10 @@ public String getTerms() {
return this.terms;
}

public boolean annotationsEnabled() {
return annotationsEnabled;
}

@Override
public boolean equals(Object other) {
if (this == other) {
Expand All @@ -78,11 +88,11 @@ public boolean equals(Object other) {
}

ModelPlotConfig that = (ModelPlotConfig) other;
return this.enabled == that.enabled && Objects.equals(this.terms, that.terms);
return this.enabled == that.enabled && Objects.equals(this.terms, that.terms) && this.annotationsEnabled == that.annotationsEnabled;
}

@Override
public int hashCode() {
return Objects.hash(enabled, terms);
return Objects.hash(enabled, terms, annotationsEnabled);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ public void testUpdateJob() throws Exception {
.setDetectorUpdates(Arrays.asList(detectorUpdate)) // <6>
.setGroups(Arrays.asList("job-group-1")) // <7>
.setResultsRetentionDays(10L) // <8>
.setModelPlotConfig(new ModelPlotConfig(true, null)) // <9>
.setModelPlotConfig(new ModelPlotConfig(true, null, true)) // <9>
.setModelSnapshotRetentionDays(7L) // <10>
.setCustomSettings(customSettings) // <11>
.setRenormalizationWindowDays(3L) // <12>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static Job.Builder createRandomizedJobBuilder() {
builder.setDataDescription(dataDescription);

if (randomBoolean()) {
builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)));
builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized());
}
if (randomBoolean()) {
builder.setRenormalizationWindowDays(randomNonNegativeLong());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static JobUpdate createRandom(String jobId) {
update.setDetectorUpdates(createRandomDetectorUpdates());
}
if (randomBoolean()) {
update.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)));
update.setModelPlotConfig(ModelPlotConfigTests.createRandomized());
}
if (randomBoolean()) {
update.setAnalysisLimits(AnalysisLimitsTests.createRandomized());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ public class ModelPlotConfigTests extends AbstractXContentTestCase<ModelPlotConf

@Override
protected ModelPlotConfig createTestInstance() {
return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30));
return createRandomized();
}

public static ModelPlotConfig createRandomized() {
return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean());
}

@Override
Expand Down
4 changes: 4 additions & 0 deletions docs/reference/ml/anomaly-detection/apis/put-job.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-enabled]
`terms`:::
experimental[] (string)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-terms]
`annotations_enabled`:::
(boolean)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-annotations-enabled]
====
//End model_plot_config

Expand Down
4 changes: 4 additions & 0 deletions docs/reference/ml/anomaly-detection/apis/update-job.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-enabled]
`terms`:::
experimental[] (string)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-terms]
`annotations_enabled`:::
(boolean)
include::{es-repo-dir}/ml/ml-shared.asciidoc[tag=model-plot-config-annotations-enabled]
====
//End model_plot_config

Expand Down
5 changes: 5 additions & 0 deletions docs/reference/ml/ml-shared.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,11 @@ applied. For example, "CPU,NetworkIn,DiskWrites". Wildcards are not supported.
Only the specified `terms` can be viewed when using the Single Metric Viewer.
end::model-plot-config-terms[]

tag::model-plot-config-annotations-enabled[]
If true, enables calculation and storage of the model change annotations
for each entity that is being analyzed. By default, this is not enabled.
end::model-plot-config-annotations-enabled[]

tag::model-snapshot-id[]
A numerical character string that uniquely identifies the model snapshot. For
example, `1575402236000 `.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.core.ml.job.config;

import org.elasticsearch.Version;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand All @@ -21,42 +22,54 @@ public class ModelPlotConfig implements ToXContentObject, Writeable {
public static final ParseField TYPE_FIELD = new ParseField("model_plot_config");
public static final ParseField ENABLED_FIELD = new ParseField("enabled");
public static final ParseField TERMS_FIELD = new ParseField("terms");
public static final ParseField ANNOTATIONS_ENABLED_FIELD = new ParseField("annotations_enabled");

// These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly
public static final ConstructingObjectParser<ModelPlotConfig, Void> LENIENT_PARSER = createParser(true);
public static final ConstructingObjectParser<ModelPlotConfig, Void> STRICT_PARSER = createParser(false);

private static ConstructingObjectParser<ModelPlotConfig, Void> createParser(boolean ignoreUnknownFields) {
ConstructingObjectParser<ModelPlotConfig, Void> parser = new ConstructingObjectParser<>(TYPE_FIELD.getPreferredName(),
ignoreUnknownFields, a -> new ModelPlotConfig((boolean) a[0], (String) a[1]));
ignoreUnknownFields, a -> new ModelPlotConfig((boolean) a[0], (String) a[1], (Boolean) a[2]));

parser.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED_FIELD);
parser.declareString(ConstructingObjectParser.optionalConstructorArg(), TERMS_FIELD);
parser.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), ANNOTATIONS_ENABLED_FIELD);

return parser;
}

private final boolean enabled;
private final String terms;
private final boolean annotationsEnabled;

public ModelPlotConfig() {
this(true, null);
this(true, null, true);
}

public ModelPlotConfig(boolean enabled, String terms) {
public ModelPlotConfig(boolean enabled, String terms, Boolean annotationsEnabled) {
this.enabled = enabled;
this.terms = terms;
this.annotationsEnabled = Boolean.TRUE.equals(annotationsEnabled);
}

public ModelPlotConfig(StreamInput in) throws IOException {
enabled = in.readBoolean();
terms = in.readOptionalString();
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
annotationsEnabled = in.readBoolean();
} else {
annotationsEnabled = enabled;
}
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(enabled);
out.writeOptionalString(terms);
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
out.writeBoolean(annotationsEnabled);
}
}

@Override
Expand All @@ -66,6 +79,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (terms != null) {
builder.field(TERMS_FIELD.getPreferredName(), terms);
}
builder.field(ANNOTATIONS_ENABLED_FIELD.getPreferredName(), annotationsEnabled);
builder.endObject();
return builder;
}
Expand All @@ -75,7 +89,11 @@ public boolean isEnabled() {
}

public String getTerms() {
return this.terms;
return terms;
}

public boolean annotationsEnabled() {
return annotationsEnabled;
}

@Override
Expand All @@ -89,11 +107,11 @@ public boolean equals(Object other) {
}

ModelPlotConfig that = (ModelPlotConfig) other;
return this.enabled == that.enabled && Objects.equals(this.terms, that.terms);
return this.enabled == that.enabled && Objects.equals(this.terms, that.terms) && this.annotationsEnabled == that.annotationsEnabled;
}

@Override
public int hashCode() {
return Objects.hash(enabled, terms);
return Objects.hash(enabled, terms, annotationsEnabled);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ public final class ReservedFieldNames {

ModelPlotConfig.ENABLED_FIELD.getPreferredName(),
ModelPlotConfig.TERMS_FIELD.getPreferredName(),
ModelPlotConfig.ANNOTATIONS_ENABLED_FIELD.getPreferredName(),

DatafeedConfig.ID.getPreferredName(),
DatafeedConfig.QUERY_DELAY.getPreferredName(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@
},
"terms" : {
"type" : "keyword"
},
"annotations_enabled" : {
"type" : "boolean"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests;
import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfig;
import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfigTests;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -21,7 +22,7 @@ public class UpdateProcessActionRequestTests extends AbstractWireSerializingTest
protected UpdateProcessAction.Request createTestInstance() {
ModelPlotConfig config = null;
if (randomBoolean()) {
config = new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10));
config = ModelPlotConfigTests.createRandomized();
}
List<JobUpdate.DetectorUpdate> updates = null;
if (randomBoolean()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ public static Job createRandomizedJob() {
builder.setDataDescription(dataDescription);

if (randomBoolean()) {
builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)));
builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized());
}
if (randomBoolean()) {
builder.setRenormalizationWindowDays(randomNonNegativeLong());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public JobUpdate createRandom(String jobId, @Nullable Job job) {
update.setDetectorUpdates(detectorUpdates);
}
if (randomBoolean()) {
update.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)));
update.setModelPlotConfig(ModelPlotConfigTests.createRandomized());
}
if (randomBoolean()) {
update.setAnalysisLimits(AnalysisLimits.validateAndSetDefaults(AnalysisLimitsTests.createRandomized(), null,
Expand Down Expand Up @@ -222,7 +222,7 @@ public void testMergeWithJob() {
new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))).build());
detectorUpdates.add(new JobUpdate.DetectorUpdate(1, "description-2", detectionRules2));

ModelPlotConfig modelPlotConfig = new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10));
ModelPlotConfig modelPlotConfig = ModelPlotConfigTests.createRandomized();
AnalysisLimits analysisLimits = new AnalysisLimits(randomNonNegativeLong(), randomNonNegativeLong());
List<String> categorizationFilters = Arrays.asList(generateRandomStringArray(10, 10, false));
Map<String, Object> customSettings = Collections.singletonMap(randomAlphaOfLength(10), randomAlphaOfLength(10));
Expand Down Expand Up @@ -300,7 +300,7 @@ public void testMergeWithJob_GivenRandomUpdates_AssertImmutability() {
public void testIsAutodetectProcessUpdate() {
JobUpdate update = new JobUpdate.Builder("foo").build();
assertFalse(update.isAutodetectProcessUpdate());
update = new JobUpdate.Builder("foo").setModelPlotConfig(new ModelPlotConfig(true, "ff")).build();
update = new JobUpdate.Builder("foo").setModelPlotConfig(new ModelPlotConfig(true, "ff", false)).build();
assertTrue(update.isAutodetectProcessUpdate());
update = new JobUpdate.Builder("foo").setDetectorUpdates(Collections.singletonList(mock(JobUpdate.DetectorUpdate.class))).build();
assertTrue(update.isAutodetectProcessUpdate());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@
public class ModelPlotConfigTests extends AbstractSerializingTestCase<ModelPlotConfig> {

public void testConstructorDefaults() {
assertThat(new ModelPlotConfig().isEnabled(), is(true));
assertThat(new ModelPlotConfig().getTerms(), is(nullValue()));
ModelPlotConfig modelPlotConfig = new ModelPlotConfig();
assertThat(modelPlotConfig.isEnabled(), is(true));
assertThat(modelPlotConfig.getTerms(), is(nullValue()));
assertThat(modelPlotConfig.annotationsEnabled(), is(true));
}

@Override
protected ModelPlotConfig createTestInstance() {
return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30));
return createRandomized();
}

public static ModelPlotConfig createRandomized() {
return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void testPartitionFieldWithoutTerms() throws Exception {

public void testPartitionFieldWithTerms() throws Exception {
Job.Builder job = jobWithPartitionUser("model-plots-it-test-partition-field-with-terms");
job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3"));
job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3", false));
registerJob(job);
putJob(job);
String datafeedId = job.getId() + "-feed";
Expand All @@ -116,7 +116,7 @@ public void testPartitionFieldWithTerms() throws Exception {

public void testByFieldWithTerms() throws Exception {
Job.Builder job = jobWithByUser("model-plots-it-test-by-field-with-terms");
job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3"));
job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3", false));
registerJob(job);
putJob(job);
String datafeedId = job.getId() + "-feed";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public void write() throws IOException {
.append(terms == null ? "" : terms)
.append(NEW_LINE);

contents.append(ModelPlotConfig.ANNOTATIONS_ENABLED_FIELD.getPreferredName())
.append(EQUALS)
.append(modelPlotConfig.annotationsEnabled())
.append(NEW_LINE);

writer.write(contents.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import org.elasticsearch.xpack.core.ml.job.config.AnalysisLimitsTests;
import org.elasticsearch.xpack.core.ml.job.config.DataDescription;
import org.elasticsearch.xpack.core.ml.job.config.Job;
import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfig;
import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfigTests;

import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -48,8 +48,7 @@ protected Job.Builder createTestInstance() {
builder.setDataDescription(dataDescription);
}
if (randomBoolean()) {
builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(),
randomAlphaOfLength(10)));
builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized());
}
if (randomBoolean()) {
builder.setRenormalizationWindowDays(randomNonNegativeLong());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,13 @@ public void testWriteResetBucketsMessage() throws IOException {
public void testWriteUpdateModelPlotMessage() throws IOException {
AutodetectControlMsgWriter writer = new AutodetectControlMsgWriter(lengthEncodedWriter, 4);

writer.writeUpdateModelPlotMessage(new ModelPlotConfig(true, "foo,bar"));
writer.writeUpdateModelPlotMessage(new ModelPlotConfig(true, "foo,bar", false));

InOrder inOrder = inOrder(lengthEncodedWriter);
inOrder.verify(lengthEncodedWriter).writeNumFields(4);
inOrder.verify(lengthEncodedWriter, times(3)).writeField("");
inOrder.verify(lengthEncodedWriter).writeField("u[modelPlotConfig]\nboundspercentile = 95.0\nterms = foo,bar\n");
inOrder.verify(lengthEncodedWriter)
.writeField("u[modelPlotConfig]\nboundspercentile = 95.0\nterms = foo,bar\nannotations_enabled = false\n");
verifyNoMoreInteractions(lengthEncodedWriter);
}

Expand Down
Loading

0 comments on commit 36c9cf4

Please sign in to comment.