Skip to content

Commit

Permalink
Improve ANRv2 implementation based on customer feedback (#2792)
Browse files Browse the repository at this point in the history
  • Loading branch information
romtsn authored Jun 22, 2023
1 parent 9246ed4 commit 2c45a25
Show file tree
Hide file tree
Showing 18 changed files with 380 additions and 24 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@
### Features

- Add debouncing mechanism and before-capture callbacks for screenshots and view hierarchies ([#2773](https://github.com/getsentry/sentry-java/pull/2773))
- Improve ANRv2 implementation ([#2792](https://github.com/getsentry/sentry-java/pull/2792))
- Add a proguard rule to keep `ApplicationNotResponding` class from obfuscation
- Add a new option `setReportHistoricalAnrs`; when enabled, it will report all of the ANRs from the [getHistoricalExitReasons](https://developer.android.com/reference/android/app/ActivityManager?hl=en#getHistoricalProcessExitReasons(java.lang.String,%20int,%20int)) list.
By default, the SDK only reports and enriches the latest ANR and only this one counts towards ANR rate.
Worth noting that this option is mainly useful when updating the SDK to the version where ANRv2 has been introduced, to report all ANRs happened prior to the SDK update. After that, the SDK will always pick up the latest ANR from the historical exit reasons list on next app restart, so there should be no historical ANRs to report.
These ANRs are reported with the `HistoricalAppExitInfo` mechanism.
- Add a new option `setAttachAnrThreadDump` to send ANR thread dump from the system as an attachment.
This is only useful as additional information, because the SDK attempts to parse the thread dump into proper threads with stacktraces by default.
- If [ApplicationExitInfo#getTraceInputStream](https://developer.android.com/reference/android/app/ApplicationExitInfo#getTraceInputStream()) returns null, the SDK no longer reports an ANR event, as these events are not very useful without it.
- Enhance regex patterns for native stackframes

## 6.23.0

Expand Down
4 changes: 4 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun getStartupCrashDurationThresholdMillis ()J
public fun isAnrEnabled ()Z
public fun isAnrReportInDebug ()Z
public fun isAttachAnrThreadDump ()Z
public fun isAttachScreenshot ()Z
public fun isAttachViewHierarchy ()Z
public fun isCollectAdditionalContext ()Z
Expand All @@ -228,9 +229,11 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun isEnableNetworkEventBreadcrumbs ()Z
public fun isEnableRootCheck ()Z
public fun isEnableSystemEventBreadcrumbs ()Z
public fun isReportHistoricalAnrs ()Z
public fun setAnrEnabled (Z)V
public fun setAnrReportInDebug (Z)V
public fun setAnrTimeoutIntervalMillis (J)V
public fun setAttachAnrThreadDump (Z)V
public fun setAttachScreenshot (Z)V
public fun setAttachViewHierarchy (Z)V
public fun setBeforeScreenshotCaptureCallback (Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;)V
Expand All @@ -249,6 +252,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setNativeSdkName (Ljava/lang/String;)V
public fun setProfilingTracesHz (I)V
public fun setProfilingTracesIntervalMillis (I)V
public fun setReportHistoricalAnrs (Z)V
}

public abstract interface class io/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback {
Expand Down
2 changes: 2 additions & 0 deletions sentry-android-core/proguard-rules.pro
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
# Also keep method names, as they're e.g. used by native via JNI calls
-keep class * extends io.sentry.SentryOptions { *; }

-keepnames class io.sentry.android.core.ApplicationNotResponding

##---------------End: proguard configuration for android-core ----------

##---------------Begin: proguard configuration for sentry-apollo-3 ----------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,12 @@ private void backfillScope(final @NotNull SentryEvent event) {
private void setTrace(final @NotNull SentryEvent event) {
final SpanContext spanContext =
PersistingScopeObserver.read(options, TRACE_FILENAME, SpanContext.class);
if (event.getContexts().getTrace() == null && spanContext != null) {
event.getContexts().setTrace(spanContext);
if (event.getContexts().getTrace() == null) {
if (spanContext != null
&& spanContext.getSpanId() != null
&& spanContext.getTraceId() != null) {
event.getContexts().setTrace(spanContext);
}
}
}

Expand Down Expand Up @@ -190,8 +194,13 @@ private void setContexts(final @NotNull SentryBaseEvent event) {
}
final Contexts eventContexts = event.getContexts();
for (Map.Entry<String, Object> entry : new Contexts(persistedContexts).entrySet()) {
final Object value = entry.getValue();
if (SpanContext.TYPE.equals(entry.getKey()) && value instanceof SpanContext) {
// we fill it in setTrace later on
continue;
}
if (!eventContexts.containsKey(entry.getKey())) {
eventContexts.put(entry.getKey(), entry.getValue());
eventContexts.put(entry.getKey(), value);
}
}
}
Expand Down Expand Up @@ -446,7 +455,12 @@ private void setExceptions(final @NotNull SentryEvent event, final @NotNull Obje
// AnrV2 threads contain a thread dump from the OS, so we just search for the main thread dump
// and make an exception out of its stacktrace
final Mechanism mechanism = new Mechanism();
mechanism.setType("AppExitInfo");
if (!((Backfillable) hint).shouldEnrich()) {
// we only enrich the latest ANR in the list, so this is historical
mechanism.setType("HistoricalAppExitInfo");
} else {
mechanism.setType("AppExitInfo");
}

final boolean isBackgroundAnr = isBackgroundAnr(hint);
String message = "ANR";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import android.app.ActivityManager;
import android.app.ApplicationExitInfo;
import android.content.Context;
import io.sentry.Attachment;
import io.sentry.DateUtils;
import io.sentry.Hint;
import io.sentry.IHub;
Expand All @@ -20,15 +21,19 @@
import io.sentry.hints.AbnormalExit;
import io.sentry.hints.Backfillable;
import io.sentry.hints.BlockingFlushHint;
import io.sentry.protocol.Message;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentryThread;
import io.sentry.transport.CurrentDateProvider;
import io.sentry.transport.ICurrentDateProvider;
import io.sentry.util.HintUtils;
import io.sentry.util.Objects;
import java.io.BufferedReader;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -186,8 +191,10 @@ public void run() {
return;
}

// report the remainder without enriching
reportNonEnrichedHistoricalAnrs(exitInfos, lastReportedAnrTimestamp);
if (options.isReportHistoricalAnrs()) {
// report the remainder without enriching
reportNonEnrichedHistoricalAnrs(exitInfos, lastReportedAnrTimestamp);
}

// report the latest ANR with enriching, if contexts are available, otherwise report it
// non-enriched
Expand Down Expand Up @@ -228,7 +235,16 @@ private void reportAsSentryEvent(
final boolean isBackground =
exitInfo.getImportance() != ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND;

final List<SentryThread> threads = parseThreadDump(exitInfo, isBackground);
final ParseResult result = parseThreadDump(exitInfo, isBackground);
if (result.type == ParseResult.Type.NO_DUMP) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Not reporting ANR event as there was no thread dump for the ANR %s",
exitInfo.toString());
return;
}
final AnrV2Hint anrHint =
new AnrV2Hint(
options.getFlushTimeoutMillis(),
Expand All @@ -240,9 +256,24 @@ private void reportAsSentryEvent(
final Hint hint = HintUtils.createWithTypeCheckHint(anrHint);

final SentryEvent event = new SentryEvent();
event.setThreads(threads);
event.setTimestamp(DateUtils.getDateTime(anrTimestamp));
if (result.type == ParseResult.Type.ERROR) {
final Message sentryMessage = new Message();
sentryMessage.setFormatted(
"Sentry Android SDK failed to parse system thread dump for "
+ "this ANR. We recommend enabling [SentryOptions.isAttachAnrThreadDump] option "
+ "to attach the thread dump as plain text and report this issue on GitHub.");
event.setMessage(sentryMessage);
} else if (result.type == ParseResult.Type.DUMP) {
event.setThreads(result.threads);
}
event.setLevel(SentryLevel.FATAL);
event.setTimestamp(DateUtils.getDateTime(anrTimestamp));

if (options.isAttachAnrThreadDump()) {
if (result.dump != null) {
hint.setThreadDump(Attachment.fromThreadDump(result.dump));
}
}

final @NotNull SentryId sentryId = hub.captureEvent(event, hint);
final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID);
Expand All @@ -259,20 +290,56 @@ private void reportAsSentryEvent(
}
}

private @Nullable List<SentryThread> parseThreadDump(
private @NotNull ParseResult parseThreadDump(
final @NotNull ApplicationExitInfo exitInfo, final boolean isBackground) {
List<SentryThread> threads = null;
InputStream trace;
try {
trace = exitInfo.getTraceInputStream();
if (trace == null) {
return new ParseResult(ParseResult.Type.NO_DUMP);
}
} catch (Throwable e) {
options.getLogger().log(SentryLevel.WARNING, "Failed to read ANR thread dump", e);
return new ParseResult(ParseResult.Type.NO_DUMP);
}

byte[] dump = null;
try {
dump = getDumpBytes(trace);
} catch (Throwable e) {
options
.getLogger()
.log(SentryLevel.WARNING, "Failed to convert ANR thread dump to byte array", e);
}

try (final BufferedReader reader =
new BufferedReader(new InputStreamReader(exitInfo.getTraceInputStream()))) {
new BufferedReader(new InputStreamReader(new ByteArrayInputStream(dump)))) {
final Lines lines = Lines.readLines(reader);

final ThreadDumpParser threadDumpParser = new ThreadDumpParser(options, isBackground);
threads = threadDumpParser.parse(lines);
final List<SentryThread> threads = threadDumpParser.parse(lines);
if (threads.isEmpty()) {
// if the list is empty this means our regex matching is garbage and this is still error
return new ParseResult(ParseResult.Type.ERROR, dump);
}
return new ParseResult(ParseResult.Type.DUMP, dump, threads);
} catch (Throwable e) {
options.getLogger().log(SentryLevel.WARNING, "Failed to parse ANR thread dump", e);
return new ParseResult(ParseResult.Type.ERROR, dump);
}
}

private byte[] getDumpBytes(final @NotNull InputStream trace) throws IOException {
final ByteArrayOutputStream buffer = new ByteArrayOutputStream();

int nRead;
final byte[] data = new byte[1024];

while ((nRead = trace.read(data, 0, data.length)) != -1) {
buffer.write(data, 0, nRead);
}

return threads;
return buffer.toByteArray();
}
}

Expand Down Expand Up @@ -313,4 +380,36 @@ public String mechanism() {
return isBackgroundAnr ? "anr_background" : "anr_foreground";
}
}

static final class ParseResult {

enum Type {
DUMP,
NO_DUMP,
ERROR
}

final Type type;
final byte[] dump;
final @Nullable List<SentryThread> threads;

ParseResult(final @NotNull Type type) {
this.type = type;
this.dump = null;
this.threads = null;
}

ParseResult(final @NotNull Type type, final byte[] dump) {
this.type = type;
this.dump = dump;
this.threads = null;
}

ParseResult(
final @NotNull Type type, final byte[] dump, final @Nullable List<SentryThread> threads) {
this.type = type;
this.dump = dump;
this.threads = threads;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.sentry.android.core;

import android.app.ActivityManager;
import android.app.ApplicationExitInfo;
import io.sentry.Hint;
import io.sentry.ISpan;
import io.sentry.Scope;
Expand All @@ -8,6 +10,7 @@
import io.sentry.SentryOptions;
import io.sentry.SpanStatus;
import io.sentry.android.core.internal.util.RootChecker;
import io.sentry.protocol.Mechanism;
import io.sentry.protocol.SdkVersion;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -182,6 +185,24 @@ public interface BeforeCaptureCallback {
boolean execute(@NotNull SentryEvent event, @NotNull Hint hint, boolean debounce);
}

/**
* Controls whether to report historical ANRs (v2) from the {@link ApplicationExitInfo} system
* API. When enabled, reports all of the ANRs available in the {@link
* ActivityManager#getHistoricalProcessExitReasons(String, int, int)} list, as opposed to
* reporting only the latest one.
*
* <p>These events do not affect ANR rate nor are they enriched with additional information from
* {@link Scope} like breadcrumbs. The events are reported with 'HistoricalAppExitInfo' {@link
* Mechanism}.
*/
private boolean reportHistoricalAnrs = false;

/**
* Controls whether to send ANR (v2) thread dump as an attachment with plain text. The thread dump
* is being attached from {@link ApplicationExitInfo#getTraceInputStream()}, if available.
*/
private boolean attachAnrThreadDump = false;

public SentryAndroidOptions() {
setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
setSdkVersion(createSdkVersion());
Expand Down Expand Up @@ -510,4 +531,20 @@ public void setBeforeViewHierarchyCaptureCallback(
final @NotNull BeforeCaptureCallback beforeViewHierarchyCaptureCallback) {
this.beforeViewHierarchyCaptureCallback = beforeViewHierarchyCaptureCallback;
}

public boolean isReportHistoricalAnrs() {
return reportHistoricalAnrs;
}

public void setReportHistoricalAnrs(final boolean reportHistoricalAnrs) {
this.reportHistoricalAnrs = reportHistoricalAnrs;
}

public boolean isAttachAnrThreadDump() {
return attachAnrThreadDump;
}

public void setAttachAnrThreadDump(final boolean attachAnrThreadDump) {
this.attachAnrThreadDump = attachAnrThreadDump;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ public class ThreadDumpParser {
private static final Pattern BEGIN_MANAGED_THREAD_RE =
Pattern.compile("\"(.*)\" (.*) ?prio=(\\d+)\\s+tid=(\\d+)\\s*(.*)");
private static final Pattern NATIVE_RE =
Pattern.compile(" (?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*)\\s+\\((.*)\\+(\\d+)\\)");
Pattern.compile(
" (?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*?)\\s+\\((.*)\\+(\\d+)\\)(?: \\(.*\\))?");
private static final Pattern NATIVE_NO_LOC_RE =
Pattern.compile(" (?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*)\\s*\\(?(.*)\\)?");
Pattern.compile(
" (?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*)\\s*\\(?(.*)\\)?(?: \\(.*\\))?");
private static final Pattern JAVA_RE =
Pattern.compile(" at (?:(.+)\\.)?([^.]+)\\.([^.]+)\\((.*):([\\d-]+)\\)");
private static final Pattern JNI_RE =
Expand Down Expand Up @@ -179,14 +181,14 @@ private SentryStackTrace parseStacktrace(
if (matches(nativeRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
frame.setPackage(nativeRe.group(1));
frame.setSymbol(nativeRe.group(2));
frame.setFunction(nativeRe.group(2));
frame.setLineno(getInteger(nativeRe, 3, null));
frames.add(frame);
lastJavaFrame = null;
} else if (matches(nativeNoLocRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
frame.setPackage(nativeNoLocRe.group(1));
frame.setSymbol(nativeNoLocRe.group(2));
frame.setFunction(nativeNoLocRe.group(2));
frames.add(frame);
lastJavaFrame = null;
} else if (matches(javaRe, text)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class AnrV2EventProcessorTest {
persistScope(
CONTEXTS_FILENAME,
Contexts().apply {
trace = SpanContext("test")
setResponse(Response().apply { bodySize = 1024 })
setBrowser(Browser().apply { name = "Google Chrome" })
}
Expand Down Expand Up @@ -169,6 +170,15 @@ class AnrV2EventProcessorTest {
assertEquals(emptyMap(), processed.contexts)
}

@Test
fun `when backfillable event is not enrichable, sets different mechanism`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint(shouldEnrich = false))

val processed = processEvent(hint)

assertEquals("HistoricalAppExitInfo", processed.exceptions!![0].mechanism!!.type)
}

@Test
fun `when backfillable event is not enrichable, sets platform`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint(shouldEnrich = false))
Expand Down
Loading

0 comments on commit 2c45a25

Please sign in to comment.