Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@kaituo
kaituo committed Oct 12, 2020
1 parent 6c92141 commit cafebc2
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 60 deletions.
72 changes: 34 additions & 38 deletions src/main/java/com/amazon/opendistroforelasticsearch/ad/model/AnomalyDetector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public AnomalyDetector(
this.filterQuery = filterQuery;
this.detectionInterval = detectionInterval;
this.windowDelay = windowDelay;
this.shingleSize = shingleSize;
this.shingleSize = getShingleSize(shingleSize, categoryField);
this.uiMetadata = uiMetadata;
this.schemaVersion = schemaVersion;
this.lastUpdateTime = lastUpdateTime;
Expand All @@ -181,36 +181,23 @@ public AnomalyDetector(
Integer schemaVersion,
Instant lastUpdateTime
) {
if (Strings.isBlank(name)) {
throw new IllegalArgumentException("Detector name should be set");
}
if (timeField == null) {
throw new IllegalArgumentException("Time field should be set");
}
if (indices == null || indices.isEmpty()) {
throw new IllegalArgumentException("Indices should be set");
}
if (detectionInterval == null) {
throw new IllegalArgumentException("Detection interval should be set");
}
if (shingleSize != null && shingleSize < 1) {
throw new IllegalArgumentException("Shingle size must be a positive integer");
}
this.detectorId = detectorId;
this.version = version;
this.name = name;
this.description = description;
this.timeField = timeField;
this.indices = indices;
this.featureAttributes = features;
this.filterQuery = filterQuery;
this.detectionInterval = detectionInterval;
this.windowDelay = windowDelay;
this.shingleSize = shingleSize;
this.uiMetadata = uiMetadata;
this.schemaVersion = schemaVersion;
this.lastUpdateTime = lastUpdateTime;
this.categoryField = null;
this(
detectorId,
version,
name,
description,
timeField,
indices,
features,
filterQuery,
detectionInterval,
windowDelay,
shingleSize,
uiMetadata,
schemaVersion,
lastUpdateTime,
null
);
}

public XContentBuilder toXContent(XContentBuilder builder) throws IOException {
Expand Down Expand Up @@ -272,7 +259,7 @@ public static AnomalyDetector parse(XContentParser parser, String detectorId) th
* @throws IOException IOException if content can't be parsed correctly
*/
public static AnomalyDetector parse(XContentParser parser, String detectorId, Long version) throws IOException {
return parse(parser, detectorId, version, null, null, null);
return parse(parser, detectorId, version, null, null);
}

/**
Expand All @@ -283,7 +270,6 @@ public static AnomalyDetector parse(XContentParser parser, String detectorId, Lo
* @param version detector document version
* @param defaultDetectionInterval default detection interval
* @param defaultDetectionWindowDelay default detection window delay
* @param defaultShingleSize default number of intervals in shingle
* @return anomaly detector instance
* @throws IOException IOException if content can't be parsed correctly
*/
Expand All @@ -292,8 +278,7 @@ public static AnomalyDetector parse(
String detectorId,
Long version,
TimeValue defaultDetectionInterval,
TimeValue defaultDetectionWindowDelay,
Integer defaultShingleSize
TimeValue defaultDetectionWindowDelay
) throws IOException {
String name = null;
String description = null;
Expand All @@ -306,7 +291,7 @@ public static AnomalyDetector parse(
TimeConfiguration windowDelay = defaultDetectionWindowDelay == null
? null
: new IntervalTimeConfiguration(defaultDetectionWindowDelay.getSeconds(), ChronoUnit.SECONDS);
Integer shingleSize = defaultShingleSize;
Integer shingleSize = null;
List<Feature> features = new ArrayList<>();
int schemaVersion = 0;
Map<String, Object> uiMetadata = null;
Expand Down Expand Up @@ -497,9 +482,20 @@ public TimeConfiguration getWindowDelay() {
}

public Integer getShingleSize() {
return shingleSize == null
return shingleSize;
}

/**
* If the given shingle size is null, return default based on the kind of detector;
* otherwise, return the given shingle size.
* @param customShingleSize Given shingle size
* @param categoryField Used to verify if this is a multi-entity or single-entity detector
* @return Shingle size
*/
private static Integer getShingleSize(Integer customShingleSize, List<String> categoryField) {
return customShingleSize == null
? (categoryField != null && categoryField.size() > 0 ? DEFAULT_MULTI_ENTITY_SHINGLE : DEFAULT_SHINGLE_SIZE)
: shingleSize;
: customShingleSize;
}

public Map<String, Object> getUiMetadata() {
Expand Down
24 changes: 13 additions & 11 deletions src/main/java/com/amazon/opendistroforelasticsearch/ad/model/AnomalyResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,19 @@ public AnomalyResult(
Instant executionEndTime,
String error
) {
this.detectorId = detectorId;
this.anomalyScore = anomalyScore;
this.anomalyGrade = anomalyGrade;
this.confidence = confidence;
this.featureData = featureData;
this.dataStartTime = dataStartTime;
this.dataEndTime = dataEndTime;
this.executionStartTime = executionStartTime;
this.executionEndTime = executionEndTime;
this.error = error;
this.entity = null;
this(
detectorId,
anomalyScore,
anomalyGrade,
confidence,
featureData,
dataStartTime,
dataEndTime,
executionStartTime,
executionEndTime,
error,
null
);
}

public AnomalyResult(
Expand Down
4 changes: 1 addition & 3 deletions ...in/java/com/amazon/opendistroforelasticsearch/ad/rest/RestIndexAnomalyDetectorAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package com.amazon.opendistroforelasticsearch.ad.rest;

import static com.amazon.opendistroforelasticsearch.ad.settings.AnomalyDetectorSettings.DEFAULT_SHINGLE_SIZE;
import static com.amazon.opendistroforelasticsearch.ad.settings.AnomalyDetectorSettings.DETECTION_INTERVAL;
import static com.amazon.opendistroforelasticsearch.ad.settings.AnomalyDetectorSettings.DETECTION_WINDOW_DELAY;
import static com.amazon.opendistroforelasticsearch.ad.settings.AnomalyDetectorSettings.MAX_ANOMALY_DETECTORS;
Expand Down Expand Up @@ -107,8 +106,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
XContentParser parser = request.contentParser();
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser::getTokenLocation);
// TODO: check detection interval < modelTTL
AnomalyDetector detector = AnomalyDetector
.parse(parser, detectorId, null, detectionInterval, detectionWindowDelay, DEFAULT_SHINGLE_SIZE);
AnomalyDetector detector = AnomalyDetector.parse(parser, detectorId, null, detectionInterval, detectionWindowDelay);

long seqNo = request.paramAsLong(IF_SEQ_NO, SequenceNumbers.UNASSIGNED_SEQ_NO);
long primaryTerm = request.paramAsLong(IF_PRIMARY_TERM, SequenceNumbers.UNASSIGNED_PRIMARY_TERM);
Expand Down
14 changes: 8 additions & 6 deletions .../java/com/amazon/opendistroforelasticsearch/ad/transport/ADResultBulkTransportAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,14 @@ protected void doExecute(Task task, ADResultBulkRequest request, ActionListener<
}
}

client
.execute(
BulkAction.INSTANCE,
bulkRequest,
ActionListener.<BulkResponse>wrap(response -> listener.onResponse(response), listener::onFailure)
);
if (bulkRequest.numberOfActions() > 0) {
client
.execute(
BulkAction.INSTANCE,
bulkRequest,
ActionListener.<BulkResponse>wrap(response -> listener.onResponse(response), listener::onFailure)
);
}
}

private void addResult(BulkRequest bulkRequest, AnomalyResult result) {
Expand Down
3 changes: 1 addition & 2 deletions src/test/java/com/amazon/opendistroforelasticsearch/ad/model/AnomalyDetectorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ public void testParseAnomalyDetectorWithoutOptionalParams() throws IOException {
+ "{\"period\":{\"interval\":425,\"unit\":\"Minutes\"}},\"schema_version\":-1203962153,\"ui_metadata\":"
+ "{\"JbAaV\":{\"feature_id\":\"rIFjS\",\"feature_name\":\"QXCmS\",\"feature_enabled\":false,"
+ "\"aggregation_query\":{\"aa\":{\"value_count\":{\"field\":\"ok\"}}}}},\"last_update_time\":1568396089028}";
AnomalyDetector parsedDetector = AnomalyDetector
.parse(TestHelpers.parser(detectorString), "id", 1L, null, null, AnomalyDetectorSettings.DEFAULT_SHINGLE_SIZE);
AnomalyDetector parsedDetector = AnomalyDetector.parse(TestHelpers.parser(detectorString), "id", 1L, null, null);
assertTrue(parsedDetector.getFilterQuery() instanceof MatchAllQueryBuilder);
assertEquals((long) parsedDetector.getShingleSize(), (long) AnomalyDetectorSettings.DEFAULT_SHINGLE_SIZE);
}
Expand Down

0 comments on commit cafebc2

Please sign in to comment.