Use AQS provided sessionId for Fireperf events. (#5060)

Author: visumickey

firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java

@@ -15,13 +15,17 @@
 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.FirebaseSessions;
+import com.google.firebase.sessions.api.SessionSubscriber;
 import java.util.concurrent.Executor;
 
 /**
@@ -34,7 +38,10 @@
 public class FirebasePerfEarly {
 
   public FirebasePerfEarly(
-      FirebaseApp app, @Nullable StartupTime startupTime, Executor uiExecutor) {
+      FirebaseApp app,
+      FirebaseSessions firebaseSessions,
+      @Nullable StartupTime startupTime,
+      Executor uiExecutor) {
     Context context = app.getApplicationContext();
 
     // Initialize ConfigResolver early for accessing device caching layer.
@@ -51,6 +58,27 @@ public FirebasePerfEarly(
       uiExecutor.execute(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
     }
 
+    // Register with Firebase sessions to receive updates about session changes.
+    firebaseSessions.register(
+        new SessionSubscriber() {
+          @Override
+          public void onSessionChanged(@NonNull SessionDetails sessionDetails) {
+            PerfSession perfSession = PerfSession.createWithId(sessionDetails.getSessionId());
+            SessionManager.getInstance().updatePerfSession(perfSession);
+          }
+
+          @Override
+          public boolean isDataCollectionEnabled() {
+            return configResolver.isPerformanceMonitoringEnabled();
+          }
+
+          @NonNull
+          @Override
+          public Name getSessionSubscriberName() {
+            return SessionSubscriber.Name.PERFORMANCE;
+          }
+        });
+
     // In the case of cold start, we create a session and start collecting gauges as early as
     // possible.
     // There is code in SessionManager that prevents us from resetting the session twice in case

firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java

@@ -65,20 +65,21 @@ public List<Component<?>> getComponents() {
             .add(Dependency.requiredProvider(RemoteConfigComponent.class))
             .add(Dependency.required(FirebaseInstallationsApi.class))
             .add(Dependency.requiredProvider(TransportFactory.class))
-            .add(Dependency.required(FirebaseSessions.class))
             .add(Dependency.required(FirebasePerfEarly.class))
             .factory(FirebasePerfRegistrar::providesFirebasePerformance)
             .build(),
         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()
             .factory(
                 container ->
                     new FirebasePerfEarly(
                         container.get(FirebaseApp.class),
+                        container.get(FirebaseSessions.class),
                         container.getProvider(StartupTime.class).get(),
                         container.get(uiExecutor)))
             .build(),
@@ -102,8 +103,7 @@ private static FirebasePerformance providesFirebasePerformance(ComponentContaine
                     container.get(FirebaseApp.class),
                     container.get(FirebaseInstallationsApi.class),
                     container.getProvider(RemoteConfigComponent.class),
-                    container.getProvider(TransportFactory.class),
-                    container.get(FirebaseSessions.class)))
+                    container.getProvider(TransportFactory.class)))
             .build();
 
     return component.getFirebasePerformance();

firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerformance.java

@@ -42,8 +42,6 @@
 import com.google.firebase.perf.util.ImmutableBundle;
 import com.google.firebase.perf.util.Timer;
 import com.google.firebase.remoteconfig.RemoteConfigComponent;
-import com.google.firebase.sessions.FirebaseSessions;
-import com.google.firebase.sessions.api.SessionSubscriber;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.net.URL;
@@ -142,7 +140,6 @@ public static FirebasePerformance getInstance() {
   private final Provider<RemoteConfigComponent> firebaseRemoteConfigProvider;
   private final FirebaseInstallationsApi firebaseInstallationsApi;
   private final Provider<TransportFactory> transportFactoryProvider;
-  private final FirebaseSessions firebaseSessions;
 
   /**
    * Constructs the FirebasePerformance class and allows injecting dependencies.
@@ -166,15 +163,13 @@ public static FirebasePerformance getInstance() {
       FirebaseInstallationsApi firebaseInstallationsApi,
       Provider<TransportFactory> transportFactoryProvider,
       RemoteConfigManager remoteConfigManager,
-      FirebaseSessions firebaseSessions,
       ConfigResolver configResolver,
       SessionManager sessionManager) {
 
     this.firebaseApp = firebaseApp;
     this.firebaseRemoteConfigProvider = firebaseRemoteConfigProvider;
     this.firebaseInstallationsApi = firebaseInstallationsApi;
     this.transportFactoryProvider = transportFactoryProvider;
-    this.firebaseSessions = firebaseSessions;
 
     if (firebaseApp == null) {
       this.mPerformanceCollectionForceEnabledState = false;
@@ -204,26 +199,6 @@ public static FirebasePerformance getInstance() {
               ConsoleUrlGenerator.generateDashboardUrl(
                   firebaseApp.getOptions().getProjectId(), appContext.getPackageName())));
     }
-
-    // Register with Firebase sessions to receive updates about session changes.
-    this.firebaseSessions.register(
-        new SessionSubscriber() {
-          @Override
-          public void onSessionChanged(@NonNull SessionDetails sessionDetails) {
-            // TODO(visum) Handle sessionID change by updating the sessionID in the sessionManager
-          }
-
-          @Override
-          public boolean isDataCollectionEnabled() {
-            return configResolver.isPerformanceMonitoringEnabled();
-          }
-
-          @NonNull
-          @Override
-          public Name getSessionSubscriberName() {
-            return SessionSubscriber.Name.PERFORMANCE;
-          }
-        });
   }
 
   /**

firebase-perf/src/main/java/com/google/firebase/perf/injection/modules/FirebasePerformanceModule.java

@@ -24,7 +24,6 @@
 import com.google.firebase.perf.config.RemoteConfigManager;
 import com.google.firebase.perf.session.SessionManager;
 import com.google.firebase.remoteconfig.RemoteConfigComponent;
-import com.google.firebase.sessions.FirebaseSessions;
 import dagger.Module;
 import dagger.Provides;
 
@@ -35,19 +34,16 @@ public class FirebasePerformanceModule {
   private final FirebaseInstallationsApi firebaseInstallations;
   private final Provider<RemoteConfigComponent> remoteConfigComponentProvider;
   private final Provider<TransportFactory> transportFactoryProvider;
-  private final FirebaseSessions firebaseSessions;
 
   public FirebasePerformanceModule(
       @NonNull FirebaseApp firebaseApp,
       @NonNull FirebaseInstallationsApi firebaseInstallations,
       @NonNull Provider<RemoteConfigComponent> remoteConfigComponentProvider,
-      @NonNull Provider<TransportFactory> transportFactoryProvider,
-      @NonNull FirebaseSessions firebaseSessions) {
+      @NonNull Provider<TransportFactory> transportFactoryProvider) {
     this.firebaseApp = firebaseApp;
     this.firebaseInstallations = firebaseInstallations;
     this.remoteConfigComponentProvider = remoteConfigComponentProvider;
     this.transportFactoryProvider = transportFactoryProvider;
-    this.firebaseSessions = firebaseSessions;
   }
 
   @Provides
@@ -84,9 +80,4 @@ ConfigResolver providesConfigResolver() {
   SessionManager providesSessionManager() {
     return SessionManager.getInstance();
   }
-
-  @Provides
-  FirebaseSessions providesFirebaseSessions() {
-    return firebaseSessions;
-  }
 }

firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java

@@ -24,7 +24,6 @@
 import com.google.firebase.perf.util.Timer;
 import com.google.firebase.perf.v1.SessionVerbosity;
 import java.util.List;
-import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
 /** Details of a session including a unique Id and related information. */
@@ -38,10 +37,9 @@ public class PerfSession implements Parcelable {
   /*
    * Creates a PerfSession object and decides what metrics to collect.
    */
-  public static PerfSession create() {
-    String sessionId = UUID.randomUUID().toString().replace("-", "");
-
-    PerfSession session = new PerfSession(sessionId, new Clock());
+  public static PerfSession createWithId(@NonNull String sessionId) {
+    String prunedSessionId = sessionId.replace("-", "");
+    PerfSession session = new PerfSession(prunedSessionId, new Clock());
     session.setGaugeAndEventCollectionEnabled(shouldCollectGaugesAndEvents());
 
     return session;
@@ -108,7 +106,7 @@ static boolean isVerbose(@NonNull com.google.firebase.perf.v1.PerfSession perfSe
    * Checks if it has been more than {@link ConfigResolver#getSessionsMaxDurationMinutes()} time
    * since the creation time of the current session.
    */
-  public boolean isExpired() {
+  public boolean isSessionRunningTooLong() {
     return TimeUnit.MICROSECONDS.toMinutes(creationTime.getDurationMicros())
         > ConfigResolver.getInstance().getSessionsMaxDurationMinutes();
   }

firebase-perf/src/main/java/com/google/firebase/perf/session/SessionManager.java

@@ -19,7 +19,6 @@
 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;
@@ -34,7 +33,7 @@
 
 /** Session manager to generate sessionIDs and broadcast to the application. */
 @Keep // Needed because of b/117526359.
-public class SessionManager extends AppStateUpdateHandler {
+public class SessionManager {
 
   @SuppressLint("StaticFieldLeak")
   private static final SessionManager instance = new SessionManager();
@@ -57,7 +56,8 @@ public final PerfSession perfSession() {
   }
 
   private SessionManager() {
-    this(GaugeManager.getInstance(), PerfSession.create(), AppStateMonitor.getInstance());
+    // Start with an empty session ID as the firebase sessions will override with real Id.
+    this(GaugeManager.getInstance(), PerfSession.createWithId(""), AppStateMonitor.getInstance());
   }
 
   @VisibleForTesting
@@ -66,14 +66,11 @@ public SessionManager(
     this.gaugeManager = gaugeManager;
     this.perfSession = perfSession;
     this.appStateMonitor = appStateMonitor;
-    registerForAppState();
   }
 
   /**
-   * Finalizes gauge initialization during app start. This must be called before app start finishes
-   * (currently that is before onResume finishes), because {@link #perfSession} can be changed by
-   * {@link #onUpdateAppState(ApplicationProcessState)} once {@link AppStateMonitor#isColdStart()}
-   * becomes false.
+   * Finalizes gauge initialization during cold start. This must be called before app start finishes
+   * (currently that is before onResume finishes) to ensure gauge collection starts on time.
    */
   public void setApplicationContext(final Context appContext) {
     // Get PerfSession in main thread first, because it is possible that app changes fg/bg state
@@ -93,51 +90,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) {
-      updatePerfSession(newAppState);
-
-    } else if (!updatePerfSessionIfExpired()) {
-      startOrStopCollectingGauges(newAppState);
-    }
-  }
-
   /**
-   * Updates the current {@link PerfSession} if it has expired/timed out.
+   * Checks if the current {@link PerfSession} is expired/timed out. If so, stop collecting gauges.
    *
-   * @return {@code true} if {@link PerfSession} is updated, {@code false} otherwise.
-   * @see PerfSession#isExpired()
+   * @see PerfSession#isSessionRunningTooLong()
    */
-  public boolean updatePerfSessionIfExpired() {
-    if (perfSession.isExpired()) {
-      updatePerfSession(appStateMonitor.getAppState());
-      return true;
+  public void stopGaugeCollectionIfSessionRunningTooLong() {
+    if (perfSession.isSessionRunningTooLong()) {
+      gaugeManager.stopCollectingGauges();
     }
-
-    return false;
   }
 
   /**
-   * Updates the currently associated {@link #perfSession} and broadcast the event.
+   * Updates the currently associated {@link #perfSession} and broadcast the change.
    *
-   * <p>Creation of new {@link PerfSession} will log the {@link GaugeMetadata} and start/stop the
-   * collection of {@link GaugeMetric} depending upon Session verbosity.
+   * <p>Uses the provided PerfSession {@link PerfSession}, log the {@link GaugeMetadata} and
+   * start/stop the collection of {@link GaugeMetric} depending upon Session verbosity.
    *
    * @see PerfSession#isVerbose()
    */
-  public void updatePerfSession(ApplicationProcessState currentAppState) {
-    synchronized (clients) {
-      perfSession = PerfSession.create();
+  public void updatePerfSession(PerfSession perfSession) {
+    // Do not update the perf session if it is the exact same sessionId.
+    if (perfSession.sessionId() == this.perfSession.sessionId()) {
+      return;
+    }
 
+    this.perfSession = perfSession;
+
+    synchronized (clients) {
       for (Iterator<WeakReference<SessionAwareObject>> i = clients.iterator(); i.hasNext(); ) {
         SessionAwareObject callback = i.next().get();
         if (callback != null) {
@@ -150,8 +130,11 @@ public void updatePerfSession(ApplicationProcessState currentAppState) {
       }
     }
 
-    logGaugeMetadataIfCollectionEnabled(currentAppState);
-    startOrStopCollectingGauges(currentAppState);
+    // Log the gauge metadata event if data collection is enabled.
+    logGaugeMetadataIfCollectionEnabled(appStateMonitor.getAppState());
+
+    // Start of stop the gauge data collection.
+    startOrStopCollectingGauges(appStateMonitor.getAppState());
   }
 
   /**

firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java

@@ -381,9 +381,8 @@ private void syncLog(PerfMetric.Builder perfMetricBuilder, ApplicationProcessSta
     if (isAllowedToDispatch(perfMetric)) {
       dispatchLog(perfMetric);
 
-      // TODO(b/172008005): This might not be the best place for this call, consider utilizing a
-      //  callback in the SessionManager itself.
-      SessionManager.getInstance().updatePerfSessionIfExpired();
+      // Check if the session is expired. If so, stop gauge collection.
+      SessionManager.getInstance().stopGaugeCollectionIfSessionRunningTooLong();
     }
   }
 

firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java

@@ -50,7 +50,6 @@ public void testGetComponents() {
             Dependency.requiredProvider(RemoteConfigComponent.class),
             Dependency.required(FirebaseInstallationsApi.class),
             Dependency.requiredProvider(TransportFactory.class),
-            Dependency.required(FirebaseSessions.class),
             Dependency.required(FirebasePerfEarly.class));
 
     assertThat(firebasePerfComponent.isLazy()).isTrue();
@@ -61,6 +60,7 @@ 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();

firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerformanceTest.java

@@ -40,7 +40,6 @@
 import com.google.firebase.perf.util.Constants;
 import com.google.firebase.perf.util.ImmutableBundle;
 import com.google.firebase.remoteconfig.RemoteConfigComponent;
-import com.google.firebase.sessions.FirebaseSessions;
 import com.google.testing.timing.FakeDirectExecutorService;
 import java.util.Map;
 import org.junit.After;
@@ -579,7 +578,6 @@ private FirebasePerformance initializeFirebasePerformancePreferences(
         mock(FirebaseInstallationsApi.class),
         transportFactoryProvider,
         spyRemoteConfigManager,
-        mock(FirebaseSessions.class),
         spyConfigResolver,
         spySessionManager);
   }