fix(frame-tracking): app lag when long running span finishes #2311

buenaflor · 2024-09-26T11:32:41Z

📜 Description

Basically when a long lasting span runs in the background (e.g 20 minutes) without ever finishing, the frame tracking keeps accumulating frames and thus takes a long time to process it.

💡 Motivation and Context

Fixes #2298

💚 How did you test it?

Unit test

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

🔮 Next steps

…imit

codecov · 2024-09-26T11:33:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.82%. Comparing base (a4c4f8c) to head (35a7001).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2311      +/-   ##
==========================================
+ Coverage   88.07%   90.82%   +2.74%     
==========================================
  Files         247       73     -174     
  Lines        8603     2386    -6217     
==========================================
- Hits         7577     2167    -5410     
+ Misses       1026      219     -807

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ueman · 2024-09-26T11:39:32Z

flutter/lib/src/sentry_flutter_options.dart

+  /// If the number of frames exceeds the maximum, the frame tracking is cancelled
+  /// and frame metrics are cleared for the current active spans.
+  /// Default value is 3 minutes.
+  int framesTrackingRetentionPeriod = 3;


Why don't you use Duration? That way you will not have any confusion regarding the unit:

Suggested change

int framesTrackingRetentionPeriod = 3;

Duration framesTrackingRetentionPeriod = Duration(minutes: 3);

Also, your calculations further down below will also be simpler :D

I think I'll just use a fixed number like 10800 as default, it's a pretty confusing option tbh

github-actions · 2024-09-26T12:00:29Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	453.51 ms	497.65 ms	44.14 ms
Size	6.49 MiB	7.56 MiB	1.07 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`7f14ddd`	428.38 ms	511.90 ms	83.51 ms
`e2d89fc`	323.84 ms	376.23 ms	52.39 ms
`b0811cc`	580.00 ms	675.69 ms	95.69 ms
`d089990`	361.67 ms	442.50 ms	80.83 ms
`7f75f32`	347.36 ms	419.58 ms	72.22 ms
`5e8d2b3`	342.17 ms	375.83 ms	33.66 ms
`55cc6ef`	300.98 ms	351.98 ms	51.00 ms
`559d28f`	302.35 ms	339.53 ms	37.18 ms
`d5fb969`	420.83 ms	490.88 ms	70.05 ms
`683fd34`	336.53 ms	418.10 ms	81.57 ms

App size

Revision	Plain	With Sentry	Diff
`7f14ddd`	6.35 MiB	7.40 MiB	1.05 MiB
`e2d89fc`	6.06 MiB	7.03 MiB	989.37 KiB
`b0811cc`	6.33 MiB	7.27 MiB	954.02 KiB
`d089990`	6.34 MiB	7.28 MiB	967.79 KiB
`7f75f32`	6.26 MiB	7.20 MiB	959.18 KiB
`5e8d2b3`	6.15 MiB	7.13 MiB	1000.11 KiB
`55cc6ef`	6.06 MiB	7.03 MiB	995.45 KiB
`559d28f`	5.94 MiB	6.92 MiB	1001.70 KiB
`d5fb969`	6.35 MiB	7.35 MiB	1021.16 KiB
`683fd34`	6.27 MiB	7.20 MiB	960.43 KiB

Previous results on branch: fix/app-lag-sff

Startup times

Revision	Plain	With Sentry	Diff
`6ddb24e`	457.34 ms	496.57 ms	39.23 ms
`45ab9fc`	459.51 ms	514.98 ms	55.47 ms
`b3716ec`	477.39 ms	558.53 ms	81.14 ms

App size

Revision	Plain	With Sentry	Diff
`6ddb24e`	6.49 MiB	7.56 MiB	1.07 MiB
`45ab9fc`	6.49 MiB	7.56 MiB	1.07 MiB
`b3716ec`	6.49 MiB	7.56 MiB	1.07 MiB

brustolin

One suggestion, but not blocking

brustolin · 2024-09-26T12:18:08Z

flutter/lib/src/sentry_flutter_options.dart

+  /// Maximum number of frames to track at a time before clearing.
+  /// Clearing cancels frame tracking for all active spans.
+  /// Default is 10800 frames, this equals to 3 minutes of frames at 60fps.
+  int maxFramesToTrack = 10800;


l: I think a duration would be more easy to understand instead of number of frames

the confusing part is that the callback we use from flutter doesn't actually trigger every x fps, it triggers only when new frames are drawn.

If the user doesn't do anything no scrolling etc... after 3 minutes the frames tracked might be 100 or so which doesnt align with a Duration option

github-actions · 2024-09-26T12:21:55Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1249.43 ms	1263.12 ms	13.69 ms
Size	8.38 MiB	9.74 MiB	1.36 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`dd933d4`	1238.73 ms	1252.43 ms	13.70 ms
`3e5ee37`	1248.25 ms	1265.38 ms	17.13 ms
`6034b0a`	1244.89 ms	1270.22 ms	25.33 ms
`824df58`	1235.72 ms	1259.02 ms	23.30 ms
`3e4b523`	1260.53 ms	1270.06 ms	9.53 ms
`33d0587`	1262.16 ms	1270.50 ms	8.34 ms
`ca7f531`	1242.51 ms	1282.61 ms	40.10 ms
`a5031f1`	1244.14 ms	1268.79 ms	24.64 ms
`9811573`	1259.78 ms	1278.33 ms	18.55 ms
`aba65ca`	1248.83 ms	1268.30 ms	19.46 ms

