Skip to content

RUM-10770: Remove KeepAlive event in RUM#3079

Merged
ambushwork merged 1 commit into
developfrom
yl/remove-keep-alive-event
Feb 10, 2026
Merged

RUM-10770: Remove KeepAlive event in RUM#3079
ambushwork merged 1 commit into
developfrom
yl/remove-keep-alive-event

Conversation

@ambushwork

@ambushwork ambushwork commented Dec 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Main changes for this PR:

  • Remove KeepAlive in RumRawEvent
  • Remove keep alive callback in DatadogRumMonitor

Due to the removal of KeepAlive event, there are some side-effect changes:

  • RumViewScope.sendViewUpdate becomes internal so RumViewManagerScope can call it directly to update the view.
  • startForegroundView used to dispatch a keep alive to update the view, now we call sendViewUpdate directly for the same purpose
  • Many unit tests relied on KeepAlive event to flush the Rum Event into the writer, now we use either StopView to flush them, either we call sendViewUpdate directly from RumViewManagerScope

Motivation

RUM-10770

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@datadog-official

This comment has been minimized.

Sign in to view
@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch from b9e9c18 to 1b2198c Compare December 29, 2025 15:12
@codecov-commenter

codecov-commenter commented Dec 29, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.97%. Comparing base (19c6623) to head (5d56b82).
⚠️ Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...lin/com/datadog/android/rum/internal/RumFeature.kt 0.00% 1 Missing ⚠️
.../android/rum/internal/domain/scope/RumViewScope.kt 66.67% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3079      +/-   ##
===========================================
- Coverage    71.03%   70.97%   -0.06%     
===========================================
  Files          912      912              
  Lines        33557    33543      -14     
  Branches      5642     5642              
===========================================
- Hits         23836    23807      -29     
- Misses        8154     8159       +5     
- Partials      1567     1577      +10
Files with missing lines Coverage Δ
...g/android/rum/internal/domain/scope/RumRawEvent.kt 99.44% <ø> (-0.01%) ⬇️
...d/rum/internal/domain/scope/RumViewManagerScope.kt 93.15% <100.00%> (ø)
.../android/rum/internal/monitor/DatadogRumMonitor.kt 88.28% <ø> (-0.47%) ⬇️
...lin/com/datadog/android/rum/internal/RumFeature.kt 90.14% <0.00%> (+0.19%) ⬆️
.../android/rum/internal/domain/scope/RumViewScope.kt 94.23% <66.67%> (-0.01%) ⬇️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch 3 times, most recently from f3b731b to 0e4edd9 Compare December 30, 2025 13:40
@ambushwork
ambushwork marked this pull request as ready for review December 31, 2025 09:28
@ambushwork
ambushwork requested a review from a team as a code owner December 31, 2025 09:28
jonathanmos
jonathanmos previously approved these changes Dec 31, 2025
View reviewed changes
@ambushwork

ambushwork commented Jan 2, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jan 2, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch from 0e4edd9 to f37cdb1 Compare February 6, 2026 15:12
@ambushwork
ambushwork requested a review from 0xnm February 6, 2026 15:15
@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch from f37cdb1 to 8765266 Compare February 9, 2026 12:36
0xnm
0xnm reviewed Feb 9, 2026
View reviewed changes
Comment thread ...dk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt Outdated
Comment on lines +1355 to +1358
if (type == RumViewType.BACKGROUND &&
event is RumRawEvent.AddError &&
event.isFatal || event is RumRawEvent.StartView
) {

@0xnm 0xnm Feb 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor: it is worth to extract complex condition instead of suppressing a warning, because order of the validation here is not obvious(is it (a && b && c) || d or a && b && (c || d)?):

Suggested change
if (type == RumViewType.BACKGROUND &&
event is RumRawEvent.AddError &&
event.isFatal || event is RumRawEvent.StartView
) {
val isBackgroundFatalError = type == RumViewType.BACKGROUND &&
event is RumRawEvent.AddError &&
event.isFatal
if (isBackgroundFatalError || event is RumRawEvent.StartView) {

Also it is worth adding a test to verify that we indeed don't have logging/telemetry for such case

@ambushwork ambushwork Feb 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

moved part of the condition into a dedicated function

@ambushwork ambushwork Feb 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

a unit test is also added to verify that when the event type is a background fatal error, we don't send any telemetry for 1ns duration

@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch 2 times, most recently from 5ec9ab7 to a42dda9 Compare February 10, 2026 10:04
0xnm
0xnm previously approved these changes Feb 10, 2026
View reviewed changes
Comment thread ...dk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt Outdated
Comment thread ...dk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt Outdated
@ambushwork
ambushwork dismissed stale reviews from aleksandr-gringauz and 0xnm via 5d56b82 February 10, 2026 14:43
@ambushwork
ambushwork force-pushed the yl/remove-keep-alive-event branch from a42dda9 to 5d56b82 Compare February 10, 2026 14:43
@ambushwork
ambushwork merged commit e7ff4e1 into develop Feb 10, 2026
27 checks passed
@ambushwork
ambushwork deleted the yl/remove-keep-alive-event branch February 10, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xnm 0xnm 0xnm approved these changes
@aleksandr-gringauz aleksandr-gringauz aleksandr-gringauz approved these changes
@jonathanmos jonathanmos jonathanmos left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ambushwork @codecov-commenter @0xnm @jonathanmos @aleksandr-gringauz