-
-
Notifications
You must be signed in to change notification settings - Fork 455
perf(connectivity): Have only one NetworkCallback active at a time #4562
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
base: rz/fix/diff-battery-crumbs
Are you sure you want to change the base?
perf(connectivity): Have only one NetworkCallback active at a time #4562
Conversation
scopes, buildInfoProvider, options.getDateProvider()); | ||
|
||
final boolean registered = | ||
AndroidConnectionStatusProvider.addNetworkCallback( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one caveat: This now will not emit AVAILABLE and CAPABILITIES_CHANGED callbacks immediately after registration as opposed to calling ConnectivityManager.registerDefaultNetworkCallback
.
But I think it's fine and how it should be - previously any event would always start with NETWORK_AVAILABLE breadcrumb, which could have been misleading, now we're actually adding breadcrumbs whenever there's a change.
if (scopes != null && options != null) { | ||
if (scopes != null | ||
&& options != null | ||
&& status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kind of unrelated to this PR, but I noticed that we also submit a task even if the status was DISCONNECTED, which was unnecessary since we'd anyway bail out in the submitted task itself.
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b0bed47 | 400.72 ms | 441.62 ms | 40.90 ms |
4522edb | 390.10 ms | 421.50 ms | 31.40 ms |
551ddda | 414.94 ms | 476.12 ms | 61.19 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b0bed47 | 1.58 MiB | 2.09 MiB | 520.80 KiB |
4522edb | 1.58 MiB | 2.09 MiB | 520.79 KiB |
551ddda | 1.58 MiB | 2.09 MiB | 520.78 KiB |
📜 Description
Event example: https://sentry-sdks.sentry.io/issues/6560149804/events/960094050aa341cca6ac9d0c175f699e/
Replay example: https://sentry-sdks.sentry.io/explore/replays/fff819393dd44cfa9a06b54255664529
💡 Motivation and Context
Less IPC
💚 How did you test it?
Manually + automated
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps