Skip to content

RUM-10770: Remove KeepAlive event in RUM #3079

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.

Sign up for GitHub

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

Open
ambushwork wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

RUM-10770: Remove KeepAlive event in RUM #3079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,7 @@ internal class RumFeature(

TELEMETRY_SESSION_REPLAY_SKIP_FRAME -> addSessionReplaySkippedFrame()
FLUSH_AND_STOP_MONITOR_MESSAGE_TYPE -> {
(GlobalRumMonitor.get(sdkCore) as? DatadogRumMonitor)?.let {
it.stopKeepAliveCallback()
it.drainExecutorService()
}
(GlobalRumMonitor.get(sdkCore) as? DatadogRumMonitor)?.drainExecutorService()
}

else -> {
Expand Down
4 changes: 0 additions & 4 deletions ...-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,6 @@ internal sealed class RumRawEvent {
override val eventTime: Time = Time()
) : RumRawEvent()

internal data class KeepAlive(
override val eventTime: Time = Time()
) : RumRawEvent()

internal data class ApplicationStarted(
override val eventTime: Time,
val applicationStartupNanos: Long
Expand Down
19 changes: 12 additions & 7 deletions ...-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewManagerScope.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ internal class RumViewManagerScope(
delegateToChildren(event, datadogContext, writeScope, writer)

if (event is RumRawEvent.StartView && !stopped) {
startForegroundView(event, datadogContext, writeScope, writer)
startForegroundView(event)
sendViewUpdateToChildren(event, datadogContext, writeScope, writer)
Comment on lines +111 to +112
Copy link
Member

@0xnm 0xnm Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this change will trigger zero view duration telemetry

dd-sdk-android/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt

Lines 1361 to 1371 in f7fffa7

sdkCore.internalLogger.log(
InternalLogger.Level.WARN,
listOf(
InternalLogger.Target.USER,
InternalLogger.Target.TELEMETRY
),
{ ZERO_DURATION_WARNING_MESSAGE.format(Locale.US, key.name) },
null,
false,
mapOf("view.name" to key.name)
)

This is because the time of the scope creation matches event for view update time. And then we will set view duration to 1ns, which may be very puzzling when exploring the telemetry.

Copy link
Member Author

@ambushwork ambushwork Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it the case already before the PR? here we try to align with the existing logic

Copy link
Member

@0xnm 0xnm Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, here is a minor change of behavior: before it was KeepAlive event which was used to send view update and which is created after the original event used to create view scope, so the duration was always positive. Now it is the same event as it was used to create view scope, so the duration is now zero for such initial view update.

Copy link
Member Author

@ambushwork ambushwork Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove KeepAlive event and call sendViewUpdate directly, indeed, the view duration becomes 1ns after the creation. But this is somehow aligned to the existing IOS logic after checking with IOS team, because if the view is the last view in the session and it doesn't have any sub events which can update it, we still need to report this view event, and we will use this duration as the time_spent.

Copy link
Member

@0xnm 0xnm Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If 0 -> 1ns is a legitimate view duration for such case you still have to deal with the logging I mentioned above, because it shouldn't log in this case (especially taking into account that it also logs to telemetry, it will blow up the telemetry volume).

lastStoppedViewTime?.let {
val gap = event.eventTime.nanoTime - it.nanoTime
if (gap in 1 until THREE_SECONDS_GAP_NS) {
Expand Down Expand Up @@ -176,6 +177,15 @@ internal class RumViewManagerScope(
childrenScopes.add(viewScope)
}

private fun sendViewUpdateToChildren(
event: RumRawEvent,
datadogContext: DatadogContext,
writeScope: EventWriteScope,
writer: DataWriter<Any>
) {
childrenScopes.forEach { it.sendViewUpdate(event, datadogContext, writeScope, writer) }
}
Comment on lines +180 to +187
Copy link
Member

@0xnm 0xnm Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: let's avoid having this method, because sendViewUpdate was made internal only for tests and we probably don't want to advertise calling this, the main entrypoint is still handleEvent. It is better to inline this at the call site.

Copy link
Member Author

@ambushwork ambushwork Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not only for tests right now, it is also called to update the view event when creating a new foreground view (before it was updated by keep alive).

Copy link
Member

@0xnm 0xnm Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still mainly for tests, because such usage in production code is a small workaround which shouldn't be used in general: there may be some logic between handleEvent and sending view update.


@WorkerThread
private fun delegateToChildren(
event: RumRawEvent,
Expand Down Expand Up @@ -262,10 +272,7 @@ internal class RumViewManagerScope(

@WorkerThread
private fun startForegroundView(
event: RumRawEvent.StartView,
datadogContext: DatadogContext,
writeScope: EventWriteScope,
writer: DataWriter<Any>
event: RumRawEvent.StartView
) {
val viewScope = RumViewScope.fromEvent(
parentScope = this,
Expand All @@ -289,7 +296,6 @@ internal class RumViewManagerScope(
)
applicationDisplayed = true
childrenScopes.add(viewScope)
viewScope.handleEvent(RumRawEvent.KeepAlive(), datadogContext, writeScope, writer)
viewChangedListener?.onViewChanged(
RumViewInfo(
key = event.key,
Expand Down Expand Up @@ -428,7 +434,6 @@ internal class RumViewManagerScope(

internal val silentOrphanEventTypes = arrayOf<Class<*>>(
RumRawEvent.ApplicationStarted::class.java,
RumRawEvent.KeepAlive::class.java,
RumRawEvent.ResetSession::class.java,
RumRawEvent.StopView::class.java,
RumRawEvent.ActionDropped::class.java,
Expand Down
16 changes: 1 addition & 15 deletions ...android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ internal open class RumViewScope(
)

is RumRawEvent.AddCustomTiming -> onAddCustomTiming(event, datadogContext, writeScope, writer)
is RumRawEvent.KeepAlive -> onKeepAlive(event, datadogContext, writeScope, writer)

is RumRawEvent.StopSession -> onStopSession(event, datadogContext, writeScope, writer)

Expand Down Expand Up @@ -898,19 +897,6 @@ internal open class RumViewScope(
stopScope(event, datadogContext, writeScope, writer)
}

@WorkerThread
private fun onKeepAlive(
event: RumRawEvent.KeepAlive,
datadogContext: DatadogContext,
writeScope: EventWriteScope,
writer: DataWriter<Any>
) {
delegateEventToChildren(event, datadogContext, writeScope, writer)
if (stopped) return

sendViewUpdate(event, datadogContext, writeScope, writer)
}

@WorkerThread
private fun delegateEventToChildren(
event: RumRawEvent,
Expand Down Expand Up @@ -1111,7 +1097,7 @@ internal open class RumViewScope(
}

@Suppress("LongMethod", "ComplexMethod")
private fun sendViewUpdate(
internal fun sendViewUpdate(
event: RumRawEvent,
datadogContext: DatadogContext,
writeScope: EventWriteScope,
Expand Down
15 changes: 0 additions & 15 deletions ...android-rum/src/main/kotlin/com/datadog/android/rum/internal/monitor/DatadogRumMonitor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,10 @@ internal class DatadogRumMonitor(
rumSessionScopeStartupManagerFactory = rumSessionScopeStartupManagerFactory
)

internal val keepAliveRunnable = Runnable {
handleEvent(RumRawEvent.KeepAlive())
}

internal var debugListener: RumDebugListener? = null

private val internalProxy = _RumInternalProxy(this)

init {
handler.postDelayed(keepAliveRunnable, KEEP_ALIVE_MS)
}

private val globalAttributes: MutableMap<String, Any?> = ConcurrentHashMap()

private val isDebugEnabled = AtomicBoolean(false)
Expand Down Expand Up @@ -804,7 +796,6 @@ internal class DatadogRumMonitor(
} else if (event is RumRawEvent.TelemetryEventWrapper) {
telemetryEventHandler.handleEvent(event, writer)
} else {
handler.removeCallbacks(keepAliveRunnable)
sdkCore.getFeature(Feature.RUM_FEATURE_NAME)
?.withWriteContext(
withFeatureContexts = setOf(Feature.SESSION_REPLAY_FEATURE_NAME)
Expand All @@ -822,7 +813,6 @@ internal class DatadogRumMonitor(
handleEventWithMethodCallPerf(event, datadogContext, writeScope)
notifyDebugListenerWithState()
}
handler.postDelayed(keepAliveRunnable, KEEP_ALIVE_MS)
currentRumContext()
}
)
Expand Down Expand Up @@ -885,10 +875,6 @@ internal class DatadogRumMonitor(
return context
}

internal fun stopKeepAliveCallback() {
handler.removeCallbacks(keepAliveRunnable)
}

internal fun notifyDebugListenerWithState() {
debugListener?.let {
val sessionScope = rootScope.activeSession
Expand Down Expand Up @@ -928,7 +914,6 @@ internal class DatadogRumMonitor(
// endregion

companion object {
internal val KEEP_ALIVE_MS = TimeUnit.MINUTES.toMillis(5)

// should be aligned with CoreFeature#DRAIN_WAIT_SECONDS, but not a requirement
internal const val DRAIN_WAIT_SECONDS = 10L
Expand Down
1 change: 0 additions & 1 deletion ...droid-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEventExt.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ internal fun Forge.silentOrphanEvent(): RumRawEvent {
listOf(
RumRawEvent.ApplicationStarted(Time(), aLong()),
RumRawEvent.ResetSession(),
RumRawEvent.KeepAlive(),
RumRawEvent.StopView(getForgery(), emptyMap()),
RumRawEvent.ActionSent(
fakeId,
Expand Down
89 changes: 18 additions & 71 deletions ...lin/com/datadog/android/rum/internal/domain/scope/RumViewScopeAttributePropagationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
import com.datadog.android.rum.RumAttributes
import com.datadog.android.rum.RumErrorSource
import com.datadog.android.rum.RumSessionType
import com.datadog.android.rum.assertj.ActionEventAssert.Companion.assertThat
import com.datadog.android.rum.assertj.ErrorEventAssert.Companion.assertThat
import com.datadog.android.rum.assertj.LongTaskEventAssert.Companion.assertThat
import com.datadog.android.rum.assertj.ViewEventAssert.Companion.assertThat
Expand Down Expand Up @@ -263,68 +262,6 @@ internal class RumViewScopeAttributePropagationTest {

// region Propagate parent attributes in View Event

@Test
fun `M send event with parent attributes W handleEvent(KeepAlive) on active view`() {
// Given
val expectedAttributes = mutableMapOf<String, Any?>()
expectedAttributes.putAll(fakeParentAttributes)
testedScope = newRumViewScope(initialAttributes = emptyMap())

// When
val result =
testedScope.handleEvent(RumRawEvent.KeepAlive(), fakeDatadogContext, mockEventWriteScope, mockWriter)

// Then
argumentCaptor<ViewEvent> {
verify(mockWriter).write(eq(mockEventBatchWriter), capture(), eq(EventType.DEFAULT))
assertThat(lastValue)
.containsExactlyContextAttributes(expectedAttributes)
}
assertThat(result).isNotNull()
}

@Test
fun `M send event with both parent and view attributes W handleEvent(KeepAlive) on active view`() {
// Given
val expectedAttributes = mutableMapOf<String, Any?>()
expectedAttributes.putAll(fakeParentAttributes)
expectedAttributes.putAll(fakeViewAttributes)
testedScope = newRumViewScope(initialAttributes = fakeViewAttributes)

// When
val result =
testedScope.handleEvent(RumRawEvent.KeepAlive(), fakeDatadogContext, mockEventWriteScope, mockWriter)

// Then
argumentCaptor<ViewEvent> {
verify(mockWriter).write(eq(mockEventBatchWriter), capture(), eq(EventType.DEFAULT))
assertThat(lastValue)
.containsExactlyContextAttributes(expectedAttributes)
}
assertThat(result).isNotNull()
}

@Test
fun `M send event with overridden parent attributes W handleEvent(KeepAlive) on active view`(
forge: Forge
) {
// Given
val overriddenAttributes = fakeParentAttributes.map { it.key to forge.aString() }.toMap()
testedScope = newRumViewScope(initialAttributes = overriddenAttributes)

// When
val result =
testedScope.handleEvent(RumRawEvent.KeepAlive(), fakeDatadogContext, mockEventWriteScope, mockWriter)

// Then
argumentCaptor<ViewEvent> {
verify(mockWriter).write(eq(mockEventBatchWriter), capture(), eq(EventType.DEFAULT))
assertThat(lastValue)
.containsExactlyContextAttributes(overriddenAttributes)
}
assertThat(result).isNotNull()
}

@Test
fun `M send event with original parent attributes W handleEvent(StopView+ErrorSent) on active view`(
@StringForgery fakeAttrKey: String,
Expand Down Expand Up @@ -360,22 +297,27 @@ internal class RumViewScopeAttributePropagationTest {
}

@Test
fun `M send event with added view attributes W handleEvent(AddViewAttributes+KeepAlive) on active view`() {
fun `M send event with added view attributes W handleEvent(AddViewAttributes+StopView) on active view`() {
// Given
val expectedAttributes = mutableMapOf<String, Any?>()
expectedAttributes.putAll(fakeParentAttributes)
expectedAttributes.putAll(fakeViewAttributes)
testedScope = newRumViewScope(initialAttributes = emptyMap())

// When
testedScope.handleEvent(
val result = testedScope.handleEvent(
RumRawEvent.AddViewAttributes(fakeViewAttributes),
fakeDatadogContext,
mockEventWriteScope,
mockWriter
)
val result =
testedScope.handleEvent(RumRawEvent.KeepAlive(), fakeDatadogContext, mockEventWriteScope, mockWriter)

testedScope.handleEvent(
RumRawEvent.StopView(testedScope.key, emptyMap()),
fakeDatadogContext,
mockEventWriteScope,
mockWriter
)

// Then
argumentCaptor<ViewEvent> {
Expand All @@ -387,7 +329,7 @@ internal class RumViewScopeAttributePropagationTest {
}

@Test
fun `M send event without removed view attributes W handleEvent(AddViewAttributes+KeepAlive) on active view`(
fun `M send event without removed view attributes W handleEvent(AddViewAttributes+StopView) on active view`(
@MapForgery(
key = AdvancedForgery(string = [StringForgery(StringForgeryType.ALPHABETICAL)]),
value = AdvancedForgery(string = [StringForgery()])
Expand All @@ -399,14 +341,19 @@ internal class RumViewScopeAttributePropagationTest {
testedScope = newRumViewScope(initialAttributes = fakeViewAttributes)

// When
testedScope.handleEvent(
val result = testedScope.handleEvent(
RumRawEvent.RemoveViewAttributes(fakeViewAttributes.keys),
fakeDatadogContext,
mockEventWriteScope,
mockWriter
)
val result =
testedScope.handleEvent(RumRawEvent.KeepAlive(), fakeDatadogContext, mockEventWriteScope, mockWriter)

testedScope.handleEvent(
RumRawEvent.StopView(testedScope.key, emptyMap()),
fakeDatadogContext,
mockEventWriteScope,
mockWriter
)

// Then
argumentCaptor<ViewEvent> {
Expand Down
Loading
Loading