-
Notifications
You must be signed in to change notification settings - Fork 135
[Deps] Update automattic-encryptedlogging to 1.1.1
#14711
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
[Deps] Update automattic-encryptedlogging to 1.1.1
#14711
Conversation
FYI: This is done because we've identified a regression where the actual encrypted log, although seen as sent, is not actually ending-up being uploaded into our systems. Thus, when someone then tries to browse it, based on it UUID, they will get: "Log File Not Found.Invalid Log File" For more info see: p1759832867149029-slack-CGPNUU63E PS: The PR that introduced this regression is #14407, which brought-in some app logs improvements.
Release Notes: https://github.com/Automattic/EncryptedLogging/releases/ tag/1.1.1 FYI: This update resolves a long standing OutOfMemory (OOM) error when using this library alongside logs that are of large size.
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.
Pull Request Overview
This PR updates the automattic-encryptedlogging dependency from version 1.0.0 to 1.1.1 to resolve OutOfMemory (OOM) errors when handling large log files.
- Updated dependency version in gradle configuration
- Refactored coroutine usage to use
runBlockinginstead of launching in app coroutine scope - Removed unnecessary dependency injection for
AppCoroutineScope
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| gradle/libs.versions.toml | Updates automattic-encryptedlogging version from 1.0.0 to 1.1.1 |
| WooCommerce/src/main/kotlin/com/woocommerce/android/util/crashlogging/EnqueueSendingEncryptedLogs.kt | Refactors coroutine handling and removes AppCoroutineScope dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...rce/src/main/kotlin/com/woocommerce/android/util/crashlogging/EnqueueSendingEncryptedLogs.kt
Outdated
Show resolved
Hide resolved
...rce/src/main/kotlin/com/woocommerce/android/util/crashlogging/EnqueueSendingEncryptedLogs.kt
Show resolved
Hide resolved
Project dependencies changeslist! Upgraded Dependencies
com.automattic:encryptedlogging:1.1.1, (changed from 1.0.0)tree-\--- com.automattic:encryptedlogging:1.0.0
+\--- com.automattic:encryptedlogging:1.1.1 |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
FYI: Because of this fix: 9892c31
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #14711 +/- ##
============================================
- Coverage 38.37% 38.37% -0.01%
+ Complexity 9855 9854 -1
============================================
Files 2098 2098
Lines 117018 117024 +6
Branches 15656 15655 -1
============================================
+ Hits 44904 44906 +2
- Misses 67949 67951 +2
- Partials 4165 4167 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
FYI: After some discussions we agreed on enqueuing the logs on the 'main' thread for 'FATAL' error, else do this off the 'main' thread.
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...rce/src/main/kotlin/com/woocommerce/android/util/crashlogging/EnqueueSendingEncryptedLogs.kt
Show resolved
Hide resolved
...rce/src/main/kotlin/com/woocommerce/android/util/crashlogging/EnqueueSendingEncryptedLogs.kt
Show resolved
Hide resolved
|
A friendly remind on this folks! 🙏 Cc @wzieba @hichamboushaba FYI: We are on a meetup next week, then sabbatical, so it would be great to merge this by the end of this week. 🤞 |
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.
Works as expected, nice work @ParaskP7, thanks for the additional improvements too 🙏
Just a FYI, I deleted the test crash reports in Sentry.
|
Awesome, thanks for much @hichamboushaba ! 🙇 ❤️ 🚀 |
Closes: AINFRA-1281
Description
This PR updates automattic-encryptedlogging to 1.1.1.
FYI: This update resolves a long standing
OutOfMemory(OOM) error when using this library alongside logs that are of large size.PS: This change also resolved a regression that was caused because of #14407, which brought-in some app logs improvements (9892c31). For more info see (internal discussion): p1759832867149029-slack-CGPNUU63E (See also: EL#29)
Testing information
App Inspectionfrom AS find theencrypted-log.dband its soleEncryptedLogEntitytable. Notice that the table is empty, there are no log to be sent.Menu->Settings->Privacy settingsand then switch-on theReport Crashesoption.Menu->Settings->Developer Optionsand then clickCrash The App.patch
App Inspectionfrom AS on thatDETACHEDapp (or not, it depends). Now, switch to theATTACHEDapp and notice the db empty. This is because this db entry/log has been already sent to Sentry already, on app start and after the app crashed. To really notice the entry, trying crashing the app again, this time put the device on offline mode first. Doing that you'll be able to see the entry on the db, even when switching to the newly createdATTACHEDapp.Issues. Findwoocommerce-androidand check the latestdebugexceptions. You could search forDeveloper caused a crash!and check the latest crash. Now navigate at the bottom and check theuuid. Verify it is the sameUUIDthat you saw on that db entry before.UUIDintoLog UUID, clickViewand make sure you can read the newly uploaded and now decrypted log.Logcatand notice/verify the below entries:Successfully uploaded encrypted log with uuid: <UUID>Failed to upload encrypted log with uuid: <UUID> [Reason: <ERROR>]Tip
FYI: If you want to be thorough about testing this, you could follow the "extra" testing instructions on EL#28. The testing instruction in there focus on how to replicate an OOM, filling the logs and asserting that this fix works no matter how big (in size) the to-be encrypted/uploaded log file ends-up being.
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.