Skip to content

feat(anr): Profile main thread when ANR and report ANR profiles to Sentry#4899

Open
markushi wants to merge 34 commits intomainfrom
markushi/feat/anr-profiling
Open

feat(anr): Profile main thread when ANR and report ANR profiles to Sentry#4899
markushi wants to merge 34 commits intomainfrom
markushi/feat/anr-profiling

Conversation

@markushi
Copy link
Member

@markushi markushi commented Nov 12, 2025

📜 Description

Adds ANR (Application Not Responding) profiling integration that profiles the main thread when an ANR is detected and reports the captured profiles to Sentry.

Key Changes:

  • New AnrProfilingIntegration to capture profiles during ANR events
  • AnrV2Integration now takes care of matching and capturing the profile on the next start
  • If the captured ANR event only contains system frames, a static fingerprint will get set, effectively changing the grouping behavior to group all noisy ANRs into a single issue

💡 Motivation and Context

This feature enables better ANR diagnostics by capturing profiling data at the time of ANR detection, allowing developers to identify performance bottlenecks and problematic code paths causing application hangs.

Example event: https://sentry-sdks.sentry.io/issues/7229210096/events/4598ff6fcc0f402d8ecca615005e7f64/

💚 How did you test it?

  • Added tests

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 39229d5

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 289.51 ms 359.74 ms 70.23 ms
Size 1.58 MiB 2.29 MiB 724.15 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a416a65 333.78 ms 410.37 ms 76.59 ms
d15471f 342.08 ms 415.44 ms 73.35 ms
e59e22a 329.74 ms 383.31 ms 53.57 ms
d15471f 315.20 ms 370.22 ms 55.02 ms
d364ace 382.77 ms 443.21 ms 60.44 ms
b3d8889 371.33 ms 426.24 ms 54.92 ms
a416a65 316.52 ms 359.67 ms 43.15 ms
b03edbb 352.20 ms 423.69 ms 71.49 ms
d15471f 304.55 ms 408.43 ms 103.87 ms
319f256 315.96 ms 372.96 ms 57.00 ms

App size

Revision Plain With Sentry Diff
a416a65 1.58 MiB 2.12 MiB 555.26 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
e59e22a 1.58 MiB 2.20 MiB 635.34 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
d364ace 1.58 MiB 2.11 MiB 539.75 KiB
b3d8889 1.58 MiB 2.10 MiB 535.07 KiB
a416a65 1.58 MiB 2.12 MiB 555.26 KiB
b03edbb 1.58 MiB 2.13 MiB 557.32 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
319f256 1.58 MiB 2.19 MiB 619.79 KiB

Previous results on branch: markushi/feat/anr-profiling

Startup times

Revision Plain With Sentry Diff
00299fd 359.87 ms 424.85 ms 64.98 ms
c10e603 367.92 ms 393.50 ms 25.58 ms
eb7143a 347.66 ms 408.54 ms 60.88 ms
2cee1ab 318.29 ms 361.00 ms 42.71 ms
fa76e86 274.32 ms 349.63 ms 75.31 ms
4c0ffee 314.94 ms 377.79 ms 62.86 ms
83a9ec4 333.84 ms 390.30 ms 56.47 ms
fca8df8 326.79 ms 379.69 ms 52.90 ms
31581b9 350.00 ms 420.63 ms 70.63 ms

App size

Revision Plain With Sentry Diff
00299fd 1.58 MiB 2.29 MiB 723.50 KiB
c10e603 1.58 MiB 2.29 MiB 723.72 KiB
eb7143a 1.58 MiB 2.29 MiB 724.12 KiB
2cee1ab 1.58 MiB 2.29 MiB 723.68 KiB
fa76e86 1.58 MiB 2.29 MiB 724.06 KiB
4c0ffee 1.58 MiB 2.29 MiB 723.67 KiB
83a9ec4 1.58 MiB 2.29 MiB 723.99 KiB
fca8df8 1.58 MiB 2.29 MiB 723.68 KiB
31581b9 1.58 MiB 2.19 MiB 624.94 KiB

@markushi markushi marked this pull request as draft December 3, 2025 07:19
@markushi
Copy link
Member Author

@sentry review

@markushi markushi marked this pull request as ready for review December 17, 2025 09:51
return;
}

if (options.isEnableAnrProfiling() && hasOnlySystemFrames(event)) {
Copy link
Member

Choose a reason for hiding this comment

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

wondering whether we should guard this behind isEnableAnrProfiling or actually just do it for all ANRs going forward? I think AEI also gives quite a lot of noise with just system frames, right?

Copy link
Member Author

@markushi markushi Feb 12, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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)

final StackTraceElement stackTraceElement = stack[0];
final String message =
stackTraceElement.getClassName() + "." + stackTraceElement.getMethodName();
final AnrException exception = new AnrException(message);
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think I looked into this early on, but ApplicationNotResponding requires a non-null Thread object, which seemed to be widely used and hard to refactor. But that's a good point, let me check that again.

options);
chunk.setSentryProfile(profile);

final SentryId profilerId = Sentry.getCurrentScopes().captureProfileChunk(chunk);
Copy link
Member

Choose a reason for hiding this comment

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

just curious, but is there a way to send both the ANR event and the profile chunk in the same envelope?

Copy link
Member Author

Choose a reason for hiding this comment

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

claude says yes, this should be valid for the backend😅

