Skip to content

Commit 5c1467e

Browse files
runningcodeclaude
andauthored
ref(transport): Measure rate-limit backoff on a monotonic clock (JAVA-574) (#6030)
* 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> * Add changelog entries for JAVA-574 * ref(transport): Create RateLimiter through a factory (JAVA-574) The RateLimiterConfig constructor is now private behind RateLimiter.create, and both SentryOptions constructors are deprecated. Handing over the whole options object hides which three of its methods a rate limiter reads, which is the very thing the config interface exists to name, so both transport factories call the factory method instead. RateLimiterTest supplies its own RateLimiterConfig rather than a SentryOptions carrying a @testonly executor setter and a reflective client-report-recorder injection. A DeferredExecutorService replaces the real executor, so the lifted-limit notification runs when the test says so instead of a real second later, leaving the deadline's virtual clock as the only clock in the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(transport): Tolerate a negative retry-after delay (JAVA-574) A server-supplied `Retry-After: -1`, or a negative delay in `X-Sentry-Rate-Limits`, reached `Deadline.after` unvalidated and threw `IllegalArgumentException` out of response handling. Clamp a negative amount to an already-passed deadline, restoring the behavior the wall-clock implementation had, and keeping the tick arithmetic away from wrapping on a large negative value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ref(transport): Keep RateLimiter(SentryOptions) as public API (JAVA-574) RateLimiter is public because ITransport.getRateLimiter returns it and a custom transport has to build one. Deprecating the options constructor pointed those callers at RateLimiter.create, whose two parameter types are both @ApiStatus.Internal, so following the deprecation meant reaching into internal API. Un-deprecate the options constructor and mark the factory internal instead, which is what it already was in practice. Both transport factories call the constructor, dropping the getMonotonicTicker/options double-pass and, in the apache module, an internal call from another module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ref(transport): Note why RateLimiter(SentryOptions) stays non-deprecated (JAVA-574) Both transport factories go back to RateLimiter.create, which names the collaborators a rate limiter reads instead of handing over the whole options object. The constructor stays as the one path a caller outside the SDK can take, and now says so: it is only non-deprecated because create takes the internal MonotonicTicker, and should be deprecated once that type is public API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 663ad97 commit 5c1467e

10 files changed

Lines changed: 242 additions & 114 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## 8.56.0
44

5+
### Behavioral Changes
6+
7+
- Measure HTTP rate-limit backoff on a monotonic clock instead of the wall clock, so that a device time change no longer lifts or extends an active rate limit ([#6030](https://github.com/getsentry/sentry-java/pull/6030))
8+
59
### Fixes
610

711
- Update `SentryTraced` so that it now honors `options.setIgnoredSpanOrigins` ([#6058](https://github.com/getsentry/sentry-java/pull/6058))
@@ -19,6 +23,7 @@
1923

2024
- Add an internal `MonotonicTicker` abstraction with `Deadline` and `Stopwatch` primitives ([#6028](https://github.com/getsentry/sentry-java/pull/6028))
2125
- 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))
26+
- Deprecate `RateLimiter(ICurrentDateProvider, SentryOptions)` in favor of `RateLimiter(SentryOptions)`, whose backoff is measured on a monotonic ticker ([#6030](https://github.com/getsentry/sentry-java/pull/6030))
2227

2328
### Dependencies
2429

‎sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransportFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public ApacheHttpClientTransportFactory(final @NotNull TimeValue connectionTimeT
6262
.setResponseTimeout(options.getReadTimeoutMillis(), TimeUnit.MILLISECONDS)
6363
.build())
6464
.build();
65-
final RateLimiter rateLimiter = new RateLimiter(options);
65+
final RateLimiter rateLimiter = RateLimiter.create(options.getMonotonicTicker(), options);
6666

6767
return new ApacheHttpClientTransport(options, requestDetails, httpclient, rateLimiter);
6868
}

‎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/RateLimiter : java/io/Closeable {
77067706
public fun <init> (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V
77077707
public fun addRateLimitObserver (Lio/sentry/transport/RateLimiter$IRateLimitObserver;)V
77087708
public fun close ()V
7709+
public static fun create (Lio/sentry/time/MonotonicTicker;Lio/sentry/transport/RateLimiterConfig;)Lio/sentry/transport/RateLimiter;
77097710
public fun filter (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/SentryEnvelope;
77107711
public fun isActiveForCategory (Lio/sentry/DataCategory;)Z
77117712
public fun isAnyRateLimitActive ()Z
@@ -7717,6 +7718,12 @@ public abstract interface class io/sentry/transport/RateLimiter$IRateLimitObserv
77177718
public abstract fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V
77187719
}
77197720

7721+
public abstract interface class io/sentry/transport/RateLimiterConfig {
7722+
public abstract fun getClientReportRecorder ()Lio/sentry/clientreport/IClientReportRecorder;
7723+
public abstract fun getLogger ()Lio/sentry/ILogger;
7724+
public abstract fun getTimerExecutorService ()Lio/sentry/ISentryExecutorService;
7725+
}
7726+
77207727
public final class io/sentry/transport/ReusableCountLatch {
77217728
public fun <init> ()V
77227729
public fun <init> (I)V

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ public final class AsyncHttpTransportFactory implements ITransportFactory {
1818
Objects.requireNonNull(requestDetails, "requestDetails is required");
1919

2020
return new AsyncHttpTransport(
21-
options, new RateLimiter(options), options.getTransportGate(), requestDetails);
21+
options,
22+
RateLimiter.create(options.getMonotonicTicker(), options),
23+
options.getTransportGate(),
24+
requestDetails);
2225
}
2326
}

‎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

@@ -847,6 +848,7 @@ public void setDebug(final boolean debug) {
847848
*
848849
* @return the logger
849850
*/
851+
@Override
850852
public @NotNull ILogger getLogger() {
851853
return logger;
852854
}
@@ -1610,6 +1612,7 @@ public void setExecutorService(final @NotNull ISentryExecutorService executorSer
16101612
* @return the timer executor service
16111613
*/
16121614
@ApiStatus.Internal
1615+
@Override
16131616
@NotNull
16141617
public ISentryExecutorService getTimerExecutorService() {
16151618
return timerExecutorService;
@@ -2601,6 +2604,7 @@ public void setInstrumenter(final @NotNull Instrumenter instrumenter) {
26012604
* @return a client report recorder or NoOp
26022605
*/
26032606
@ApiStatus.Internal
2607+
@Override
26042608
public @NotNull IClientReportRecorder getClientReportRecorder() {
26052609
return clientReportRecorder;
26062610
}

‎sentry/src/main/java/io/sentry/time/Deadline.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,18 @@ private Deadline(final @NotNull MonotonicTicker ticker, final long deadlineNanos
2424
}
2525

2626
/**
27-
* A deadline {@code amount} of {@code unit} from now.
27+
* A deadline {@code amount} of {@code unit} from now, or one that has already {@link #passed} if
28+
* {@code amount} is negative.
2829
*
29-
* @throws IllegalArgumentException if {@code amount} is negative. A deadline that starts out in
30-
* the past is a sign error at the call site; {@link #passed} says it deliberately.
30+
* <p>Negative amounts are tolerated because callers pass durations parsed from server headers,
31+
* where a bogus value must degrade to "no wait" rather than throw out of response handling.
32+
* Clamping rather than adding a negative offset also keeps the tick arithmetic away from
33+
* wrapping.
3134
*/
3235
public static @NotNull Deadline after(
3336
final @NotNull MonotonicTicker ticker, final long amount, final @NotNull TimeUnit unit) {
3437
if (amount < 0) {
35-
throw new IllegalArgumentException("Deadline amount must not be negative, but was " + amount);
38+
return passed(ticker);
3639
}
3740
return new Deadline(ticker, ticker.tickNanos() + unit.toNanos(amount));
3841
}

0 commit comments

Comments
 (0)