Calculate frame delay on a span level

[markushi]

📜 Description

Fixes #3183

💡 Motivation and Context

Screen_recording_20240208_093207.mp4

💚 How did you test it?

📝 Checklist

• 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

[github-actions]

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

Generated by 🚫 dangerJS against 02befdc

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	422.46 ms	528.46 ms	106.00 ms
Size	1.70 MiB	2.27 MiB	586.31 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
93a76ca	377.96 ms	447.52 ms	69.56 ms
baaf637	462.32 ms	579.22 ms	116.90 ms
2465853	411.39 ms	461.10 ms	49.72 ms
c3f503e	360.41 ms	434.67 ms	74.27 ms
93a76ca	381.08 ms	459.22 ms	78.14 ms
c7e2fbc	393.98 ms	478.24 ms	84.27 ms
2465853	422.61 ms	491.20 ms	68.58 ms
5e04ee8	302.68 ms	343.36 ms	40.68 ms
283d83e	416.81 ms	497.22 ms	80.41 ms
d6d2b2e	413.20 ms	486.76 ms	73.56 ms

App size

Revision	Plain	With Sentry	Diff
93a76ca	1.72 MiB	2.29 MiB	576.75 KiB
baaf637	1.72 MiB	2.27 MiB	558.42 KiB
2465853	1.70 MiB	2.27 MiB	583.82 KiB
c3f503e	1.72 MiB	2.29 MiB	577.04 KiB
93a76ca	1.72 MiB	2.29 MiB	576.75 KiB
c7e2fbc	1.72 MiB	2.29 MiB	576.40 KiB
2465853	1.70 MiB	2.27 MiB	583.82 KiB
5e04ee8	1.70 MiB	2.27 MiB	584.64 KiB
283d83e	1.72 MiB	2.29 MiB	577.69 KiB
d6d2b2e	1.72 MiB	2.27 MiB	555.05 KiB

Previous results on branch: feat/frame-delay-per-span

Startup times

Revision	Plain	With Sentry	Diff
cd12d56	391.75 ms	461.53 ms	69.78 ms
af827fa	439.17 ms	516.45 ms	77.28 ms
2d2263f	348.71 ms	388.90 ms	40.18 ms
2248393	448.29 ms	525.46 ms	77.17 ms
d9a60b1	405.83 ms	465.22 ms	59.39 ms
8dff466	390.89 ms	469.38 ms	78.49 ms

App size

Revision	Plain	With Sentry	Diff
cd12d56	1.70 MiB	2.27 MiB	586.08 KiB
af827fa	1.70 MiB	2.27 MiB	586.30 KiB
2d2263f	1.70 MiB	2.27 MiB	586.22 KiB
2248393	1.70 MiB	2.27 MiB	586.38 KiB
d9a60b1	1.70 MiB	2.27 MiB	586.30 KiB
8dff466	1.70 MiB	2.27 MiB	586.42 KiB

[stefanosiano]

It looks good overall.
I guess we have to revisit the SpanFrameMetricsCollector, as it doesn't work correctly for multiple spans at the same time.
Maybe there is also a way to get rid of those intermediate classes (SpanStart and NanoTimeStamp), too?

[markushi]

It looks good overall. I guess we have to revisit the SpanFrameMetricsCollector, as it doesn't work correctly for multiple spans at the same time. Maybe there is also a way to get rid of those intermediate classes (SpanStart and NanoTimeStamp), too?

I've resolved the issue for multi-spans, mixed up head/tailSet APIs 🙈
I'd love to get rid of SpanStart and NanoTimeStamp, I'll have a look and provide a follow up PR if something more elegant would work here.

[romtsn]

just trying to wrap my head around - why this wasn't working and why do we need a separate collection to track start timestamps?

 final long spanDurationNanos = spanFinishDate.diff(span.getStartDate());

[stefanosiano]

in this case it should work fine

[markushi]

Because our ISpans only have SentryDate, which itself has no guarantees around precision - it could be e.g. a time synced unix timestamp. SentryDate is also not Comparable, so it can't be saved in an ordered set.
I'd be happy to change this within the whole SDK though. Maybe an extra one, to not blow this PR up too much 😅

[romtsn]

but SentryDate is an abstract class, in the end it'll be either SentryNanotimeDate or SentryInstantDate which both have the nanoseconds precision. And it's actually Comparable ;)

sentry-java/sentry/src/main/java/io/sentry/SentryDate.java

Lines 48 to 51 in a537f8a

	@Override
	public int compareTo(@NotNull SentryDate otherDate) {
	return Long.valueOf(nanoTimestamp()).compareTo(otherDate.nanoTimestamp());
	}

[markushi]

Alright, I've looked into it and got it working by applying a little trick

[stefanosiano]

you can remove that trick if you use the SentryNanotimeDate in the Frame object, instead of nanoseconds

[romtsn]

Great stuff overall, I just have a couple of questions to clarify. Great work with the new sample screen for testing this stuff, looks really helpful 🚀

[stefanosiano]

this class can be private

[stefanosiano]

just make Frame class private and we're good to go.
Your call if you want to change the nanoseconds to SentryNanotimeDate (or evaluate to do it in another PR)
very nice!