From 3f81050272fed3adf90ddafe22be21107291b9be Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Tue, 6 Aug 2024 12:58:01 -0700 Subject: [PATCH] fix(stackdriver): handle null timeSeries and empty points --- .../metrics/StackdriverMetricsService.java | 44 ++++--- .../StackdriverMetricsServiceTest.java | 118 ++++++++++++++++++ 2 files changed, 145 insertions(+), 17 deletions(-) diff --git a/kayenta-stackdriver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java b/kayenta-stackdriver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java index 8502b6c4b..a8c2e672e 100644 --- a/kayenta-stackdriver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java +++ b/kayenta-stackdriver/src/main/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsService.java @@ -401,24 +401,34 @@ public List queryMetrics( // TODO(duftler): What if there are no data points? List 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/kayenta-stackdriver/src/test/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsServiceTest.java b/kayenta-stackdriver/src/test/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsServiceTest.java index e0b58eb07..a393e026f 100644 --- a/kayenta-stackdriver/src/test/java/com/netflix/kayenta/stackdriver/metrics/StackdriverMetricsServiceTest.java +++ b/kayenta-stackdriver/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 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 timeSeriesListWithInvalidMetricType = new ArrayList<>(); + + // Create a time series with an invalid metric type + List 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 queriedMetrics = + stackdriverMetricsService.queryMetrics( + ACCOUNT, canaryConfig, canaryMetricConfig, canaryScope); + + // Verify that the values are extracted as Double + assertThat(queriedMetrics.get(0).getValues()).contains(3.14); + } }