From 2d9b8420db498c799aeb04754b6d91ada891fe97 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 13 Sep 2023 14:22:49 +0200 Subject: [PATCH 1/5] Don't send cached envelopes when rate-limiting is active --- .../core/SendCachedEnvelopeIntegration.java | 11 ++++ .../android/core/InternalSentrySdkTest.kt | 5 ++ .../api/sentry-apache-http-client-5.api | 1 + .../apache/ApacheHttpClientTransport.java | 5 ++ sentry/api/sentry.api | 13 ++++- sentry/src/main/java/io/sentry/Hub.java | 7 +++ .../src/main/java/io/sentry/HubAdapter.java | 6 +++ sentry/src/main/java/io/sentry/IHub.java | 4 ++ .../main/java/io/sentry/ISentryClient.java | 5 ++ sentry/src/main/java/io/sentry/NoOpHub.java | 6 +++ .../main/java/io/sentry/NoOpSentryClient.java | 6 +++ ...achedEnvelopeFireAndForgetIntegration.java | 12 +++++ .../src/main/java/io/sentry/SentryClient.java | 6 +++ .../sentry/transport/AsyncHttpTransport.java | 5 ++ .../java/io/sentry/transport/ITransport.java | 4 ++ .../io/sentry/transport/NoOpTransport.java | 6 +++ .../java/io/sentry/transport/RateLimiter.java | 50 +++++++++---------- .../io/sentry/transport/StdoutTransport.java | 6 +++ 18 files changed, 132 insertions(+), 26 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index 7fb75975b81..4f756b0296c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -1,11 +1,13 @@ package io.sentry.android.core; +import io.sentry.DataCategory; import io.sentry.IConnectionStatusProvider; import io.sentry.IHub; import io.sentry.Integration; import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.transport.RateLimiter; import io.sentry.util.LazyEvaluator; import io.sentry.util.Objects; import java.io.Closeable; @@ -81,6 +83,15 @@ private synchronized void sendCachedEnvelopes( return; } + // in case there's rate limiting active, skip processing + final @Nullable RateLimiter rateLimiter = hub.getRateLimiter(); + if (rateLimiter != null && rateLimiter.isActiveForCategory(DataCategory.All)) { + options + .getLogger() + .log(SentryLevel.INFO, "SendCachedEnvelopeIntegration, rate limiting active."); + return; + } + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = factory.create(hub, options); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt index f10594cf0b7..c2ffb9489ee 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt @@ -23,6 +23,7 @@ import io.sentry.protocol.Mechanism import io.sentry.protocol.SentryId import io.sentry.protocol.User import io.sentry.transport.ITransport +import io.sentry.transport.RateLimiter import org.junit.runner.RunWith import org.mockito.kotlin.mock import org.mockito.kotlin.whenever @@ -63,6 +64,10 @@ class InternalSentrySdkTest { override fun flush(timeoutMillis: Long) { // no-op } + + override fun getRateLimiter(): RateLimiter? { + return null + } } } } diff --git a/sentry-apache-http-client-5/api/sentry-apache-http-client-5.api b/sentry-apache-http-client-5/api/sentry-apache-http-client-5.api index 7d7479c9fd3..9f33a4f115e 100644 --- a/sentry-apache-http-client-5/api/sentry-apache-http-client-5.api +++ b/sentry-apache-http-client-5/api/sentry-apache-http-client-5.api @@ -2,6 +2,7 @@ public final class io/sentry/transport/apache/ApacheHttpClientTransport : io/sen public fun (Lio/sentry/SentryOptions;Lio/sentry/RequestDetails;Lorg/apache/hc/client5/http/impl/async/CloseableHttpAsyncClient;Lio/sentry/transport/RateLimiter;)V public fun close ()V public fun flush (J)V + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun send (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } diff --git a/sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java b/sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java index 8ff2aa01bf8..b5a0dc5c9f6 100644 --- a/sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java +++ b/sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java @@ -189,6 +189,11 @@ public void flush(long timeoutMillis) { } } + @Override + public @NotNull RateLimiter getRateLimiter() { + return rateLimiter; + } + @Override public void close() throws IOException { options.getLogger().log(DEBUG, "Shutting down"); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6b96967eb3a..60e3bce14b2 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -373,6 +373,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun getBaggage ()Lio/sentry/BaggageHeader; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun getSpan ()Lio/sentry/ISpan; public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun getTransaction ()Lio/sentry/ITransaction; @@ -422,6 +423,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public static fun getInstance ()Lio/sentry/HubAdapter; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun getSpan ()Lio/sentry/ISpan; public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun getTransaction ()Lio/sentry/ITransaction; @@ -515,6 +517,7 @@ public abstract interface class io/sentry/IHub { public abstract fun getBaggage ()Lio/sentry/BaggageHeader; public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; public abstract fun getOptions ()Lio/sentry/SentryOptions; + public abstract fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public abstract fun getSpan ()Lio/sentry/ISpan; public abstract fun getTraceparent ()Lio/sentry/SentryTraceHeader; public abstract fun getTransaction ()Lio/sentry/ITransaction; @@ -608,6 +611,7 @@ public abstract interface class io/sentry/ISentryClient { public abstract fun captureUserFeedback (Lio/sentry/UserFeedback;)V public abstract fun close ()V public abstract fun flush (J)V + public abstract fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public abstract fun isEnabled ()Z } @@ -910,6 +914,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public static fun getInstance ()Lio/sentry/NoOpHub; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun getSpan ()Lio/sentry/ISpan; public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun getTransaction ()Lio/sentry/ITransaction; @@ -1516,6 +1521,7 @@ public final class io/sentry/SentryClient : io/sentry/ISentryClient { public fun captureUserFeedback (Lio/sentry/UserFeedback;)V public fun close ()V public fun flush (J)V + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun isEnabled ()Z } @@ -2472,6 +2478,7 @@ public final class io/sentry/TypeCheckHint { public static final field OKHTTP_RESPONSE Ljava/lang/String; public static final field OPEN_FEIGN_REQUEST Ljava/lang/String; public static final field OPEN_FEIGN_RESPONSE Ljava/lang/String; + public static final field SENTRY_CACHED_ENVELOPE_FILE_PATH Ljava/lang/String; public static final field SENTRY_DART_SDK_NAME Ljava/lang/String; public static final field SENTRY_DOTNET_SDK_NAME Ljava/lang/String; public static final field SENTRY_EVENT_DROP_REASON Ljava/lang/String; @@ -4137,6 +4144,7 @@ public final class io/sentry/transport/AsyncHttpTransport : io/sentry/transport/ public fun (Lio/sentry/transport/QueuedThreadPoolExecutor;Lio/sentry/SentryOptions;Lio/sentry/transport/RateLimiter;Lio/sentry/transport/ITransportGate;Lio/sentry/transport/HttpConnection;)V public fun close ()V public fun flush (J)V + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun send (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } @@ -4151,6 +4159,7 @@ public abstract interface class io/sentry/transport/ICurrentDateProvider { public abstract interface class io/sentry/transport/ITransport : java/io/Closeable { public abstract fun flush (J)V + public abstract fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun send (Lio/sentry/SentryEnvelope;)V public abstract fun send (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } @@ -4172,6 +4181,7 @@ public final class io/sentry/transport/NoOpTransport : io/sentry/transport/ITran public fun close ()V public fun flush (J)V public static fun getInstance ()Lio/sentry/transport/NoOpTransport; + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun send (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } @@ -4184,7 +4194,7 @@ public final class io/sentry/transport/RateLimiter { public fun (Lio/sentry/SentryOptions;)V public fun (Lio/sentry/transport/ICurrentDateProvider;Lio/sentry/SentryOptions;)V public fun filter (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/SentryEnvelope; - public fun getRateLimitedUntilFor (Ljava/lang/String;)Ljava/util/Date; + public fun isActiveForCategory (Lio/sentry/DataCategory;)Z public fun updateRetryAfterLimits (Ljava/lang/String;Ljava/lang/String;I)V } @@ -4202,6 +4212,7 @@ public final class io/sentry/transport/StdoutTransport : io/sentry/transport/ITr public fun (Lio/sentry/ISerializer;)V public fun close ()V public fun flush (J)V + public fun getRateLimiter ()Lio/sentry/transport/RateLimiter; public fun send (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index cc6dc0e6ef8..cd1477f81fe 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -7,6 +7,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import io.sentry.transport.RateLimiter; import io.sentry.util.ExceptionUtils; import io.sentry.util.HintUtils; import io.sentry.util.Objects; @@ -883,4 +884,10 @@ private Scope buildLocalScope( return null; } + + @Override + public @Nullable RateLimiter getRateLimiter() { + final StackItem item = stack.peek(); + return item.getClient().getRateLimiter(); + } } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 1a64484a6b7..ad476284acd 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -3,6 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import io.sentry.transport.RateLimiter; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -262,4 +263,9 @@ public void reportFullyDisplayed() { public @Nullable BaggageHeader getBaggage() { return Sentry.getBaggage(); } + + @Override + public @Nullable RateLimiter getRateLimiter() { + return Sentry.getCurrentHub().getRateLimiter(); + } } diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 36b695e11b3..fe2b797e113 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -3,6 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import io.sentry.transport.RateLimiter; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -633,4 +634,7 @@ TransactionContext continueTrace( */ @Nullable BaggageHeader getBaggage(); + + @Nullable + RateLimiter getRateLimiter(); } diff --git a/sentry/src/main/java/io/sentry/ISentryClient.java b/sentry/src/main/java/io/sentry/ISentryClient.java index 4789eb3deca..0978d1d97b9 100644 --- a/sentry/src/main/java/io/sentry/ISentryClient.java +++ b/sentry/src/main/java/io/sentry/ISentryClient.java @@ -3,6 +3,7 @@ import io.sentry.protocol.Message; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; +import io.sentry.transport.RateLimiter; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -264,4 +265,8 @@ SentryId captureTransaction( default @NotNull SentryId captureTransaction(@NotNull SentryTransaction transaction) { return captureTransaction(transaction, null, null, null); } + + @ApiStatus.Internal + @Nullable + RateLimiter getRateLimiter(); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 79a0fb1960d..719d9e2ffae 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -3,6 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import io.sentry.transport.RateLimiter; import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -215,4 +216,9 @@ public void reportFullyDisplayed() {} public @Nullable BaggageHeader getBaggage() { return null; } + + @Override + public @Nullable RateLimiter getRateLimiter() { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryClient.java b/sentry/src/main/java/io/sentry/NoOpSentryClient.java index 4afbdbb8c89..f2f8a16ba0d 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryClient.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryClient.java @@ -2,6 +2,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; +import io.sentry.transport.RateLimiter; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -52,4 +53,9 @@ public SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint @Nullable ProfilingTraceData profilingTraceData) { return SentryId.EMPTY_ID; } + + @Override + public @Nullable RateLimiter getRateLimiter() { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 739c3ee0d2f..ae41194d556 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -2,6 +2,7 @@ import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion; +import io.sentry.transport.RateLimiter; import io.sentry.util.Objects; import java.io.Closeable; import java.io.File; @@ -112,6 +113,17 @@ private synchronized void sendCachedEnvelopes(@NotNull IHub hub, @NotNull Sentry return; } + // in case there's rate limiting active, skip processing + final @Nullable RateLimiter rateLimiter = hub.getRateLimiter(); + if (rateLimiter != null && rateLimiter.isActiveForCategory(DataCategory.All)) { + options + .getLogger() + .log( + SentryLevel.INFO, + "SendCachedEnvelopeFireAndForgetIntegration, rate limiting active."); + return; + } + final SendFireAndForget sender = factory.create(hub, options); if (sender == null) { options.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index bd981842e38..c10721c66dc 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -9,6 +9,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.transport.ITransport; +import io.sentry.transport.RateLimiter; import io.sentry.util.HintUtils; import io.sentry.util.Objects; import io.sentry.util.TracingUtils; @@ -819,6 +820,11 @@ public void flush(final long timeoutMillis) { transport.flush(timeoutMillis); } + @Override + public @Nullable RateLimiter getRateLimiter() { + return transport.getRateLimiter(); + } + private boolean sample() { // https://docs.sentry.io/development/sdk-dev/features/#event-sampling if (options.getSampleRate() != null && random != null) { diff --git a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java index 3cf0712b7f3..d1736504730 100644 --- a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java @@ -135,6 +135,11 @@ private static QueuedThreadPoolExecutor initExecutor( 1, maxQueueSize, new AsyncConnectionThreadFactory(), storeEvents, logger); } + @Override + public @NotNull RateLimiter getRateLimiter() { + return rateLimiter; + } + @Override public void close() throws IOException { executor.shutdown(); diff --git a/sentry/src/main/java/io/sentry/transport/ITransport.java b/sentry/src/main/java/io/sentry/transport/ITransport.java index 131a5f04070..09fc034246c 100644 --- a/sentry/src/main/java/io/sentry/transport/ITransport.java +++ b/sentry/src/main/java/io/sentry/transport/ITransport.java @@ -5,6 +5,7 @@ import java.io.Closeable; import java.io.IOException; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** A transport is in charge of sending the event to the Sentry server. */ public interface ITransport extends Closeable { @@ -20,4 +21,7 @@ default void send(@NotNull SentryEnvelope envelope) throws IOException { * @param timeoutMillis time in milliseconds */ void flush(long timeoutMillis); + + @Nullable + RateLimiter getRateLimiter(); } diff --git a/sentry/src/main/java/io/sentry/transport/NoOpTransport.java b/sentry/src/main/java/io/sentry/transport/NoOpTransport.java index 79d639ee0ba..f73605b049d 100644 --- a/sentry/src/main/java/io/sentry/transport/NoOpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/NoOpTransport.java @@ -5,6 +5,7 @@ import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class NoOpTransport implements ITransport { @@ -24,6 +25,11 @@ public void send(final @NotNull SentryEnvelope envelope, final @NotNull Hint hin @Override public void flush(long timeoutMillis) {} + @Override + public @Nullable RateLimiter getRateLimiter() { + return null; + } + @Override public void close() throws IOException {} } diff --git a/sentry/src/main/java/io/sentry/transport/RateLimiter.java b/sentry/src/main/java/io/sentry/transport/RateLimiter.java index 0c818707a6f..ed4c04c6309 100644 --- a/sentry/src/main/java/io/sentry/transport/RateLimiter.java +++ b/sentry/src/main/java/io/sentry/transport/RateLimiter.java @@ -48,7 +48,7 @@ public RateLimiter(final @NotNull SentryOptions options) { // Optimize for/No allocations if no items are under 429 List dropItems = null; for (SentryEnvelopeItem item : envelope.getItems()) { - // using the raw value of the enum to not expose SentryEnvelopeItemType + // using the raw value of the enum to not expose SentryEnvelopeItemType if (isRetryAfter(item.getHeader().getType().getItemType())) { if (dropItems == null) { dropItems = new ArrayList<>(); @@ -87,31 +87,8 @@ public RateLimiter(final @NotNull SentryOptions options) { return envelope; } - @Nullable - public Date getRateLimitedUntilFor(final @NotNull String itemType) { - return null; - } - - /** - * It marks the hint when sending has failed, so it's not necessary to wait the timeout - * - * @param hint the Hints - * @param retry if event should be retried or not - */ - private static void markHintWhenSendingFailed(final @NotNull Hint hint, final boolean retry) { - HintUtils.runIfHasType(hint, SubmissionResult.class, result -> result.setResult(false)); - HintUtils.runIfHasType(hint, Retryable.class, retryable -> retryable.setRetry(retry)); - } - - /** - * Check if an itemType is retry after or not - * - * @param itemType the itemType (eg event, session, etc...) - * @return true if retry after or false otherwise - */ @SuppressWarnings({"JdkObsolete", "JavaUtilDate"}) - private boolean isRetryAfter(final @NotNull String itemType) { - final DataCategory dataCategory = getCategoryFromItemType(itemType); + public boolean isActiveForCategory(final @NotNull DataCategory dataCategory) { final Date currentDate = new Date(currentDateProvider.getCurrentTimeMillis()); // check all categories @@ -136,6 +113,29 @@ private boolean isRetryAfter(final @NotNull String itemType) { return false; } + /** + * It marks the hint when sending has failed, so it's not necessary to wait the timeout + * + * @param hint the Hints + * @param retry if event should be retried or not + */ + private static void markHintWhenSendingFailed(final @NotNull Hint hint, final boolean retry) { + HintUtils.runIfHasType(hint, SubmissionResult.class, result -> result.setResult(false)); + HintUtils.runIfHasType(hint, Retryable.class, retryable -> retryable.setRetry(retry)); + } + + /** + * Check if an itemType is retry after or not + * + * @param itemType the itemType (eg event, session, etc...) + * @return true if retry after or false otherwise + */ + @SuppressWarnings({"JdkObsolete", "JavaUtilDate"}) + private boolean isRetryAfter(final @NotNull String itemType) { + final DataCategory dataCategory = getCategoryFromItemType(itemType); + return isActiveForCategory(dataCategory); + } + /** * Returns a rate limiting category from item itemType * diff --git a/sentry/src/main/java/io/sentry/transport/StdoutTransport.java b/sentry/src/main/java/io/sentry/transport/StdoutTransport.java index c503df9f94b..99aed10eac7 100644 --- a/sentry/src/main/java/io/sentry/transport/StdoutTransport.java +++ b/sentry/src/main/java/io/sentry/transport/StdoutTransport.java @@ -6,6 +6,7 @@ import io.sentry.util.Objects; import java.io.IOException; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public final class StdoutTransport implements ITransport { @@ -33,6 +34,11 @@ public void flush(long timeoutMillis) { System.out.println("Flushing"); } + @Override + public @Nullable RateLimiter getRateLimiter() { + return null; + } + @Override public void close() {} } From 01bdce99b738045be26b4b436392199b5111a38e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 26 Sep 2023 13:39:07 +0200 Subject: [PATCH 2/5] Dont process envelopes that are already enqueued --- .../java/io/sentry/DirectoryProcessor.java | 34 +++++++++++++++---- .../main/java/io/sentry/EnvelopeSender.java | 16 ++------- .../src/main/java/io/sentry/OutboxSender.java | 3 +- .../SendFireAndForgetEnvelopeSender.java | 4 +-- .../main/java/io/sentry/TypeCheckHint.java | 3 -- .../java/io/sentry/cache/EnvelopeCache.java | 23 +++++-------- .../main/java/io/sentry/hints/Enqueable.java | 5 +++ .../sentry/transport/AsyncHttpTransport.java | 6 ++++ .../java/io/sentry/transport/RateLimiter.java | 2 +- .../test/java/io/sentry/EnvelopeSenderTest.kt | 9 +++-- 10 files changed, 58 insertions(+), 47 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/hints/Enqueable.java diff --git a/sentry/src/main/java/io/sentry/DirectoryProcessor.java b/sentry/src/main/java/io/sentry/DirectoryProcessor.java index 9387c502b01..9be1eb83e45 100644 --- a/sentry/src/main/java/io/sentry/DirectoryProcessor.java +++ b/sentry/src/main/java/io/sentry/DirectoryProcessor.java @@ -3,11 +3,13 @@ import static io.sentry.SentryLevel.ERROR; import io.sentry.hints.Cached; +import io.sentry.hints.Enqueable; import io.sentry.hints.Flushable; import io.sentry.hints.Retryable; import io.sentry.hints.SubmissionResult; import io.sentry.util.HintUtils; import java.io.File; +import java.util.Queue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; @@ -17,9 +19,12 @@ abstract class DirectoryProcessor { private final @NotNull ILogger logger; private final long flushTimeoutMillis; - DirectoryProcessor(final @NotNull ILogger logger, final long flushTimeoutMillis) { + private final Queue processedEnvelopes; + + DirectoryProcessor(final @NotNull ILogger logger, final long flushTimeoutMillis, final int maxQueueSize) { this.logger = logger; this.flushTimeoutMillis = flushTimeoutMillis; + this.processedEnvelopes = SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxQueueSize)); } public void processDirectory(final @NotNull File directory) { @@ -60,13 +65,22 @@ public void processDirectory(final @NotNull File directory) { continue; } - logger.log(SentryLevel.DEBUG, "Processing file: %s", file.getAbsolutePath()); + final String filePath = file.getAbsolutePath(); + // if envelope has already been submitted into the transport queue, we don't process it again + if (processedEnvelopes.contains(filePath)) { + logger.log( + SentryLevel.DEBUG, + "File '%s' has already been processed so it will not be processed again.", + filePath); + continue; + } + + logger.log(SentryLevel.DEBUG, "Processing file: %s", filePath); final SendCachedEnvelopeHint cachedHint = - new SendCachedEnvelopeHint(flushTimeoutMillis, logger); + new SendCachedEnvelopeHint(flushTimeoutMillis, logger, () -> processedEnvelopes.add(filePath)); final Hint hint = HintUtils.createWithTypeCheckHint(cachedHint); - hint.set(TypeCheckHint.SENTRY_CACHED_ENVELOPE_FILE_PATH, file.getAbsolutePath()); processFile(file, hint); } } catch (Throwable e) { @@ -79,16 +93,19 @@ public void processDirectory(final @NotNull File directory) { protected abstract boolean isRelevantFileName(String fileName); private static final class SendCachedEnvelopeHint - implements Cached, Retryable, SubmissionResult, Flushable { + implements Cached, Retryable, SubmissionResult, Flushable, Enqueable { boolean retry = false; boolean succeeded = false; private final CountDownLatch latch; private final long flushTimeoutMillis; private final @NotNull ILogger logger; + private final @NotNull Runnable onEnqueued; - public SendCachedEnvelopeHint(final long flushTimeoutMillis, final @NotNull ILogger logger) { + public SendCachedEnvelopeHint(final long flushTimeoutMillis, final @NotNull ILogger logger, + final @NotNull Runnable onEnqueued) { this.flushTimeoutMillis = flushTimeoutMillis; + this.onEnqueued = onEnqueued; this.latch = new CountDownLatch(1); this.logger = logger; } @@ -124,5 +141,10 @@ public void setResult(boolean succeeded) { public boolean isSuccess() { return succeeded; } + + @Override + public void markEnqueued() { + onEnqueued.run(); + } } } diff --git a/sentry/src/main/java/io/sentry/EnvelopeSender.java b/sentry/src/main/java/io/sentry/EnvelopeSender.java index 9b0b94ec293..d6f8f4849a9 100644 --- a/sentry/src/main/java/io/sentry/EnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/EnvelopeSender.java @@ -1,7 +1,6 @@ package io.sentry; import io.sentry.cache.EnvelopeCache; -import io.sentry.cache.IEnvelopeCache; import io.sentry.hints.Flushable; import io.sentry.hints.Retryable; import io.sentry.util.HintUtils; @@ -21,19 +20,16 @@ public final class EnvelopeSender extends DirectoryProcessor implements IEnvelop private final @NotNull IHub hub; private final @NotNull ISerializer serializer; private final @NotNull ILogger logger; - private final @NotNull IEnvelopeCache envelopeCache; public EnvelopeSender( final @NotNull IHub hub, final @NotNull ISerializer serializer, final @NotNull ILogger logger, - final long flushTimeoutMillis, - final @NotNull IEnvelopeCache envelopeCache) { - super(logger, flushTimeoutMillis); + final long flushTimeoutMillis) { + super(logger, flushTimeoutMillis, hub.getOptions().getMaxQueueSize()); this.hub = Objects.requireNonNull(hub, "Hub is required."); this.serializer = Objects.requireNonNull(serializer, "Serializer is required."); this.logger = Objects.requireNonNull(logger, "Logger is required."); - this.envelopeCache = Objects.requireNonNull(envelopeCache, "IEnvelopeCache is required."); } @Override @@ -57,14 +53,6 @@ protected void processFile(final @NotNull File file, final @NotNull Hint hint) { return; } - if (envelopeCache.containsFile(file)) { - logger.log( - SentryLevel.DEBUG, - "File '%s' is already in the cache so it will not be processed again.", - file.getAbsolutePath()); - return; - } - try (final InputStream is = new BufferedInputStream(new FileInputStream(file))) { SentryEnvelope envelope = serializer.deserializeEnvelope(is); if (envelope == null) { diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 9e704f0a0e9..f2c503015f1 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -11,6 +11,7 @@ import io.sentry.hints.SubmissionResult; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; +import io.sentry.transport.RateLimiter; import io.sentry.util.CollectionUtils; import io.sentry.util.HintUtils; import io.sentry.util.LogUtils; @@ -47,7 +48,7 @@ public OutboxSender( final @NotNull ISerializer serializer, final @NotNull ILogger logger, final long flushTimeoutMillis) { - super(logger, flushTimeoutMillis); + super(logger, flushTimeoutMillis, hub.getOptions().getMaxQueueSize()); this.hub = Objects.requireNonNull(hub, "Hub is required."); this.envelopeReader = Objects.requireNonNull(envelopeReader, "Envelope reader is required."); this.serializer = Objects.requireNonNull(serializer, "Serializer is required."); diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java index b67a53d34b9..80005e410a7 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java @@ -36,8 +36,8 @@ public SendFireAndForgetEnvelopeSender( hub, options.getSerializer(), options.getLogger(), - options.getFlushTimeoutMillis(), - options.getEnvelopeDiskCache()); + options.getFlushTimeoutMillis() + ); return processDir(envelopeSender, dirPath, options.getLogger()); } diff --git a/sentry/src/main/java/io/sentry/TypeCheckHint.java b/sentry/src/main/java/io/sentry/TypeCheckHint.java index 289e7112558..3c66307602c 100644 --- a/sentry/src/main/java/io/sentry/TypeCheckHint.java +++ b/sentry/src/main/java/io/sentry/TypeCheckHint.java @@ -13,9 +13,6 @@ public final class TypeCheckHint { @ApiStatus.Internal public static final String SENTRY_EVENT_DROP_REASON = "sentry:eventDropReason"; - @ApiStatus.Internal - public static final String SENTRY_CACHED_ENVELOPE_FILE_PATH = "sentry:cachedEnvelopeFilePath"; - @ApiStatus.Internal public static final String SENTRY_JAVASCRIPT_SDK_NAME = "sentry.javascript"; @ApiStatus.Internal public static final String SENTRY_DOTNET_SDK_NAME = "sentry.dotnet"; diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 06264895d74..020cae02f0d 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -163,7 +163,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi // TODO: probably we need to update the current session file for session updates to because of // hardcrash events - final File envelopeFile = getEnvelopeFile(envelope, hint); + final File envelopeFile = getEnvelopeFile(envelope); if (envelopeFile.exists()) { options .getLogger() @@ -339,7 +339,7 @@ private void writeSessionToDisk(final @NotNull File file, final @NotNull Session public void discard(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { Objects.requireNonNull(envelope, "Envelope is required."); - final File envelopeFile = getEnvelopeFile(envelope, hint); + final File envelopeFile = getEnvelopeFile(envelope); if (envelopeFile.exists()) { options .getLogger() @@ -363,25 +363,18 @@ public void discard(final @NotNull SentryEnvelope envelope, final @NotNull Hint * @return the file */ private synchronized @NotNull File getEnvelopeFile( - final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { + final @NotNull SentryEnvelope envelope) { String fileName; if (fileNameMap.containsKey(envelope)) { fileName = fileNameMap.get(envelope); } else { - final @Nullable Object cachedFilePath = - hint.get(TypeCheckHint.SENTRY_CACHED_ENVELOPE_FILE_PATH); - if (cachedFilePath instanceof String) { - fileName = (String) cachedFilePath; - fileNameMap.put(envelope, fileName); + if (envelope.getHeader().getEventId() != null) { + fileName = envelope.getHeader().getEventId().toString(); } else { - if (envelope.getHeader().getEventId() != null) { - fileName = envelope.getHeader().getEventId().toString(); - } else { - fileName = UUID.randomUUID().toString(); - } - fileName += SUFFIX_ENVELOPE_FILE; - fileNameMap.put(envelope, fileName); + fileName = UUID.randomUUID().toString(); } + fileName += SUFFIX_ENVELOPE_FILE; + fileNameMap.put(envelope, fileName); } return new File(directory.getAbsolutePath(), fileName); diff --git a/sentry/src/main/java/io/sentry/hints/Enqueable.java b/sentry/src/main/java/io/sentry/hints/Enqueable.java new file mode 100644 index 00000000000..293423a8fe3 --- /dev/null +++ b/sentry/src/main/java/io/sentry/hints/Enqueable.java @@ -0,0 +1,5 @@ +package io.sentry.hints; + +public interface Enqueable { + void markEnqueued(); +} diff --git a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java index d1736504730..49c0c3e1ab2 100644 --- a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java @@ -13,6 +13,7 @@ import io.sentry.clientreport.DiscardReason; import io.sentry.hints.Cached; import io.sentry.hints.DiskFlushNotification; +import io.sentry.hints.Enqueable; import io.sentry.hints.Retryable; import io.sentry.hints.SubmissionResult; import io.sentry.util.HintUtils; @@ -103,6 +104,11 @@ public void send(final @NotNull SentryEnvelope envelope, final @NotNull Hint hin options .getClientReportRecorder() .recordLostEnvelope(DiscardReason.QUEUE_OVERFLOW, envelopeThatMayIncludeClientReport); + } else { + HintUtils.runIfHasType(hint, Enqueable.class, enqueable -> { + enqueable.markEnqueued(); + options.getLogger().log(SentryLevel.DEBUG, "Envelope enqueued"); + }); } } } diff --git a/sentry/src/main/java/io/sentry/transport/RateLimiter.java b/sentry/src/main/java/io/sentry/transport/RateLimiter.java index ed4c04c6309..82654733491 100644 --- a/sentry/src/main/java/io/sentry/transport/RateLimiter.java +++ b/sentry/src/main/java/io/sentry/transport/RateLimiter.java @@ -131,7 +131,7 @@ private static void markHintWhenSendingFailed(final @NotNull Hint hint, final bo * @return true if retry after or false otherwise */ @SuppressWarnings({"JdkObsolete", "JavaUtilDate"}) - private boolean isRetryAfter(final @NotNull String itemType) { + public boolean isRetryAfter(final @NotNull String itemType) { final DataCategory dataCategory = getCategoryFromItemType(itemType); return isActiveForCategory(dataCategory); } diff --git a/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt b/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt index 08490ee17f1..1d729971992 100644 --- a/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt +++ b/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt @@ -33,11 +33,10 @@ class EnvelopeSenderTest { fun getSut(): EnvelopeSender { return EnvelopeSender( - hub!!, - serializer!!, - logger!!, - options.flushTimeoutMillis, - options.envelopeDiskCache + hub!!, + serializer!!, + logger!!, + options.flushTimeoutMillis ) } } From 3acb766733401525039750388b5cf9c0b41a9995 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 26 Sep 2023 11:44:47 +0000 Subject: [PATCH 3/5] Format code --- .../java/io/sentry/DirectoryProcessor.java | 24 ++++++++++++------- .../src/main/java/io/sentry/OutboxSender.java | 1 - .../SendFireAndForgetEnvelopeSender.java | 6 +---- .../java/io/sentry/cache/EnvelopeCache.java | 4 +--- .../sentry/transport/AsyncHttpTransport.java | 11 +++++---- .../test/java/io/sentry/EnvelopeSenderTest.kt | 8 +++---- 6 files changed, 28 insertions(+), 26 deletions(-) diff --git a/sentry/src/main/java/io/sentry/DirectoryProcessor.java b/sentry/src/main/java/io/sentry/DirectoryProcessor.java index 9be1eb83e45..7f6f178ebb2 100644 --- a/sentry/src/main/java/io/sentry/DirectoryProcessor.java +++ b/sentry/src/main/java/io/sentry/DirectoryProcessor.java @@ -21,10 +21,12 @@ abstract class DirectoryProcessor { private final Queue processedEnvelopes; - DirectoryProcessor(final @NotNull ILogger logger, final long flushTimeoutMillis, final int maxQueueSize) { + DirectoryProcessor( + final @NotNull ILogger logger, final long flushTimeoutMillis, final int maxQueueSize) { this.logger = logger; this.flushTimeoutMillis = flushTimeoutMillis; - this.processedEnvelopes = SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxQueueSize)); + this.processedEnvelopes = + SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxQueueSize)); } public void processDirectory(final @NotNull File directory) { @@ -66,19 +68,21 @@ public void processDirectory(final @NotNull File directory) { } final String filePath = file.getAbsolutePath(); - // if envelope has already been submitted into the transport queue, we don't process it again + // if envelope has already been submitted into the transport queue, we don't process it + // again if (processedEnvelopes.contains(filePath)) { logger.log( - SentryLevel.DEBUG, - "File '%s' has already been processed so it will not be processed again.", - filePath); + SentryLevel.DEBUG, + "File '%s' has already been processed so it will not be processed again.", + filePath); continue; } logger.log(SentryLevel.DEBUG, "Processing file: %s", filePath); final SendCachedEnvelopeHint cachedHint = - new SendCachedEnvelopeHint(flushTimeoutMillis, logger, () -> processedEnvelopes.add(filePath)); + new SendCachedEnvelopeHint( + flushTimeoutMillis, logger, () -> processedEnvelopes.add(filePath)); final Hint hint = HintUtils.createWithTypeCheckHint(cachedHint); processFile(file, hint); @@ -102,8 +106,10 @@ private static final class SendCachedEnvelopeHint private final @NotNull ILogger logger; private final @NotNull Runnable onEnqueued; - public SendCachedEnvelopeHint(final long flushTimeoutMillis, final @NotNull ILogger logger, - final @NotNull Runnable onEnqueued) { + public SendCachedEnvelopeHint( + final long flushTimeoutMillis, + final @NotNull ILogger logger, + final @NotNull Runnable onEnqueued) { this.flushTimeoutMillis = flushTimeoutMillis; this.onEnqueued = onEnqueued; this.latch = new CountDownLatch(1); diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index f2c503015f1..69cfbb6bcbe 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -11,7 +11,6 @@ import io.sentry.hints.SubmissionResult; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; -import io.sentry.transport.RateLimiter; import io.sentry.util.CollectionUtils; import io.sentry.util.HintUtils; import io.sentry.util.LogUtils; diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java index 80005e410a7..ecf4cb79136 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java @@ -33,11 +33,7 @@ public SendFireAndForgetEnvelopeSender( final EnvelopeSender envelopeSender = new EnvelopeSender( - hub, - options.getSerializer(), - options.getLogger(), - options.getFlushTimeoutMillis() - ); + hub, options.getSerializer(), options.getLogger(), options.getFlushTimeoutMillis()); return processDir(envelopeSender, dirPath, options.getLogger()); } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 020cae02f0d..5582379639c 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -16,7 +16,6 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.Session; -import io.sentry.TypeCheckHint; import io.sentry.UncaughtExceptionHandlerIntegration; import io.sentry.hints.AbnormalExit; import io.sentry.hints.SessionEnd; @@ -362,8 +361,7 @@ public void discard(final @NotNull SentryEnvelope envelope, final @NotNull Hint * @param envelope the SentryEnvelope object * @return the file */ - private synchronized @NotNull File getEnvelopeFile( - final @NotNull SentryEnvelope envelope) { + private synchronized @NotNull File getEnvelopeFile(final @NotNull SentryEnvelope envelope) { String fileName; if (fileNameMap.containsKey(envelope)) { fileName = fileNameMap.get(envelope); diff --git a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java index 49c0c3e1ab2..f3b235a4abb 100644 --- a/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java +++ b/sentry/src/main/java/io/sentry/transport/AsyncHttpTransport.java @@ -105,10 +105,13 @@ public void send(final @NotNull SentryEnvelope envelope, final @NotNull Hint hin .getClientReportRecorder() .recordLostEnvelope(DiscardReason.QUEUE_OVERFLOW, envelopeThatMayIncludeClientReport); } else { - HintUtils.runIfHasType(hint, Enqueable.class, enqueable -> { - enqueable.markEnqueued(); - options.getLogger().log(SentryLevel.DEBUG, "Envelope enqueued"); - }); + HintUtils.runIfHasType( + hint, + Enqueable.class, + enqueable -> { + enqueable.markEnqueued(); + options.getLogger().log(SentryLevel.DEBUG, "Envelope enqueued"); + }); } } } diff --git a/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt b/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt index 1d729971992..345dbd07191 100644 --- a/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt +++ b/sentry/src/test/java/io/sentry/EnvelopeSenderTest.kt @@ -33,10 +33,10 @@ class EnvelopeSenderTest { fun getSut(): EnvelopeSender { return EnvelopeSender( - hub!!, - serializer!!, - logger!!, - options.flushTimeoutMillis + hub!!, + serializer!!, + logger!!, + options.flushTimeoutMillis ) } } From 9a2b6d2d6bf10712051325a5550e5c0bec696e49 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 27 Sep 2023 12:35:13 +0200 Subject: [PATCH 4/5] Check rate limits in the for-loop and add a 100 delay --- .../java/io/sentry/DirectoryProcessor.java | 39 +++++++++++++++++-- .../main/java/io/sentry/EnvelopeSender.java | 2 +- .../src/main/java/io/sentry/OutboxSender.java | 2 +- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/sentry/src/main/java/io/sentry/DirectoryProcessor.java b/sentry/src/main/java/io/sentry/DirectoryProcessor.java index 7f6f178ebb2..8384bfb3065 100644 --- a/sentry/src/main/java/io/sentry/DirectoryProcessor.java +++ b/sentry/src/main/java/io/sentry/DirectoryProcessor.java @@ -7,26 +7,30 @@ import io.sentry.hints.Flushable; import io.sentry.hints.Retryable; import io.sentry.hints.SubmissionResult; +import io.sentry.transport.RateLimiter; import io.sentry.util.HintUtils; import java.io.File; import java.util.Queue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; abstract class DirectoryProcessor { + private static final long ENVELOPE_PROCESS_DELAY = 100L; + private final @NotNull IHub hub; private final @NotNull ILogger logger; private final long flushTimeoutMillis; - private final Queue processedEnvelopes; - DirectoryProcessor( - final @NotNull ILogger logger, final long flushTimeoutMillis, final int maxQueueSize) { + DirectoryProcessor(final @NotNull IHub hub, + final @NotNull ILogger logger, final long flushTimeoutMillis) { + this.hub = hub; this.logger = logger; this.flushTimeoutMillis = flushTimeoutMillis; this.processedEnvelopes = - SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxQueueSize)); + SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(hub.getOptions().getMaxQueueSize())); } public void processDirectory(final @NotNull File directory) { @@ -78,6 +82,15 @@ public void processDirectory(final @NotNull File directory) { continue; } + // in case there's rate limiting active, skip processing + final @Nullable RateLimiter rateLimiter = hub.getRateLimiter(); + if (rateLimiter != null && rateLimiter.isActiveForCategory(DataCategory.All)) { + logger.log( + SentryLevel.INFO, + "DirectoryProcessor, rate limiting active."); + return; + } + logger.log(SentryLevel.DEBUG, "Processing file: %s", filePath); final SendCachedEnvelopeHint cachedHint = @@ -86,6 +99,24 @@ public void processDirectory(final @NotNull File directory) { final Hint hint = HintUtils.createWithTypeCheckHint(cachedHint); processFile(file, hint); + + // a short delay between processing envelopes to avoid bursting our server and hitting + // another rate limit https://develop.sentry.dev/sdk/features/#additional-capabilities + try { + Thread.sleep(ENVELOPE_PROCESS_DELAY); + } catch (InterruptedException e) { + try { + Thread.currentThread().interrupt(); + } catch (SecurityException ignored) { + logger.log( + SentryLevel.INFO, + "Failed to interrupt due to SecurityException: %s", + e.getMessage()); + return; + } + logger.log(SentryLevel.INFO, "Interrupted: %s", e.getMessage()); + return; + } } } catch (Throwable e) { logger.log(SentryLevel.ERROR, e, "Failed processing '%s'", directory.getAbsolutePath()); diff --git a/sentry/src/main/java/io/sentry/EnvelopeSender.java b/sentry/src/main/java/io/sentry/EnvelopeSender.java index d6f8f4849a9..dbcc832111d 100644 --- a/sentry/src/main/java/io/sentry/EnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/EnvelopeSender.java @@ -26,7 +26,7 @@ public EnvelopeSender( final @NotNull ISerializer serializer, final @NotNull ILogger logger, final long flushTimeoutMillis) { - super(logger, flushTimeoutMillis, hub.getOptions().getMaxQueueSize()); + super(hub, logger, flushTimeoutMillis); this.hub = Objects.requireNonNull(hub, "Hub is required."); this.serializer = Objects.requireNonNull(serializer, "Serializer is required."); this.logger = Objects.requireNonNull(logger, "Logger is required."); diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 69cfbb6bcbe..7ce2983a659 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -47,7 +47,7 @@ public OutboxSender( final @NotNull ISerializer serializer, final @NotNull ILogger logger, final long flushTimeoutMillis) { - super(logger, flushTimeoutMillis, hub.getOptions().getMaxQueueSize()); + super(hub, logger, flushTimeoutMillis); this.hub = Objects.requireNonNull(hub, "Hub is required."); this.envelopeReader = Objects.requireNonNull(envelopeReader, "Envelope reader is required."); this.serializer = Objects.requireNonNull(serializer, "Serializer is required."); From 4a4d6a40ccf844a08acb9dd7f3f9b8a8ac08cb6a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 27 Sep 2023 12:40:04 +0200 Subject: [PATCH 5/5] Don't handle interrupted exception --- .../java/io/sentry/DirectoryProcessor.java | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/sentry/src/main/java/io/sentry/DirectoryProcessor.java b/sentry/src/main/java/io/sentry/DirectoryProcessor.java index 8384bfb3065..951c5475533 100644 --- a/sentry/src/main/java/io/sentry/DirectoryProcessor.java +++ b/sentry/src/main/java/io/sentry/DirectoryProcessor.java @@ -24,13 +24,14 @@ abstract class DirectoryProcessor { private final long flushTimeoutMillis; private final Queue processedEnvelopes; - DirectoryProcessor(final @NotNull IHub hub, - final @NotNull ILogger logger, final long flushTimeoutMillis) { + DirectoryProcessor( + final @NotNull IHub hub, final @NotNull ILogger logger, final long flushTimeoutMillis) { this.hub = hub; this.logger = logger; this.flushTimeoutMillis = flushTimeoutMillis; this.processedEnvelopes = - SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(hub.getOptions().getMaxQueueSize())); + SynchronizedQueue.synchronizedQueue( + new CircularFifoQueue<>(hub.getOptions().getMaxQueueSize())); } public void processDirectory(final @NotNull File directory) { @@ -85,9 +86,7 @@ public void processDirectory(final @NotNull File directory) { // in case there's rate limiting active, skip processing final @Nullable RateLimiter rateLimiter = hub.getRateLimiter(); if (rateLimiter != null && rateLimiter.isActiveForCategory(DataCategory.All)) { - logger.log( - SentryLevel.INFO, - "DirectoryProcessor, rate limiting active."); + logger.log(SentryLevel.INFO, "DirectoryProcessor, rate limiting active."); return; } @@ -102,21 +101,8 @@ public void processDirectory(final @NotNull File directory) { // a short delay between processing envelopes to avoid bursting our server and hitting // another rate limit https://develop.sentry.dev/sdk/features/#additional-capabilities - try { - Thread.sleep(ENVELOPE_PROCESS_DELAY); - } catch (InterruptedException e) { - try { - Thread.currentThread().interrupt(); - } catch (SecurityException ignored) { - logger.log( - SentryLevel.INFO, - "Failed to interrupt due to SecurityException: %s", - e.getMessage()); - return; - } - logger.log(SentryLevel.INFO, "Interrupted: %s", e.getMessage()); - return; - } + // InterruptedException will be handled by the outer try-catch + Thread.sleep(ENVELOPE_PROCESS_DELAY); } } catch (Throwable e) { logger.log(SentryLevel.ERROR, e, "Failed processing '%s'", directory.getAbsolutePath());