Skip to content

Commit 617a292

Browse files
romtsnromtsn
authored
perf(connectivity): Have only one NetworkCallback active at a time (#4562)
1 parent 6fc8adc commit 617a292

File tree

10 files changed

+160
-195
lines changed

10 files changed

+160
-195
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
- Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560))
1212
- Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561))
1313
- Remove unused method in ManifestMetadataReader ([#4585](https://github.com/getsentry/sentry-java/pull/4585))
14-
14+
- Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562))
1515

1616
## 8.18.0
1717

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration
263263
}
264264

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

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,7 @@ static void installDefaultIntegrations(
382382
}
383383
options.addIntegration(new AppComponentsBreadcrumbsIntegration(context));
384384
options.addIntegration(new SystemEventsBreadcrumbsIntegration(context));
385-
options.addIntegration(
386-
new NetworkBreadcrumbsIntegration(context, buildInfoProvider, options.getLogger()));
385+
options.addIntegration(new NetworkBreadcrumbsIntegration(context, buildInfoProvider));
387386
if (isReplayAvailable) {
388387
final ReplayIntegration replay =
389388
new ReplayIntegration(context, CurrentDateProvider.getInstance());

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

Lines changed: 33 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import io.sentry.Breadcrumb;
1313
import io.sentry.DateUtils;
1414
import io.sentry.Hint;
15-
import io.sentry.ILogger;
1615
import io.sentry.IScopes;
1716
import io.sentry.ISentryLifecycleToken;
1817
import io.sentry.Integration;
@@ -33,22 +32,17 @@ public final class NetworkBreadcrumbsIntegration implements Integration, Closeab
3332

3433
private final @NotNull Context context;
3534
private final @NotNull BuildInfoProvider buildInfoProvider;
36-
private final @NotNull ILogger logger;
3735
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
38-
private volatile boolean isClosed;
3936
private @Nullable SentryOptions options;
4037

4138
@TestOnly @Nullable volatile NetworkBreadcrumbsNetworkCallback networkCallback;
4239

4340
public NetworkBreadcrumbsIntegration(
44-
final @NotNull Context context,
45-
final @NotNull BuildInfoProvider buildInfoProvider,
46-
final @NotNull ILogger logger) {
41+
final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider) {
4742
this.context =
4843
Objects.requireNonNull(ContextUtils.getApplicationContext(context), "Context is required");
4944
this.buildInfoProvider =
5045
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
51-
this.logger = Objects.requireNonNull(logger, "ILogger is required");
5246
}
5347

5448
@Override
@@ -59,87 +53,61 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
5953
(options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null,
6054
"SentryAndroidOptions is required");
6155

62-
logger.log(
63-
SentryLevel.DEBUG,
64-
"NetworkBreadcrumbsIntegration enabled: %s",
65-
androidOptions.isEnableNetworkEventBreadcrumbs());
66-
6756
this.options = options;
6857

58+
options
59+
.getLogger()
60+
.log(
61+
SentryLevel.DEBUG,
62+
"NetworkBreadcrumbsIntegration enabled: %s",
63+
androidOptions.isEnableNetworkEventBreadcrumbs());
64+
6965
if (androidOptions.isEnableNetworkEventBreadcrumbs()) {
7066

7167
// The specific error is logged in the ConnectivityChecker method
7268
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
73-
logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
69+
options.getLogger().log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
7470
return;
7571
}
7672

77-
try {
78-
options
79-
.getExecutorService()
80-
.submit(
81-
new Runnable() {
82-
@Override
83-
public void run() {
84-
// in case integration is closed before the task is executed, simply return
85-
if (isClosed) {
86-
return;
87-
}
88-
89-
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
90-
networkCallback =
91-
new NetworkBreadcrumbsNetworkCallback(
92-
scopes, buildInfoProvider, options.getDateProvider());
93-
94-
final boolean registered =
95-
AndroidConnectionStatusProvider.registerNetworkCallback(
96-
context, logger, buildInfoProvider, networkCallback);
97-
if (registered) {
98-
logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed.");
99-
addIntegrationToSdkVersion("NetworkBreadcrumbs");
100-
} else {
101-
logger.log(
102-
SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed.");
103-
// The specific error is logged by AndroidConnectionStatusProvider
104-
}
105-
}
106-
}
107-
});
108-
} catch (Throwable t) {
109-
logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t);
73+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
74+
networkCallback =
75+
new NetworkBreadcrumbsNetworkCallback(
76+
scopes, buildInfoProvider, options.getDateProvider());
77+
78+
final boolean registered =
79+
AndroidConnectionStatusProvider.addNetworkCallback(
80+
context, options.getLogger(), buildInfoProvider, networkCallback);
81+
if (registered) {
82+
options.getLogger().log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed.");
83+
addIntegrationToSdkVersion("NetworkBreadcrumbs");
84+
} else {
85+
options
86+
.getLogger()
87+
.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed.");
88+
// The specific error is logged by AndroidConnectionStatusProvider
89+
}
11090
}
11191
}
11292
}
11393

11494
@Override
11595
public void close() throws IOException {
116-
isClosed = true;
96+
final @Nullable ConnectivityManager.NetworkCallback callbackRef;
97+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
98+
callbackRef = networkCallback;
99+
networkCallback = null;
100+
}
117101

118-
try {
119-
Objects.requireNonNull(options, "Options is required")
120-
.getExecutorService()
121-
.submit(
122-
() -> {
123-
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
124-
if (networkCallback != null) {
125-
AndroidConnectionStatusProvider.unregisterNetworkCallback(
126-
context, logger, networkCallback);
127-
logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration removed.");
128-
}
129-
networkCallback = null;
130-
}
131-
});
132-
} catch (Throwable t) {
133-
logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t);
102+
if (callbackRef != null) {
103+
AndroidConnectionStatusProvider.removeNetworkCallback(callbackRef);
134104
}
135105
}
136106

137107
static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager.NetworkCallback {
138108
final @NotNull IScopes scopes;
139109
final @NotNull BuildInfoProvider buildInfoProvider;
140110

141-
@Nullable Network currentNetwork = null;
142-
143111
@Nullable NetworkCapabilities lastCapabilities = null;
144112
long lastCapabilityNanos = 0;
145113
final @NotNull SentryDateProvider dateProvider;
@@ -156,21 +124,14 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager
156124

157125
@Override
158126
public void onAvailable(final @NonNull Network network) {
159-
if (network.equals(currentNetwork)) {
160-
return;
161-
}
162127
final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_AVAILABLE");
163128
scopes.addBreadcrumb(breadcrumb);
164-
currentNetwork = network;
165129
lastCapabilities = null;
166130
}
167131

168132
@Override
169133
public void onCapabilitiesChanged(
170134
final @NonNull Network network, final @NonNull NetworkCapabilities networkCapabilities) {
171-
if (!network.equals(currentNetwork)) {
172-
return;
173-
}
174135
final long nowNanos = dateProvider.now().nanoTimestamp();
175136
final @Nullable NetworkBreadcrumbConnectionDetail connectionDetail =
176137
getNewConnectionDetails(
@@ -195,12 +156,8 @@ public void onCapabilitiesChanged(
195156

196157
@Override
197158
public void onLost(final @NonNull Network network) {
198-
if (!network.equals(currentNetwork)) {
199-
return;
200-
}
201159
final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_LOST");
202160
scopes.addBreadcrumb(breadcrumb);
203-
currentNetwork = null;
204161
lastCapabilities = null;
205162
}
206163

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ public void close() throws IOException {
7575
@Override
7676
public void onConnectionStatusChanged(
7777
final @NotNull IConnectionStatusProvider.ConnectionStatus status) {
78-
if (scopes != null && options != null) {
78+
if (scopes != null
79+
&& options != null
80+
&& status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) {
7981
sendCachedEnvelopes(scopes, options);
8082
}
8183
}

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

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import android.net.NetworkCapabilities;
1010
import android.os.Build;
1111
import androidx.annotation.NonNull;
12+
import androidx.annotation.RequiresApi;
1213
import io.sentry.IConnectionStatusProvider;
1314
import io.sentry.ILogger;
1415
import io.sentry.ISentryLifecycleToken;
@@ -45,6 +46,10 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP
4546
new AutoClosableReentrantLock();
4647
private static volatile @Nullable ConnectivityManager connectivityManager;
4748

49+
private static final @NotNull AutoClosableReentrantLock childCallbacksLock =
50+
new AutoClosableReentrantLock();
51+
private static final @NotNull List<NetworkCallback> childCallbacks = new ArrayList<>();
52+
4853
private static final int[] transports = {
4954
NetworkCapabilities.TRANSPORT_WIFI,
5055
NetworkCapabilities.TRANSPORT_CELLULAR,
@@ -161,11 +166,24 @@ private void ensureNetworkCallbackRegistered() {
161166
@Override
162167
public void onAvailable(final @NotNull Network network) {
163168
currentNetwork = network;
169+
170+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
171+
for (final @NotNull NetworkCallback cb : childCallbacks) {
172+
cb.onAvailable(network);
173+
}
174+
}
164175
}
165176

177+
@RequiresApi(Build.VERSION_CODES.O)
166178
@Override
167179
public void onUnavailable() {
168180
clearCacheAndNotifyObservers();
181+
182+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
183+
for (final @NotNull NetworkCallback cb : childCallbacks) {
184+
cb.onUnavailable();
185+
}
186+
}
169187
}
170188

171189
@Override
@@ -174,6 +192,12 @@ public void onLost(final @NotNull Network network) {
174192
return;
175193
}
176194
clearCacheAndNotifyObservers();
195+
196+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
197+
for (final @NotNull NetworkCallback cb : childCallbacks) {
198+
cb.onLost(network);
199+
}
200+
}
177201
}
178202

179203
private void clearCacheAndNotifyObservers() {
@@ -203,6 +227,12 @@ public void onCapabilitiesChanged(
203227
return;
204228
}
205229
updateCacheAndNotifyObservers(network, networkCapabilities);
230+
231+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
232+
for (final @NotNull NetworkCallback cb : childCallbacks) {
233+
cb.onCapabilitiesChanged(network, networkCapabilities);
234+
}
235+
}
206236
}
207237

208238
private void updateCacheAndNotifyObservers(
@@ -410,6 +440,9 @@ public void close() {
410440
currentNetwork = null;
411441
lastCacheUpdateTime = 0;
412442
}
443+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
444+
childCallbacks.clear();
445+
}
413446
try (final @NotNull ISentryLifecycleToken ignored =
414447
connectivityManagerLock.acquire()) {
415448
connectivityManager = null;
@@ -618,8 +651,35 @@ public NetworkCapabilities getCachedNetworkCapabilities() {
618651
}
619652
}
620653

654+
public static boolean addNetworkCallback(
655+
final @NotNull Context context,
656+
final @NotNull ILogger logger,
657+
final @NotNull BuildInfoProvider buildInfoProvider,
658+
final @NotNull NetworkCallback networkCallback) {
659+
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
660+
logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
661+
return false;
662+
}
663+
664+
if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) {
665+
logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
666+
return false;
667+
}
668+
669+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
670+
childCallbacks.add(networkCallback);
671+
}
672+
return true;
673+
}
674+
675+
public static void removeNetworkCallback(final @NotNull NetworkCallback networkCallback) {
676+
try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) {
677+
childCallbacks.remove(networkCallback);
678+
}
679+
}
680+
621681
@SuppressLint({"MissingPermission", "NewApi"})
622-
public static boolean registerNetworkCallback(
682+
static boolean registerNetworkCallback(
623683
final @NotNull Context context,
624684
final @NotNull ILogger logger,
625685
final @NotNull BuildInfoProvider buildInfoProvider,
@@ -646,7 +706,7 @@ public static boolean registerNetworkCallback(
646706
}
647707

648708
@SuppressLint("NewApi")
649-
public static void unregisterNetworkCallback(
709+
static void unregisterNetworkCallback(
650710
final @NotNull Context context,
651711
final @NotNull ILogger logger,
652712
final @NotNull NetworkCallback networkCallback) {
@@ -681,7 +741,8 @@ public Thread getInitThread() {
681741
}
682742

683743
@TestOnly
684-
public static void setConnectivityManager(final @Nullable ConnectivityManager cm) {
685-
connectivityManager = cm;
744+
@NotNull
745+
public static List<NetworkCallback> getChildCallbacks() {
746+
return childCallbacks;
686747
}
687748
}

0 commit comments

Comments
 (0)