diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt b/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt index d3e13c63aa9..71062422d51 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/FirebasePerformanceSessionSubscriber.kt +++ b/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) } } 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 5f01a50a032..6fceb64d1e5 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 @@ -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 client } } + private void logGaugeMetadataIfCollectionEnabled() { + FirebaseSessionsEnforcementCheck.checkSession( + perfSession, "logGaugeMetadataIfCollectionEnabled"); + if (perfSession.isVerbose()) { + gaugeManager.logGaugeMetadata(perfSession.sessionId()); + } + } + private void startOrStopCollectingGauges() { FirebaseSessionsEnforcementCheck.checkSession(perfSession, "startOrStopCollectingGauges"); diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt index 72093922cbc..bb91258d73d 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt +++ b/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) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java index ad3b19eb272..372c961257a 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeManager.java +++ b/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; diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java index 54d5b0792c3..a38fb666c85 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/transport/TransportManager.java +++ b/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)); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java index e0e1f8d8d48..c111f5fb7a2 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java +++ b/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 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);