-
Notifications
You must be signed in to change notification settings - Fork 27
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
App lifecycle logging & log persistence #286
Conversation
RadarSDK/RadarFileSystem.m
Outdated
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.
think RadarFileStorage
is probably better naming than RadarFileSystem
RadarSDK/RadarLogBuffer.m
Outdated
} | ||
} | ||
|
||
//for use in testing |
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.
Comments describing functions follow the format:
/**
* Return copy of replays in buffer
*/
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.
lgtm! let's sync with @tjulien during mobile meeting before shipping
also needs rebase & version bump
RadarSDK/RadarLogBuffer.m
Outdated
[self write:level type:type message:message forcePersist:NO]; | ||
} | ||
|
||
- (void)write:(RadarLogLevel)level type:(RadarLogType)type message:(NSString *)message forcePersist:(BOOL)forcePersist{ |
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.
need space before {
RadarSDK/RadarLogBuffer.m
Outdated
[self writeToFileStorage:flushableLogs]; | ||
[self purgeOldestLogs]; | ||
[inMemoryLogBuffer removeAllObjects]; | ||
if([inMemoryLogBuffer count] > 0) { |
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.
spacing between if
and (
https://expo.dev/accounts/radarlabs/projects/waypoint/builds/674c5efa-1ff1-4de9-beb1-61622404631e In addition to the change of names, we have also decided to not have expose the logging methods via RN, this waypoint build reflects that. |
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.
well done, let's ship 🚢 💯
* comment out opened_app * change numbering on migration guides
This PR intends to replace the purely in-memory log buffer with a persistent buffer on disk.
RadarFileClass
is as simple wrapper for file I/O operations, it can delete files, read files, replace or append data in files.RadarLog
is extended to be serialized into string JSONs and back.RadarLogger
is extended to append time and battery information behind some logs.RadarLogBuffer
is modified and extended to write new logs a in memory buffer that is flushed into disk periodically and when the app is backgrounded. The logs on disk are then read and flushed to the dashboard, similar to the old memory buffer. We also allow for quick appending of logs which is needed when IOS limits the time and resources the SDK has (when backgrounding and terminating the app).QA:
Unit tests are included for
RadarFileClass
andRadarLogBuffer
Waypoint build: https://expo.dev/accounts/radarlabs/projects/waypoint/builds/168fb1ba-3517-4036-9fff-811e23f77a37
was able to flush logs as expected to dashboard: