Skip to content

Commit

Permalink
Merge pull request #2439 from DataDog/mconstantin/rum-7590/fix-the-wa…
Browse files Browse the repository at this point in the history 
…y-we-record-the-initial-resource

RUM-7590 Correct the way we register the initial resources for TTNS metric
  • Loading branch information
@mariusc83
mariusc83 authored Dec 9, 2024
2 parents 426cd07 + 199dec9 commit 558855a
Show file tree
Hide file tree
Showing 14 changed files with 247 additions and 95 deletions.
3 changes: 3 additions & 0 deletions ...-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEvent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ internal sealed class RumRawEvent {

internal data class ResourceSent(
val viewId: String,
val resourceId: String,
val resourceEndTimestampInNanos: Long,
override val eventTime: Time = Time()
) : RumRawEvent()

Expand All @@ -138,6 +140,7 @@ internal sealed class RumRawEvent {

internal data class ResourceDropped(
val viewId: String,
val resourceId: String,
override val eventTime: Time = Time()
) : RumRawEvent()

Expand Down
5 changes: 2 additions & 3 deletions ...oid-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumResourceScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ internal class RumResourceScope(
eventTime: Time,
writer: DataWriter<Any>
) {
reportResourceStoppedMetric(eventTime.nanoTime)
attributes.putAll(GlobalRumMonitor.get(sdkCore).getAttributes())
val traceId = attributes.remove(RumAttributes.TRACE_ID)?.toString()
val spanId = attributes.remove(RumAttributes.SPAN_ID)?.toString()
Expand Down Expand Up @@ -303,10 +302,10 @@ internal class RumResourceScope(
)
}
.onError {
it.eventDropped(rumContext.viewId.orEmpty(), StorageEvent.Resource)
it.eventDropped(rumContext.viewId.orEmpty(), StorageEvent.Resource(resourceId, eventTime.nanoTime))
}
.onSuccess {
it.eventSent(rumContext.viewId.orEmpty(), StorageEvent.Resource)
it.eventSent(rumContext.viewId.orEmpty(), StorageEvent.Resource(resourceId, eventTime.nanoTime))
}
.submit()

Expand Down
8 changes: 8 additions & 0 deletions ...android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.datadog.android.rum.internal.domain.Time
import com.datadog.android.rum.internal.metric.SessionMetricDispatcher
import com.datadog.android.rum.internal.metric.interactiontonextview.InteractionToNextViewMetricResolver
import com.datadog.android.rum.internal.metric.interactiontonextview.InternalInteractionContext
import com.datadog.android.rum.internal.metric.networksettled.InternalResourceContext
import com.datadog.android.rum.internal.metric.networksettled.NetworkSettledMetricResolver
import com.datadog.android.rum.internal.monitor.StorageEvent
import com.datadog.android.rum.internal.utils.hasUserData
Expand Down Expand Up @@ -719,6 +720,12 @@ internal open class RumViewScope(
if (event.viewId == viewId || event.viewId in oldViewIds) {
pendingResourceCount--
resourceCount++
networkSettledMetricResolver.resourceWasStopped(
InternalResourceContext(
event.resourceId,
event.resourceEndTimestampInNanos
)
)
sendViewUpdate(event, writer)
}
}
Expand Down Expand Up @@ -773,6 +780,7 @@ internal open class RumViewScope(

private fun onResourceDropped(event: RumRawEvent.ResourceDropped) {
if (event.viewId == viewId || event.viewId in oldViewIds) {
networkSettledMetricResolver.resourceWasDropped(event.resourceId)
pendingResourceCount--
}
}
Expand Down
11 changes: 11 additions & 0 deletions ...in/com/datadog/android/rum/internal/metric/networksettled/NetworkSettledMetricResolver.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package com.datadog.android.rum.internal.metric.networksettled

import androidx.annotation.VisibleForTesting
import com.datadog.android.api.InternalLogger
import com.datadog.android.rum.metric.networksettled.InitialResourceIdentifier
import com.datadog.android.rum.metric.networksettled.NetworkSettledResourceContext
Expand Down Expand Up @@ -63,6 +64,11 @@ internal class NetworkSettledMetricResolver(
}
}

fun resourceWasDropped(resourceId: String) {
if (viewWasStopped) return
resourceStartedTimestamps.remove(resourceId)
}

fun viewWasStopped() {
viewWasStopped = true
// clear all the resources for this view
Expand All @@ -77,6 +83,11 @@ internal class NetworkSettledMetricResolver(
return lastComputedMetric
}

@VisibleForTesting
fun getResourceStartedCacheSize(): Int {
return resourceStartedTimestamps.size
}

@Suppress("ReturnCount")
private fun computeMetric(): Long? {
if (viewCreatedTimestamp == null) {
Expand Down
11 changes: 9 additions & 2 deletions ...android-rum/src/main/kotlin/com/datadog/android/rum/internal/monitor/DatadogRumMonitor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,14 @@ internal class DatadogRumMonitor(
)
)

is StorageEvent.Resource -> handleEvent(RumRawEvent.ResourceSent(viewId))
is StorageEvent.Resource -> handleEvent(
RumRawEvent.ResourceSent(
viewId,
event.resourceId,
event.resourceStopTimestampInNanos
)
)

is StorageEvent.Error -> handleEvent(RumRawEvent.ErrorSent(viewId))
is StorageEvent.LongTask -> handleEvent(RumRawEvent.LongTaskSent(viewId, false))
is StorageEvent.FrozenFrame -> handleEvent(RumRawEvent.LongTaskSent(viewId, true))
Expand All @@ -579,7 +586,7 @@ internal class DatadogRumMonitor(
override fun eventDropped(viewId: String, event: StorageEvent) {
when (event) {
is StorageEvent.Action -> handleEvent(RumRawEvent.ActionDropped(viewId))
is StorageEvent.Resource -> handleEvent(RumRawEvent.ResourceDropped(viewId))
is StorageEvent.Resource -> handleEvent(RumRawEvent.ResourceDropped(viewId, event.resourceId))
is StorageEvent.Error -> handleEvent(RumRawEvent.ErrorDropped(viewId))
is StorageEvent.LongTask -> handleEvent(RumRawEvent.LongTaskDropped(viewId, false))
is StorageEvent.FrozenFrame -> handleEvent(RumRawEvent.LongTaskDropped(viewId, true))
Expand Down
3 changes: 1 addition & 2 deletions ...-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/monitor/StorageEvent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ internal sealed class StorageEvent {
val type: ActionEvent.ActionEventActionType,
val eventEndTimestampInNanos: Long
) : StorageEvent()

object Resource : StorageEvent()
data class Resource(val resourceId: String, val resourceStopTimestampInNanos: Long) : StorageEvent()
object Error : StorageEvent()
object LongTask : StorageEvent()
object FrozenFrame : StorageEvent()
Expand Down
4 changes: 2 additions & 2 deletions ...droid-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEventExt.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,11 @@ internal fun Forge.silentOrphanEvent(): RumRawEvent {
),
RumRawEvent.ErrorSent(fakeId),
RumRawEvent.LongTaskSent(fakeId),
RumRawEvent.ResourceSent(fakeId),
RumRawEvent.ResourceSent(fakeId, getForgery<UUID>().toString(), aPositiveLong()),
RumRawEvent.ActionDropped(fakeId),
RumRawEvent.ErrorDropped(fakeId),
RumRawEvent.LongTaskDropped(fakeId),
RumRawEvent.ResourceDropped(fakeId)
RumRawEvent.ResourceDropped(fakeId, getForgery<UUID>().toString())
)
)
}
63 changes: 12 additions & 51 deletions ...rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumResourceScopeTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -387,9 +384,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -475,9 +469,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -553,9 +544,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -626,9 +614,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -718,9 +703,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -782,9 +764,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -837,9 +816,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand All @@ -861,9 +837,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -970,9 +943,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -1044,9 +1014,6 @@ internal class RumResourceScopeTest {
}
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -1120,9 +1087,6 @@ internal class RumResourceScopeTest {
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
assertThat(resultTiming).isEqualTo(testedScope)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -1196,9 +1160,6 @@ internal class RumResourceScopeTest {
verify(mockParentScope, never()).handleEvent(any(), any())
verifyNoMoreInteractions(mockWriter)
assertThat(resultTiming).isEqualTo(testedScope)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(result).isEqualTo(null)
}

Expand Down Expand Up @@ -2499,9 +2460,6 @@ internal class RumResourceScopeTest {
}
verifyNoMoreInteractions(mockWriter)
verify(mockParentScope, never()).handleEvent(any(), any())
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(resultWaitForTiming).isSameAs(testedScope)
assertThat(resultStop).isEqualTo(null)
}
Expand Down Expand Up @@ -2571,9 +2529,6 @@ internal class RumResourceScopeTest {
verify(mockParentScope, never()).handleEvent(any(), any())
assertThat(resultWaitForTiming).isEqualTo(testedScope)
assertThat(resultTiming).isEqualTo(testedScope)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(resultStop).isEqualTo(null)
}

Expand Down Expand Up @@ -2642,9 +2597,6 @@ internal class RumResourceScopeTest {
verifyNoMoreInteractions(mockWriter)
verify(mockParentScope, never()).handleEvent(any(), any())
assertThat(resultWaitForTiming).isEqualTo(testedScope)
verify(mockNetworkSettledMetricResolver).resourceWasStopped(
InternalResourceContext(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
assertThat(resultStop).isEqualTo(testedScope)
assertThat(resultTiming).isEqualTo(null)
}
Expand Down Expand Up @@ -2887,7 +2839,10 @@ internal class RumResourceScopeTest {

// Then
verify(rumMonitor.mockInstance as AdvancedRumMonitor)
.eventSent(fakeParentContext.viewId.orEmpty(), StorageEvent.Resource)
.eventSent(
fakeParentContext.viewId.orEmpty(),
StorageEvent.Resource(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
}

@Test
Expand All @@ -2908,7 +2863,10 @@ internal class RumResourceScopeTest {

// Then
verify(rumMonitor.mockInstance as AdvancedRumMonitor)
.eventDropped(fakeParentContext.viewId.orEmpty(), StorageEvent.Resource)
.eventDropped(
fakeParentContext.viewId.orEmpty(),
StorageEvent.Resource(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
}

@Test
Expand All @@ -2931,7 +2889,10 @@ internal class RumResourceScopeTest {

// Then
verify(rumMonitor.mockInstance as AdvancedRumMonitor)
.eventDropped(fakeParentContext.viewId.orEmpty(), StorageEvent.Resource)
.eventDropped(
fakeParentContext.viewId.orEmpty(),
StorageEvent.Resource(testedScope.resourceId, mockEvent.eventTime.nanoTime)
)
}

@Test
Expand Down
Loading

0 comments on commit 558855a

Please sign in to comment.