feat(anr): Profile main thread when ANR and report ANR profiles to Sentry #4899
+2,522
−42
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,13 +29,23 @@ | |
| import io.sentry.Breadcrumb; | ||
| import io.sentry.Hint; | ||
| import io.sentry.IpAddressUtils; | ||
| import io.sentry.ProfileChunk; | ||
| import io.sentry.ProfileContext; | ||
| import io.sentry.Sentry; | ||
| import io.sentry.SentryBaseEvent; | ||
| import io.sentry.SentryEvent; | ||
| import io.sentry.SentryExceptionFactory; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.SentryStackTraceFactory; | ||
| import io.sentry.SpanContext; | ||
| 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.internal.util.CpuInfoUtils; | ||
| import io.sentry.cache.PersistingOptionsObserver; | ||
| import io.sentry.cache.PersistingScopeObserver; | ||
|
|
@@ -50,10 +60,14 @@ | |
| import io.sentry.protocol.OperatingSystem; | ||
| import io.sentry.protocol.Request; | ||
| import io.sentry.protocol.SdkVersion; | ||
| 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.SentryTransaction; | ||
| import io.sentry.protocol.User; | ||
| import io.sentry.protocol.profiling.SentryProfile; | ||
| import io.sentry.util.HintUtils; | ||
| import io.sentry.util.SentryRandom; | ||
| import java.io.File; | ||
|
|
@@ -700,16 +714,33 @@ public void applyPreEnrichment( | |
| public void applyPostEnrichment( | ||
| @NotNull SentryEvent event, @NotNull Backfillable hint, @NotNull Object rawHint) { | ||
| final boolean isBackgroundAnr = isBackgroundAnr(rawHint); | ||
|
|
||
| if (options.isEnableAnrProfiling()) { | ||
| applyAnrProfile(event, hint, isBackgroundAnr); | ||
| } | ||
|
|
||
markushi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| setDefaultAnrFingerprint(event, isBackgroundAnr); | ||
|
|
||
| // Set app foreground state | ||
| setAppForeground(event, !isBackgroundAnr); | ||
| } | ||
|
|
||
| private void setDefaultAnrFingerprint( | ||
| final @NotNull SentryEvent event, final boolean isBackgroundAnr) { | ||
| // sentry does not yet have a capability to provide default server-side fingerprint rules, | ||
| // so we're doing this on the SDK side to group background and foreground ANRs separately | ||
| // even if they have similar stacktraces. | ||
| if (event.getFingerprints() != null) { | ||
| return; | ||
| } | ||
|
|
||
| if (options.isEnableAnrProfiling() && hasOnlySystemFrames(event)) { | ||
|
Member
There was a problem hiding this comment. wondering whether we should guard this behind
Member
Author
There was a problem hiding this comment. Yes, exactly - we should do this everywhere in the long run. I still would guard this right now - otherwise we'll have a breaking change in default behavior. I can create a follow up ticket for the next major.
Member
There was a problem hiding this comment. I think we've done this quite a lot in the past (breaking behaviour change that affects grouping) so I'm not opposed to doing this now and not wait for the next major 😅 If it improves things I think I'd rather do it sooner. But your call here (we could also wait to get some adoption and see how it performs before doing this for everyone) |
||
| // If profiling did not identify any app frames, we want to statically group these events | ||
| // to avoid ANR noise due to {{ default }} stacktrace grouping | ||
| event.setFingerprints( | ||
| Arrays.asList( | ||
| "system-frames-only-anr", isBackgroundAnr ? "background-anr" : "foreground-anr")); | ||
| } else { | ||
| event.setFingerprints( | ||
| Arrays.asList("{{ default }}", isBackgroundAnr ? "background-anr" : "foreground-anr")); | ||
| } | ||
|
|
@@ -777,5 +808,142 @@ private void setAnrExceptions( | |
| event.setExceptions( | ||
| sentryExceptionFactory.getSentryExceptionsFromThread(mainThread, mechanism, anr)); | ||
| } | ||
|
|
||
| private void applyAnrProfile( | ||
| @NotNull SentryEvent event, @NotNull Backfillable hint, boolean isBackgroundAnr) { | ||
|
|
||
| // Skip background ANRs (as profiling only runs in foreground) | ||
| if (isBackgroundAnr) { | ||
| return; | ||
| } | ||
|
|
||
| final String cacheDirPath = options.getCacheDirPath(); | ||
| if (cacheDirPath == null) { | ||
| return; | ||
| } | ||
| final File cacheDir = new File(cacheDirPath); | ||
|
|
||
| if (!(hint instanceof AbnormalExit)) { | ||
| return; | ||
| } | ||
| final Long anrTimestampObj = ((AbnormalExit) hint).timestamp(); | ||
| final long anrTimestamp; | ||
| if (anrTimestampObj != null) { | ||
| anrTimestamp = anrTimestampObj; | ||
| } else if (event.getTimestamp() != null) { | ||
| anrTimestamp = event.getTimestamp().getTime(); | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
| 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"); | ||
| } | ||
| } | ||
|
|
||
| if (anrProfile == null) { | ||
| return; | ||
| } | ||
|
|
||
| options.getLogger().log(SentryLevel.INFO, "ANR profile found"); | ||
| if (anrTimestamp < anrProfile.startTimeMs || anrTimestamp > anrProfile.endTimeMs) { | ||
| options.getLogger().log(SentryLevel.DEBUG, "ANR profile found, but doesn't match"); | ||
| return; | ||
| } | ||
|
|
||
| final AggregatedStackTrace culprit = AnrCulpritIdentifier.identify(anrProfile.stacks); | ||
| if (culprit == null) { | ||
| return; | ||
| } | ||
|
|
||
| // Capture profile chunk | ||
| final SentryId profilerId = captureAnrProfile(anrTimestamp, anrProfile); | ||
|
|
||
| final StackTraceElement[] stack = culprit.getStack(); | ||
| if (stack.length > 0) { | ||
| final StackTraceElement stackTraceElement = stack[0]; | ||
| final String message = | ||
| stackTraceElement.getClassName() + "." + stackTraceElement.getMethodName(); | ||
| final AnrException exception = new AnrException(message); | ||
|
Member
There was a problem hiding this comment. is the new AnrException type intentional here? Because it's a new type it will create new groups (even if the profile-derived stacktrace matches the one from AEI). Not sure if we should keep it as ApplicationNotResponding?
Member
Author
There was a problem hiding this comment. Yes, I think I looked into this early on, but |
||
| exception.setStackTrace(stack); | ||
|
|
||
| final @NotNull List<SentryException> sentryException = | ||
| sentryExceptionFactory.getSentryExceptions(exception); | ||
| for (final @NotNull SentryException e : sentryException) { | ||
| final Mechanism mechanism = new Mechanism(); | ||
| mechanism.setType("ANR"); | ||
romtsn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| e.setMechanism(mechanism); | ||
| } | ||
| // Replaces the original ANR exception with the profile-derived one, | ||
| // as we assume the profiling stacktrace is more detailed | ||
| event.setExceptions(sentryException); | ||
|
|
||
| if (profilerId != null) { | ||
| event.getContexts().setProfile(new ProfileContext(profilerId)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| private SentryId captureAnrProfile(final long anrTimestampMs, @NotNull AnrProfile anrProfile) { | ||
| final SentryProfile profile = StackTraceConverter.convert(anrProfile); | ||
| final ProfileChunk chunk = | ||
| new ProfileChunk( | ||
| new SentryId(), | ||
| new SentryId(), | ||
| null, | ||
| new HashMap<>(0), | ||
| anrTimestampMs / 1000.0d, | ||
| ProfileChunk.PLATFORM_JAVA, | ||
| options); | ||
| chunk.setSentryProfile(profile); | ||
|
|
||
| final SentryId profilerId = Sentry.getCurrentScopes().captureProfileChunk(chunk); | ||
|
Member
There was a problem hiding this comment. just curious, but is there a way to send both the ANR event and the profile chunk in the same envelope?
Member
Author
There was a problem hiding this comment. claude says yes, this should be valid for the backend😅
We'd need to attach the profile directly to the event (or via hint) and then do something similar as we do for e.g. attachments ( sentry-java/sentry/src/main/java/io/sentry/SentryClient.java Lines 422 to 432 in abf451a
Member
There was a problem hiding this comment. yeah, or replay_recording, too. I guess this will be better in terms of not sending one without the other, but I don't have a strong preference right now, we can probably create a followup issue if you don't feel like doing it now. |
||
| if (SentryId.EMPTY_ID.equals(profilerId)) { | ||
| return null; | ||
| } else { | ||
| return chunk.getProfilerId(); | ||
| } | ||
| } | ||
|
|
||
| private boolean hasOnlySystemFrames(@NotNull SentryEvent event) { | ||
| final List<SentryException> exceptions = event.getExceptions(); | ||
| if (exceptions == null || exceptions.isEmpty()) { | ||
| return false; | ||
| } | ||
|
|
||
| for (final SentryException exception : exceptions) { | ||
| final SentryStackTrace stacktrace = exception.getStacktrace(); | ||
| if (stacktrace != null) { | ||
| final List<SentryStackFrame> frames = stacktrace.getFrames(); | ||
| if (frames != null && !frames.isEmpty()) { | ||
| for (final SentryStackFrame frame : frames) { | ||
| if (frame.isInApp() != null && frame.isInApp()) { | ||
| return false; | ||
| } | ||
| final String module = frame.getModule(); | ||
| if (module != null && !AnrCulpritIdentifier.isSystemFrame(module)) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
markushi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.