fix(judge): Disable first pass outlier removal by default. (#621)

spinnaker · Oct 21, 2019 · 08b119a · 08b119a
1 parent fde9a03
commit 08b119a
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 8 deletions.
diff --git a/kayenta-judge/src/main/scala/com/netflix/kayenta/judge/NetflixACAJudge.scala b/kayenta-judge/src/main/scala/com/netflix/kayenta/judge/NetflixACAJudge.scala
@@ -63,14 +63,26 @@ class NetflixACAJudge extends CanaryJudge with StrictLogging {
   /**
     * Metric Transformations
     */
-  def transformMetric(metric: Metric, nanStrategy: NaNStrategy): Metric = {
-    val detector = new IQRDetector(factor = 3.0, reduceSensitivity = true)
-    val transform = if (nanStrategy == NaNStrategy.Remove) {
-      Function.chain[Metric](Seq(Transforms.removeNaNs(_), Transforms.removeOutliers(_, detector)))
+  def transformMetric(metric: Metric, nanStrategy: NaNStrategy, outlierStrategy: OutlierStrategy, outlierFactor: Double): Metric = {
+    handleOutliers(handleNaNs(metric, nanStrategy), outlierStrategy, outlierFactor)
+  }
+
+  def handleNaNs(metric: Metric, nanStrategy: NaNStrategy): Metric = {
+    if (nanStrategy == NaNStrategy.Remove) {
+      Transforms.removeNaNs(metric)
+    } else {
+      Transforms.replaceNaNs(metric)
+    }
+  }
+
+  def handleOutliers(metric: Metric, outlierStrategy: OutlierStrategy, outlierFactor: Double): Metric = {
+    if (outlierStrategy == OutlierStrategy.Remove) {
+      val detector = new IQRDetector(outlierFactor, reduceSensitivity = true)
+
+      Transforms.removeOutliers(metric, detector)
     } else {
-      Function.chain[Metric](Seq(Transforms.replaceNaNs(_), Transforms.removeOutliers(_, detector)))
+      metric
     }
-    transform(metric)
   }
 
   /**
@@ -95,6 +107,10 @@ class NetflixACAJudge extends CanaryJudge with StrictLogging {
     val nanStrategyString = MapUtils.getAsStringWithDefault("none", metricConfig.getAnalysisConfigurations, "canary", "nanStrategy")
     val nanStrategy = NaNStrategy.parse(nanStrategyString)
 
+    val outlierStrategyString = MapUtils.getAsStringWithDefault("none", metricConfig.getAnalysisConfigurations, "canary", "outliers", "strategy")
+    val outlierStrategy = OutlierStrategy.parse(outlierStrategyString)
+    val outlierFactor = MapUtils.getAsDoubleWithDefault(3.0, metricConfig.getAnalysisConfigurations, "canary", "outliers", "outlierFactor")
+
     val isCriticalMetric = MapUtils.getAsBooleanWithDefault(false, metricConfig.getAnalysisConfigurations, "canary", "critical")
 
     val isDataRequired = MapUtils.getAsBooleanWithDefault(false, metricConfig.getAnalysisConfigurations, "canary", "mustHaveData")
@@ -112,8 +128,8 @@ class NetflixACAJudge extends CanaryJudge with StrictLogging {
     //=============================================
     // Metric Transformation (Remove NaN values, etc.)
     // ============================================
-    val transformedExperiment = transformMetric(experiment, nanStrategy)
-    val transformedControl = transformMetric(control, nanStrategy)
+    val transformedExperiment = transformMetric(experiment, nanStrategy, outlierStrategy, outlierFactor)
+    val transformedControl = transformMetric(control, nanStrategy, outlierStrategy, outlierFactor)
 
     //=============================================
     // Calculate metric statistics

diff --git a/...ge/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/BaseMetricClassifier.scala b/...ge/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/BaseMetricClassifier.scala
@@ -56,6 +56,20 @@ object NaNStrategy {
   }
 }
 
+sealed trait OutlierStrategy
+object OutlierStrategy {
+  case object Remove extends OutlierStrategy
+  case object Keep extends OutlierStrategy
+
+  def parse(outlierStrategy: String): OutlierStrategy = {
+    outlierStrategy match {
+      case "remove" => OutlierStrategy.Remove
+      case "keep" => OutlierStrategy.Keep
+      case _ => OutlierStrategy.Keep
+    }
+  }
+}
+
 case class MetricClassification(classification: MetricClassificationLabel,
                                 reason: Option[String],
                                 deviation: Double,