-
-
Notifications
You must be signed in to change notification settings - Fork 434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check app start spans time and foreground state #3550
Conversation
…to add spans to the transaction App starts longer than 1 minute are dropped (same as Firebase)
Performance metrics 🚀
|
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Outdated
Show resolved
Hide resolved
…foreground status updated tests
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Outdated
Show resolved
Hide resolved
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
Show resolved
Hide resolved
@stefanosiano would be good to test if the scenario in #3567 is also fixed:
|
added foreground check to SentryAndroid.init
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
📜 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
sendDefaultPII
is enabled.🔮 Next steps