feat: Obey HTTP Status codes from upstream.

Previously we have treated any status code as equal. This PR changes that to allow 429 and 50x status codes to incrementally increase the interval between each time we call. And then gradually decrease once it starts succeeding. I'm a bit worried about the metrics here, because we drop the metric bucket on the floor if we don't get a 20x back from the server.
Unleash · Nov 7, 2023 · 9641c98 · 9641c98
1 parent 95bc59d
commit 9641c98
Show file tree

Hide file tree

Showing 11 changed files with 825 additions and 202 deletions.
diff --git a/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java b/src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java
@@ -37,26 +37,32 @@ public DefaultHttpMetricsSender(UnleashConfig unleashConfig) {
                         .create();
     }
 
-    public void registerClient(ClientRegistration registration) {
+    public int registerClient(ClientRegistration registration) {
         if (!unleashConfig.isDisableMetrics()) {
             try {
-                post(clientRegistrationURL, registration);
+                int statusCode = post(clientRegistrationURL, registration);
                 eventDispatcher.dispatch(registration);
+                return statusCode;
             } catch (UnleashException ex) {
                 eventDispatcher.dispatch(ex);
+                return -1;
             }
         }
+        return -1;
     }
 
-    public void sendMetrics(ClientMetrics metrics) {
+    public int sendMetrics(ClientMetrics metrics) {
         if (!unleashConfig.isDisableMetrics()) {
             try {
-                post(clientMetricsURL, metrics);
+                int statusCode = post(clientMetricsURL, metrics);
                 eventDispatcher.dispatch(metrics);
+                return statusCode;
             } catch (UnleashException ex) {
                 eventDispatcher.dispatch(ex);
+                return -1;
             }
         }
+        return -1;
     }
 
     private int post(URL url, Object o) throws UnleashException {

diff --git a/src/main/java/io/getunleash/metric/MetricSender.java b/src/main/java/io/getunleash/metric/MetricSender.java
@@ -1,7 +1,7 @@
 package io.getunleash.metric;
 
 public interface MetricSender {
-    void registerClient(ClientRegistration registration);
+    int registerClient(ClientRegistration registration);
 
-    void sendMetrics(ClientMetrics metrics);
+    int sendMetrics(ClientMetrics metrics);
 }
diff --git a/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java b/src/main/java/io/getunleash/metric/OkHttpMetricsSender.java
@@ -60,19 +60,21 @@ public OkHttpMetricsSender(UnleashConfig config) {
     }
 
     @Override
-    public void registerClient(ClientRegistration registration) {
+    public int registerClient(ClientRegistration registration) {
         if (!config.isDisableMetrics()) {
             try {
-                post(clientRegistrationUrl, registration);
+                int statusCode = post(clientRegistrationUrl, registration);
                 eventDispatcher.dispatch(registration);
+                return statusCode;
             } catch (UnleashException ex) {
                 eventDispatcher.dispatch(ex);
             }
         }
+        return -1;
     }
 
     @Override
-    public void sendMetrics(ClientMetrics metrics) {
+    public int sendMetrics(ClientMetrics metrics) {
         if (!config.isDisableMetrics()) {
             try {
                 post(clientMetricsUrl, metrics);
@@ -81,6 +83,7 @@ public void sendMetrics(ClientMetrics metrics) {
                 eventDispatcher.dispatch(ex);
             }
         }
+        return -1;
     }
 
     private int post(HttpUrl url, Object o) {

diff --git a/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java b/src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java
@@ -2,11 +2,17 @@
 
 import io.getunleash.util.UnleashConfig;
 import io.getunleash.util.UnleashScheduledExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.HttpURLConnection;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 
 public class UnleashMetricServiceImpl implements UnleashMetricService {
+    private static final Logger LOGGER = LoggerFactory.getLogger(UnleashMetricServiceImpl.class);
     private final LocalDateTime started;
     private final UnleashConfig unleashConfig;
 
@@ -15,6 +21,10 @@ public class UnleashMetricServiceImpl implements UnleashMetricService {
     // mutable
     private volatile MetricsBucket currentMetricsBucket;
 
+    private final int maxInterval;
+    private AtomicInteger failures = new AtomicInteger();
+    private AtomicInteger interval = new AtomicInteger();
+
     public UnleashMetricServiceImpl(
             UnleashConfig unleashConfig, UnleashScheduledExecutor executor) {
         this(unleashConfig, unleashConfig.getMetricSenderFactory().apply(unleashConfig), executor);
@@ -28,6 +38,7 @@ public UnleashMetricServiceImpl(
         this.started = LocalDateTime.now(ZoneId.of("UTC"));
         this.unleashConfig = unleashConfig;
         this.metricSender = metricSender;
+        this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getSendMetricsInterval()).intValue(), 1));
         long metricsInterval = unleashConfig.getSendMetricsInterval();
         executor.setInterval(sendMetrics(), metricsInterval, metricsInterval);
     }
@@ -51,11 +62,49 @@ public void countVariant(String toggleName, String variantName) {
 
     private Runnable sendMetrics() {
         return () -> {
-            MetricsBucket metricsBucket = this.currentMetricsBucket;
-            this.currentMetricsBucket = new MetricsBucket();
-            metricsBucket.end();
-            ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket);
-            metricSender.sendMetrics(metrics);
+            if (interval.get() == 0) {
+                MetricsBucket metricsBucket = this.currentMetricsBucket;
+                this.currentMetricsBucket = new MetricsBucket();
+                metricsBucket.end();
+                ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket);
+                int statusCode = metricSender.sendMetrics(metrics);
+                if (statusCode >= 200 && statusCode < 400) {
+                    if (failures.get() > 0) {
+                        interval.set(Integer.max(failures.decrementAndGet(), 0));
+                    }
+                }
+                if (statusCode >= 400) {
+                    handleHttpErrorCodes(statusCode);
+                }
+            } else {
+                interval.decrementAndGet();
+            }
         };
     }
+
+    private void handleHttpErrorCodes(int responseCode) {
+        if (responseCode == 404) {
+            interval.set(maxInterval);
+            failures.incrementAndGet();
+            LOGGER.error("Server said that the Metrics receiving endpoint at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval);
+        } else if (responseCode == 429) {
+            interval.set(Math.min(failures.incrementAndGet(), maxInterval));
+            LOGGER.info("Client Metrics was RATE LIMITED for the {}. time. Further backing off. Current backoff at {} times our metrics post interval", failures.get(), interval.get());
+        } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
+            failures.incrementAndGet();
+            interval.set(maxInterval);
+            LOGGER.error("Client was not authorized to post metrics to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashURLs().getClientMetricsURL(), maxInterval);
+        } else if (responseCode >= 500) {
+            interval.set(Math.min(failures.incrementAndGet(), maxInterval));
+            LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get());
+        }
+    }
+
+    protected int getInterval() {
+        return this.interval.get();
+    }
+
+    protected int getFailures() {
+        return this.failures.get();
+    }
 }
diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java
@@ -8,13 +8,19 @@
 import io.getunleash.lang.Nullable;
 import io.getunleash.util.UnleashConfig;
 import io.getunleash.util.UnleashScheduledExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.HttpURLConnection;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 public class FeatureRepository implements IFeatureRepository {
-
+    private final Integer maxInterval; // Set to 20 times our polling interval, so with our default of 15 seconds, the longest interval will be 5 minutes.
+    private static final Logger LOGGER = LoggerFactory.getLogger(FeatureRepository.class);
     private final UnleashConfig unleashConfig;
     private final BackupHandler<FeatureCollection> featureBackupHandler;
     private final FeatureBootstrapHandler featureBootstrapHandler;
@@ -24,48 +30,54 @@ public class FeatureRepository implements IFeatureRepository {
     private FeatureCollection featureCollection;
     private boolean ready;
 
+    private AtomicInteger failures = new AtomicInteger(0);
+    private AtomicInteger interval = new AtomicInteger(0);
+
     public FeatureRepository(UnleashConfig unleashConfig) {
         this(unleashConfig, new FeatureBackupHandlerFile(unleashConfig));
     }
 
     public FeatureRepository(
-            UnleashConfig unleashConfig,
-            final BackupHandler<FeatureCollection> featureBackupHandler) {
+        UnleashConfig unleashConfig,
+        final BackupHandler<FeatureCollection> featureBackupHandler) {
         this.unleashConfig = unleashConfig;
         this.featureBackupHandler = featureBackupHandler;
         this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig);
         this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig);
         this.eventDispatcher = new EventDispatcher(unleashConfig);
+        this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1));
 
         this.initCollections(unleashConfig.getScheduledExecutor());
     }
 
     protected FeatureRepository(
-            UnleashConfig unleashConfig,
-            BackupHandler<FeatureCollection> featureBackupHandler,
-            EventDispatcher eventDispatcher,
-            FeatureFetcher featureFetcher,
-            FeatureBootstrapHandler featureBootstrapHandler) {
+        UnleashConfig unleashConfig,
+        BackupHandler<FeatureCollection> featureBackupHandler,
+        EventDispatcher eventDispatcher,
+        FeatureFetcher featureFetcher,
+        FeatureBootstrapHandler featureBootstrapHandler) {
 
         this.unleashConfig = unleashConfig;
         this.featureBackupHandler = featureBackupHandler;
         this.featureFetcher = featureFetcher;
         this.featureBootstrapHandler = featureBootstrapHandler;
         this.eventDispatcher = eventDispatcher;
+        this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1));
         this.initCollections(unleashConfig.getScheduledExecutor());
     }
 
     protected FeatureRepository(
-            UnleashConfig unleashConfig,
-            FeatureBackupHandlerFile featureBackupHandler,
-            UnleashScheduledExecutor executor,
-            FeatureFetcher featureFetcher,
-            FeatureBootstrapHandler featureBootstrapHandler) {
+        UnleashConfig unleashConfig,
+        FeatureBackupHandlerFile featureBackupHandler,
+        UnleashScheduledExecutor executor,
+        FeatureFetcher featureFetcher,
+        FeatureBootstrapHandler featureBootstrapHandler) {
         this.unleashConfig = unleashConfig;
         this.featureBackupHandler = featureBackupHandler;
         this.featureFetcher = featureFetcher;
         this.featureBootstrapHandler = featureBootstrapHandler;
         this.eventDispatcher = new EventDispatcher(unleashConfig);
+        this.maxInterval = Integer.max(20, 300 / Integer.max(Long.valueOf(unleashConfig.getFetchTogglesInterval()).intValue(), 1));
         this.initCollections(executor);
     }
 
@@ -90,35 +102,66 @@ private void initCollections(UnleashScheduledExecutor executor) {
         }
     }
 
