fix(judge): for NaN replacement, nodata means pass (#339)

spinnaker · Jun 29, 2018 · 19a717d · 19a717d
1 parent 9c9993e
commit 19a717d
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 9 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
@@ -170,7 +170,7 @@ class NetflixACAJudge extends CanaryJudge with StrictLogging {
       .critical(critical)
 
     try {
-      val metricClassification = mannWhitney.classify(transformedControl, transformedExperiment, directionality)
+      val metricClassification = mannWhitney.classify(transformedControl, transformedExperiment, directionality, nanStrategy)
       resultBuilder
         .classification(metricClassification.classification.toString)
         .classificationReason(metricClassification.reason.orNull)

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
@@ -59,5 +59,6 @@ case class MetricClassification(classification: MetricClassificationLabel, reaso
 
 abstract class BaseMetricClassifier {
   def classify(control: Metric, experiment: Metric,
-               direction: MetricDirection = MetricDirection.Either): MetricClassification
+               direction: MetricDirection = MetricDirection.Either,
+               nanStrategy: NaNStrategy = NaNStrategy.Remove): MetricClassification
 }
diff --git a/...e/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/MannWhitneyClassifier.scala b/...e/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/MannWhitneyClassifier.scala
@@ -55,11 +55,15 @@ class MannWhitneyClassifier(tolerance: Double=0.25, confLevel: Double=0.95) exte
     (lowerBound, upperBound)
   }
 
-  override def classify(control: Metric, experiment: Metric, direction: MetricDirection): MetricClassification = {
+  override def classify(control: Metric, experiment: Metric, direction: MetricDirection, nanStrategy: NaNStrategy): MetricClassification = {
 
     //Check if there is no-data for the experiment or control
     if (experiment.values.isEmpty || control.values.isEmpty) {
-      return MetricClassification(Nodata, None, 0.0)
+      if (nanStrategy == NaNStrategy.Remove) {
+        return MetricClassification(Nodata, None, 0.0)
+      } else {
+        return MetricClassification(Pass, None, 1.0)
+      }
     }
 
     //Check if the experiment and control data are equal

diff --git a/...rc/main/scala/com/netflix/kayenta/judge/classifiers/metric/MeanInequalityClassifier.scala b/...rc/main/scala/com/netflix/kayenta/judge/classifiers/metric/MeanInequalityClassifier.scala
@@ -27,11 +27,15 @@ import org.apache.commons.math3.stat.StatUtils
   */
 class MeanInequalityClassifier extends BaseMetricClassifier {
 
-  override def classify(control: Metric, experiment: Metric, direction: MetricDirection): MetricClassification = {
+  override def classify(control: Metric, experiment: Metric, direction: MetricDirection, nanStrategy: NaNStrategy): MetricClassification = {
 
     //Check if there is no-data for the experiment or control
     if (experiment.values.isEmpty || control.values.isEmpty) {
-      return MetricClassification(Nodata, None, 0.0)
+      if (nanStrategy == NaNStrategy.Remove) {
+        return MetricClassification(Nodata, None, 0.0)
+      } else {
+        return MetricClassification(Pass, None, 1.0)
+      }
     }
 
     val experimentMean = StatUtils.mean(experiment.values)

diff --git a/...-judge/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/RandomClassifier.scala b/...-judge/src/main/scala/com/netflix/kayenta/judge/classifiers/metric/RandomClassifier.scala
@@ -34,11 +34,15 @@ class RandomClassifier(labels: List[MetricClassificationLabel] = List(Pass, High
     */
   def getRandomLabel(list: List[MetricClassificationLabel]): MetricClassificationLabel = Random.shuffle(list).head
 
-  override def classify(control: Metric, experiment: Metric, direction: MetricDirection): MetricClassification = {
+  override def classify(control: Metric, experiment: Metric, direction: MetricDirection, nanStrategy: NaNStrategy): MetricClassification = {
 
     //Check if there is no-data for the experiment or control
-    if(experiment.values.isEmpty || control.values.isEmpty){
-      return MetricClassification(Nodata, None, 0.0)
+    if (experiment.values.isEmpty || control.values.isEmpty) {
+      if (nanStrategy == NaNStrategy.Remove) {
+        return MetricClassification(Nodata, None, 0.0)
+      } else {
+        return MetricClassification(Pass, None, 1.0)
+      }
     }
 
     //Check if the experiment and control data are equal

diff --git a/kayenta-judge/src/test/scala/com/netflix/kayenta/judge/ClassifierSuite.scala b/kayenta-judge/src/test/scala/com/netflix/kayenta/judge/ClassifierSuite.scala
@@ -361,6 +361,19 @@ class ClassifierSuite extends FunSuite{
     assert(result.classification == Nodata)
   }
 
+  test("Mann-Whitney Missing Experiment Data with NaN Replacement") {
+    val experimentData = Array[Double]()
+    val controlData = Array(1.0, 2.0, 3.0, 4.0, 5.0)
+
+    val experimentMetric = Metric("test-metric", experimentData, "canary")
+    val controlMetric = Metric("test-metric", controlData, "baseline")
+
+    val classifier = new MannWhitneyClassifier(tolerance = 0.10, confLevel = 0.95)
+    val result = classifier.classify(controlMetric, experimentMetric, MetricDirection.Decrease, NaNStrategy.Replace)
+
+    assert(result.classification == Pass)
+  }
+
   test("Mean Inequality Classifier Test: High Metric"){
     val experimentData = Array(10.0, 20.0, 30.0, 40.0, 50.0)
     val controlData = Array(1.0, 2.0, 3.0, 4.0, 5.0)