From b78b7ea25d766ce4640b65224f67a85a6a96a5ba Mon Sep 17 00:00:00 2001 From: Visu Date: Fri, 27 Oct 2023 11:14:43 -0700 Subject: [PATCH] Remove AQS based session ID dependency from Fireperf. (#5454) --- firebase-perf/CHANGELOG.md | 10 +- .../firebase/perf/FirebasePerfEarly.java | 30 +---- .../firebase/perf/FirebasePerfRegistrar.java | 8 -- .../firebase/perf/session/SessionManager.java | 40 +++++- .../perf/FirebasePerfRegistrarTest.java | 2 - .../perf/session/SessionManagerTest.java | 122 ++++++++++++++++++ 6 files changed, 162 insertions(+), 50 deletions(-) diff --git a/firebase-perf/CHANGELOG.md b/firebase-perf/CHANGELOG.md index 841db1ee053..6ed2fb1827a 100644 --- a/firebase-perf/CHANGELOG.md +++ b/firebase-perf/CHANGELOG.md @@ -1,23 +1,18 @@ # Unreleased - +* [changed] Make Fireperf generate its own session Id. # 20.5.0 * [changed] Added Kotlin extensions (KTX) APIs from `com.google.firebase:firebase-perf-ktx` to `com.google.firebase:firebase-perf` under the `com.google.firebase.perf` package. For details, see the [FAQ about this initiative](https://firebase.google.com/docs/android/kotlin-migration) + * [deprecated] All the APIs from `com.google.firebase:firebase-perf-ktx` have been added to `com.google.firebase:firebase-perf` under the `com.google.firebase.perf` package, and all the Kotlin extensions (KTX) APIs in `com.google.firebase:firebase-perf-ktx` are now deprecated. As early as April 2024, we'll no longer release KTX modules. For details, see the [FAQ about this initiative](https://firebase.google.com/docs/android/kotlin-migration) - -## Kotlin -The Kotlin extensions library transitively includes the updated -`firebase-performance` library. The Kotlin extensions library has no additional -updates. - # 20.4.1 * [changed] Updated `firebase-sessions` dependency to v1.0.2 * [fixed] Make fireperf data collection state is reliable for Firebase Sessions library. @@ -369,4 +364,3 @@ updates. # 16.1.0 * [fixed] Fixed a `SecurityException` crash on certain devices that do not have Google Play Services on them. - diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java index 5e5d05ee7a4..5b89deaad82 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java @@ -15,17 +15,13 @@ package com.google.firebase.perf; import android.content.Context; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.google.firebase.FirebaseApp; import com.google.firebase.StartupTime; import com.google.firebase.perf.application.AppStateMonitor; import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.metrics.AppStartTrace; -import com.google.firebase.perf.session.PerfSession; import com.google.firebase.perf.session.SessionManager; -import com.google.firebase.sessions.api.FirebaseSessionsDependencies; -import com.google.firebase.sessions.api.SessionSubscriber; import java.util.concurrent.Executor; /** @@ -55,31 +51,7 @@ public FirebasePerfEarly( uiExecutor.execute(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace)); } - // Register with Firebase sessions to receive updates about session changes. - FirebaseSessionsDependencies.register( - new SessionSubscriber() { - @Override - public void onSessionChanged(@NonNull SessionDetails sessionDetails) { - PerfSession perfSession = PerfSession.createWithId(sessionDetails.getSessionId()); - SessionManager.getInstance().updatePerfSession(perfSession); - } - - @Override - public boolean isDataCollectionEnabled() { - // If there is no cached config data available for data collection, be conservative. - // Return false. - if (!configResolver.isCollectionEnabledConfigValueAvailable()) { - return false; - } - return ConfigResolver.getInstance().isPerformanceMonitoringEnabled(); - } - - @NonNull - @Override - public Name getSessionSubscriberName() { - return SessionSubscriber.Name.PERFORMANCE; - } - }); + // TODO: Bring back Firebase Sessions dependency to watch for updates to sessions. // In the case of cold start, we create a session and start collecting gauges as early as // possible. diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java index 7e1500447a3..c01f035af1f 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java @@ -30,9 +30,6 @@ import com.google.firebase.perf.injection.modules.FirebasePerformanceModule; import com.google.firebase.platforminfo.LibraryVersionComponent; import com.google.firebase.remoteconfig.RemoteConfigComponent; -import com.google.firebase.sessions.FirebaseSessions; -import com.google.firebase.sessions.api.FirebaseSessionsDependencies; -import com.google.firebase.sessions.api.SessionSubscriber; import java.util.Arrays; import java.util.List; import java.util.concurrent.Executor; @@ -50,10 +47,6 @@ public class FirebasePerfRegistrar implements ComponentRegistrar { private static final String LIBRARY_NAME = "fire-perf"; private static final String EARLY_LIBRARY_NAME = "fire-perf-early"; - static { - FirebaseSessionsDependencies.INSTANCE.addDependency(SessionSubscriber.Name.PERFORMANCE); - } - @Override @Keep public List> getComponents() { @@ -71,7 +64,6 @@ public List> getComponents() { Component.builder(FirebasePerfEarly.class) .name(EARLY_LIBRARY_NAME) .add(Dependency.required(FirebaseApp.class)) - .add(Dependency.required(FirebaseSessions.class)) .add(Dependency.optionalProvider(StartupTime.class)) .add(Dependency.required(uiExecutor)) .eagerInDefaultApp() diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java index 73c505a8b47..c7eb4ca53f8 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java @@ -19,6 +19,7 @@ import androidx.annotation.Keep; import com.google.android.gms.common.util.VisibleForTesting; import com.google.firebase.perf.application.AppStateMonitor; +import com.google.firebase.perf.application.AppStateUpdateHandler; import com.google.firebase.perf.session.gauges.GaugeManager; import com.google.firebase.perf.v1.ApplicationProcessState; import com.google.firebase.perf.v1.GaugeMetadata; @@ -27,13 +28,14 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Set; +import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; /** Session manager to generate sessionIDs and broadcast to the application. */ @Keep // Needed because of b/117526359. -public class SessionManager { +public class SessionManager extends AppStateUpdateHandler { @SuppressLint("StaticFieldLeak") private static final SessionManager instance = new SessionManager(); @@ -56,8 +58,11 @@ public final PerfSession perfSession() { } private SessionManager() { - // Start with an empty session ID as the firebase sessions will override with real Id. - this(GaugeManager.getInstance(), PerfSession.createWithId(""), AppStateMonitor.getInstance()); + // Generate a new sessionID for every cold start. + this( + GaugeManager.getInstance(), + PerfSession.createWithId(UUID.randomUUID().toString()), + AppStateMonitor.getInstance()); } @VisibleForTesting @@ -66,6 +71,7 @@ public SessionManager( this.gaugeManager = gaugeManager; this.perfSession = perfSession; this.appStateMonitor = appStateMonitor; + registerForAppState(); } /** @@ -90,6 +96,34 @@ public void setApplicationContext(final Context appContext) { }); } + @Override + public void onUpdateAppState(ApplicationProcessState newAppState) { + super.onUpdateAppState(newAppState); + + if (appStateMonitor.isColdStart()) { + // We want the Session to remain unchanged if this is a cold start of the app since we already + // update the PerfSession in FirebasePerfProvider#onAttachInfo(). + return; + } + + if (newAppState == ApplicationProcessState.FOREGROUND) { + // A new foregrounding of app will force a new sessionID generation. + PerfSession session = PerfSession.createWithId(UUID.randomUUID().toString()); + updatePerfSession(session); + } else { + // If the session is running for too long, generate a new session and collect gauges as + // necessary. + if (perfSession.isSessionRunningTooLong()) { + PerfSession session = PerfSession.createWithId(UUID.randomUUID().toString()); + updatePerfSession(session); + } else { + // For any other state change of the application, modify gauge collection state as + // necessary. + startOrStopCollectingGauges(newAppState); + } + } + } + /** * Checks if the current {@link PerfSession} is expired/timed out. If so, stop collecting gauges. * diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java index 524949cd124..7df39fe6a1e 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java @@ -25,7 +25,6 @@ import com.google.firebase.components.Qualified; import com.google.firebase.installations.FirebaseInstallationsApi; import com.google.firebase.remoteconfig.RemoteConfigComponent; -import com.google.firebase.sessions.FirebaseSessions; import java.util.List; import java.util.concurrent.Executor; import org.junit.Test; @@ -60,7 +59,6 @@ public void testGetComponents() { .containsExactly( Dependency.required(Qualified.qualified(UiThread.class, Executor.class)), Dependency.required(FirebaseApp.class), - Dependency.required(FirebaseSessions.class), Dependency.optionalProvider(StartupTime.class)); assertThat(firebasePerfEarlyComponent.isLazy()).isFalse(); diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/SessionManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/SessionManagerTest.java index f55b89dd001..f3e3795f3f8 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/SessionManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/SessionManagerTest.java @@ -16,7 +16,9 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -31,12 +33,14 @@ import com.google.firebase.perf.session.gauges.GaugeManager; import com.google.firebase.perf.util.Clock; import com.google.firebase.perf.util.Timer; +import com.google.firebase.perf.v1.ApplicationProcessState; import java.lang.ref.WeakReference; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.AdditionalMatchers; import org.mockito.ArgumentMatchers; import org.mockito.InOrder; import org.mockito.Mock; @@ -83,6 +87,124 @@ public void setApplicationContext_logGaugeMetadata_afterGaugeMetadataManagerIsIn inOrder.verify(mockGaugeManager).logGaugeMetadata(any(), any()); } + @Test + public void testOnUpdateAppStateDoesNothingDuringAppStart() { + String oldSessionId = SessionManager.getInstance().perfSession().sessionId(); + + assertThat(oldSessionId).isNotNull(); + assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId()); + + AppStateMonitor.getInstance().setIsColdStart(true); + + SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.FOREGROUND); + assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId()); + } + + @Test + public void testOnUpdateAppStateGeneratesNewSessionIdOnForegroundState() { + String oldSessionId = SessionManager.getInstance().perfSession().sessionId(); + + assertThat(oldSessionId).isNotNull(); + assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId()); + + SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.FOREGROUND); + assertThat(oldSessionId).isNotEqualTo(SessionManager.getInstance().perfSession().sessionId()); + } + + @Test + public void testOnUpdateAppStateDoesntGenerateNewSessionIdOnBackgroundState() { + String oldSessionId = SessionManager.getInstance().perfSession().sessionId(); + + assertThat(oldSessionId).isNotNull(); + assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId()); + + SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.BACKGROUND); + assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId()); + } + + @Test + public void testOnUpdateAppStateGeneratesNewSessionIdOnBackgroundStateIfPerfSessionExpires() { + when(mockPerfSession.isSessionRunningTooLong()).thenReturn(true); + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + String oldSessionId = testSessionManager.perfSession().sessionId(); + + assertThat(oldSessionId).isNotNull(); + assertThat(oldSessionId).isEqualTo(testSessionManager.perfSession().sessionId()); + + testSessionManager.onUpdateAppState(ApplicationProcessState.BACKGROUND); + assertThat(oldSessionId).isNotEqualTo(testSessionManager.perfSession().sessionId()); + } + + @Test + public void + testOnUpdateAppStateMakesGaugeManagerLogGaugeMetadataOnForegroundStateIfSessionIsVerbose() { + forceVerboseSession(); + + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + testSessionManager.onUpdateAppState(ApplicationProcessState.FOREGROUND); + + verify(mockGaugeManager) + .logGaugeMetadata( + anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class)); + } + + @Test + public void + testOnUpdateAppStateDoesntMakeGaugeManagerLogGaugeMetadataOnForegroundStateIfSessionIsNonVerbose() { + forceNonVerboseSession(); + + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + testSessionManager.onUpdateAppState(ApplicationProcessState.FOREGROUND); + + verify(mockGaugeManager, never()) + .logGaugeMetadata( + anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class)); + } + + @Test + public void + testOnUpdateAppStateDoesntMakeGaugeManagerLogGaugeMetadataOnBackgroundStateEvenIfSessionIsVerbose() { + forceVerboseSession(); + + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + testSessionManager.onUpdateAppState(ApplicationProcessState.BACKGROUND); + + verify(mockGaugeManager, never()) + .logGaugeMetadata( + anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class)); + } + + @Test + public void + testOnUpdateAppStateMakesGaugeManagerLogGaugeMetadataOnBackgroundAppStateIfSessionIsVerboseAndTimedOut() { + when(mockPerfSession.isSessionRunningTooLong()).thenReturn(true); + forceVerboseSession(); + + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + testSessionManager.onUpdateAppState(ApplicationProcessState.BACKGROUND); + + verify(mockGaugeManager) + .logGaugeMetadata( + anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class)); + } + + @Test + public void testOnUpdateAppStateMakesGaugeManagerStartCollectingGaugesIfSessionIsVerbose() { + forceVerboseSession(); + + SessionManager testSessionManager = + new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + testSessionManager.onUpdateAppState(ApplicationProcessState.FOREGROUND); + + verify(mockGaugeManager) + .startCollectingGauges(AdditionalMatchers.not(eq(mockPerfSession)), any()); + } + // LogGaugeData on new perf session when Verbose // NotLogGaugeData on new perf session when not Verbose // Mark Session as expired after time limit.