perf(connectivity): Have only one NetworkCallback active at a time

CHANGELOG.md

@@ -11,7 +11,7 @@
 - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560))
 - 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))
 
 ## 8.18.0
 

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

@@ -263,7 +263,7 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration
 }
 
 public final class io/sentry/android/core/NetworkBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable {
-	public fun <init> (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/ILogger;)V
+	public fun <init> (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;)V
 	public fun close ()V
 	public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
 }

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

@@ -382,8 +382,7 @@ static void installDefaultIntegrations(
     }
     options.addIntegration(new AppComponentsBreadcrumbsIntegration(context));
     options.addIntegration(new SystemEventsBreadcrumbsIntegration(context));
-    options.addIntegration(
-        new NetworkBreadcrumbsIntegration(context, buildInfoProvider, options.getLogger()));
+    options.addIntegration(new NetworkBreadcrumbsIntegration(context, buildInfoProvider));
     if (isReplayAvailable) {
       final ReplayIntegration replay =
           new ReplayIntegration(context, CurrentDateProvider.getInstance());

sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java

@@ -12,7 +12,6 @@
 import io.sentry.Breadcrumb;
 import io.sentry.DateUtils;
 import io.sentry.Hint;
-import io.sentry.ILogger;
 import io.sentry.IScopes;
 import io.sentry.ISentryLifecycleToken;
 import io.sentry.Integration;
@@ -33,22 +32,17 @@ public final class NetworkBreadcrumbsIntegration implements Integration, Closeab
 
   private final @NotNull Context context;
   private final @NotNull BuildInfoProvider buildInfoProvider;
-  private final @NotNull ILogger logger;
   private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
-  private volatile boolean isClosed;
   private @Nullable SentryOptions options;
 
   @TestOnly @Nullable volatile NetworkBreadcrumbsNetworkCallback networkCallback;
 
   public NetworkBreadcrumbsIntegration(
-      final @NotNull Context context,
-      final @NotNull BuildInfoProvider buildInfoProvider,
-      final @NotNull ILogger logger) {
+      final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider) {
     this.context =
         Objects.requireNonNull(ContextUtils.getApplicationContext(context), "Context is required");
     this.buildInfoProvider =
         Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
-    this.logger = Objects.requireNonNull(logger, "ILogger is required");
   }
 
   @Override
@@ -59,87 +53,61 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
             (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null,
             "SentryAndroidOptions is required");
 
-    logger.log(
-        SentryLevel.DEBUG,
-        "NetworkBreadcrumbsIntegration enabled: %s",
-        androidOptions.isEnableNetworkEventBreadcrumbs());
-
     this.options = options;
 
+    options
+        .getLogger()
+        .log(
+            SentryLevel.DEBUG,
+            "NetworkBreadcrumbsIntegration enabled: %s",
+            androidOptions.isEnableNetworkEventBreadcrumbs());
+
     if (androidOptions.isEnableNetworkEventBreadcrumbs()) {
 
       // The specific error is logged in the ConnectivityChecker method
       if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
-        logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
+        options.getLogger().log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
         return;
       }
 
-      try {
-        options
-            .getExecutorService()
-            .submit(
-                new Runnable() {
-                  @Override
-                  public void run() {
-                    // in case integration is closed before the task is executed, simply return
-                    if (isClosed) {
-                      return;
-                    }
-
-                    try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
-                      networkCallback =
-                          new NetworkBreadcrumbsNetworkCallback(
-                              scopes, buildInfoProvider, options.getDateProvider());
-
-                      final boolean registered =
-                          AndroidConnectionStatusProvider.registerNetworkCallback(
-                              context, logger, buildInfoProvider, networkCallback);
-                      if (registered) {
-                        logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed.");
-                        addIntegrationToSdkVersion("NetworkBreadcrumbs");
-                      } else {
-                        logger.log(
-                            SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed.");
-                        // The specific error is logged by AndroidConnectionStatusProvider
-                      }
-                    }
-                  }
-                });
-      } catch (Throwable t) {
-        logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t);
+      try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+        networkCallback =
+            new NetworkBreadcrumbsNetworkCallback(
+                scopes, buildInfoProvider, options.getDateProvider());
+
+        final boolean registered =
+            AndroidConnectionStatusProvider.addNetworkCallback(
+                context, options.getLogger(), buildInfoProvider, networkCallback);
+        if (registered) {
+          options.getLogger().log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed.");
+          addIntegrationToSdkVersion("NetworkBreadcrumbs");
+        } else {
+          options
+              .getLogger()
+              .log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed.");
+          // The specific error is logged by AndroidConnectionStatusProvider
+        }
       }
     }
   }
 
   @Override
   public void close() throws IOException {
-    isClosed = true;
+    final @Nullable ConnectivityManager.NetworkCallback callbackRef;
+    try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
+      callbackRef = networkCallback;
+      networkCallback = null;
+    }
 