App size

Revision	Plain	With Sentry	Diff
`dd933d4`	8.33 MiB	9.64 MiB	1.31 MiB
`3e5ee37`	8.15 MiB	9.12 MiB	986.23 KiB
`6034b0a`	8.33 MiB	9.40 MiB	1.07 MiB
`824df58`	8.33 MiB	9.64 MiB	1.31 MiB
`3e4b523`	8.28 MiB	9.33 MiB	1.05 MiB
`33d0587`	8.29 MiB	9.38 MiB	1.09 MiB
`ca7f531`	8.32 MiB	9.38 MiB	1.06 MiB
`a5031f1`	8.32 MiB	9.50 MiB	1.18 MiB
`9811573`	8.16 MiB	9.17 MiB	1.01 MiB
`aba65ca`	8.38 MiB	9.73 MiB	1.35 MiB

Previous results on branch: fix/app-lag-sff

Startup times

Revision	Plain	With Sentry	Diff
`45ab9fc`	1237.96 ms	1267.60 ms	29.64 ms
`6ddb24e`	1251.69 ms	1272.11 ms	20.41 ms
`b3716ec`	1250.29 ms	1278.38 ms	28.09 ms

App size

Revision	Plain	With Sentry	Diff
`45ab9fc`	8.38 MiB	9.74 MiB	1.36 MiB
`6ddb24e`	8.38 MiB	9.74 MiB	1.36 MiB
`b3716ec`	8.38 MiB	9.74 MiB	1.36 MiB

buenaflor · 2024-09-26T15:13:10Z

we will keep the limit internal for now until we properly fix it.

we might not need to keep the references to the normal frames in the future which would not require a frame limit

buenaflor added 4 commits September 26, 2024 00:50

add frame retention option

e683d34

add safeguard for frames tracking that stops frame tracking after a l…

ccbbf2d

…imit

add test

78b95a8

update test

6ddb24e

ueman reviewed Sep 26, 2024

View reviewed changes

update option name

3d60b19

buenaflor added 5 commits September 26, 2024 14:00

update comment and fix tests

bb4f254

remove debugging code

93b7885

update changelog

dbc783e

change debug to warning log

c5d1299

Merge branch 'main' into fix/app-lag-sff

b3716ec

buenaflor marked this pull request as ready for review September 26, 2024 12:07

buenaflor requested review from krystofwoldrich, stefanosiano and martinhaintz as code owners September 26, 2024 12:07

buenaflor changed the title ~~fix(frame-tracking): app lag when span finishes~~ fix(frame-tracking): app lag when long running span finishes Sep 26, 2024

brustolin approved these changes Sep 26, 2024

View reviewed changes

buenaflor added 3 commits September 26, 2024 15:59

Update sentry_flutter_options.dart

45ab9fc

dont make option configurable

896151d

fix test

35a7001

stefanosiano approved these changes Sep 26, 2024

View reviewed changes

buenaflor merged commit 84c28cd into main Sep 26, 2024
49 checks passed

buenaflor deleted the fix/app-lag-sff branch September 26, 2024 21:52

getsentry deleted a comment from github-actions bot Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(frame-tracking): app lag when long running span finishes #2311

fix(frame-tracking): app lag when long running span finishes #2311

buenaflor commented Sep 26, 2024 •

edited

Loading

codecov bot commented Sep 26, 2024 •

edited

Loading

ueman Sep 26, 2024 •

edited

Loading

buenaflor Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/app-lag-sff

Startup times

App size

brustolin left a comment

brustolin Sep 26, 2024

buenaflor Sep 26, 2024

github-actions bot commented Sep 26, 2024 •

edited

Loading

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/app-lag-sff

Startup times

App size

buenaflor commented Sep 26, 2024 •

edited

Loading

	int framesTrackingRetentionPeriod = 3;
	Duration framesTrackingRetentionPeriod = Duration(minutes: 3);

fix(frame-tracking): app lag when long running span finishes #2311

fix(frame-tracking): app lag when long running span finishes #2311

Conversation

buenaflor commented Sep 26, 2024 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

codecov bot commented Sep 26, 2024 • edited Loading

Codecov Report

ueman Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

buenaflor Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2024 • edited Loading

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/app-lag-sff

Startup times

App size

brustolin left a comment

Choose a reason for hiding this comment

brustolin Sep 26, 2024

Choose a reason for hiding this comment

buenaflor Sep 26, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2024 • edited Loading

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/app-lag-sff

Startup times

App size

buenaflor commented Sep 26, 2024 • edited Loading

buenaflor commented Sep 26, 2024 •

edited

Loading

codecov bot commented Sep 26, 2024 •

edited

Loading

ueman Sep 26, 2024 •

edited

Loading

buenaflor Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading

buenaflor commented Sep 26, 2024 •

edited

Loading