fix(stackdriver): handle null timeSeries and empty points (#1047) (#1050

) (cherry picked from commit 6684f9d) Co-authored-by: Shlomo Daari <[email protected]>
spinnaker · Aug 8, 2024 · e231949 · e231949
1 parent 13fe623
commit e231949
Show file tree

Hide file tree

Showing 2 changed files with 145 additions and 17 deletions.
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
@@ -401,24 +401,34 @@ public List<MetricSet> queryMetrics(
       // TODO(duftler): What if there are no data points?
       List<Double> pointValues;
 
-      if (timeSeries.getValueType().equals("INT64")) {
-        pointValues =
-            points.stream()
-                .map(point -> (double) point.getValue().getInt64Value())
-                .collect(Collectors.toList());
-      } else if (timeSeries.getValueType().equals("DOUBLE")) {
-        pointValues =
-            points.stream()
-                .map(point -> point.getValue().getDoubleValue())
-                .collect(Collectors.toList());
+      if (points.isEmpty()) {
+        log.warn("No data points available.");
+        pointValues = Collections.emptyList();
       } else {
-        log.warn(
-            "expected timeSeries value type to be either DOUBLE or INT64. Got {}.",
-            timeSeries.getValueType());
-        pointValues =
-            points.stream()
-                .map(point -> point.getValue().getDoubleValue())
-                .collect(Collectors.toList());
+        if (timeSeries.getValueType() != null) {
+          if (timeSeries.getValueType().equals("INT64")) {
+            pointValues =
+                points.stream()
+                    .map(point -> (double) point.getValue().getInt64Value())
+                    .collect(Collectors.toList());
+          } else if (timeSeries.getValueType().equals("DOUBLE")) {
+            pointValues =
+                points.stream()
+                    .map(point -> point.getValue().getDoubleValue())
+                    .collect(Collectors.toList());
+          } else {
+            log.warn(
+                "Expected timeSeries value type to be either DOUBLE or INT64. Got {}.",
+                timeSeries.getValueType());
+            pointValues =
+                points.stream()
+                    .map(point -> point.getValue().getDoubleValue())
+                    .collect(Collectors.toList());
+          }
+        } else {
+          log.warn("timeSeries valueType is null.");
+          pointValues = Collections.emptyList(); // Handle null valueType case as well
+        }
       }
 
       MetricSet.MetricSetBuilder metricSetBuilder =

diff --git a/.../src/test/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsServiceTest.java b/.../src/test/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsServiceTest.java
@@ -25,6 +25,7 @@
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -151,4 +152,121 @@ void returnsSingleMetricDescriptorInCache() throws IOException {
 
     assertThat(metadata).containsOnly(exampleMetricDescriptor);
   }
+
+  @Test
+  void handlesEmptyResponse() throws IOException {
+    GoogleNamedAccountCredentials stackdriverCredentialsMock =
+        mock(GoogleNamedAccountCredentials.class);
+    when(accountCredentialsRepoMock.getRequiredOne(ACCOUNT)).thenReturn(stackdriverCredentialsMock);
+
+    Monitoring monitoringMock = mock(Monitoring.class, Mockito.RETURNS_DEEP_STUBS);
+    when(stackdriverCredentialsMock.getMonitoring()).thenReturn(monitoringMock);
+
+    Monitoring.Projects.TimeSeries.List timeSeriesListMock =
+        mock(Monitoring.Projects.TimeSeries.List.class);
+    when(monitoringMock
+            .projects()
+            .timeSeries()
+            .list(anyString())
+            .setAggregationAlignmentPeriod(anyString())
+            .setAggregationCrossSeriesReducer(anyString())
+            .setAggregationPerSeriesAligner(anyString())
+            .setFilter(anyString())
+            .setIntervalStartTime(anyString())
+            .setIntervalEndTime(anyString()))
+        .thenReturn(timeSeriesListMock);
+
+    ListTimeSeriesResponse responseMock = mock(ListTimeSeriesResponse.class);
+    when(timeSeriesListMock.execute()).thenReturn(responseMock);
+
+    // Return an empty list for time series
+    when(responseMock.getTimeSeries()).thenReturn(Collections.emptyList());
+
+    CanaryConfig canaryConfig = new CanaryConfig();
+    CanaryMetricConfig canaryMetricConfig =
+        CanaryMetricConfig.builder()
+            .name("metricConfig")
+            .query(
+                StackdriverCanaryMetricSetQueryConfig.builder()
+                    .resourceType("global")
+                    .metricType("instance")
+                    .build())
+            .build();
+
+    StackdriverCanaryScope canaryScope = new StackdriverCanaryScope();
+    canaryScope.setStart(Instant.EPOCH).setEnd(Instant.EPOCH.plusSeconds(1)).setStep(1l);
+    canaryScope.setProject("my-project");
+    List<MetricSet> queriedMetrics =
+        stackdriverMetricsService.queryMetrics(
+            ACCOUNT, canaryConfig, canaryMetricConfig, canaryScope);
+
+    // Verify that an empty metric set is returned
+    assertThat(queriedMetrics).hasSize(1);
+    assertThat(queriedMetrics.get(0).getValues()).isEmpty();
+  }
+
+  @Test
+  void handlesInvalidMetricType() throws IOException {
+    GoogleNamedAccountCredentials stackdriverCredentialsMock =
+        mock(GoogleNamedAccountCredentials.class);
+    when(accountCredentialsRepoMock.getRequiredOne(ACCOUNT)).thenReturn(stackdriverCredentialsMock);
+
+    Monitoring monitoringMock = mock(Monitoring.class, Mockito.RETURNS_DEEP_STUBS);
+    when(stackdriverCredentialsMock.getMonitoring()).thenReturn(monitoringMock);
+
+    Monitoring.Projects.TimeSeries.List timeSeriesListMock =
+        mock(Monitoring.Projects.TimeSeries.List.class);
+    when(monitoringMock
+            .projects()
+            .timeSeries()
+            .list(anyString())
+            .setAggregationAlignmentPeriod(anyString())
+            .setAggregationCrossSeriesReducer(anyString())
+            .setAggregationPerSeriesAligner(anyString())
+            .setFilter(anyString())
+            .setIntervalStartTime(anyString())
+            .setIntervalEndTime(anyString()))
+        .thenReturn(timeSeriesListMock);
+
+    ListTimeSeriesResponse responseMock = mock(ListTimeSeriesResponse.class);
+    when(timeSeriesListMock.execute()).thenReturn(responseMock);
+
+    List<TimeSeries> timeSeriesListWithInvalidMetricType = new ArrayList<>();
+
+    // Create a time series with an invalid metric type
+    List<Point> points = new ArrayList<>();
+    points.add(
+        new Point()
+            .setValue(new TypedValue().setDoubleValue(3.14))
+            .setInterval(
+                new TimeInterval()
+                    .setStartTime("1970-01-01T00:00:00.00Z")
+                    .setEndTime("1970-01-01T00:00:01.00Z")));
+    TimeSeries timeSeriesWithInvalidMetricType =
+        new TimeSeries().setValueType("STRING").setPoints(points);
+    timeSeriesListWithInvalidMetricType.add(timeSeriesWithInvalidMetricType);
+
+    when(responseMock.getTimeSeries()).thenReturn(timeSeriesListWithInvalidMetricType);
+
+    CanaryConfig canaryConfig = new CanaryConfig();
+    CanaryMetricConfig canaryMetricConfig =
+        CanaryMetricConfig.builder()
+            .name("metricConfig")
+            .query(
+                StackdriverCanaryMetricSetQueryConfig.builder()
+                    .resourceType("global")
+                    .metricType("instance")
+                    .build())
+            .build();
+
+    StackdriverCanaryScope canaryScope = new StackdriverCanaryScope();
+    canaryScope.setStart(Instant.EPOCH).setEnd(Instant.EPOCH.plusSeconds(1)).setStep(1l);
+    canaryScope.setProject("my-project");
+    List<MetricSet> queriedMetrics =
+        stackdriverMetricsService.queryMetrics(
+            ACCOUNT, canaryConfig, canaryMetricConfig, canaryScope);
+
+    // Verify that the values are extracted as Double
+    assertThat(queriedMetrics.get(0).getValues()).contains(3.14);
+  }
 }