Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Introduce Annotation.event field #57144

Merged
merged 6 commits into from
Jun 1, 2020
Merged

Introduce Annotation.event field #57144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c4512c0
Introduce Annotation.event field
przemekwitek May 26, 2020
c220db1
Use Annotation parser rather than Annotation.Builder throughout the code
przemekwitek May 29, 2020
606b443
Apply review comments
przemekwitek May 28, 2020
b0e372e
Add Event.USER enum value for user-generated annotations
przemekwitek May 28, 2020
c13a5ce
Make Annotation parser strict
przemekwitek Jun 1, 2020
0624aa0
Rename PARSER to STRICT_PARSER
przemekwitek Jun 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 106 additions & 27 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/Annotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,46 @@
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.common.time.TimeUtils;
import org.elasticsearch.xpack.core.ml.job.config.Job;

import java.io.IOException;
import java.util.Date;
import java.util.Locale;
import java.util.Objects;

public class Annotation implements ToXContentObject, Writeable {

public enum Type {
ANNOTATION,
COMMENT;

public static Type fromString(String value) {
return valueOf(value.toUpperCase(Locale.ROOT));
}

@Override
public String toString() {
return name().toLowerCase(Locale.ROOT);
}
}

public enum Event {
USER,
DELAYED_DATA,
MODEL_SNAPSHOT_STORED;
przemekwitek marked this conversation as resolved.
Show resolved Hide resolved

public static Event fromString(String value) {
return valueOf(value.toUpperCase(Locale.ROOT));
}

@Override
public String toString() {
return name().toLowerCase(Locale.ROOT);
}
}

public static final ParseField ANNOTATION = new ParseField("annotation");
public static final ParseField CREATE_TIME = new ParseField("create_time");
public static final ParseField CREATE_USERNAME = new ParseField("create_username");
Expand All @@ -31,6 +62,7 @@ public class Annotation implements ToXContentObject, Writeable {
public static final ParseField MODIFIED_TIME = new ParseField("modified_time");
public static final ParseField MODIFIED_USERNAME = new ParseField("modified_username");
public static final ParseField TYPE = new ParseField("type");
public static final ParseField EVENT = new ParseField("event");
public static final ParseField DETECTOR_INDEX = new ParseField("detector_index");
public static final ParseField PARTITION_FIELD_NAME = new ParseField("partition_field_name");
public static final ParseField PARTITION_FIELD_VALUE = new ParseField("partition_field_value");
Expand All @@ -39,29 +71,50 @@ public class Annotation implements ToXContentObject, Writeable {
public static final ParseField BY_FIELD_NAME = new ParseField("by_field_name");
public static final ParseField BY_FIELD_VALUE = new ParseField("by_field_value");

public static final ObjectParser<Builder, Void> PARSER = new ObjectParser<>(TYPE.getPreferredName(), true, Builder::new);
/**
* Parses {@link Annotation} using a strict parser.
*/
public static Annotation fromXContent(XContentParser parser, Void context) {
return STRICT_PARSER.apply(parser, context).build();
}

/**
* Strict parser for cases when {@link Annotation} is returned from C++ as an ML result.
*/
private static final ObjectParser<Builder, Void> STRICT_PARSER = new ObjectParser<>(ANNOTATION.getPreferredName(), false, Builder::new);

static {
PARSER.declareString(Builder::setAnnotation, ANNOTATION);
PARSER.declareField(Builder::setCreateTime,
STRICT_PARSER.declareString(Builder::setAnnotation, ANNOTATION);
STRICT_PARSER.declareField(Builder::setCreateTime,
p -> TimeUtils.parseTimeField(p, CREATE_TIME.getPreferredName()), CREATE_TIME, ObjectParser.ValueType.VALUE);
PARSER.declareString(Builder::setCreateUsername, CREATE_USERNAME);
PARSER.declareField(Builder::setTimestamp,
STRICT_PARSER.declareString(Builder::setCreateUsername, CREATE_USERNAME);
STRICT_PARSER.declareField(Builder::setTimestamp,
p -> TimeUtils.parseTimeField(p, TIMESTAMP.getPreferredName()), TIMESTAMP, ObjectParser.ValueType.VALUE);
PARSER.declareField(Builder::setEndTimestamp,
STRICT_PARSER.declareField(Builder::setEndTimestamp,
p -> TimeUtils.parseTimeField(p, END_TIMESTAMP.getPreferredName()), END_TIMESTAMP, ObjectParser.ValueType.VALUE);
PARSER.declareString(Builder::setJobId, Job.ID);
PARSER.declareField(Builder::setModifiedTime,
STRICT_PARSER.declareString(Builder::setJobId, Job.ID);
STRICT_PARSER.declareField(Builder::setModifiedTime,
p -> TimeUtils.parseTimeField(p, MODIFIED_TIME.getPreferredName()), MODIFIED_TIME, ObjectParser.ValueType.VALUE);
PARSER.declareString(Builder::setModifiedUsername, MODIFIED_USERNAME);
PARSER.declareString(Builder::setType, TYPE);
PARSER.declareInt(Builder::setDetectorIndex, DETECTOR_INDEX);
PARSER.declareString(Builder::setPartitionFieldName, PARTITION_FIELD_NAME);
PARSER.declareString(Builder::setPartitionFieldValue, PARTITION_FIELD_VALUE);
PARSER.declareString(Builder::setOverFieldName, OVER_FIELD_NAME);
PARSER.declareString(Builder::setOverFieldValue, OVER_FIELD_VALUE);
PARSER.declareString(Builder::setByFieldName, BY_FIELD_NAME);
PARSER.declareString(Builder::setByFieldValue, BY_FIELD_VALUE);
STRICT_PARSER.declareString(Builder::setModifiedUsername, MODIFIED_USERNAME);
STRICT_PARSER.declareField(Builder::setType, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return Type.fromString(p.text());
}
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
}, TYPE, ObjectParser.ValueType.STRING);
STRICT_PARSER.declareField(Builder::setEvent, p -> {
if (p.currentToken() == XContentParser.Token.VALUE_STRING) {
return Event.fromString(p.text());
przemekwitek marked this conversation as resolved.
Show resolved Hide resolved
}
throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]");
}, EVENT, ObjectParser.ValueType.STRING);
STRICT_PARSER.declareInt(Builder::setDetectorIndex, DETECTOR_INDEX);
STRICT_PARSER.declareString(Builder::setPartitionFieldName, PARTITION_FIELD_NAME);
STRICT_PARSER.declareString(Builder::setPartitionFieldValue, PARTITION_FIELD_VALUE);
STRICT_PARSER.declareString(Builder::setOverFieldName, OVER_FIELD_NAME);
STRICT_PARSER.declareString(Builder::setOverFieldValue, OVER_FIELD_VALUE);
STRICT_PARSER.declareString(Builder::setByFieldName, BY_FIELD_NAME);
STRICT_PARSER.declareString(Builder::setByFieldValue, BY_FIELD_VALUE);
}

private final String annotation;
Expand All @@ -75,7 +128,8 @@ public class Annotation implements ToXContentObject, Writeable {
private final String jobId;
private final Date modifiedTime;
private final String modifiedUsername;
private final String type;
private final Type type;
private final Event event;
/**
* Scope-related fields.
*/
Expand All @@ -88,8 +142,9 @@ public class Annotation implements ToXContentObject, Writeable {
private final String byFieldValue;

private Annotation(String annotation, Date createTime, String createUsername, Date timestamp, Date endTimestamp, String jobId,
Date modifiedTime, String modifiedUsername, String type, Integer detectorIndex, String partitionFieldName,
String partitionFieldValue, String overFieldName, String overFieldValue, String byFieldName, String byFieldValue) {
Date modifiedTime, String modifiedUsername, Type type, Event event, Integer detectorIndex,
String partitionFieldName, String partitionFieldValue, String overFieldName, String overFieldValue,
String byFieldName, String byFieldValue) {
this.annotation = Objects.requireNonNull(annotation);
this.createTime = Objects.requireNonNull(createTime);
this.createUsername = Objects.requireNonNull(createUsername);
Expand All @@ -99,6 +154,7 @@ private Annotation(String annotation, Date createTime, String createUsername, Da
this.modifiedTime = modifiedTime;
this.modifiedUsername = modifiedUsername;
this.type = Objects.requireNonNull(type);
this.event = event;
this.detectorIndex = detectorIndex;
this.partitionFieldName = partitionFieldName;
this.partitionFieldValue = partitionFieldValue;
Expand All @@ -125,8 +181,9 @@ public Annotation(StreamInput in) throws IOException {
modifiedTime = null;
}
modifiedUsername = in.readOptionalString();
type = in.readString();
type = Type.fromString(in.readString());
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
event = in.readBoolean() ? in.readEnum(Event.class) : null;
detectorIndex = in.readOptionalInt();
partitionFieldName = in.readOptionalString();
partitionFieldValue = in.readOptionalString();
Expand All @@ -135,6 +192,7 @@ public Annotation(StreamInput in) throws IOException {
byFieldName = in.readOptionalString();
byFieldValue = in.readOptionalString();
} else {
event = null;
detectorIndex = null;
partitionFieldName = null;
partitionFieldValue = null;
Expand Down Expand Up @@ -165,8 +223,14 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(false);
}
out.writeOptionalString(modifiedUsername);
out.writeString(type);
out.writeString(type.toString());
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
if (event != null) {
out.writeBoolean(true);
out.writeEnum(event);
} else {
out.writeBoolean(false);
}
out.writeOptionalInt(detectorIndex);
out.writeOptionalString(partitionFieldName);
out.writeOptionalString(partitionFieldValue);
Expand Down Expand Up @@ -209,10 +273,14 @@ public String getModifiedUsername() {
return modifiedUsername;
}

public String getType() {
public Type getType() {
return type;
}

public Event getEvent() {
return event;
}

public Integer getDetectorIndex() {
return detectorIndex;
}
Expand Down Expand Up @@ -261,6 +329,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(MODIFIED_USERNAME.getPreferredName(), modifiedUsername);
}
builder.field(TYPE.getPreferredName(), type);
if (event != null) {
builder.field(EVENT.getPreferredName(), event);
}
if (detectorIndex != null) {
builder.field(DETECTOR_INDEX.getPreferredName(), detectorIndex);
}
Expand Down Expand Up @@ -289,7 +360,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
@Override
public int hashCode() {
return Objects.hash(
annotation, createTime, createUsername, timestamp, endTimestamp, jobId, modifiedTime, modifiedUsername, type,
annotation, createTime, createUsername, timestamp, endTimestamp, jobId, modifiedTime, modifiedUsername, type, event,
detectorIndex, partitionFieldName, partitionFieldValue, overFieldName, overFieldValue, byFieldName, byFieldValue);
}

Expand All @@ -311,6 +382,7 @@ public boolean equals(Object obj) {
Objects.equals(modifiedTime, other.modifiedTime) &&
Objects.equals(modifiedUsername, other.modifiedUsername) &&
Objects.equals(type, other.type) &&
Objects.equals(event, other.event) &&
Objects.equals(detectorIndex, other.detectorIndex) &&
Objects.equals(partitionFieldName, other.partitionFieldName) &&
Objects.equals(partitionFieldValue, other.partitionFieldValue) &&
Expand Down Expand Up @@ -338,7 +410,8 @@ public static class Builder {
private String jobId;
private Date modifiedTime;
private String modifiedUsername;
private String type;
private Type type;
private Event event;
private Integer detectorIndex;
private String partitionFieldName;
private String partitionFieldValue;
Expand All @@ -360,6 +433,7 @@ public Builder(Annotation other) {
this.modifiedTime = other.modifiedTime == null ? null : new Date(other.modifiedTime.getTime());
this.modifiedUsername = other.modifiedUsername;
this.type = other.type;
this.event = other.event;
this.detectorIndex = other.detectorIndex;
this.partitionFieldName = other.partitionFieldName;
this.partitionFieldValue = other.partitionFieldValue;
Expand Down Expand Up @@ -409,11 +483,16 @@ public Builder setModifiedUsername(String modifiedUsername) {
return this;
}

public Builder setType(String type) {
public Builder setType(Type type) {
this.type = Objects.requireNonNull(type);
return this;
}

public Builder setEvent(Event event) {
this.event = event;
return this;
}

public Builder setDetectorIndex(Integer index) {
this.detectorIndex = index;
return this;
Expand Down Expand Up @@ -451,7 +530,7 @@ public Builder setByFieldValue(String value) {

public Annotation build() {
return new Annotation(
annotation, createTime, createUsername, timestamp, endTimestamp, jobId, modifiedTime, modifiedUsername, type,
annotation, createTime, createUsername, timestamp, endTimestamp, jobId, modifiedTime, modifiedUsername, type, event,
detectorIndex, partitionFieldName, partitionFieldValue, overFieldName, overFieldValue, byFieldName, byFieldValue);
}
}
Expand Down
3 changes: 3 additions & 0 deletions ...n/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
"type" : {
"type" : "keyword"
},
"event" : {
"type" : "keyword"
},
"detector_index" : {
"type" : "integer"
},
Expand Down
5 changes: 3 additions & 2 deletions ...lugin/core/src/test/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class AnnotationTests extends AbstractSerializingTestCase<Annotation> {

@Override
protected Annotation doParseInstance(XContentParser parser) {
return Annotation.PARSER.apply(parser, null).build();
return Annotation.fromXContent(parser, null);
}

@Override
Expand All @@ -35,7 +35,8 @@ public static Annotation randomAnnotation(String jobId) {
.setJobId(jobId)
.setModifiedTime(randomBoolean() ? new Date(randomNonNegativeLong()) : null)
.setModifiedUsername(randomBoolean() ? randomAlphaOfLengthBetween(5, 20) : null)
.setType(randomAlphaOfLengthBetween(10, 15))
.setType(randomFrom(Annotation.Type.values()))
.setEvent(randomBoolean() ? randomFrom(Annotation.Event.values()) : null)
.setDetectorIndex(randomBoolean() ? randomIntBetween(0, 10) : null)
.setPartitionFieldName(randomBoolean() ? randomAlphaOfLengthBetween(5, 20) : null)
.setPartitionFieldValue(randomBoolean() ? randomAlphaOfLengthBetween(5, 20) : null)
Expand Down
2 changes: 1 addition & 1 deletion ...lClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ private List<Annotation> getAnnotations() throws Exception {

private Annotation parseAnnotation(BytesReference source) throws IOException {
try (XContentParser parser = createParser(jsonXContent, source)) {
return Annotation.PARSER.parse(parser, null).build();
return Annotation.fromXContent(parser, null);
}
}

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ private Annotation createDelayedDataAnnotation(Date startTime, Date endTime, Str
.setJobId(jobId)
.setModifiedTime(currentTime)
.setModifiedUsername(XPackUser.NAME)
.setType("annotation")
.setType(Annotation.Type.ANNOTATION)
.setEvent(Annotation.Event.DELAYED_DATA)
.build();
}

Expand Down
3 changes: 2 additions & 1 deletion ...a/org/elasticsearch/xpack/ml/job/process/autodetect/output/AutodetectResultProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,8 @@ private Annotation createModelSnapshotAnnotation(ModelSnapshot modelSnapshot) {
.setJobId(jobId)
.setModifiedTime(currentTime)
.setModifiedUsername(XPackUser.NAME)
.setType("annotation")
.setType(Annotation.Type.ANNOTATION)
.setEvent(Annotation.Event.MODEL_SNAPSHOT_STORED)
.build();
}

Expand Down
2 changes: 1 addition & 1 deletion ...gin/ml/src/test/java/org/elasticsearch/xpack/ml/annotations/AnnotationPersisterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ private static BulkItemResponse bulkItemFailure(String docId) {

private Annotation parseAnnotation(BytesReference source) throws IOException {
try (XContentParser parser = createParser(jsonXContent, source)) {
return Annotation.PARSER.parse(parser, null).build();
return Annotation.fromXContent(parser, null);
}
}
}
6 changes: 4 additions & 2 deletions x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJobTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@ public void testRealtimeRun() throws Exception {
.setJobId(jobId)
.setModifiedTime(new Date(annotationCreateTime))
.setModifiedUsername(XPackUser.NAME)
.setType("annotation")
.setType(Annotation.Type.ANNOTATION)
.setEvent(Annotation.Event.DELAYED_DATA)
.build();
BytesReference expectedSource =
BytesReference.bytes(expectedAnnotation.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS));
Expand Down Expand Up @@ -342,7 +343,8 @@ public void testRealtimeRun() throws Exception {
.setJobId(jobId)
.setModifiedTime(new Date(annotationUpdateTime))
.setModifiedUsername(XPackUser.NAME)
.setType("annotation")
.setType(Annotation.Type.ANNOTATION)
.setEvent(Annotation.Event.DELAYED_DATA)
.build();
BytesReference expectedSource =
BytesReference.bytes(expectedUpdatedAnnotation.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS));
Expand Down
3 changes: 2 additions & 1 deletion .../elasticsearch/xpack/ml/job/process/autodetect/output/AutodetectResultProcessorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,8 @@ public void testProcessResult_modelSnapshot() {
.setJobId(JOB_ID)
.setModifiedTime(Date.from(CURRENT_TIME))
.setModifiedUsername(XPackUser.NAME)
.setType("annotation")
.setType(Annotation.Type.ANNOTATION)
.setEvent(Annotation.Event.MODEL_SNAPSHOT_STORED)
.build();
assertThat(annotation, is(equalTo(expectedAnnotation)));

Expand Down