Skip to content

Commit c984c23

Browse files
Christopher Kolstadgastonfournier
Christopher Kolstad
and
gastonfournier
authored
feat: Obey HTTP Status codes from upstream. (#221)
* 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. With 401,403 and 404 we immediately backoff to at minimum 300 seconds of interval as well as log at error level that we were not authorized or the URL did not exist. Co-authored-by: Gastón Fournier <[email protected]>
1 parent e50bbf7 commit c984c23

15 files changed

+789
-52
lines changed

‎.github/workflows/pull_requests.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
version: [8,11,17]
1111
steps:
1212
- name: Checkout
13-
uses: actions/checkout@v3
13+
uses: actions/checkout@v4
1414
- name: Setup Java
1515
uses: actions/setup-java@v3
1616
with:

‎src/main/java/io/getunleash/metric/DefaultHttpMetricsSender.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,32 @@ public DefaultHttpMetricsSender(UnleashConfig unleashConfig) {
3737
.create();
3838
}
3939

40-
public void registerClient(ClientRegistration registration) {
40+
public int registerClient(ClientRegistration registration) {
4141
if (!unleashConfig.isDisableMetrics()) {
4242
try {
43-
post(clientRegistrationURL, registration);
43+
int statusCode = post(clientRegistrationURL, registration);
4444
eventDispatcher.dispatch(registration);
45+
return statusCode;
4546
} catch (UnleashException ex) {
4647
eventDispatcher.dispatch(ex);
48+
return -1;
4749
}
4850
}
51+
return -1;
4952
}
5053

51-
public void sendMetrics(ClientMetrics metrics) {
54+
public int sendMetrics(ClientMetrics metrics) {
5255
if (!unleashConfig.isDisableMetrics()) {
5356
try {
54-
post(clientMetricsURL, metrics);
57+
int statusCode = post(clientMetricsURL, metrics);
5558
eventDispatcher.dispatch(metrics);
59+
return statusCode;
5660
} catch (UnleashException ex) {
5761
eventDispatcher.dispatch(ex);
62+
return -1;
5863
}
5964
}
65+
return -1;
6066
}
6167

6268
private int post(URL url, Object o) throws UnleashException {

‎src/main/java/io/getunleash/metric/MetricSender.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package io.getunleash.metric;
22

33
public interface MetricSender {
4-
void registerClient(ClientRegistration registration);
4+
int registerClient(ClientRegistration registration);
55

6-
void sendMetrics(ClientMetrics metrics);
6+
int sendMetrics(ClientMetrics metrics);
77
}

‎src/main/java/io/getunleash/metric/OkHttpMetricsSender.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,21 @@ public OkHttpMetricsSender(UnleashConfig config) {
6060
}
6161

6262
@Override
63-
public void registerClient(ClientRegistration registration) {
63+
public int registerClient(ClientRegistration registration) {
6464
if (!config.isDisableMetrics()) {
6565
try {
66-
post(clientRegistrationUrl, registration);
66+
int statusCode = post(clientRegistrationUrl, registration);
6767
eventDispatcher.dispatch(registration);
68+
return statusCode;
6869
} catch (UnleashException ex) {
6970
eventDispatcher.dispatch(ex);
7071
}
7172
}
73+
return -1;
7274
}
7375

7476
@Override
75-
public void sendMetrics(ClientMetrics metrics) {
77+
public int sendMetrics(ClientMetrics metrics) {
7678
if (!config.isDisableMetrics()) {
7779
try {
7880
post(clientMetricsUrl, metrics);
@@ -81,6 +83,7 @@ public void sendMetrics(ClientMetrics metrics) {
8183
eventDispatcher.dispatch(ex);
8284
}
8385
}
86+
return -1;
8487
}
8588

8689
private int post(HttpUrl url, Object o) {

‎src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package io.getunleash.metric;
22

3+
import io.getunleash.util.Throttler;
34
import io.getunleash.util.UnleashConfig;
45
import io.getunleash.util.UnleashScheduledExecutor;
56
import java.time.LocalDateTime;
67
import java.time.ZoneId;
78
import java.util.Set;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
811

912
public class UnleashMetricServiceImpl implements UnleashMetricService {
13+
private static final Logger LOGGER = LoggerFactory.getLogger(UnleashMetricServiceImpl.class);
1014
private final LocalDateTime started;
1115
private final UnleashConfig unleashConfig;
1216

@@ -15,6 +19,8 @@ public class UnleashMetricServiceImpl implements UnleashMetricService {
1519
// mutable
1620
private volatile MetricsBucket currentMetricsBucket;
1721

22+
private final Throttler throttler;
23+
1824
public UnleashMetricServiceImpl(
1925
UnleashConfig unleashConfig, UnleashScheduledExecutor executor) {
2026
this(unleashConfig, unleashConfig.getMetricSenderFactory().apply(unleashConfig), executor);
@@ -28,6 +34,11 @@ public UnleashMetricServiceImpl(
2834
this.started = LocalDateTime.now(ZoneId.of("UTC"));
2935
this.unleashConfig = unleashConfig;
3036
this.metricSender = metricSender;
37+
this.throttler =
38+
new Throttler(
39+
(int) unleashConfig.getSendMetricsInterval(),
40+
300,
41+
unleashConfig.getUnleashURLs().getClientMetricsURL());
3142
long metricsInterval = unleashConfig.getSendMetricsInterval();
3243
executor.setInterval(sendMetrics(), metricsInterval, metricsInterval);
3344
}
@@ -51,11 +62,29 @@ public void countVariant(String toggleName, String variantName) {
5162

5263
private Runnable sendMetrics() {
5364
return () -> {
54-
MetricsBucket metricsBucket = this.currentMetricsBucket;
55-
this.currentMetricsBucket = new MetricsBucket();
56-
metricsBucket.end();
57-
ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket);
58-
metricSender.sendMetrics(metrics);
65+
if (throttler.performAction()) {
66+
MetricsBucket metricsBucket = this.currentMetricsBucket;
67+
this.currentMetricsBucket = new MetricsBucket();
68+
metricsBucket.end();
69+
ClientMetrics metrics = new ClientMetrics(unleashConfig, metricsBucket);
70+
int statusCode = metricSender.sendMetrics(metrics);
71+
if (statusCode >= 200 && statusCode < 400) {
72+
throttler.decrementFailureCountAndResetSkips();
73+
}
74+
if (statusCode >= 400) {
75+
throttler.handleHttpErrorCodes(statusCode);
76+
}
77+
} else {
78+
throttler.skipped();
79+
}
5980
};
6081
}
82+
83+
protected int getSkips() {
84+
return this.throttler.getSkips();
85+
}
86+
87+
protected int getFailures() {
88+
return this.throttler.getFailures();
89+
}
6190
}

‎src/main/java/io/getunleash/repository/FeatureRepository.java‎

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,26 @@
66
import io.getunleash.event.EventDispatcher;
77
import io.getunleash.event.UnleashReady;
88
import io.getunleash.lang.Nullable;
9+
import io.getunleash.util.Throttler;
910
import io.getunleash.util.UnleashConfig;
1011
import io.getunleash.util.UnleashScheduledExecutor;
1112
import java.util.Collections;
1213
import java.util.List;
1314
import java.util.function.Consumer;
1415
import java.util.stream.Collectors;
16+
import org.slf4j.Logger;
17+
import org.slf4j.LoggerFactory;
1518

1619
public class FeatureRepository implements IFeatureRepository {
17-
20+
private static final Logger LOGGER = LoggerFactory.getLogger(FeatureRepository.class);
1821
private final UnleashConfig unleashConfig;
1922
private final BackupHandler<FeatureCollection> featureBackupHandler;
2023
private final FeatureBootstrapHandler featureBootstrapHandler;
2124
private final FeatureFetcher featureFetcher;
2225
private final EventDispatcher eventDispatcher;
2326

27+
private final Throttler throttler;
28+
2429
private FeatureCollection featureCollection;
2530
private boolean ready;
2631

@@ -36,7 +41,11 @@ public FeatureRepository(
3641
this.featureFetcher = unleashConfig.getUnleashFeatureFetcherFactory().apply(unleashConfig);
3742
this.featureBootstrapHandler = new FeatureBootstrapHandler(unleashConfig);
3843
this.eventDispatcher = new EventDispatcher(unleashConfig);
39-
44+
this.throttler =
45+
new Throttler(
46+
(int) unleashConfig.getFetchTogglesInterval(),
47+
300,
48+
unleashConfig.getUnleashURLs().getFetchTogglesURL());
4049
this.initCollections(unleashConfig.getScheduledExecutor());
4150
}
4251

@@ -46,12 +55,16 @@ protected FeatureRepository(
4655
EventDispatcher eventDispatcher,
4756
FeatureFetcher featureFetcher,
4857
FeatureBootstrapHandler featureBootstrapHandler) {
49-
5058
this.unleashConfig = unleashConfig;
5159
this.featureBackupHandler = featureBackupHandler;
5260
this.featureFetcher = featureFetcher;
5361
this.featureBootstrapHandler = featureBootstrapHandler;
5462
this.eventDispatcher = eventDispatcher;
63+
this.throttler =
64+
new Throttler(
65+
(int) unleashConfig.getFetchTogglesInterval(),
66+
300,
67+
unleashConfig.getUnleashURLs().getFetchTogglesURL());
5568
this.initCollections(unleashConfig.getScheduledExecutor());
5669
}
5770

@@ -66,6 +79,11 @@ protected FeatureRepository(
6679
this.featureFetcher = featureFetcher;
6780
this.featureBootstrapHandler = featureBootstrapHandler;
6881
this.eventDispatcher = new EventDispatcher(unleashConfig);
82+
this.throttler =
83+
new Throttler(
84+
(int) unleashConfig.getFetchTogglesInterval(),
85+
300,
86+
unleashConfig.getUnleashURLs().getFetchTogglesURL());
6987
this.initCollections(executor);
7088
}
7189

@@ -90,33 +108,44 @@ private void initCollections(UnleashScheduledExecutor executor) {
90108
}
91109
}
92110

111+
private Integer calculateMaxSkips(int fetchTogglesInterval) {
112+
return Integer.max(20, 300 / Integer.max(fetchTogglesInterval, 1));
113+
}
114+
93115
private Runnable updateFeatures(@Nullable final Consumer<UnleashException> handler) {
94116
return () -> {
95-
try {
96-
ClientFeaturesResponse response = featureFetcher.fetchFeatures();
97-
eventDispatcher.dispatch(response);
98-
if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) {
99-
SegmentCollection segmentCollection = response.getSegmentCollection();
100-
featureCollection =
101-
new FeatureCollection(
102-
response.getToggleCollection(),
103-
segmentCollection != null
104-
? segmentCollection
105-
: new SegmentCollection(Collections.emptyList()));
106-
107-
featureBackupHandler.write(featureCollection);
108-
}
109-
110-
if (!ready) {
111-
eventDispatcher.dispatch(new UnleashReady());
112-
ready = true;
113-
}
114-
} catch (UnleashException e) {
115-
if (handler != null) {
116-
handler.accept(e);
117-
} else {
118-
throw e;
117+
if (throttler.performAction()) {
118+
try {
119+
ClientFeaturesResponse response = featureFetcher.fetchFeatures();
120+
eventDispatcher.dispatch(response);
121+
if (response.getStatus() == ClientFeaturesResponse.Status.CHANGED) {
122+
SegmentCollection segmentCollection = response.getSegmentCollection();
123+
featureCollection =
124+
new FeatureCollection(
125+
response.getToggleCollection(),
126+
segmentCollection != null
127+
? segmentCollection
128+
: new SegmentCollection(Collections.emptyList()));
129+
130+
featureBackupHandler.write(featureCollection);
131+
} else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) {
132+
throttler.handleHttpErrorCodes(response.getHttpStatusCode());
133+
return;
134+
}
135+
throttler.decrementFailureCountAndResetSkips();
136+
if (!ready) {
137+
eventDispatcher.dispatch(new UnleashReady());
138+
ready = true;
139+
}
140+
} catch (UnleashException e) {
141+
if (handler != null) {
142+
handler.accept(e);
143+
} else {
144+
throw e;
145+
}
119146
}
147+
} else {
148+
throttler.skipped(); // We didn't do anything this iteration, just reduce the count
120149
}
121150
};
122151
}
@@ -137,4 +166,12 @@ public List<String> getFeatureNames() {
137166
public Segment getSegment(Integer id) {
138167
return featureCollection.getSegmentCollection().getSegment(id);
139168
}
169+
170+
public Integer getFailures() {
171+
return this.throttler.getFailures();
172+
}
173+
174+
public Integer getSkips() {
175+
return this.throttler.getSkips();
176+
}
140177
}

‎src/main/java/io/getunleash/repository/FeatureToggleResponse.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public class FeatureToggleResponse implements UnleashEvent {
1313
public enum Status {
1414
NOT_CHANGED,
1515
CHANGED,
16-
UNAVAILABLE
16+
UNAVAILABLE,
1717
}
1818

1919
private final Status status;

‎src/main/java/io/getunleash/util/ConstraintMerger.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ public static List<Constraint> mergeConstraints(
1717
Optional.ofNullable(strategy.getConstraints())
1818
.orElseGet(Collections::emptyList),
1919
Optional.ofNullable(strategy.getSegments())
20-
.orElseGet(Collections::emptyList).stream()
20+
.orElseGet(Collections::emptyList)
21+
.stream()
2122
.map(repository::getSegment)
2223
.map(s -> s == null ? DENY_SEGMENT : s)
2324
.map(Segment::getConstraints)

0 commit comments

Comments
 (0)