Update Gauge logging to better match the existing implementation

firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt

@@ -17,8 +17,6 @@
 package com.google.firebase.perf.session
 
 import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck
-import com.google.firebase.perf.session.gauges.GaugeManager
-import com.google.firebase.perf.v1.ApplicationProcessState
 import com.google.firebase.sessions.api.SessionSubscriber
 
 class FirebasePerformanceSessionSubscriber(override val isDataCollectionEnabled: Boolean) :
@@ -33,7 +31,5 @@ class FirebasePerformanceSessionSubscriber(override val isDataCollectionEnabled:
 
     val updatedSession = PerfSession.createWithId(sessionDetails.sessionId)
     SessionManager.getInstance().updatePerfSession(updatedSession)
-    GaugeManager.getInstance()
-      .logGaugeMetadata(updatedSession.sessionId(), ApplicationProcessState.FOREGROUND)
   }
 }

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

@@ -115,6 +115,9 @@ public void updatePerfSession(PerfSession perfSession) {
       }
     }
 
+    // Log gauge metadata.
+    logGaugeMetadataIfCollectionEnabled();
+
     // Start of stop the gauge data collection.
     startOrStopCollectingGauges();
   }
@@ -153,6 +156,14 @@ public void unregisterForSessionUpdates(WeakReference<SessionAwareObject> client
     }
   }
 
