Skip to content

Commit b971599

Browse files
runningcodeclaude
andcommitted
ref(transport): Measure rate-limit backoff on a monotonic clock (JAVA-574)
Retry-after limits were java.util.Date values derived from System.currentTimeMillis(). A wall clock is the wrong instrument for a backoff window: it steps when the device syncs time, so an NTP correction could lift a 60 second rate limit early or extend it by however far the clock jumped. The limits now live on the monotonic clock, which counts forward at a steady rate and keeps counting while the device sleeps, which is what a server-dictated wait means. Storing Deadline rather than a timestamp also removes the duplicated parameter on applyRetryAfterOnlyIfLonger, which took both an absolute deadline and the delay needed to reach it, and lets three JdkObsolete and JavaUtilDate suppressions go with the Dates. RateLimiter also took the whole SentryOptions while reading exactly three methods from it. It now depends on RateLimiterConfig, declared next to its consumer, so what a rate limiter touches is three lines to read rather than three hundred. SentryOptions implements it with no new methods, so every existing caller compiles unchanged. Both existing constructors stay, so the .api diff is additions only. The ICurrentDateProvider one is deprecated and adapts the injected provider rather than ignoring it, since a custom ITransportFactory may be passing one. One boundary moves by a nanosecond: a limit used to be active while `now <= deadline` and is now active while `now < deadline`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c45669e commit b971599

5 files changed

Lines changed: 141 additions & 80 deletions

File tree

