[RUM-8654] Slow frames collection support by satween · Pull Request #2518 · DataDog/dd-sdk-android

satween · 2025-02-28T16:23:34Z

What does this PR do?

Adds a SlowFrames listener that collects occurrences of jank frames during a view's lifetime.
This PR introduces the underlying logic and corresponding tests but does not yet integrate the functionality. The remaining integration will be completed in future tickets.

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)

ambushwork · 2025-02-28T16:52:25Z

+        currentViewStartedTimeStampNs = startedTimestampNs
+    }
+
+    override fun resolveReport(viewId: String): ViewUIPerformanceData {


This function name is not as indicative enough as onViewCreated to explain where it should be called. (especially you haven't call it yet in this PR). should it be called when the rum view scope ends?

this approach follows common one,
in InteractionToNextViewMetricResolver and NetworkSettledMetricResolver

How do you think we should call it?

I see, because I didn't find any call site of this function (because you haven't finish the whole feature yet), I am trying to understand where it should be called.

codecov-commenter · 2025-02-28T18:50:23Z

Codecov Report

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

Project coverage is 70.21%. Comparing base (4c23077) to head (f942dd6).
Report is 24 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../android/rum/internal/domain/scope/RumViewScope.kt	64.71%	0 Missing and 6 partials ⚠️
...m/internal/domain/state/ViewUIPerformanceReport.kt	91.30%	1 Missing and 1 partial ⚠️
...adog/android/internal/collections/EvictingQueue.kt	95.45%	0 Missing and 1 partial ⚠️
...lin/com/datadog/android/rum/internal/RumFeature.kt	97.50%	0 Missing and 1 partial ⚠️
...droid/rum/internal/domain/state/SlowFrameRecord.kt	80.00%	1 Missing ⚠️
...m/internal/metric/slowframes/SlowFramesListener.kt	97.37%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2518      +/-   ##
===========================================
+ Coverage    70.01%   70.21%   +0.20%     
===========================================
  Files          798      803       +5     
  Lines        30047    30196     +149     
  Branches      5029     5059      +30     
===========================================
+ Hits         21035    21200     +165     
+ Misses        7614     7598      -16     
  Partials      1398     1398

Files with missing lines	Coverage Δ
...rum/src/main/kotlin/com/datadog/android/rum/Rum.kt	`87.72% <100.00%> (+0.22%)`	⬆️
...kotlin/com/datadog/android/rum/RumConfiguration.kt	`97.18% <100.00%> (+0.08%)`	⬆️
...droid/rum/configuration/SlowFramesConfiguration.kt	`100.00% <100.00%> (ø)`
...og/android/rum/internal/domain/FrameMetricsData.kt	`100.00% <100.00%> (ø)`
...d/rum/internal/domain/scope/RumApplicationScope.kt	`95.15% <100.00%> (+0.15%)`	⬆️
...droid/rum/internal/domain/scope/RumSessionScope.kt	`96.72% <100.00%> (+0.03%)`	⬆️
...d/rum/internal/domain/scope/RumViewManagerScope.kt	`91.41% <100.00%> (+0.18%)`	⬆️
.../android/rum/internal/monitor/DatadogRumMonitor.kt	`85.31% <100.00%> (-0.27%)`	⬇️
...droid/rum/internal/vitals/FrameStatesAggregator.kt	`71.13% <100.00%> (ø)`
...adog/android/internal/collections/EvictingQueue.kt	`95.45% <95.45%> (ø)`
... and 5 more

... 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.

0xnm

I went through the production code so far and left some comments/questions. Didn't check test code yet.

ambushwork · 2025-03-03T12:26:10Z

Question: in the description it says part of the implementation has not been done yet, should this PR targets to a feature branch instead of develop branch then?

satween · 2025-03-07T13:32:34Z

Question: in the description it says part of the implementation has not been done yet, should this PR targets to a feature branch instead of develop branch then?

I am gonna merge this PR after new version release, but there is no core changes so should be fine

0xnm

nice work! I left some questions, but nothing blocking.

0xnm · 2025-03-12T13:59:07Z

+
+    fun slowFramesRate(viewEndedTimeStamp: Long): Double = when {
+        viewEndedTimeStamp - viewStartedTimeStamp <= minViewLifetimeThresholdNs -> 0.0
+        totalFramesDurationNs > 0.0 -> slowFramesDurationNs.toDouble() / totalFramesDurationNs


to me it is not a rate, but ratio. Are we sure we are using a correct terminology?

Gonna update event schema first, and then change this in a serate ticket

This is a good point.
We followed the naming used by Apple here:

hitch rate

hang rate

0xnm

I tentatively approve this PR, but we really need to decide on the lingvo rate vs ratio before the next release. The chosen terminology should be:

easily understandable by the customers
common in the industry (ideally the same wording used by the Play Store as well)

satween requested review from a team as code owners February 28, 2025 16:23

ambushwork reviewed Feb 28, 2025

View reviewed changes

ambushwork previously approved these changes Mar 3, 2025

View reviewed changes

0xnm reviewed Mar 3, 2025

View reviewed changes

simaoseica-dd reviewed Mar 6, 2025

View reviewed changes

satween dismissed ambushwork’s stale review via acf3ed2 March 7, 2025 13:31

satween force-pushed the tvaleev/rum-8654 branch from ad51807 to acf3ed2 Compare March 7, 2025 13:31

satween requested a review from 0xnm March 7, 2025 15:19

satween mentioned this pull request Mar 7, 2025

[RUM-8630]: Rates calculation #2532

Merged

3 tasks

0xnm reviewed Mar 7, 2025

View reviewed changes

satween force-pushed the tvaleev/rum-8654 branch from ec95a18 to b2ae399 Compare March 10, 2025 11:37

0xnm previously approved these changes Mar 10, 2025

View reviewed changes

satween dismissed 0xnm’s stale review via 7e4029e March 10, 2025 14:37

satween force-pushed the tvaleev/rum-8654 branch from 1e7d5f4 to 2778cc6 Compare March 12, 2025 12:57

0xnm reviewed Mar 12, 2025

View reviewed changes

satween force-pushed the tvaleev/rum-8654 branch from 32297c3 to d12bf4b Compare March 12, 2025 14:21

satween requested review from 0xnm and ambushwork March 12, 2025 16:08

0xnm previously approved these changes Mar 13, 2025

View reviewed changes

satween dismissed 0xnm’s stale review via 15449be March 13, 2025 11:07

satween force-pushed the tvaleev/rum-8654 branch from 15449be to 8612f3c Compare March 13, 2025 11:14

0xnm previously approved these changes Mar 13, 2025

View reviewed changes

satween dismissed 0xnm’s stale review via fc2e06f March 19, 2025 09:49

satween added 3 commits March 19, 2025 09:51

RUM-8654: supporting slow frames collection

cb9a417

RUM-8654: EvictingQueue support

3c166aa

RUM-8654: fix detekt

5c3f4b6

satween added 20 commits March 19, 2025 09:51

RUM-8654: post-review fixes

8d1ca49

RUM-8654: refactoring of EvictingQueue

a08aea8

RUM-8654: post-review fixes

35ce15b

RUM-8654: roll back debug code

4dfe847

RUM-8654: post-review fixes

abbe82d

RUM-8630: avoiding spikes in short views, support anr ratio

a6c8196

RUM-8630: fix apiSurface

9b0ec31

RUM-8630: test support

3826df2

RUM-8630: avoiding spikes in short views, support anr ratio

786d24a

RUM-8657: event schema support for ui slowness

b6749fc

RUM-8657: post-review fixes

d0b470c

RUM-8947: SlowFrameListener configuration in rum config support

acfca21

RUM-8947: fix detekt, tests, adding some logs

bac7727

RUM-8947: enabling slow frames monitoring in sample app

828d7c2

RUM-8654: post-review fixes

1f6c00b

RUM-9065: fix computation errors

01cc0d7

RUM-9065: detekt & fix api surface

ae48be9

RUM-9065: pos-review fixes

a12b3ca

RUM-9065: fix freeze rate and slow frames rate

f3c24f3

RUM-9065: post-review fixes

f942dd6

satween force-pushed the tvaleev/rum-8654 branch from fc2e06f to f942dd6 Compare March 19, 2025 10:08

0xnm approved these changes Mar 19, 2025

View reviewed changes

satween merged commit 344e805 into develop Mar 19, 2025

satween deleted the tvaleev/rum-8654 branch March 19, 2025 13:43

satween mentioned this pull request Mar 19, 2025

[RUM-9151]: fix issue with missing freeze rate and slow frames rate #2557

Merged

3 tasks

0xnm mentioned this pull request Apr 20, 2026

RUM-15742: Support setSlowFramesConfiguration API for Android DataDog/dd-sdk-kotlin-multiplatform#258

Merged

3 tasks

Uh oh!

Conversation

satween commented Feb 28, 2025

What does this PR do?

Review checklist (to be filled by reviewers)

Uh oh!

Uh oh!

ambushwork Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

satween Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

ambushwork Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-commenter commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ambushwork commented Mar 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

satween commented Mar 7, 2025

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xnm Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

satween Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

simaoseica-dd Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Feb 28, 2025 •

edited

Loading