Skip to content

Commit a59bf08

Browse files
markushimarkushi
committed
Add more tests and address feedback
1 parent 93141dd commit a59bf08

13 files changed

+521
-61
lines changed

‎sentry-android-core/api/sentry-android-core.api‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ public class io/sentry/android/core/anr/AggregatedStackTrace {
491491
public class io/sentry/android/core/anr/AnrCulpritIdentifier {
492492
public fun <init> ()V
493493
public static fun identify (Ljava/util/List;)Lio/sentry/android/core/anr/AggregatedStackTrace;
494+
public static fun isSystemFrame (Ljava/lang/String;)Z
494495
}
495496

496497
public class io/sentry/android/core/anr/AnrException : java/lang/Exception {
@@ -517,7 +518,7 @@ public class io/sentry/android/core/anr/AnrProfileManager : java/io/Closeable {
517518
public class io/sentry/android/core/anr/AnrProfileRotationHelper {
518519
public fun <init> ()V
519520
public static fun deleteLastFile (Ljava/io/File;)Z
520-
public static fun getCurrentFile (Ljava/io/File;)Ljava/io/File;
521+
public static fun getFileForRecording (Ljava/io/File;)Ljava/io/File;
521522
public static fun getLastFile (Ljava/io/File;)Ljava/io/File;
522523
public static fun rotate ()V
523524
}

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,7 @@ static void installDefaultIntegrations(
395395
// it to set the replayId in case of an ANR
396396
options.addIntegration(AnrIntegrationFactory.create(context, buildInfoProvider));
397397

398-
if (options.isEnableAnrProfiling()) {
399-
options.addIntegration(new AnrProfilingIntegration());
400-
}
398+
options.addIntegration(new AnrProfilingIntegration());
401399

402400
// registerActivityLifecycleCallbacks is only available if Context is an AppContext
403401
if (context instanceof Application) {

‎sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java‎

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@
3737
import io.sentry.protocol.DebugImage;
3838
import io.sentry.protocol.DebugMeta;
3939
import io.sentry.protocol.Message;
40+
import io.sentry.protocol.SentryException;
4041
import io.sentry.protocol.SentryId;
42+
import io.sentry.protocol.SentryStackFrame;
43+
import io.sentry.protocol.SentryStackTrace;
4144
import io.sentry.protocol.SentryThread;
4245
import io.sentry.protocol.profiling.SentryProfile;
4346
import io.sentry.transport.CurrentDateProvider;
@@ -53,6 +56,7 @@
5356
import java.io.InputStream;
5457
import java.io.InputStreamReader;
5558
import java.util.ArrayList;
59+
import java.util.Arrays;
5660
import java.util.Collections;
5761
import java.util.HashMap;
5862
import java.util.List;
@@ -298,7 +302,19 @@ private void reportAsSentryEvent(
298302
}
299303
}
300304

301-
applyAnrProfile(isBackground, anrTimestamp, event);
305+
if (options.isEnableAnrProfiling()) {
306+
applyAnrProfile(isBackground, anrTimestamp, event);
307+
// TODO: maybe move to AnrV2EventProcessor instead
308+
if (hasOnlySystemFrames(event)) {
309+
// By omitting the {{ default }} fingerprint, the stacktrace will be completely ignored
310+
// and all events will be grouped
311+
// into the same issue
312+
event.setFingerprints(
313+
Arrays.asList(
314+
"{{ system-frames-only-anr }}",
315+
isBackground ? "background-anr" : "foreground-anr"));
316+
}
317+
}
302318

303319
final @NotNull SentryId sentryId = scopes.captureEvent(event, hint);
304320
final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID);
@@ -323,14 +339,19 @@ private void applyAnrProfile(
323339
return;
324340
}
325341

342+
final @Nullable String cacheDirPath = options.getCacheDirPath();
343+
if (cacheDirPath == null) {
344+
return;
345+
}
346+
final @NotNull File cacheDir = new File(cacheDirPath);
347+
326348
@Nullable AnrProfile anrProfile = null;
327-
final File cacheDir = new File(options.getCacheDirPath());
328349

329350
try {
330351
final File lastFile = AnrProfileRotationHelper.getLastFile(cacheDir);
331352

332353
if (lastFile.exists()) {
333-
options.getLogger().log(SentryLevel.DEBUG, "Reading ANR profile from rotated file");
354+
options.getLogger().log(SentryLevel.DEBUG, "Reading ANR profile");
334355
try (final AnrProfileManager provider = new AnrProfileManager(options, lastFile)) {
335356
anrProfile = provider.load();
336357
}
@@ -340,32 +361,19 @@ private void applyAnrProfile(
340361
} catch (Throwable t) {
341362
options.getLogger().log(SentryLevel.INFO, "Could not retrieve ANR profile", t);
342363
} finally {
343-
if (AnrProfileRotationHelper.deleteLastFile(cacheDir)) {
344-
options.getLogger().log(SentryLevel.DEBUG, "Deleted old ANR profile file");
364+
if (!AnrProfileRotationHelper.deleteLastFile(cacheDir)) {
365+
options.getLogger().log(SentryLevel.INFO, "Could not delete ANR profile file");
345366
}
346367
}
347368

348369
if (anrProfile != null) {
349370
options.getLogger().log(SentryLevel.INFO, "ANR profile found");
350371
if (anrTimestamp >= anrProfile.startTimeMs && anrTimestamp <= anrProfile.endtimeMs) {
351-
final SentryProfile profile = StackTraceConverter.convert(anrProfile);
352-
final ProfileChunk chunk =
353-
new ProfileChunk(
354-
new SentryId(),
355-
new SentryId(),
356-
null,
357-
new HashMap<>(0),
358-
anrTimestamp / 1000.0d,
359-
ProfileChunk.PLATFORM_JAVA,
360-
options);
361-
chunk.setSentryProfile(profile);
362-
scopes.captureProfileChunk(chunk);
363-
364372
final @Nullable AggregatedStackTrace culprit =
365373
AnrCulpritIdentifier.identify(anrProfile.stacks);
366374
if (culprit != null) {
367-
// TODO Consider setting a static fingerprint to reduce noise
368-
// if culprit quality is low (e.g. when culprit frame is pollNative())
375+
final @Nullable SentryId profilerId = captureAnrProfile(anrTimestamp, anrProfile);
376+
369377
final @NotNull StackTraceElement[] stack = culprit.getStack();
370378
if (stack.length > 0) {
371379
final StackTraceElement stackTraceElement = culprit.getStack()[0];
@@ -378,7 +386,9 @@ private void applyAnrProfile(
378386
final SentryExceptionFactory factory =
379387
new SentryExceptionFactory(new SentryStackTraceFactory(options));
380388
event.setExceptions(factory.getSentryExceptions(exception));
381-
event.getContexts().setProfile(new ProfileContext(chunk.getProfilerId()));
389+
if (profilerId != null) {
390+
event.getContexts().setProfile(new ProfileContext(profilerId));
391+
}
382392
}
383393
}
384394
} else {
@@ -387,6 +397,28 @@ private void applyAnrProfile(
387397
}
388398
}
389399

400+
@Nullable
401+
private SentryId captureAnrProfile(
402+
final long anrTimestamp, final @NotNull AnrProfile anrProfile) {
403+
final SentryProfile profile = StackTraceConverter.convert(anrProfile);
404+
final ProfileChunk chunk =
405+
new ProfileChunk(
406+
new SentryId(),
407+
new SentryId(),
408+
null,
409+
new HashMap<>(0),
410+
anrTimestamp / 1000.0d,
411+
ProfileChunk.PLATFORM_JAVA,
412+
options);
413+
chunk.setSentryProfile(profile);
414+
final SentryId profilerId = scopes.captureProfileChunk(chunk);
415+
if (profilerId.equals(SentryId.EMPTY_ID)) {
416+
return null;
417+
} else {
418+
return chunk.getProfilerId();
419+
}
420+
}
421+
390422
private @NotNull ParseResult parseThreadDump(
391423
final @NotNull ApplicationExitInfo exitInfo, final boolean isBackground) {
392424
final byte[] dump;
@@ -440,6 +472,30 @@ private byte[] getDumpBytes(final @NotNull InputStream trace) throws IOException
440472
}
441473
}
442474

475+
private static boolean hasOnlySystemFrames(final @NotNull SentryEvent event) {
476+
final List<SentryException> exceptions = event.getExceptions();
477+
if (exceptions != null) {
478+
for (final SentryException exception : exceptions) {
479+
final @Nullable SentryStackTrace stacktrace = exception.getStacktrace();
480+
if (stacktrace != null) {
481+
final @Nullable List<SentryStackFrame> frames = stacktrace.getFrames();
482+
if (frames != null && !frames.isEmpty()) {
483+
for (final SentryStackFrame frame : frames) {
484+
if (frame.isInApp() != null && frame.isInApp()) {
485+
return false;
486+
}
487+
final @Nullable String module = frame.getModule();
488+
if (module != null && !AnrCulpritIdentifier.isSystemFrame(frame.getModule())) {
489+
return false;
490+
}
491+
}
492+
}
493+
}
494+
}
495+
}
496+
return true;
497+
}
498+
443499
@ApiStatus.Internal
444500
public static final class AnrV2Hint extends BlockingFlushHint
445501
implements Backfillable, AbnormalExit {

‎sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrCulpritIdentifier.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,18 @@
1313
public class AnrCulpritIdentifier {
1414

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

1818
static {
19-
systemAndFrameWorkPackages.add("java.lang");
20-
systemAndFrameWorkPackages.add("java.util");
21-
systemAndFrameWorkPackages.add("android.app");
22-
systemAndFrameWorkPackages.add("android.os.Handler");
23-
systemAndFrameWorkPackages.add("android.os.Looper");
24-
systemAndFrameWorkPackages.add("android.view");
25-
systemAndFrameWorkPackages.add("android.widget");
26-
systemAndFrameWorkPackages.add("com.android.internal");
27-
systemAndFrameWorkPackages.add("com.google.android");
19+
systemAndFrameworkPackages.add("java.lang");
20+
systemAndFrameworkPackages.add("java.util");
21+
systemAndFrameworkPackages.add("android.app");
22+
systemAndFrameworkPackages.add("android.os.Handler");
23+
systemAndFrameworkPackages.add("android.os.Looper");
24+
systemAndFrameworkPackages.add("android.view");
25+
systemAndFrameworkPackages.add("android.widget");
26+
systemAndFrameworkPackages.add("com.android.internal");
27+
systemAndFrameworkPackages.add("com.google.android");
2828
}
2929

3030
private static final class StackTraceKey {
@@ -148,8 +148,8 @@ public static AggregatedStackTrace identify(final @NotNull List<AnrStackTrace> s
148148
Float.compare(c1.count * c1.quality * c1.depth, c2.count * c2.quality * c2.depth));
149149
}
150150

151-
private static boolean isSystemFrame(final @NotNull String clazz) {
152-
for (final String systemPackage : systemAndFrameWorkPackages) {
151+
public static boolean isSystemFrame(final @NotNull String clazz) {
152+
for (final String systemPackage : systemAndFrameworkPackages) {
153153
if (clazz.startsWith(systemPackage)) {
154154
return true;
155155
}

‎sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfileRotationHelper.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
@ApiStatus.Internal
1313
public class AnrProfileRotationHelper {
1414

15-
private static final String CURRENT_FILE_NAME = "anr_profile";
15+
private static final String RECORDING_FILE_NAME = "anr_profile";
1616
private static final String OLD_FILE_NAME = "anr_profile_old";
1717

18-
private static final AtomicBoolean shouldRotate = new AtomicBoolean(false);
18+
private static final AtomicBoolean shouldRotate = new AtomicBoolean(true);
1919
private static final Object rotationLock = new Object();
2020

2121
public static void rotate() {
@@ -32,7 +32,7 @@ private static void performRotationIfNeeded(final @NotNull File cacheDir) {
3232
return;
3333
}
3434

35-
final File currentFile = new File(cacheDir, CURRENT_FILE_NAME);
35+
final File currentFile = new File(cacheDir, RECORDING_FILE_NAME);
3636
final File oldFile = new File(cacheDir, OLD_FILE_NAME);
3737

3838
if (oldFile.exists()) {
@@ -48,9 +48,9 @@ private static void performRotationIfNeeded(final @NotNull File cacheDir) {
4848
}
4949

5050
@NotNull
51-
public static File getCurrentFile(final @NotNull File cacheDir) {
51+
public static File getFileForRecording(final @NotNull File cacheDir) {
5252
performRotationIfNeeded(cacheDir);
53-
return new File(cacheDir, CURRENT_FILE_NAME);
53+
return new File(cacheDir, RECORDING_FILE_NAME);
5454
}
5555

5656
@NotNull
@@ -61,6 +61,9 @@ public static File getLastFile(final @NotNull File cacheDir) {
6161

6262
public static boolean deleteLastFile(final @NotNull File cacheDir) {
6363
final File oldFile = new File(cacheDir, OLD_FILE_NAME);
64-
return oldFile.exists() && oldFile.delete();
64+
if (!oldFile.exists()) {
65+
return true;
66+
}
67+
return oldFile.delete();
6568
}
6669
}

‎sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package io.sentry.android.core.anr;
22

3+
import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion;
4+
35
import android.os.Handler;
46
import android.os.Looper;
57
import android.os.SystemClock;
6-
import androidx.annotation.NonNull;
78
import io.sentry.ILogger;
89
import io.sentry.IScopes;
910
import io.sentry.ISentryLifecycleToken;
@@ -12,6 +13,7 @@
1213
import io.sentry.SentryLevel;
1314
import io.sentry.SentryOptions;
1415
import io.sentry.android.core.AppState;
16+
import io.sentry.android.core.SentryAndroidOptions;
1517
import io.sentry.util.AutoClosableReentrantLock;
1618
import io.sentry.util.Objects;
1719
import java.io.Closeable;
@@ -40,15 +42,26 @@ public class AnrProfilingIntegration
4042
private volatile MainThreadState mainThreadState = MainThreadState.IDLE;
4143
private volatile @Nullable AnrProfileManager profileManager;
4244
private volatile @NotNull ILogger logger = NoOpLogger.getInstance();
43-
private volatile @Nullable SentryOptions options;
45+
private volatile @Nullable SentryAndroidOptions options;
4446
private volatile @Nullable Thread thread = null;
4547
private volatile boolean inForeground = false;
4648

4749
@Override
48-
public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) {
49-
this.options = options;
50-
logger = options.getLogger();
51-
AppState.getInstance().addAppStateListener(this);
50+
public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions options) {
51+
this.options =
52+
Objects.requireNonNull(
53+
(options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null,
54+
"SentryAndroidOptions is required");
55+
this.logger = options.getLogger();
56+
57+
if (this.options == null) {
58+
return;
59+
}
60+
61+
if (((SentryAndroidOptions) options).isEnableAnrProfiling()) {
62+
addIntegrationToSdkVersion("AnrProfiling");
63+
AppState.getInstance().addAppStateListener(this);
64+
}
5265
}
5366

5467
@Override
@@ -81,9 +94,9 @@ public void onForeground() {
8194
oldThread.interrupt();
8295
}
8396

84-
final @NotNull Thread newThread = new Thread(this, "AnrProfilingIntegration");
85-
newThread.start();
86-
thread = newThread;
97+
final @NotNull Thread profilingThread = new Thread(this, "AnrProfilingIntegration");
98+
profilingThread.start();
99+
thread = profilingThread;
87100
}
88101
}
89102

@@ -177,14 +190,14 @@ protected MainThreadState getState() {
177190
}
178191

179192
@TestOnly
180-
@NonNull
193+
@NotNull
181194
protected AnrProfileManager getProfileManager() {
182195
try (final @NotNull ISentryLifecycleToken ignored = profileManagerLock.acquire()) {
183196
if (profileManager == null) {
184197
final @NotNull SentryOptions opts =
185198
Objects.requireNonNull(options, "Options can't be null");
186199
final @NotNull File currentFile =
187-
AnrProfileRotationHelper.getCurrentFile(new File(opts.getCacheDirPath()));
200+
AnrProfileRotationHelper.getFileForRecording(new File(opts.getCacheDirPath()));
188201
profileManager = new AnrProfileManager(opts, currentFile);
189202
}
190203

‎sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrStackTrace.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ public int compareTo(final @NotNull AnrStackTrace o) {
2626
}
2727

2828
public void serialize(final @NotNull DataOutputStream dos) throws IOException {
29-
dos.writeShort(1);
29+
dos.writeShort(1); // version
3030
dos.writeLong(timestampMs);
3131
dos.writeInt(stack.length);
3232
for (final @NotNull StackTraceElement element : stack) {
3333
dos.writeUTF(StringUtils.getOrEmpty(element.getClassName()));
3434
dos.writeUTF(StringUtils.getOrEmpty(element.getMethodName()));
35-
dos.writeUTF(StringUtils.getOrEmpty(element.getFileName()));
35+
// Write null as a special marker to preserve null vs empty string distinction
36+
final @Nullable String fileName = element.getFileName();
37+
dos.writeBoolean(fileName == null);
38+
dos.writeUTF(fileName == null ? "" : fileName);
3639
dos.writeInt(element.getLineNumber());
3740
}
3841
}
@@ -49,7 +52,10 @@ public static AnrStackTrace deserialize(final @NotNull DataInputStream dis) thro
4952
for (int i = 0; i < stackLength; i++) {
5053
final @NotNull String className = dis.readUTF();
5154
final @NotNull String methodName = dis.readUTF();
52-
final @Nullable String fileName = dis.readUTF();
55+
// Read the null marker to restore null vs empty string distinction
56+
final boolean isFileNameNull = dis.readBoolean();
57+
final @NotNull String fileNameStr = dis.readUTF();
58+
final @Nullable String fileName = isFileNameNull ? null : fileNameStr;
5359
final int lineNumber = dis.readInt();
5460
final StackTraceElement element =
5561
new StackTraceElement(className, methodName, fileName, lineNumber);

0 commit comments

Comments
 (0)