Wait for a valid SessionID before logging the app start event. #5437
+803
−167
Conversation
This file contains hidden or 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
#5397) Introduces a new SessionDataService that is bound to by each process and used to deliver session-ids.
Co-authored-by: jrothfeder <rothbutter@google.com>
Created a SessionLifecycleService class which contains all the service-side code, and a SessionLifecycleClient object which handles the connection to the server and the sending and receiving of events.
Avoid changing the api surface. Having `val`s at the file level causes Kotlin to generate a public class with the filenameKt which adds a public class to the public api surface: "The public api surface has changed for the subproject firebase-sessions: error: Added class com.google.firebase.sessions.SessionDatastoreKt [AddedClass]". So we have to put them in a class or companion object.
…ance` so it can be accessed from a bound service. (#5407) Co-authored-by: jrothfeder <rothbutter@google.com>
Make `SessionGenerator` injectable. Also made `WallClock` an object to avoid needing to pass it around. Moved `collectEvents` out of SessionGenerator since it will need to be handled in the service.
Co-authored-by: jrothfeder <rothbutter@google.com>
…ave vimean superpowers. (#5413) Co-authored-by: jrothfeder <rothbutter@google.com>
Using the SessionGenerator to generate new sessions and create SessionDetails instead of directly creating UUID session ids in the service. Also updated the boundClients queue to be a LinkedBlockingQueue to avoid ConcurrentModificationExceptions if a client binds while a message is being handled by the server.
Made SessionsSettings injectable. Also removed SessionMaintainer.
Co-authored-by: jrothfeder <rothbutter@google.com>
This will make it safe to call `Firebase.app` any time from anywhere in this SDK.
#5427) … the FirelogPublisher handle the full SessionEvent creation since that's only relevant to Firelog.
#5431) … the MainLooper which is the application main thread and so could interfere with things there. Also fixes issue with missing import.
#5434) …and use the same Messenger instance for every client. This makes sure that all state manipulation happens on the same looper, so we can get rid of the LinkedBlockingQueue or worry about volatile booleans.
visumickey
requested review from
raymondlam,
jrothfeder,
bryanatkinson and
mrober
raymondlam
requested changes
Oct 17, 2023
|
|
||
| @Override | ||
| public void updateSession(PerfSession session) { | ||
| if (session.sessionId().isEmpty()) { |
There was a problem hiding this comment.
When is this true that updateSession is called but the id is null:
Do we need to unregister ourshelves from session updates?
Size Report 1Affected Products
Test Logs |
bryanatkinson
approved these changes
Oct 18, 2023
| this.startSession = SessionManager.getInstance().perfSession(); | ||
| dispatchAppStartTrace(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Should this call updateSession(perfSession) instead?
mrober
approved these changes
Oct 18, 2023
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.
No description provided.