‎sentry/api/sentry.api‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3654,7 +3654,7 @@ public final class io/sentry/SentryOpenTelemetryMode : java/lang/Enum {
36543654
public static fun values ()[Lio/sentry/SentryOpenTelemetryMode;
36553655
}
36563656

3657-
public class io/sentry/SentryOptions {
3657+
public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig {
36583658
public static final field DEFAULT_PROPAGATION_TARGETS Ljava/lang/String;
36593659
public static final field MAX_EVENT_SIZE_BYTES J
36603660
protected final field lock Lio/sentry/util/AutoClosableReentrantLock;
@@ -7706,6 +7706,7 @@ public final class io/sentry/transport/NoOpTransportGate : io/sentry/transport/I
77067706

77077707
public final class io/sentry/transport/RateLimiter : java/io/Closeable {
77087708
public fun <init> (Lio/sentry/SentryOptions;)V
7709+
public fun <init> (Lio/sentry/time/MonotonicClock;Lio/sentry/transport/RateLimiterConfig;)V
77097710
public fun <init> (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V
77107711
public fun addRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V
77117712
public fun close ()V
@@ -7720,6 +7721,12 @@ public abstract interface class io/sentry/transport/RateLimiter$IRateLimitObserv
77207721
public abstract fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
77217722
}
77227723

7724+
public abstract interface class io/sentry/transport/RateLimiterConfig {
7725+
public abstract fun getClientReportRecorder ()Lio/sentry/clientreport/IClientReportRecorder;
7726+
public abstract fun getLogger ()Lio/sentry/ILogger;
7727+
public abstract fun getTimerExecutorService ()Lio/sentry/ISentryExecutorService;
7728+
}
7729+
77237730
public final class io/sentry/transport/ReusableCountLatch {
77247731
public fun <init> ()V
77257732
public fun <init> (I)V

‎sentry/src/main/java/io/sentry/SentryOptions.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import io.sentry.transport.ITransportGate;
3030
import io.sentry.transport.NoOpEnvelopeCache;
3131
import io.sentry.transport.NoOpTransportGate;
32+
import io.sentry.transport.RateLimiterConfig;
3233
import io.sentry.util.AutoClosableReentrantLock;
3334
import io.sentry.util.LazyEvaluator;
3435
import io.sentry.util.LoadClass;
@@ -58,7 +59,7 @@
5859

5960
/** Sentry SDK options */
6061
@Open
61-
public class SentryOptions {
62+
public class SentryOptions implements RateLimiterConfig {
6263

6364
@ApiStatus.Internal public static final @NotNull String DEFAULT_PROPAGATION_TARGETS = ".*";
6465

@@ -850,6 +851,7 @@ public void setDebug(final boolean debug) {
850851
*
851852
* @return the logger
852853
*/
854+
@Override
853855
public @NotNull ILogger getLogger() {
854856
return logger;
855857
}
@@ -1613,6 +1615,7 @@ public void setExecutorService(final @NotNull ISentryExecutorService executorSer
16131615
* @return the timer executor service
16141616
*/
16151617
@ApiStatus.Internal
1618+
@Override
16161619
@NotNull
16171620
public ISentryExecutorService getTimerExecutorService() {
16181621
return timerExecutorService;
@@ -2604,6 +2607,7 @@ public void setInstrumenter(final @NotNull Instrumenter instrumenter) {
26042607
* @return a client report recorder or NoOp
26052608
*/
26062609
@ApiStatus.Internal
2610+
@Override
26072611
public @NotNull IClientReportRecorder getClientReportRecorder() {
26082612
return clientReportRecorder;
26092613
}

‎sentry/src/main/java/io/sentry/transport/RateLimiter.java‎

Lines changed: 59 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import io.sentry.hints.DiskFlushNotification;
1515
import io.sentry.hints.Retryable;
1616
import io.sentry.hints.SubmissionResult;
17+
import io.sentry.time.Deadline;
18+
import io.sentry.time.MonotonicClock;
1719
import io.sentry.util.AutoClosableReentrantLock;
1820
import io.sentry.util.HintUtils;
1921
import io.sentry.util.StringUtils;
@@ -22,40 +24,55 @@
2224
import java.util.ArrayList;
2325
import java.util.Arrays;
2426
import java.util.Collections;
25-
import java.util.Date;
2627
import java.util.Iterator;
2728
import java.util.List;
2829
import java.util.Map;
2930
import java.util.concurrent.ConcurrentHashMap;
3031
import java.util.concurrent.CopyOnWriteArrayList;
3132
import java.util.concurrent.Future;
3233
import java.util.concurrent.RejectedExecutionException;
34+
import java.util.concurrent.TimeUnit;
3335
import org.jetbrains.annotations.NotNull;
3436
import org.jetbrains.annotations.Nullable;
3537

3638
/** Controls retry limits on different category types sent to Sentry. */
3739
public final class RateLimiter implements Closeable {
3840

39-
private static final int HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS = 60000;
41+
private static final long HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS = 60_000;
4042

41-
private final @NotNull ICurrentDateProvider currentDateProvider;
42-
private final @NotNull SentryOptions options;
43-
private final @NotNull Map<DataCategory, @NotNull Date> sentryRetryAfterLimit =
43+
private final @NotNull MonotonicClock clock;
44+
private final @NotNull RateLimiterConfig config;
45+
private final @NotNull Map<DataCategory, @NotNull Deadline> sentryRetryAfterLimit =
4446
new ConcurrentHashMap<>();
4547
private final @NotNull List<IRateLimitObserver> rateLimitObservers = new CopyOnWriteArrayList<>();
4648
private final @NotNull List<Future<?>> notifyObserversFutures = new ArrayList<>();
4749
private final @NotNull AutoClosableReentrantLock notifyFuturesLock =
4850
new AutoClosableReentrantLock();
4951

50-
public RateLimiter(
51-
final @NotNull ICurrentDateProvider currentDateProvider,
52-
final @NotNull SentryOptions options) {
53-
this.currentDateProvider = currentDateProvider;
54-
this.options = options;
52+
public RateLimiter(final @NotNull MonotonicClock clock, final @NotNull RateLimiterConfig config) {
53+
this.clock = clock;
54+
this.config = config;
5555
}
5656

5757
public RateLimiter(final @NotNull SentryOptions options) {
58-
this(CurrentDateProvider.getInstance(), options);
58+
this(options.getMonotonicClock(), options);
59+
}
60+
61+
/**
62+
* @deprecated backoff is measured on {@link SentryOptions#getMonotonicClock()}; use {@link
63+
* #RateLimiter(SentryOptions)}. An injected wall clock is adapted so that an existing custom
64+
* transport keeps the behaviour it has today, but it is not monotonic and can step.
65+
*/
66+
@Deprecated
67+
public RateLimiter(
68+
final @NotNull ICurrentDateProvider currentDateProvider,
69+
final @NotNull SentryOptions options) {
70+
// ICurrentDateProvider is itself a `long ()` interface, so an unadorned lambda matches this
71+
// constructor as readily as the intended one. The cast is what makes the call non-recursive.
72+
this(
73+
(MonotonicClock)
74+
() -> TimeUnit.MILLISECONDS.toNanos(currentDateProvider.getCurrentTimeMillis()),
75+
options);
5976
}
6077

6178
public @Nullable SentryEnvelope filter(
@@ -70,14 +87,14 @@ public RateLimiter(final @NotNull SentryOptions options) {
7087
}
7188

7289
dropItems.add(item);
73-
options
90+
config
7491
.getClientReportRecorder()
7592
.recordLostEnvelopeItem(DiscardReason.RATELIMIT_BACKOFF, item);
7693
}
7794
}
7895

7996
if (dropItems != null) {
80-
options
97+
config
8198
.getLogger()
8299
.log(
83100
SentryLevel.WARNING,
@@ -94,7 +111,7 @@ public RateLimiter(final @NotNull SentryOptions options) {
94111

95112
// no reason to continue
96113
if (toSend.isEmpty()) {
97-
options
114+
config
98115
.getLogger()
99116
.log(SentryLevel.WARNING, "Envelope discarded due all items rate limited.");
100117

@@ -107,16 +124,11 @@ public RateLimiter(final @NotNull SentryOptions options) {
107124
return envelope;
108125
}
109126

110-
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
111127
public boolean isActiveForCategory(final @NotNull DataCategory dataCategory) {
112-
final Date currentDate = new Date(currentDateProvider.getCurrentTimeMillis());
113-
114128
// check all categories
115-
final Date dateAllCategories = sentryRetryAfterLimit.get(DataCategory.All);
116-
if (dateAllCategories != null) {
117-
if (!currentDate.after(dateAllCategories)) {
118-
return true;
119-
}
129+
final @Nullable Deadline allCategories = sentryRetryAfterLimit.get(DataCategory.All);
130+
if (allCategories != null && !allCategories.hasPassed()) {
131+
return true;
120132
}
121133

122134
// Unknown should not be rate limited
@@ -125,24 +137,15 @@ public boolean isActiveForCategory(final @NotNull DataCategory dataCategory) {
125137
}
126138

127139
// check for specific dataCategory
128-
final Date dateCategory = sentryRetryAfterLimit.get(dataCategory);
129-
if (dateCategory != null) {
130-
return !currentDate.after(dateCategory);
131-
}
132-
133-
return false;
140+
final @Nullable Deadline categoryLimit = sentryRetryAfterLimit.get(dataCategory);
141+
return categoryLimit != null && !categoryLimit.hasPassed();
134142
}
135143

136144
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
137145
public boolean isAnyRateLimitActive() {
138-
final Date currentDate = new Date(currentDateProvider.getCurrentTimeMillis());
139-
140-
for (DataCategory dataCategory : sentryRetryAfterLimit.keySet()) {
141-
final Date dateCategory = sentryRetryAfterLimit.get(dataCategory);
142-
if (dateCategory != null) {
143-
if (!currentDate.after(dateCategory)) {
144-
return true;
145-
}
146+
for (final @NotNull Deadline limit : sentryRetryAfterLimit.values()) {
147+
if (!limit.hasPassed()) {
148+
return true;
146149
}
147150
}
148151

@@ -163,7 +166,7 @@ private void markHintWhenSendingFailed(final @NotNull Hint hint, final boolean r
163166
DiskFlushNotification.class,
164167
(diskFlushNotification) -> {
165168
diskFlushNotification.markFlushed();
166-
options.getLogger().log(SentryLevel.DEBUG, "Disk flush envelope fired due to rate limit");
169+
config.getLogger().log(SentryLevel.DEBUG, "Disk flush envelope fired due to rate limit");
167170
});
168171
}
169172

@@ -251,15 +254,11 @@ public void updateRetryAfterLimits(
251254

252255
if (rateLimit.length > 0) {
253256
final String retryAfter = rateLimit[0];
254-
long retryAfterMillis = parseRetryAfterOrDefault(retryAfter);
257+
final @NotNull Deadline deadline = parseRetryAfterOrDefault(retryAfter);
255258

256259
if (rateLimit.length > 1) {
257260
final String allCategories = rateLimit[1];
258261

259-
// we dont care if Date is UTC as we just add the relative seconds
260-
final Date date =
261-
new Date(currentDateProvider.getCurrentTimeMillis() + retryAfterMillis);
262-
263262
if (allCategories != null && !allCategories.isEmpty()) {
264263
final String[] categories = allCategories.split(";", -1);
265264

@@ -270,48 +269,43 @@ public void updateRetryAfterLimits(
270269
if (catItemCapitalized != null) {
271270
dataCategory = DataCategory.valueOf(catItemCapitalized);
272271
} else {
273-
options.getLogger().log(ERROR, "Couldn't capitalize: %s", catItem);
272+
config.getLogger().log(ERROR, "Couldn't capitalize: %s", catItem);
274273
}
275274
} catch (IllegalArgumentException e) {
276-
options.getLogger().log(INFO, e, "Unknown category: %s", catItem);
275+
config.getLogger().log(INFO, e, "Unknown category: %s", catItem);
277276
}
278277
// we dont apply rate limiting for unknown categories
279278
if (DataCategory.Unknown.equals(dataCategory)) {
280279
continue;
281280
}
282281

283-
applyRetryAfterOnlyIfLonger(dataCategory, date, retryAfterMillis);
282+
applyRetryAfterOnlyIfLonger(dataCategory, deadline);
284283
}
285284
} else {
286285
// if categories are empty, we should apply to "all" categories.
287-
applyRetryAfterOnlyIfLonger(DataCategory.All, date, retryAfterMillis);
286+
applyRetryAfterOnlyIfLonger(DataCategory.All, deadline);
288287
}
289288
}
290289
}
291290
}
292291
} else if (errorCode == 429) {
293-
final long retryAfterMillis = parseRetryAfterOrDefault(retryAfterHeader);
294-
// we dont care if Date is UTC as we just add the relative seconds
295-
final Date date = new Date(currentDateProvider.getCurrentTimeMillis() + retryAfterMillis);
296-
applyRetryAfterOnlyIfLonger(DataCategory.All, date, retryAfterMillis);
292+
applyRetryAfterOnlyIfLonger(DataCategory.All, parseRetryAfterOrDefault(retryAfterHeader));
297293
}
298294
}
299295

300296
/**
301-
* apply new timestamp for rate limiting only if its longer than the previous one
297+
* apply the new deadline for rate limiting only if it is longer than the previous one
302298
*
303299
* @param dataCategory the DataCategory
304-
* @param date the Date to be applied
305-
* @param delayMillis the millis until the rate limit is lifted
300+
* @param deadline when the rate limit is lifted
306301
*/
307-
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
308302
private void applyRetryAfterOnlyIfLonger(
309-
final @NotNull DataCategory dataCategory, final @NotNull Date date, final long delayMillis) {
310-
final Date oldDate = sentryRetryAfterLimit.get(dataCategory);
303+
final @NotNull DataCategory dataCategory, final @NotNull Deadline deadline) {
304+
final @Nullable Deadline oldLimit = sentryRetryAfterLimit.get(dataCategory);
311305

312-
// only overwrite its previous date if the limit is even longer
313-
if (oldDate == null || date.after(oldDate)) {
314-
sentryRetryAfterLimit.put(dataCategory, date);
306+
// only overwrite the previous deadline if the limit is even longer
307+
if (oldLimit == null || deadline.isAfter(oldLimit)) {
308+
sentryRetryAfterLimit.put(dataCategory, deadline);
315309

316310
notifyRateLimitObservers();
317311

@@ -326,11 +320,12 @@ private void applyRetryAfterOnlyIfLonger(
326320
}
327321
try {
328322
notifyObserversFutures.add(
329-
options
323+
config
330324
.getTimerExecutorService()
331-
.schedule(this::notifyRateLimitObservers, delayMillis));
325+
.schedule(
326+
this::notifyRateLimitObservers, deadline.remaining(TimeUnit.MILLISECONDS)));
332327
} catch (RejectedExecutionException e) {
333-
options
328+
config
334329
.getLogger()
335330
.log(SentryLevel.WARNING, "Failed to schedule rate limit lifted notification.", e);
336331
}
@@ -344,7 +339,7 @@ private void applyRetryAfterOnlyIfLonger(
344339
* @param retryAfterHeader the header
345340
* @return the millis in seconds or the default seconds value
346341
*/
347-
private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
342+
private @NotNull Deadline parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
348343
long retryAfterMillis = HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS;
349344
if (retryAfterHeader != null) {
350345
try {
@@ -354,7 +349,7 @@ private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
354349
// let's use the default then
355350
}
356351
}
357-
return retryAfterMillis;
352+
return Deadline.after(clock, retryAfterMillis, TimeUnit.MILLISECONDS);
358353
}
359354

360355
private void notifyRateLimitObservers() {
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package io.sentry.transport;
2+
3+
import io.sentry.ILogger;
4+
import io.sentry.ISentryExecutorService;
5+
import io.sentry.clientreport.IClientReportRecorder;
6+
import org.jetbrains.annotations.ApiStatus;
7+
import org.jetbrains.annotations.NotNull;
8+
9+
/**
10+
* The configuration {@link RateLimiter} reads. Declared next to its consumer rather than alongside
11+
* the implementation, so that the collaborators a rate limiter actually touches are three lines to
12+
* read instead of three hundred, and a test can supply them without building a {@link
13+
* io.sentry.SentryOptions}.
14+
*
15+
* <p>Implementations are expected to delegate to live configuration rather than snapshot it, so
16+
* that a logger or executor replaced after {@code Sentry.init} is still picked up.
17+
*/
18+
@ApiStatus.Internal
19+
public interface RateLimiterConfig {
20+
21+
@NotNull
22+
ILogger getLogger();
23+
24+
@NotNull
25+
IClientReportRecorder getClientReportRecorder();
26+
27+
@NotNull
28+
ISentryExecutorService getTimerExecutorService();
29+
}

0 commit comments

Comments
 (0)