+    //
     private Runnable updateFeatures(@Nullable final Consumer<UnleashException> handler) {
-        return () -> {
+        return () -> updateFeaturesInternal(handler);
+    }
+
+    private void updateFeaturesInternal(@Nullable final Consumer<UnleashException> handler) {
+        LOGGER.info("Interval: {}. Failures: {}", interval.get(), failures.get());
+        if (interval.get() <= 0L) {
             try {
                 ClientFeaturesResponse response = featureFetcher.fetchFeatures();
                 eventDispatcher.dispatch(response);
                 if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) {
                     SegmentCollection segmentCollection = response.getSegmentCollection();
                     featureCollection =
-                            new FeatureCollection(
-                                    response.getToggleCollection(),
-                                    segmentCollection != null
-                                            ? segmentCollection
-                                            : new SegmentCollection(Collections.emptyList()));
+                        new FeatureCollection(
+                            response.getToggleCollection(),
+                            segmentCollection != null
+                                ? segmentCollection
+                                : new SegmentCollection(Collections.emptyList()));
 
                     featureBackupHandler.write(featureCollection);
+                } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) {
+                    handleHttpErrorCodes(response.getHttpStatusCode());
+                    return;
                 }
 
+                interval.set(Math.max(failures.decrementAndGet(),0));
                 if (!ready) {
                     eventDispatcher.dispatch(new UnleashReady());
                     ready = true;
                 }
             } catch (UnleashException e) {
+                interval.set(Math.min(failures.incrementAndGet(), maxInterval));
                 if (handler != null) {
                     handler.accept(e);
                 } else {
                     throw e;
                 }
             }
-        };
+        } else {
+            interval.decrementAndGet();
+        }
+    }
+
+    private void handleHttpErrorCodes(int responseCode) {
+        if (responseCode == 404) {
+            interval.set(maxInterval);
+            failures.incrementAndGet();
+            LOGGER.error("Server said that the API at {} does not exist. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval);
+        } else if (responseCode == 429) {
+            interval.set(Math.min(failures.incrementAndGet(), maxInterval));
+            LOGGER.info("Client was RATE LIMITED for the {} time. Further backing off. Current backoff at {} times our poll interval", failures.get(), interval.get());
+        } else if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
+            failures.incrementAndGet();
+            interval.set(maxInterval);
+            LOGGER.error("Client failed to authenticate to the Unleash API at {}. Backing off to {} times our poll interval to avoid overloading server", unleashConfig.getUnleashAPI(), maxInterval);
+        } else if (responseCode >= 500) {
+            interval.set(Math.min(failures.incrementAndGet(), maxInterval));
+            LOGGER.info("Server failed with a {} status code. Backing off. Current backoff at {} times our poll interval", responseCode, interval.get());
+        }
     }
 
     @Override
