Add capabilities to track jetpack compose composition/rendering time #2507

markushi · 2023-02-01T12:31:28Z

📜 Description

Add capabilities to track compose composition/rendering time. This requires some changes to our span interface.
Since composition and rendering has no clear start and end, we'd need to have spans

which get removed from the transaction in case they have no children
which auto-finish whenever their root transaction finishes
which trim their start/end based on start/end of their children

💡 Motivation and Context

In terms of performance monitoring we want to track

Composition Time
Rendering Time

💚 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
No breaking changes

🔮 Next steps

github-actions · 2023-02-01T12:32:47Z

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

Generated by 🚫 dangerJS against e8d857d

github-actions · 2023-02-01T12:48:45Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	294.40 ms	329.58 ms	35.18 ms
Size	1.73 MiB	2.34 MiB	626.08 KiB

Previous results on branch: feat/compose-tracing

Startup times

Revision	Plain	With Sentry	Diff
`04675d0`	244.33 ms	318.90 ms	74.57 ms
`cd0142b`	358.20 ms	394.85 ms	36.65 ms
`17a5b01`	306.35 ms	333.92 ms	27.57 ms
`0ab9720`	333.73 ms	393.38 ms	59.64 ms
`73d3f00`	286.64 ms	319.79 ms	33.15 ms
`beae470`	331.42 ms	362.17 ms	30.75 ms
`f0108ba`	323.04 ms	370.94 ms	47.90 ms
`118defd`	309.04 ms	347.06 ms	38.02 ms
`dda3459`	355.41 ms	426.02 ms	70.61 ms

App size

Revision	Plain	With Sentry	Diff
`04675d0`	1.73 MiB	2.34 MiB	625.74 KiB
`cd0142b`	1.73 MiB	2.33 MiB	620.68 KiB
`17a5b01`	1.73 MiB	2.34 MiB	624.25 KiB
`0ab9720`	1.73 MiB	2.33 MiB	620.68 KiB
`73d3f00`	1.73 MiB	2.34 MiB	625.06 KiB
`beae470`	1.73 MiB	2.33 MiB	620.91 KiB
`f0108ba`	1.73 MiB	2.34 MiB	625.63 KiB
`118defd`	1.73 MiB	2.34 MiB	626.08 KiB
`dda3459`	1.73 MiB	2.34 MiB	625.63 KiB

…va into feat/compose-tracing

codecov · 2023-02-03T08:55:06Z

Codecov Report

Patch coverage: 84.84% and project coverage change: +0.08 🎉

Comparison is base (e0df34f) 81.21% compared to head (e8d857d) 81.30%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2507      +/-   ##
============================================
+ Coverage     81.21%   81.30%   +0.08%     
- Complexity     4152     4186      +34     
============================================
  Files           335      336       +1     
  Lines         15444    15484      +40     
  Branches       2011     2022      +11     
============================================
+ Hits          12543    12589      +46     
+ Misses         2108     2101       -7     
- Partials        793      794       +1

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/NoOpSpan.java	`22.58% <0.00%> (-1.56%)`	⬇️
...entry/src/main/java/io/sentry/NoOpTransaction.java	`22.72% <0.00%> (-1.09%)`	⬇️
sentry/src/main/java/io/sentry/Span.java	`89.55% <78.37%> (-2.45%)`	⬇️
sentry/src/main/java/io/sentry/SentryTracer.java	`93.35% <91.42%> (+1.37%)`	⬆️
sentry/src/main/java/io/sentry/Hub.java	`76.82% <100.00%> (-0.17%)`	⬇️
sentry/src/main/java/io/sentry/SentryDate.java	`100.00% <100.00%> (+16.66%)`	⬆️
sentry/src/main/java/io/sentry/SpanOptions.java	`100.00% <100.00%> (ø)`
...ry/src/main/java/io/sentry/TransactionOptions.java	`88.00% <100.00%> (+36.27%)`	⬆️

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

CHANGELOG.md

sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt

sentry/src/main/java/io/sentry/Sentry.java

romtsn · 2023-02-08T10:22:38Z

sentry/src/main/java/io/sentry/Span.java

