Merge branch 'main' into feat/auto-trace-id-generation

.craft.yml

@@ -1,10 +1,6 @@
 minVersion: 0.29.3
 changelogPolicy: auto
 targets:
-  - name: symbol-collector
-    includeNames: /libsentry(-android)?\.so/
-    batchType: android
-    bundleIdPrefix: sentry-android-ndk-
   - name: maven
     includeNames: /^sentry.*$/
     gradleCliPath: ./gradlew

CHANGELOG.md

@@ -8,6 +8,7 @@
 - A list of active Spring profiles is attached to payloads sent to Sentry (errors, traces, etc.) and displayed in the UI when using our Spring or Spring Boot integrations ([#4147](https://github.com/getsentry/sentry-java/pull/4147))
   - This consists of an empty list when only the default profile is active
 - Added `enableTraceIdGeneration` to the AndroidOptions. This allows Hybrid SDKs to "freeze" and control the trace and connect errors on different layers of the application ([4188](https://github.com/getsentry/sentry-java/pull/4188))
+- Move to a single NetworkCallback listener to reduce number of IPC calls on Android ([#4164](https://github.com/getsentry/sentry-java/pull/4164))
 
 ### Fixes
 

sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java

@@ -4,17 +4,19 @@
 import android.annotation.SuppressLint;
 import android.content.Context;
 import android.net.ConnectivityManager;
+import android.net.ConnectivityManager.NetworkCallback;
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.os.Build;
-import androidx.annotation.NonNull;
 import io.sentry.IConnectionStatusProvider;
 import io.sentry.ILogger;
+import io.sentry.ISentryLifecycleToken;
 import io.sentry.SentryLevel;
 import io.sentry.android.core.BuildInfoProvider;
 import io.sentry.android.core.ContextUtils;
-import java.util.HashMap;
-import java.util.Map;
+import io.sentry.util.AutoClosableReentrantLock;
+import java.util.ArrayList;
+import java.util.List;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -31,8 +33,9 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP
   private final @NotNull Context context;
   private final @NotNull ILogger logger;
   private final @NotNull BuildInfoProvider buildInfoProvider;
-  private final @NotNull Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
-      registeredCallbacks;
+  private final @NotNull List<IConnectionStatusObserver> connectionStatusObservers;
+  private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
+  private volatile @Nullable NetworkCallback networkCallback;
 
   public AndroidConnectionStatusProvider(
       @NotNull Context context,
@@ -41,7 +44,7 @@ public AndroidConnectionStatusProvider(
     this.context = ContextUtils.getApplicationContext(context);
     this.logger = logger;
     this.buildInfoProvider = buildInfoProvider;
-    this.registeredCallbacks = new HashMap<>();
+    this.connectionStatusObservers = new ArrayList<>();
   }
 
   @Override
@@ -65,40 +68,64 @@ public AndroidConnectionStatusProvider(
 
   @Override
   public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
+    try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+      connectionStatusObservers.add(observer);
+    }
 
-    final ConnectivityManager.NetworkCallback callback =
-        new ConnectivityManager.NetworkCallback() {
-          @Override
-          public void onAvailable(@NonNull Network network) {
-            observer.onConnectionStatusChanged(getConnectionStatus());
-          }
-
-          @Override
-          public void onLosing(@NonNull Network network, int maxMsToLive) {
-            observer.onConnectionStatusChanged(getConnectionStatus());
-          }
-
-          @Override
-          public void onLost(@NonNull Network network) {
-            observer.onConnectionStatusChanged(getConnectionStatus());
-          }
-
-          @Override
-          public void onUnavailable() {
-            observer.onConnectionStatusChanged(getConnectionStatus());
+    if (networkCallback == null) {
+      try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+        if (networkCallback == null) {
+          final @NotNull NetworkCallback newNetworkCallback =
+              new NetworkCallback() {
+                @Override
+                public void onAvailable(final @NotNull Network network) {
+                  updateObservers();
+                }
+
+                @Override
+                public void onUnavailable() {
+                  updateObservers();
+                }
+
+                @Override
+                public void onLost(final @NotNull Network network) {
+                  updateObservers();
+                }
+
+                public void updateObservers() {
+                  final @NotNull ConnectionStatus status = getConnectionStatus();
+                  try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+                    for (final @NotNull IConnectionStatusObserver observer :
+                        connectionStatusObservers) {
+                      observer.onConnectionStatusChanged(status);
+                    }
+                  }
+                }
+              };
+
+          if (registerNetworkCallback(context, logger, buildInfoProvider, newNetworkCallback)) {
+            networkCallback = newNetworkCallback;
+            return true;
+          } else {
+            return false;
           }
-        };
-
-    registeredCallbacks.put(observer, callback);
-    return registerNetworkCallback(context, logger, buildInfoProvider, callback);
+        }
+      }
+    }
+    // networkCallback is already registered, so we can safely return true
+    return true;
   }
 
   @Override
   public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
-    final @Nullable ConnectivityManager.NetworkCallback callback =
-        registeredCallbacks.remove(observer);
-    if (callback != null) {
-      unregisterNetworkCallback(context, logger, callback);
+    try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+      connectionStatusObservers.remove(observer);
+      if (connectionStatusObservers.isEmpty()) {
+        if (networkCallback != null) {
+          unregisterNetworkCallback(context, logger, networkCallback);
+          networkCallback = null;
+        }
+      }
     }
   }
 
@@ -281,7 +308,7 @@ public static boolean registerNetworkCallback(
       final @NotNull Context context,
       final @NotNull ILogger logger,
       final @NotNull BuildInfoProvider buildInfoProvider,
-      final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
+      final @NotNull NetworkCallback networkCallback) {
     if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
       logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
       return false;
@@ -307,7 +334,7 @@ public static boolean registerNetworkCallback(
   public static void unregisterNetworkCallback(
       final @NotNull Context context,
       final @NotNull ILogger logger,
-      final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
+      final @NotNull NetworkCallback networkCallback) {
 
     final ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
     if (connectivityManager == null) {
@@ -322,8 +349,13 @@ public static void unregisterNetworkCallback(
 
   @TestOnly
   @NotNull
-  public Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
-      getRegisteredCallbacks() {
-    return registeredCallbacks;
+  public List<IConnectionStatusObserver> getStatusObservers() {
+    return connectionStatusObservers;
+  }
+
+  @TestOnly
+  @Nullable
+  public NetworkCallback getNetworkCallback() {
+    return networkCallback;
   }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt

@@ -24,6 +24,7 @@ import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
@@ -258,9 +259,12 @@ class AndroidConnectionStatusProviderTest {
         val observer = IConnectionStatusProvider.IConnectionStatusObserver { }
         val addResult = connectionStatusProvider.addConnectionStatusObserver(observer)
         assertTrue(addResult)
+        assertEquals(1, connectionStatusProvider.statusObservers.size)
+        assertNotNull(connectionStatusProvider.networkCallback)
 
         connectionStatusProvider.removeConnectionStatusObserver(observer)
-        assertTrue(connectionStatusProvider.registeredCallbacks.isEmpty())
+        assertTrue(connectionStatusProvider.statusObservers.isEmpty())
+        assertNull(connectionStatusProvider.networkCallback)
     }
 
     @Test
@@ -269,18 +273,16 @@ class AndroidConnectionStatusProviderTest {
 
         var callback: NetworkCallback? = null
         whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation ->
-            callback = invocation.getArgument(0, NetworkCallback::class.java)
+            callback = invocation.getArgument(0, NetworkCallback::class.java) as NetworkCallback
             Unit
         }
         val observer = mock<IConnectionStatusProvider.IConnectionStatusObserver>()
         connectionStatusProvider.addConnectionStatusObserver(observer)
         callback!!.onAvailable(mock<Network>())
         callback!!.onUnavailable()
-        callback!!.onLosing(mock<Network>(), 0)
         callback!!.onLost(mock<Network>())
-        callback!!.onUnavailable()
         connectionStatusProvider.removeConnectionStatusObserver(observer)
 
-        verify(observer, times(5)).onConnectionStatusChanged(any())
+        verify(observer, times(3)).onConnectionStatusChanged(any())
     }
 }