Skip to content

Commit

Permalink
made sure to return window delay rec if its the only issue apparent
Browse files Browse the repository at this point in the history 
Signed-off-by: Amit Galitzky <[email protected]>
  • Loading branch information
@amitgalitz
amitgalitz committed Mar 8, 2022
1 parent 098bb93 commit e926907
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 49 deletions.
1 change: 1 addition & 0 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ List<String> jacocoExclusions = [
'org.opensearch.ad.AnomalyDetectorPlugin',
'org.opensearch.ad.settings.AnomalyDetectorSettings',

//TODO: Add more cases for model validation API, both UT and IT
//TODO: add more test cases later for these package
'org.opensearch.ad.model.*',
'org.opensearch.ad.rest.*',
Expand Down
46 changes: 29 additions & 17 deletions src/main/java/org/opensearch/ad/rest/handler/ModelValidationActionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,11 @@ private void processIntervalRecommendation(IntervalTimeConfiguration interval, l
} else {
if (interval.equals(anomalyDetector.getDetectionInterval())) {
logger.info("Using the current interval there is enough dense data ");
// Check if there is a window delay recommendation if everything else is successful and send exception
if (Instant.now().toEpochMilli() - latestTime > timeConfigToMilliSec(anomalyDetector.getWindowDelay())) {
sendWindowDelayRec(latestTime);
return;
}
// The rate of buckets with at least 1 doc with given interval is above the success rate
listener.onResponse(null);
return;
Expand Down Expand Up @@ -692,26 +697,33 @@ private void checkFeatureQuery(long latestTime) throws IOException {
client.search(searchRequest, ActionListener.wrap(response -> processFeatureQuery(response, latestTime), listener::onFailure));
}

private void sendWindowDelayRec(long latestTime) {
long minutesSinceLastStamp = TimeUnit.MILLISECONDS.toMinutes(Instant.now().toEpochMilli() - latestTime);
listener
.onFailure(
new ADValidationException(
String.format(Locale.ROOT, CommonErrorMessages.WINDOW_DELAY_REC, minutesSinceLastStamp, minutesSinceLastStamp),
DetectorValidationIssueType.WINDOW_DELAY,
ValidationAspect.MODEL,
new IntervalTimeConfiguration(minutesSinceLastStamp, ChronoUnit.MINUTES)
)
);
}

private void windowDelayRecommendation(long latestTime) {
long delayMillis = timeConfigToMilliSec(anomalyDetector.getWindowDelay());
if ((Instant.now().toEpochMilli() - latestTime > delayMillis)) {
long minutesSinceLastStamp = TimeUnit.MILLISECONDS.toMinutes(Instant.now().toEpochMilli() - latestTime);
listener
.onFailure(
new ADValidationException(
String.format(Locale.ROOT, CommonErrorMessages.WINDOW_DELAY_REC, minutesSinceLastStamp, minutesSinceLastStamp),
DetectorValidationIssueType.WINDOW_DELAY,
ValidationAspect.MODEL,
new IntervalTimeConfiguration(minutesSinceLastStamp, ChronoUnit.MINUTES)
)
);
// Check if there is a better window-delay to recommend and if one was recommended
// then send exception and return, otherwise continue to let user know data is too sparse as explained below
if (Instant.now().toEpochMilli() - latestTime > timeConfigToMilliSec(anomalyDetector.getWindowDelay())) {
sendWindowDelayRec(latestTime);
return;
}
// This case has been reached if no interval recommendation was found that leads to a bucket success rate of >= 0.75
// but no single configuration during the following checks reduced the bucket success rate below 0.25
// This means the rate with all configs applied was below 0.75 but the rate when checking each configuration at time
// was always above 0.25 meaning the best suggestion is to simply ingest more data since we have no more insight
// regarding the root cause of the lower density.
// This case has been reached if following conditions are met:
// 1. no interval recommendation was found that leads to a bucket success rate of >= 0.75
// 2. bucket success rate with the given interval and just raw data is also below 0.75.
// 3. no single configuration during the following checks reduced the bucket success rate below 0.25
// This means the rate with all configs applied or just raw data was below 0.75 but the rate when checking each configuration at
// a time was always above 0.25 meaning the best suggestion is to simply ingest more data or change interval since
// we have no more insight regarding the root cause of the lower density.
listener
.onFailure(
new ADValidationException(
Expand Down
38 changes: 38 additions & 0 deletions src/test/java/org/opensearch/ad/e2e/DetectionResultEvalutationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,44 @@ public void testValidationIntervalRecommendation() throws Exception {
);
}

public void testValidationWindowDelayRecommendation() throws Exception {
RestClient client = client();
long recDetectorIntervalMillis = 180000;
// this would be equivalent to the window delay in this data test
long recDetectorIntervalMinutes = recDetectorIntervalMillis / 60000;
List<JsonObject> data = createData(2000, recDetectorIntervalMillis);
indexTrainData("validation", data, 2000, client);
long detectorInterval = 4;
String requestBody = String
.format(
Locale.ROOT,
"{ \"name\": \"test\", \"description\": \"test\", \"time_field\": \"timestamp\""
+ ", \"indices\": [\"validation\"], \"feature_attributes\": [{ \"feature_name\": \"feature 1\", \"feature_enabled\": "
+ "\"true\", \"aggregation_query\": { \"Feature1\": { \"sum\": { \"field\": \"Feature1\" } } } }, { \"feature_name\""
+ ": \"feature 2\", \"feature_enabled\": \"true\", \"aggregation_query\": { \"Feature2\": { \"sum\": { \"field\": "
+ "\"Feature2\" } } } }], \"detection_interval\": { \"period\": { \"interval\": %d, \"unit\": \"Minutes\" } }"
+ ",\"window_delay\":{\"period\":{\"interval\":1,\"unit\":\"Minutes\"}}}",
detectorInterval
);
Response resp = TestHelpers
.makeRequest(
client(),
"POST",
TestHelpers.AD_BASE_DETECTORS_URI + "/_validate/model",
ImmutableMap.of(),
toHttpEntity(requestBody),
null
);
Map<String, Object> responseMap = entityAsMap(resp);
@SuppressWarnings("unchecked")
Map<String, Map<String, String>> messageMap = (Map<String, Map<String, String>>) XContentMapValues
.extractValue("model", responseMap);
assertEquals(
String.format(Locale.ROOT, CommonErrorMessages.WINDOW_DELAY_REC, +recDetectorIntervalMinutes, recDetectorIntervalMinutes),
messageMap.get("window_delay").get("message")
);
}

private List<JsonObject> createData(int numOfDataPoints, long detectorIntervalMS) {
List<JsonObject> list = new ArrayList<>();
for (int i = 1; i < numOfDataPoints; i++) {
Expand Down
56 changes: 24 additions & 32 deletions src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,20 @@ public void testCreateAnomalyDetectorWithEmptyIndices() throws Exception {
);
}

private AnomalyDetector createIndexAndGetAnomalyDetector() throws IOException {
TestHelpers.createIndexWithTimeField(client(), INDEX_NAME, TIME_FIELD);
private AnomalyDetector createIndexAndGetAnomalyDetector(String indexName) throws IOException {
return createIndexAndGetAnomalyDetector(indexName, ImmutableList.of(TestHelpers.randomFeature(true)));
}

private AnomalyDetector createIndexAndGetAnomalyDetector(String indexName, List<Feature> features) throws IOException {
TestHelpers.createIndexWithTimeField(client(), indexName, TIME_FIELD);
String testIndexData = "{\"keyword-field\": \"field-1\", \"ip-field\": \"1.2.3.4\", \"timestamp\": 1}";
TestHelpers.ingestDataToIndex(client(), INDEX_NAME, TestHelpers.toHttpEntity(testIndexData));
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TIME_FIELD, INDEX_NAME);
TestHelpers.ingestDataToIndex(client(), indexName, TestHelpers.toHttpEntity(testIndexData));
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TIME_FIELD, indexName, features);
return detector;
}

public void testCreateAnomalyDetectorWithDuplicateName() throws Exception {
AnomalyDetector detector = createIndexAndGetAnomalyDetector();
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
AnomalyDetector detectorDuplicateName = new AnomalyDetector(
AnomalyDetector.NO_ID,
randomLong(),
Expand Down Expand Up @@ -157,7 +161,7 @@ public void testCreateAnomalyDetectorWithDuplicateName() throws Exception {
}

public void testCreateAnomalyDetector() throws Exception {
AnomalyDetector detector = createIndexAndGetAnomalyDetector();
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
updateClusterSettings(EnabledSetting.AD_PLUGIN_ENABLED, false);

Exception ex = expectThrows(
Expand Down Expand Up @@ -186,7 +190,7 @@ public void testCreateAnomalyDetector() throws Exception {
}

public void testUpdateAnomalyDetectorCategoryField() throws Exception {
AnomalyDetector detector = createIndexAndGetAnomalyDetector();
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
Response response = TestHelpers
.makeRequest(client(), "POST", TestHelpers.AD_BASE_DETECTORS_URI, ImmutableMap.of(), TestHelpers.toHttpEntity(detector), null);
assertEquals("Create anomaly detector failed", RestStatus.CREATED, TestHelpers.restStatus(response));
Expand Down Expand Up @@ -246,7 +250,7 @@ public void testGetNotExistingAnomalyDetector() throws Exception {
}

public void testUpdateAnomalyDetector() throws Exception {
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(), true, client());
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(INDEX_NAME), true, client());
String newDescription = randomAlphaOfLength(5);
AnomalyDetector newDetector = new AnomalyDetector(
detector.getDetectorId(),
Expand Down Expand Up @@ -307,13 +311,8 @@ public void testUpdateAnomalyDetector() throws Exception {
}

public void testUpdateAnomalyDetectorNameToExisting() throws Exception {
AnomalyDetector detector1 = TestHelpers.randomAnomalyDetector(TestHelpers.randomUiMetadata(), null);
String indexName1 = detector1.getIndices().get(0);
TestHelpers.createIndex(client(), indexName1, TestHelpers.toHttpEntity("{\"name\": \"test\"}"));
AnomalyDetector detector2 = TestHelpers.randomAnomalyDetector(TestHelpers.randomUiMetadata(), null);
String indexName2 = detector2.getIndices().get(0);
TestHelpers.createIndex(client(), indexName2, TestHelpers.toHttpEntity("{\"name\": \"test\"}"));

AnomalyDetector detector1 = createIndexAndGetAnomalyDetector("index-test-one");
AnomalyDetector detector2 = createIndexAndGetAnomalyDetector("index-test-two");
AnomalyDetector newDetector1WithDetector2Name = new AnomalyDetector(
detector1.getDetectorId(),
detector1.getVersion(),
Expand Down Expand Up @@ -351,7 +350,7 @@ public void testUpdateAnomalyDetectorNameToExisting() throws Exception {
}

public void testUpdateAnomalyDetectorNameToNew() throws Exception {
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(), true, client());
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(INDEX_NAME), true, client());
AnomalyDetector detectorWithNewName = new AnomalyDetector(
detector.getDetectorId(),
detector.getVersion(),
Expand Down Expand Up @@ -766,7 +765,7 @@ public void testDeleteAnomalyDetectorWithRunningAdJob() throws Exception {
}

public void testUpdateAnomalyDetectorWithRunningAdJob() throws Exception {
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(), true, client());
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(INDEX_NAME), true, client());
Response startAdJobResponse = TestHelpers
.makeRequest(
client(),
Expand Down Expand Up @@ -1196,7 +1195,7 @@ public void testDeleteAnomalyDetectorWhileRunning() throws Exception {

public void testBackwardCompatibilityWithOpenDistro() throws IOException {
// Create a detector
AnomalyDetector detector = createIndexAndGetAnomalyDetector();
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
// Verify the detector is created using legacy _opendistro API
Response response = TestHelpers
.makeRequest(
Expand Down Expand Up @@ -1233,7 +1232,7 @@ public void testBackwardCompatibilityWithOpenDistro() throws IOException {
}

public void testValidateAnomalyDetectorWithDuplicateName() throws Exception {
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(), true, client());
AnomalyDetector detector = createAnomalyDetector(createIndexAndGetAnomalyDetector(INDEX_NAME), true, client());
Response resp = TestHelpers
.makeRequest(
client(),
Expand Down Expand Up @@ -1331,9 +1330,7 @@ public void testValidateAnomalyDetectorWithIncorrectShingleSize() throws Excepti
}

public void testValidateAnomalyDetectorWithNoIssue() throws Exception {
String indexName = "testindex";
TestHelpers.createIndexWithTimeField(client(), "testindex", TIME_FIELD);
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TIME_FIELD, indexName);
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
Response resp = TestHelpers
.makeRequest(
client(),
Expand All @@ -1348,9 +1345,7 @@ public void testValidateAnomalyDetectorWithNoIssue() throws Exception {
}

public void testValidateAnomalyDetectorOnWrongValidationType() throws Exception {
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TestHelpers.randomUiMetadata(), null);
String indexName = detector.getIndices().get(0);
TestHelpers.createIndex(client(), indexName, TestHelpers.toHttpEntity("{\"timestamp\": " + Instant.now().toEpochMilli() + "}"));
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
TestHelpers
.assertFailWith(
ResponseException.class,
Expand All @@ -1359,7 +1354,7 @@ public void testValidateAnomalyDetectorOnWrongValidationType() throws Exception
.makeRequest(
client(),
"POST",
TestHelpers.AD_BASE_DETECTORS_URI + "/_validate/model",
TestHelpers.AD_BASE_DETECTORS_URI + "/_validate/models",
ImmutableMap.of(),
TestHelpers.toHttpEntity(detector),
null
Expand Down Expand Up @@ -1429,8 +1424,7 @@ public void testValidateAnomalyDetectorWithInvalidName() throws Exception {

public void testValidateAnomalyDetectorWithFeatureQueryReturningNoData() throws Exception {
Feature emptyFeature = TestHelpers.randomFeature("f-empty", "cpu", "avg", true);
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TIME_FIELD, "index-test", ImmutableList.of(emptyFeature));
TestHelpers.createIndexWithTimeField(client(), "index-test", TIME_FIELD);
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME, ImmutableList.of(emptyFeature));
Response resp = TestHelpers
.makeRequest(
client(),
Expand All @@ -1452,10 +1446,8 @@ public void testValidateAnomalyDetectorWithFeatureQueryReturningNoData() throws
}

public void testValidateAnomalyDetectorWithFeatureQueryRuntimeException() throws Exception {
String nonNumericField = "_type";
Feature nonNumericFeature = TestHelpers.randomFeature("non-numeric-feature", nonNumericField, "avg", true);
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TIME_FIELD, "index-test", ImmutableList.of(nonNumericFeature));
TestHelpers.createIndexWithTimeField(client(), "index-test", TIME_FIELD);
Feature nonNumericFeature = TestHelpers.randomFeature("non-numeric-feature", "_type", "avg", true);
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME, ImmutableList.of(nonNumericFeature));
Response resp = TestHelpers
.makeRequest(
client(),
Expand Down

0 comments on commit e926907

Please sign in to comment.