diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abe7ece787..ec63c894fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Deprecate `sendDefaultPii` in favor of `dataCollection` ahead of its removal in 9.0 ([#6158](https://github.com/getsentry/sentry-java/pull/6158)) +- Make the tombstone merge time threshold configurable via `SentryAndroidOptions.setTombstoneMergeTimeThresholdMillis` and the `io.sentry.tombstone.merge-time-threshold-millis` manifest option ([#6154](https://github.com/getsentry/sentry-java/pull/6154)) ## 8.58.0 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 9a8b9d835db..efd776cabb8 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -435,6 +435,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getNdkHandlerStrategy ()I public fun getScreenshot ()Lio/sentry/android/core/SentryScreenshotOptions; public fun getStartupCrashDurationThresholdMillis ()J + public fun getTombstoneMergeTimeThresholdMillis ()J public fun isAnrEnabled ()Z public fun isAnrProfilingEnabled ()Z public fun isAnrReportInDebug ()Z @@ -505,6 +506,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setReportHistoricalMemoryLimiterExits (Z)V public fun setReportHistoricalTombstones (Z)V public fun setTombstoneEnabled (Z)V + public fun setTombstoneMergeTimeThresholdMillis (J)V } public abstract interface class io/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index b5ef861a1a4..7de5a0c2716 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -52,6 +52,8 @@ final class ManifestMetadataReader { static final String TOMBSTONE_ENABLE = "io.sentry.tombstone.enable"; static final String TOMBSTONE_ATTACH_RAW = "io.sentry.tombstone.attach-raw"; static final String TOMBSTONE_REPORT_HISTORICAL = "io.sentry.tombstone.report-historical"; + static final String TOMBSTONE_MERGE_TIME_THRESHOLD_MILLIS = + "io.sentry.tombstone.merge-time-threshold-millis"; static final String MEMORY_LIMITER_ENABLE = "io.sentry.memory-limiter.enable"; static final String MEMORY_LIMITER_REPORT_HISTORICAL = "io.sentry.memory-limiter.report-historical"; @@ -278,6 +280,12 @@ static void applyMetadata( logger, TOMBSTONE_REPORT_HISTORICAL, options.isReportHistoricalTombstones())); + options.setTombstoneMergeTimeThresholdMillis( + readLong( + metadata, + logger, + TOMBSTONE_MERGE_TIME_THRESHOLD_MILLIS, + options.getTombstoneMergeTimeThresholdMillis())); options.setMemoryLimiterEnabled( readBool(metadata, logger, MEMORY_LIMITER_ENABLE, options.isMemoryLimiterEnabled())); options.setReportHistoricalMemoryLimiterExits( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/NativeEventCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/NativeEventCollector.java index 2cf5acd05fa..69dfa9c16be 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/NativeEventCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/NativeEventCollector.java @@ -38,8 +38,6 @@ public final class NativeEventCollector { private static final String NATIVE_PLATFORM = "native"; - private static final long TIMESTAMP_TOLERANCE_MS = 5000; - private final @NotNull SentryAndroidOptions options; /** Lightweight metadata collected during scan phase. */ @@ -175,18 +173,30 @@ public void collect() { // Lazily collect on first use (runs on executor thread, not main thread) collect(); + final long thresholdMs = options.getTombstoneMergeTimeThresholdMillis(); for (final NativeEnvelopeMetadata metadata : nativeEnvelopes) { final long timeDiff = Math.abs(tombstoneTimestampMs - metadata.getTimestampMs()); - if (timeDiff <= TIMESTAMP_TOLERANCE_MS) { + if (timeDiff <= thresholdMs) { options .getLogger() - .log(SentryLevel.DEBUG, "Matched native event by timestamp (diff: %d ms)", timeDiff); + .log( + SentryLevel.DEBUG, + "Matched native event by timestamp (diff: %d ms, threshold: %d ms)", + timeDiff, + thresholdMs); nativeEnvelopes.remove(metadata); // Only load full event data when we have a match return loadFullNativeEventData(metadata.getFile()); } } + options + .getLogger() + .log( + SentryLevel.DEBUG, + "No native event matched the tombstone timestamp %d within %d ms.", + tombstoneTimestampMs, + thresholdMs); return null; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 825d0cbedb9..c135172bc5c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -305,6 +305,13 @@ public interface BeforeCaptureCallback { private boolean enableTombstone = false; + /** + * The maximum time difference, in milliseconds, between a tombstone from {@link + * ApplicationExitInfo} and a native crash event in the outbox for the two to be merged into a + * single event. + */ + private long tombstoneMergeTimeThresholdMillis = 5000; + /** * Screenshot masking options. Configure which views should be masked when capturing screenshots * on error events. @@ -752,6 +759,34 @@ public void setAttachAnrThreadDump(final boolean attachAnrThreadDump) { this.attachAnrThreadDump = attachAnrThreadDump; } + public long getTombstoneMergeTimeThresholdMillis() { + return tombstoneMergeTimeThresholdMillis; + } + + /** + * Sets the maximum time difference, in milliseconds, between a tombstone from {@link + * ApplicationExitInfo} and a native crash event in the outbox for the two to be merged into a + * single event. Defaults to 5000 ms. + * + *

The two timestamps come from different sources: the tombstone timestamp is recorded by the + * system when the process died, the native event timestamp is recorded by the SDK signal handler. + * Raise the threshold when crashes are reported as separate 'signalhandler' events instead of a + * merged event, because the gap between the two exceeded the threshold. + * + *

Do not raise it more than necessary. The threshold is the only criterion used to pair the + * two, so a high value can merge a tombstone with a native crash that belongs to a different + * process death. The merged event then reports the wrong stack trace, and the native crash it + * consumed is never sent on its own. + * + *

A value of 0 merges only events with identical timestamps. A negative value disables merging + * completely, so tombstone and native crash are both reported as separate events. + * + * @param tombstoneMergeTimeThresholdMillis the threshold in milliseconds + */ + public void setTombstoneMergeTimeThresholdMillis(final long tombstoneMergeTimeThresholdMillis) { + this.tombstoneMergeTimeThresholdMillis = tombstoneMergeTimeThresholdMillis; + } + public boolean isAttachRawTombstone() { return attachRawTombstone; } 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 48832ca8dce..43de4034ec5 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 @@ -241,7 +241,7 @@ public void markReported(final long timestamp) { nativeEventCollector.findAndRemoveMatchingNativeEvent(tombstoneTimestamp); if (matchingNativeEvent == null) { - options.getLogger().log(SentryLevel.DEBUG, "No matching native event found for tombstone."); + // NativeEventCollector already logs why no event matched. return null; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 229e22d55f2..463b5d82ddb 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -464,6 +464,31 @@ class ManifestMetadataReaderTest { assertEquals(false, fixture.options.isReportHistoricalTombstones) } + @Test + fun `applyMetadata reads tombstone merge time threshold to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.TOMBSTONE_MERGE_TIME_THRESHOLD_MILLIS to 10000) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertEquals(10000, fixture.options.tombstoneMergeTimeThresholdMillis) + } + + @Test + fun `applyMetadata reads tombstone merge time threshold to options and keeps default`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertEquals(5000, fixture.options.tombstoneMergeTimeThresholdMillis) + } + @Test fun `applyMetadata reads anr report historical to options`() { // Arrange diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NativeEventCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NativeEventCollectorTest.kt index 243c20a2069..490847b3182 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NativeEventCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NativeEventCollectorTest.kt @@ -179,6 +179,35 @@ class NativeEventCollectorTest { assertNull(noMatch) } + @Test + fun `does not match when the gap exceeds the default threshold`() { + val sut = fixture.getSut(tmpDir) + copyEnvelopeToOutbox("native-event.txt") + + val timestamp = DateUtils.getDateTime("2023-07-15T10:30:05.800Z").time + assertNull(sut.findAndRemoveMatchingNativeEvent(timestamp)) + } + + @Test + fun `matches when the gap is within a raised threshold`() { + fixture.options.tombstoneMergeTimeThresholdMillis = 10000 + val sut = fixture.getSut(tmpDir) + copyEnvelopeToOutbox("native-event.txt") + + val timestamp = DateUtils.getDateTime("2023-07-15T10:30:05.800Z").time + assertNotNull(sut.findAndRemoveMatchingNativeEvent(timestamp)) + } + + @Test + fun `does not match when the gap exceeds a lowered threshold`() { + fixture.options.tombstoneMergeTimeThresholdMillis = 1000 + val sut = fixture.getSut(tmpDir) + copyEnvelopeToOutbox("native-event.txt") + + val timestamp = DateUtils.getDateTime("2023-07-15T10:30:02.000Z").time + assertNull(sut.findAndRemoveMatchingNativeEvent(timestamp)) + } + private fun copyEnvelopeToOutbox(name: String): File { val resourcePath = "envelopes/$name" val inputStream = diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index 94857b91058..d580ff54cd0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -246,6 +246,15 @@ class SentryAndroidOptionsTest { assertEquals(5000L, sentryOptions.ndkAppHangTimeoutIntervalMillis) } + @Test + fun `tombstone merge time threshold defaults to 5s and is configurable`() { + val sentryOptions = SentryAndroidOptions() + assertEquals(5000L, sentryOptions.tombstoneMergeTimeThresholdMillis) + + sentryOptions.tombstoneMergeTimeThresholdMillis = 10000L + assertEquals(10000L, sentryOptions.tombstoneMergeTimeThresholdMillis) + } + private class CustomDebugImagesLoader : IDebugImagesLoader { override fun loadDebugImages(): List? = null