Skip to content
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

RUM-2910: Report time since the application start for crashes in RUM #1961

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ interface com.datadog.android.core.InternalSdkCore : com.datadog.android.api.fea
val firstPartyHostResolver: com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
val lastViewEvent: com.google.gson.JsonObject?
val lastFatalAnrSent: Long?
val appStartTimeNs: Long
fun writeLastViewEvent(ByteArray)
fun deleteLastViewEvent()
fun writeLastFatalAnrSent(Long)
Expand Down
1 change: 1 addition & 0 deletions dd-sdk-android-core/api/dd-sdk-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ public final class com/datadog/android/api/storage/RawBatchEvent {
public abstract interface class com/datadog/android/core/InternalSdkCore : com/datadog/android/api/feature/FeatureSdkCore {
public abstract fun deleteLastViewEvent ()V
public abstract fun getAllFeatures ()Ljava/util/List;
public abstract fun getAppStartTimeNs ()J
public abstract fun getDatadogContext ()Lcom/datadog/android/api/context/DatadogContext;
public abstract fun getFirstPartyHostResolver ()Lcom/datadog/android/core/internal/net/FirstPartyHostHeaderTypeResolver;
public abstract fun getLastFatalAnrSent ()Ljava/lang/Long;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import com.datadog.android.core.internal.lifecycle.ProcessLifecycleCallback
import com.datadog.android.core.internal.lifecycle.ProcessLifecycleMonitor
import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
import com.datadog.android.core.internal.time.DefaultAppStartTimeProvider
import com.datadog.android.core.internal.utils.scheduleSafe
import com.datadog.android.core.thread.FlushableExecutorService
import com.datadog.android.error.internal.CrashReportsFeature
Expand Down Expand Up @@ -288,6 +289,9 @@ internal class DatadogCore(
override val lastFatalAnrSent: Long?
get() = coreFeature.lastFatalAnrSent

override val appStartTimeNs: Long
get() = coreFeature.appStartTimeNs

@WorkerThread
override fun writeLastViewEvent(data: ByteArray) {
// we need to write it only if we are going to read ApplicationExitInfo (available on
Expand Down Expand Up @@ -351,6 +355,7 @@ internal class DatadogCore(
executorServiceFactory ?: CoreFeature.DEFAULT_FLUSHABLE_EXECUTOR_SERVICE_FACTORY
coreFeature = CoreFeature(
internalLogger,
DefaultAppStartTimeProvider(),
flushableExecutorServiceFactory,
CoreFeature.DEFAULT_SCHEDULED_EXECUTOR_SERVICE_FACTORY
)
Expand Down Expand Up @@ -575,5 +580,8 @@ internal class DatadogCore(
" crash reports feature is not enabled and API is below 30."

internal val CONFIGURATION_TELEMETRY_DELAY_MS = TimeUnit.SECONDS.toMillis(5)

// fallback for APIs below Android N, see [DefaultAppStartTimeProvider].
internal val startupTimeNs: Long = System.nanoTime()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,15 @@ interface InternalSdkCore : FeatureSdkCore {
@InternalApi
val lastFatalAnrSent: Long?

/**
* Provide the time the application started in nanoseconds from device boot, or our best guess
* if the actual start time is not available. Note: since the implementation may rely on [System.nanoTime],
* this property can only be used to measure elapsed time and is not related to any other notion of system
* or wall-clock time. The value is the time since VM start.
*/
@InternalApi
val appStartTimeNs: Long

/**
* Writes current RUM view event to the dedicated file for the needs of NDK crash reporting.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ internal object NoOpInternalSdkCore : InternalSdkCore {
get() = null
override val lastFatalAnrSent: Long?
get() = null
override val appStartTimeNs: Long
get() = 0

// endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import com.datadog.android.core.internal.system.SystemInfoProvider
import com.datadog.android.core.internal.thread.BackPressureExecutorService
import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
import com.datadog.android.core.internal.thread.ScheduledExecutorServiceFactory
import com.datadog.android.core.internal.time.AppStartTimeProvider
import com.datadog.android.core.internal.time.DatadogNtpEndpoint
import com.datadog.android.core.internal.time.KronosTimeProvider
import com.datadog.android.core.internal.time.LoggingSyncListener
Expand Down Expand Up @@ -102,8 +103,9 @@ import java.util.concurrent.atomic.AtomicBoolean
@Suppress("TooManyFunctions")
internal class CoreFeature(
private val internalLogger: InternalLogger,
val executorServiceFactory: FlushableExecutorService.Factory,
val scheduledExecutorServiceFactory: ScheduledExecutorServiceFactory
private val appStartTimeProvider: AppStartTimeProvider,
private val executorServiceFactory: FlushableExecutorService.Factory,
private val scheduledExecutorServiceFactory: ScheduledExecutorServiceFactory
) {

internal val initialized = AtomicBoolean(false)
Expand Down Expand Up @@ -147,6 +149,9 @@ internal class CoreFeature(

internal val featuresContext: MutableMap<String, Map<String, Any?>> = ConcurrentHashMap()

internal val appStartTimeNs: Long
get() = appStartTimeProvider.appStartTimeNs

// lazy here on purpose: we need to read it only once, even if it is used in different features
@get:WorkerThread
internal val lastViewEvent: JsonObject? by lazy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.rum.internal
package com.datadog.android.core.internal.time

internal interface AppStartTimeProvider {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.rum.internal
package com.datadog.android.core.internal.time

import android.annotation.SuppressLint
import android.os.Build
import android.os.Process
import android.os.SystemClock
import com.datadog.android.core.DatadogCore
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
import java.util.concurrent.TimeUnit

Expand All @@ -24,7 +25,7 @@ internal class DefaultAppStartTimeProvider(
val diffMs = SystemClock.elapsedRealtime() - Process.getStartElapsedRealtime()
System.nanoTime() - TimeUnit.MILLISECONDS.toNanos(diffMs)
}
else -> RumFeature.startupTimeNs
else -> DatadogCore.startupTimeNs
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ internal class DatadogNdkCrashHandler(
"type" to "ndk_crash",
"sourceType" to nativeCrashSourceType,
"timestamp" to ndkCrashLog.timestamp,
"timeSinceAppStartMs" to ndkCrashLog.timeSinceAppStartMs,
"signalName" to ndkCrashLog.signalName,
"stacktrace" to ndkCrashLog.stacktrace,
"message" to errorLogMessage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.datadog.android.ndk.internal

import com.google.gson.JsonNull
import com.google.gson.JsonObject
import com.google.gson.JsonParseException
import com.google.gson.JsonParser
Expand All @@ -14,6 +15,7 @@ import java.lang.IllegalStateException
internal data class NdkCrashLog(
val signal: Int,
val timestamp: Long,
val timeSinceAppStartMs: Long?,
val signalName: String,
val message: String,
val stacktrace: String
Expand All @@ -24,6 +26,7 @@ internal data class NdkCrashLog(
jsonObject.addProperty(SIGNAL_KEY_NAME, signal)
jsonObject.addProperty(SIGNAL_NAME_KEY_NAME, signalName)
jsonObject.addProperty(TIMESTAMP_KEY_NAME, timestamp)
jsonObject.addProperty(TIME_SINCE_APP_START_MS_NAME, timeSinceAppStartMs)
jsonObject.addProperty(MESSAGE_KEY_NAME, message)
jsonObject.addProperty(STACKTRACE_KEY_NAME, stacktrace)
return jsonObject.toString()
Expand All @@ -33,6 +36,7 @@ internal data class NdkCrashLog(

internal const val SIGNAL_KEY_NAME = "signal"
internal const val TIMESTAMP_KEY_NAME = "timestamp"
internal const val TIME_SINCE_APP_START_MS_NAME = "time_since_app_start_ms"
internal const val MESSAGE_KEY_NAME = "message"
internal const val SIGNAL_NAME_KEY_NAME = "signal_name"
internal const val STACKTRACE_KEY_NAME = "stacktrace"
Expand All @@ -44,6 +48,8 @@ internal data class NdkCrashLog(
return NdkCrashLog(
jsonObject.get(SIGNAL_KEY_NAME).asInt,
jsonObject.get(TIMESTAMP_KEY_NAME).asLong,
jsonObject.get(TIME_SINCE_APP_START_MS_NAME)
?.let { if (it is JsonNull) null else it.asLong },
jsonObject.get(SIGNAL_NAME_KEY_NAME).asString,
jsonObject.get(MESSAGE_KEY_NAME).asString,
jsonObject.get(STACKTRACE_KEY_NAME).asString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,21 @@ internal class DatadogCoreTest {
assertThat(lastFatalAnrSent).isEqualTo(fakeLastFatalAnrSent)
}

@Test
fun `𝕄 provide app start time 𝕎 appStartTimeNs()`(
@LongForgery(min = 0L) fakeAppStartTimeNs: Long
) {
// Given
testedCore.coreFeature = mock()
whenever(testedCore.coreFeature.appStartTimeNs) doReturn fakeAppStartTimeNs

// When
val appStartTimeNs = testedCore.appStartTimeNs

// Then
assertThat(appStartTimeNs).isEqualTo(fakeAppStartTimeNs)
}

@Test
fun `𝕄 return tracking consent 𝕎 trackingConsent()`(
@Forgery fakeTrackingConsent: TrackingConsent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.datadog.android.core.internal.privacy.NoOpConsentProvider
import com.datadog.android.core.internal.privacy.TrackingConsentProvider
import com.datadog.android.core.internal.system.BroadcastReceiverSystemInfoProvider
import com.datadog.android.core.internal.system.NoOpSystemInfoProvider
import com.datadog.android.core.internal.time.AppStartTimeProvider
import com.datadog.android.core.internal.time.KronosTimeProvider
import com.datadog.android.core.internal.time.NoOpTimeProvider
import com.datadog.android.core.internal.user.DatadogUserInfoProvider
Expand Down Expand Up @@ -117,6 +118,9 @@ internal class CoreFeatureTest {
@Mock
lateinit var mockScheduledExecutorService: ScheduledExecutorService

@Mock
lateinit var mockAppStartTimeProvider: AppStartTimeProvider

@Forgery
lateinit var fakeConfig: Configuration

Expand All @@ -131,6 +135,7 @@ internal class CoreFeatureTest {
CoreFeature.disableKronosBackgroundSync = true
testedFeature = CoreFeature(
mockInternalLogger,
mockAppStartTimeProvider,
executorServiceFactory = { _, _ -> mockPersistenceExecutorService },
scheduledExecutorServiceFactory = { _, _ -> mockScheduledExecutorService }
)
Expand Down Expand Up @@ -1107,6 +1112,20 @@ internal class CoreFeatureTest {
assertThat(lastViewEvent.toString()).isEqualTo(fakeViewEvent.toString())
}

@Test
fun `𝕄 return app start time 𝕎 appStartTimeNs`(
@LongForgery(min = 0L) fakeAppStartTimeNs: Long
) {
// Given
whenever(mockAppStartTimeProvider.appStartTimeNs) doReturn fakeAppStartTimeNs

// When
val appStartTimeNs = testedFeature.appStartTimeNs

// Then
assertThat(appStartTimeNs).isEqualTo(fakeAppStartTimeNs)
}

// region shutdown

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.android.rum.internal
package com.datadog.android.core.internal.time

import android.os.Build
import android.os.Process
import android.os.SystemClock
import com.datadog.android.core.DatadogCore
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
import fr.xgouchet.elmyr.annotation.IntForgery
import fr.xgouchet.elmyr.junit5.ForgeExtension
Expand Down Expand Up @@ -52,7 +53,7 @@ class DefaultAppStartTimeProviderTest {
// GIVEN
val mockBuildSdkVersionProvider: BuildSdkVersionProvider = mock()
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
val startTimeNs = RumFeature.startupTimeNs
val startTimeNs = DatadogCore.startupTimeNs

// WHEN
val timeProvider = DefaultAppStartTimeProvider(mockBuildSdkVersionProvider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import java.util.concurrent.ExecutorService
@ForgeConfiguration(Configurator::class)
internal class DatadogNdkCrashHandlerTest {

lateinit var testedHandler: DatadogNdkCrashHandler
private lateinit var testedHandler: DatadogNdkCrashHandler

@Mock
lateinit var mockExecutorService: ExecutorService
Expand Down Expand Up @@ -527,6 +527,7 @@ internal class DatadogNdkCrashHandlerTest {
"type" to "ndk_crash",
"sourceType" to "ndk",
"timestamp" to ndkCrashLog.timestamp,
"timeSinceAppStartMs" to ndkCrashLog.timeSinceAppStartMs,
"signalName" to ndkCrashLog.signalName,
"stacktrace" to ndkCrashLog.stacktrace,
"message" to DatadogNdkCrashHandler.LOG_CRASH_MSG
Expand Down Expand Up @@ -570,6 +571,7 @@ internal class DatadogNdkCrashHandlerTest {
"type" to "ndk_crash",
"sourceType" to "ndk+il2cpp",
"timestamp" to ndkCrashLog.timestamp,
"timeSinceAppStartMs" to ndkCrashLog.timeSinceAppStartMs,
"signalName" to ndkCrashLog.signalName,
"stacktrace" to ndkCrashLog.stacktrace,
"message" to DatadogNdkCrashHandler.LOG_CRASH_MSG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.mockito.quality.Strictness
@ForgeConfiguration(Configurator::class)
internal class NdkCrashLogDeserializerTest {

lateinit var testedDeserializer: NdkCrashLogDeserializer
private lateinit var testedDeserializer: NdkCrashLogDeserializer

@Mock
lateinit var mockInternalLogger: InternalLogger
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ internal class NdkCrashLogForgeryFactory :
ForgeryFactory<NdkCrashLog> {
override fun getForgery(forge: Forge): NdkCrashLog {
return NdkCrashLog(
forge.anInt(min = 1),
System.currentTimeMillis(),
forge.anAlphabeticalString(),
forge.anAlphabeticalString(),
forge.anAlphabeticalString()
signal = forge.anInt(min = 1),
timestamp = System.currentTimeMillis(),
timeSinceAppStartMs = forge.aNullable { aPositiveLong() },
signalName = forge.anAlphabeticalString(),
message = forge.anAlphabeticalString(),
stacktrace = forge.anAlphabeticalString()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ class NdkTests {
val fakeErrorStack = forge.anAlphabeticalString()
initNdkErrorHandler(temporaryFolder.root.absolutePath)
updateTrackingConsent(1)
val fakeAppStartTimeMs = forge.aLong(min = 0L, max = System.currentTimeMillis())
updateAppStartTime(fakeAppStartTimeMs)

val expectedTimestamp = System.currentTimeMillis()
val expectedTimeSinceAppStartMs = expectedTimestamp - fakeAppStartTimeMs
simulateSignalInterception(
fakeSignal,
fakeSignalName,
fakeErrorMessage,
fakeErrorStack
)

val expectedTimestamp = System.currentTimeMillis()

// we need to give time to native part to write the file
// otherwise we will get into race condition issues
ConditionWatcher {
Expand All @@ -84,7 +87,12 @@ class NdkTests {
assertThat(jsonObject).hasField(
"timestamp",
expectedTimestamp,
Offset.offset(TimeUnit.SECONDS.toMillis(10))
Offset.offset(TimeUnit.SECONDS.toMillis(2))
)
assertThat(jsonObject).hasField(
"time_since_app_start_ms",
expectedTimeSinceAppStartMs,
Offset.offset(TimeUnit.SECONDS.toMillis(2))
)
}
true
Expand Down Expand Up @@ -222,5 +230,9 @@ class NdkTests {
consent: Int
)

private external fun updateAppStartTime(
appStartTimeMs: Long
)

// endregion
}