Skip to content
Draft
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Parse memory and GC info from ANR thread dumps and enrich Device context ([#5428](https://github.com/getsentry/sentry-java/pull/5428))
- Add support to configure reporting historical ANRs via `AndroidManifest.xml` using the `io.sentry.anr.report-historical` attribute ([#5387](https://github.com/getsentry/sentry-java/pull/5387))

## 8.41.0
Expand Down
2 changes: 2 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ public class io/sentry/android/core/AnrV2Integration : io/sentry/Integration, ja

public final class io/sentry/android/core/AnrV2Integration$AnrV2Hint : io/sentry/hints/BlockingFlushHint, io/sentry/hints/AbnormalExit, io/sentry/hints/Backfillable {
public fun <init> (JLio/sentry/ILogger;JZZ)V
public fun <init> (JLio/sentry/ILogger;JZZLio/sentry/android/core/internal/threaddump/ThreadDumpMemoryInfo;)V
public fun getThreadDumpMemoryInfo ()Lio/sentry/android/core/internal/threaddump/ThreadDumpMemoryInfo;
public fun ignoreCurrentThread ()Z
public fun isFlushable (Lio/sentry/protocol/SentryId;)Z
public fun mechanism ()Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.sentry.SentryOptions;
import io.sentry.android.core.cache.AndroidEnvelopeCache;
import io.sentry.android.core.internal.threaddump.Lines;
import io.sentry.android.core.internal.threaddump.ThreadDumpMemoryInfo;
import io.sentry.android.core.internal.threaddump.ThreadDumpParser;
import io.sentry.hints.AbnormalExit;
import io.sentry.hints.Backfillable;
Expand Down Expand Up @@ -154,7 +155,8 @@ public boolean shouldReportHistorical() {
options.getLogger(),
anrTimestamp,
shouldEnrich,
isBackground);
isBackground,
result.memoryInfo);

final Hint hint = HintUtils.createWithTypeCheckHint(anrHint);

Expand Down Expand Up @@ -209,6 +211,7 @@ public boolean shouldReportHistorical() {

final @NotNull List<SentryThread> threads = threadDumpParser.getThreads();
final @NotNull List<DebugImage> debugImages = threadDumpParser.getDebugImages();
final @Nullable ThreadDumpMemoryInfo memoryInfo = threadDumpParser.getMemoryInfo();

if (threads.isEmpty()) {
// if the list is empty this means the system failed to capture a proper thread dump of
Expand All @@ -217,7 +220,7 @@ public boolean shouldReportHistorical() {
// fall back to not reporting them
return new ParseResult(ParseResult.Type.NO_DUMP);
}
return new ParseResult(ParseResult.Type.DUMP, dump, threads, debugImages);
return new ParseResult(ParseResult.Type.DUMP, dump, threads, debugImages, memoryInfo);
} catch (Throwable e) {
options.getLogger().log(SentryLevel.WARNING, "Failed to parse ANR thread dump", e);
return new ParseResult(ParseResult.Type.ERROR, dump);
Expand Down Expand Up @@ -249,16 +252,33 @@ public static final class AnrV2Hint extends BlockingFlushHint

private final boolean isBackgroundAnr;

private final @Nullable ThreadDumpMemoryInfo threadDumpMemoryInfo;

public AnrV2Hint(
final long flushTimeoutMillis,
final @NotNull ILogger logger,
final long timestamp,
final boolean shouldEnrich,
final boolean isBackgroundAnr) {
this(flushTimeoutMillis, logger, timestamp, shouldEnrich, isBackgroundAnr, null);
}

public AnrV2Hint(
final long flushTimeoutMillis,
final @NotNull ILogger logger,
final long timestamp,
final boolean shouldEnrich,
final boolean isBackgroundAnr,
final @Nullable ThreadDumpMemoryInfo threadDumpMemoryInfo) {
super(flushTimeoutMillis, logger);
this.timestamp = timestamp;
this.shouldEnrich = shouldEnrich;
this.isBackgroundAnr = isBackgroundAnr;
this.threadDumpMemoryInfo = threadDumpMemoryInfo;
}

public @Nullable ThreadDumpMemoryInfo getThreadDumpMemoryInfo() {
return threadDumpMemoryInfo;
}

@Override
Expand Down Expand Up @@ -303,30 +323,35 @@ enum Type {
final byte[] dump;
final @Nullable List<SentryThread> threads;
final @Nullable List<DebugImage> debugImages;
final @Nullable ThreadDumpMemoryInfo memoryInfo;

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

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

ParseResult(
final @NotNull Type type,
final byte[] dump,
final @Nullable List<SentryThread> threads,
final @Nullable List<DebugImage> debugImages) {
final @Nullable List<DebugImage> debugImages,
final @Nullable ThreadDumpMemoryInfo memoryInfo) {
this.type = type;
this.dump = dump;
this.threads = threads;
this.debugImages = debugImages;
this.memoryInfo = memoryInfo;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
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.threaddump.ThreadDumpMemoryInfo;
import io.sentry.android.core.internal.util.CpuInfoUtils;
import io.sentry.cache.PersistingOptionsObserver;
import io.sentry.cache.PersistingScopeObserver;
Expand Down Expand Up @@ -723,6 +724,36 @@ public void applyPostEnrichment(

// Set app foreground state
setAppForeground(event, !isBackgroundAnr);

enrichDeviceFromThreadDump(event, rawHint);
}

private void enrichDeviceFromThreadDump(
final @NotNull SentryEvent event, final @NotNull Object rawHint) {
if (!(rawHint instanceof AnrV2Integration.AnrV2Hint)) {
return;
}
final @Nullable ThreadDumpMemoryInfo memoryInfo =
((AnrV2Integration.AnrV2Hint) rawHint).getThreadDumpMemoryInfo();
if (memoryInfo == null) {
return;
}

Device device = event.getContexts().getDevice();
if (device == null) {
device = new Device();
event.getContexts().setDevice(device);
}

if (memoryInfo.getFreeMemoryUntilOOMEBytes() != null) {
device.setFreeMemory(memoryInfo.getFreeMemoryUntilOOMEBytes());
}
if (memoryInfo.getFreeMemoryBytes() != null) {
device.setUsableMemory(memoryInfo.getFreeMemoryBytes());
}
if (memoryInfo.getMaxMemoryBytes() != null) {
device.setMemorySize(memoryInfo.getMaxMemoryBytes());
}
Comment on lines +754 to +756
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The device's total physical RAM (device.memorySize) is incorrectly overwritten with the app's maximum heap size from the ANR thread dump, leading to inaccurate memory reporting.
Severity: MEDIUM

Suggested Fix

Remove the logic that overwrites the device.memorySize. The value from maxMemoryBytes parsed from the thread dump should not replace the device's total physical RAM. Consider storing the JVM heap max in a separate field or only setting the memory size if it hasn't been set already from ActivityManager.MemoryInfo.

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#L754-L756

Potential issue: In Sentry ANR events, the `device.memorySize` field, which should
represent the device's total physical RAM, is being incorrectly overwritten. Initially,
it is set correctly using `ActivityManager.MemoryInfo.totalMem`. However, the
`applyPostEnrichment` method later overwrites this value with the JVM maximum heap size
(`-Xmx`) parsed from the ART thread dump's "Max memory" line. This causes the reported
device memory to be drastically smaller than the actual physical RAM (e.g., reporting
192MB instead of 4GB), leading to significantly inaccurate device data associated with
ANR events.

Did we get this right? ๐Ÿ‘ / ๐Ÿ‘Ž to inform future reviews.

}

private void setDefaultAnrFingerprint(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package io.sentry.android.core.internal.threaddump;

import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.Nullable;

/**
* Holds memory and GC metrics parsed from an ANRv2 thread dump.
*
* <p>Memory sizes are in bytes. GC times are in milliseconds.
*/
@ApiStatus.Internal
public final class ThreadDumpMemoryInfo {

private @Nullable Long freeMemoryBytes;
private @Nullable Long freeMemoryUntilGcBytes;
private @Nullable Long freeMemoryUntilOOMEBytes;
private @Nullable Long totalMemoryBytes;
private @Nullable Long maxMemoryBytes;

private @Nullable Long totalGcCount;
private @Nullable Double totalGcTimeMs;
private @Nullable Long totalBlockingGcCount;
private @Nullable Double totalBlockingGcTimeMs;
private @Nullable Long totalPreOomeGcCount;
private @Nullable Double totalTimeWaitingForGcMs;

public @Nullable Long getFreeMemoryBytes() {
return freeMemoryBytes;
}

public void setFreeMemoryBytes(final @Nullable Long freeMemoryBytes) {
this.freeMemoryBytes = freeMemoryBytes;
}

public @Nullable Long getFreeMemoryUntilGcBytes() {
return freeMemoryUntilGcBytes;
}

public void setFreeMemoryUntilGcBytes(final @Nullable Long freeMemoryUntilGcBytes) {
this.freeMemoryUntilGcBytes = freeMemoryUntilGcBytes;
}

public @Nullable Long getFreeMemoryUntilOOMEBytes() {
return freeMemoryUntilOOMEBytes;
}

public void setFreeMemoryUntilOOMEBytes(final @Nullable Long freeMemoryUntilOOMEBytes) {
this.freeMemoryUntilOOMEBytes = freeMemoryUntilOOMEBytes;
}

public @Nullable Long getTotalMemoryBytes() {
return totalMemoryBytes;
}

public void setTotalMemoryBytes(final @Nullable Long totalMemoryBytes) {
this.totalMemoryBytes = totalMemoryBytes;
}

public @Nullable Long getMaxMemoryBytes() {
return maxMemoryBytes;
}

public void setMaxMemoryBytes(final @Nullable Long maxMemoryBytes) {
this.maxMemoryBytes = maxMemoryBytes;
}

public @Nullable Long getTotalGcCount() {
return totalGcCount;
}

public void setTotalGcCount(final @Nullable Long totalGcCount) {
this.totalGcCount = totalGcCount;
}

public @Nullable Double getTotalGcTimeMs() {
return totalGcTimeMs;
}

public void setTotalGcTimeMs(final @Nullable Double totalGcTimeMs) {
this.totalGcTimeMs = totalGcTimeMs;
}

public @Nullable Long getTotalBlockingGcCount() {
return totalBlockingGcCount;
}

public void setTotalBlockingGcCount(final @Nullable Long totalBlockingGcCount) {
this.totalBlockingGcCount = totalBlockingGcCount;
}

public @Nullable Double getTotalBlockingGcTimeMs() {
return totalBlockingGcTimeMs;
}

public void setTotalBlockingGcTimeMs(final @Nullable Double totalBlockingGcTimeMs) {
this.totalBlockingGcTimeMs = totalBlockingGcTimeMs;
}

public @Nullable Long getTotalPreOomeGcCount() {
return totalPreOomeGcCount;
}

public void setTotalPreOomeGcCount(final @Nullable Long totalPreOomeGcCount) {
this.totalPreOomeGcCount = totalPreOomeGcCount;
}

public @Nullable Double getTotalTimeWaitingForGcMs() {
return totalTimeWaitingForGcMs;
}

public void setTotalTimeWaitingForGcMs(final @Nullable Double totalTimeWaitingForGcMs) {
this.totalTimeWaitingForGcMs = totalTimeWaitingForGcMs;
}
}
Loading
Loading