From c75541ca7a923355aa651275b6c8b9229b27f1e5 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Fri, 23 Jun 2023 15:57:36 -0400 Subject: [PATCH 1/8] add spinnakerRetrofitErrorHandler and replace RetrofitError catch blocks with SpinnakerServerException, etc --- kayenta-atlas/kayenta-atlas.gradle | 2 + .../atlas/backends/AtlasStorageUpdater.java | 15 +++-- .../atlas/backends/BackendUpdater.java | 15 +++-- kayenta-core/kayenta-core.gradle | 1 + .../metrics/SynchronousQueryProcessor.java | 58 +++++++++---------- .../config/RetrofitClientFactory.java | 2 + .../SynchronousQueryProcessorTest.java | 2 + .../storage/ConfigBinStorageService.java | 8 ++- .../service/PrometheusRemoteServiceTest.java | 4 +- .../metrics/SignalFxMetricsService.java | 7 ++- 10 files changed, 63 insertions(+), 51 deletions(-) diff --git a/kayenta-atlas/kayenta-atlas.gradle b/kayenta-atlas/kayenta-atlas.gradle index 6c8341aeb..9cfe81d9f 100644 --- a/kayenta-atlas/kayenta-atlas.gradle +++ b/kayenta-atlas/kayenta-atlas.gradle @@ -8,4 +8,6 @@ dependencies { api "com.squareup.okhttp:okhttp-apache" api "org.apache.commons:commons-io:1.3.2" + + api "io.spinnaker.kork:kork-retrofit" } diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java index 88381ba02..56bfb558b 100644 --- a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java +++ b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java @@ -20,6 +20,8 @@ import com.netflix.kayenta.atlas.service.AtlasStorageRemoteService; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.OkHttpClient; import java.util.Map; @@ -27,7 +29,6 @@ import lombok.Builder; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import retrofit.RetrofitError; import retrofit.converter.JacksonConverter; @Slf4j @@ -58,11 +59,13 @@ boolean run( Map> atlasStorageMap = AuthenticatedRequest.allowAnonymous(atlasStorageRemoteService::fetch); atlasStorageDatabase.update(atlasStorageMap); - } catch (RetrofitError e) { - log.warn("While fetching atlas backends from " + uri, e); - return succeededAtLeastOnce; + succeededAtLeastOnce = true; + } catch (SpinnakerHttpException e) { + log.warn(e.getResponseCode() + " error while fetching atlas backends from " + uri, e); + } catch (SpinnakerServerException e) { + log.warn("Error while fetching atlas backends from " + uri, e); } - succeededAtLeastOnce = true; - return true; + + return succeededAtLeastOnce; } } diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java index a6187ae04..0ce2bf9dd 100644 --- a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java +++ b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java @@ -20,6 +20,8 @@ import com.netflix.kayenta.atlas.service.BackendsRemoteService; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.OkHttpClient; import java.util.List; @@ -27,7 +29,6 @@ import lombok.Builder; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import retrofit.RetrofitError; import retrofit.converter.JacksonConverter; @Slf4j @@ -57,11 +58,13 @@ boolean run( try { List backends = AuthenticatedRequest.allowAnonymous(backendsRemoteService::fetch); backendDatabase.update(backends); - } catch (RetrofitError e) { - log.warn("While fetching atlas backends from " + uri, e); - return succeededAtLeastOnce; + succeededAtLeastOnce = true; + } catch (SpinnakerHttpException e) { + log.warn(e.getResponseCode() + " error while fetching atlas backends from " + uri, e); + } catch (SpinnakerServerException e) { + log.warn("Error while fetching atlas backends from " + uri, e); } - succeededAtLeastOnce = true; - return true; + + return succeededAtLeastOnce; } } diff --git a/kayenta-core/kayenta-core.gradle b/kayenta-core/kayenta-core.gradle index 04f05a640..992f08987 100644 --- a/kayenta-core/kayenta-core.gradle +++ b/kayenta-core/kayenta-core.gradle @@ -2,6 +2,7 @@ dependencies { api "redis.clients:jedis" api "io.spinnaker.kork:kork-core" api "io.spinnaker.kork:kork-web" + api "io.spinnaker.kork:kork-retrofit" api "com.netflix.spectator:spectator-api" // TODO update korkSwagger? diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index 326567cfc..6889162a7 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -24,6 +24,8 @@ import com.netflix.kayenta.storage.StorageServiceRepository; import com.netflix.spectator.api.Id; import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; import java.io.IOException; @@ -34,9 +36,7 @@ import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component @Slf4j @@ -88,10 +88,8 @@ public String executeQuery( metricsService.queryMetrics( metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); success = true; - } catch (RetrofitError e) { - - boolean retryable = isRetryable(e); - if (retryable) { + } catch (SpinnakerHttpException e) { + if (e.getRetryable()) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -101,18 +99,38 @@ public String executeQuery( Thread.sleep(backoffPeriod); } catch (InterruptedException ignored) { } - Object error = e.getResponse() != null ? e.getResponse().getStatus() : e.getCause(); log.warn( "Got {} result when querying for metrics. Retrying request (current attempt: " + "{}, max attempts: {}, last backoff period: {}ms)", - error, + e.getResponseCode(), retries, retryConfiguration.getAttempts(), backoffPeriod); } else { throw e; } - } catch (IOException | UncheckedIOException | RetryableQueryException e) { + } catch (SpinnakerNetworkException e) { + if (e.getRetryable()) { + retries++; + if (retries >= retryConfiguration.getAttempts()) { + throw e; + } + long backoffPeriod = getBackoffPeriodMs(retries); + try { + Thread.sleep(backoffPeriod); + } catch (InterruptedException ignored) { + } + log.warn( + "Got network error when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + retries, + retryConfiguration.getAttempts(), + backoffPeriod); + } else { + throw e; + } + } + catch (IOException | UncheckedIOException | RetryableQueryException e) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -146,28 +164,6 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { * retryConfiguration.getBackoffPeriodMultiplierMs(); } - private boolean isRetryable(RetrofitError e) { - if (isNetworkError(e)) { - // retry in case of network errors - return true; - } - if (e.getResponse() == null) { - // We don't have a network error, but the response is null. It's better to not retry these. - return false; - } - HttpStatus responseStatus = HttpStatus.resolve(e.getResponse().getStatus()); - if (responseStatus == null) { - return false; - } - return retryConfiguration.getStatuses().contains(responseStatus) - || retryConfiguration.getSeries().contains(responseStatus.series()); - } - - private boolean isNetworkError(RetrofitError e) { - return e.getKind() == RetrofitError.Kind.NETWORK - || (e.getResponse() == null && e.getCause() instanceof IOException); - } - public Map processQueryAndReturnMap( String metricsAccountName, String storageAccountName, diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java b/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java index e2c708422..1067fecd4 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.netflix.spinnaker.kork.annotations.VisibleForTesting; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler; import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger; import com.squareup.okhttp.Authenticator; import com.squareup.okhttp.Credentials; @@ -119,6 +120,7 @@ public T createClient( .setEndpoint(endpoint) .setClient(new OkClient(okHttpClient)) .setConverter(converter) + .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) .setLogLevel(RestAdapter.LogLevel.valueOf(retrofitLogLevel)) .setLog(logger) .build() diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java index 3dfe600f6..f262c911b 100644 --- a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java +++ b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java @@ -45,6 +45,8 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; + +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java index 81f4e38ff..3916f77ae 100644 --- a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java +++ b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java @@ -30,6 +30,9 @@ import com.netflix.kayenta.storage.ObjectType; import com.netflix.kayenta.storage.StorageService; import com.netflix.kayenta.util.Retry; +import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.MediaType; @@ -45,7 +48,6 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.util.StringUtils; -import retrofit.RetrofitError; @Builder @Slf4j @@ -87,7 +89,7 @@ public T loadObject(String accountName, ObjectType objectType, String object () -> remoteService.get(ownerApp, configType, objectKey), MAX_RETRIES, RETRY_BACKOFF)); - } catch (RetrofitError e) { + } catch (SpinnakerServerException e) { throw new NotFoundException("No such object named " + objectKey); } @@ -337,7 +339,7 @@ private Map metadataFor(ConfigBinNamedAccountCredentials credent () -> remoteService.get(ownerApp, configType, id), MAX_RETRIES, RETRY_BACKOFF)); - } catch (RetrofitError e) { + } catch (SpinnakerServerException e) { throw new IllegalArgumentException("No such object named " + id); } diff --git a/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java b/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java index 2e884f47f..1d1f93b00 100644 --- a/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java +++ b/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java @@ -27,6 +27,7 @@ import com.netflix.kayenta.prometheus.model.PrometheusResults; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.squareup.okhttp.OkHttpClient; import java.io.File; import java.nio.charset.StandardCharsets; @@ -42,7 +43,6 @@ import org.mockserver.client.MockServerClient; import org.mockserver.junit.MockServerRule; import org.mockserver.model.MediaType; -import retrofit.RetrofitError; public class PrometheusRemoteServiceTest { @Rule public MockServerRule mockServerRule = new MockServerRule(this, true); @@ -78,7 +78,7 @@ public void isHealthyReturnsInternalServerError() { .respond(response().withStatusCode(500)); assertThatThrownBy(() -> prometheusRemoteService.isHealthy()) - .isInstanceOf(RetrofitError.class) + .isInstanceOf(SpinnakerServerException.class) .hasMessageContaining("500 Internal Server Error"); } diff --git a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java index 9898c93be..ea5169ea9 100644 --- a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java +++ b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java @@ -35,6 +35,7 @@ import com.netflix.kayenta.signalfx.service.SignalFlowExecutionResult; import com.netflix.kayenta.signalfx.service.SignalFxRequestError; import com.netflix.kayenta.signalfx.service.SignalFxSignalFlowRemoteService; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.signalfx.signalflow.ChannelMessage; import java.time.Duration; import java.time.Instant; @@ -50,7 +51,6 @@ import lombok.Singular; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import retrofit.RetrofitError; @Builder @Slf4j @@ -144,8 +144,9 @@ public List queryMetrics( signalFlowExecutionResult = signalFlowService.executeSignalFlowProgram( accessToken, startEpochMilli, endEpochMilli, stepMilli, maxDelay, immediate, program); - } catch (RetrofitError e) { - ErrorResponse errorResponse = (ErrorResponse) e.getBodyAs(ErrorResponse.class); + } catch (SpinnakerServerException e) { + ErrorResponse errorResponse = new ErrorResponse(); + errorResponse.setMessage(e.getMessage()); throw new SignalFxRequestError( errorResponse, program, startEpochMilli, endEpochMilli, stepMilli, metricsAccountName); } From 02c54b085088f0b1cd678e17401e607168807824 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Fri, 23 Jun 2023 15:57:36 -0400 Subject: [PATCH 2/8] feat(exceptions): add spinnakerRetrofitErrorHandler and replace RetrofitError catch blocks with SpinnakerServerException, etc --- kayenta-atlas/kayenta-atlas.gradle | 2 + .../atlas/backends/AtlasStorageUpdater.java | 15 +++-- .../atlas/backends/BackendUpdater.java | 15 +++-- kayenta-core/kayenta-core.gradle | 1 + .../metrics/SynchronousQueryProcessor.java | 58 +++++++++---------- .../config/RetrofitClientFactory.java | 2 + .../SynchronousQueryProcessorTest.java | 2 + .../storage/ConfigBinStorageService.java | 8 ++- .../service/PrometheusRemoteServiceTest.java | 4 +- .../metrics/SignalFxMetricsService.java | 7 ++- 10 files changed, 63 insertions(+), 51 deletions(-) diff --git a/kayenta-atlas/kayenta-atlas.gradle b/kayenta-atlas/kayenta-atlas.gradle index 6c8341aeb..9cfe81d9f 100644 --- a/kayenta-atlas/kayenta-atlas.gradle +++ b/kayenta-atlas/kayenta-atlas.gradle @@ -8,4 +8,6 @@ dependencies { api "com.squareup.okhttp:okhttp-apache" api "org.apache.commons:commons-io:1.3.2" + + api "io.spinnaker.kork:kork-retrofit" } diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java index 88381ba02..56bfb558b 100644 --- a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java +++ b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/AtlasStorageUpdater.java @@ -20,6 +20,8 @@ import com.netflix.kayenta.atlas.service.AtlasStorageRemoteService; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.OkHttpClient; import java.util.Map; @@ -27,7 +29,6 @@ import lombok.Builder; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import retrofit.RetrofitError; import retrofit.converter.JacksonConverter; @Slf4j @@ -58,11 +59,13 @@ boolean run( Map> atlasStorageMap = AuthenticatedRequest.allowAnonymous(atlasStorageRemoteService::fetch); atlasStorageDatabase.update(atlasStorageMap); - } catch (RetrofitError e) { - log.warn("While fetching atlas backends from " + uri, e); - return succeededAtLeastOnce; + succeededAtLeastOnce = true; + } catch (SpinnakerHttpException e) { + log.warn(e.getResponseCode() + " error while fetching atlas backends from " + uri, e); + } catch (SpinnakerServerException e) { + log.warn("Error while fetching atlas backends from " + uri, e); } - succeededAtLeastOnce = true; - return true; + + return succeededAtLeastOnce; } } diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java index a6187ae04..0ce2bf9dd 100644 --- a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java +++ b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/backends/BackendUpdater.java @@ -20,6 +20,8 @@ import com.netflix.kayenta.atlas.service.BackendsRemoteService; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.OkHttpClient; import java.util.List; @@ -27,7 +29,6 @@ import lombok.Builder; import lombok.Getter; import lombok.extern.slf4j.Slf4j; -import retrofit.RetrofitError; import retrofit.converter.JacksonConverter; @Slf4j @@ -57,11 +58,13 @@ boolean run( try { List backends = AuthenticatedRequest.allowAnonymous(backendsRemoteService::fetch); backendDatabase.update(backends); - } catch (RetrofitError e) { - log.warn("While fetching atlas backends from " + uri, e); - return succeededAtLeastOnce; + succeededAtLeastOnce = true; + } catch (SpinnakerHttpException e) { + log.warn(e.getResponseCode() + " error while fetching atlas backends from " + uri, e); + } catch (SpinnakerServerException e) { + log.warn("Error while fetching atlas backends from " + uri, e); } - succeededAtLeastOnce = true; - return true; + + return succeededAtLeastOnce; } } diff --git a/kayenta-core/kayenta-core.gradle b/kayenta-core/kayenta-core.gradle index 04f05a640..992f08987 100644 --- a/kayenta-core/kayenta-core.gradle +++ b/kayenta-core/kayenta-core.gradle @@ -2,6 +2,7 @@ dependencies { api "redis.clients:jedis" api "io.spinnaker.kork:kork-core" api "io.spinnaker.kork:kork-web" + api "io.spinnaker.kork:kork-retrofit" api "com.netflix.spectator:spectator-api" // TODO update korkSwagger? diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index 326567cfc..6889162a7 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -24,6 +24,8 @@ import com.netflix.kayenta.storage.StorageServiceRepository; import com.netflix.spectator.api.Id; import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; import java.io.IOException; @@ -34,9 +36,7 @@ import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component @Slf4j @@ -88,10 +88,8 @@ public String executeQuery( metricsService.queryMetrics( metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); success = true; - } catch (RetrofitError e) { - - boolean retryable = isRetryable(e); - if (retryable) { + } catch (SpinnakerHttpException e) { + if (e.getRetryable()) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -101,18 +99,38 @@ public String executeQuery( Thread.sleep(backoffPeriod); } catch (InterruptedException ignored) { } - Object error = e.getResponse() != null ? e.getResponse().getStatus() : e.getCause(); log.warn( "Got {} result when querying for metrics. Retrying request (current attempt: " + "{}, max attempts: {}, last backoff period: {}ms)", - error, + e.getResponseCode(), retries, retryConfiguration.getAttempts(), backoffPeriod); } else { throw e; } - } catch (IOException | UncheckedIOException | RetryableQueryException e) { + } catch (SpinnakerNetworkException e) { + if (e.getRetryable()) { + retries++; + if (retries >= retryConfiguration.getAttempts()) { + throw e; + } + long backoffPeriod = getBackoffPeriodMs(retries); + try { + Thread.sleep(backoffPeriod); + } catch (InterruptedException ignored) { + } + log.warn( + "Got network error when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + retries, + retryConfiguration.getAttempts(), + backoffPeriod); + } else { + throw e; + } + } + catch (IOException | UncheckedIOException | RetryableQueryException e) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -146,28 +164,6 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { * retryConfiguration.getBackoffPeriodMultiplierMs(); } - private boolean isRetryable(RetrofitError e) { - if (isNetworkError(e)) { - // retry in case of network errors - return true; - } - if (e.getResponse() == null) { - // We don't have a network error, but the response is null. It's better to not retry these. - return false; - } - HttpStatus responseStatus = HttpStatus.resolve(e.getResponse().getStatus()); - if (responseStatus == null) { - return false; - } - return retryConfiguration.getStatuses().contains(responseStatus) - || retryConfiguration.getSeries().contains(responseStatus.series()); - } - - private boolean isNetworkError(RetrofitError e) { - return e.getKind() == RetrofitError.Kind.NETWORK - || (e.getResponse() == null && e.getCause() instanceof IOException); - } - public Map processQueryAndReturnMap( String metricsAccountName, String storageAccountName, diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java b/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java index e2c708422..1067fecd4 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.netflix.spinnaker.kork.annotations.VisibleForTesting; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler; import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger; import com.squareup.okhttp.Authenticator; import com.squareup.okhttp.Credentials; @@ -119,6 +120,7 @@ public T createClient( .setEndpoint(endpoint) .setClient(new OkClient(okHttpClient)) .setConverter(converter) + .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) .setLogLevel(RestAdapter.LogLevel.valueOf(retrofitLogLevel)) .setLog(logger) .build() diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java index 3dfe600f6..f262c911b 100644 --- a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java +++ b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java @@ -45,6 +45,8 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; + +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java index 81f4e38ff..3916f77ae 100644 --- a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java +++ b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java @@ -30,6 +30,9 @@ import com.netflix.kayenta.storage.ObjectType; import com.netflix.kayenta.storage.StorageService; import com.netflix.kayenta.util.Retry; +import com.netflix.spinnaker.kork.exceptions.SpinnakerException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import com.netflix.spinnaker.security.AuthenticatedRequest; import com.squareup.okhttp.MediaType; @@ -45,7 +48,6 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.util.StringUtils; -import retrofit.RetrofitError; @Builder @Slf4j @@ -87,7 +89,7 @@ public T loadObject(String accountName, ObjectType objectType, String object () -> remoteService.get(ownerApp, configType, objectKey), MAX_RETRIES, RETRY_BACKOFF)); - } catch (RetrofitError e) { + } catch (SpinnakerServerException e) { throw new NotFoundException("No such object named " + objectKey); } @@ -337,7 +339,7 @@ private Map metadataFor(ConfigBinNamedAccountCredentials credent () -> remoteService.get(ownerApp, configType, id), MAX_RETRIES, RETRY_BACKOFF)); - } catch (RetrofitError e) { + } catch (SpinnakerServerException e) { throw new IllegalArgumentException("No such object named " + id); } diff --git a/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java b/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java index 2e884f47f..1d1f93b00 100644 --- a/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java +++ b/kayenta-prometheus/src/test/java/com/netflix/kayenta/prometheus/service/PrometheusRemoteServiceTest.java @@ -27,6 +27,7 @@ import com.netflix.kayenta.prometheus.model.PrometheusResults; import com.netflix.kayenta.retrofit.config.RemoteService; import com.netflix.kayenta.retrofit.config.RetrofitClientFactory; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.squareup.okhttp.OkHttpClient; import java.io.File; import java.nio.charset.StandardCharsets; @@ -42,7 +43,6 @@ import org.mockserver.client.MockServerClient; import org.mockserver.junit.MockServerRule; import org.mockserver.model.MediaType; -import retrofit.RetrofitError; public class PrometheusRemoteServiceTest { @Rule public MockServerRule mockServerRule = new MockServerRule(this, true); @@ -78,7 +78,7 @@ public void isHealthyReturnsInternalServerError() { .respond(response().withStatusCode(500)); assertThatThrownBy(() -> prometheusRemoteService.isHealthy()) - .isInstanceOf(RetrofitError.class) + .isInstanceOf(SpinnakerServerException.class) .hasMessageContaining("500 Internal Server Error"); } diff --git a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java index 9898c93be..ea5169ea9 100644 --- a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java +++ b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java @@ -35,6 +35,7 @@ import com.netflix.kayenta.signalfx.service.SignalFlowExecutionResult; import com.netflix.kayenta.signalfx.service.SignalFxRequestError; import com.netflix.kayenta.signalfx.service.SignalFxSignalFlowRemoteService; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.signalfx.signalflow.ChannelMessage; import java.time.Duration; import java.time.Instant; @@ -50,7 +51,6 @@ import lombok.Singular; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import retrofit.RetrofitError; @Builder @Slf4j @@ -144,8 +144,9 @@ public List queryMetrics( signalFlowExecutionResult = signalFlowService.executeSignalFlowProgram( accessToken, startEpochMilli, endEpochMilli, stepMilli, maxDelay, immediate, program); - } catch (RetrofitError e) { - ErrorResponse errorResponse = (ErrorResponse) e.getBodyAs(ErrorResponse.class); + } catch (SpinnakerServerException e) { + ErrorResponse errorResponse = new ErrorResponse(); + errorResponse.setMessage(e.getMessage()); throw new SignalFxRequestError( errorResponse, program, startEpochMilli, endEpochMilli, stepMilli, metricsAccountName); } From 8cddfdfbc4f8c0bd56d8d5f888146c28374339c0 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Thu, 29 Jun 2023 21:02:39 -0400 Subject: [PATCH 3/8] revert reduction in functionality for SignalFxMetricsService + other minor changes --- kayenta-atlas/kayenta-atlas.gradle | 2 +- .../configbin/storage/ConfigBinStorageService.java | 2 +- .../kayenta/signalfx/metrics/SignalFxMetricsService.java | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/kayenta-atlas/kayenta-atlas.gradle b/kayenta-atlas/kayenta-atlas.gradle index 9cfe81d9f..0b018e772 100644 --- a/kayenta-atlas/kayenta-atlas.gradle +++ b/kayenta-atlas/kayenta-atlas.gradle @@ -9,5 +9,5 @@ dependencies { api "org.apache.commons:commons-io:1.3.2" - api "io.spinnaker.kork:kork-retrofit" + implementation "io.spinnaker.kork:kork-retrofit" } diff --git a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java index 3916f77ae..555614ab9 100644 --- a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java +++ b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java @@ -90,7 +90,7 @@ public T loadObject(String accountName, ObjectType objectType, String object MAX_RETRIES, RETRY_BACKOFF)); } catch (SpinnakerServerException e) { - throw new NotFoundException("No such object named " + objectKey); + throw e.newInstance("No such object named " + objectKey); } try { diff --git a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java index ea5169ea9..8074fd50d 100644 --- a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java +++ b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java @@ -35,7 +35,6 @@ import com.netflix.kayenta.signalfx.service.SignalFlowExecutionResult; import com.netflix.kayenta.signalfx.service.SignalFxRequestError; import com.netflix.kayenta.signalfx.service.SignalFxSignalFlowRemoteService; -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.signalfx.signalflow.ChannelMessage; import java.time.Duration; import java.time.Instant; @@ -51,6 +50,7 @@ import lombok.Singular; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; +import retrofit.RetrofitError; @Builder @Slf4j @@ -144,9 +144,10 @@ public List queryMetrics( signalFlowExecutionResult = signalFlowService.executeSignalFlowProgram( accessToken, startEpochMilli, endEpochMilli, stepMilli, maxDelay, immediate, program); - } catch (SpinnakerServerException e) { - ErrorResponse errorResponse = new ErrorResponse(); - errorResponse.setMessage(e.getMessage()); + //TODO: replace with SpinnakerServerException/SpinnakerHttpException + // once an accessor for json response bodies as a Map is available + } catch (RetrofitError e) { + ErrorResponse errorResponse = (ErrorResponse) e.getBodyAs(ErrorResponse.class); throw new SignalFxRequestError( errorResponse, program, startEpochMilli, endEpochMilli, stepMilli, metricsAccountName); } From da701fa535565902d19279e5cb9994f7563cb6ff Mon Sep 17 00:00:00 2001 From: abe garcia Date: Wed, 19 Jul 2023 12:49:33 -0400 Subject: [PATCH 4/8] feat(exceptions): replace retrofit error in kayenta-signalfx --- gradle.properties | 2 +- .../kayenta/signalfx/metrics/SignalFxMetricsService.java | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/gradle.properties b/gradle.properties index ad981ec70..a7e36c155 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -orcaVersion=8.33.0 +orcaVersion=8.35.0 org.gradle.parallel=true spinnakerGradleVersion=8.27.0 targetJava11=true diff --git a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java index 8074fd50d..c449c07ff 100644 --- a/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java +++ b/kayenta-signalfx/src/main/java/com/netflix/kayenta/signalfx/metrics/SignalFxMetricsService.java @@ -35,6 +35,7 @@ import com.netflix.kayenta.signalfx.service.SignalFlowExecutionResult; import com.netflix.kayenta.signalfx.service.SignalFxRequestError; import com.netflix.kayenta.signalfx.service.SignalFxSignalFlowRemoteService; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.signalfx.signalflow.ChannelMessage; import java.time.Duration; import java.time.Instant; @@ -50,7 +51,6 @@ import lombok.Singular; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; -import retrofit.RetrofitError; @Builder @Slf4j @@ -144,10 +144,8 @@ public List queryMetrics( signalFlowExecutionResult = signalFlowService.executeSignalFlowProgram( accessToken, startEpochMilli, endEpochMilli, stepMilli, maxDelay, immediate, program); - //TODO: replace with SpinnakerServerException/SpinnakerHttpException - // once an accessor for json response bodies as a Map is available - } catch (RetrofitError e) { - ErrorResponse errorResponse = (ErrorResponse) e.getBodyAs(ErrorResponse.class); + } catch (SpinnakerHttpException e) { + ErrorResponse errorResponse = (ErrorResponse) e.getResponseBody(); throw new SignalFxRequestError( errorResponse, program, startEpochMilli, endEpochMilli, stepMilli, metricsAccountName); } From 40c86bcfd2feab1ec6f2260ee9fc62837bf74397 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Thu, 3 Aug 2023 12:44:37 -0400 Subject: [PATCH 5/8] fix format violations --- .../kayenta/metrics/SynchronousQueryProcessor.java | 13 ++++++------- .../metrics/SynchronousQueryProcessorTest.java | 2 -- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index 6889162a7..ebb212ae6 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -121,16 +121,15 @@ public String executeQuery( } catch (InterruptedException ignored) { } log.warn( - "Got network error when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - retries, - retryConfiguration.getAttempts(), - backoffPeriod); + "Got network error when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + retries, + retryConfiguration.getAttempts(), + backoffPeriod); } else { throw e; } - } - catch (IOException | UncheckedIOException | RetryableQueryException e) { + } catch (IOException | UncheckedIOException | RetryableQueryException e) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java index f262c911b..3dfe600f6 100644 --- a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java +++ b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java @@ -45,8 +45,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; - -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; From 85206bcb3f662f01076a8f67c6c17b6c46f8cc32 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Sun, 6 Aug 2023 14:33:20 -0400 Subject: [PATCH 6/8] fix failing tests and add missing cause for IllegalArgumentException in ConfigBinStorageService.java --- .../metrics/SynchronousQueryProcessor.java | 163 ++++++++++-------- .../SynchronousQueryProcessorTest.java | 33 ++-- .../storage/ConfigBinStorageService.java | 4 +- 3 files changed, 107 insertions(+), 93 deletions(-) diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index ebb212ae6..83784402a 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -26,6 +26,7 @@ import com.netflix.spectator.api.Registry; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; import java.io.IOException; @@ -36,7 +37,9 @@ import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; +import retrofit.RetrofitError; @Component @Slf4j @@ -48,10 +51,10 @@ public class SynchronousQueryProcessor { @Autowired public SynchronousQueryProcessor( - MetricsServiceRepository metricsServiceRepository, - StorageServiceRepository storageServiceRepository, - Registry registry, - MetricsRetryConfigurationProperties retryConfiguration) { + MetricsServiceRepository metricsServiceRepository, + StorageServiceRepository storageServiceRepository, + Registry registry, + MetricsRetryConfigurationProperties retryConfiguration) { this.metricsServiceRepository = metricsServiceRepository; this.storageServiceRepository = storageServiceRepository; this.registry = registry; @@ -59,20 +62,20 @@ public SynchronousQueryProcessor( } public String executeQuery( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - int metricIndex, - CanaryScope canaryScope) - throws IOException { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + int metricIndex, + CanaryScope canaryScope) + throws IOException { MetricsService metricsService = metricsServiceRepository.getRequiredOne(metricsAccountName); StorageService storageService = storageServiceRepository.getRequiredOne(storageAccountName); Id queryId = - registry - .createId("canary.telemetry.query") - .withTag("metricsStore", metricsService.getType()); + registry + .createId("canary.telemetry.query") + .withTag("metricsStore", metricsService.getType()); CanaryMetricConfig canaryMetricConfig = canaryConfig.getMetrics().get(metricIndex); List metricSetList = null; @@ -85,32 +88,31 @@ public String executeQuery( try { registry.counter(queryId.withTag("retries", retries + "")).increment(); metricSetList = - metricsService.queryMetrics( - metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); + metricsService.queryMetrics( + metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); success = true; - } catch (SpinnakerHttpException e) { - if (e.getRetryable()) { - retries++; - if (retries >= retryConfiguration.getAttempts()) { - throw e; - } - long backoffPeriod = getBackoffPeriodMs(retries); - try { - Thread.sleep(backoffPeriod); - } catch (InterruptedException ignored) { - } - log.warn( - "Got {} result when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - e.getResponseCode(), - retries, - retryConfiguration.getAttempts(), - backoffPeriod); - } else { + } catch(SpinnakerNetworkException e) { + retries++; + if (retries >= retryConfiguration.getAttempts()) { throw e; } - } catch (SpinnakerNetworkException e) { - if (e.getRetryable()) { + long backoffPeriod = getBackoffPeriodMs(retries); + try { + Thread.sleep(backoffPeriod); + } catch (InterruptedException ignored) { + } + Object error = e.getCause(); + log.warn( + "Got {} result when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + error, + retries, + retryConfiguration.getAttempts(), + backoffPeriod); + } + catch (SpinnakerHttpException e) { + boolean retryable = isRetryable(e); + if (retryable) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -120,12 +122,14 @@ public String executeQuery( Thread.sleep(backoffPeriod); } catch (InterruptedException ignored) { } + Object error = e.getResponseCode(); log.warn( - "Got network error when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - retries, - retryConfiguration.getAttempts(), - backoffPeriod); + "Got {} result when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + error, + retries, + retryConfiguration.getAttempts(), + backoffPeriod); } else { throw e; } @@ -140,18 +144,18 @@ public String executeQuery( } catch (InterruptedException ignored) { } log.warn( - "Got error when querying for metrics. Retrying request (current attempt: {}, max " - + "attempts: {}, last backoff period: {}ms)", - retries, - retryConfiguration.getAttempts(), - backoffPeriod, - e); + "Got error when querying for metrics. Retrying request (current attempt: {}, max " + + "attempts: {}, last backoff period: {}ms)", + retries, + retryConfiguration.getAttempts(), + backoffPeriod, + e); } } String metricSetListId = UUID.randomUUID() + ""; storageService.storeObject( - storageAccountName, ObjectType.METRIC_SET_LIST, metricSetListId, metricSetList); + storageAccountName, ObjectType.METRIC_SET_LIST, metricSetListId, metricSetList); return metricSetListId; } @@ -160,18 +164,27 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { // The retries range from 1..max, but we want the backoff periods to range from Math.pow(2, // 0)..Math.pow(2, max-1). return (long) Math.pow(2, (retryAttemptNumber - 1)) - * retryConfiguration.getBackoffPeriodMultiplierMs(); + * retryConfiguration.getBackoffPeriodMultiplierMs(); + } + + private boolean isRetryable(SpinnakerHttpException e) { + HttpStatus responseStatus = HttpStatus.resolve(e.getResponseCode()); + if (responseStatus == null) { + return false; + } + return retryConfiguration.getStatuses().contains(responseStatus) + || retryConfiguration.getSeries().contains(responseStatus.series()); } public Map processQueryAndReturnMap( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - CanaryMetricConfig canaryMetricConfig, - int metricIndex, - CanaryScope canaryScope, - boolean dryRun) - throws IOException { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + CanaryMetricConfig canaryMetricConfig, + int metricIndex, + CanaryScope canaryScope, + boolean dryRun) + throws IOException { if (canaryConfig == null) { canaryConfig = CanaryConfig.builder().metric(canaryMetricConfig).build(); } @@ -180,35 +193,35 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { MetricsService metricsService = metricsServiceRepository.getRequiredOne(metricsAccountName); String query = - metricsService.buildQuery( - metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); + metricsService.buildQuery( + metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); return Collections.singletonMap("query", query); } else { String metricSetListId = - executeQuery( - metricsAccountName, storageAccountName, canaryConfig, metricIndex, canaryScope); + executeQuery( + metricsAccountName, storageAccountName, canaryConfig, metricIndex, canaryScope); return Collections.singletonMap("metricSetListId", metricSetListId); } } public TaskResult executeQueryAndProduceTaskResult( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - int metricIndex, - CanaryScope canaryScope) { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + int metricIndex, + CanaryScope canaryScope) { try { Map outputs = - processQueryAndReturnMap( - metricsAccountName, - storageAccountName, - canaryConfig, - null /* canaryMetricConfig */, - metricIndex, - canaryScope, - false /* dryRun */); + processQueryAndReturnMap( + metricsAccountName, + storageAccountName, + canaryConfig, + null /* canaryMetricConfig */, + metricIndex, + canaryScope, + false /* dryRun */); return TaskResult.builder(ExecutionStatus.SUCCEEDED).outputs(outputs).build(); @@ -216,4 +229,4 @@ public TaskResult executeQueryAndProduceTaskResult( throw new RuntimeException(e); } } -} +} \ No newline at end of file diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java index 3dfe600f6..f674cf5f1 100644 --- a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java +++ b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java @@ -45,6 +45,9 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; + +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -94,7 +97,7 @@ public void retriesRetryableHttpSeriesTillMaxAttemptsAndThrowsException() throws any(CanaryConfig.class), any(CanaryMetricConfig.class), any(CanaryScope.class))) - .thenThrow(getRetrofitErrorWithHttpStatus(INTERNAL_SERVER_ERROR.value())); + .thenThrow(getSpinnakerHttpException(INTERNAL_SERVER_ERROR.value())); assertThatThrownBy( () -> @@ -104,7 +107,7 @@ public void retriesRetryableHttpSeriesTillMaxAttemptsAndThrowsException() throws mock(CanaryConfig.class, RETURNS_DEEP_STUBS), 1, mock(CanaryScope.class))) - .isInstanceOf(RetrofitError.class); + .isInstanceOf(SpinnakerHttpException.class); verify(metricsService, times(ATTEMPTS)) .queryMetrics( @@ -123,9 +126,9 @@ public void retriesRetryableHttpSeriesAndReturnsSuccessfulResponse() throws IOEx any(CanaryConfig.class), any(CanaryMetricConfig.class), any(CanaryScope.class))) - .thenThrow(getRetrofitErrorWithHttpStatus(INTERNAL_SERVER_ERROR.value())) - .thenThrow(getRetrofitErrorWithHttpStatus(BAD_GATEWAY.value())) - .thenThrow(getRetrofitErrorWithHttpStatus(HttpStatus.TEMPORARY_REDIRECT.value())) + .thenThrow(getSpinnakerHttpException(INTERNAL_SERVER_ERROR.value())) + .thenThrow(getSpinnakerHttpException(BAD_GATEWAY.value())) + .thenThrow(getSpinnakerHttpException(HttpStatus.TEMPORARY_REDIRECT.value())) .thenReturn(response); processor.executeQuery( @@ -149,9 +152,9 @@ public void retriesRetryableHttpStatusAndReturnsSuccessfulResponse() throws IOEx any(CanaryConfig.class), any(CanaryMetricConfig.class), any(CanaryScope.class))) - .thenThrow(getRetrofitErrorWithHttpStatus(LOCKED.value())) - .thenThrow(getRetrofitErrorWithHttpStatus(LOCKED.value())) - .thenThrow(getRetrofitErrorWithHttpStatus(LOCKED.value())) + .thenThrow(getSpinnakerHttpException(LOCKED.value())) + .thenThrow(getSpinnakerHttpException(LOCKED.value())) + .thenThrow(getSpinnakerHttpException(LOCKED.value())) .thenReturn(response); processor.executeQuery( @@ -174,7 +177,7 @@ public void doesNotRetryNonRetryableHttpStatusAndThrowsException() throws IOExce any(CanaryConfig.class), any(CanaryMetricConfig.class), any(CanaryScope.class))) - .thenThrow(getRetrofitErrorWithHttpStatus(BAD_REQUEST.value())); + .thenThrow(getSpinnakerHttpException(BAD_REQUEST.value())); assertThatThrownBy( () -> @@ -184,7 +187,7 @@ public void doesNotRetryNonRetryableHttpStatusAndThrowsException() throws IOExce mock(CanaryConfig.class, RETURNS_DEEP_STUBS), 1, mock(CanaryScope.class))) - .isInstanceOf(RetrofitError.class); + .isInstanceOf(SpinnakerHttpException.class); verify(metricsService, times(1)) .queryMetrics( @@ -230,7 +233,7 @@ public void retriesNetworkErrorTillMaxAttemptsAndThrowsException() throws IOExce any(CanaryConfig.class), any(CanaryMetricConfig.class), any(CanaryScope.class))) - .thenThrow(RetrofitError.networkError("url", new SocketTimeoutException())); + .thenThrow(new SpinnakerNetworkException(new SocketTimeoutException())); assertThatThrownBy( () -> @@ -240,7 +243,7 @@ public void retriesNetworkErrorTillMaxAttemptsAndThrowsException() throws IOExce mock(CanaryConfig.class, RETURNS_DEEP_STUBS), 1, mock(CanaryScope.class))) - .isInstanceOf(RetrofitError.class); + .isInstanceOf(SpinnakerNetworkException.class); verify(metricsService, times(ATTEMPTS)) .queryMetrics( @@ -251,8 +254,8 @@ public void retriesNetworkErrorTillMaxAttemptsAndThrowsException() throws IOExce verifyZeroInteractions(storageService); } - private RetrofitError getRetrofitErrorWithHttpStatus(int status) { - return RetrofitError.httpError( - "url", new Response("url", status, "reason", Collections.emptyList(), null), null, null); + private SpinnakerHttpException getSpinnakerHttpException(int status) { + return new SpinnakerHttpException(RetrofitError.httpError( + "url", new Response("url", status, "reason", Collections.emptyList(), null), null, null)); } } diff --git a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java index 555614ab9..db1606863 100644 --- a/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java +++ b/kayenta-objectstore-configbin/src/main/java/com/netflix/kayenta/configbin/storage/ConfigBinStorageService.java @@ -30,8 +30,6 @@ import com.netflix.kayenta.storage.ObjectType; import com.netflix.kayenta.storage.StorageService; import com.netflix.kayenta.util.Retry; -import com.netflix.spinnaker.kork.exceptions.SpinnakerException; -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; import com.netflix.spinnaker.security.AuthenticatedRequest; @@ -340,7 +338,7 @@ private Map metadataFor(ConfigBinNamedAccountCredentials credent MAX_RETRIES, RETRY_BACKOFF)); } catch (SpinnakerServerException e) { - throw new IllegalArgumentException("No such object named " + id); + throw new IllegalArgumentException("No such object named " + id, e); } CanaryConfig config; From 72591f71efda8c283ee93634b16fba858ba180d7 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Sun, 6 Aug 2023 14:35:47 -0400 Subject: [PATCH 7/8] kayenta-core format fixes --- .../metrics/SynchronousQueryProcessor.java | 131 +++++++++--------- .../SynchronousQueryProcessorTest.java | 13 +- 2 files changed, 72 insertions(+), 72 deletions(-) diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index 83784402a..0205b59dd 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -26,7 +26,6 @@ import com.netflix.spectator.api.Registry; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; import java.io.IOException; @@ -39,7 +38,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component @Slf4j @@ -51,10 +49,10 @@ public class SynchronousQueryProcessor { @Autowired public SynchronousQueryProcessor( - MetricsServiceRepository metricsServiceRepository, - StorageServiceRepository storageServiceRepository, - Registry registry, - MetricsRetryConfigurationProperties retryConfiguration) { + MetricsServiceRepository metricsServiceRepository, + StorageServiceRepository storageServiceRepository, + Registry registry, + MetricsRetryConfigurationProperties retryConfiguration) { this.metricsServiceRepository = metricsServiceRepository; this.storageServiceRepository = storageServiceRepository; this.registry = registry; @@ -62,20 +60,20 @@ public SynchronousQueryProcessor( } public String executeQuery( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - int metricIndex, - CanaryScope canaryScope) - throws IOException { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + int metricIndex, + CanaryScope canaryScope) + throws IOException { MetricsService metricsService = metricsServiceRepository.getRequiredOne(metricsAccountName); StorageService storageService = storageServiceRepository.getRequiredOne(storageAccountName); Id queryId = - registry - .createId("canary.telemetry.query") - .withTag("metricsStore", metricsService.getType()); + registry + .createId("canary.telemetry.query") + .withTag("metricsStore", metricsService.getType()); CanaryMetricConfig canaryMetricConfig = canaryConfig.getMetrics().get(metricIndex); List metricSetList = null; @@ -88,10 +86,10 @@ public String executeQuery( try { registry.counter(queryId.withTag("retries", retries + "")).increment(); metricSetList = - metricsService.queryMetrics( - metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); + metricsService.queryMetrics( + metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); success = true; - } catch(SpinnakerNetworkException e) { + } catch (SpinnakerNetworkException e) { retries++; if (retries >= retryConfiguration.getAttempts()) { throw e; @@ -103,14 +101,13 @@ public String executeQuery( } Object error = e.getCause(); log.warn( - "Got {} result when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - error, - retries, - retryConfiguration.getAttempts(), - backoffPeriod); - } - catch (SpinnakerHttpException e) { + "Got {} result when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + error, + retries, + retryConfiguration.getAttempts(), + backoffPeriod); + } catch (SpinnakerHttpException e) { boolean retryable = isRetryable(e); if (retryable) { retries++; @@ -124,12 +121,12 @@ public String executeQuery( } Object error = e.getResponseCode(); log.warn( - "Got {} result when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - error, - retries, - retryConfiguration.getAttempts(), - backoffPeriod); + "Got {} result when querying for metrics. Retrying request (current attempt: " + + "{}, max attempts: {}, last backoff period: {}ms)", + error, + retries, + retryConfiguration.getAttempts(), + backoffPeriod); } else { throw e; } @@ -144,18 +141,18 @@ public String executeQuery( } catch (InterruptedException ignored) { } log.warn( - "Got error when querying for metrics. Retrying request (current attempt: {}, max " - + "attempts: {}, last backoff period: {}ms)", - retries, - retryConfiguration.getAttempts(), - backoffPeriod, - e); + "Got error when querying for metrics. Retrying request (current attempt: {}, max " + + "attempts: {}, last backoff period: {}ms)", + retries, + retryConfiguration.getAttempts(), + backoffPeriod, + e); } } String metricSetListId = UUID.randomUUID() + ""; storageService.storeObject( - storageAccountName, ObjectType.METRIC_SET_LIST, metricSetListId, metricSetList); + storageAccountName, ObjectType.METRIC_SET_LIST, metricSetListId, metricSetList); return metricSetListId; } @@ -164,7 +161,7 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { // The retries range from 1..max, but we want the backoff periods to range from Math.pow(2, // 0)..Math.pow(2, max-1). return (long) Math.pow(2, (retryAttemptNumber - 1)) - * retryConfiguration.getBackoffPeriodMultiplierMs(); + * retryConfiguration.getBackoffPeriodMultiplierMs(); } private boolean isRetryable(SpinnakerHttpException e) { @@ -173,18 +170,18 @@ private boolean isRetryable(SpinnakerHttpException e) { return false; } return retryConfiguration.getStatuses().contains(responseStatus) - || retryConfiguration.getSeries().contains(responseStatus.series()); + || retryConfiguration.getSeries().contains(responseStatus.series()); } public Map processQueryAndReturnMap( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - CanaryMetricConfig canaryMetricConfig, - int metricIndex, - CanaryScope canaryScope, - boolean dryRun) - throws IOException { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + CanaryMetricConfig canaryMetricConfig, + int metricIndex, + CanaryScope canaryScope, + boolean dryRun) + throws IOException { if (canaryConfig == null) { canaryConfig = CanaryConfig.builder().metric(canaryMetricConfig).build(); } @@ -193,35 +190,35 @@ private boolean isRetryable(SpinnakerHttpException e) { MetricsService metricsService = metricsServiceRepository.getRequiredOne(metricsAccountName); String query = - metricsService.buildQuery( - metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); + metricsService.buildQuery( + metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); return Collections.singletonMap("query", query); } else { String metricSetListId = - executeQuery( - metricsAccountName, storageAccountName, canaryConfig, metricIndex, canaryScope); + executeQuery( + metricsAccountName, storageAccountName, canaryConfig, metricIndex, canaryScope); return Collections.singletonMap("metricSetListId", metricSetListId); } } public TaskResult executeQueryAndProduceTaskResult( - String metricsAccountName, - String storageAccountName, - CanaryConfig canaryConfig, - int metricIndex, - CanaryScope canaryScope) { + String metricsAccountName, + String storageAccountName, + CanaryConfig canaryConfig, + int metricIndex, + CanaryScope canaryScope) { try { Map outputs = - processQueryAndReturnMap( - metricsAccountName, - storageAccountName, - canaryConfig, - null /* canaryMetricConfig */, - metricIndex, - canaryScope, - false /* dryRun */); + processQueryAndReturnMap( + metricsAccountName, + storageAccountName, + canaryConfig, + null /* canaryMetricConfig */, + metricIndex, + canaryScope, + false /* dryRun */); return TaskResult.builder(ExecutionStatus.SUCCEEDED).outputs(outputs).build(); @@ -229,4 +226,4 @@ public TaskResult executeQueryAndProduceTaskResult( throw new RuntimeException(e); } } -} \ No newline at end of file +} diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java index f674cf5f1..194312381 100644 --- a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java +++ b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/SynchronousQueryProcessorTest.java @@ -39,15 +39,14 @@ import com.netflix.kayenta.storage.StorageService; import com.netflix.kayenta.storage.StorageServiceRepository; import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; import java.io.IOException; import java.net.SocketTimeoutException; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; - -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; -import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -255,7 +254,11 @@ public void retriesNetworkErrorTillMaxAttemptsAndThrowsException() throws IOExce } private SpinnakerHttpException getSpinnakerHttpException(int status) { - return new SpinnakerHttpException(RetrofitError.httpError( - "url", new Response("url", status, "reason", Collections.emptyList(), null), null, null)); + return new SpinnakerHttpException( + RetrofitError.httpError( + "url", + new Response("url", status, "reason", Collections.emptyList(), null), + null, + null)); } } From b7d2e1d8443f4abf539bd838331fec3f9a388573 Mon Sep 17 00:00:00 2001 From: abe garcia Date: Tue, 8 Aug 2023 11:37:06 -0400 Subject: [PATCH 8/8] catch SpinnakerServerException instead of both SpinnakerNetworkException and SpinnakerHttpException --- .../metrics/SynchronousQueryProcessor.java | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java index 0205b59dd..2127c7ea9 100644 --- a/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java +++ b/kayenta-core/src/main/java/com/netflix/kayenta/metrics/SynchronousQueryProcessor.java @@ -26,6 +26,7 @@ import com.netflix.spectator.api.Registry; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; import java.io.IOException; @@ -89,25 +90,7 @@ public String executeQuery( metricsService.queryMetrics( metricsAccountName, canaryConfig, canaryMetricConfig, canaryScope); success = true; - } catch (SpinnakerNetworkException e) { - retries++; - if (retries >= retryConfiguration.getAttempts()) { - throw e; - } - long backoffPeriod = getBackoffPeriodMs(retries); - try { - Thread.sleep(backoffPeriod); - } catch (InterruptedException ignored) { - } - Object error = e.getCause(); - log.warn( - "Got {} result when querying for metrics. Retrying request (current attempt: " - + "{}, max attempts: {}, last backoff period: {}ms)", - error, - retries, - retryConfiguration.getAttempts(), - backoffPeriod); - } catch (SpinnakerHttpException e) { + } catch (SpinnakerServerException e) { boolean retryable = isRetryable(e); if (retryable) { retries++; @@ -119,7 +102,13 @@ public String executeQuery( Thread.sleep(backoffPeriod); } catch (InterruptedException ignored) { } - Object error = e.getResponseCode(); + Object error; + if (isNetworkError(e)) { + error = e.getCause(); + } else { + SpinnakerHttpException httpError = (SpinnakerHttpException) e; + error = httpError.getResponseCode(); + } log.warn( "Got {} result when querying for metrics. Retrying request (current attempt: " + "{}, max attempts: {}, last backoff period: {}ms)", @@ -164,8 +153,12 @@ private long getBackoffPeriodMs(int retryAttemptNumber) { * retryConfiguration.getBackoffPeriodMultiplierMs(); } - private boolean isRetryable(SpinnakerHttpException e) { - HttpStatus responseStatus = HttpStatus.resolve(e.getResponseCode()); + private boolean isRetryable(SpinnakerServerException e) { + if (isNetworkError(e)) { + return true; + } + SpinnakerHttpException httpError = (SpinnakerHttpException) e; + HttpStatus responseStatus = HttpStatus.resolve(httpError.getResponseCode()); if (responseStatus == null) { return false; } @@ -173,6 +166,10 @@ private boolean isRetryable(SpinnakerHttpException e) { || retryConfiguration.getSeries().contains(responseStatus.series()); } + private boolean isNetworkError(SpinnakerServerException e) { + return e instanceof SpinnakerNetworkException || (e.getCause() instanceof IOException); + } + public Map processQueryAndReturnMap( String metricsAccountName, String storageAccountName,