Skip to content

Commit

Permalink
File I/O on main thread (#2382)
Browse files Browse the repository at this point in the history
  • Loading branch information
romtsn committed Dec 15, 2022
1 parent 87598a5 commit 81a3c32
Show file tree
Hide file tree
Showing 44 changed files with 684 additions and 136 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
- Add time-to-initial-display span to Activity transactions ([#2369](https://github.com/getsentry/sentry-java/pull/2369))
- Start a session after init if AutoSessionTracking is enabled ([#2356](https://github.com/getsentry/sentry-java/pull/2356))
- Provide automatic breadcrumbs and transactions for click/scroll events for Compose ([#2390](https://github.com/getsentry/sentry-java/pull/2390))
- Add `blocked_main_thread` and `call_stack` to File I/O spans to detect performance issues ([#2382](https://github.com/getsentry/sentry-java/pull/2382))

### Dependencies

Expand Down
Expand Up @@ -5,7 +5,7 @@
import androidx.core.app.FrameMetricsAggregator;
import io.sentry.MeasurementUnit;
import io.sentry.SentryLevel;
import io.sentry.android.core.internal.util.MainThreadChecker;
import io.sentry.android.core.internal.util.AndroidMainThreadChecker;
import io.sentry.protocol.MeasurementValue;
import io.sentry.protocol.SentryId;
import java.util.HashMap;
Expand Down Expand Up @@ -208,7 +208,7 @@ public synchronized void stop() {

private void runSafelyOnUiThread(final Runnable runnable, final String tag) {
try {
if (MainThreadChecker.isMainThread()) {
if (AndroidMainThreadChecker.getInstance().isMainThread()) {
runnable.run();
} else {
handler.post(
Expand Down
Expand Up @@ -14,6 +14,7 @@
import io.sentry.android.core.cache.AndroidEnvelopeCache;
import io.sentry.android.core.internal.gestures.AndroidViewGestureTargetLocator;
import io.sentry.android.core.internal.modules.AssetsModulesLoader;
import io.sentry.android.core.internal.util.AndroidMainThreadChecker;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import io.sentry.android.fragment.FragmentLifecycleIntegration;
import io.sentry.android.timber.SentryTimberIntegration;
Expand Down Expand Up @@ -154,6 +155,7 @@ static void initializeIntegrationsAndProcessors(
}
options.setGestureTargetLocators(gestureTargetLocators);
}
options.setMainThreadChecker(AndroidMainThreadChecker.getInstance());
}

private static void installDefaultIntegrations(
Expand Down
Expand Up @@ -5,7 +5,7 @@
import io.sentry.Integration;
import io.sentry.SentryLevel;
import io.sentry.SentryOptions;
import io.sentry.android.core.internal.util.MainThreadChecker;
import io.sentry.android.core.internal.util.AndroidMainThreadChecker;
import io.sentry.util.Objects;
import java.io.Closeable;
import java.io.IOException;
Expand Down Expand Up @@ -56,7 +56,7 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio
try {
Class.forName("androidx.lifecycle.DefaultLifecycleObserver");
Class.forName("androidx.lifecycle.ProcessLifecycleOwner");
if (MainThreadChecker.isMainThread()) {
if (AndroidMainThreadChecker.getInstance().isMainThread()) {
addObserver(hub);
} else {
// some versions of the androidx lifecycle-process require this to be executed on the main
Expand Down Expand Up @@ -115,7 +115,7 @@ private void removeObserver() {
@Override
public void close() throws IOException {
if (watcher != null) {
if (MainThreadChecker.isMainThread()) {
if (AndroidMainThreadChecker.getInstance().isMainThread()) {
removeObserver();
} else {
// some versions of the androidx lifecycle-process require this to be executed on the main
Expand Down
Expand Up @@ -26,9 +26,9 @@
import io.sentry.SentryBaseEvent;
import io.sentry.SentryEvent;
import io.sentry.SentryLevel;
import io.sentry.android.core.internal.util.AndroidMainThreadChecker;
import io.sentry.android.core.internal.util.ConnectivityChecker;
import io.sentry.android.core.internal.util.DeviceOrientations;
import io.sentry.android.core.internal.util.MainThreadChecker;
import io.sentry.android.core.internal.util.RootChecker;
import io.sentry.protocol.App;
import io.sentry.protocol.Device;
Expand Down Expand Up @@ -217,7 +217,7 @@ private void setThreads(final @NotNull SentryEvent event) {
if (event.getThreads() != null) {
for (SentryThread thread : event.getThreads()) {
if (thread.isCurrent() == null) {
thread.setCurrent(MainThreadChecker.isMainThread(thread));
thread.setCurrent(AndroidMainThreadChecker.getInstance().isMainThread(thread));
}
}
}
Expand Down
@@ -0,0 +1,23 @@
package io.sentry.android.core.internal.util;

import android.os.Looper;
import io.sentry.util.thread.IMainThreadChecker;
import org.jetbrains.annotations.ApiStatus;

/** Class that checks if a given thread is the Android Main/UI thread */
@ApiStatus.Internal
public final class AndroidMainThreadChecker implements IMainThreadChecker {

private static final AndroidMainThreadChecker instance = new AndroidMainThreadChecker();

public static AndroidMainThreadChecker getInstance() {
return instance;
}

private AndroidMainThreadChecker() {}

@Override
public boolean isMainThread(final long threadId) {
return Looper.getMainLooper().getThread().getId() == threadId;
}
}

This file was deleted.

Expand Up @@ -9,6 +9,7 @@ import io.sentry.MainEventProcessor
import io.sentry.SentryOptions
import io.sentry.android.core.cache.AndroidEnvelopeCache
import io.sentry.android.core.internal.modules.AssetsModulesLoader
import io.sentry.android.core.internal.util.AndroidMainThreadChecker
import io.sentry.android.fragment.FragmentLifecycleIntegration
import io.sentry.android.timber.SentryTimberIntegration
import org.junit.runner.RunWith
Expand Down Expand Up @@ -447,4 +448,11 @@ class AndroidOptionsInitializerTest {

assertTrue { fixture.sentryOptions.modulesLoader is AssetsModulesLoader }
}

@Test
fun `AndroidMainThreadChecker is set to options`() {
fixture.initSut()

assertTrue { fixture.sentryOptions.mainThreadChecker is AndroidMainThreadChecker }
}
}
Expand Up @@ -8,23 +8,23 @@ import kotlin.test.assertFalse
import kotlin.test.assertTrue

@RunWith(AndroidJUnit4::class)
class MainThreadCheckerTest {
class AndroidMainThreadCheckerTest {

@Test
fun `When calling isMainThread from the same thread, it should return true`() {
assertTrue(MainThreadChecker.isMainThread())
assertTrue(AndroidMainThreadChecker.getInstance().isMainThread)
}

@Test
fun `When calling isMainThread with the current thread, it should return true`() {
val thread = Thread.currentThread()
assertTrue(MainThreadChecker.isMainThread(thread))
assertTrue(AndroidMainThreadChecker.getInstance().isMainThread(thread))
}

@Test
fun `When calling isMainThread from a different thread, it should return false`() {
val thread = Thread()
assertFalse(MainThreadChecker.isMainThread(thread))
assertFalse(AndroidMainThreadChecker.getInstance().isMainThread(thread))
}

@Test
Expand All @@ -33,7 +33,7 @@ class MainThreadCheckerTest {
val sentryThread = SentryThread().apply {
id = thread.id
}
assertTrue(MainThreadChecker.isMainThread(sentryThread))
assertTrue(AndroidMainThreadChecker.getInstance().isMainThread(sentryThread))
}

@Test
Expand All @@ -42,6 +42,6 @@ class MainThreadCheckerTest {
val sentryThread = SentryThread().apply {
id = thread.id
}
assertFalse(MainThreadChecker.isMainThread(sentryThread))
assertFalse(AndroidMainThreadChecker.getInstance().isMainThread(sentryThread))
}
}
1 change: 0 additions & 1 deletion sentry-samples/sentry-samples-android/proguard-rules.pro
Expand Up @@ -16,7 +16,6 @@
# https://developer.android.com/studio/build/shrink-code#decode-stack-trace
-keepattributes LineNumberTable,SourceFile


# For native methods, see http://proguard.sourceforge.net/manual/examples.html#native
-keepclasseswithmembernames,includedescriptorclasses class * {
native <methods>;
Expand Down
Expand Up @@ -8,6 +8,7 @@
import io.sentry.MeasurementUnit;
import io.sentry.Sentry;
import io.sentry.UserFeedback;
import io.sentry.instrumentation.file.SentryFileOutputStream;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.samples.android.compose.ComposeActivity;
Expand Down Expand Up @@ -78,7 +79,7 @@ protected void onCreate(Bundle savedInstanceState) {
view -> {
String fileName = Calendar.getInstance().getTimeInMillis() + "_file.txt";
File file = getApplication().getFileStreamPath(fileName);
try (final FileOutputStream fileOutputStream = new FileOutputStream(file);
try (final FileOutputStream fileOutputStream = new SentryFileOutputStream(file);
final OutputStreamWriter outputStreamWriter =
new OutputStreamWriter(fileOutputStream);
final Writer writer = new BufferedWriter(outputStreamWriter)) {
Expand Down
Expand Up @@ -12,6 +12,9 @@ import retrofit2.Response
class ThirdFragment : Fragment(R.layout.third_fragment) {

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
view.findViewById<View>(R.id.third_button).setOnClickListener {
throw RuntimeException("Test")
}
val span = Sentry.getSpan()
val child = span?.startChild("calc")

Expand Down
Expand Up @@ -4,6 +4,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent">
<Button
android:id="@+id/third_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/tap_me"/>
Expand Down
33 changes: 30 additions & 3 deletions sentry/api/sentry.api
Expand Up @@ -1130,6 +1130,7 @@ public abstract class io/sentry/SentryBaseEvent {
public fun addBreadcrumb (Ljava/lang/String;)V
public fun getBreadcrumbs ()Ljava/util/List;
public fun getContexts ()Lio/sentry/protocol/Contexts;
public fun getDebugMeta ()Lio/sentry/protocol/DebugMeta;
public fun getDist ()Ljava/lang/String;
public fun getEnvironment ()Ljava/lang/String;
public fun getEventId ()Lio/sentry/protocol/SentryId;
Expand All @@ -1146,6 +1147,7 @@ public abstract class io/sentry/SentryBaseEvent {
public fun removeExtra (Ljava/lang/String;)V
public fun removeTag (Ljava/lang/String;)V
public fun setBreadcrumbs (Ljava/util/List;)V
public fun setDebugMeta (Lio/sentry/protocol/DebugMeta;)V
public fun setDist (Ljava/lang/String;)V
public fun setEnvironment (Ljava/lang/String;)V
public fun setEventId (Lio/sentry/protocol/SentryId;)V
Expand All @@ -1170,6 +1172,7 @@ public final class io/sentry/SentryBaseEvent$Deserializer {
public final class io/sentry/SentryBaseEvent$JsonKeys {
public static final field BREADCRUMBS Ljava/lang/String;
public static final field CONTEXTS Ljava/lang/String;
public static final field DEBUG_META Ljava/lang/String;
public static final field DIST Ljava/lang/String;
public static final field ENVIRONMENT Ljava/lang/String;
public static final field EVENT_ID Ljava/lang/String;
Expand Down Expand Up @@ -1290,7 +1293,6 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
public fun <init> ()V
public fun <init> (Ljava/lang/Throwable;)V
public fun <init> (Ljava/util/Date;)V
public fun getDebugMeta ()Lio/sentry/protocol/DebugMeta;
public fun getExceptions ()Ljava/util/List;
public fun getFingerprints ()Ljava/util/List;
public fun getLevel ()Lio/sentry/SentryLevel;
Expand All @@ -1305,7 +1307,6 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
public fun isErrored ()Z
public fun removeModule (Ljava/lang/String;)V
public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V
public fun setDebugMeta (Lio/sentry/protocol/DebugMeta;)V
public fun setExceptions (Ljava/util/List;)V
public fun setFingerprints (Ljava/util/List;)V
public fun setLevel (Lio/sentry/SentryLevel;)V
Expand All @@ -1325,7 +1326,6 @@ public final class io/sentry/SentryEvent$Deserializer : io/sentry/JsonDeserializ
}

public final class io/sentry/SentryEvent$JsonKeys {
public static final field DEBUG_META Ljava/lang/String;
public static final field EXCEPTION Ljava/lang/String;
public static final field FINGERPRINT Ljava/lang/String;
public static final field LEVEL Ljava/lang/String;
Expand Down Expand Up @@ -1402,6 +1402,7 @@ public class io/sentry/SentryOptions {
public fun getInstrumenter ()Lio/sentry/Instrumenter;
public fun getIntegrations ()Ljava/util/List;
public fun getLogger ()Lio/sentry/ILogger;
public fun getMainThreadChecker ()Lio/sentry/util/thread/IMainThreadChecker;
public fun getMaxAttachmentSize ()J
public fun getMaxBreadcrumbs ()I
public fun getMaxCacheItems ()I
Expand Down Expand Up @@ -1488,6 +1489,7 @@ public class io/sentry/SentryOptions {
public fun setIdleTimeout (Ljava/lang/Long;)V
public fun setInstrumenter (Lio/sentry/Instrumenter;)V
public fun setLogger (Lio/sentry/ILogger;)V
public fun setMainThreadChecker (Lio/sentry/util/thread/IMainThreadChecker;)V
public fun setMaxAttachmentSize (J)V
public fun setMaxBreadcrumbs (I)V
public fun setMaxCacheItems (I)V
Expand Down Expand Up @@ -1577,6 +1579,12 @@ public final class io/sentry/SentrySpanStorage {
public fun store (Ljava/lang/String;Lio/sentry/ISpan;)V
}

public final class io/sentry/SentryStackTraceFactory {
public fun <init> (Ljava/util/List;Ljava/util/List;)V
public fun getInAppCallStack ()Ljava/util/List;
public fun getStackFrames ([Ljava/lang/StackTraceElement;)Ljava/util/List;
}

public final class io/sentry/SentryTraceHeader {
public static final field SENTRY_TRACE_HEADER Ljava/lang/String;
public fun <init> (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Ljava/lang/Boolean;)V
Expand Down Expand Up @@ -3411,6 +3419,7 @@ public abstract class io/sentry/transport/TransportResult {
}

public final class io/sentry/util/CollectionUtils {
public static fun filterListEntries (Ljava/util/List;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/List;
public static fun filterMapEntries (Ljava/util/Map;Lio/sentry/util/CollectionUtils$Predicate;)Ljava/util/Map;
public static fun map (Ljava/util/List;Lio/sentry/util/CollectionUtils$Mapper;)Ljava/util/List;
public static fun newArrayList (Ljava/util/List;)Ljava/util/List;
Expand Down Expand Up @@ -3523,6 +3532,24 @@ public final class io/sentry/util/StringUtils {
public static fun removeSurrounding (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
}

public abstract interface class io/sentry/util/thread/IMainThreadChecker {
public fun isMainThread ()Z
public abstract fun isMainThread (J)Z
public fun isMainThread (Lio/sentry/protocol/SentryThread;)Z
public fun isMainThread (Ljava/lang/Thread;)Z
}

public final class io/sentry/util/thread/MainThreadChecker : io/sentry/util/thread/IMainThreadChecker {
public static fun getInstance ()Lio/sentry/util/thread/MainThreadChecker;
public fun isMainThread (J)Z
}

public final class io/sentry/util/thread/NoOpMainThreadChecker : io/sentry/util/thread/IMainThreadChecker {
public fun <init> ()V
public static fun getInstance ()Lio/sentry/util/thread/NoOpMainThreadChecker;
public fun isMainThread (J)Z
}

public class io/sentry/vendor/Base64 {
public static final field CRLF I
public static final field DEFAULT I
Expand Down
3 changes: 2 additions & 1 deletion sentry/src/main/java/io/sentry/MainEventProcessor.java
Expand Up @@ -70,7 +70,7 @@ public MainEventProcessor(final @NotNull SentryOptions options) {
return event;
}

private void setDebugMeta(final @NotNull SentryEvent event) {
private void setDebugMeta(final @NotNull SentryBaseEvent event) {
if (options.getProguardUuid() != null) {
DebugMeta debugMeta = event.getDebugMeta();

Expand Down Expand Up @@ -134,6 +134,7 @@ private void processNonCachedEvent(final @NotNull SentryBaseEvent event) {
public @NotNull SentryTransaction process(
final @NotNull SentryTransaction transaction, final @NotNull Hint hint) {
setCommons(transaction);
setDebugMeta(transaction);

if (shouldApplyScopeData(transaction, hint)) {
processNonCachedEvent(transaction);
Expand Down

0 comments on commit 81a3c32

Please sign in to comment.