add time-to-full-display span to activity transactions

[stefanosiano]

📜 Description

added ttfd span
added a FullyDrawnReporter into the options
added a new api Sentry.reportFullyDrawn()

💡 Motivation and Context

We want to allow the user to report when the ui is fully loaded for them, e.g. after retrieving some data from some api call and fill their own ui
More info can be found on this rfc

💚 How did you test it?

Unit tests

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

🔮 Next steps

Update docs

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	287.04 ms	412.90 ms	125.86 ms
Size	1.73 MiB	2.34 MiB	623.94 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
4b32504	357.14 ms	404.04 ms	46.90 ms
fe30606	327.46 ms	351.74 ms	24.28 ms
fe30606	310.82 ms	335.36 ms	24.55 ms
c1399c1	345.06 ms	385.49 ms	40.43 ms
754021c	358.70 ms	361.98 ms	3.28 ms
14c083a	350.82 ms	388.86 ms	38.04 ms
5fa24ec	326.29 ms	384.53 ms	58.24 ms
f6a135d	263.96 ms	383.59 ms	119.63 ms

App size

Revision	Plain	With Sentry	Diff
4b32504	1.73 MiB	2.34 MiB	623.74 KiB
fe30606	1.73 MiB	2.34 MiB	623.74 KiB
fe30606	1.73 MiB	2.34 MiB	623.74 KiB
c1399c1	1.73 MiB	2.33 MiB	620.61 KiB
754021c	1.73 MiB	2.33 MiB	623.06 KiB
14c083a	1.73 MiB	2.33 MiB	620.61 KiB
5fa24ec	1.73 MiB	2.33 MiB	620.61 KiB
f6a135d	1.73 MiB	2.33 MiB	623.10 KiB

Previous results on branch: android/ttfd

Startup times

Revision	Plain	With Sentry	Diff
a16e791	345.26 ms	360.78 ms	15.52 ms
16ebe65	333.80 ms	386.75 ms	52.95 ms
5604d15	308.69 ms	358.10 ms	49.41 ms
e8cdc9f	361.57 ms	405.46 ms	43.89 ms
9bbb1ce	334.72 ms	357.70 ms	22.98 ms

App size

Revision	Plain	With Sentry	Diff
a16e791	1.73 MiB	2.34 MiB	623.96 KiB
16ebe65	1.73 MiB	2.34 MiB	624.06 KiB
5604d15	1.73 MiB	2.34 MiB	624.06 KiB
e8cdc9f	1.73 MiB	2.34 MiB	623.94 KiB
9bbb1ce	1.73 MiB	2.33 MiB	623.52 KiB

[codecov-commenter]

Codecov Report

Base: 80.19% // Head: 80.20% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (2bbc3f1) compared to base (4b32504).
Patch coverage: 84.61% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #2432   +/-   ##
=========================================
  Coverage     80.19%   80.20%           
- Complexity     3948     3960   +12     
=========================================
  Files           323      324    +1     
  Lines         14911    14937   +26     
  Branches       1967     1968    +1     
=========================================
+ Hits          11958    11980   +22     
- Misses         2179     2183    +4     
  Partials        774      774           

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/HubAdapter.java	9.23% <0.00%> (-0.30%)	⬇️
sentry/src/main/java/io/sentry/IHub.java	92.00% <ø> (ø)
sentry/src/main/java/io/sentry/Sentry.java	55.13% <0.00%> (-0.61%)	⬇️
...src/main/java/io/sentry/FullDisplayedReporter.java	100.00% <100.00%> (ø)
sentry/src/main/java/io/sentry/Hub.java	76.99% <100.00%> (+0.16%)	⬆️
sentry/src/main/java/io/sentry/NoOpHub.java	47.72% <100.00%> (+1.21%)	⬆️
sentry/src/main/java/io/sentry/SentryOptions.java	79.78% <100.00%> (+0.31%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[romtsn]

So, I quickly checked how the new Activity transactions look, and I've spotted 2 problems there:

• First, we should probably align the AppStart span with the TTID span, because TTID shows you the truly correct time it took to display the first frame, as mentioned in the docs. We can tackle this separately (perhaps, we could even deprecate/remove the AppStart span in favor of the TTID span, but that's a separate discussion).
• Second, in my mind the TTFD span should've actually taken longer than the TTID span. I quickly checked the documentation around reportFullyDrawn, and they actually cap TTFD to the TTID time, if TTFD takes less time than TTID. From their docs:

If this method is called before the activity's window is first drawn and displayed as measured by the system, the reported time here will be shifted to the system measured time.

I did some quick tests, and, indeed, if I call Activity.reportFullyDrawn() inside onResume it shows the same time as TTID (Displayed in the logcat):

However, If I post an async runnable to call reportFullyDrawn() after 2 seconds, it shows correct time after approximately 2 seconds:

So, I believe we should align this behavior with how Google does it and cap the TTFD end time to TTID, if it's called earlier than TTID happened (and also document it for other SDKs). Sorry for the long-read :)

[stefanosiano]

First, we should probably align the AppStart span with the TTID span, because TTID shows you the truly correct time it took to display the first frame, as mentioned in the docs. We can tackle this separately (perhaps, we could even deprecate/remove the AppStart span in favor of the TTID span, but that's a separate discussion).

We can think about it for sure

Second, in my mind the TTFD span should've actually taken longer than the TTID span. I quickly checked the documentation around reportFullyDrawn, and they actually cap TTFD to the TTID time, if TTFD takes less time than TTID. From their docs:

I was thinking the same, as that's what Firebase is doing, too.
Maybe I should do another pr for that? That would also mean adding a new (internal) API to the span to change its endTimestamp after it's finished

[romtsn]

I was thinking the same, as that's what Firebase is doing, too.
Maybe I should do another pr for that? That would also mean adding a new (internal) API to the span to change its endTimestamp after it's finished

Sounds good, I've just approved the PR, feel free to merge and open a new one