Skip to content
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 start detection #3937

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Warm start detection #3937

wants to merge 8 commits into from

Conversation

stefanosiano
Copy link
Member

@stefanosiano stefanosiano commented Nov 28, 2024

📜 Description

  • ActivityLifecycleIntegration:
    • 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

Confirm warm starts are registered for background starts (BroadcastReceiver)

Cold start:
Cold start
Open second Activity:
open second activity
Reopen first Activity without closing app:
reopen first activity
Open first Activity after close app with Don't keep activity developer option enabled:
open after close and no keep activity

💡 Motivation and Context

Fixes #3896
Relates and possibly fixes #3899

💚 How did you test it?

📝 Checklist

  • 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.
  • I updated the wizard 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

In a followup PR:

  • Remove the activityCallback completely from the SentryPerformanceProvider
  • Create and attach spans directly in ActivityLifecycleIntegration instead of adding them to AppStartMetrics and let the PerformanceAndroidEventProcessor add them to the transaction
  • Avoid creating process init span using setStartUnixTimeMs as it is a testOnly method. This means care, as the method is exposed to hybrid sdks

- 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
Copy link
Contributor

github-actions bot commented Nov 29, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 386.73 ms 475.60 ms 88.87 ms
Size 1.70 MiB 2.36 MiB 669.76 KiB

Previous results on branch: fix/warm-starts

Startup times

Revision Plain With Sentry Diff
c21a34f 369.28 ms 441.12 ms 71.84 ms
34c4186 410.50 ms 437.28 ms 26.78 ms

App size

Revision Plain With Sentry Diff
c21a34f 1.70 MiB 2.36 MiB 669.76 KiB
34c4186 1.70 MiB 2.36 MiB 669.76 KiB

…ld start was invalid (app was started in background, like via BroadcastReceiver)
@stefanosiano stefanosiano marked this pull request as ready for review November 29, 2024 15:07
@stefanosiano stefanosiano changed the title ActivityLifecycleIntegration: Fix Warm start detection Dec 2, 2024
@stefanosiano stefanosiano changed the title Fix Warm start detection Warm start detection Dec 2, 2024
@stefanosiano stefanosiano mentioned this pull request Dec 3, 2024
7 tasks
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.

Not all warm start cases covered by SDK? Initial display time is reported incorrectly
2 participants