Skip to content

App start: Create transaction when no SentryNavigatorObserver is present#2017

Merged
buenaflor merged 45 commits into
mainfrom
feat/app-start-improve-2
May 10, 2024
Merged

App start: Create transaction when no SentryNavigatorObserver is present#2017
buenaflor merged 45 commits into
mainfrom
feat/app-start-improve-2

Conversation

@buenaflor

@buenaflor buenaflor commented Apr 29, 2024

Copy link
Copy Markdown
Contributor

📜 Description

Builds upon #2009

When SentryNavigatorObserver is not present, it will not create a transaction for the app start spans to attach to - so we need to create it manually

💡 Motivation and Context

Mobile starfish, improve app start instrumentation

💚 How did you test it?

Existing tests from #2009

📝 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

🔮 Next steps

@github-actions

github-actions Bot commented Apr 29, 2024

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 402.24 ms 478.31 ms 76.07 ms
Size 6.33 MiB 7.30 MiB 992.08 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
abcdba3 354.68 ms 399.04 ms 44.36 ms
683fd34 336.53 ms 418.10 ms 81.57 ms
afa6e2a 349.73 ms 428.48 ms 78.75 ms
873fb42 352.10 ms 397.43 ms 45.33 ms
f770c4c 385.88 ms 449.86 ms 63.98 ms
464b4d0 320.71 ms 380.02 ms 59.31 ms
69670c9 325.59 ms 385.82 ms 60.22 ms
08a7b4f 346.47 ms 403.29 ms 56.82 ms
333903e 332.76 ms 406.86 ms 74.10 ms
e2d89fc 323.84 ms 376.23 ms 52.39 ms

App size

Revision Plain With Sentry Diff
abcdba3 5.94 MiB 6.95 MiB 1.01 MiB
683fd34 6.27 MiB 7.20 MiB 960.43 KiB
afa6e2a 6.27 MiB 7.20 MiB 955.69 KiB
873fb42 5.94 MiB 6.96 MiB 1.02 MiB
f770c4c 6.33 MiB 7.26 MiB 950.37 KiB
464b4d0 6.06 MiB 7.03 MiB 990.27 KiB
69670c9 6.06 MiB 7.09 MiB 1.03 MiB
08a7b4f 5.94 MiB 6.95 MiB 1.01 MiB
333903e 6.06 MiB 7.10 MiB 1.04 MiB
e2d89fc 6.06 MiB 7.03 MiB 989.37 KiB

Previous results on branch: feat/app-start-improve-2

Startup times

Revision Plain With Sentry Diff
8b9c0c0 380.41 ms 462.63 ms 82.22 ms
04724d5 433.62 ms 500.70 ms 67.08 ms
f7573f1 349.55 ms 411.79 ms 62.23 ms
58f2585 431.80 ms 500.70 ms 68.90 ms
e294533 355.73 ms 421.98 ms 66.25 ms
7001b46 351.55 ms 408.36 ms 56.81 ms
8984d63 357.78 ms 430.02 ms 72.24 ms
26e5834 450.93 ms 526.94 ms 76.01 ms
5fff0be 404.65 ms 476.16 ms 71.51 ms

App size

Revision Plain With Sentry Diff
8b9c0c0 6.33 MiB 7.30 MiB 987.83 KiB
04724d5 6.33 MiB 7.30 MiB 990.95 KiB
f7573f1 6.33 MiB 7.30 MiB 990.94 KiB
58f2585 6.33 MiB 7.30 MiB 990.98 KiB
e294533 6.33 MiB 7.30 MiB 991.27 KiB
7001b46 6.33 MiB 7.30 MiB 992.04 KiB
8984d63 6.33 MiB 7.30 MiB 991.35 KiB
26e5834 6.33 MiB 7.30 MiB 990.95 KiB
5fff0be 6.33 MiB 7.30 MiB 990.97 KiB

@github-actions

