Diagnostics: Add jsonl file management for diagnostics#784
Conversation
There was a problem hiding this comment.
Note that all these need to be synchronized. The idea is that all this code executes in a single background thread with a lower priority than other threads. To avoid race conditions, all these should run in that same thread.
e77f081 to
313ca01
Compare
Codecov Report
@@ Coverage Diff @@
## diagnostics #784 +/- ##
==============================================
Coverage ? 81.83%
==============================================
Files ? 125
Lines ? 4084
Branches ? 517
==============================================
Hits ? 3342
Misses ? 536
Partials ? 206 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
313ca01 to
b5e7f30
Compare
| /** | ||
| * All methods in this file should be executed within the diagnostics thread to ensure there are no threading issues. | ||
| */ | ||
| class DiagnosticsFileHelper( |
There was a problem hiding this comment.
it'd be so nice to be able to use actors here
There was a problem hiding this comment.
I think so yeah. And yeah, we don't have that in Android 😢
| appendToFile(filePath, textToAppend) | ||
| } | ||
|
|
||
| fun fileIsEmpty(filePath: String): Boolean { |
There was a problem hiding this comment.
Might be useful to document that this also returns true if the file doesn't exist.
There was a problem hiding this comment.
Sorry, merged this without pushing this. Added it to the diagnostics branch though: 74d6c21
| /** | ||
| * All methods in this file should be executed within the diagnostics thread to ensure there are no threading issues. | ||
| */ | ||
| class DiagnosticsFileHelper( |
| @Synchronized | ||
| fun deleteDiagnosticsFile() { | ||
| if (!fileHelper.deleteFile(DIAGNOSTICS_FILE_PATH)) { | ||
| verboseLog("Failed to delete diagnostics file.") |
There was a problem hiding this comment.
Should this be a warning instead?
There was a problem hiding this comment.
Yeah, I made all logs for diagnostics verbose, to not pollute the normal logs. Lmk if you think we should still send these as warnings/errors though, I'm not opposed to either approach.
|
|
||
| private lateinit var applicationContext: Context | ||
|
|
||
| private lateinit var fileHelper: FileHelper |
There was a problem hiding this comment.
I'd pay money to have lateinit in Swift
Based off #2672 Equivalent to RevenueCat/purchases-android#784 Adds DiagnosticsEntry classes and implements adding an entry to the file, getting all entries, and deleting X first entries
Description
Based on #783, this PR adds the logic to store and read JSONL files with the events we want to send to the backend.
This has been extracted from #766 and #771