This is an automated email from the ASF dual-hosted git repository. jihao pushed a commit to branch threshold-rule-filter-fix in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit e8e9d82150a79d90affad4c26d48fe1772f42ee4 Author: Jihao Zhang <[email protected]> AuthorDate: Mon Apr 20 13:29:18 2020 -0700 [TE] Add validations for the threshold rule anomaly filter The threshold rule anomaly filter is used for filtering the anomaly based on the current value in the anomaly duration. If the metric is not aggregated by SUM or COUNT, user may set the wrong parameter and cause the system to filter the anomaly based on a wrong value. This PR fixes the issue. --- .../components/ThresholdRuleAnomalyFilter.java | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/ThresholdRuleAnomalyFilter.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/ThresholdRuleAnomalyFilter.java index 0f2e7b4..3620ace 100644 --- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/ThresholdRuleAnomalyFilter.java +++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/ThresholdRuleAnomalyFilter.java @@ -19,10 +19,14 @@ package org.apache.pinot.thirdeye.detection.components; +import com.google.common.base.Preconditions; import java.util.concurrent.TimeUnit; +import org.apache.pinot.thirdeye.constant.MetricAggFunction; import org.apache.pinot.thirdeye.dataframe.DataFrame; import org.apache.pinot.thirdeye.dataframe.util.MetricSlice; import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO; +import org.apache.pinot.thirdeye.datalayer.dto.MetricConfigDTO; +import org.apache.pinot.thirdeye.datasource.comparison.Row; import org.apache.pinot.thirdeye.detection.InputDataFetcher; import org.apache.pinot.thirdeye.detection.annotation.Components; import org.apache.pinot.thirdeye.detection.annotation.DetectionTag; @@ -52,8 +56,14 @@ public class ThresholdRuleAnomalyFilter implements AnomalyFilter<ThresholdRuleFi private double maxValueDaily; private double maxValue; private double minValue; + private InputDataFetcher dataFetcher; + @Override public boolean isQualified(MergedAnomalyResultDTO anomaly) { + MetricEntity me = MetricEntity.fromURN(anomaly.getMetricUrn()); + MetricConfigDTO metric = dataFetcher.fetchData(new InputDataSpec().withMetricIds(Collections.singleton(me.getId()))) + .getMetrics() + .get(me.getId()); double currentValue = anomaly.getAvgCurrentVal(); Interval anomalyInterval = new Interval(anomaly.getStartTime(), anomaly.getEndTime()); @@ -64,20 +74,37 @@ public class ThresholdRuleAnomalyFilter implements AnomalyFilter<ThresholdRuleFi return false; } if (!Double.isNaN(this.minValueHourly) && currentValue * hourlyMultiplier < this.minValueHourly) { + Preconditions.checkArgument(isAdditive(metric), String.format( + "Aggregation function for the metric %s is %s. It must be SUM or COUNT to be filtered based on minValueHourly, please use minValue instead.", + metric.getName(), metric.getDefaultAggFunction())); return false; } if (!Double.isNaN(this.maxValueHourly) && currentValue * hourlyMultiplier > this.maxValueHourly) { + Preconditions.checkArgument(isAdditive(metric), String.format( + "Aggregation function for the metric %s is %s. It must be SUM or COUNT to be filtered based on maxValueHourly, please use maxValue instead", + metric.getName(), metric.getDefaultAggFunction())); return false; } if (!Double.isNaN(this.minValueDaily) && currentValue * dailyMultiplier < this.minValueDaily) { + Preconditions.checkArgument(isAdditive(metric), String.format( + "Aggregation function for the metric %s is %s. It must be SUM or COUNT to be filtered based on minValueDaily, please use minValue instead.", + metric.getName(), metric.getDefaultAggFunction())); return false; } if (!Double.isNaN(this.maxValueDaily) && currentValue * dailyMultiplier > this.maxValueDaily) { + Preconditions.checkArgument(isAdditive(metric), String.format( + "Aggregation function for the metric %s is %s. It must be SUM or COUNT to be filtered based on maxValueDaily, please use maxValue instead.", + metric.getName(), metric.getDefaultAggFunction())); return false; } return true; } + private boolean isAdditive(MetricConfigDTO metric) { + MetricAggFunction aggFunction = metric.getDefaultAggFunction(); + return aggFunction.equals(MetricAggFunction.SUM) || aggFunction.equals(MetricAggFunction.COUNT); + } + @Override public void init(ThresholdRuleFilterSpec spec, InputDataFetcher dataFetcher) { this.minValueHourly = spec.getMinValueHourly(); @@ -86,5 +113,6 @@ public class ThresholdRuleAnomalyFilter implements AnomalyFilter<ThresholdRuleFi this.maxValueDaily = spec.getMaxValueDaily(); this.maxValue = spec.getMaxValue(); this.minValue = spec.getMinValue(); + this.dataFetcher = dataFetcher; } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
