Skip to content
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

Fix BroadcastReceivers #4052

Merged
merged 8 commits into from
Jan 16, 2025
Merged

Fix BroadcastReceivers #4052

merged 8 commits into from
Jan 16, 2025

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Jan 15, 2025

📜 Description

  • Removes TempSensor and PhoneState breadcrumbs integrations, as those don't seem to be very useful and potentially register for too many broadcasts that sometimes cannot be delivered. We plan to introduce a new integration that would listen to the ThermalState changes instead.
  • Reduces the number of actions that we subscribe for in SystemEvents breadcrumbs integration. Some of those seem to be not helpful, but there are some that are sketchy, like ACTION_REBOOT (only for use of system apps) or ACTION_POWER_CONNECTED (which will wake the app up)
  • Changed receiver flag to be RECEIVER_NOT_EXPORTED. If you check the ContextCompat docs, it says RECEIVER_NOT_EXPORTED is enough to receive broadcasts from the system. I tested it locally with a script that runs over the actions we're subscribed for and send them via adb shell on API 33, and it worked fine with NOT_EXPORTED.

The event with all of the broadcast breadcrumbs that we listen to now can be found here: https://sentry-sdks.sentry.io/issues/5990712373/events/dc048d7441d7496f99f09b1c38c56f7e/

💡 Motivation and Context

Closes #2395
Closes #1774

Internal customer reports. Looks like when we're registered for too many broadcasts, the OS will kill the app process silently if it can't deliver a broadcast, so we're trying to reduce the likelihood of that by reducing the number of broadcasts we're listening to, just keeping the essential ones.

See here: https://cs.android.com/android/platform/superproject/+/android14-qpr3-release:frameworks/base/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java;l=1166-1167?q=broadcastqueuemod

💚 How did you test it?

Manually (on API 33+) and automated

📝 Checklist

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

We may need to add docs around SystemEventsIntegration to show off how to add more actions. For now I'm mentioning it in the changelog

@romtsn romtsn changed the title Drop TempSesnorBreadcrumbIntegration Fix BroadcastReceivers Jan 15, 2025
Copy link
Contributor

github-actions bot commented Jan 15, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 435.81 ms 487.11 ms 51.30 ms
Size 1.70 MiB 2.36 MiB 670.24 KiB

Previous results on branch: rz/fix/broadcast-receivers

Startup times

Revision Plain With Sentry Diff
dcdd715 410.44 ms 473.58 ms 63.15 ms

App size

Revision Plain With Sentry Diff
dcdd715 1.70 MiB 2.36 MiB 670.13 KiB

@romtsn romtsn requested a review from kahest January 15, 2025 13:41
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@stefanosiano stefanosiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

romtsn and others added 2 commits January 16, 2025 21:44
Co-authored-by: Stefano <stefano.siano@sentry.io>
Co-authored-by: Stefano <stefano.siano@sentry.io>
@romtsn romtsn merged commit e509503 into 7.x.x Jan 16, 2025
29 of 30 checks passed
@romtsn romtsn deleted the rz/fix/broadcast-receivers branch January 16, 2025 21:38
markushi pushed a commit that referenced this pull request Jan 28, 2025
* Drop TempSesnorBreadcrumbIntegration

* Drop PhoneStateBreadcrumbsIntegration

* Reduce number of system events we're listening to and use RECEIVER_NOT_EXPORTED

* Format code

* Changelog

* Update CHANGELOG.md

Co-authored-by: Stefano <stefano.siano@sentry.io>

* Update CHANGELOG.md

Co-authored-by: Stefano <stefano.siano@sentry.io>

---------

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Stefano <stefano.siano@sentry.io>
markushi added a commit that referenced this pull request Jan 30, 2025
* Various fixes to instrumentations running on the main thread (#4051)

* Get rid of redundant requireNonNull

* Do not instrument Window.Callback multiple times

* Do not instrument FileIO if tracing is disabled

* Do not traverse children if a touch event is not within view groups bounds

* Add test for SentryFileOutputStream

* Fix test

* Fix test

* Changelog

* pr id

* Fix api dump

* Fix BroadcastReceivers (#4052)

* Drop TempSesnorBreadcrumbIntegration

* Drop PhoneStateBreadcrumbsIntegration

* Reduce number of system events we're listening to and use RECEIVER_NOT_EXPORTED

* Format code

* Changelog

* Update CHANGELOG.md

Co-authored-by: Stefano <stefano.siano@sentry.io>

* Update CHANGELOG.md

Co-authored-by: Stefano <stefano.siano@sentry.io>

---------

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Stefano <stefano.siano@sentry.io>

* Only provide {{auto}} ip-address if sendDefaultPii is enabled

* Update changelog

* Reduce the number of IPC calls (#4058)

* Remove binder call for external storage

* Remove binder call for memory in profiler

* Cache static values to avoid binder calls

* Comment

* Changelog

* Formatting

* Fix tests

* Minor fixes

* change protected method in final class to private

---------

Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io>
Co-authored-by: stefanosiano <stefano.siano@sentry.io>

* Update Changelog

* Fix tests

---------

Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Co-authored-by: Stefano <stefano.siano@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants