-
Notifications
You must be signed in to change notification settings - Fork 376
WIP - Feat: OpenTelemetry crash reporting #2405
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
base: main
Are you sure you want to change the base?
Changes from all commits
e5b4e4f
483c8d1
3a60cef
04a620c
17dcf00
052ddfa
38ba6fb
fc0d830
f7ab8a0
a61e2bc
597b858
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ class ParamsObject( | |
| var opRepoExecutionInterval: Long? = null, | ||
| var influenceParams: InfluenceParamsObject, | ||
| var fcmParams: FCMParamsObject, | ||
| val remoteLoggingParams: RemoteLoggingParamsObject, | ||
| ) | ||
|
|
||
| class InfluenceParamsObject( | ||
|
|
@@ -53,3 +54,7 @@ class FCMParamsObject( | |
| val appId: String? = null, | ||
| val apiKey: String? = null, | ||
| ) | ||
|
|
||
| class RemoteLoggingParamsObject( | ||
| val enable: Boolean? = null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we default this to false instead of null? |
||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,14 @@ | ||
| package com.onesignal.core.internal.time.impl | ||
|
|
||
| import android.os.Build | ||
| import android.os.SystemClock | ||
| import androidx.annotation.RequiresApi | ||
| import com.onesignal.core.internal.time.ITime | ||
|
|
||
| internal class Time : ITime { | ||
| override val currentTimeMillis: Long | ||
| get() = System.currentTimeMillis() | ||
| override val processUptimeMillis: Long | ||
| @RequiresApi(Build.VERSION_CODES.N) | ||
| get() = SystemClock.uptimeMillis() - android.os.Process.getStartUptimeMillis() | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package com.onesignal.debug.internal.crash | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we place all the logging related code in its own module, just in case we go in the direction of KMP, it will be extremely easy to build an artifact out this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll make this a new module, that core depends on. We can switch the module to KMP later. |
||
|
|
||
| internal interface IOneSignalCrashReporter { | ||
| suspend fun saveCrash(thread: Thread, throwable: Throwable) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.onesignal.debug.internal.crash | ||
|
|
||
| import kotlinx.coroutines.runBlocking | ||
|
|
||
| /** | ||
| * Purpose: Writes any crashes involving OneSignal to a file where they can | ||
| * later be send to OneSignal to help improve reliability. | ||
| * NOTE: For future refactors, code is written assuming this is a singleton | ||
| */ | ||
| internal class OneSignalCrashHandler( | ||
| private val _crashReporter: IOneSignalCrashReporter, | ||
| ) : Thread.UncaughtExceptionHandler { | ||
| private var existingHandler: Thread.UncaughtExceptionHandler? = null | ||
| private val seenThrowables: MutableList<Throwable> = mutableListOf() | ||
|
|
||
| init { | ||
| existingHandler = Thread.getDefaultUncaughtExceptionHandler() | ||
| Thread.setDefaultUncaughtExceptionHandler(this) | ||
| } | ||
|
|
||
| override fun uncaughtException(thread: Thread, throwable: Throwable) { | ||
| // Ensure we never attempt to process the same throwable instance | ||
| // more than once. This would only happen if there was another crash | ||
| // handler and was faulty in a specific way. | ||
| synchronized(seenThrowables) { | ||
| if (seenThrowables.contains(throwable)) { | ||
| return | ||
| } | ||
| seenThrowables.add(throwable) | ||
| } | ||
|
|
||
| // TODO: Catch anything we may throw and print only to logcat | ||
| // TODO: Also send a stop command to OneSignalCrashUploader, | ||
| // give a bit of time to finish and then call existingHandler. | ||
| // * This way the app doesn't have to open a 2nd time to get the | ||
| // crash report and should help prevent duplicated reports. | ||
| if (!isOneSignalAtFault(throwable)) { | ||
| existingHandler?.uncaughtException(thread, throwable) | ||
| return | ||
| } | ||
|
|
||
| /** | ||
| * NOTE: The order and running sequentially is important as: | ||
| * The existingHandler.uncaughtException can immediately terminate the | ||
| * process, either directly (if this is Android's | ||
| * KillApplicationHandler) OR the app's handler / 3rd party SDK (either | ||
| * directly or more likely, by it calling Android's | ||
| * KillApplicationHandler). | ||
| * Given this, we can't parallelize the existingHandler work with ours. | ||
| * The safest thing is to try to finish our work as fast as possible | ||
| * (including ensuring our logging write buffers are flushed) then call | ||
| * the existingHandler so any crash handlers the app also has gets the | ||
| * crash even too. | ||
| * | ||
| * NOTE: addShutdownHook() isn't a workaround as it doesn't fire for | ||
| * Process.killProcess, which KillApplicationHandler calls. | ||
| */ | ||
| runBlocking { _crashReporter.saveCrash(thread, throwable) } | ||
| existingHandler?.uncaughtException(thread, throwable) | ||
| } | ||
| } | ||
|
|
||
| internal fun isOneSignalAtFault(throwable: Throwable): Boolean = | ||
jkasten2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| throwable.stackTrace.any { it.className.startsWith("com.onesignal") } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package com.onesignal.debug.internal.logging.otel | ||
|
|
||
| import io.opentelemetry.api.logs.LogRecordBuilder | ||
| import io.opentelemetry.sdk.common.CompletableResultCode | ||
| import io.opentelemetry.sdk.logs.export.LogRecordExporter | ||
|
|
||
| internal interface IOneSignalOpenTelemetry { | ||
| suspend fun getLogger(): LogRecordBuilder | ||
|
|
||
| suspend fun forceFlush(): CompletableResultCode | ||
| } | ||
|
|
||
| internal interface IOneSignalOpenTelemetryCrash : IOneSignalOpenTelemetry | ||
|
|
||
| internal interface IOneSignalOpenTelemetryRemote : IOneSignalOpenTelemetry { | ||
| val logExporter: LogRecordExporter | ||
| } |
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.
can we create variables for the versions numbers used here instead of hardcoding them here?