You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ak...@apache.org on 2020/09/03 17:19:16 UTC
[incubator-pinot] branch master updated: [TE] only merge same trend
in ChildKeepingMerge (#5920)
This is an automated email from the ASF dual-hosted git repository.
akshayrai09 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 2dbc94c [TE] only merge same trend in ChildKeepingMerge (#5920)
2dbc94c is described below
commit 2dbc94c201557e3bce5a3a944abac46c644c3cbd
Author: Vincent Chen <ji...@linkedin.com>
AuthorDate: Thu Sep 3 10:18:33 2020 -0700
[TE] only merge same trend in ChildKeepingMerge (#5920)
This PR is to fix the issue that ChildKeepMergeWrapper merges anomalies with different trend. After this PR, the wrapper will only merge anomalies with same trend, e.g anomalies with UP trend, meaning current value higher than predicted value.
---
.../wrapper/ChildKeepingMergeWrapper.java | 13 ++++--
.../thirdeye/detection/DetectionTestUtils.java | 30 ++++++++++----
.../wrapper/ChildKeepingMergeWrapperTest.java | 48 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java
index d2a1320..064e4f1 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java
@@ -21,6 +21,7 @@ package org.apache.pinot.thirdeye.detection.wrapper;
import com.google.common.collect.Collections2;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -45,6 +46,7 @@ import org.apache.pinot.thirdeye.util.ThirdEyeUtils;
*/
public class ChildKeepingMergeWrapper extends BaselineFillingMergeWrapper {
private static final String PROP_GROUP_KEY = "groupKey";
+ private static final String PROP_PATTERN_KEY = "pattern";
public ChildKeepingMergeWrapper(DataProvider provider, DetectionConfigDTO config, long startTime, long endTime) {
super(provider, config, startTime, endTime);
@@ -91,10 +93,15 @@ public class ChildKeepingMergeWrapper extends BaselineFillingMergeWrapper {
if (anomaly.getProperties().containsKey(PROP_GROUP_KEY)) {
groupKey = anomaly.getProperties().get(PROP_GROUP_KEY);
}
-
+ String patternKey = "";
+ if (anomaly.getProperties().containsKey(PROP_PATTERN_KEY)) {
+ patternKey = anomaly.getProperties().get(PROP_PATTERN_KEY);
+ } else if (!Double.isNaN(anomaly.getAvgBaselineVal()) && !Double.isNaN(anomaly.getAvgCurrentVal())) {
+ patternKey = (anomaly.getAvgCurrentVal() > anomaly.getAvgBaselineVal()) ? "UP" : "DOWN";
+ }
MergeWrapper.AnomalyKey key =
- new MergeWrapper.AnomalyKey(anomaly.getMetric(), anomaly.getCollection(), anomaly.getDimensions(), groupKey, "",
- anomaly.getType());
+ new MergeWrapper.AnomalyKey(anomaly.getMetric(), anomaly.getCollection(), anomaly.getDimensions(),
+ StringUtils.join(Arrays.asList(groupKey, patternKey), ","), "", anomaly.getType());
MergedAnomalyResultDTO parent = parents.get(key);
if (parent == null || anomaly.getStartTime() - parent.getEndTime() > this.maxGap) {
diff --git a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/DetectionTestUtils.java b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/DetectionTestUtils.java
index 745ba95..8a9eac6 100644
--- a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/DetectionTestUtils.java
+++ b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/DetectionTestUtils.java
@@ -33,7 +33,8 @@ import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity;
public class DetectionTestUtils {
private static final Long PROP_ID_VALUE = 1000L;
- public static MergedAnomalyResultDTO makeAnomaly(Long configId, Long legacyFunctionId, long start, long end, String metric, String dataset, Map<String, String> dimensions) {
+ public static MergedAnomalyResultDTO makeAnomaly(Long configId, Long legacyFunctionId, long start, long end,
+ String metric, String dataset, Map<String, String> dimensions, Map<String, String> props) {
MergedAnomalyResultDTO anomaly = new MergedAnomalyResultDTO();
anomaly.setDetectionConfigId(configId);
anomaly.setStartTime(start);
@@ -41,6 +42,7 @@ public class DetectionTestUtils {
anomaly.setMetric(metric);
anomaly.setCollection(dataset);
anomaly.setFunctionId(legacyFunctionId);
+ anomaly.setProperties(props);
Multimap<String, String> filters = HashMultimap.create();
for (Map.Entry<String, String> dimension : dimensions.entrySet()) {
@@ -55,8 +57,10 @@ public class DetectionTestUtils {
return anomaly;
}
- public static MergedAnomalyResultDTO makeAnomaly(Long configId, long start, long end, String metric, String dataset, Map<String, String> dimensions) {
- return DetectionTestUtils.makeAnomaly(configId, null, start, end, metric, dataset, dimensions);
+ public static MergedAnomalyResultDTO makeAnomaly(Long configId, long start, long end, String metric, String dataset,
+ Map<String, String> dimensions) {
+ return DetectionTestUtils.makeAnomaly(configId, null, start, end, metric, dataset, dimensions,
+ new HashMap<>());
}
public static MergedAnomalyResultDTO setAnomalyId(MergedAnomalyResultDTO anomaly, long id) {
@@ -65,11 +69,12 @@ public class DetectionTestUtils {
}
public static MergedAnomalyResultDTO makeAnomaly(long start, long end) {
- return DetectionTestUtils.makeAnomaly(PROP_ID_VALUE, start, end, null, null, Collections.<String, String>emptyMap());
+ return DetectionTestUtils.makeAnomaly(PROP_ID_VALUE, start, end, null, null, Collections.emptyMap());
}
public static MergedAnomalyResultDTO makeAnomaly(Long configId, Long legacyFuncId, long start, long end) {
- return DetectionTestUtils.makeAnomaly(configId, legacyFuncId, start, end, null, null, Collections.<String, String>emptyMap());
+ return DetectionTestUtils.makeAnomaly(configId, legacyFuncId, start, end, null, null,
+ Collections.emptyMap(), new HashMap<>());
}
public static MergedAnomalyResultDTO makeAnomaly(Long configId, long start, long end) {
@@ -80,7 +85,8 @@ public class DetectionTestUtils {
return DetectionTestUtils.makeAnomaly(PROP_ID_VALUE, start, end, null, null, dimensions);
}
- public static MergedAnomalyResultDTO makeAnomaly(Long configId, long start, long end, Map<String, String> dimensions) {
+ public static MergedAnomalyResultDTO makeAnomaly(Long configId, long start, long end,
+ Map<String, String> dimensions) {
return DetectionTestUtils.makeAnomaly(configId, start, end, null, null, dimensions);
}
@@ -90,13 +96,15 @@ public class DetectionTestUtils {
return result;
}
- public static MergedAnomalyResultDTO makeAnomaly(long start, long end, Map<String, String> dimensions, Set<MergedAnomalyResultDTO> children) {
+ public static MergedAnomalyResultDTO makeAnomaly(long start, long end, Map<String, String> dimensions,
+ Set<MergedAnomalyResultDTO> children) {
MergedAnomalyResultDTO result = makeAnomaly(start, end, dimensions);
result.setChildren(children);
return result;
}
- public static MergedAnomalyResultDTO makeAnomaly(long start, long end, String metricUrn, long currentValue, long baselineValue) {
+ public static MergedAnomalyResultDTO makeAnomaly(long start, long end, String metricUrn, long currentValue,
+ long baselineValue) {
MergedAnomalyResultDTO result = makeAnomaly(start, end);
result.setMetricUrn(metricUrn);
result.setAvgCurrentVal(currentValue);
@@ -111,4 +119,10 @@ public class DetectionTestUtils {
anomaly.setAvgCurrentVal(currentVal);
return anomaly;
}
+
+ public static MergedAnomalyResultDTO makeAnomalyWithProps(long start, long end, Map<String, String> props) {
+ return DetectionTestUtils.makeAnomaly(PROP_ID_VALUE, null, start, end, null, null,
+ Collections.emptyMap(), props);
+ }
+
}
diff --git a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapperTest.java b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapperTest.java
index cb78d8b..7703209 100644
--- a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapperTest.java
+++ b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapperTest.java
@@ -302,4 +302,52 @@ public class ChildKeepingMergeWrapperTest {
Assert.assertEquals(anomalyResults.get(0).getAvgCurrentVal(), 998.0);
}
+
+ @Test
+ public void testMergerDifferentPattern() throws Exception {
+ this.config.getProperties().put(PROP_MAX_GAP, 200);
+ this.config.getProperties().put(PROP_MAX_DURATION, 1250);
+
+ this.outputs.add(new MockPipelineOutput(Arrays.asList(
+ makeAnomalyWithProps(2800, 3800, Collections.singletonMap("pattern", "UP")),
+ makeAnomalyWithProps(3500, 3600, Collections.singletonMap("pattern", "DOWN"))
+ ), 3700));
+
+ Map<String, Object> nestedProperties = new HashMap<>();
+ nestedProperties.put(PROP_CLASS_NAME, "none");
+ nestedProperties.put(PROP_METRIC_URN, "thirdeye:metric:3");
+
+ this.nestedProperties.add(nestedProperties);
+
+ this.wrapper = new ChildKeepingMergeWrapper(this.provider, this.config, 1000, 4000);
+ DetectionPipelineResult output = this.wrapper.run();
+
+ Assert.assertEquals(output.getAnomalies().size(), 5);
+ Assert.assertTrue(output.getAnomalies().contains(makeAnomalyWithProps(2800, 3800, Collections.singletonMap("pattern", "UP"))));
+ Assert.assertTrue(output.getAnomalies().contains(makeAnomalyWithProps(3500, 3600, Collections.singletonMap("pattern", "DOWN"))));
+ }
+
+ @Test
+ public void testMergerInferredPattern() throws Exception{
+ this.config.getProperties().put(PROP_MAX_GAP, 200);
+ this.config.getProperties().put(PROP_MAX_DURATION, 1250);
+
+ this.outputs.add(new MockPipelineOutput(Arrays.asList(
+ makeAnomaly(2800, 3600, null, 3, 2),
+ makeAnomaly(3500, 3800, null, 1, 2)
+ ), 3700));
+
+ Map<String, Object> nestedProperties = new HashMap<>();
+ nestedProperties.put(PROP_CLASS_NAME, "none");
+ nestedProperties.put(PROP_METRIC_URN, "thirdeye:metric:3");
+
+ this.nestedProperties.add(nestedProperties);
+
+ this.wrapper = new ChildKeepingMergeWrapper(this.provider, this.config, 1000, 4000);
+ DetectionPipelineResult output = this.wrapper.run();
+
+ Assert.assertEquals(output.getAnomalies().size(), 5);
+ Assert.assertTrue(output.getAnomalies().contains(makeAnomaly(2800, 3600, null, 3, 2)));
+ Assert.assertTrue(output.getAnomalies().contains(makeAnomaly(3500, 3800, null, 1, 2)));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org