chore(canary-config): Move judge configuration to the top level. (#84)

spinnaker · Sep 15, 2017 · 47f0dcd · 47f0dcd
1 parent 49c47e0
commit 47f0dcd
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 31 deletions.
diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryConfig.java b/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryConfig.java
@@ -69,6 +69,10 @@ public class CanaryConfig {
   @Getter
   private String configVersion;
 
+  @NotNull
+  @Getter
+  private CanaryJudgeConfig judge;
+
   @NotNull
   @Singular
   @Getter

diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryJudgeConfig.java b/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryJudgeConfig.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2017 Google, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License")
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.kayenta.canary;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Singular;
+import lombok.ToString;
+
+import javax.validation.constraints.NotNull;
+import java.util.Map;
+
+@Builder
+@ToString
+@NoArgsConstructor
+@AllArgsConstructor
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class CanaryJudgeConfig {
+
+  @NotNull
+  @Getter
+  private String name;
+
+  @NotNull
+  @Singular
+  @Getter
+  private Map<String, Object> judgeConfigurations;
+}
diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/canary/orca/CanaryJudgeTask.java b/kayenta-core/src/main/java/com/netflix/kayenta/canary/orca/CanaryJudgeTask.java
@@ -20,7 +20,7 @@
 import com.netflix.kayenta.canary.CanaryClassifierThresholdsConfig;
 import com.netflix.kayenta.canary.CanaryConfig;
 import com.netflix.kayenta.canary.CanaryJudge;
-import com.netflix.kayenta.canary.CanaryMetricConfig;
+import com.netflix.kayenta.canary.CanaryJudgeConfig;
 import com.netflix.kayenta.canary.results.CanaryJudgeResult;
 import com.netflix.kayenta.metrics.MetricSetPair;
 import com.netflix.kayenta.security.AccountCredentials;
@@ -33,6 +33,7 @@
 import com.netflix.spinnaker.orca.RetryableTask;
 import com.netflix.spinnaker.orca.TaskResult;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
+import org.apache.commons.lang.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -87,26 +88,19 @@ public TaskResult execute(Stage stage) {
 
     CanaryConfig canaryConfig = storageService.loadObject(resolvedAccountName, ObjectType.CANARY_CONFIG, canaryConfigId.toLowerCase());
     List<MetricSetPair> metricSetPairList = storageService.loadObject(resolvedAccountName, ObjectType.METRIC_SET_PAIR_LIST, metricSetPairListId);
-    List<CanaryMetricConfig> metricConfigList = canaryConfig.getMetrics();
+    CanaryJudgeConfig canaryJudgeConfig = canaryConfig.getJudge();
     CanaryJudge canaryJudge = null;
 
-    if (metricConfigList != null && metricConfigList.size() > 0) {
-      // TODO(duftler): We're just considering the judge reference in the first metric analysis configuration here. Should we be considering each one?
-      Map<String, Map> analysisConfigurations = metricConfigList.get(0).getAnalysisConfigurations();
+    if (canaryJudgeConfig != null) {
+      String judgeName = canaryJudgeConfig.getName();
 
-      if (analysisConfigurations != null && analysisConfigurations.containsKey("canary")) {
-        Map canaryAnalysisConfiguration = analysisConfigurations.get("canary");
-
-        if (canaryAnalysisConfiguration != null && canaryAnalysisConfiguration.containsKey("judge")) {
-          String judgeName = (String)canaryAnalysisConfiguration.get("judge");
-
-          canaryJudge =
-            canaryJudges
-              .stream()
-              .filter(c -> c.getName().equals(judgeName))
-              .findFirst()
-              .orElseThrow(() -> new IllegalArgumentException("Unable to resolve canary judge '" + judgeName + "'."));
-        }
+      if (!StringUtils.isEmpty(judgeName)) {
+        canaryJudge =
+          canaryJudges
+            .stream()
+            .filter(c -> c.getName().equals(judgeName))
+            .findFirst()
+            .orElseThrow(() -> new IllegalArgumentException("Unable to resolve canary judge '" + judgeName + "'."));
       }
     }
 

diff --git a/scratch/atlas_canary_config.json b/scratch/atlas_canary_config.json
@@ -2,6 +2,11 @@
   "name": "MySampleAtlasCanaryConfig",
   "description": "Example Automated Canary Analysis (ACA) Configuration using Atlas",
   "configVersion": "1.0",
+  "judge": {
+    "name": "dredd-v1.0",
+    "judgeConfigurations": {
+    }
+  },
   "metrics": [
     {
       "name": "cpu",
@@ -14,9 +19,6 @@
         "system"
       ],
       "analysisConfigurations": {
-        "canary": {
-          "judge": "dredd-v1.0"
-        }
       }
     },
     {
@@ -30,9 +32,6 @@
         "requests"
       ],
       "analysisConfigurations": {
-        "canary": {
-
-        }
       }
     }
   ],

diff --git a/scratch/prometheus_canary_config.json b/scratch/prometheus_canary_config.json
@@ -2,6 +2,11 @@
   "name": "MySamplePrometheusCanaryConfig",
   "description": "Example Automated Canary Analysis (ACA) Configuration using Prometheus",
   "configVersion": 1.0,
+  "judge": {
+    "name": "dredd-v1.0",
+    "judgeConfigurations": {
+    }
+  },
   "metrics": [
     {
       "serviceName": "prometheus",
@@ -10,13 +15,10 @@
         "type": "prometheus",
         "metricName": "node_cpu",
         "aggregationPeriod": "60s",
-        "labelBindings": ["mode=~\"user|system\"", "job=\"node\""]
+        "labelBindings": ["mode=~\"user|system\"", "job=\"node\""],
         "sumByFields": ["mode"]
       },
       "analysisConfigurations": {
-        "canary": {
-          "judge": "dredd-v1.0"
-        }
       },
       "groups": [
         "system"

diff --git a/scratch/stackdriver_canary_config.json b/scratch/stackdriver_canary_config.json
@@ -2,6 +2,11 @@
   "name": "MySampleStackdriverCanaryConfig",
   "description": "Example Automated Canary Analysis (ACA) Configuration using Stackdriver",
   "configVersion": 1.0,
+  "judge": {
+    "name": "dredd-v1.0",
+    "judgeConfigurations": {
+    }
+  },
   "metrics": [
     {
       "serviceName": "stackdriver",
@@ -11,9 +16,6 @@
         "metricType": "compute.googleapis.com/instance/cpu/utilization"
       },
       "analysisConfigurations": {
-        "canary": {
-          "judge": "dredd-v1.0"
-        }
       },
       "groups": [
         "system"