diff --git a/CHANGELOG.md b/CHANGELOG.md index 878238076f3..5abcb0bddcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Prevent duplicated breadcrumbs on tombstone-merged native crash events ([#5888](https://github.com/getsentry/sentry-java/pull/5888)) - Prevent a class of Session Replay deadlocks by confining lifecycle state changes to Android's main thread ([#5965](https://github.com/getsentry/sentry-java/pull/5965)) - Symbolicate tombstone native frames for libraries loaded directly from APKs ([#5992](https://github.com/getsentry/sentry-java/pull/5992)) +- Keep tombstone and ANR events discarded from `beforeSend` discarded, instead of reporting them again at every app start ([#6002](https://github.com/getsentry/sentry-java/pull/6002)) ### Features diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 65bf072f0a0..4b8b41d41c1 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -638,6 +638,7 @@ public class io/sentry/android/core/TombstoneIntegration$TombstonePolicy : io/se public fun getLabel ()Ljava/lang/String; public fun getLastReportedTimestamp ()Ljava/lang/Long; public fun getTargetReason ()I + public fun markReported (J)V public fun shouldReportHistorical ()Z } @@ -749,6 +750,8 @@ public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z public static fun lastReportedAnr (Lio/sentry/SentryOptions;)Ljava/lang/Long; public static fun lastReportedTombstone (Lio/sentry/SentryOptions;)Ljava/lang/Long; + public static fun markAnrReported (Lio/sentry/SentryOptions;J)V + public static fun markTombstoneReported (Lio/sentry/SentryOptions;J)V public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java index 285c3b77ade..e28a08f7e8d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java @@ -132,6 +132,11 @@ public boolean shouldReportHistorical() { return AndroidEnvelopeCache.lastReportedAnr(options); } + @Override + public void markReported(final long timestamp) { + AndroidEnvelopeCache.markAnrReported(options, timestamp); + } + @Override public @Nullable ApplicationExitInfoHistoryDispatcher.Report buildReport( final @NotNull ApplicationExitInfo exitInfo, final boolean shouldEnrich) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoHistoryDispatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoHistoryDispatcher.java index 79c3f19c6e5..88a81022582 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoHistoryDispatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoHistoryDispatcher.java @@ -12,8 +12,10 @@ import io.sentry.cache.EnvelopeCache; import io.sentry.cache.IEnvelopeCache; import io.sentry.hints.BlockingFlushHint; +import io.sentry.hints.EventDropReason; import io.sentry.protocol.SentryId; import io.sentry.transport.ICurrentDateProvider; +import io.sentry.util.HintUtils; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; @@ -177,6 +179,7 @@ private void reportHistorical( } } + @RequiresApi(api = Build.VERSION_CODES.R) private void report(final @NotNull ApplicationExitInfo exitInfo, final boolean enrich) { final @Nullable Report report = policy.buildReport(exitInfo, enrich); @@ -186,7 +189,24 @@ private void report(final @NotNull ApplicationExitInfo exitInfo, final boolean e final @NotNull SentryId sentryId = scopes.captureEvent(report.getEvent(), report.getHint()); final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID); - if (!isEventDropped) { + if (isEventDropped) { + // A dropped event never reaches the envelope disk cache, which is where the last reported + // marker is normally written. Without writing it here, the very same exit would be turned + // into an event again on the next app start, ignoring the user's decision to drop it. + // An empty id alone is not enough: capturing also returns one when building or handing over + // the envelope failed, and such an exit has to stay eligible for the next app start. + final @Nullable EventDropReason dropReason = HintUtils.getEventDropReason(report.getHint()); + if (dropReason != null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "%s event was dropped (%s), marking the exit as reported.", + policy.getLabel(), + dropReason); + policy.markReported(exitInfo.getTimestamp()); + } + } else { final @Nullable BlockingFlushHint flushHint = report.getFlushHint(); if (flushHint != null && !flushHint.waitFlush()) { options @@ -211,6 +231,9 @@ interface ApplicationExitInfoPolicy { @Nullable Long getLastReportedTimestamp(); + /** Records {@code timestamp} as the last reported exit, so it is not reported again. */ + void markReported(long timestamp); + @Nullable Report buildReport(@NotNull ApplicationExitInfo exitInfo, boolean enrich); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/TombstoneIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/TombstoneIntegration.java index 2663051f7e4..d6d6a7767c8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/TombstoneIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/TombstoneIntegration.java @@ -147,6 +147,11 @@ public boolean shouldReportHistorical() { return AndroidEnvelopeCache.lastReportedTombstone(options); } + @Override + public void markReported(final long timestamp) { + AndroidEnvelopeCache.markTombstoneReported(options, timestamp); + } + @RequiresApi(api = Build.VERSION_CODES.R) @Override public @Nullable ApplicationExitInfoHistoryDispatcher.Report buildReport( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index 1ef02dfdd2c..0373c39deea 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -85,7 +85,7 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull } for (TimestampMarkerHandler handler : TIMESTAMP_MARKER_HANDLERS) { - handler.handle(this, hint, options); + handler.handle(hint, options); } return didStore; @@ -182,7 +182,8 @@ public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options return null; } - private void writeLastReportedMarker( + private static void writeLastReportedMarker( + final @NotNull SentryOptions options, final @Nullable Long timestamp, @NotNull String reportFilename, @NotNull String markerCategory) { @@ -215,6 +216,15 @@ private void writeLastReportedMarker( return lastReportedMarker(options, LAST_TOMBSTONE_REPORT, LAST_TOMBSTONE_MARKER_LABEL); } + public static void markAnrReported(final @NotNull SentryOptions options, final long timestamp) { + writeLastReportedMarker(options, timestamp, LAST_ANR_REPORT, LAST_ANR_MARKER_LABEL); + } + + public static void markTombstoneReported( + final @NotNull SentryOptions options, final long timestamp) { + writeLastReportedMarker(options, timestamp, LAST_TOMBSTONE_REPORT, LAST_TOMBSTONE_MARKER_LABEL); + } + private static final class TimestampMarkerHandler { interface TimestampExtractor { @NotNull @@ -237,10 +247,7 @@ interface TimestampExtractor { this.timestampProvider = timestampProvider; } - void handle( - final @NotNull AndroidEnvelopeCache cache, - final @NotNull Hint hint, - final @NotNull SentryAndroidOptions options) { + void handle(final @NotNull Hint hint, final @NotNull SentryAndroidOptions options) { HintUtils.runIfHasType( hint, type, @@ -253,7 +260,7 @@ void handle( "Writing last reported %s marker with timestamp %d", label, timestamp); - cache.writeLastReportedMarker(timestamp, reportFilename, label); + writeLastReportedMarker(options, timestamp, reportFilename, label); }); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt index edb2ce1df24..1bcef8b7e7b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt @@ -12,6 +12,7 @@ import io.sentry.SentryEvent import io.sentry.SentryLevel import io.sentry.cache.EnvelopeCache import io.sentry.hints.DiskFlushNotification +import io.sentry.hints.EventDropReason import io.sentry.hints.SessionStartHint import io.sentry.protocol.SentryId import io.sentry.test.ImmediateExecutorService @@ -187,6 +188,33 @@ abstract class ApplicationExitIntegrationTestBase { .log(any(), argThat { startsWith(config.flushLogPrefix) }, any()) } + @Test + fun `when latest event was dropped by beforeSend, marks the exit as reported`() { + val integration = + fixture.getSut( + tmpDir, + lastReportedTimestamp = oldTimestamp, + lastEventId = SentryId.EMPTY_ID, + eventDropReason = EventDropReason.BEFORE_SEND, + ) + fixture.addAppExitInfo(timestamp = newTimestamp) + + integration.register(fixture.scopes, fixture.options) + + assertEquals(newTimestamp.toString(), fixture.lastReportedFile.readText()) + } + + @Test + fun `when capturing the latest event failed, does not mark the exit as reported`() { + val integration = + fixture.getSut(tmpDir, lastReportedTimestamp = oldTimestamp, lastEventId = SentryId.EMPTY_ID) + fixture.addAppExitInfo(timestamp = newTimestamp) + + integration.register(fixture.scopes, fixture.options) + + assertEquals(oldTimestamp.toString(), fixture.lastReportedFile.readText()) + } + @Test fun `historical exits are reported non-enriched`() { val integration = fixture.getSut(tmpDir, lastReportedTimestamp = oldTimestamp) @@ -403,6 +431,7 @@ abstract class ApplicationExitIntegrationTestBase { sessionFlushTimeoutMillis: Long = 0L, lastReportedTimestamp: Long? = null, lastEventId: SentryId = SentryId(), + eventDropReason: EventDropReason? = null, sessionTrackingEnabled: Boolean = true, reportHistorical: Boolean = true, extraOptions: (SentryAndroidOptions) -> Unit = {}, @@ -426,7 +455,10 @@ abstract class ApplicationExitIntegrationTestBase { lastReportedFile = File(cacheDir, config.lastReportedFileName) lastReportedFile.writeText(lastReportedTimestamp.toString()) } - whenever(scopes.captureEvent(any(), anyOrNull())).thenReturn(lastEventId) + whenever(scopes.captureEvent(any(), anyOrNull())).thenAnswer { invocation -> + eventDropReason?.let { HintUtils.setEventDropReason(invocation.getArgument(1), it) } + lastEventId + } return config.createIntegration(context) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index fa876b3312f..cf64be19ea9 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5197,7 +5197,10 @@ public abstract interface class io/sentry/hints/Enqueable { } public final class io/sentry/hints/EventDropReason : java/lang/Enum { + public static final field BEFORE_SEND Lio/sentry/hints/EventDropReason; + public static final field IGNORED Lio/sentry/hints/EventDropReason; public static final field MULTITHREADED_DEDUPLICATION Lio/sentry/hints/EventDropReason; + public static final field SAMPLE_RATE Lio/sentry/hints/EventDropReason; public static fun valueOf (Ljava/lang/String;)Lio/sentry/hints/EventDropReason; public static fun values ()[Lio/sentry/hints/EventDropReason; } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 18e247c510a..4dd0cb5b4b8 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -7,6 +7,7 @@ import io.sentry.hints.Backfillable; import io.sentry.hints.Cached; import io.sentry.hints.DiskFlushNotification; +import io.sentry.hints.EventDropReason; import io.sentry.hints.TransactionEnd; import io.sentry.logger.ILoggerBatchProcessor; import io.sentry.logger.NoOpLoggerBatchProcessor; @@ -137,6 +138,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul options .getClientReportRecorder() .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Error); + HintUtils.setEventDropReason(hint, EventDropReason.IGNORED); return SentryId.EMPTY_ID; } @@ -150,6 +152,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul options .getClientReportRecorder() .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Error); + HintUtils.setEventDropReason(hint, EventDropReason.IGNORED); return SentryId.EMPTY_ID; } } @@ -176,6 +179,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul options .getClientReportRecorder() .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); + HintUtils.setEventDropReason(hint, EventDropReason.BEFORE_SEND); } } @@ -208,6 +212,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul options .getClientReportRecorder() .recordLostEvent(DiscardReason.SAMPLE_RATE, DataCategory.Error); + HintUtils.setEventDropReason(hint, EventDropReason.SAMPLE_RATE); // setting event as null to not be sent as its been discarded by sample rate event = null; } diff --git a/sentry/src/main/java/io/sentry/hints/EventDropReason.java b/sentry/src/main/java/io/sentry/hints/EventDropReason.java index 56a4bbfeabc..bd2564a0134 100644 --- a/sentry/src/main/java/io/sentry/hints/EventDropReason.java +++ b/sentry/src/main/java/io/sentry/hints/EventDropReason.java @@ -5,5 +5,11 @@ /** A reason for which an event was dropped, used for (not to confuse with ClientReports) */ @ApiStatus.Internal public enum EventDropReason { - MULTITHREADED_DEDUPLICATION + MULTITHREADED_DEDUPLICATION, + /** The event matched {@code ignoredExceptionsForType} or {@code ignoredErrors}. */ + IGNORED, + /** The {@code beforeSend} callback returned {@code null}. */ + BEFORE_SEND, + /** The event lost the {@code sampleRate} draw. */ + SAMPLE_RATE }