Fix: Make App start cold/warm visible to Hybrid SDKs #1848
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marandaneto
commented
Dec 16, 2021
| @@ -44,7 +44,7 @@ void setAppStartEnd(final long appStartEndMillis) { | |||
| } | |||
|
|
|||
| @Nullable | |||
| synchronized Long getAppStartInterval() { | |||
| public synchronized Long getAppStartInterval() { | |||
There was a problem hiding this comment.
this needs to be public to be used in hybrid SDKs, e.g.
reason is https://github.com/getsentry/sentry-java/blob/main/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java#L63-L64
romtsn
approved these changes
Dec 16, 2021
|
LGTM, but wouldn't it be better to propagate the performanceEnabled flag from the hybrid SDKs rather? If they are utilising native SDKs for perf anyway |
no, we've decided that users would need to enable that manually if they want to, most hybrid apps don't do "native screens" so it'd not make much sense anyway. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
馃摐 Description
Fix: Make App start cold/warm visible to Hybrid SDKs
馃挕 Motivation and Context
getsentry/sentry-react-native#1968
React native reads the Cold or Warm start from the Android SDK, but the Android SDK only sets it if performance is enabled on the Android SDK as well, by default, performance is not enabled, only on the RN SDK, hence we have to flip the flag anyway, even if performance is disabled on the Android SDK.
馃挌 How did you test it?
馃摑 Checklist
馃敭 Next steps