+      @Nullable String description,
+      @Nullable SentryDate timestamp,
+      @NotNull Instrumenter instrumenter) {
+    return startChild(operation, description, timestamp, instrumenter, new SpanOptions());


l: just thinking if we should propagate the options of this span to the children, in case they are not provided? probably not

romtsn · 2023-02-08T11:34:41Z

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -343,6 +397,44 @@ public void finish(@Nullable SpanStatus status) {
  @Override
  @ApiStatus.Internal
  public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) {
+
+    // auto-finish any open spans
+    for (Span span : getSpans()) {


h: as discussed internally, maybe let's try to create a new type for this kind of spans instead of introducing SpanOptions and packing everything into SentryTracer. There may be 2 different ways to solve this going forward:

try to have IdleSpan with idleTimeout which finishes itself after some timeout. This would require changing Activity txs to trimEnd = true to have the correct end timestamp for them

try to have IdleSpan without the idle timeout, so we could just iterate here the same way we do now, and check if the span is instance of IdleSpan, and force-finish it. It would then do the trimStart/trimEnd logic inside its own finish method.

@romtsn Sounds good, but I'm wondering how far I should go here? Spans right now are half-baked POJOs, e.g. a span can not directly access it's children. On the other hand a span has access to it's root transaction.

hmm, good point. Would you mind asking about that in team-client-infra to see if anyone has concerns if we keep track of child spans in Span and not only in SentryTracer? For me it would be fine, but maybe there's something we should align on, or at least document.

Also, there's a SpanRecorder class available on JS. Not sure if it's exactly what we want to achieve, but maybe we could reuse some bits.

sentry/src/main/java/io/sentry/Span.java

...es/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt

Co-authored-by: Roman Zavarnitsyn <[email protected]>

romtsn · 2023-02-21T15:52:31Z

sentry/api/sentry.api

@@ -1742,7 +1750,7 @@ public final class io/sentry/SentryTraceHeader {

 public final class io/sentry/SentryTracer : io/sentry/ITransaction {
 	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;)V
-	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;ZLio/sentry/TransactionFinishedCallback;)V
+	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;Lio/sentry/TransactionOptions;Lio/sentry/TransactionFinishedCallback;)V


m: technically a breaking change, but I doubt anyone ever used that. Should we remove it altogether and just use the other ctor in the ActivityLifecycleIntegrationTest?

I think that's fine as SentryTracer is ApiStatus.Internal

sentry/src/main/java/io/sentry/Span.java

romtsn · 2023-02-21T16:01:32Z

sentry/src/main/java/io/sentry/Span.java

+
+      final @NotNull List<Span> children = getChildren();
+      for (final Span child : children) {
+        if (child.getParentSpanId() != null && child.getParentSpanId().equals(getSpanId())) {


m: we don't need this check, because we already do it in getChildren, right?

sentry/src/main/java/io/sentry/Span.java

sentry/src/main/java/io/sentry/SentryTracer.java

romtsn

lgtm, great stuff! 👍

left some comments. Also there's one more point from the previous iteration - to surround the testTag with a boolean flag, which is turned on by default.

Instead of immediatelly finishing the transaction in onPostResume, this way delayed JPC spans can be properly created

sentry/src/main/java/io/sentry/Span.java

romtsn · 2023-03-13T15:51:40Z

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -345,8 +352,30 @@ public void finish(@Nullable SpanStatus status) {
  @Override
  @ApiStatus.Internal
  public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) {


@adinauer wanna take a look at this, if it has any effect for backend? I don't think it does, but just to make sure we do not introduce some new potential memory leak or something

sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java

romtsn · 2023-03-13T15:58:26Z

a couple of minor things, but looks good after the final pass!

Also: reformat code

adinauer

Didn't see any obvious problems.

…2507) Co-authored-by: Roman Zavarnitsyn <[email protected]> Co-authored-by: Sentry Github Bot <[email protected]>

markushi added 6 commits January 26, 2023 12:30

Add initial poc for tracking compose rendering time

c720257

Merge branch 'main' into feat/compose-tracing

777da18

Add SpanOptions to auto-remove, trim and finish spans

b28318b

Improve Compose tracing, only first composition/rendering is tracked

cd8e3e4

Merge branch 'main' into feat/compose-tracing

aa9f166

Fix span trimming logic

81f1ba5

Format code

c43fcd4

markushi added 2 commits February 3, 2023 09:28

Add tests, remove optional span

a105676

Merge branch 'feat/compose-tracing' of github.com:getsentry/sentry-ja…

50ed172

…va into feat/compose-tracing

markushi added 2 commits February 3, 2023 10:29

Merge branch 'main' into feat/compose-tracing

a2718a8

Update changelog

17500bd

romtsn reviewed Feb 8, 2023

View reviewed changes

markushi commented Feb 9, 2023

View reviewed changes

...es/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt Outdated Show resolved Hide resolved

markushi and others added 6 commits February 9, 2023 10:34

Apply suggestions from code review

78c7345

Co-authored-by: Roman Zavarnitsyn <[email protected]>

Merge branch 'main' into feat/compose-tracing

b8689b2

Update based on PR comments

49f1310

Merge branch 'main' into feat/compose-tracing

bde92b7

Let TransactionOptions inherit from SpanOptions

4098d79

Fix Changelog

5929c5a

markushi marked this pull request as ready for review February 21, 2023 12:37

markushi requested review from adinauer and stefanosiano as code owners February 21, 2023 12:37

romtsn reviewed Feb 21, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Span.java Outdated Show resolved Hide resolved

romtsn reviewed Feb 21, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Span.java Outdated Show resolved Hide resolved

romtsn reviewed Feb 21, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Span.java Outdated Show resolved Hide resolved

romtsn reviewed Feb 21, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Span.java Show resolved Hide resolved

romtsn reviewed Feb 21, 2023

View reviewed changes

sentry/src/main/java/io/sentry/SentryTracer.java Outdated Show resolved Hide resolved

romtsn approved these changes Feb 21, 2023

View reviewed changes

markushi added 8 commits February 27, 2023 08:54

Added enableUserInteractionTracing to SentryTraced

b87364e

Add more tests for span trimming

be159b4

Fix use proper status for finishing idle spans

ef3702f

Merge branch 'main' into feat/compose-tracing

845b7a6

Fix merge conflict

babea52

Change visibility of span start date setter

83e5a6f

Merge branch 'main' into feat/compose-tracing

72c276e

Make activity transactions idle

672fb22

Instead of immediatelly finishing the transaction in onPostResume, this way delayed JPC spans can be properly created

romtsn reviewed Mar 13, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Span.java Show resolved Hide resolved

romtsn reviewed Mar 13, 2023

View reviewed changes

sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java Show resolved Hide resolved

markushi added 4 commits March 14, 2023 15:04

Fix span trim start/end logic

8750da7

Add e2e UI test to ensure ttid/ttfd spans are created

f3463db

Also: reformat code

Merge branch 'main' into feat/compose-tracing

ac5f687

Merge branch 'main' into feat/compose-tracing

e8d857d

markushi changed the title ~~Add capabilities to track compose composition/rendering time~~ Add capabilities to track jetpack compose composition/rendering time Mar 15, 2023

adinauer approved these changes Mar 15, 2023

View reviewed changes

markushi merged commit 550d8ec into main Mar 16, 2023

markushi deleted the feat/compose-tracing branch March 16, 2023 07:23

romtsn added a commit that referenced this pull request Mar 16, 2023

Add capabilities to track jetpack compose composition/rendering time (#…

1d79c71

…2507) Co-authored-by: Roman Zavarnitsyn <[email protected]> Co-authored-by: Sentry Github Bot <[email protected]>

romtsn mentioned this pull request Apr 5, 2023

Track correct App Startup for compose #2144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add capabilities to track jetpack compose composition/rendering time #2507

Add capabilities to track jetpack compose composition/rendering time #2507

markushi commented Feb 1, 2023

github-actions bot commented Feb 1, 2023 •

edited

Loading

github-actions bot commented Feb 1, 2023 •

edited

Loading

Previous results on branch: feat/compose-tracing

Startup times

App size

codecov bot commented Feb 3, 2023 •

edited

Loading

romtsn Feb 8, 2023

romtsn Feb 8, 2023

markushi Feb 14, 2023 •

edited

Loading

romtsn Feb 14, 2023

romtsn Feb 21, 2023

markushi Feb 27, 2023

romtsn Feb 21, 2023

romtsn left a comment

romtsn Mar 13, 2023

romtsn commented Mar 13, 2023

adinauer left a comment

Add capabilities to track jetpack compose composition/rendering time #2507

Add capabilities to track jetpack compose composition/rendering time #2507

Conversation

markushi commented Feb 1, 2023

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Feb 1, 2023 • edited Loading

github-actions bot commented Feb 1, 2023 • edited Loading

Performance metrics 🚀

Previous results on branch: feat/compose-tracing

Startup times

App size

codecov bot commented Feb 3, 2023 • edited Loading

Codecov Report

romtsn Feb 8, 2023

Choose a reason for hiding this comment

romtsn Feb 8, 2023

Choose a reason for hiding this comment

markushi Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

romtsn Feb 14, 2023

Choose a reason for hiding this comment

romtsn Feb 21, 2023

Choose a reason for hiding this comment

markushi Feb 27, 2023

Choose a reason for hiding this comment

romtsn Feb 21, 2023

Choose a reason for hiding this comment

romtsn left a comment

Choose a reason for hiding this comment

romtsn Mar 13, 2023

Choose a reason for hiding this comment

romtsn commented Mar 13, 2023

adinauer left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 1, 2023 •

edited

Loading

github-actions bot commented Feb 1, 2023 •

edited

Loading

codecov bot commented Feb 3, 2023 •

edited

Loading

markushi Feb 14, 2023 •

edited

Loading