Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
- This feature will capture a stack profile of the main thread when it gets unresponsive
- The profile gets attached to the ANR event on the next app start, providing a flamegraph of the ANR issue on the sentry issue details page
- Breaking change: if the ANR stacktrace contains only system frames (e.g. `java.lang` or `android.os`), a static fingerprint is set on the ANR event, causing all ANR events to be grouped into a single issue, reducing the overall ANR issue noise
- Enable via `options.setEnableAnrProfiling(true)` or Android manifest: `<meta-data android:name="io.sentry.anr.profiling.enable" android:value="true" />`
- Enable via `options.setAnrProfilingSampleRate(<sample-rate>)` or AndroidManifest.xml: `<meta-data android:name="io.sentry.anr.profiling.sample-rate" android:value="[0.0-1.0]" />`
- The sample rate controls the probability of collecting a profile for each detected foreground ANR (0.0 to 1.0, null to disable)

### Fixes

Expand Down
5 changes: 3 additions & 2 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ public final class io/sentry/android/core/SentryAndroidDateProvider : io/sentry/
public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/SentryOptions {
public fun <init> ()V
public fun enableAllAutoBreadcrumbs (Z)V
public fun getAnrProfilingSampleRate ()Ljava/lang/Double;
public fun getAnrTimeoutIntervalMillis ()J
public fun getBeforeScreenshotCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;
public fun getBeforeViewHierarchyCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback;
Expand All @@ -357,6 +358,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun getScreenshot ()Lio/sentry/android/core/SentryScreenshotOptions;
public fun getStartupCrashDurationThresholdMillis ()J
public fun isAnrEnabled ()Z
public fun isAnrProfilingEnabled ()Z
public fun isAnrReportInDebug ()Z
public fun isAttachAnrThreadDump ()Z
public fun isAttachScreenshot ()Z
Expand All @@ -365,7 +367,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun isCollectExternalStorageContext ()Z
public fun isEnableActivityLifecycleBreadcrumbs ()Z
public fun isEnableActivityLifecycleTracingAutoFinish ()Z
public fun isEnableAnrProfiling ()Z
public fun isEnableAppComponentBreadcrumbs ()Z
public fun isEnableAppLifecycleBreadcrumbs ()Z
public fun isEnableAutoActivityLifecycleTracing ()Z
Expand All @@ -382,6 +383,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun isReportHistoricalTombstones ()Z
public fun isTombstoneEnabled ()Z
public fun setAnrEnabled (Z)V
public fun setAnrProfilingSampleRate (Ljava/lang/Double;)V
public fun setAnrReportInDebug (Z)V
public fun setAnrTimeoutIntervalMillis (J)V
public fun setAttachAnrThreadDump (Z)V
Expand All @@ -394,7 +396,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setDebugImagesLoader (Lio/sentry/android/core/IDebugImagesLoader;)V
public fun setEnableActivityLifecycleBreadcrumbs (Z)V
public fun setEnableActivityLifecycleTracingAutoFinish (Z)V
public fun setEnableAnrProfiling (Z)V
public fun setEnableAppComponentBreadcrumbs (Z)V
public fun setEnableAppLifecycleBreadcrumbs (Z)V
public fun setEnableAutoActivityLifecycleTracing (Z)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ public void applyPostEnrichment(
@NotNull SentryEvent event, @NotNull Backfillable hint, @NotNull Object rawHint) {
final boolean isBackgroundAnr = isBackgroundAnr(rawHint);

if (options.isEnableAnrProfiling()) {
if (options.isAnrProfilingEnabled()) {
applyAnrProfile(event, hint, isBackgroundAnr);
}

Expand All @@ -734,7 +734,7 @@ private void setDefaultAnrFingerprint(
return;
}

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

Choose a reason for hiding this comment

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

Bug: Unsampled ANR events are incorrectly fingerprinted as "system-frames-only-anr" because the check isAnrProfilingEnabled() doesn't confirm if the specific event was actually profiled.
Severity: MEDIUM

Suggested Fix

The setDefaultAnrFingerprint() method should only apply the "system-frames-only-anr" fingerprint if the ANR event was actually profiled. This can be achieved by having applyAnrProfile() set a flag on the event to indicate a profile was applied. The fingerprinting logic should then check this flag instead of relying on the general isAnrProfilingEnabled() setting.

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/ApplicationExitInfoEventProcessor.java#L737

Potential issue: When ANR profiling is enabled with a sample rate less than 1.0, ANR
events that are not sampled are incorrectly fingerprinted. The logic in
`setDefaultAnrFingerprint` checks if profiling is enabled via `isAnrProfilingEnabled()`,
but does not check if the specific ANR event was actually profiled. If an unsampled ANR
event happens to only contain system frames, it will be assigned the
`"system-frames-only-anr"` fingerprint. This fingerprint is intended for cases where
profiling was performed but found no application frames, leading to incorrect issue
grouping in Sentry for unsampled ANRs.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Member

Choose a reason for hiding this comment

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

sounds legit?

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, let me create a follow-up PR for this.

// 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ final class ManifestMetadataReader {

static final String SCREENSHOT_MASK_ALL_IMAGES = "io.sentry.screenshot.mask-all-images";

static final String ENABLE_ANR_PROFILING = "io.sentry.anr.profiling.enable";
static final String ANR_PROFILING_SAMPLE_RATE = "io.sentry.anr.profiling.sample-rate";

/** ManifestMetadataReader ctor */
private ManifestMetadataReader() {}
Expand Down Expand Up @@ -677,8 +677,13 @@ static void applyMetadata(
.getScreenshot()
.setMaskAllImages(readBool(metadata, logger, SCREENSHOT_MASK_ALL_IMAGES, false));

options.setEnableAnrProfiling(
readBool(metadata, logger, ENABLE_ANR_PROFILING, options.isEnableAnrProfiling()));
if (options.getAnrProfilingSampleRate() == null) {
final double anrProfilingSampleRate =
readDouble(metadata, logger, ANR_PROFILING_SAMPLE_RATE);
if (anrProfilingSampleRate != -1) {
options.setAnrProfilingSampleRate(anrProfilingSampleRate);
}
}
}
options
.getLogger()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.sentry.protocol.Mechanism;
import io.sentry.protocol.SdkVersion;
import io.sentry.protocol.SentryId;
import io.sentry.util.SampleRateUtils;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -252,7 +253,7 @@ public interface BeforeCaptureCallback {
*/
private final @NotNull SentryScreenshotOptions screenshot = new SentryScreenshotOptions();

private boolean enableAnrProfiling = false;
private @Nullable Double anrProfilingSampleRate;

public SentryAndroidOptions() {
setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
Expand Down Expand Up @@ -697,12 +698,22 @@ public void setEnableSystemEventBreadcrumbsExtras(
return screenshot;
}

public boolean isEnableAnrProfiling() {
return enableAnrProfiling;
public @Nullable Double getAnrProfilingSampleRate() {
return anrProfilingSampleRate;
}

public void setEnableAnrProfiling(final boolean enableAnrProfiling) {
this.enableAnrProfiling = enableAnrProfiling;
public void setAnrProfilingSampleRate(final @Nullable Double anrProfilingSampleRate) {
if (!SampleRateUtils.isValidSampleRate(anrProfilingSampleRate)) {
throw new IllegalArgumentException(
"The value "
+ anrProfilingSampleRate
+ " is not valid. Use null to disable or values >= 0.0 and <= 1.0.");
}
this.anrProfilingSampleRate = anrProfilingSampleRate;
}

public boolean isAnrProfilingEnabled() {
return anrProfilingSampleRate != null && anrProfilingSampleRate > 0;
}

static class AndroidUserFeedbackIDialogHandler implements SentryFeedbackOptions.IDialogHandler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.sentry.android.core.SentryAndroidOptions;
import io.sentry.util.AutoClosableReentrantLock;
import io.sentry.util.Objects;
import io.sentry.util.SentryRandom;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -47,6 +48,7 @@ public class AnrProfilingIntegration
private volatile @NotNull ILogger logger = NoOpLogger.getInstance();
private volatile @Nullable SentryAndroidOptions options;
private volatile @Nullable Thread thread = null;
private volatile boolean sampled = false;
private volatile boolean inForeground = false;
private volatile @Nullable Handler mainHandler;
private volatile @Nullable Thread mainThread;
Expand All @@ -59,7 +61,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
"SentryAndroidOptions is required");
this.logger = options.getLogger();

if (this.options.isEnableAnrProfiling()) {
if (this.options.isAnrProfilingEnabled()) {
if (this.options.getCacheDirPath() == null) {
logger.log(SentryLevel.WARNING, "ANR Profiling is enabled but cacheDirPath is not set");
return;
Expand Down Expand Up @@ -207,19 +209,30 @@ protected void checkMainThread(final @NotNull Thread mainThread) throws IOExcept

if (diff < THRESHOLD_SUSPICION_MS) {
mainThreadState = MainThreadState.IDLE;
sampled = false;
}

if (mainThreadState == MainThreadState.IDLE && diff > THRESHOLD_SUSPICION_MS) {
if (logger.isEnabled(SentryLevel.DEBUG)) {
logger.log(SentryLevel.DEBUG, "ANR: main thread is suspicious");
}
mainThreadState = MainThreadState.SUSPICIOUS;
clearStacks();

final @Nullable SentryAndroidOptions opts = options;
final @Nullable Double sampleRate = opts != null ? opts.getAnrProfilingSampleRate() : null;
if (sampleRate != null && SentryRandom.current().nextDouble() < sampleRate) {
sampled = true;
}

if (sampled) {
clearStacks();
}
}

// if we are suspicious, we need to collect stack traces
if (mainThreadState == MainThreadState.SUSPICIOUS
|| mainThreadState == MainThreadState.ANR_DETECTED) {
// if we are suspicious and sampled, we need to collect stack traces
if (sampled
&& (mainThreadState == MainThreadState.SUSPICIOUS
|| mainThreadState == MainThreadState.ANR_DETECTED)) {
if (numCollectedStacks.get() < MAX_NUM_STACKS) {
final long start = SystemClock.uptimeMillis();
final @NotNull AnrStackTrace trace =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `sets system-frames-only fingerprint when ANR profiling enabled and no app frames`() {
fixture.options.isEnableAnrProfiling = true
fixture.options.anrProfilingSampleRate = 1.0
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint(mechanism = "anr_foreground"))

val processed =
Expand Down Expand Up @@ -666,7 +666,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `does not set system-frames-only fingerprint when ANR profiling is disabled but no app frames are present`() {
fixture.options.isEnableAnrProfiling = false
fixture.options.anrProfilingSampleRate = null
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint(mechanism = "anr_foreground"))

val processed =
Expand Down Expand Up @@ -695,7 +695,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `sets default fingerprint when ANR profiling enabled and app frames are present`() {
fixture.options.isEnableAnrProfiling = true
fixture.options.anrProfilingSampleRate = 1.0
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint(mechanism = "anr_foreground"))

val processed =
Expand Down Expand Up @@ -723,7 +723,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `does not set profile context when ANR profiling is disabled`() {
fixture.options.isEnableAnrProfiling = false
fixture.options.anrProfilingSampleRate = null
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint(mechanism = "anr_foreground"))
val processed =
processEvent(hint, populateScopeCache = false) {
Expand All @@ -749,7 +749,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `applies ANR profile if available`() {
fixture.options.isEnableAnrProfiling = true
fixture.options.anrProfilingSampleRate = 1.0
val processor =
fixture.getSut(
tmpDir,
Expand Down Expand Up @@ -798,7 +798,7 @@ class ApplicationExitInfoEventProcessorTest {

@Test
fun `does not crash when ANR profiling is enabled but cache dir is null`() {
fixture.options.isEnableAnrProfiling = true
fixture.options.anrProfilingSampleRate = 1.0
fixture.options.cacheDirPath = null
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint(mechanism = "anr_foreground"))
val original = SentryEvent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1998,28 +1998,28 @@ class ManifestMetadataReaderTest {
}

@Test
fun `applyMetadata reads enableAnrProfiling to options`() {
fun `applyMetadata reads anrProfilingSampleRate to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.ENABLE_ANR_PROFILING to true)
val bundle = bundleOf(ManifestMetadataReader.ANR_PROFILING_SAMPLE_RATE to 0.5f)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.isEnableAnrProfiling)
assertEquals(0.5, fixture.options.anrProfilingSampleRate!!, 0.01)
}

@Test
fun `applyMetadata reads enableAnrProfiling to options and keeps default`() {
fun `applyMetadata keeps anrProfilingSampleRate default when not set in manifest`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.isEnableAnrProfiling)
assertNull(fixture.options.anrProfilingSampleRate)
}

// Network Detail Configuration Tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,25 +196,41 @@ class SentryAndroidOptionsTest {
}

@Test
fun `anr profiling disabled by default`() {
fun `anr profiling sample rate is null by default`() {
val sentryOptions = SentryAndroidOptions()

assertFalse(sentryOptions.isEnableAnrProfiling)
assertNull(sentryOptions.anrProfilingSampleRate)
assertFalse(sentryOptions.isAnrProfilingEnabled)
}

@Test
fun `anr profiling can be enabled`() {
fun `anr profiling can be enabled via sample rate`() {
val sentryOptions = SentryAndroidOptions()
sentryOptions.isEnableAnrProfiling = true
assertTrue(sentryOptions.isEnableAnrProfiling)
sentryOptions.anrProfilingSampleRate = 1.0
assertEquals(1.0, sentryOptions.anrProfilingSampleRate)
assertTrue(sentryOptions.isAnrProfilingEnabled)
}

@Test
fun `anr profiling can be disabled`() {
fun `anr profiling can be disabled via null sample rate`() {
val sentryOptions = SentryAndroidOptions()
sentryOptions.isEnableAnrProfiling = true
sentryOptions.isEnableAnrProfiling = false
assertFalse(sentryOptions.isEnableAnrProfiling)
sentryOptions.anrProfilingSampleRate = 1.0
sentryOptions.anrProfilingSampleRate = null
assertNull(sentryOptions.anrProfilingSampleRate)
assertFalse(sentryOptions.isAnrProfilingEnabled)
}

@Test
fun `anr profiling is disabled when sample rate is zero`() {
val sentryOptions = SentryAndroidOptions()
sentryOptions.anrProfilingSampleRate = 0.0
assertFalse(sentryOptions.isAnrProfilingEnabled)
}

@Test(expected = IllegalArgumentException::class)
fun `anr profiling rejects invalid sample rate`() {
val sentryOptions = SentryAndroidOptions()
sentryOptions.anrProfilingSampleRate = 2.0
}

private class CustomDebugImagesLoader : IDebugImagesLoader {
Expand Down
Loading
Loading