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.
This is a proposal for removing SessionManager as a singleton
Current implementation
SessionManagerprimary purposes are:Session Creation and Management: It creates and manages
PerfSessionobjects. Each session is identified by a unique ID (a UUID), which is crucial for grouping related performance data. A new session is generated at every cold start of the app and also when the app comes to the foreground.Session ID Distribution: The class acts as a broadcaster. It informs other parts of the application (referred to as
SessionAwareObjectclients) whenever the active performance session changes. This ensures that all performance data (like traces and metrics) is correctly associated with the current session ID.Lifecycle Awareness: It listens to application state changes (like going into the foreground or background) via the
AppStateMonitor. This allows it to make decisions, such as starting a new session when the app is foregrounded.Gauge Metric Control: It controls the collection of periodic metrics (gauges), like CPU and memory usage. It tells the
GaugeManagerwhen to start or stop collecting these metrics based on the current session's configuration and the application's state (foreground/background).Currently this class is referenced in some others like
TraceUsage in
start()method: When aTraceis started, it interacts with theSessionManagerfor two primary reasons:SessionManager.getInstance().perfSession()to retrieve the activePerfSession. This session object (containing the session ID) is then associated with the trace.SessionManager.getInstance().registerForSessionUpdates(sessionAwareObject)to listen for future session changes. This ensures that if a new session starts while the trace is still running (e.g., the app goes to the background and comes back), the trace can be associated with the new session ID as well.Usage in
stop()method: When theTraceis stopped, it callsSessionManager.getInstance().unregisterForSessionUpdates(sessionAwareObject)to stop listening for session updates, preventing memory leaks.GaugeManagerGaugeManageris responsible for collecting periodic data like CPU and memory usage (gauges).SessionManagerholds the logic for when to start or stop collecting these gauges. It calls methods onGaugeManagerto start or stop gauge collection based on whether a session is running and if the app is in the foreground. This ensures that gauge metrics are only collected during an active, verbose session.AppStateMonitorAppStateMonitornotifies its listeners, one of which is theSessionManager, of these state changes. TheSessionManageruses this information to decide when to update the session. For example, when the app comes to the foreground after a certain period of inactivity in the background,SessionManagerwill generate a new session ID.FirebasePerformanceFirebasePerformancelikely triggers the initialization of theSessionManagerto start the very first session when the app starts up.Proposed refactor
The core change is that the
Traceclass no longer directly calls a staticSessionManager.getInstance()method. Instead, it now receives itsSessionManagerinstance fromAppStateMonitorduring its own initialization.In order to eliminate
SessionManageras a singletonSessionManageris Now Injected During ConstructionThe
Traceconstructors have been updated to receive theSessionManagerinstance fromAppStateMonitor.AppStateMonitornow acts as a provider or factory for theSessionManager. However more effort should be made to also removeAppStateMonitoras a singleton.Impact of These Changes
Traceis no longer tightly coupled to the static singleton implementation ofSessionManager. It now only knows that it needs an instance ofSessionManager, which it receives fromAppStateMonitor.Traceclass, we can now provide a mock or fakeSessionManagerobject in the constructor. This allows us to test the logic of theTraceclass in isolation without relying on the realSessionManagerand the entire Firebase SDK initialization.AppStateMonitorbe the source for theSessionManagerinstance, we have centralized the responsibility of creating and providing this dependency. This makes the code more organized and is a significant step toward completely removing the global singleton pattern.