-
-
Notifications
You must be signed in to change notification settings - Fork 451
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
Warm starts cleanup #3954
Warm starts cleanup #3954
Conversation
- creates `onCreate` and `onStart` TimeSpans - set app start type to warm in AppStartMetrics when needed AppStartMetrics has now a method to restart appStartSpan and reset its uptime_ms PerformanceAndroidEventProcessor now attaches activity start spans to warm starts, too SentryPerformanceProvider doesn't create spans anymore TimeSpan.setStartUnixTimeMs now shifts other timestamps accordingly
…t app start timestamp reverted TimeSpan.setStartUnixTimeMs to @testonly method
…ld start was invalid (app was started in background, like via BroadcastReceiver)
… in perfv1, too updated tests
…fecycleSpanHelper moved processInitSpan creation to AppStartMetrics ActivityLifecycleIntegration now create regular spans, and adds TimeSpans to AppStartMetrics to handle hybrid SDKs PerformanceAndroidEventProcessor does not add activity lifecycle spans to the transaction, as they are added by ActivityLifecycleIntegration directly
# Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
55f973b | 445.35 ms | 510.94 ms | 65.59 ms |
91b2d45 | 432.93 ms | 479.85 ms | 46.92 ms |
b66bbdb | 419.41 ms | 429.41 ms | 10.00 ms |
3c20f24 | 404.14 ms | 488.88 ms | 84.74 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
55f973b | 1.70 MiB | 2.36 MiB | 672.08 KiB |
91b2d45 | 1.70 MiB | 2.36 MiB | 672.05 KiB |
b66bbdb | 1.70 MiB | 2.36 MiB | 669.84 KiB |
3c20f24 | 1.70 MiB | 2.36 MiB | 669.84 KiB |
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
# Conflicts: # CHANGELOG.md # sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java # sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java # sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java # sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt # sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt # sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
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 one, looks goods to me! Left a few minor comments 🚀
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java
Outdated
Show resolved
Hide resolved
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
📜 Description
💡 Motivation and Context
Cleans some code after the warm start changes
I added a separate PR to avoid having too many logical changes in one
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps