Skip to content

Commit ffeb3b6

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 elapsed-real-time 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 7eae6db commit ffeb3b6

5 files changed

Lines changed: 141 additions & 79 deletions

File tree

‎sentry/api/sentry.api‎

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

3655-
public class io/sentry/SentryOptions {
3655+
public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig {
36563656
public static final field DEFAULT_PROPAGATION_TARGETS Ljava/lang/String;
36573657
public static final field MAX_EVENT_SIZE_BYTES J
36583658
protected final field lock Lio/sentry/util/AutoClosableReentrantLock;
@@ -7680,6 +7680,7 @@ public final class io/sentry/transport/NoOpTransportGate : io/sentry/transport/I
76807680

76817681
public final class io/sentry/transport/RateLimiter : java/io/Closeable {
76827682
public fun <init> (Lio/sentry/SentryOptions;)V
7683+
public fun <init> (Lio/sentry/time/ElapsedRealtimeClock;Lio/sentry/transport/RateLimiterConfig;)V
76837684
public fun <init> (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V
76847685
public fun addRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V
76857686
public fun close ()V
@@ -7694,6 +7695,12 @@ public abstract interface class io/sentry/transport/RateLimiter$IRateLimitObserv
76947695
public abstract fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
76957696
}
76967697

7698+
public abstract interface class io/sentry/transport/RateLimiterConfig {
7699+
public abstract fun getClientReportRecorder ()Lio/sentry/clientreport/IClientReportRecorder;
7700+
public abstract fun getLogger ()Lio/sentry/ILogger;
7701+
public abstract fun getTimerExecutorService ()Lio/sentry/ISentryExecutorService;
7702+
}
7703+
76977704
public final class io/sentry/transport/ReusableCountLatch {
76987705
public fun <init> ()V
76997706
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
@@ -27,6 +27,7 @@
2727
import io.sentry.transport.ITransportGate;
2828
import io.sentry.transport.NoOpEnvelopeCache;
2929
import io.sentry.transport.NoOpTransportGate;
30+
import io.sentry.transport.RateLimiterConfig;
3031
import io.sentry.util.AutoClosableReentrantLock;
3132
import io.sentry.util.LazyEvaluator;
3233
import io.sentry.util.LoadClass;
@@ -56,7 +57,7 @@
5657

5758
/** Sentry SDK options */
5859
@Open
59-
public class SentryOptions {
60+
public class SentryOptions implements RateLimiterConfig {
6061

6162
@ApiStatus.Internal public static final @NotNull String DEFAULT_PROPAGATION_TARGETS = ".*";
6263

@@ -845,6 +846,7 @@ public void setDebug(final boolean debug) {
845846
*
846847
* @return the logger
847848
*/
849+
@Override
848850
public @NotNull ILogger getLogger() {
849851
return logger;
850852
}
@@ -1608,6 +1610,7 @@ public void setExecutorService(final @NotNull ISentryExecutorService executorSer
16081610
* @return the timer executor service
16091611
*/
16101612
@ApiStatus.Internal
1613+
@Override
16111614
@NotNull
16121615
public ISentryExecutorService getTimerExecutorService() {
16131616
return timerExecutorService;
@@ -2599,6 +2602,7 @@ public void setInstrumenter(final @NotNull Instrumenter instrumenter) {
25992602
* @return a client report recorder or NoOp
26002603
*/
26012604
@ApiStatus.Internal
2605+
@Override
26022606
public @NotNull IClientReportRecorder getClientReportRecorder() {
26032607
return clientReportRecorder;
26042608
}

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

Lines changed: 59 additions & 63 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.ElapsedRealtimeClock;
1719
import io.sentry.util.AutoClosableReentrantLock;
1820
import io.sentry.util.HintUtils;
1921
import io.sentry.util.StringUtils;
@@ -22,40 +24,56 @@
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 ElapsedRealtimeClock 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

5052
public RateLimiter(
51-
final @NotNull ICurrentDateProvider currentDateProvider,
52-
final @NotNull SentryOptions options) {
53-
this.currentDateProvider = currentDateProvider;
54-
this.options = options;
53+
final @NotNull ElapsedRealtimeClock clock, final @NotNull RateLimiterConfig config) {
54+
this.clock = clock;
55+
this.config = config;
5556
}
5657

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

6179
public @Nullable SentryEnvelope filter(
@@ -70,14 +88,14 @@ public RateLimiter(final @NotNull SentryOptions options) {
7088
}
7189

7290
dropItems.add(item);
73-
options
91+
config
7492
.getClientReportRecorder()
7593
.recordLostEnvelopeItem(DiscardReason.RATELIMIT_BACKOFF, item);
7694
}
7795
}
7896

7997
if (dropItems != null) {
80-
options
98+
config
8199
.getLogger()
82100
.log(
83101
SentryLevel.WARNING,
@@ -94,7 +112,7 @@ public RateLimiter(final @NotNull SentryOptions options) {
94112

95113
// no reason to continue
96114
if (toSend.isEmpty()) {
97-
options
115+
config
98116
.getLogger()
99117
.log(SentryLevel.WARNING, "Envelope discarded due all items rate limited.");
100118

@@ -107,16 +125,11 @@ public RateLimiter(final @NotNull SentryOptions options) {
107125
return envelope;
108126
}
109127

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

122135
// Unknown should not be rate limited
@@ -125,24 +138,15 @@ public boolean isActiveForCategory(final @NotNull DataCategory dataCategory) {
125138
}
126139

127140
// 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;
141+
final @Nullable Deadline categoryLimit = sentryRetryAfterLimit.get(dataCategory);
142+
return categoryLimit != null && !categoryLimit.hasPassed();
134143
}
135144

136145
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
137146
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-
}
147+
for (final @NotNull Deadline limit : sentryRetryAfterLimit.values()) {
148+
if (!limit.hasPassed()) {
149+
return true;
146150
}
147151
}
148152

@@ -163,7 +167,7 @@ private void markHintWhenSendingFailed(final @NotNull Hint hint, final boolean r
163167
DiskFlushNotification.class,
164168
(diskFlushNotification) -> {
165169
diskFlushNotification.markFlushed();
166-
options.getLogger().log(SentryLevel.DEBUG, "Disk flush envelope fired due to rate limit");
170+
config.getLogger().log(SentryLevel.DEBUG, "Disk flush envelope fired due to rate limit");
167171
});
168172
}
169173

@@ -251,15 +255,11 @@ public void updateRetryAfterLimits(
251255

252256
if (rateLimit.length > 0) {
253257
final String retryAfter = rateLimit[0];
254-
long retryAfterMillis = parseRetryAfterOrDefault(retryAfter);
258+
final @NotNull Deadline deadline = parseRetryAfterOrDefault(retryAfter);
255259

256260
if (rateLimit.length > 1) {
257261
final String allCategories = rateLimit[1];
258262

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-
263263
if (allCategories != null && !allCategories.isEmpty()) {
264264
final String[] categories = allCategories.split(";", -1);
265265

@@ -270,48 +270,43 @@ public void updateRetryAfterLimits(
270270
if (catItemCapitalized != null) {
271271
dataCategory = DataCategory.valueOf(catItemCapitalized);
272272
} else {
273-
options.getLogger().log(ERROR, "Couldn't capitalize: %s", catItem);
273+
config.getLogger().log(ERROR, "Couldn't capitalize: %s", catItem);
274274
}
275275
} catch (IllegalArgumentException e) {
276-
options.getLogger().log(INFO, e, "Unknown category: %s", catItem);
276+
config.getLogger().log(INFO, e, "Unknown category: %s", catItem);
277277
}
278278
// we dont apply rate limiting for unknown categories
279279
if (DataCategory.Unknown.equals(dataCategory)) {
280280
continue;
281281
}
282282

283-
applyRetryAfterOnlyIfLonger(dataCategory, date, retryAfterMillis);
283+
applyRetryAfterOnlyIfLonger(dataCategory, deadline);
284284
}
285285
} else {
286286
// if categories are empty, we should apply to "all" categories.
287-
applyRetryAfterOnlyIfLonger(DataCategory.All, date, retryAfterMillis);
287+
applyRetryAfterOnlyIfLonger(DataCategory.All, deadline);
288288
}
289289
}
290290
}
291291
}
292292
} 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);
293+
applyRetryAfterOnlyIfLonger(DataCategory.All, parseRetryAfterOrDefault(retryAfterHeader));
297294
}
298295
}
299296

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

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

316311
notifyRateLimitObservers();
317312

@@ -326,11 +321,12 @@ private void applyRetryAfterOnlyIfLonger(
326321
}
327322
try {
328323
notifyObserversFutures.add(
329-
options
324+
config
330325
.getTimerExecutorService()
331-
.schedule(this::notifyRateLimitObservers, delayMillis));
326+
.schedule(
327+
this::notifyRateLimitObservers, deadline.remaining(TimeUnit.MILLISECONDS)));
332328
} catch (RejectedExecutionException e) {
333-
options
329+
config
334330
.getLogger()
335331
.log(SentryLevel.WARNING, "Failed to schedule rate limit lifted notification.", e);
336332
}
@@ -344,7 +340,7 @@ private void applyRetryAfterOnlyIfLonger(
344340
* @param retryAfterHeader the header
345341
* @return the millis in seconds or the default seconds value
346342
*/
347-
private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
343+
private @NotNull Deadline parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
348344
long retryAfterMillis = HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS;
349345
if (retryAfterHeader != null) {
350346
try {
@@ -354,7 +350,7 @@ private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) {
354350
// let's use the default then
355351
}
356352
}
357-
return retryAfterMillis;
353+
return Deadline.after(clock, retryAfterMillis, TimeUnit.MILLISECONDS);
358354
}
359355

360356
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)