-
Notifications
You must be signed in to change notification settings - Fork 10
android: avoid cloning previous global fields on each log #810
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
Conversation
Size Comparison Report (x86_64)
|
2f33b7a to
ccb8e1b
Compare
Android Benchmark Results
Allocations
Timing
|
kattrali
left a comment
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.
tidy ✨ 🧹
| logger.log(LogLevel.INFO, null, null) { "test log" } | ||
|
|
||
| var nextLogFound = false | ||
| while (!nextLogFound) { |
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.
optional nit: you could remove this to prevent potential infinite loop and use generateSequence instead
val testLog = generateSequence { CaptureTestJniLibrary.nextUploadedLog() }
.take(10)
.first { it.message == "test log" }
assertThat(testLog.sessionId).isEqualTo("bar")
assertThat(testLog.fields["app_version"]).isEqualTo(FieldValue.StringField(newAppVersion))
assertThat(testLog.fields["_app_version_code"]).isEqualTo(FieldValue.StringField(newAppVersionCode.toString()))
Address PR comment to prevent potential infinite loop and improve code readability.
The UIKit framework is not used in this file and causes build failures when building for macOS. This file only uses Foundation types.
In order to support global fields being persisted on the AppExit log emitted by Android on next app start we were overriding the logs for each log with the global state fields from the previous app run. This instead relies on new behavior in shared core to apply the old values to the logs that specify the previous process run session ID, which avoids the extra allocations in the common case
Also enhances the integration test to verify that we are correctly using the state fields from the process run that crashed and not the reporting one.
Depends on bitdriftlabs/shared-core#357
CHANGELOG.md's "Unreleased" section has been updated, if applicable.