+  private void logGaugeMetadataIfCollectionEnabled() {
+    FirebaseSessionsEnforcementCheck.checkSession(
+        perfSession, "logGaugeMetadataIfCollectionEnabled");
+    if (perfSession.isVerbose()) {
+      gaugeManager.logGaugeMetadata(perfSession.sessionId());
+    }
+  }
+
   private void startOrStopCollectingGauges() {
     FirebaseSessionsEnforcementCheck.checkSession(perfSession, "startOrStopCollectingGauges");
 

firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt

@@ -15,17 +15,16 @@
 package com.google.firebase.perf.session.gauges
 
 import androidx.annotation.VisibleForTesting
-import com.google.firebase.perf.logging.AndroidLogger
 import java.util.concurrent.atomic.AtomicInteger
 
 /**
- * [GaugeCounter] is a thread-safe counter for gauge metrics. If the metrics count exceeds
- * [MAX_METRIC_COUNT], it attempts to log the metrics to Firelog.
+ * [GaugeCounter] is a thread-safe counter for gauge metrics. If the metrics count reaches or
+ * exceeds [MAX_METRIC_COUNT], it attempts to log the metrics to Firelog.
  */
 object GaugeCounter {
-  private const val MAX_METRIC_COUNT = 25
+  private const val MAX_METRIC_COUNT = 50
+  // For debugging explore re-introducing logging.
   private val counter = AtomicInteger(0)
-  private val logger = AndroidLogger.getInstance()
 
   @set:VisibleForTesting(otherwise = VisibleForTesting.NONE)
   @set:JvmStatic
@@ -36,16 +35,16 @@ object GaugeCounter {
     val metricsCount = counter.incrementAndGet()
 
     if (metricsCount >= MAX_METRIC_COUNT) {
+      // TODO(b/394127311): There can be rare conditions where there's an attempt to log metrics
+      //  even when it's currently logging them. While this is a no-op, it might be worth
+      //  exploring optimizing it further to prevent additional calls to [GaugeManager].
       gaugeManager.logGaugeMetrics()
     }
-
-    logger.debug("Incremented logger to $metricsCount")
   }
 
   @JvmStatic
   fun decrementCounter() {
-    val curr = counter.decrementAndGet()
-    logger.debug("Decremented logger to $curr")
+    counter.decrementAndGet()
   }
 
   @VisibleForTesting(otherwise = VisibleForTesting.NONE)

firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java

@@ -274,18 +274,17 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) {
    * Log the Gauge Metadata information to the transport.
    *
    * @param sessionId The {@link PerfSession#sessionId()} ()} to which the collected Gauge Metrics
-   *     should be associated with.
-   * @param appState The {@link ApplicationProcessState} for which these gauges are collected.
+   *                  should be associated with.
    * @return true if GaugeMetadata was logged, false otherwise.
    */
-  public boolean logGaugeMetadata(String sessionId, ApplicationProcessState appState) {
+  public boolean logGaugeMetadata(String sessionId) {
     if (gaugeMetadataManager != null) {
       GaugeMetric gaugeMetric =
           GaugeMetric.newBuilder()
               .setSessionId(sessionId)
               .setGaugeMetadata(getGaugeMetadata())
               .build();
-      transportManager.log(gaugeMetric, appState);
+      transportManager.log(gaugeMetric);
       return true;
     }
     return false;

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

@@ -356,7 +356,6 @@ public void log(final GaugeMetric gaugeMetric) {
    * {@link #isAllowedToDispatch(PerfMetric)}).
    */
   public void log(final GaugeMetric gaugeMetric, final ApplicationProcessState appState) {
-    // TODO(b/394127311): This *might* potentially be the right place to get AQS.
     executorService.execute(
         () -> syncLog(PerfMetric.newBuilder().setGaugeMetric(gaugeMetric), appState));
   }

firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java

@@ -60,12 +60,14 @@ public final class GaugeManagerTest extends FirebasePerformanceTestBase {
   // This is a guesstimate of the max amount of time to wait before any pending metrics' collection
   // might take.
   private static final long TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS = 20;
-  private static final long APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC = 20;
   private static final long DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_BG_MS = 100;
   private static final long DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS = 50;
   private static final long DEFAULT_MEMORY_GAUGE_COLLECTION_FREQUENCY_BG_MS = 120;
   private static final long DEFAULT_MEMORY_GAUGE_COLLECTION_FREQUENCY_FG_MS = 60;
 
+  // See [com.google.firebase.perf.session.gauges.GaugeCounter].
+  private static final long MAX_GAUGE_COUNTER_LIMIT = 50;
+
   private GaugeManager testGaugeManager = null;
   private FakeScheduledExecutorService fakeScheduledExecutorService = null;
   private TransportManager mockTransportManager = null;
@@ -340,8 +342,7 @@ public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() {
     // There's no job to log the gauges.
     assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
 
-    // Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
-    generateMetricsAndIncrementCounter(20);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     // There's still no job to log the gauges.
     assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -366,7 +367,7 @@ public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() {
     int recordedGaugeMetricsCount =
         recordedGaugeMetric.getAndroidMemoryReadingsCount()
             + recordedGaugeMetric.getCpuMetricReadingsCount();
-    assertThat(recordedGaugeMetricsCount).isEqualTo(30);
+    assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT);
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
   }
@@ -383,8 +384,7 @@ public void testGaugeCounterIsDecrementedWhenLogged() {
     // There's no job to log the gauges.
     assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
 
-    // Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
-    generateMetricsAndIncrementCounter(20);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     // There's still no job to log the gauges.
     assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -395,9 +395,47 @@ public void testGaugeCounterIsDecrementedWhenLogged() {
     assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS))
         .isEqualTo(TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS);
 
-    assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter + 30);
+    assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter + MAX_GAUGE_COUNTER_LIMIT);
+    fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask();
+
+    assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter);
+  }
+
+  @Test
+  public void testDuplicateGaugeLoggingIsAvoided() {
+    int priorGaugeCounter = GaugeCounter.count();
+    PerfSession fakeSession = createTestSession(1);
+    testGaugeManager.setApplicationProcessState(ApplicationProcessState.FOREGROUND);
+    testGaugeManager.startCollectingGauges(fakeSession);
+    GaugeCounter.setGaugeManager(testGaugeManager);
+
+    // There's no job to log the gauges.
+    assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
+
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 20);
+
+    // There's still no job to log the gauges.
+    assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
+
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT);
+
+    assertThat(fakeScheduledExecutorService.isEmpty()).isFalse();
+    assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS))
+        .isEqualTo(TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS);
+
     fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask();
+    assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
 
+    GaugeMetric recordedGaugeMetric =
+        getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
+
+    // It flushes all the metrics in the ConcurrentLinkedQueues that were added.
+    int recordedGaugeMetricsCount =
+        recordedGaugeMetric.getAndroidMemoryReadingsCount()
+            + recordedGaugeMetric.getCpuMetricReadingsCount();
+    assertThat(recordedGaugeMetricsCount).isEqualTo(2 * MAX_GAUGE_COUNTER_LIMIT - 20);
+
+    assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
     assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter);
   }
 
@@ -410,7 +448,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
     GaugeCounter.setGaugeManager(testGaugeManager);
 
     // Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
-    generateMetricsAndIncrementCounter(10);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     // There's no job to log the gauges.
     assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -425,7 +463,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
     shadowOf(Looper.getMainLooper()).idle();
 
     // Generate additional metrics in the new app state.
-    generateMetricsAndIncrementCounter(26);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT + 1);
 
     GaugeMetric recordedGaugeMetric =
         getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
@@ -434,7 +472,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
     int recordedGaugeMetricsCount =
         recordedGaugeMetric.getAndroidMemoryReadingsCount()
             + recordedGaugeMetric.getCpuMetricReadingsCount();
-    assertThat(recordedGaugeMetricsCount).isEqualTo(10);
+    assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
 
@@ -448,7 +486,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
     recordedGaugeMetricsCount =
         recordedGaugeMetric.getAndroidMemoryReadingsCount()
             + recordedGaugeMetric.getCpuMetricReadingsCount();
-    assertThat(recordedGaugeMetricsCount).isEqualTo(26);
+    assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT + 1);
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
   }
@@ -462,7 +500,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
     GaugeCounter.setGaugeManager(testGaugeManager);
 
     // Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
-    generateMetricsAndIncrementCounter(10);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     PerfSession updatedPerfSession = createTestSession(2);
     updatedPerfSession.setGaugeAndEventCollectionEnabled(true);
@@ -479,7 +517,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
     shadowOf(Looper.getMainLooper()).idle();
 
     // Generate metrics for the new session.
-    generateMetricsAndIncrementCounter(26);
+    generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT + 1);
 
     GaugeMetric recordedGaugeMetric =
         getLastRecordedGaugeMetric(ApplicationProcessState.BACKGROUND);
@@ -488,7 +526,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
     int recordedGaugeMetricsCount =
         recordedGaugeMetric.getAndroidMemoryReadingsCount()
             + recordedGaugeMetric.getCpuMetricReadingsCount();
-    assertThat(recordedGaugeMetricsCount).isEqualTo(10);
+    assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT - 10);
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
 
@@ -502,7 +540,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
     recordedGaugeMetricsCount =
         recordedGaugeMetric.getAndroidMemoryReadingsCount()
             + recordedGaugeMetric.getCpuMetricReadingsCount();
-    assertThat(recordedGaugeMetricsCount).isEqualTo(26);
+    assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT + 1);
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(2));
   }
@@ -559,10 +597,10 @@ public void testLogGaugeMetadataSendDataToTransport() {
     when(fakeGaugeMetadataManager.getMaxAppJavaHeapMemoryKb()).thenReturn(1000);
     when(fakeGaugeMetadataManager.getMaxEncouragedAppJavaHeapMemoryKb()).thenReturn(800);
 
-    testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND);
+    testGaugeManager.logGaugeMetadata(testSessionId(1));
 
     GaugeMetric recordedGaugeMetric =
-        getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
+        getLastRecordedGaugeMetric(ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN);
     GaugeMetadata recordedGaugeMetadata = recordedGaugeMetric.getGaugeMetadata();
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
@@ -586,9 +624,7 @@ public void testLogGaugeMetadataDoesNotLogWhenGaugeMetadataManagerNotAvailable()
             new Lazy<>(() -> fakeCpuGaugeCollector),
             new Lazy<>(() -> fakeMemoryGaugeCollector));
 
-    assertThat(
-            testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
-        .isFalse();
+    assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isFalse();
   }
 
   @Test
@@ -603,17 +639,13 @@ public void testLogGaugeMetadataLogsAfterApplicationContextIsSet() {
             new Lazy<>(() -> fakeCpuGaugeCollector),
             new Lazy<>(() -> fakeMemoryGaugeCollector));
 
-    assertThat(
-            testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
-        .isFalse();
+    assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isFalse();
 
     testGaugeManager.initializeGaugeMetadataManager(ApplicationProvider.getApplicationContext());
-    assertThat(
-            testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
-        .isTrue();
+    assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isTrue();
 
     GaugeMetric recordedGaugeMetric =
-        getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
+        getLastRecordedGaugeMetric(ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN);
     GaugeMetadata recordedGaugeMetadata = recordedGaugeMetric.getGaugeMetadata();
 
     assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
@@ -638,7 +670,7 @@ private long getMinimumBackgroundCollectionFrequency() {
   }
 
   // Simulates the behavior of Cpu and Memory Gauge collector.
-  private void generateMetricsAndIncrementCounter(int count) {
+  private void generateMetricsAndIncrementCounter(long count) {
     // TODO(b/394127311): Explore actually collecting metrics using the fake Cpu and Memory
     //  metric collectors.
     Random random = new Random();
@@ -679,8 +711,16 @@ private AndroidMemoryReading createFakeAndroidMetricReading(int currentUsedAppJa
   private GaugeMetric getLastRecordedGaugeMetric(
       ApplicationProcessState expectedApplicationProcessState) {
     ArgumentCaptor<GaugeMetric> argMetric = ArgumentCaptor.forClass(GaugeMetric.class);
-    verify(mockTransportManager, times(1))
-        .log(argMetric.capture(), eq(expectedApplicationProcessState));
+
+    // TODO(b/394127311): Revisit transportManager.log method which is only being called in unit
+    //  tests.
+    if (expectedApplicationProcessState
+        == ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN) {
+      verify(mockTransportManager, times(1)).log(argMetric.capture());
+    } else {
+      verify(mockTransportManager, times(1))
+          .log(argMetric.capture(), eq(expectedApplicationProcessState));
+    }
     reset(mockTransportManager);
     // Required after resetting the mock. By default we assume that Transport is initialized.
     when(mockTransportManager.isInitialized()).thenReturn(true);