Skip to content

RUM-9017: Ignore secondary displays in SR#2574

Merged
jonathanmos merged 1 commit into
developfrom
jmoskovich/rum-9017/handle-secondary-displays
Apr 1, 2025
Merged

RUM-9017: Ignore secondary displays in SR#2574
jonathanmos merged 1 commit into
developfrom
jmoskovich/rum-9017/handle-secondary-displays

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Mar 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

This pr handles secondary displays overlapping in Session Replay sessions. The previous behavior was that views on the secondary display would be recorded overlaying the views on the primary display which led to a bad experience. In the solution provided here any view that is not on the primary display (Display.DEFAULT_DISPLAY) will be ignored.

Also added to Configuration telemetry info on how frequently multiple displays are being used.

Motivation

#2536

Additional Notes

Anything else we should know when reviewing?

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)

@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-9017/handle-secondary-displays branch 2 times, most recently from 2442c80 to 09eacfa Compare March 31, 2025 15:47
@codecov-commenter

codecov-commenter commented Mar 31, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 53.84615% with 12 lines in your changes missing coverage. Please review.

Project coverage is 70.13%. Comparing base (f3f9c52) to head (b6e8d99).
Report is 218 commits behind head on develop.

Files with missing lines Patch % Lines
...tadog/android/core/internal/NoOpContextProvider.kt 0.00% 11 Missing ⚠️
...id/core/internal/system/NoOpAndroidInfoProvider.kt 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2574      +/-   ##
===========================================
- Coverage    70.15%   70.13%   -0.03%     
===========================================
  Files          804      804              
  Lines        30196    30217      +21     
  Branches      5050     5051       +1     
===========================================
+ Hits         21184    21191       +7     
- Misses        7608     7620      +12     
- Partials      1404     1406       +2
Files with missing lines Coverage Δ
...tlin/com/datadog/android/api/context/DeviceInfo.kt 100.00% <100.00%> (ø)
...og/android/core/internal/DatadogContextProvider.kt 100.00% <100.00%> (ø)
...core/internal/system/DefaultAndroidInfoProvider.kt 83.93% <100.00%> (+2.30%) ⬆️
...ndroid/telemetry/internal/TelemetryEventHandler.kt 88.80% <ø> (+1.19%) ⬆️
...ssionreplay/internal/recorder/TreeViewTraversal.kt 68.35% <100.00%> (+0.41%) ⬆️
...ssionreplay/internal/recorder/ViewUtilsInternal.kt 100.00% <100.00%> (ø)
...id/core/internal/system/NoOpAndroidInfoProvider.kt 90.91% <0.00%> (-9.09%) ⬇️
...tadog/android/core/internal/NoOpContextProvider.kt 2.50% <0.00%> (-0.83%) ⬇️

... and 29 files with indirect coverage changes

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

@jonathanmos
jonathanmos marked this pull request as ready for review April 1, 2025 07:06
@jonathanmos
jonathanmos requested review from a team as code owners April 1, 2025 07:06
ambushwork
ambushwork previously approved these changes Apr 1, 2025
View reviewed changes

@ambushwork ambushwork left a comment

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.

LGTM!

0xnm
0xnm previously approved these changes Apr 1, 2025
View reviewed changes
Comment thread dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/context/DeviceInfo.kt Outdated
Comment thread ...e/src/test/kotlin/com/datadog/android/core/internal/system/DefaultAndroidInfoProviderTest.kt Outdated
@jonathanmos
jonathanmos dismissed stale reviews from 0xnm and ambushwork via d39f894 April 1, 2025 07:50
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-9017/handle-secondary-displays branch from 09eacfa to d39f894 Compare April 1, 2025 07:50
0xnm
0xnm previously approved these changes Apr 1, 2025
View reviewed changes
Comment thread ...e/src/test/kotlin/com/datadog/android/core/internal/system/DefaultAndroidInfoProviderTest.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-9017/handle-secondary-displays branch from d39f894 to b6e8d99 Compare April 1, 2025 08:35
@jonathanmos
jonathanmos merged commit bfceeac into develop Apr 1, 2025
@jonathanmos
jonathanmos deleted the jmoskovich/rum-9017/handle-secondary-displays branch April 1, 2025 10:42
@0xnm 0xnm mentioned this pull request Apr 4, 2025
Closed
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
@ambushwork ambushwork ambushwork 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.

4 participants

@jonathanmos @codecov-commenter @0xnm @ambushwork