-
-
Notifications
You must be signed in to change notification settings - Fork 466
feat(anr): Profile main thread when ANR and report ANR profiles to Sentry #4899
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
base: main
Are you sure you want to change the base?
Changes from all commits
b286ad5
a62b5e8
ae66f73
f226d84
7d423a4
5824f8f
6e7fff2
3d4d952
69170ce
cee86fc
49c2e20
93141dd
a59bf08
5bfff6a
6c9acd7
4c95292
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,9 +12,20 @@ | |
| import io.sentry.ILogger; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.Integration; | ||
| import io.sentry.ProfileChunk; | ||
| import io.sentry.ProfileContext; | ||
| import io.sentry.SentryEvent; | ||
| import io.sentry.SentryExceptionFactory; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.SentryStackTraceFactory; | ||
| import io.sentry.android.core.anr.AggregatedStackTrace; | ||
| import io.sentry.android.core.anr.AnrCulpritIdentifier; | ||
| import io.sentry.android.core.anr.AnrException; | ||
| import io.sentry.android.core.anr.AnrProfile; | ||
| import io.sentry.android.core.anr.AnrProfileManager; | ||
| import io.sentry.android.core.anr.AnrProfileRotationHelper; | ||
| import io.sentry.android.core.anr.StackTraceConverter; | ||
| import io.sentry.android.core.cache.AndroidEnvelopeCache; | ||
| import io.sentry.android.core.internal.threaddump.Lines; | ||
| import io.sentry.android.core.internal.threaddump.ThreadDumpParser; | ||
|
|
@@ -26,8 +37,12 @@ | |
| import io.sentry.protocol.DebugImage; | ||
| import io.sentry.protocol.DebugMeta; | ||
| import io.sentry.protocol.Message; | ||
| import io.sentry.protocol.SentryException; | ||
| import io.sentry.protocol.SentryId; | ||
| import io.sentry.protocol.SentryStackFrame; | ||
| import io.sentry.protocol.SentryStackTrace; | ||
| import io.sentry.protocol.SentryThread; | ||
| import io.sentry.protocol.profiling.SentryProfile; | ||
| import io.sentry.transport.CurrentDateProvider; | ||
| import io.sentry.transport.ICurrentDateProvider; | ||
| import io.sentry.util.HintUtils; | ||
|
|
@@ -36,11 +51,14 @@ | |
| import java.io.ByteArrayInputStream; | ||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.Closeable; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.InputStreamReader; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
|
|
@@ -284,6 +302,20 @@ private void reportAsSentryEvent( | |
| } | ||
| } | ||
|
|
||
| if (options.isEnableAnrProfiling()) { | ||
| applyAnrProfile(isBackground, anrTimestamp, event); | ||
| // TODO: maybe move to AnrV2EventProcessor instead | ||
| if (hasOnlySystemFrames(event)) { | ||
| // By omitting the {{ default }} fingerprint, the stacktrace will be completely ignored | ||
| // and all events will be grouped | ||
| // into the same issue | ||
| event.setFingerprints( | ||
| Arrays.asList( | ||
| "{{ system-frames-only-anr }}", | ||
| isBackground ? "background-anr" : "foreground-anr")); | ||
| } | ||
| } | ||
|
|
||
| final @NotNull SentryId sentryId = scopes.captureEvent(event, hint); | ||
| final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID); | ||
| if (!isEventDropped) { | ||
|
|
@@ -299,6 +331,94 @@ private void reportAsSentryEvent( | |
| } | ||
| } | ||
|
|
||
| private void applyAnrProfile( | ||
| final boolean isBackground, final long anrTimestamp, final @NotNull SentryEvent event) { | ||
|
|
||
| // as of now AnrProfilingIntegration only generates profiles in foreground | ||
| if (isBackground) { | ||
| return; | ||
| } | ||
|
|
||
| final @Nullable String cacheDirPath = options.getCacheDirPath(); | ||
| if (cacheDirPath == null) { | ||
| return; | ||
| } | ||
| final @NotNull File cacheDir = new File(cacheDirPath); | ||
|
|
||
| @Nullable AnrProfile anrProfile = null; | ||
|
|
||
| try { | ||
| final File lastFile = AnrProfileRotationHelper.getLastFile(cacheDir); | ||
|
|
||
| if (lastFile.exists()) { | ||
| options.getLogger().log(SentryLevel.DEBUG, "Reading ANR profile"); | ||
| try (final AnrProfileManager provider = new AnrProfileManager(options, lastFile)) { | ||
| anrProfile = provider.load(); | ||
| } | ||
| } else { | ||
| options.getLogger().log(SentryLevel.DEBUG, "No ANR profile file found"); | ||
| } | ||
| } catch (Throwable t) { | ||
| options.getLogger().log(SentryLevel.INFO, "Could not retrieve ANR profile", t); | ||
| } finally { | ||
| if (!AnrProfileRotationHelper.deleteLastFile(cacheDir)) { | ||
| options.getLogger().log(SentryLevel.INFO, "Could not delete ANR profile file"); | ||
| } | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (anrProfile != null) { | ||
| options.getLogger().log(SentryLevel.INFO, "ANR profile found"); | ||
| if (anrTimestamp >= anrProfile.startTimeMs && anrTimestamp <= anrProfile.endtimeMs) { | ||
| final @Nullable AggregatedStackTrace culprit = | ||
| AnrCulpritIdentifier.identify(anrProfile.stacks); | ||
| if (culprit != null) { | ||
| final @Nullable SentryId profilerId = captureAnrProfile(anrTimestamp, anrProfile); | ||
|
|
||
| final @NotNull StackTraceElement[] stack = culprit.getStack(); | ||
| if (stack.length > 0) { | ||
| final StackTraceElement stackTraceElement = culprit.getStack()[0]; | ||
| final String message = | ||
| stackTraceElement.getClassName() + "." + stackTraceElement.getMethodName(); | ||
| final AnrException exception = new AnrException(message); | ||
| exception.setStackTrace(stack); | ||
|
|
||
| // TODO should this be re-used from somewhere else? | ||
| final SentryExceptionFactory factory = | ||
| new SentryExceptionFactory(new SentryStackTraceFactory(options)); | ||
| event.setExceptions(factory.getSentryExceptions(exception)); | ||
| if (profilerId != null) { | ||
| event.getContexts().setProfile(new ProfileContext(profilerId)); | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| options.getLogger().log(SentryLevel.DEBUG, "ANR profile found, but doesn't match"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| private SentryId captureAnrProfile( | ||
| final long anrTimestamp, final @NotNull AnrProfile anrProfile) { | ||
| final SentryProfile profile = StackTraceConverter.convert(anrProfile); | ||
| final ProfileChunk chunk = | ||
| new ProfileChunk( | ||
| new SentryId(), | ||
| new SentryId(), | ||
| null, | ||
| new HashMap<>(0), | ||
| anrTimestamp / 1000.0d, | ||
| ProfileChunk.PLATFORM_JAVA, | ||
| options); | ||
| chunk.setSentryProfile(profile); | ||
| final SentryId profilerId = scopes.captureProfileChunk(chunk); | ||
| if (profilerId.equals(SentryId.EMPTY_ID)) { | ||
| return null; | ||
| } else { | ||
| return chunk.getProfilerId(); | ||
markushi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| private @NotNull ParseResult parseThreadDump( | ||
| final @NotNull ApplicationExitInfo exitInfo, final boolean isBackground) { | ||
| final byte[] dump; | ||
|
|
@@ -352,6 +472,30 @@ private byte[] getDumpBytes(final @NotNull InputStream trace) throws IOException | |
| } | ||
| } | ||
|
|
||
| private static boolean hasOnlySystemFrames(final @NotNull SentryEvent event) { | ||
| final List<SentryException> exceptions = event.getExceptions(); | ||
| if (exceptions != null) { | ||
| for (final SentryException exception : exceptions) { | ||
| final @Nullable SentryStackTrace stacktrace = exception.getStacktrace(); | ||
| if (stacktrace != null) { | ||
| final @Nullable List<SentryStackFrame> frames = stacktrace.getFrames(); | ||
| if (frames != null && !frames.isEmpty()) { | ||
| for (final SentryStackFrame frame : frames) { | ||
| if (frame.isInApp() != null && frame.isInApp()) { | ||
| return false; | ||
| } | ||
| final @Nullable String module = frame.getModule(); | ||
| if (module != null && !AnrCulpritIdentifier.isSystemFrame(frame.getModule())) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Fingerprinting applied when no exceptions existThe Additional Locations (1) |
||
|
|
||
| @ApiStatus.Internal | ||
| public static final class AnrV2Hint extends BlockingFlushHint | ||
| implements Backfillable, AbnormalExit { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: When ANR profiling fails,
hasOnlySystemFrames()incorrectly returnstruebecause no exceptions are added, causing a "system-frames-only-anr" fingerprint to be applied to events with unverified frames.Severity: HIGH | Confidence: High
🔍 Detailed Analysis
When ANR profiling is enabled but fails to apply a profile for any reason (e.g., missing file, timestamp mismatch), the event will have no exceptions. The
hasOnlySystemFrames()method incorrectly returnstruewhen the exception list isnull. As a result, a special"system-frames-only-anr"fingerprint is applied to the event, even though it contains a valid thread dump with potentially non-system frames. This leads to incorrect event grouping, merging diverse ANRs into a single issue and obscuring their true root causes when profiling fails.💡 Suggested Fix
Modify the logic to only apply the
"system-frames-only-anr"fingerprint when the ANR profile has been successfully applied and exceptions have been added to the event. ThehasOnlySystemFrames()method should returnfalseif the exception list isnullor empty, ensuring the fingerprint is only applied after frames have been verified.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID:
7690328