fix(stackdriver): Improve query-building so it works for all types of…

… stackdriver metrics. (#128)
spinnaker · Nov 16, 2017 · 1301a7c · 1301a7c
1 parent df7afe2
commit 1301a7c
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 14 deletions.
diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/canary/AtlasCanaryScope.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/canary/AtlasCanaryScope.java
@@ -37,9 +37,6 @@ public class AtlasCanaryScope extends CanaryScope {
   @NotNull
   private String dataset;
 
-  @NotNull
-  private String region;
-
   @NotNull
   private String environment;
 

diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/canary/AtlasCanaryScopeFactory.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/canary/AtlasCanaryScopeFactory.java
@@ -35,6 +35,7 @@ public boolean handles(String serviceType) {
   public CanaryScope buildCanaryScope(CanaryScope canaryScope){
     AtlasCanaryScope atlasCanaryScope = new AtlasCanaryScope();
     atlasCanaryScope.setScope(canaryScope.getScope());
+    atlasCanaryScope.setRegion(canaryScope.getRegion());
     atlasCanaryScope.setStart(canaryScope.getStart());
     atlasCanaryScope.setEnd(canaryScope.getEnd());
     atlasCanaryScope.setStep(canaryScope.getStep());
@@ -48,7 +49,6 @@ public CanaryScope buildCanaryScope(CanaryScope canaryScope){
     atlasCanaryScope.setType(extendedScopeParams.getOrDefault("type", "cluster"));
     atlasCanaryScope.setDeployment(extendedScopeParams.getOrDefault("deployment", "main"));
     atlasCanaryScope.setDataset(extendedScopeParams.getOrDefault("dataset", "regional"));
-    atlasCanaryScope.setRegion(extendedScopeParams.getOrDefault("region", "us-east-1"));
     atlasCanaryScope.setEnvironment(extendedScopeParams.getOrDefault("environment", "test"));
 
     return atlasCanaryScope;

diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/controllers/AtlasFetchController.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/controllers/AtlasFetchController.java
@@ -54,6 +54,7 @@ public Map queryMetrics(@RequestParam(required = false) final String metricsAcco
                           @ApiParam(defaultValue = "cpu") @RequestParam String metricSetName,
                           @ApiParam(defaultValue = "cluster") @RequestParam String type,
                           @RequestParam String scope,
+                          @ApiParam(defaultValue = "us-east-1") @RequestParam String region,
                           @ApiParam(defaultValue = "2000-01-01T00:00:00Z") @RequestParam Instant start,
                           @ApiParam(defaultValue = "2000-01-01T04:00:00Z") @RequestParam Instant end,
                           @ApiParam(defaultValue = "300") @RequestParam Long step) throws IOException {
@@ -79,6 +80,7 @@ public Map queryMetrics(@RequestParam(required = false) final String metricsAcco
     AtlasCanaryScope atlasCanaryScope = new AtlasCanaryScope();
     atlasCanaryScope.setType(type);
     atlasCanaryScope.setScope(scope);
+    atlasCanaryScope.setRegion(region);
     atlasCanaryScope.setStart(start);
     atlasCanaryScope.setEnd(end);
     atlasCanaryScope.setStep(step);

diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryScope.java b/kayenta-core/src/main/java/com/netflix/kayenta/canary/CanaryScope.java
@@ -31,6 +31,9 @@ public class CanaryScope {
   @NotNull
   protected String scope;
 
+  @NotNull
+  private String region;
+
   @NotNull
   protected Instant start;
 

diff --git a/...s/src/main/java/com/netflix/kayenta/prometheus/controllers/PrometheusFetchController.java b/...s/src/main/java/com/netflix/kayenta/prometheus/controllers/PrometheusFetchController.java
@@ -83,7 +83,7 @@ public Map queryMetrics(@RequestParam(required = false) final String metricsAcco
         .query(prometheusCanaryMetricSetQueryConfig)
         .build();
 
-    CanaryScope canaryScope = new CanaryScope(scope, start, end, step, Collections.emptyMap());
+    CanaryScope canaryScope = new CanaryScope(scope, null /* region */, start, end, step, Collections.emptyMap());
 
     String metricSetListId = synchronousQueryProcessor.processQuery(resolvedMetricsAccountName,
                                                                     resolvedStorageAccountName,

diff --git a/...src/main/java/com/netflix/kayenta/stackdriver/controllers/StackdriverFetchController.java b/...src/main/java/com/netflix/kayenta/stackdriver/controllers/StackdriverFetchController.java
@@ -55,6 +55,7 @@ public Map queryMetrics(@RequestParam(required = false) final String metricsAcco
                           @ApiParam(defaultValue = "compute.googleapis.com/instance/cpu/utilization") @RequestParam String metricType,
                           @RequestParam(required = false) List<String> groupByFields, // metric.label.instance_name
                           @ApiParam(defaultValue = "myapp-v010-") @RequestParam String scope,
+                          @ApiParam(defaultValue = "us-central1") @RequestParam String region,
                           @ApiParam(defaultValue = "2017-10-01T15:13:00Z") @RequestParam Instant startTimeIso,
                           @ApiParam(defaultValue = "2017-10-02T15:27:00Z") @RequestParam Instant endTimeIso,
                           @ApiParam(defaultValue = "3600") @RequestParam Long step) throws IOException {
@@ -83,6 +84,7 @@ public Map queryMetrics(@RequestParam(required = false) final String metricsAcco
 
     CanaryScope canaryScope = new CanaryScope();
     canaryScope.setScope(scope);
+    canaryScope.setRegion(region);
     canaryScope.setStart(startTimeIso);
     canaryScope.setEnd(endTimeIso);
     canaryScope.setStep(step);

diff --git a/...iver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java b/...iver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java
@@ -19,6 +19,7 @@
 import com.google.api.services.monitoring.v3.Monitoring;
 import com.google.api.services.monitoring.v3.model.ListTimeSeriesResponse;
 import com.google.api.services.monitoring.v3.model.Metric;
+import com.google.api.services.monitoring.v3.model.MonitoredResource;
 import com.google.api.services.monitoring.v3.model.Point;
 import com.google.api.services.monitoring.v3.model.TimeSeries;
 import com.netflix.kayenta.canary.CanaryMetricConfig;
@@ -69,22 +70,26 @@ public boolean servicesAccount(String accountName) {
   public List<MetricSet> queryMetrics(String metricsAccountName,
                                       CanaryMetricConfig canaryMetricConfig,
                                       CanaryScope canaryScope) throws IOException {
-    GoogleNamedAccountCredentials credentials = (GoogleNamedAccountCredentials)accountCredentialsRepository
+    GoogleNamedAccountCredentials stackdriverCredentials = (GoogleNamedAccountCredentials)accountCredentialsRepository
       .getOne(metricsAccountName)
       .orElseThrow(() -> new IllegalArgumentException("Unable to resolve account " + metricsAccountName + "."));
-    Monitoring monitoring = credentials.getMonitoring();
+    Monitoring monitoring = stackdriverCredentials.getMonitoring();
     StackdriverCanaryMetricSetQueryConfig stackdriverMetricSetQuery = (StackdriverCanaryMetricSetQueryConfig)canaryMetricConfig.getQuery();
+    // TODO(duftler): Make this filter general-purpose so it works for more than just GCE.
+    // TODO(duftler): The 'project' attribute should be part of the scope (a StackdriverCanaryScope), and not just re-used from stackdriverCredentials.
+    String filter = "metric.type=\"" + stackdriverMetricSetQuery.getMetricType() + "\"" +
+                    " AND resource.metadata.tag.spinnaker-member-of=" + stackdriverCredentials.getProject() + "_" + canaryScope.getRegion() + "_" + canaryScope.getScope() +
+                    " AND resource.type = gce_instance";
     long alignmentPeriodSec = canaryScope.getStep();
     Monitoring.Projects.TimeSeries.List list = monitoring
       .projects()
       .timeSeries()
-      .list("projects/" + credentials.getProject())
+      .list("projects/" + stackdriverCredentials.getProject())
       .setAggregationAlignmentPeriod(alignmentPeriodSec + "s")
       .setAggregationCrossSeriesReducer("REDUCE_MEAN")
       .setAggregationPerSeriesAligner("ALIGN_MEAN")
-      // TODO(duftler): Support 'filter' directly on StackdriverMetricSetQuery?
-      .setFilter("metric.type=\"" + stackdriverMetricSetQuery.getMetricType() + "\" AND " +
-                 "metric.label.instance_name=starts_with(\"" + canaryScope.getScope() + "\")")
+      // TODO(duftler): Support 'filter' directly on StackdriverCanaryMetricSetQueryConfig?
+      .setFilter(filter)
       .setIntervalStartTime(canaryScope.getStart().toString())
       .setIntervalEndTime(canaryScope.getEnd().toString());
 
@@ -147,10 +152,14 @@ public List<MetricSet> queryMetrics(String metricsAccountName,
           .stepMillis(alignmentPeriodSec * 1000)
           .values(pointValues);
 
-      Map<String, String> labels = timeSeries.getMetric().getLabels();
+      MonitoredResource monitoredResource = timeSeries.getResource();
 
-      if (labels != null) {
-        metricSetBuilder.tags(labels);
+      if (monitoredResource != null) {
+        Map<String, String> labels = monitoredResource.getLabels();
+
+        if (labels != null) {
+          metricSetBuilder.tags(labels);
+        }
       }
 
       metricSetList.add(metricSetBuilder.build());