Skip to content

Commit d40fd82

Browse files
committed
fixup! ref(transport): Measure rate-limit backoff on a monotonic clock (JAVA-574)
1 parent cdc5897 commit d40fd82

4 files changed

Lines changed: 23 additions & 22 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
- Add an internal `MonotonicTicker` abstraction with `Deadline` and `Stopwatch` primitives ([#6028](https://github.com/getsentry/sentry-java/pull/6028))
1919
- Add internal `Timestamp`, `EpochClock` and `AnchoredClock`, so related instants project from one wall-clock reading instead of each reading the clock ([#6045](https://github.com/getsentry/sentry-java/pull/6045))
20-
- Deprecate `RateLimiter(ICurrentDateProvider, SentryOptions)` in favour of `RateLimiter(MonotonicClock, RateLimiterConfig)` ([#6030](https://github.com/getsentry/sentry-java/pull/6030))
20+
- Deprecate `RateLimiter(ICurrentDateProvider, SentryOptions)` in favour of `RateLimiter(MonotonicTicker, RateLimiterConfig)` ([#6030](https://github.com/getsentry/sentry-java/pull/6030))
2121

2222
## 8.55.0
2323

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7706,7 +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
7709+
public fun <init> (Lio/sentry/time/MonotonicTicker;Lio/sentry/transport/RateLimiterConfig;)V
77107710
public fun <init> (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V
77117711
public fun addRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V
77127712
public fun close ()V

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import io.sentry.hints.Retryable;
1616
import io.sentry.hints.SubmissionResult;
1717
import io.sentry.time.Deadline;
18-
import io.sentry.time.MonotonicClock;
18+
import io.sentry.time.MonotonicTicker;
1919
import io.sentry.util.AutoClosableReentrantLock;
2020
import io.sentry.util.HintUtils;
2121
import io.sentry.util.StringUtils;
@@ -40,7 +40,7 @@ public final class RateLimiter implements Closeable {
4040

4141
private static final long HTTP_RETRY_AFTER_DEFAULT_DELAY_MILLIS = 60_000;
4242

43-
private final @NotNull MonotonicClock clock;
43+
private final @NotNull MonotonicTicker ticker;
4444
private final @NotNull RateLimiterConfig config;
4545
private final @NotNull Map<DataCategory, @NotNull Deadline> sentryRetryAfterLimit =
4646
new ConcurrentHashMap<>();
@@ -49,17 +49,18 @@ public final class RateLimiter implements Closeable {
4949
private final @NotNull AutoClosableReentrantLock notifyFuturesLock =
5050
new AutoClosableReentrantLock();
5151

52-
public RateLimiter(final @NotNull MonotonicClock clock, final @NotNull RateLimiterConfig config) {
53-
this.clock = clock;
52+
public RateLimiter(
53+
final @NotNull MonotonicTicker ticker, final @NotNull RateLimiterConfig config) {
54+
this.ticker = ticker;
5455
this.config = config;
5556
}
5657

5758
public RateLimiter(final @NotNull SentryOptions options) {
58-
this(options.getMonotonicClock(), options);
59+
this(options.getMonotonicTicker(), options);
5960
}
6061

6162
/**
62-
* @deprecated backoff is measured on {@link SentryOptions#getMonotonicClock()}; use {@link
63+
* @deprecated backoff is measured on {@link SentryOptions#getMonotonicTicker()}; use {@link
6364
* #RateLimiter(SentryOptions)}. An injected wall clock is adapted so that an existing custom
6465
* transport keeps the behaviour it has today, but it is not monotonic and can step.
6566
*/
@@ -70,7 +71,7 @@ public RateLimiter(
7071
// ICurrentDateProvider is itself a `long ()` interface, so an unadorned lambda matches this
7172
// constructor as readily as the intended one. The cast is what makes the call non-recursive.
7273
this(
73-
(MonotonicClock)
74+
(MonotonicTicker)
7475
() -> TimeUnit.MILLISECONDS.toNanos(currentDateProvider.getCurrentTimeMillis()),
7576
options);
7677
}
@@ -349,7 +350,7 @@ private void applyRetryAfterOnlyIfLonger(
349350
// let's use the default then
350351
}
351352
}
352-
return Deadline.after(clock, retryAfterMillis, TimeUnit.MILLISECONDS);
353+
return Deadline.after(ticker, retryAfterMillis, TimeUnit.MILLISECONDS);
353354
}
354355

355356
private void notifyRateLimitObservers() {

‎sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import io.sentry.protocol.SentryId
3939
import io.sentry.protocol.SentryTransaction
4040
import io.sentry.protocol.User
4141
import io.sentry.test.getProperty
42-
import io.sentry.time.TestMonotonicClock
42+
import io.sentry.time.TestMonotonicTicker
4343
import io.sentry.util.HintUtils
4444
import java.io.File
4545
import java.util.UUID
@@ -65,7 +65,7 @@ import org.mockito.kotlin.whenever
6565

6666
class RateLimiterTest {
6767
private class Fixture {
68-
val clock = TestMonotonicClock()
68+
val ticker = TestMonotonicTicker()
6969
val clientReportRecorder = mock<IClientReportRecorder>()
7070
val serializer = mock<ISerializer>()
7171
var executorService: SentryExecutorService? = null
@@ -79,7 +79,7 @@ class RateLimiterTest {
7979

8080
SentryOptionsManipulator.setClientReportRecorder(options, clientReportRecorder)
8181

82-
return RateLimiter(clock, options)
82+
return RateLimiter(ticker, options)
8383
}
8484
}
8585

@@ -104,7 +104,7 @@ class RateLimiterTest {
104104
)
105105

106106
// the shortest limit in the header has now lapsed
107-
fixture.clock.advance(1001, MILLISECONDS)
107+
fixture.ticker.advance(1001, MILLISECONDS)
108108

109109
val result = rateLimiter.filter(envelope, Hint())
110110
assertNotNull(result)
@@ -148,7 +148,7 @@ class RateLimiterTest {
148148
)
149149

150150
// the shortest limit in the header has now lapsed
151-
fixture.clock.advance(1001, MILLISECONDS)
151+
fixture.ticker.advance(1001, MILLISECONDS)
152152

153153
val result = rateLimiter.filter(envelope, Hint())
154154
assertNotNull(result)
@@ -176,7 +176,7 @@ class RateLimiterTest {
176176
rateLimiter.updateRetryAfterLimits("1::key, 60:default;error;security:organization", null, 1)
177177

178178
// the shortest limit in the header has now lapsed
179-
fixture.clock.advance(1001, MILLISECONDS)
179+
fixture.ticker.advance(1001, MILLISECONDS)
180180

181181
val result = rateLimiter.filter(envelope, Hint())
182182
assertNull(result)
@@ -191,7 +191,7 @@ class RateLimiterTest {
191191
rateLimiter.updateRetryAfterLimits("60:error:key, 1:error:organization", null, 1)
192192

193193
// the shortest limit in the header has now lapsed
194-
fixture.clock.advance(1001, MILLISECONDS)
194+
fixture.ticker.advance(1001, MILLISECONDS)
195195

196196
val result = rateLimiter.filter(envelope, Hint())
197197
assertNull(result)
@@ -206,7 +206,7 @@ class RateLimiterTest {
206206
rateLimiter.updateRetryAfterLimits("1:error:key, 5:error:organization", null, 1)
207207

208208
// the shortest limit in the header has now lapsed
209-
fixture.clock.advance(1001, MILLISECONDS)
209+
fixture.ticker.advance(1001, MILLISECONDS)
210210

211211
val result = rateLimiter.filter(envelope, Hint())
212212
assertNull(result)
@@ -221,7 +221,7 @@ class RateLimiterTest {
221221
rateLimiter.updateRetryAfterLimits(null, null, 429)
222222

223223
// a second in, the 60s default delay is still running
224-
fixture.clock.advance(1001, MILLISECONDS)
224+
fixture.ticker.advance(1001, MILLISECONDS)
225225

226226
val result = rateLimiter.filter(envelope, Hint())
227227
assertNull(result)
@@ -392,10 +392,10 @@ class RateLimiterTest {
392392

393393
rateLimiter.updateRetryAfterLimits("1:error:key", null, 1)
394394

395-
fixture.clock.advance(999, MILLISECONDS)
395+
fixture.ticker.advance(999, MILLISECONDS)
396396
assertTrue(rateLimiter.isActiveForCategory(DataCategory.Error))
397397

398-
fixture.clock.advance(1, MILLISECONDS)
398+
fixture.ticker.advance(1, MILLISECONDS)
399399
assertFalse(rateLimiter.isActiveForCategory(DataCategory.Error))
400400
}
401401

@@ -698,7 +698,7 @@ class RateLimiterTest {
698698
rateLimiter.updateRetryAfterLimits("1:replay:key", null, 1)
699699

700700
// the notification is scheduled ~1s out in real time; by then the limit has lapsed
701-
fixture.clock.advance(2, SECONDS)
701+
fixture.ticker.advance(2, SECONDS)
702702

703703
await.untilFalse(applied)
704704
assertFalse(applied.get())

0 commit comments

Comments
 (0)