github-actions Bot commented Apr 29, 2024

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1245.53 ms 1269.61 ms 24.08 ms
Size 8.32 MiB 9.51 MiB 1.19 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a1a1545 1270.85 ms 1289.82 ms 18.96 ms
4b943a1 1250.23 ms 1262.28 ms 12.05 ms
d189e01 1219.94 ms 1228.20 ms 8.27 ms
bf4aed7 1274.63 ms 1286.48 ms 11.85 ms
6572f8d 1242.16 ms 1246.63 ms 4.47 ms
1d47eb7 1212.57 ms 1222.00 ms 9.43 ms
24b6e60 1250.69 ms 1268.63 ms 17.94 ms
bd37365 1236.22 ms 1243.27 ms 7.04 ms
ae02632 1286.77 ms 1300.37 ms 13.60 ms
c978477 1228.43 ms 1249.17 ms 20.74 ms

App size

Revision Plain With Sentry Diff
a1a1545 8.16 MiB 9.17 MiB 1.01 MiB
4b943a1 8.33 MiB 9.40 MiB 1.07 MiB
d189e01 8.29 MiB 9.38 MiB 1.09 MiB
bf4aed7 8.10 MiB 9.17 MiB 1.08 MiB
6572f8d 8.29 MiB 9.36 MiB 1.07 MiB
1d47eb7 8.29 MiB 9.39 MiB 1.09 MiB
24b6e60 8.32 MiB 9.38 MiB 1.06 MiB
bd37365 8.28 MiB 9.34 MiB 1.06 MiB
ae02632 8.16 MiB 9.15 MiB 1020.68 KiB
c978477 8.32 MiB 9.50 MiB 1.18 MiB

Previous results on branch: feat/app-start-improve-2

Startup times

Revision Plain With Sentry Diff
58f2585 1243.20 ms 1259.61 ms 16.41 ms
f7573f1 1251.57 ms 1276.22 ms 24.65 ms
26e5834 1236.00 ms 1264.20 ms 28.20 ms
e294533 1257.85 ms 1280.23 ms 22.37 ms
5fff0be 1250.41 ms 1271.00 ms 20.59 ms
8984d63 1233.57 ms 1258.60 ms 25.04 ms
7001b46 1252.49 ms 1275.23 ms 22.74 ms
04724d5 1225.65 ms 1244.87 ms 19.23 ms
8b9c0c0 1251.80 ms 1272.17 ms 20.38 ms

App size

Revision Plain With Sentry Diff
58f2585 8.32 MiB 9.43 MiB 1.11 MiB
f7573f1 8.32 MiB 9.37 MiB 1.05 MiB
26e5834 8.32 MiB 9.37 MiB 1.05 MiB
e294533 8.32 MiB 9.50 MiB 1.19 MiB
5fff0be 8.32 MiB 9.37 MiB 1.05 MiB
8984d63 8.32 MiB 9.50 MiB 1.19 MiB
7001b46 8.32 MiB 9.51 MiB 1.19 MiB
04724d5 8.32 MiB 9.37 MiB 1.05 MiB
8b9c0c0 8.32 MiB 9.43 MiB 1.10 MiB

@buenaflor
buenaflor marked this pull request as ready for review May 3, 2024 17:38
@buenaflor

Copy link
Copy Markdown
Contributor Author

@stefanosiano this is now ready for review pls

@stefanosiano stefanosiano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good!

// This is a workaround since there is no api that tells us if
// the navigator observer exists or not. The currentRouteName is always
// set during a didPush triggered by the navigator observer.
if (SentryNavigatorObserver.currentRouteName == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we could even have an additional static flag isAttached that is set to true when the class is instantiated, and use it together with this check, but not sure if that would actually help some case.
Your call here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hm I think it would be good to have it additionally, I'll add it 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think naming it something like isCreated is more accurate since you can also instantiate it earlier and attach it much later

@buenaflor
buenaflor merged commit 40e30e1 into main May 10, 2024
@buenaflor
buenaflor deleted the feat/app-start-improve-2 branch May 10, 2024 10:30
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.

2 participants