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

Fix TTID timing issue #2326

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Fix TTID timing issue #2326

merged 6 commits into from
Oct 4, 2024

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Oct 1, 2024

📜 Description

  • Await finish before starting new route
  • Persist timestamp if completed was called before start because of async transaction start

💡 Motivation and Context

The creation of the transaction takes longer than SentryDisplayWidget takes to finish init. This PR safes the completion timestamp internally of the ttid tracker and uses it when finishing. The start timestamp is always before, as it is created before the async transaction start.

Closes #2274

💚 How did you test it?

Unit tests

📝 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

Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.84%. Comparing base (8c0b6dc) to head (b17e607).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2326      +/-   ##
==========================================
+ Coverage   87.07%   90.84%   +3.77%     
==========================================
  Files         104       73      -31     
  Lines        3706     2393    -1313     
==========================================
- Hits         3227     2174    -1053     
+ Misses        479      219     -260     

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

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 450.14 ms 507.22 ms 57.08 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
90db9ff 334.86 ms 388.14 ms 53.28 ms
90a08ea 477.25 ms 534.10 ms 56.85 ms
21d4150 379.31 ms 449.23 ms 69.93 ms
211a7aa 324.19 ms 393.26 ms 69.07 ms
64af39c 386.80 ms 471.11 ms 84.31 ms
8c0b6dc 454.21 ms 512.22 ms 58.01 ms
895becc 326.94 ms 376.02 ms 49.08 ms
f275487 369.08 ms 432.44 ms 63.36 ms
fdac48a 329.50 ms 396.46 ms 66.96 ms
633cf2e 289.36 ms 340.38 ms 51.02 ms

App size

Revision Plain With Sentry Diff
90db9ff 6.06 MiB 7.10 MiB 1.04 MiB
90a08ea 6.49 MiB 7.55 MiB 1.06 MiB
21d4150 5.94 MiB 6.97 MiB 1.03 MiB
211a7aa 6.06 MiB 7.03 MiB 997.24 KiB
64af39c 6.27 MiB 7.20 MiB 958.83 KiB
8c0b6dc 6.49 MiB 7.56 MiB 1.07 MiB
895becc 6.06 MiB 7.03 MiB 997.23 KiB
f275487 6.33 MiB 7.26 MiB 947.03 KiB
fdac48a 6.06 MiB 7.09 MiB 1.03 MiB
633cf2e 5.94 MiB 6.92 MiB 1001.53 KiB

Previous results on branch: fix/ttid_ttfd_timing_issue

Startup times

Revision Plain With Sentry Diff
30db7b8 361.09 ms 392.45 ms 31.37 ms
3bd5c5d 481.33 ms 527.78 ms 46.45 ms
9e6147d 488.65 ms 516.20 ms 27.55 ms

App size

Revision Plain With Sentry Diff
30db7b8 6.49 MiB 7.56 MiB 1.07 MiB
3bd5c5d 6.49 MiB 7.56 MiB 1.07 MiB
9e6147d 6.49 MiB 7.56 MiB 1.07 MiB

Copy link
Contributor

github-actions bot commented Oct 1, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.14 ms 1252.06 ms 17.92 ms
Size 8.38 MiB 9.74 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
3ba8d7e 1224.14 ms 1226.12 ms 1.98 ms
3500574 1263.02 ms 1263.73 ms 0.71 ms
4829ad3 1260.14 ms 1264.41 ms 4.27 ms
abcdba3 1257.31 ms 1283.49 ms 26.18 ms
586d7d2 1248.73 ms 1257.39 ms 8.66 ms
7e7f0b1 1230.52 ms 1251.49 ms 20.97 ms
b8562d0 1249.92 ms 1267.56 ms 17.64 ms
04bd9e6 1230.78 ms 1250.71 ms 19.94 ms
a510d1d 1277.04 ms 1291.57 ms 14.53 ms
5f2f77b 1231.76 ms 1248.43 ms 16.67 ms

App size

Revision Plain With Sentry Diff
3ba8d7e 8.29 MiB 9.36 MiB 1.07 MiB
3500574 8.29 MiB 9.38 MiB 1.09 MiB
4829ad3 8.32 MiB 9.38 MiB 1.05 MiB
abcdba3 8.15 MiB 9.12 MiB 989.76 KiB
586d7d2 8.33 MiB 9.54 MiB 1.22 MiB
7e7f0b1 8.33 MiB 9.61 MiB 1.27 MiB
b8562d0 8.33 MiB 9.54 MiB 1.22 MiB
04bd9e6 8.33 MiB 9.61 MiB 1.27 MiB
a510d1d 8.16 MiB 9.17 MiB 1.01 MiB
5f2f77b 8.33 MiB 9.64 MiB 1.31 MiB

Previous results on branch: fix/ttid_ttfd_timing_issue

Startup times

Revision Plain With Sentry Diff
9e6147d 1230.33 ms 1247.18 ms 16.86 ms
3bd5c5d 1223.96 ms 1246.42 ms 22.46 ms
30db7b8 1259.06 ms 1277.19 ms 18.13 ms

App size

Revision Plain With Sentry Diff
9e6147d 8.38 MiB 9.74 MiB 1.36 MiB
3bd5c5d 8.38 MiB 9.74 MiB 1.36 MiB
30db7b8 8.38 MiB 9.74 MiB 1.36 MiB

@buenaflor
Copy link
Contributor

nice, I think that might be the fix, looks right to me at least

@denrase denrase marked this pull request as ready for review October 2, 2024 12:06
@denrase denrase requested a review from buenaflor October 2, 2024 13:49
@buenaflor buenaflor merged commit 905bf99 into main Oct 4, 2024
49 checks passed
@buenaflor buenaflor deleted the fix/ttid_ttfd_timing_issue branch October 4, 2024 10:49
martinhaintz added a commit that referenced this pull request Oct 8, 2024
…e-for-events' of https://github.com/getsentry/sentry-dart into feat/only-send-debug-images-referenced-in-the-stacktrace-for-events

* 'feat/only-send-debug-images-referenced-in-the-stacktrace-for-events' of https://github.com/getsentry/sentry-dart:
  Fix TTID timing issue (#2326)
  fix: dart2wasm test (#2327)
  Deprecate: metrics api (#2312)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TTID and TTFD are not measured, due to timing issues
2 participants