From ed018e2b3d55e27a4d80bf9ffa09281c48ed1266 Mon Sep 17 00:00:00 2001 From: Cyril de Catheu Date: Wed, 13 Dec 2023 21:59:55 +0100 Subject: [PATCH] [plugin-postprocessor] change default merge gap from 2 hours to consecutive anomalies only (#1268) --- .../resources/common-properties/common-properties.json | 4 ++-- .../postprocessor/AnomalyMergerPostProcessor.java | 3 ++- .../postprocessor/AnomalyMergerPostProcessorTest.java | 9 ++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/thirdeye-plugins/thirdeye-bootstrap-open-core/src/main/resources/common-properties/common-properties.json b/thirdeye-plugins/thirdeye-bootstrap-open-core/src/main/resources/common-properties/common-properties.json index e3bec4e404..a9ad84726c 100644 --- a/thirdeye-plugins/thirdeye-bootstrap-open-core/src/main/resources/common-properties/common-properties.json +++ b/thirdeye-plugins/thirdeye-bootstrap-open-core/src/main/resources/common-properties/common-properties.json @@ -281,7 +281,7 @@ "defaultValue": null, "min": null, "max": null, - "description": "Maximum gap between 2 anomalies for anomalies to be merged. In ISO-8601 format. Example: `PT2H`. To disable anomalies merging, set this value to `P0D`.", + "description": "Maximum duration of an anomaly merger. At merge time, if an anomaly merger would get bigger than this limit, the anomalies are not merged. In ISO-8601 format. Example: `P7D`.", "options": null, "multiselect": false, "jsonType": "STRING", @@ -294,7 +294,7 @@ "defaultValue": null, "min": null, "max": null, - "description": "Maximum duration of an anomaly merger. At merge time, if an anomaly merger would get bigger than this limit, the anomalies are not merged. In ISO-8601 format. Example: `P7D`.", + "description": "Maximum gap between 2 anomalies for anomalies to be merged. In ISO-8601 format. Example: `PT2H`. The default behavior is to merge consecutive anomalies only. To disable anomaly merging entirely, set this value to `P0D`.", "options": null, "multiselect": false, "jsonType": "STRING", diff --git a/thirdeye-plugins/thirdeye-postprocessors/src/main/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessor.java b/thirdeye-plugins/thirdeye-postprocessors/src/main/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessor.java index 3dca9d0c93..92a5bcfe47 100644 --- a/thirdeye-plugins/thirdeye-postprocessors/src/main/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessor.java +++ b/thirdeye-plugins/thirdeye-postprocessors/src/main/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessor.java @@ -89,8 +89,9 @@ public class AnomalyMergerPostProcessor implements AnomalyPostProcessor { // more children first return -1 * Integer.compare(o1.getChildren().size(), o2.getChildren().size()); }; + // by default, only merge consecutive anomalies. And P0D is a special value to disable merging entirely @VisibleForTesting - protected static final Period DEFAULT_MERGE_MAX_GAP = Period.hours(2); + protected static final Period DEFAULT_MERGE_MAX_GAP = Period.seconds(1); @VisibleForTesting protected static final Period DEFAULT_ANOMALY_MAX_DURATION = Period.days(7); public static final String NEW_AFTER_REPLAY_LABEL_NAME = "NEW_AFTER_REPLAY"; diff --git a/thirdeye-plugins/thirdeye-postprocessors/src/test/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessorTest.java b/thirdeye-plugins/thirdeye-postprocessors/src/test/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessorTest.java index 70581c9632..904ca6b263 100644 --- a/thirdeye-plugins/thirdeye-postprocessors/src/test/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessorTest.java +++ b/thirdeye-plugins/thirdeye-postprocessors/src/test/java/ai/startree/thirdeye/plugins/postprocessor/AnomalyMergerPostProcessorTest.java @@ -99,8 +99,8 @@ public void setUp() { final AnomalyFilter filter = (AnomalyFilter) i.getArguments()[0]; // pseudo database that filters by start time, end time, enumerationItemId return existingAnomalies.stream() - .filter(a -> a.getStartTime() >= filter.getStartEndWindow().getStartMillis()) - .filter(a -> a.getEndTime() <= filter.getStartEndWindow().getEndMillis()) + .filter(a -> a.getStartTime() < filter.getStartEndWindow().getEndMillis()) + .filter(a -> a.getEndTime() > filter.getStartEndWindow().getStartMillis()) .filter(a -> filter.getEnumerationItemId() == null || filter.getEnumerationItemId() .equals(a.getEnumerationItem().getId())) .collect(Collectors.toList()); @@ -255,7 +255,7 @@ public void testMergeNewInExisting() { final long newEndTime = new DateTime(JANUARY_1_2021_02H, UTC).plus(Period.hours(2)) .getMillis(); final AnomalyDTO new1 = newAnomaly( - new DateTime(JANUARY_1_2021_02H, UTC).plus(Period.hours(1)).getMillis(), + new DateTime(JANUARY_1_2021_02H, UTC).getMillis(), newEndTime); final List merged = detectionMerger.doMerge(List.of(new1), List.of(existing1)); @@ -369,8 +369,7 @@ public void testMergeExistingInNew() { final AnomalyDTO new1 = newAnomaly(JANUARY_1_2021_01H, JANUARY_1_2021_02H); final long newEndTime = new DateTime(JANUARY_1_2021_02H, UTC).plus(Period.hours(2)) .getMillis(); - final AnomalyDTO existing1 = existingAnomaly( - new DateTime(JANUARY_1_2021_02H, UTC).plus(Period.hours(1)).getMillis(), + final AnomalyDTO existing1 = existingAnomaly(new DateTime(JANUARY_1_2021_02H, UTC).getMillis(), newEndTime); final List merged = detectionMerger.doMerge(List.of(new1), List.of(existing1));