Skip to content
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.

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

feat: Add frames delay to transactions #3487

Merged
merged 36 commits into from
Dec 14, 2023
Merged

feat: Add frames delay to transactions #3487

merged 36 commits into from
Dec 14, 2023

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Dec 7, 2023

📜 Description

Add frames delay measurement to transactions. I will open an extra PR to add the measurement as span data to spans because this PR is already huge.

💡 Motivation and Context

Fixes GH-3481

💚 How did you test it?

Unit tests, simulator and a real device.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

github-actions bot commented Dec 7, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 3af37da

Copy link

github-actions bot commented Dec 7, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1213.15 ms 1236.29 ms 23.14 ms
Size 21.58 KiB 418.15 KiB 396.57 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ddc9b9a 1201.71 ms 1226.70 ms 24.99 ms
a6f8b18 1238.54 ms 1265.56 ms 27.02 ms
bd2cb64 1216.14 ms 1241.78 ms 25.64 ms
3a31fc9 1237.35 ms 1249.02 ms 11.67 ms
4b08ceb 1237.75 ms 1249.61 ms 11.86 ms
881a955 1222.16 ms 1237.22 ms 15.06 ms
3437454 1235.54 ms 1244.82 ms 9.28 ms
6c31077 1233.80 ms 1245.34 ms 11.54 ms
2af280d 1227.86 ms 1240.36 ms 12.50 ms
9d56232 1192.09 ms 1228.86 ms 36.77 ms

App size

Revision Plain With Sentry Diff
ddc9b9a 20.76 KiB 420.40 KiB 399.65 KiB
a6f8b18 20.76 KiB 431.87 KiB 411.11 KiB
bd2cb64 22.85 KiB 413.45 KiB 390.60 KiB
3a31fc9 20.76 KiB 414.45 KiB 393.69 KiB
4b08ceb 20.76 KiB 431.99 KiB 411.23 KiB
881a955 22.85 KiB 407.63 KiB 384.78 KiB
3437454 22.85 KiB 408.88 KiB 386.03 KiB
6c31077 22.84 KiB 401.65 KiB 378.81 KiB
2af280d 20.76 KiB 435.22 KiB 414.46 KiB
9d56232 20.76 KiB 425.80 KiB 405.04 KiB

Previous results on branch: feat/frame-delay

Startup times

Revision Plain With Sentry Diff
7971d04 1203.70 ms 1232.47 ms 28.77 ms
d7c8764 1216.94 ms 1229.13 ms 12.19 ms
acfb07d 1220.59 ms 1231.10 ms 10.51 ms
be5eebc 1232.80 ms 1240.37 ms 7.57 ms
187485e 1245.55 ms 1261.12 ms 15.57 ms
261201d 1238.31 ms 1252.24 ms 13.93 ms
806f634 1202.00 ms 1212.79 ms 10.79 ms
773497b 1195.69 ms 1217.18 ms 21.49 ms
8866a9f 1244.98 ms 1264.70 ms 19.72 ms

App size

Revision Plain With Sentry Diff
7971d04 21.58 KiB 417.78 KiB 396.19 KiB
d7c8764 21.58 KiB 418.06 KiB 396.48 KiB
acfb07d 21.58 KiB 418.11 KiB 396.53 KiB
be5eebc 21.58 KiB 418.06 KiB 396.48 KiB
187485e 21.58 KiB 417.99 KiB 396.41 KiB
261201d 21.58 KiB 418.11 KiB 396.53 KiB
806f634 21.58 KiB 418.13 KiB 396.54 KiB
773497b 21.58 KiB 418.13 KiB 396.55 KiB
8866a9f 21.58 KiB 418.11 KiB 396.53 KiB

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Merging #3487 (3af37da) into main (f25febb) will increase coverage by 0.123%.
The diff coverage is 100.000%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3487       +/-   ##
=============================================
+ Coverage   89.056%   89.180%   +0.123%     
=============================================
  Files          526       528        +2     
  Lines        56937     57403      +466     
  Branches     20476     20569       +93     
=============================================
+ Hits         50706     51192      +486     
+ Misses        5312      5294       -18     
+ Partials       919       917        -2     
Files Coverage Δ
SentryTestUtils/TestDisplayLinkWrapper.swift 95.121% <100.000%> (+0.060%) ⬆️
Sources/Sentry/SentryDelayedFrame.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryDelayedFramesTracker.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryDependencyContainer.m 95.256% <100.000%> (+0.037%) ⬆️
Sources/Sentry/SentryFramesTracker.m 100.000% <100.000%> (+1.398%) ⬆️
Sources/Sentry/SentryTracer.m 96.353% <100.000%> (-0.104%) ⬇️
...SentryProfilerTests/SentryProfilerSwiftTests.swift 97.769% <100.000%> (ø)
...ance/FramesTracking/SentryFramesTrackerTests.swift 99.040% <100.000%> (+1.313%) ⬆️
...iewController/SentryTimeToDisplayTrackerTest.swift 97.975% <100.000%> (ø)
...ests/SentryTests/Transaction/SentrySpanTests.swift 99.404% <100.000%> (ø)
... and 1 more

... and 24 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f25febb...3af37da. Read the comment docs.

@philipphofmann philipphofmann marked this pull request as ready for review December 7, 2023 16:02
Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

LGTM

Just left a few small comments

Sources/Sentry/SentryDelayedFramesTracker.m Show resolved Hide resolved
Sources/Sentry/SentryFramesTracker.m Outdated Show resolved Hide resolved
Sources/Sentry/SentryDelayedFramesTracker.m Show resolved Hide resolved
@philipphofmann philipphofmann merged commit de033da into main Dec 14, 2023
70 checks passed
@philipphofmann philipphofmann deleted the feat/frame-delay branch December 14, 2023 13:09
@philipphofmann philipphofmann self-assigned this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add frames delay for transactions and spans
2 participants