Check app start spans time and foreground state #3550

stefanosiano · 2024-07-04T14:52:42Z

📜 Description

App start now takes AppStartMetrics.appLaunchedInForeground variable in consideration when adding spans to the transaction. For some reason we were setting it, but never used it.
App starts longer than 1 minute are dropped (approach taken from Firebase)
Updated foreground app launch check, based on Firebase

💡 Motivation and Context

Should fix #3538 and #3329, implementing #3084

💚 How did you test it?

Unit tests

📝 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

…to add spans to the transaction App starts longer than 1 minute are dropped (same as Firebase)

github-actions · 2024-07-04T15:05:05Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	430.80 ms	494.08 ms	63.28 ms
Size	1.70 MiB	2.33 MiB	644.95 KiB

Previous results on branch: fix/app-start-foreground

Startup times

Revision	Plain	With Sentry	Diff
`e0d3b36`	420.52 ms	521.90 ms	101.37 ms
`e65564b`	410.64 ms	481.40 ms	70.76 ms
`99bdeb7`	404.61 ms	476.26 ms	71.65 ms
`09db9ad`	433.32 ms	494.82 ms	61.50 ms
`bcf684e`	406.80 ms	477.94 ms	71.14 ms
`331b61a`	441.43 ms	494.88 ms	53.45 ms
`2a3bf05`	377.29 ms	459.94 ms	82.65 ms
`715ec3c`	544.45 ms	626.55 ms	82.10 ms

App size

Revision	Plain	With Sentry	Diff
`e0d3b36`	1.70 MiB	2.33 MiB	644.95 KiB
`e65564b`	1.70 MiB	2.28 MiB	593.57 KiB
`99bdeb7`	1.70 MiB	2.28 MiB	593.89 KiB
`09db9ad`	1.70 MiB	2.28 MiB	594.01 KiB
`bcf684e`	1.70 MiB	2.28 MiB	594.07 KiB
`331b61a`	1.70 MiB	2.28 MiB	593.57 KiB
`2a3bf05`	1.70 MiB	2.28 MiB	593.69 KiB
`715ec3c`	1.70 MiB	2.28 MiB	593.89 KiB

added tests

markushi

Nice! If left one comment about uptime vs. unix time that should be addressed, but apart from that LGTM!

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java

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

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java

…foreground status updated tests

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java

added app launch foreground check to AppStartMetrics ctor

…yPerformanceProvider, other than AppStartMetrics.onApplicationCreate

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java

kahest · 2024-07-11T15:05:08Z

@stefanosiano would be good to test if the scenario in #3567 is also fixed:

reproduce it if a push notification is received and the app is opened after some time(minutes or hours).

added foreground check to SentryAndroid.init

romtsn

LGTM!

# Conflicts: # CHANGELOG.md # sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt

…ate instead of class instantiation AppStartMetrics callback is now registered only once AppStartMetrics stops app start profiler if no activity is being created

CHANGELOG.md

Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>

App start now takes AppStartMetrics.appLaunchedInForeground variable …

fa15544

…to add spans to the transaction App starts longer than 1 minute are dropped (same as Firebase)

updated changelog

a12a15c

added tests

stefanosiano marked this pull request as ready for review July 4, 2024 15:44

stefanosiano requested review from adinauer, romtsn and markushi as code owners July 4, 2024 15:44

markushi approved these changes Jul 5, 2024

View reviewed changes

romtsn reviewed Jul 5, 2024

View reviewed changes

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

romtsn reviewed Jul 5, 2024

View reviewed changes

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java Outdated Show resolved Hide resolved

stefanosiano added 2 commits July 5, 2024 19:29

added Activity lifecycle registration to check start launch time and …

bf5e505

…foreground status updated tests

updated api file

c2b6daa

romtsn reviewed Jul 9, 2024

View reviewed changes

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java Outdated Show resolved Hide resolved

romtsn reviewed Jul 9, 2024

View reviewed changes

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java Outdated Show resolved Hide resolved

stefanosiano added 2 commits July 10, 2024 10:34

changed AppStartMetrics to use ActivityLifecycleCallbacksAdapter

31d9b25

added app launch foreground check to AppStartMetrics ctor

added AppStartMetrics.registerApplicationForegroundCheck in the Sentr…

9b09e73

…yPerformanceProvider, other than AppStartMetrics.onApplicationCreate

romtsn reviewed Jul 10, 2024

View reviewed changes

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

zhukic mentioned this pull request Jul 11, 2024

Incorrect cold app start measurement when the app was opened after recev push notification receive #3567

Closed

added AppStartMetrics automatically unregister lifecycle callback

082edcb

added foreground check to SentryAndroid.init

romtsn approved these changes Jul 16, 2024

View reviewed changes

stefanosiano added 2 commits July 16, 2024 15:47

Merge branch 'main' into fix/app-start-foreground

a5b1e28

# Conflicts: # CHANGELOG.md # sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt

ActivityLifecycleIntegration now reads first activity creation on cre…

93a9962

…ate instead of class instantiation AppStartMetrics callback is now registered only once AppStartMetrics stops app start profiler if no activity is being created

romtsn reviewed Jul 16, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

f33347d

Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>

stefanosiano merged commit 73237da into main Jul 17, 2024
24 of 26 checks passed

stefanosiano deleted the fix/app-start-foreground branch July 17, 2024 08:10

This was referenced Jul 25, 2024

Android transactions lasting for days, TTID halfway in, all spans at the very end. #3329

Closed

[Starfish] Use a better way to determine app foreground launch #3084

Closed

krystofwoldrich mentioned this pull request Aug 6, 2024

Use appLaunchedInForeground to determinate invalid app start data on Android getsentry/sentry-react-native#3995

Open

krystofwoldrich mentioned this pull request Aug 6, 2024

Use appLaunchedInForeground to determinate invalid app start data on Android getsentry/sentry-dart#2215

Open

kahest mentioned this pull request Aug 27, 2024

[Epic] App Start/TTID instrumentation improvements getsentry/team-mobile#193

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check app start spans time and foreground state #3550

Check app start spans time and foreground state #3550

stefanosiano commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading

Previous results on branch: fix/app-start-foreground

Startup times

App size

markushi left a comment

kahest commented Jul 11, 2024

romtsn left a comment

Check app start spans time and foreground state #3550

Check app start spans time and foreground state #3550

Conversation

stefanosiano commented Jul 4, 2024 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Jul 4, 2024 • edited Loading

Performance metrics 🚀

Previous results on branch: fix/app-start-foreground

Startup times

App size

markushi left a comment

Choose a reason for hiding this comment

kahest commented Jul 11, 2024

romtsn left a comment

Choose a reason for hiding this comment

stefanosiano commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading