Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File I/O on main thread #2382

Merged
merged 15 commits into from Dec 15, 2022
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