Skip to content

Commit 75df8d5

Browse files
kikoveigakikoveiga
committed
RUM-10363: Apply suggested changes
1 parent de5c02b commit 75df8d5

File tree

23 files changed

+147
-114
lines changed

23 files changed

+147
-114
lines changed

‎dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ internal class DatadogCore(
457457
executorServiceFactory ?: CoreFeature.DEFAULT_FLUSHABLE_EXECUTOR_SERVICE_FACTORY
458458
coreFeature = CoreFeature(
459459
internalLogger,
460-
DefaultAppStartTimeProvider(),
460+
DefaultAppStartTimeProvider(timeProviderFactory = { timeProvider }),
461461
flushableExecutorServiceFactory,
462462
CoreFeature.DEFAULT_SCHEDULED_EXECUTOR_SERVICE_FACTORY
463463
)

‎dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/time/DefaultAppStartTimeProvider.kt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@ import android.os.Build
1111
import android.os.Process
1212
import android.os.SystemClock
1313
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
14-
import com.datadog.android.internal.time.DefaultTimeProvider
1514
import com.datadog.android.internal.time.TimeProvider
1615
import com.datadog.android.rum.DdRumContentProvider
1716
import java.util.concurrent.TimeUnit
1817
import kotlin.time.Duration.Companion.seconds
1918

2019
internal class DefaultAppStartTimeProvider(
21-
private val timeProvider: TimeProvider = DefaultTimeProvider(),
20+
private val timeProviderFactory: () -> TimeProvider,
2221
buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT
2322
) : AppStartTimeProvider {
2423

@@ -27,7 +26,7 @@ internal class DefaultAppStartTimeProvider(
2726
when {
2827
buildSdkVersionProvider.version >= Build.VERSION_CODES.N -> {
2928
val diffMs = SystemClock.elapsedRealtime() - Process.getStartElapsedRealtime()
30-
val result = timeProvider.getDeviceElapsedTimeNs() - TimeUnit.MILLISECONDS.toNanos(diffMs)
29+
val result = timeProviderFactory().getDeviceElapsedTimeNs() - TimeUnit.MILLISECONDS.toNanos(diffMs)
3130

3231
/**
3332
* Occasionally [Process.getStartElapsedRealtime] returns buggy values. We filter them and fall back
@@ -44,7 +43,7 @@ internal class DefaultAppStartTimeProvider(
4443
}
4544

4645
override val appUptimeNs: Long
47-
get() = timeProvider.getDeviceElapsedTimeNs() - appStartTimeNs
46+
get() = timeProviderFactory().getDeviceElapsedTimeNs() - appStartTimeNs
4847

4948
companion object {
5049
internal val PROCESS_START_TO_CP_START_DIFF_THRESHOLD_NS = 10.seconds.inWholeNanoseconds

‎dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/time/KronosTimeProvider.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,24 @@
77
package com.datadog.android.core.internal.time
88

99
import com.datadog.android.api.InternalLogger
10+
import com.datadog.android.internal.time.BaseTimeProvider
1011
import com.datadog.android.internal.time.TimeProvider
1112
import com.lyft.kronos.Clock
1213
import java.util.concurrent.TimeUnit
1314

1415
/**
1516
* A [TimeProvider] implementation that uses Kronos NTP for server time synchronization.
1617
*
17-
* Device timestamp and elapsed time are inherited from [TimeProvider] default implementations.
18+
* Device timestamp and elapsed time are inherited from [BaseTimeProvider].
1819
*/
1920
internal class KronosTimeProvider(
2021
private val clock: Clock,
2122
private val internalLogger: InternalLogger
22-
) : TimeProvider {
23+
) : BaseTimeProvider() {
2324

2425
override fun getServerTimestamp(): Long {
2526
return clock.safeGetCurrentTimeMs()
26-
.getOrElse { System.currentTimeMillis() }
27+
.getOrElse { getDeviceTimestamp() }
2728
}
2829

2930
override fun getServerOffsetMillis(): Long {

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import com.datadog.android.core.internal.privacy.ConsentProvider
2828
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
2929
import com.datadog.android.core.internal.user.MutableUserInfoProvider
3030
import com.datadog.android.core.thread.FlushableExecutorService
31+
import com.datadog.android.internal.tests.stub.StubTimeProvider
3132
import com.datadog.android.internal.time.TimeProvider
3233
import com.datadog.android.ndk.internal.NdkCrashHandler
3334
import com.datadog.android.privacy.TrackingConsent
@@ -39,7 +40,6 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension
3940
import com.datadog.tools.unit.extensions.config.TestConfiguration
4041
import com.datadog.tools.unit.forge.aThrowable
4142
import com.datadog.tools.unit.forge.exhaustiveAttributes
42-
import com.datadog.tools.unit.stub.StubTimeProvider
4343
import com.google.gson.JsonObject
4444
import fr.xgouchet.elmyr.Forge
4545
import fr.xgouchet.elmyr.annotation.AdvancedForgery

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/data/upload/RotatingDnsResolverTest.kt‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ internal class RotatingDnsResolverTest {
6060
fun `set up`(forge: Forge) {
6161
fakeInetAddresses = forge.aList { mock() }
6262

63-
whenever(mockTimeProvider.getDeviceElapsedTimeNs()) doReturn fakeElapsedTimeNs
64-
6563
testedDns = RotatingDnsResolver(mockDelegate, TEST_TTL_MS, mockTimeProvider)
6664
}
6765

@@ -84,7 +82,7 @@ internal class RotatingDnsResolverTest {
8482
val result = mutableListOf<InetAddress>()
8583

8684
// When
87-
fakeInetAddresses.forEach { _ ->
85+
repeat(fakeInetAddresses.size) {
8886
result.add(testedDns.lookup(fakeHostname).first())
8987
}
9088

@@ -134,15 +132,15 @@ internal class RotatingDnsResolverTest {
134132
// the real use case where we have a small number of addresses to rotate
135133
fakeInetAddresses = forge.aList(size = forge.anInt(min = 1, max = 3)) { mock() }
136134
whenever(mockDelegate.lookup(fakeHostname)) doReturn fakeInetAddresses
137-
138-
val fakeExpiredTime = fakeElapsedTimeNs + TEST_TTL_MS.inWholeNanoseconds
139-
whenever(mockTimeProvider.getDeviceElapsedTimeNs()).doReturn(fakeElapsedTimeNs, fakeExpiredTime)
135+
// just wait the TTL time to make sure all threads are concurrently accessing the lookup
136+
Thread.sleep(TEST_TTL_MS.inWholeMilliseconds)
140137

141138
var exceptionThrown: Exception? = null
142139

143140
// When
144141
List(100) {
145142
Thread {
143+
Thread.sleep(forge.aLong(min = 0, max = 100))
146144
try {
147145
testedDns.lookup(fakeHostname)
148146
} catch (e: Exception) {

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/MoveDataMigrationOperationTest.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,9 @@ internal class MoveDataMigrationOperationTest {
150150
}
151151

152152
@Test
153-
fun `M not wait for real delay W run() {move fails once, time provider mocked}`() {
153+
fun `M retry with 500ms delay W run() {move fails once}`() {
154154
// Given
155+
whenever(mockTimeProvider.getDeviceElapsedTimeNs()).thenAnswer { System.nanoTime() }
155156
whenever(mockFileMover.moveFiles(fakeFromDirectory, fakeToDirectory))
156157
.doReturn(false, true)
157158

@@ -162,7 +163,7 @@ internal class MoveDataMigrationOperationTest {
162163

163164
// Then
164165
verify(mockFileMover, times(2)).moveFiles(fakeFromDirectory, fakeToDirectory)
165-
assertThat(duration).isLessThan(100L)
166+
assertThat(duration).isBetween(500L, 550L)
166167
}
167168

168169
@Test
@@ -179,8 +180,9 @@ internal class MoveDataMigrationOperationTest {
179180
}
180181

181182
@Test
182-
fun `M not wait for real delay W run() {move always fails, time provider mocked}`() {
183+
fun `M retry with 500ms delay W run() {move always fails}`() {
183184
// Given
185+
whenever(mockTimeProvider.getDeviceElapsedTimeNs()).thenAnswer { System.nanoTime() }
184186
whenever(mockFileMover.moveFiles(fakeFromDirectory, fakeToDirectory))
185187
.doReturn(false)
186188

@@ -191,7 +193,7 @@ internal class MoveDataMigrationOperationTest {
191193

192194
// Then
193195
verify(mockFileMover, times(3)).moveFiles(fakeFromDirectory, fakeToDirectory)
194-
assertThat(duration).isLessThan(100L)
196+
assertThat(duration).isBetween(1000L, 1100L)
195197
}
196198

197199
companion object {

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/advanced/WipeDataMigrationOperationTest.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ internal class WipeDataMigrationOperationTest {
134134
}
135135

136136
@Test
137-
fun `M not wait for real delay W run() {move always fails, time provider mocked}`() {
137+
fun `M retry with 500ms delay W run() {move always fails}`() {
138138
// Given
139+
whenever(mockTimeProvider.getDeviceElapsedTimeNs()).thenAnswer { System.nanoTime() }
139140
whenever(mockFileMover.delete(fakeTargetDirectory))
140141
.doReturn(false)
141142

@@ -146,7 +147,7 @@ internal class WipeDataMigrationOperationTest {
146147

147148
// Then
148149
verify(mockFileMover, times(3)).delete(fakeTargetDirectory)
149-
assertThat(duration).isLessThan(100L)
150+
assertThat(duration).isBetween(1000L, 1100L)
150151
}
151152

152153
companion object {

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileOrchestratorTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import com.datadog.android.core.internal.metrics.MetricsDispatcher
1212
import com.datadog.android.core.internal.metrics.RemovalReason
1313
import com.datadog.android.core.internal.persistence.file.FileOrchestrator
1414
import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig
15+
import com.datadog.android.internal.tests.stub.StubTimeProvider
1516
import com.datadog.android.utils.forge.Configurator
1617
import com.datadog.android.utils.verifyLog
17-
import com.datadog.tools.unit.stub.StubTimeProvider
1818
import fr.xgouchet.elmyr.Forge
1919
import fr.xgouchet.elmyr.annotation.IntForgery
2020
import fr.xgouchet.elmyr.annotation.LongForgery

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/time/DefaultAppStartTimeProviderTest.kt‎

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import android.os.SystemClock
1212
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
1313
import com.datadog.android.internal.time.TimeProvider
1414
import com.datadog.android.rum.DdRumContentProvider
15+
import fr.xgouchet.elmyr.Forge
1516
import fr.xgouchet.elmyr.annotation.IntForgery
1617
import fr.xgouchet.elmyr.annotation.LongForgery
1718
import fr.xgouchet.elmyr.junit5.ForgeExtension
@@ -42,39 +43,64 @@ class DefaultAppStartTimeProviderTest {
4243
@IntForgery(min = Build.VERSION_CODES.N) apiVersion: Int,
4344
@LongForgery(min = 0L) fakeCurrentTimeNs: Long
4445
) {
45-
// Given
46+
// GIVEN
4647
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
4748
whenever(mockTimeProvider.getDeviceElapsedTimeNs()) doReturn fakeCurrentTimeNs
4849
val diffMs = SystemClock.elapsedRealtime() - Process.getStartElapsedRealtime()
4950
val expectedStartTimeNs = fakeCurrentTimeNs - TimeUnit.MILLISECONDS.toNanos(diffMs)
50-
51-
// When
52-
val testedAppStartTimeProvider = DefaultAppStartTimeProvider(
53-
mockTimeProvider,
51+
val testedProvider = DefaultAppStartTimeProvider(
52+
{ mockTimeProvider },
5453
mockBuildSdkVersionProvider
5554
)
56-
val providedStartTime = testedAppStartTimeProvider.appStartTimeNs
5755

58-
// Then
56+
// WHEN
57+
val providedStartTime = testedProvider.appStartTimeNs
58+
59+
// THEN
5960
assertThat(providedStartTime).isEqualTo(expectedStartTimeNs)
6061
}
6162

63+
@Test
64+
fun `M fall back to DdRumContentProvider W appStartTime { N+ getStartElapsedRealtime returns buggy value }`(
65+
@IntForgery(min = Build.VERSION_CODES.N) apiVersion: Int,
66+
@LongForgery(min = 0L) fakeCurrentTimeNs: Long,
67+
forge: Forge
68+
) {
69+
// GIVEN
70+
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
71+
whenever(mockTimeProvider.getDeviceElapsedTimeNs()) doReturn fakeCurrentTimeNs
72+
val diffMs = SystemClock.elapsedRealtime() - Process.getStartElapsedRealtime()
73+
val startTimeNs = fakeCurrentTimeNs - TimeUnit.MILLISECONDS.toNanos(diffMs)
74+
DdRumContentProvider.createTimeNs = startTimeNs +
75+
forge.aLong(min = DefaultAppStartTimeProvider.PROCESS_START_TO_CP_START_DIFF_THRESHOLD_NS + 1)
76+
val testedProvider = DefaultAppStartTimeProvider(
77+
{ mockTimeProvider },
78+
mockBuildSdkVersionProvider
79+
)
80+
81+
// WHEN
82+
val providedStartTime = testedProvider.appStartTimeNs
83+
84+
// THEN
85+
assertThat(providedStartTime).isEqualTo(DdRumContentProvider.createTimeNs)
86+
}
87+
6288
@Test
6389
fun `M return content provider load time W appStartTime { Legacy }`(
6490
@IntForgery(min = Build.VERSION_CODES.M, max = Build.VERSION_CODES.N) apiVersion: Int
6591
) {
66-
// Given
92+
// GIVEN
6793
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
6894
val startTimeNs = DdRumContentProvider.createTimeNs
69-
70-
// When
71-
val testedAppStartTimeProvider = DefaultAppStartTimeProvider(
72-
mockTimeProvider,
95+
val testedProvider = DefaultAppStartTimeProvider(
96+
{ mockTimeProvider },
7397
mockBuildSdkVersionProvider
7498
)
75-
val providedStartTime = testedAppStartTimeProvider.appStartTimeNs
7699

77-
// Then
100+
// WHEN
101+
val providedStartTime = testedProvider.appStartTimeNs
102+
103+
// THEN
78104
assertThat(providedStartTime).isEqualTo(startTimeNs)
79105
}
80106

@@ -95,7 +121,7 @@ class DefaultAppStartTimeProviderTest {
95121
.doReturn(fakeStartTimeNs + fakeUptimeNs)
96122

97123
val testedProvider = DefaultAppStartTimeProvider(
98-
mockTimeProvider,
124+
{ mockTimeProvider },
99125
mockBuildSdkVersionProvider
100126
)
101127

@@ -123,7 +149,7 @@ class DefaultAppStartTimeProviderTest {
123149
.doReturn(fakeStartTimeNs + 200L)
124150

125151
val testedProvider = DefaultAppStartTimeProvider(
126-
mockTimeProvider,
152+
{ mockTimeProvider },
127153
mockBuildSdkVersionProvider
128154
)
129155

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/error/internal/DatadogExceptionHandlerTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import com.datadog.android.core.internal.CoreFeature
2121
import com.datadog.android.core.internal.thread.waitToIdle
2222
import com.datadog.android.core.internal.utils.TAG_DATADOG_UPLOAD
2323
import com.datadog.android.core.internal.utils.UPLOAD_WORKER_NAME
24+
import com.datadog.android.internal.tests.stub.StubTimeProvider
2425
import com.datadog.android.internal.time.TimeProvider
2526
import com.datadog.android.internal.utils.loggableStackTrace
2627
import com.datadog.android.utils.config.ApplicationContextTestConfiguration
@@ -30,7 +31,6 @@ import com.datadog.tools.unit.annotations.TestConfigurationsProvider
3031
import com.datadog.tools.unit.extensions.TestConfigurationExtension
3132
import com.datadog.tools.unit.extensions.config.TestConfiguration
3233
import com.datadog.tools.unit.setStaticValue
33-
import com.datadog.tools.unit.stub.StubTimeProvider
3434
import fr.xgouchet.elmyr.Forge
3535
import fr.xgouchet.elmyr.annotation.Forgery
3636
import fr.xgouchet.elmyr.annotation.LongForgery

0 commit comments

Comments
 (0)