Yes, this works. The client can send both in the same envelope.
During split_envelope() at relay-server/src/services/processor.rs:357-365, Relay splits them apart before any event-type routing:

  1. ProfileChunk items are extracted into their own ProcessingGroup::ProfileChunk envelope
  2. The remaining error event goes into ProcessingGroup::Error
  3. Both are processed independently through their respective pipelines

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 (

if (attachments != null) {
for (final Attachment attachment : attachments) {
final SentryEnvelopeItem attachmentItem =
SentryEnvelopeItem.fromAttachment(
options.getSerializer(),
options.getLogger(),
attachment,
options.getMaxAttachmentSize());
envelopeItems.add(attachmentItem);
}
}
)

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

threadMetadata.setName(MAIN_THREAD_NAME);
threadMetadata.setPriority(Thread.NORM_PRIORITY);

final @NotNull Map<String, SentryThreadMetadata> threadMetadataMap = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

I guess this could've been Collections.singletonMap()

// common Java and Android packages who are less relevant for being the actual culprit
private static final List<String> systemAndFrameworkPackages = new ArrayList<>(9);

static {
Copy link
Member

Choose a reason for hiding this comment

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

not sure if it's worth adding some kotlin frames in here as well?

"SentryAndroidOptions is required");
this.logger = options.getLogger();

if (((SentryAndroidOptions) options).isEnableAnrProfiling()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (((SentryAndroidOptions) options).isEnableAnrProfiling()) {
if (this.options.isEnableAnrProfiling()) {

since we already cast it above

Comment on lines +44 to +48
try {
currentFile.renameTo(oldFile);
} catch (Throwable e) {
// ignored
}
Copy link

Choose a reason for hiding this comment

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

Bug: The return value of File.renameTo() is not checked, causing silent failures in ANR profile rotation and leading to a permanently inconsistent state.
Severity: MEDIUM

Suggested Fix

Check the boolean return value of currentFile.renameTo(oldFile). If it returns false, log a warning and do not set shouldRotate to false. This will allow the rotation to be re-attempted on the next trigger, preventing a permanently broken state.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfileRotationHelper.java#L44-L48

Potential issue: The `renameTo()` method is used to rotate ANR profile files, but its
boolean return value, which indicates success or failure, is ignored. The code is
wrapped in a `try-catch (Throwable)` block that also silences any potential issues.
After the call, `shouldRotate` is unconditionally set to `false`. If `renameTo()` fails
for any reason (e.g., file permissions, cross-filesystem move), the rotation will
silently fail, but the system will act as if it succeeded. This leads to a permanently
inconsistent state where future rotations are blocked, potentially causing ANR profile
data loss or corruption as both profiling integrations might access the same file.

try (final @NotNull ISentryLifecycleToken ignored = profileManagerLock.acquire()) {
final @Nullable AnrProfileManager p = profileManager;
if (p != null) {
p.close();
Copy link
Member

Choose a reason for hiding this comment

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

hm, I think this one does I/O under the hood right (raf.close())? We should probably move it out of the lock here (since we capture it in the p variable) and also offload to a bg thread perhaps?

oldThread.interrupt();
}

final @NotNull Thread profilingThread = new Thread(this, "AnrProfilingIntegration");
Copy link
Member

@romtsn romtsn Feb 13, 2026

Choose a reason for hiding this comment

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

can we actually reuse the thread as opposed to creating a new one on every foreground? like checking the inForeground flag inside the run method in addition to isInterrupted?

also not sure if it makes any difference on android as to whether to make it isDaemon or not

// get main thread Handler so we can post messages
final Looper mainLooper = Looper.getMainLooper();
final Thread mainThread = mainLooper.getThread();
final Handler mainHandler = new Handler(mainLooper);
Copy link
Member

Choose a reason for hiding this comment

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

I guess we could memoize this and capture it in register if anr profiling is enabled? Would avoid allocating a new handler on every run

static final int MAX_NUM_STACKS = (int) (10_000 / POLLING_INTERVAL_MS);

private final AtomicBoolean enabled = new AtomicBoolean(true);
private final Runnable updater = () -> lastMainThreadExecutionTime = SystemClock.uptimeMillis();
Copy link
Member

Choose a reason for hiding this comment

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

I realized we use AndroidCourrentDateProvider instead of SystemClock.uptimeMillis directly, but I'm fine with using systemclock directly since you can override it in tests easily

Comment on lines +179 to +180
final long duration = SystemClock.uptimeMillis() - start;
if (logger.isEnabled(SentryLevel.DEBUG)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final long duration = SystemClock.uptimeMillis() - start;
if (logger.isEnabled(SentryLevel.DEBUG)) {
if (logger.isEnabled(SentryLevel.DEBUG)) {
final long duration = SystemClock.uptimeMillis() - start;

Copy link
Member

Choose a reason for hiding this comment

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

btw, does mainThread.getStackTrace() affects the main thread? or is this log just for debugging purposes?

if (cacheDirPath == null) {
throw new IllegalStateException("cacheDirPath is required for ANR profiling");
}
final @NotNull File currentFile =
Copy link
Member

Choose a reason for hiding this comment

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

alright, I think I found a way this will cause ANRs (similar to replay) - if close() is called from the main thread, we'll be holding the lock, awaiting on the operations here to finish, which can take a while due to I/O in getFileForRecording and AnrProfileManager.<init>.

I guess to make it non-blocking we'd still have to make QueueFile instantiation lazy and move getFileForRecording inside the lazy block, wdyt?

Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

Great work already! I believe there are some things to address but I can check once more after that


final Mechanism mechanism = new Mechanism();
mechanism.setType("ANR");
final ExceptionMechanismException error =
Copy link
Member

Choose a reason for hiding this comment

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

I think this one turns to be unused, shall we pass it to getSentryExceptions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants