Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate events happening in multiple threads simultaneously #2845

Merged
merged 7 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

- Fix ANRv2 thread dump parsing for native-only threads ([#2839](https://github.com/getsentry/sentry-java/pull/2839))
- Derive `TracingContext` values from event for ANRv2 events ([#2839](https://github.com/getsentry/sentry-java/pull/2839))
- Deduplicate events happening in multiple threads simultaneously (e.g. `OutOfMemoryError`) ([#2845](https://github.com/getsentry/sentry-java/pull/2845))
- This will improve Crash-Free Session Rate as we no longer will send multiple Session updates with `Crashed` status, but only the one that is relevant

## 6.25.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.content.Context;
import android.content.pm.PackageInfo;
import android.os.Build;
import io.sentry.DeduplicateMultithreadedEventProcessor;
import io.sentry.DefaultTransactionPerformanceCollector;
import io.sentry.ILogger;
import io.sentry.SendFireAndForgetEnvelopeSender;
Expand Down Expand Up @@ -125,6 +126,7 @@ static void initializeIntegrationsAndProcessors(
options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options));
}

options.addEventProcessor(new DeduplicateMultithreadedEventProcessor(options));
options.addEventProcessor(
new DefaultAndroidEventProcessor(context, buildInfoProvider, options));
options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import java.io.InputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.CountDownLatch;
import timber.log.Timber;

public class MainActivity extends AppCompatActivity {
Expand Down Expand Up @@ -139,6 +142,28 @@ protected void onCreate(Bundle savedInstanceState) {
Sentry.setUser(user);
});

binding.outOfMemory.setOnClickListener(
view -> {
final CountDownLatch latch = new CountDownLatch(1);
for (int i = 0; i < 20; i++) {
new Thread(
() -> {
final List<String> data = new ArrayList<>();
try {
latch.await();
for (int j = 0; j < 1_000_000; j++) {
data.add(new String(new byte[1024 * 8]));
}
} catch (InterruptedException e) {
e.printStackTrace();
}
})
.start();
}

latch.countDown();
});

binding.nativeCrash.setOnClickListener(view -> NativeSample.crash());

binding.nativeCapture.setOnClickListener(view -> NativeSample.message());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@
android:layout_height="wrap_content"
android:text="@string/anr" />

<Button
android:id="@+id/out_of_memory"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/out_of_memory" />

<Button
android:id="@+id/native_crash"
android:layout_width="wrap_content"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<resources>
<string name="app_name">Sentry sample</string>
<string name="crash_from_java">Crash from Java (UncaughtException)</string>
<string name="out_of_memory">Out of Memory (Mulithreaded)</string>
<string name="send_message">Send Message</string>
<string name="send_message_from_inner_fragment">Send Message from inner fragment</string>
<string name="add_attachment">Add Attachment</string>
Expand Down
15 changes: 15 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,11 @@ public final class io/sentry/DateUtils {
public static fun toUtilDateNotNull (Lio/sentry/SentryDate;)Ljava/util/Date;
}

public final class io/sentry/DeduplicateMultithreadedEventProcessor : io/sentry/EventProcessor {
public fun <init> (Lio/sentry/SentryOptions;)V
public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
}

public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
public fun <init> (Lio/sentry/SentryOptions;)V
public fun close ()V
Expand Down Expand Up @@ -1521,6 +1526,7 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
public fun getThreads ()Ljava/util/List;
public fun getTimestamp ()Ljava/util/Date;
public fun getTransaction ()Ljava/lang/String;
public fun getUnhandledException ()Lio/sentry/protocol/SentryException;
public fun getUnknown ()Ljava/util/Map;
public fun isCrashed ()Z
public fun isErrored ()Z
Expand Down Expand Up @@ -2345,6 +2351,7 @@ public final class io/sentry/TypeCheckHint {
public static final field OPEN_FEIGN_RESPONSE 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;
public static final field SENTRY_IS_FROM_HYBRID_SDK Ljava/lang/String;
public static final field SENTRY_JAVASCRIPT_SDK_NAME Ljava/lang/String;
public static final field SENTRY_SYNTHETIC_EXCEPTION Ljava/lang/String;
Expand Down Expand Up @@ -2628,6 +2635,12 @@ public abstract interface class io/sentry/hints/DiskFlushNotification {
public abstract fun markFlushed ()V
}

public final class io/sentry/hints/EventDropReason : java/lang/Enum {
public static final field MULTITHREADED_DEDUPLICATION Lio/sentry/hints/EventDropReason;
public static fun valueOf (Ljava/lang/String;)Lio/sentry/hints/EventDropReason;
public static fun values ()[Lio/sentry/hints/EventDropReason;
}

public abstract interface class io/sentry/hints/Flushable {
public abstract fun waitFlush ()Z
}
Expand Down Expand Up @@ -4111,13 +4124,15 @@ public final class io/sentry/util/FileUtils {

public final class io/sentry/util/HintUtils {
public static fun createWithTypeCheckHint (Ljava/lang/Object;)Lio/sentry/Hint;
public static fun getEventDropReason (Lio/sentry/Hint;)Lio/sentry/hints/EventDropReason;
public static fun getSentrySdkHint (Lio/sentry/Hint;)Ljava/lang/Object;
public static fun hasType (Lio/sentry/Hint;Ljava/lang/Class;)Z
public static fun isFromHybridSdk (Lio/sentry/Hint;)Z
public static fun runIfDoesNotHaveType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryNullableConsumer;)V
public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;)V
public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;Lio/sentry/util/HintUtils$SentryHintFallback;)V
public static fun runIfHasTypeLogIfNot (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/ILogger;Lio/sentry/util/HintUtils$SentryConsumer;)V
public static fun setEventDropReason (Lio/sentry/Hint;Lio/sentry/hints/EventDropReason;)V
public static fun setIsFromHybridSdk (Lio/sentry/Hint;Ljava/lang/String;)V
public static fun setTypeCheckHint (Lio/sentry/Hint;Ljava/lang/Object;)V
public static fun shouldApplyScopeData (Lio/sentry/Hint;)Z
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package io.sentry;

import io.sentry.hints.EventDropReason;
import io.sentry.protocol.SentryException;
import io.sentry.util.HintUtils;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/**
* An event processor that deduplicates crash events of the same type that are simultaneously from
* multiple threads. This can be the case for OutOfMemory errors or CursorWindowAllocationException,
* basically any error related to allocating memory when it's low.
*/
public final class DeduplicateMultithreadedEventProcessor implements EventProcessor {

private final @NotNull Map<String, Long> processedEvents =
Collections.synchronizedMap(new HashMap<>());

private final @NotNull SentryOptions options;

public DeduplicateMultithreadedEventProcessor(final @NotNull SentryOptions options) {
this.options = options;
}

@Override
public @Nullable SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) {
if (!HintUtils.hasType(hint, UncaughtExceptionHandlerIntegration.UncaughtExceptionHint.class)) {
// only dedupe crashes coming from our exception handler, because custom errors/crashes might
// be sent on purpose
return event;
}

final SentryException exception = event.getUnhandledException();
if (exception == null) {
return event;
}

final String type = exception.getType();
if (type == null) {
return event;
}

final Long currentEventTid = exception.getThreadId();
if (currentEventTid == null) {
return event;
}

final Long tid = processedEvents.get(type);
if (tid != null && !tid.equals(currentEventTid)) {
options
.getLogger()
.log(
SentryLevel.INFO,
"Event %s has been dropped due to multi-threaded deduplication",
event.getEventId());
HintUtils.setEventDropReason(hint, EventDropReason.MULTITHREADED_DEDUPLICATION);
return null;
}
processedEvents.put(type, currentEventTid);
return event;
}
}
9 changes: 6 additions & 3 deletions sentry/src/main/java/io/sentry/SentryEvent.java
Original file line number Diff line number Diff line change
Expand Up @@ -211,17 +211,20 @@ public void removeModule(final @NotNull String key) {
* @return true if its crashed or false otherwise
*/
public boolean isCrashed() {
return getUnhandledException() != null;
}

public @Nullable SentryException getUnhandledException() {
if (exception != null) {
for (SentryException e : exception.getValues()) {
if (e.getMechanism() != null
&& e.getMechanism().isHandled() != null
&& !e.getMechanism().isHandled()) {
return true;
return e;
}
}
}

return false;
return null;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/TypeCheckHint.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ public final class TypeCheckHint {
@ApiStatus.Internal
public static final String SENTRY_IS_FROM_HYBRID_SDK = "sentry:isFromHybridSdk";

@ApiStatus.Internal
public static final String SENTRY_EVENT_DROP_REASON = "sentry:eventDropReason";

@ApiStatus.Internal public static final String SENTRY_JAVASCRIPT_SDK_NAME = "sentry.javascript";

@ApiStatus.Internal public static final String SENTRY_DOTNET_SDK_NAME = "sentry.dotnet";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.jakewharton.nopen.annotation.Open;
import io.sentry.exception.ExceptionMechanismException;
import io.sentry.hints.BlockingFlushHint;
import io.sentry.hints.EventDropReason;
import io.sentry.hints.SessionEnd;
import io.sentry.protocol.Mechanism;
import io.sentry.protocol.SentryId;
Expand Down Expand Up @@ -98,7 +99,11 @@ public void uncaughtException(Thread thread, Throwable thrown) {

final @NotNull SentryId sentryId = hub.captureEvent(event, hint);
final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID);
if (!isEventDropped) {
final EventDropReason eventDropReason = HintUtils.getEventDropReason(hint);
// in case the event has been dropped by multithreaded deduplicator, the other threads will
// crash the app without a chance to persist the main event so we have to special-case this
if (!isEventDropped
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is needed because otherwise the other crashing threads will not hold a lock anymore and will crash the app immediately, not giving a chance to persist the crash that was the first one (the one which we're going to deduplicate).

|| EventDropReason.MULTITHREADED_DEDUPLICATION.equals(eventDropReason)) {
// Block until the event is flushed to disk
if (!exceptionHint.waitFlush()) {
options
Expand Down
9 changes: 9 additions & 0 deletions sentry/src/main/java/io/sentry/hints/EventDropReason.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.sentry.hints;

import org.jetbrains.annotations.ApiStatus;

/** A reason for which an event was dropped, used for (not to confuse with ClientReports) */
@ApiStatus.Internal
public enum EventDropReason {
MULTITHREADED_DEDUPLICATION
}
12 changes: 12 additions & 0 deletions sentry/src/main/java/io/sentry/util/HintUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static io.sentry.TypeCheckHint.SENTRY_DART_SDK_NAME;
import static io.sentry.TypeCheckHint.SENTRY_DOTNET_SDK_NAME;
import static io.sentry.TypeCheckHint.SENTRY_EVENT_DROP_REASON;
import static io.sentry.TypeCheckHint.SENTRY_IS_FROM_HYBRID_SDK;
import static io.sentry.TypeCheckHint.SENTRY_JAVASCRIPT_SDK_NAME;
import static io.sentry.TypeCheckHint.SENTRY_TYPE_CHECK_HINT;
Expand All @@ -11,6 +12,7 @@
import io.sentry.hints.ApplyScopeData;
import io.sentry.hints.Backfillable;
import io.sentry.hints.Cached;
import io.sentry.hints.EventDropReason;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -33,6 +35,16 @@ public static boolean isFromHybridSdk(final @NotNull Hint hint) {
return Boolean.TRUE.equals(hint.getAs(SENTRY_IS_FROM_HYBRID_SDK, Boolean.class));
}

public static void setEventDropReason(
final @NotNull Hint hint, final @NotNull EventDropReason eventDropReason) {
hint.set(SENTRY_EVENT_DROP_REASON, eventDropReason);
}

@Nullable
public static EventDropReason getEventDropReason(final @NotNull Hint hint) {
return hint.getAs(SENTRY_EVENT_DROP_REASON, EventDropReason.class);
}

public static Hint createWithTypeCheckHint(Object typeCheckHint) {
Hint hint = new Hint();
setTypeCheckHint(hint, typeCheckHint);
Expand Down