@@ -129,12 +172,20 @@ private Runnable updateFeatures(@Nullable final Consumer<UnleashException> handl
     @Override
     public List<String> getFeatureNames() {
         return featureCollection.getToggleCollection().getFeatures().stream()
-                .map(FeatureToggle::getName)
-                .collect(Collectors.toList());
+            .map(FeatureToggle::getName)
+            .collect(Collectors.toList());
     }
 
     @Override
     public Segment getSegment(Integer id) {
         return featureCollection.getSegmentCollection().getSegment(id);
     }
+
+    public Integer getFailures() {
+        return failures.get();
+    }
+
+    public Integer getInterval() {
+        return interval.get();
+    }
 }
diff --git a/src/main/java/io/getunleash/repository/FeatureToggleResponse.java b/src/main/java/io/getunleash/repository/FeatureToggleResponse.java
@@ -13,7 +13,7 @@ public class FeatureToggleResponse implements UnleashEvent {
     public enum Status {
         NOT_CHANGED,
         CHANGED,
-        UNAVAILABLE
+        UNAVAILABLE,
     }
 
     private final Status status;

diff --git a/src/main/java/io/getunleash/variant/VariantUtil.java b/src/main/java/io/getunleash/variant/VariantUtil.java
@@ -133,4 +133,5 @@ public static Variant selectVariant(
         }
         return null;
     }
+
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -133,4 +133,5 @@ public static Variant selectVariant( @@
             }
             return null;
         }
     }