-    try {
-      Objects.requireNonNull(options, "Options is required")
-          .getExecutorService()
-          .submit(
-              () -> {
-                try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
-                  if (networkCallback != null) {
-                    AndroidConnectionStatusProvider.unregisterNetworkCallback(
-                        context, logger, networkCallback);
-                    logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration removed.");
-                  }
-                  networkCallback = null;
-                }
-              });
-    } catch (Throwable t) {
-      logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t);
+    if (callbackRef != null) {
+      AndroidConnectionStatusProvider.removeNetworkCallback(callbackRef);
     }
   }
 
   static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager.NetworkCallback {
     final @NotNull IScopes scopes;
     final @NotNull BuildInfoProvider buildInfoProvider;
 
-    @Nullable Network currentNetwork = null;
-
     @Nullable NetworkCapabilities lastCapabilities = null;
     long lastCapabilityNanos = 0;
     final @NotNull SentryDateProvider dateProvider;
@@ -156,21 +124,14 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager
 
     @Override
     public void onAvailable(final @NonNull Network network) {
-      if (network.equals(currentNetwork)) {
-        return;
-      }
       final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_AVAILABLE");
       scopes.addBreadcrumb(breadcrumb);
-      currentNetwork = network;
       lastCapabilities = null;
     }
 
     @Override
     public void onCapabilitiesChanged(
         final @NonNull Network network, final @NonNull NetworkCapabilities networkCapabilities) {
-      if (!network.equals(currentNetwork)) {
-        return;
-      }
       final long nowNanos = dateProvider.now().nanoTimestamp();
       final @Nullable NetworkBreadcrumbConnectionDetail connectionDetail =
           getNewConnectionDetails(
@@ -195,12 +156,8 @@ public void onCapabilitiesChanged(
 
     @Override
     public void onLost(final @NonNull Network network) {
-      if (!network.equals(currentNetwork)) {
-        return;
-      }
       final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_LOST");
       scopes.addBreadcrumb(breadcrumb);
-      currentNetwork = null;
       lastCapabilities = null;
     }
 

sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java

@@ -75,7 +75,9 @@ public void close() throws IOException {
   @Override
   public void onConnectionStatusChanged(
       final @NotNull IConnectionStatusProvider.ConnectionStatus status) {
-    if (scopes != null && options != null) {
+    if (scopes != null
+        && options != null
+        && status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) {
       sendCachedEnvelopes(scopes, options);
     }
   }

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

@@ -9,6 +9,7 @@
 import android.net.NetworkCapabilities;
 import android.os.Build;
 import androidx.annotation.NonNull;
+import androidx.annotation.RequiresApi;
 import io.sentry.IConnectionStatusProvider;
 import io.sentry.ILogger;
 import io.sentry.ISentryLifecycleToken;
@@ -45,6 +46,10 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP
       new AutoClosableReentrantLock();
   private static volatile @Nullable ConnectivityManager connectivityManager;
 
+  private static final @NotNull AutoClosableReentrantLock childCallbacksLock =
+      new AutoClosableReentrantLock();
+  private static final @NotNull List<NetworkCallback> childCallbacks = new ArrayList<>();
+
   private static final int[] transports = {
     NetworkCapabilities.TRANSPORT_WIFI,
     NetworkCapabilities.TRANSPORT_CELLULAR,
@@ -161,11 +166,24 @@ private void ensureNetworkCallbackRegistered() {
             @Override
             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);
+                }
+              }
             }
 
+            @RequiresApi(Build.VERSION_CODES.O)
             @Override
             public void onUnavailable() {
               clearCacheAndNotifyObservers();
+
+              try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+                for (final @NotNull NetworkCallback cb : childCallbacks) {
+                  cb.onUnavailable();
+                }
+              }
             }
 
             @Override
@@ -174,6 +192,12 @@ public void onLost(final @NotNull Network network) {
                 return;
               }
               clearCacheAndNotifyObservers();
+
+              try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+                for (final @NotNull NetworkCallback cb : childCallbacks) {
+                  cb.onLost(network);
+                }
+              }
             }
 
             private void clearCacheAndNotifyObservers() {
@@ -203,6 +227,12 @@ public void onCapabilitiesChanged(
                 return;
               }
               updateCacheAndNotifyObservers(network, networkCapabilities);
+
+              try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+                for (final @NotNull NetworkCallback cb : childCallbacks) {
+                  cb.onCapabilitiesChanged(network, networkCapabilities);
+                }
+              }
             }
 
             private void updateCacheAndNotifyObservers(
@@ -410,6 +440,9 @@ public void close() {
                   currentNetwork = null;
                   lastCacheUpdateTime = 0;
                 }
+                try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+                  childCallbacks.clear();
+                }
                 try (final @NotNull ISentryLifecycleToken ignored =
                         connectivityManagerLock.acquire()) {
                   connectivityManager = null;
@@ -618,8 +651,35 @@ public NetworkCapabilities getCachedNetworkCapabilities() {
     }
   }
 
+  public static boolean addNetworkCallback(
+      final @NotNull Context context,
+      final @NotNull ILogger logger,
+      final @NotNull BuildInfoProvider buildInfoProvider,
+      final @NotNull NetworkCallback networkCallback) {
+    if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
+      logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
+      return false;
+    }
+
+    if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) {
+      logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
+      return false;
+    }
+
+    try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+      childCallbacks.add(networkCallback);
+    }
+    return true;
+  }
+
+  public static void removeNetworkCallback(final @NotNull NetworkCallback networkCallback) {
+    try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
+      childCallbacks.remove(networkCallback);
+    }
+  }
+
   @SuppressLint({"MissingPermission", "NewApi"})
-  public static boolean registerNetworkCallback(
+  static boolean registerNetworkCallback(
       final @NotNull Context context,
       final @NotNull ILogger logger,
       final @NotNull BuildInfoProvider buildInfoProvider,
@@ -646,7 +706,7 @@ public static boolean registerNetworkCallback(
   }
 
   @SuppressLint("NewApi")
-  public static void unregisterNetworkCallback(
+  static void unregisterNetworkCallback(
       final @NotNull Context context,
       final @NotNull ILogger logger,
       final @NotNull NetworkCallback networkCallback) {
@@ -681,7 +741,8 @@ public Thread getInitThread() {
   }
 
   @TestOnly
-  public static void setConnectivityManager(final @Nullable ConnectivityManager cm) {
-    connectivityManager = cm;
+  @NotNull
+  public static List<NetworkCallback> getChildCallbacks() {
+    return childCallbacks;
   }
 }