diff --git a/CHANGELOG.md b/CHANGELOG.md index cd7efb989cc..666d1f527e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,9 @@ Breaking Changes: * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). +* Ref: Optimize DuplicateEventDetectionEventProcessor performance (#1247). * Enchancement: Add overloads for startTransaction taking op and description (#1244) - # 4.1.0 * Improve Kotlin compatibility for SdkVersion (#1213) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 0fdce7aacd1..f33466e9b77 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -750,6 +750,7 @@ public class io/sentry/SentryOptions { public fun isAttachStacktrace ()Z public fun isAttachThreads ()Z public fun isDebug ()Z + public fun isEnableDeduplication ()Z public fun isEnableExternalConfiguration ()Z public fun isEnableNdk ()Z public fun isEnableScopeSync ()Z @@ -769,6 +770,7 @@ public class io/sentry/SentryOptions { public fun setDist (Ljava/lang/String;)V public fun setDistinctId (Ljava/lang/String;)V public fun setDsn (Ljava/lang/String;)V + public fun setEnableDeduplication (Ljava/lang/Boolean;)V public fun setEnableExternalConfiguration (Z)V public fun setEnableNdk (Z)V public fun setEnableScopeSync (Z)V diff --git a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java index 97e6f8b2b33..1d70d2f2b7f 100644 --- a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java +++ b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java @@ -2,6 +2,7 @@ import io.sentry.util.Objects; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -10,7 +11,8 @@ /** Deduplicates events containing throwable that has been already processed. */ public final class DuplicateEventDetectionEventProcessor implements EventProcessor { - private final WeakHashMap capturedObjects = new WeakHashMap<>(); + private final Map capturedObjects = + Collections.synchronizedMap(new WeakHashMap<>()); private final SentryOptions options; public DuplicateEventDetectionEventProcessor(final @NotNull SentryOptions options) { @@ -19,20 +21,24 @@ public DuplicateEventDetectionEventProcessor(final @NotNull SentryOptions option @Override public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { - final Throwable throwable = event.getOriginThrowable(); - if (throwable != null) { - if (capturedObjects.containsKey(throwable) - || containsAnyKey(capturedObjects, allCauses(throwable))) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Duplicate Exception detected. Event %s will be discarded.", - event.getEventId()); - return null; - } else { - capturedObjects.put(throwable, null); + if (options.isEnableDeduplication()) { + final Throwable throwable = event.getOriginThrowable(); + if (throwable != null) { + if (capturedObjects.containsKey(throwable) + || containsAnyKey(capturedObjects, allCauses(throwable))) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Duplicate Exception detected. Event %s will be discarded.", + event.getEventId()); + return null; + } else { + capturedObjects.put(throwable, null); + } } + } else { + options.getLogger().log(SentryLevel.DEBUG, "Event deduplication is disabled."); } return event; } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index b420fb563dc..e55e07c8b91 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -254,6 +254,13 @@ public class SentryOptions { /** max attachment size in bytes. */ private long maxAttachmentSize = 20 * 1024 * 1024; + /** + * Enables event deduplication with {@link DuplicateEventDetectionEventProcessor}. Event + * deduplication prevents from receiving the same exception multiple times when there is more than + * one framework active that captures errors, for example Logback and Spring Boot. + */ + private Boolean enableDeduplication = true; + /** * Creates {@link SentryOptions} from properties provided by a {@link PropertiesProvider}. * @@ -271,6 +278,7 @@ public class SentryOptions { propertiesProvider.getBooleanProperty("uncaught.handler.enabled")); options.setTracesSampleRate(propertiesProvider.getDoubleProperty("traces-sample-rate")); options.setDebug(propertiesProvider.getBooleanProperty("debug")); + options.setEnableDeduplication(propertiesProvider.getBooleanProperty("enable-deduplication")); final Map tags = propertiesProvider.getMap("tags"); for (final Map.Entry tag : tags.entrySet()) { options.setTag(tag.getKey(), tag.getValue()); @@ -1224,6 +1232,33 @@ public void setMaxAttachmentSize(long maxAttachmentSize) { this.maxAttachmentSize = maxAttachmentSize; } + /** + * Returns if event deduplication is turned on. + * + * @return if event deduplication is turned on. + */ + public boolean isEnableDeduplication() { + return Boolean.TRUE.equals(enableDeduplication); + } + + /** + * Returns if event deduplication is turned on or of or {@code null} if not specified. + * + * @return if event deduplication is turned on or of or {@code null} if not specified. + */ + private @Nullable Boolean getEnableDeduplication() { + return enableDeduplication; + } + + /** + * Enables or disables event deduplication. + * + * @param enableDeduplication true if enabled false otherwise + */ + public void setEnableDeduplication(final @Nullable Boolean enableDeduplication) { + this.enableDeduplication = enableDeduplication; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1318,6 +1353,9 @@ void merge(final @NotNull SentryOptions options) { if (options.getDebug() != null) { setDebug(options.getDebug()); } + if (options.getEnableDeduplication() != null) { + setEnableDeduplication(options.getEnableDeduplication()); + } final Map tags = new HashMap<>(options.getTags()); for (final Map.Entry tag : tags.entrySet()) { this.tags.put(tag.getKey(), tag.getValue()); diff --git a/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt b/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt index 578b14bda5a..22e569cc6a4 100644 --- a/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt @@ -9,10 +9,22 @@ import kotlin.test.assertNull class DuplicateEventDetectionEventProcessorTest { - val processor = DuplicateEventDetectionEventProcessor(SentryOptions()) + class Fixture { + fun getSut(enableDeduplication: Boolean? = null): DuplicateEventDetectionEventProcessor { + val options = SentryOptions().apply { + if (enableDeduplication != null) { + this.setEnableDeduplication(enableDeduplication) + } + } + return DuplicateEventDetectionEventProcessor(options) + } + } + + var fixture = Fixture() @Test fun `does not drop event if no previous event with same exception was processed`() { + val processor = fixture.getSut() processor.process(SentryEvent(), null) val result = processor.process(SentryEvent(RuntimeException()), null) @@ -22,6 +34,7 @@ class DuplicateEventDetectionEventProcessorTest { @Test fun `drops event with the same exception`() { + val processor = fixture.getSut() val event = SentryEvent(RuntimeException()) processor.process(event, null) @@ -31,6 +44,7 @@ class DuplicateEventDetectionEventProcessorTest { @Test fun `drops event with mechanism exception having an exception that has already been processed`() { + val processor = fixture.getSut() val event = SentryEvent(RuntimeException()) processor.process(event, null) @@ -40,6 +54,7 @@ class DuplicateEventDetectionEventProcessorTest { @Test fun `drops event with exception that has already been processed with event with mechanism exception`() { + val processor = fixture.getSut() val sentryEvent = SentryEvent(ExceptionMechanismException(Mechanism(), RuntimeException(), Thread.currentThread())) processor.process(sentryEvent, null) @@ -50,6 +65,7 @@ class DuplicateEventDetectionEventProcessorTest { @Test fun `drops event with the cause equal to exception in already processed event`() { + val processor = fixture.getSut() val event = SentryEvent(RuntimeException()) processor.process(event, null) @@ -60,6 +76,7 @@ class DuplicateEventDetectionEventProcessorTest { @Test fun `drops event with any of the causes has been already processed`() { + val processor = fixture.getSut() val event = SentryEvent(RuntimeException()) processor.process(event, null) @@ -67,4 +84,12 @@ class DuplicateEventDetectionEventProcessorTest { assertNull(result) } + + @Test + fun `does not deduplicate is deduplication is disabled`() { + val processor = fixture.getSut(enableDeduplication = false) + val event = SentryEvent(RuntimeException()) + assertNotNull(processor.process(event, null)) + assertNotNull(processor.process(event, null)) + } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 98fa7e2e575..a621b06ee50 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -436,6 +436,24 @@ class SentryOptionsTest { } } + @Test + fun `creates options with enableDeduplication using external properties`() { + // create a sentry.properties file in temporary folder + val temporaryFolder = TemporaryFolder() + temporaryFolder.create() + val file = temporaryFolder.newFile("sentry.properties") + file.appendText("enable-deduplication=true") + // set location of the sentry.properties file + System.setProperty("sentry.properties.file", file.absolutePath) + + try { + val options = SentryOptions.from(PropertiesProviderFactory.create()) + assertTrue(options.isEnableDeduplication) + } finally { + temporaryFolder.delete() + } + } + @Test fun `when options are initialized, maxAttachmentSize is 20`() { assertEquals(20 * 1024 * 1024, SentryOptions().maxAttachmentSize)