diff --git a/CHANGELOG.md b/CHANGELOG.md index df0f8aaf49..c617242fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) - Remove unused method in ManifestMetadataReader ([#4585](https://github.com/getsentry/sentry-java/pull/4585)) - Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562)) +- Do not register for SystemEvents and NetworkCallbacks immediately when launched with non-foreground importance ([#4579](https://github.com/getsentry/sentry-java/pull/4579)) - Limit ProGuard keep rules for native methods within `sentry-android-ndk` to the `io.sentry.**` namespace. ([#4427](https://github.com/getsentry/sentry-java/pull/4427)) - If you relied on the Sentry SDK to keep native method names for JNI compatibility within your namespace, please review your ProGuard rules and ensure the configuration still works. Especially when you're not consuming any of the default Android proguard rules (`proguard-android.txt` or `proguard-android-optimize.txt`) the following config should be present: ``` diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 044a1fded2..6bcb961b32 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -167,9 +167,15 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In } public final class io/sentry/android/core/AppState : java/io/Closeable { + public fun addAppStateListener (Lio/sentry/android/core/AppState$AppStateListener;)V public fun close ()V public static fun getInstance ()Lio/sentry/android/core/AppState; + public fun getLifecycleObserver ()Lio/sentry/android/core/AppState$LifecycleObserver; public fun isInBackground ()Ljava/lang/Boolean; + public fun registerLifecycleObserver (Lio/sentry/SentryOptions;)V + public fun removeAppStateListener (Lio/sentry/android/core/AppState$AppStateListener;)V + public fun resetInstance ()V + public fun unregisterLifecycleObserver ()V } public abstract interface class io/sentry/android/core/AppState$AppStateListener { @@ -177,6 +183,13 @@ public abstract interface class io/sentry/android/core/AppState$AppStateListener public abstract fun onForeground ()V } +public final class io/sentry/android/core/AppState$LifecycleObserver : androidx/lifecycle/DefaultLifecycleObserver { + public fun (Lio/sentry/android/core/AppState;)V + public fun getListeners ()Ljava/util/List; + public fun onStart (Landroidx/lifecycle/LifecycleOwner;)V + public fun onStop (Landroidx/lifecycle/LifecycleOwner;)V +} + public final class io/sentry/android/core/BuildConfig { public static final field BUILD_TYPE Ljava/lang/String; public static final field DEBUG Z diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index 8fc1c8ab0b..74522f7aaa 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -2,13 +2,13 @@ import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.LifecycleObserver; import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; import io.sentry.NoOpLogger; import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; import java.io.Closeable; @@ -36,21 +36,23 @@ private AppState() {} private volatile @Nullable Boolean inBackground = null; - @TestOnly - LifecycleObserver getLifecycleObserver() { - return lifecycleObserver; - } - @TestOnly void setHandler(final @NotNull MainLooperHandler handler) { this.handler = handler; } + @ApiStatus.Internal @TestOnly - void resetInstance() { + public void resetInstance() { instance = new AppState(); } + @ApiStatus.Internal + @TestOnly + public LifecycleObserver getLifecycleObserver() { + return lifecycleObserver; + } + public @Nullable Boolean isInBackground() { return inBackground; } @@ -59,7 +61,7 @@ void setInBackground(final boolean inBackground) { this.inBackground = inBackground; } - void addAppStateListener(final @NotNull AppStateListener listener) { + public void addAppStateListener(final @NotNull AppStateListener listener) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { ensureLifecycleObserver(NoOpLogger.getInstance()); @@ -69,7 +71,7 @@ void addAppStateListener(final @NotNull AppStateListener listener) { } } - void removeAppStateListener(final @NotNull AppStateListener listener) { + public void removeAppStateListener(final @NotNull AppStateListener listener) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { if (lifecycleObserver != null) { lifecycleObserver.listeners.remove(listener); @@ -77,7 +79,8 @@ void removeAppStateListener(final @NotNull AppStateListener listener) { } } - void registerLifecycleObserver(final @Nullable SentryAndroidOptions options) { + @ApiStatus.Internal + public void registerLifecycleObserver(final @Nullable SentryOptions options) { if (lifecycleObserver != null) { return; } @@ -133,7 +136,8 @@ private void addObserverInternal(final @NotNull ILogger logger) { } } - void unregisterLifecycleObserver() { + @ApiStatus.Internal + public void unregisterLifecycleObserver() { if (lifecycleObserver == null) { return; } @@ -167,7 +171,8 @@ public void close() throws IOException { unregisterLifecycleObserver(); } - final class LifecycleObserver implements DefaultLifecycleObserver { + @ApiStatus.Internal + public final class LifecycleObserver implements DefaultLifecycleObserver { final List listeners = new CopyOnWriteArrayList() { @Override @@ -184,6 +189,12 @@ public boolean add(AppStateListener appStateListener) { } }; + @ApiStatus.Internal + @TestOnly + public List getListeners() { + return listeners; + } + @Override public void onStart(@NonNull LifecycleOwner owner) { setInBackground(false); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index 0f1c63ef27..8ba0e16b7c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -21,7 +21,6 @@ import io.sentry.SentryOptions; import io.sentry.android.core.util.AndroidLazyEvaluator; import io.sentry.protocol.App; -import io.sentry.util.LazyEvaluator; import java.io.BufferedReader; import java.io.File; import java.io.FileReader; @@ -91,20 +90,6 @@ private ContextUtils() {} // to avoid doing a bunch of Binder calls we use LazyEvaluator to cache the values that are static // during the app process running - private static final @NotNull LazyEvaluator isForegroundImportance = - new LazyEvaluator<>( - () -> { - try { - final ActivityManager.RunningAppProcessInfo appProcessInfo = - new ActivityManager.RunningAppProcessInfo(); - ActivityManager.getMyMemoryState(appProcessInfo); - return appProcessInfo.importance == IMPORTANCE_FOREGROUND; - } catch (Throwable ignored) { - // should never happen - } - return false; - }); - /** * Since this packageInfo uses flags 0 we can assume it's static and cache it as the package name * or version code cannot change during runtime, only after app update (which will spin up a new @@ -284,7 +269,15 @@ static String getVersionName(final @NotNull PackageInfo packageInfo) { */ @ApiStatus.Internal public static boolean isForegroundImportance() { - return isForegroundImportance.getValue(); + try { + final ActivityManager.RunningAppProcessInfo appProcessInfo = + new ActivityManager.RunningAppProcessInfo(); + ActivityManager.getMyMemoryState(appProcessInfo); + return appProcessInfo.importance == IMPORTANCE_FOREGROUND; + } catch (Throwable ignored) { + // should never happen + } + return false; } /** @@ -544,7 +537,6 @@ public static Context getApplicationContext(final @NotNull Context context) { @TestOnly static void resetInstance() { - isForegroundImportance.resetValue(); staticPackageInfo33.resetValue(); staticPackageInfo.resetValue(); applicationName.resetValue(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index f8950ee762..b08b429fdf 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -43,6 +43,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -62,6 +63,7 @@ public final class SystemEventsBreadcrumbsIntegration private volatile boolean isClosed = false; private volatile boolean isStopped = false; private volatile IntentFilter filter = null; + private final @NotNull AtomicBoolean isReceiverRegistered = new AtomicBoolean(false); private final @NotNull AutoClosableReentrantLock receiverLock = new AutoClosableReentrantLock(); // Track previous battery state to avoid duplicate breadcrumbs when values haven't changed private @Nullable BatteryState previousBatteryState; @@ -101,14 +103,15 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions if (this.options.isEnableSystemEventBreadcrumbs()) { AppState.getInstance().addAppStateListener(this); - registerReceiver(this.scopes, this.options, /* reportAsNewIntegration= */ true); + + if (ContextUtils.isForegroundImportance()) { + registerReceiver(this.scopes, this.options); + } } } private void registerReceiver( - final @NotNull IScopes scopes, - final @NotNull SentryAndroidOptions options, - final boolean reportAsNewIntegration) { + final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) { if (!options.isEnableSystemEventBreadcrumbs()) { return; @@ -139,7 +142,7 @@ private void registerReceiver( // registerReceiver can throw SecurityException but it's not documented in the // official docs ContextUtils.registerReceiver(context, options, receiver, filter); - if (reportAsNewIntegration) { + if (!isReceiverRegistered.getAndSet(true)) { options .getLogger() .log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); @@ -239,7 +242,7 @@ public void onForeground() { isStopped = false; - registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); + registerReceiver(scopes, options); } @Override diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 16ccc0a2ee..4e657c9c64 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -15,12 +15,14 @@ import io.sentry.ISentryLifecycleToken; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.android.core.AppState; import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -32,7 +34,8 @@ * details */ @ApiStatus.Internal -public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider { +public final class AndroidConnectionStatusProvider + implements IConnectionStatusProvider, AppState.AppStateListener { private final @NotNull Context context; private final @NotNull SentryOptions options; @@ -59,11 +62,11 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP private static final int[] capabilities = new int[2]; - private final @NotNull Thread initThread; private volatile @Nullable NetworkCapabilities cachedNetworkCapabilities; private volatile @Nullable Network currentNetwork; private volatile long lastCacheUpdateTime = 0; private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes + private final @NotNull AtomicBoolean isConnected = new AtomicBoolean(false); @SuppressLint("InlinedApi") public AndroidConnectionStatusProvider( @@ -84,8 +87,9 @@ public AndroidConnectionStatusProvider( // Register network callback immediately for caching //noinspection Convert2MethodRef - initThread = new Thread(() -> ensureNetworkCallbackRegistered()); - initThread.start(); + submitSafe(() -> ensureNetworkCallbackRegistered()); + + AppState.getInstance().addAppStateListener(this); } /** @@ -152,6 +156,10 @@ private boolean isNetworkEffectivelyConnected( } private void ensureNetworkCallbackRegistered() { + if (!ContextUtils.isForegroundImportance()) { + return; + } + if (networkCallback != null) { return; // Already registered } @@ -167,9 +175,14 @@ private void ensureNetworkCallbackRegistered() { public void onAvailable(final @NotNull Network network) { currentNetwork = network; - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onAvailable(network); + // have to only dispatch this on first registration + when the connection got + // re-established + // otherwise it would've been dispatched on every foreground + if (!isConnected.getAndSet(true)) { + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onAvailable(network); + } } } } @@ -201,6 +214,7 @@ public void onLost(final @NotNull Network network) { } private void clearCacheAndNotifyObservers() { + isConnected.set(false); // Clear cached capabilities and network reference atomically try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { cachedNetworkCapabilities = null; @@ -417,42 +431,90 @@ public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObser } } + private void unregisterNetworkCallback(final boolean clearObservers) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (clearObservers) { + connectionStatusObservers.clear(); + } + + final @Nullable NetworkCallback callbackRef = networkCallback; + networkCallback = null; + + if (callbackRef != null) { + unregisterNetworkCallback(context, options.getLogger(), callbackRef); + } + // Clear cached state + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = 0; + } + options.getLogger().log(SentryLevel.DEBUG, "Network callback unregistered"); + } + /** Clean up resources - should be called when the provider is no longer needed */ @Override public void close() { - try { - options - .getExecutorService() - .submit( - () -> { - final NetworkCallback callbackRef; - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - connectionStatusObservers.clear(); + submitSafe( + () -> { + unregisterNetworkCallback(/* clearObservers= */ true); + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.clear(); + } + try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { + connectivityManager = null; + } + AppState.getInstance().removeAppStateListener(this); + }); + } - callbackRef = networkCallback; - networkCallback = null; + @Override + public void onForeground() { + if (networkCallback != null) { + return; + } - if (callbackRef != null) { - unregisterNetworkCallback(context, options.getLogger(), callbackRef); - } - // Clear cached state - cachedNetworkCapabilities = null; - currentNetwork = null; - lastCacheUpdateTime = 0; - } - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - childCallbacks.clear(); - } - try (final @NotNull ISentryLifecycleToken ignored = - connectivityManagerLock.acquire()) { - connectivityManager = null; - } - }); - } catch (Throwable t) { - options - .getLogger() - .log(SentryLevel.ERROR, "Error submitting AndroidConnectionStatusProvider task", t); + submitSafe( + () -> { + // proactively update cache and notify observers on foreground to ensure connectivity + // state is not stale + updateCache(null); + + final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); + if (status == ConnectionStatus.DISCONNECTED) { + // onLost is not called retroactively when we registerNetworkCallback (as opposed to + // onAvailable), so we have to do it manually for the DISCONNECTED case + isConnected.set(false); + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + //noinspection DataFlowIssue + cb.onLost(null); + } + } + } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + for (final @NotNull IConnectionStatusObserver observer : connectionStatusObservers) { + observer.onConnectionStatusChanged(status); + } + } + + // this will ONLY do the necessary parts like registerNetworkCallback and onAvailable, but + // it won't updateCache and notify observes because we just did it above and the cached + // capabilities will be the same + ensureNetworkCallbackRegistered(); + }); + } + + @Override + public void onBackground() { + if (networkCallback == null) { + return; } + + submitSafe( + () -> { + //noinspection Convert2MethodRef + unregisterNetworkCallback(/* clearObservers= */ false); + }); } /** @@ -599,7 +661,6 @@ public NetworkCapabilities getCachedNetworkCapabilities() { if (cellular) { return "cellular"; } - } catch (Throwable exception) { logger.log(SentryLevel.ERROR, "Failed to retrieve network info", exception); } @@ -734,15 +795,19 @@ public NetworkCallback getNetworkCallback() { return networkCallback; } - @TestOnly - @NotNull - public Thread getInitThread() { - return initThread; - } - @TestOnly @NotNull public static List getChildCallbacks() { return childCallbacks; } + + private void submitSafe(@NotNull Runnable r) { + try { + options.getExecutorService().submit(r); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, "AndroidConnectionStatusProvider submit failed", e); + } + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index c156eafd1e..1c17ac7c65 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -1,10 +1,13 @@ package io.sentry.android.core +import android.app.ActivityManager +import android.app.ActivityManager.RunningAppProcessInfo import android.content.Context import android.content.Intent import android.os.BatteryManager import android.os.Build import android.os.Looper +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Breadcrumb import io.sentry.IScopes @@ -33,6 +36,9 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config +import org.robolectric.shadow.api.Shadow +import org.robolectric.shadows.ShadowActivityManager +import org.robolectric.shadows.ShadowBuild @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -41,6 +47,7 @@ class SystemEventsBreadcrumbsIntegrationTest { val context = mock() var options = SentryAndroidOptions() val scopes = mock() + lateinit var shadowActivityManager: ShadowActivityManager fun getSut( enableSystemEventBreadcrumbs: Boolean = true, @@ -64,6 +71,11 @@ class SystemEventsBreadcrumbsIntegrationTest { fun `set up`() { AppState.getInstance().resetInstance() AppState.getInstance().registerLifecycleObserver(fixture.options) + ShadowBuild.reset() + val activityManager = + ApplicationProvider.getApplicationContext() + .getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager? + fixture.shadowActivityManager = Shadow.extract(activityManager) } @AfterTest @@ -501,4 +513,17 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNull(sut.receiver) } + + @Test + fun `when integration is registered in background, receiver is not registered`() { + val process = + RunningAppProcessInfo().apply { this.importance = RunningAppProcessInfo.IMPORTANCE_CACHED } + val processes = mutableListOf(process) + fixture.shadowActivityManager.setProcesses(processes) + + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + assertNull(sut.receiver) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index 6769ac1530..7d27984a59 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -15,10 +15,14 @@ import android.net.NetworkCapabilities.TRANSPORT_ETHERNET import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkInfo import android.os.Build +import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.IConnectionStatusProvider import io.sentry.ILogger import io.sentry.SentryOptions +import io.sentry.android.core.AppState import io.sentry.android.core.BuildInfoProvider +import io.sentry.android.core.ContextUtils +import io.sentry.android.core.SystemEventsBreadcrumbsIntegration import io.sentry.test.ImmediateExecutorService import io.sentry.transport.ICurrentDateProvider import kotlin.test.AfterTest @@ -29,8 +33,13 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import org.junit.runner.RunWith +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.clearInvocations import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.mockingDetails @@ -39,7 +48,10 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever +import org.robolectric.annotation.Config +@RunWith(AndroidJUnit4::class) +@Config(sdk = [Build.VERSION_CODES.P]) class AndroidConnectionStatusProviderTest { private lateinit var connectionStatusProvider: AndroidConnectionStatusProvider private lateinit var contextMock: Context @@ -51,6 +63,7 @@ class AndroidConnectionStatusProviderTest { private lateinit var network: Network private lateinit var networkCapabilities: NetworkCapabilities private lateinit var logger: ILogger + private lateinit var contextUtilsStaticMock: MockedStatic private var currentTime = 1000L @@ -91,17 +104,28 @@ class AndroidConnectionStatusProviderTest { // Reset current time for each test to ensure cache isolation currentTime = 1000L + // Mock ContextUtils to return foreground importance + contextUtilsStaticMock = mockStatic(ContextUtils::class.java) + contextUtilsStaticMock + .`when` { ContextUtils.isForegroundImportance() } + .thenReturn(true) + contextUtilsStaticMock + .`when` { ContextUtils.getApplicationContext(any()) } + .thenReturn(contextMock) + + AppState.getInstance().resetInstance() + AppState.getInstance().registerLifecycleObserver(options) + connectionStatusProvider = AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) - - // Wait for async callback registration to complete - connectionStatusProvider.initThread.join() } @AfterTest fun `tear down`() { // clear the cache and ensure proper cleanup connectionStatusProvider.close() + contextUtilsStaticMock.close() + AppState.getInstance().unregisterLifecycleObserver() } @Test @@ -161,10 +185,14 @@ class AndroidConnectionStatusProviderTest { ) .thenReturn(PERMISSION_GRANTED) + // Need to mock ContextUtils for the new provider as well + contextUtilsStaticMock + .`when` { ContextUtils.getApplicationContext(eq(nullConnectivityContext)) } + .thenReturn(nullConnectivityContext) + // Create a new provider with the null connectivity manager val providerWithNullConnectivity = AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) - providerWithNullConnectivity.initThread.join() // Wait for async init to complete assertEquals( IConnectionStatusProvider.ConnectionStatus.UNKNOWN, @@ -443,7 +471,7 @@ class AndroidConnectionStatusProviderTest { verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) val callback = callbackCaptor.firstValue - // Set current network + // IMPORTANT: Set network as current first callback.onAvailable(network) // Lose the network @@ -632,4 +660,185 @@ class AndroidConnectionStatusProviderTest { mainCallback.onAvailable(network) verifyNoMoreInteractions(childCallback) } + + @Test + fun `onForeground notifies child callbacks when disconnected`() { + val childCallback = mock() + AndroidConnectionStatusProvider.addNetworkCallback( + contextMock, + logger, + buildInfo, + childCallback, + ) + connectionStatusProvider.onBackground() + + // Setup disconnected state + whenever(connectivityManager.activeNetwork).thenReturn(null) + + connectionStatusProvider.onForeground() + + // Verify child callback was notified of lost connection with any network parameter + verify(childCallback).onLost(anyOrNull()) + } + + @Test + fun `close removes AppState listener`() { + // Clear any setup interactions + clearInvocations(connectivityManager) + + // Close the provider + connectionStatusProvider.close() + + // Now test that after closing, the provider no longer responds to lifecycle events + connectionStatusProvider.onForeground() + connectionStatusProvider.onBackground() + + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) + } + + @Test + fun `network callbacks work correctly across foreground background transitions`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Get the registered callback + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Simulate network available + callback.onAvailable(network) + + // Go to background + connectionStatusProvider.onBackground() + + // Clear the mock to reset interaction count + clearInvocations(connectivityManager) + + // Go back to foreground + connectionStatusProvider.onForeground() + + // Verify callback was re-registered + verify(connectivityManager).registerDefaultNetworkCallback(any()) + + // Verify we can still receive network events + val newCallbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(newCallbackCaptor.capture()) + val newCallback = newCallbackCaptor.lastValue + + // Simulate network capabilities change + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + + // First make the network available to set it as current + newCallback.onAvailable(network) + + // Then change capabilities + newCallback.onCapabilitiesChanged(network, newCaps) + + // Verify observer was notified (once for onForeground status update, once for capabilities + // change) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `onForeground registers network callback if not already registered`() { + // First ensure the network callback is not registered (simulate background state) + connectionStatusProvider.onBackground() + + // Verify callback was unregistered + assertNull(connectionStatusProvider.networkCallback) + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify network callback was registered + assertNotNull(connectionStatusProvider.networkCallback) + verify(connectivityManager, times(2)).registerDefaultNetworkCallback(any()) + } + + @Test + fun `onForeground updates cache and notifies observers`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Simulate going to background first + connectionStatusProvider.onBackground() + + // Reset mock to clear previous interactions + whenever(connectivityManager.activeNetwork).thenReturn(network) + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(networkCapabilities) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify observer was notified with current status + verify(observer).onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.CONNECTED) + } + + @Test + fun `onForeground does nothing if callback already registered`() { + // Ensure callback is already registered + assertNotNull(connectionStatusProvider.networkCallback) + val initialCallback = connectionStatusProvider.networkCallback + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify callback hasn't changed + assertEquals(initialCallback, connectionStatusProvider.networkCallback) + // Verify registerDefaultNetworkCallback was only called once (during construction) + verify(connectivityManager, times(1)).registerDefaultNetworkCallback(any()) + } + + @Test + fun `onBackground unregisters network callback`() { + // Ensure callback is registered + assertNotNull(connectionStatusProvider.networkCallback) + + // Call onBackground + connectionStatusProvider.onBackground() + + // Verify callback was unregistered + assertNull(connectionStatusProvider.networkCallback) + verify(connectivityManager).unregisterNetworkCallback(any()) + } + + @Test + fun `onBackground does not clear observers`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Call onBackground + connectionStatusProvider.onBackground() + + // Verify observer is still registered + assertEquals(1, connectionStatusProvider.statusObservers.size) + assertTrue(connectionStatusProvider.statusObservers.contains(observer)) + } + + @Test + fun `onBackground does nothing if callback not registered`() { + // First unregister by going to background + connectionStatusProvider.onBackground() + assertNull(connectionStatusProvider.networkCallback) + + // Reset mock to clear previous interactions + clearInvocations(connectivityManager) + + // Call onBackground again + connectionStatusProvider.onBackground() + + // Verify no additional unregister calls + verifyNoInteractions(connectivityManager) + } } diff --git a/sentry-samples/sentry-samples-android/sdkperf/README.md b/sentry-samples/sentry-samples-android/sdkperf/README.md new file mode 100644 index 0000000000..a350ec0d80 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/README.md @@ -0,0 +1,78 @@ +This folder contains various artifacts and info related to testing SDK performance and behaviour under different circumstances. + +## Perfetto + +The `basic.pbtx` file contains a perfetto config which covers some basic data sources and things that you usually would be interested in while experimenting with the SDK. + +You can adjust some certain things like `duration_ms` to make the trace last longer or add additional [data sources](https://perfetto.dev/docs/data-sources/atrace). + +To run it, ensure you have a device available via `adb` and then run: + +```bash +adb shell perfetto \ + -c basic.pbtx --txt \ + -o /data/misc/perfetto-traces/trace +``` + +And then perform various activities you're interested in. After the trace has finished, you can pull it: + +```bash +adb pull /data/misc/perfetto-traces/trace +``` + +And open it up in https://ui.perfetto.dev/. + +## Network Connectivity + +Android has a weird behavior which has been fixed in [Android 15](https://cs.android.com/android/_/android/platform/packages/modules/Connectivity/+/2d78124348f4864d054ea7a7b52683d225bd7c1f), where it would queue up pending NetworkCallbacks while an app is being frozen and would deliver **all** of them after the app unfreezes in a quick succession. + +Since our SDK is listening to NetworkCallbacks in [AndroidConnectionStatusProvider](../../../sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java) to determine current network connectivity status and to create breadcrumbs, our SDK can be burst with potentially hundreds or thousands of events after hours or days of the hosting app inactivity. + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app and send it to background +2. Freeze it with `adb shell am freeze --sticky io.sentry.samples.android` +3. Run the `./wifi_flap` script which looses and obtains network connectivity 10 times. +4. Unfreeze the app with `adb shell am unfreeze io.sentry.samples.android` + +You can either watch Logcat or better start a Perfetto trace beforehand and then open it and observe the number of binder calls our SDK is doing to the Connectivity service. + +### Solution + +We have addressed the issue in [#4579](https://github.com/getsentry/sentry-java/pull/4579) by unsubscribing from network connectivity updates when app goes to background and re-subscribing again on foreground. + +## System Events + +[Android 14](https://developer.android.com/develop/background-work/background-tasks/broadcasts#android-14) introduced a new behavior that defers any system broadcasts while an app is in a cached state. These pending broadcasts will be delivered to the app once it gets uncached in a quick succession. + +Our SDK is listening to a bunch of broadcasts in [SystemEventsBreadcrumbsIntegration](../../../sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java) to create breadcrumbs, the SDK can be burst with potentially hundreds or thousands of pending broadcasts after hours or days of the hosting app inactivity. + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app and send it to background +2. Freeze it with `adb shell am freeze --sticky io.sentry.samples.android` +3. Run the `./screen_flap` script which turns the screen on and off 10 times. +4. Unfreeze the app with `adb shell am unfreeze io.sentry.samples.android` + +And watch Logcat for a bunch of `SCREEN_OFF`/`SCREEN_ON` breadcrumbs created microseconds apart. + +### Solution + +We have addressed the issue in [#4338](https://github.com/getsentry/sentry-java/pull/4338) by unregistering the `BroadcastReceiver` when app goes to background and registering it again on foreground. + +## App Launch with Background Importance + +While the above two issues can be fixed by observing the App lifecycle, they still may become a problem if the app process has been launched with non-foreground importance (e.g. received a push notification). In this case our SDK would be initialized too and would still subscribe for SystemEvents and NetworkCallbacks while in background. + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app +2. Kill it with `adb shell am force-stop io.sentry.samples.android` +3. Now launch a dummy service with `adb shell am start-foreground-service -n io.sentry.samples.android/io.sentry.samples.android.DummyService`. This ensures the app process is run with non-foreground importance. +4. Follow any of the steps described in the sections above. + +Observe (Logcat or Perfetto) that the faulty behaviours are still reproducible. + +### Solution + +We have addressed the issue in [#4579](https://github.com/getsentry/sentry-java/pull/4579) by not registering any of the offending integrations when the hosting app process is launched with non-foreground `importance`. We still keep observing the App Lifecycle to ensure we register the integrations when the App has been brought to foreground. \ No newline at end of file diff --git a/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx b/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx new file mode 100644 index 0000000000..20f599b9b0 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx @@ -0,0 +1,103 @@ +buffers { + size_kb: 265536 + fill_policy: DISCARD +} +buffers { + size_kb: 4096 + fill_policy: DISCARD +} +data_sources { + config { + name: "linux.ftrace" + ftrace_config { + ftrace_events: "sched/sched_process_exit" + ftrace_events: "sched/sched_process_free" + ftrace_events: "task/task_newtask" + ftrace_events: "task/task_rename" + ftrace_events: "sched/sched_switch" + ftrace_events: "power/suspend_resume" + ftrace_events: "sched/sched_blocked_reason" + ftrace_events: "sched/sched_wakeup" + ftrace_events: "sched/sched_wakeup_new" + ftrace_events: "sched/sched_waking" + ftrace_events: "sched/sched_process_exit" + ftrace_events: "sched/sched_process_free" + ftrace_events: "task/task_newtask" + ftrace_events: "task/task_rename" + ftrace_events: "power/cpu_frequency" + ftrace_events: "power/cpu_idle" + ftrace_events: "power/suspend_resume" + ftrace_events: "power/gpu_frequency" + ftrace_events: "power/gpu_work_period" + ftrace_events: "ftrace/print" + atrace_categories: "adb" + atrace_categories: "camera" + atrace_categories: "gfx" + atrace_categories: "network" + atrace_categories: "power" + atrace_categories: "wm" + atrace_categories: "am" + atrace_categories: "dalvik" + atrace_categories: "bionic" + atrace_categories: "binder_driver" + atrace_categories: "binder_lock" + atrace_categories: "ss" + atrace_apps: "*" + + symbolize_ksyms: true + } + } +} +data_sources { + config { + name: "linux.process_stats" + process_stats_config { + scan_all_processes_on_start: true + proc_stats_poll_ms: 250 + } + } +} +data_sources { + config { + name: "linux.sys_stats" + } +} +data_sources { + config { + name: "android.log" + android_log_config { + } + } +} +data_sources { + config { + name: "android.surfaceflinger.frametimeline" + } +} +data_sources { + config { + name: "linux.perf" + perf_event_config { + remote_descriptor_timeout_ms: 10000 + ring_buffer_pages: 8192 + + timebase { + period: 1 + + tracepoint { + #name: "binder/binder_transaction" + name: "binder/binder_transaction_received" + } + } + callstack_sampling { + scope { + target_cmdline: "io.sentry.samples.android" + } + kernel_frames: true + } + } + } +} + + +duration_ms: 100000 \ No newline at end of file diff --git a/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh new file mode 100755 index 0000000000..828d0deeef --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +# Loop 4 times to toggle Screen +for i in {1..4} +do + echo "[$i] Turning screen off..." + adb shell input keyevent 223 + sleep 5 + + echo "[$i] Turning screen on..." + adb shell input keyevent 224 + sleep 5 +done + +echo "Done flapping Screen 4 times." diff --git a/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh new file mode 100755 index 0000000000..a468f5be47 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh @@ -0,0 +1,20 @@ +#!/bin/bash + +# Disable mobile data first +echo "Disabling mobile data..." +adb shell svc data disable + +# Loop 8 times to toggle Wi-Fi +for i in {1..8} +do + echo "[$i] Disabling Wi-Fi..." + adb shell svc wifi disable + sleep 2 + + echo "[$i] Enabling Wi-Fi..." + adb shell svc wifi enable + sleep 6 +done +# Turn mobile data back on +adb shell svc data enable +echo "Done flapping Wi-Fi 8 times." diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index d26b2a485e..9d084ed97e 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -12,7 +12,10 @@ - + + + + @@ -29,6 +32,12 @@ android:networkSecurityConfig="@xml/network" tools:ignore="GoogleAppIndexingWarning, UnusedAttribute"> + + diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java new file mode 100644 index 0000000000..b1d936e0cc --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java @@ -0,0 +1,68 @@ +package io.sentry.samples.android; + +import android.app.Notification; +import android.app.NotificationChannel; +import android.app.NotificationManager; +import android.app.Service; +import android.content.Intent; +import android.os.Build; +import android.os.IBinder; +import android.util.Log; + +public class DummyService extends Service { + + private static final String TAG = "DummyService"; + private static final String CHANNEL_ID = "dummy_service_channel"; + + @Override + public void onCreate() { + super.onCreate(); + Log.d(TAG, "DummyService created"); + createNotificationChannel(); + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + Log.d(TAG, "DummyService started"); + + Notification notification = null; + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + notification = + new Notification.Builder(this, CHANNEL_ID) + .setContentTitle("Dummy Service Running") + .setContentText("Used for background broadcast testing.") + .setSmallIcon(android.R.drawable.ic_menu_info_details) + .build(); + } + + if (notification != null) { + startForeground(1, notification); + } + + // You can stop immediately or keep running + // stopSelf(); + + return START_NOT_STICKY; + } + + @Override + public void onDestroy() { + super.onDestroy(); + Log.d(TAG, "DummyService destroyed"); + } + + @Override + public IBinder onBind(Intent intent) { + return null; + } + + private void createNotificationChannel() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + NotificationChannel channel = + new NotificationChannel( + CHANNEL_ID, "Dummy Service Channel", NotificationManager.IMPORTANCE_LOW); + NotificationManager manager = getSystemService(NotificationManager.class); + manager.createNotificationChannel(channel); + } + } +}