From 79623cb4c3b5275c4d1b2ab7bd7eac07cb54af10 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 5 May 2025 17:03:36 -0400 Subject: [PATCH 01/17] Update gauge collection based on app update state --- .../firebase/perf/session/SessionManager.java | 2 +- .../perf/session/gauges/GaugeManager.java | 77 ++++++++----------- .../perf/session/SessionManagerTest.java | 3 +- .../perf/session/gauges/GaugeManagerTest.java | 77 +++++++++++-------- 4 files changed, 80 insertions(+), 79 deletions(-) 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 5a8540a6ffb..85da11bc6ac 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 @@ -165,7 +165,7 @@ private void startOrStopCollectingGauges(ApplicationProcessState appState) { perfSession, "Session is not ready while trying to startOrStopCollectingGauges"); if (perfSession.isGaugeAndEventCollectionEnabled()) { - gaugeManager.startCollectingGauges(perfSession, appState); + gaugeManager.startCollectingGauges(perfSession); } else { gaugeManager.stopCollectingGauges(); } 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 1c06ceac9dd..8a769902ec9 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 @@ -20,6 +20,7 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.firebase.components.Lazy; +import com.google.firebase.perf.application.AppStateUpdateHandler; import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.logging.AndroidLogger; import com.google.firebase.perf.session.PerfSession; @@ -31,7 +32,6 @@ import com.google.firebase.perf.v1.GaugeMetadata; import com.google.firebase.perf.v1.GaugeMetric; import java.util.concurrent.Executors; -import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -41,7 +41,7 @@ * information and logging it to the Transport. */ @Keep // Needed because of b/117526359. -public class GaugeManager { +public class GaugeManager extends AppStateUpdateHandler { private static final AndroidLogger logger = AndroidLogger.getInstance(); private static final GaugeManager instance = new GaugeManager(); @@ -59,8 +59,8 @@ public class GaugeManager { private final TransportManager transportManager; @Nullable private GaugeMetadataManager gaugeMetadataManager; - @Nullable private ScheduledFuture gaugeManagerDataCollectionJob = null; - @Nullable private String sessionId = null; + @Nullable private ScheduledFuture gaugeManagerDataCollectionJob = null; + @Nullable private PerfSession session = null; private ApplicationProcessState applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; @@ -91,6 +91,7 @@ private GaugeManager() { this.gaugeMetadataManager = gaugeMetadataManager; this.cpuGaugeCollector = cpuGaugeCollector; this.memoryGaugeCollector = memoryGaugeCollector; + registerForAppState(); } /** Initializes GaugeMetadataManager which requires application context. */ @@ -98,6 +99,19 @@ public void initializeGaugeMetadataManager(Context appContext) { this.gaugeMetadataManager = new GaugeMetadataManager(appContext); } + @Override + public void onUpdateAppState(ApplicationProcessState applicationProcessState) { + this.applicationProcessState = applicationProcessState; + + if (session == null) { + // If the session is null, it means there's no gauges being collected. + return; + } + + stopCollectingGauges(); + startCollectingGauges(this.applicationProcessState, session.getTimer()); + } + /** Returns the singleton instance of this class. */ public static synchronized GaugeManager getInstance() { return instance; @@ -112,47 +126,22 @@ public static synchronized GaugeManager getInstance() { * will then be associated with the same or new sessionId and applicationProcessState. * * @param session The {@link PerfSession} to which the collected gauges will be associated with. - * @param applicationProcessState The {@link ApplicationProcessState} the collected GaugeMetrics - * will be associated with. * @note: This method is NOT thread safe - {@link this.startCollectingGauges()} and {@link - * this.stopCollectingGauges()} should always be called from the same thread. + * this.stopCollectingGauges()} should always be called from the same thread. */ - public void startCollectingGauges( - PerfSession session, ApplicationProcessState applicationProcessState) { - if (this.sessionId != null) { + public void startCollectingGauges(PerfSession session) { + if (this.session != null) { stopCollectingGauges(); } - long collectionFrequency = startCollectingGauges(applicationProcessState, session.getTimer()); + long collectionFrequency = + startCollectingGauges(this.applicationProcessState, session.getTimer()); if (collectionFrequency == INVALID_GAUGE_COLLECTION_FREQUENCY) { logger.warn("Invalid gauge collection frequency. Unable to start collecting Gauges."); return; } - this.sessionId = session.sessionId(); - this.applicationProcessState = applicationProcessState; - - // This is needed, otherwise the Runnable might use a stale value. - final String sessionIdForScheduledTask = sessionId; - final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; - - // TODO(b/394127311): Switch to using AQS. - try { - gaugeManagerDataCollectionJob = - gaugeManagerExecutor - .get() - .scheduleWithFixedDelay( - () -> { - syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); - }, - /* initialDelay= */ collectionFrequency - * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, - /* period= */ collectionFrequency * APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC, - TimeUnit.MILLISECONDS); - - } catch (RejectedExecutionException e) { - logger.warn("Unable to start collecting Gauges: " + e.getMessage()); - } + this.session = session; } /** @@ -190,12 +179,12 @@ private long startCollectingGauges(ApplicationProcessState appState, Timer refer * this.stopCollectingGauges()} should always be called from the same thread. */ public void stopCollectingGauges() { - if (this.sessionId == null) { + if (session == null) { return; } // This is needed, otherwise the Runnable might use a stale value. - final String sessionIdForScheduledTask = sessionId; + final String sessionIdForScheduledTask = session.sessionId(); final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; cpuGaugeCollector.get().stopCollecting(); @@ -205,10 +194,9 @@ public void stopCollectingGauges() { gaugeManagerDataCollectionJob.cancel(false); } - // TODO(b/394127311): Switch to using AQS. // Flush any data that was collected for this session one last time. @SuppressWarnings("FutureReturnValueIgnored") - ScheduledFuture unusedFuture = + ScheduledFuture unusedFuture = gaugeManagerExecutor .get() .schedule( @@ -218,7 +206,7 @@ public void stopCollectingGauges() { TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, TimeUnit.MILLISECONDS); - this.sessionId = null; + this.session = null; this.applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; } @@ -227,7 +215,7 @@ public void stopCollectingGauges() { * proto and logs it to transport. * * @param sessionId The sessionId to which the collected GaugeMetrics should be associated with. - * @param appState The app state for which these gauges are collected. + * @param appState The app state for which these gauges are attributed to. */ private void syncFlush(String sessionId, ApplicationProcessState appState) { GaugeMetric.Builder gaugeMetricBuilder = GaugeMetric.newBuilder(); @@ -244,7 +232,6 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) { } // Adding Session ID info. - // TODO(b/394127311): Switch to using AQS. gaugeMetricBuilder.setSessionId(sessionId); transportManager.log(gaugeMetricBuilder.build(), appState); @@ -253,16 +240,16 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) { /** * Log the Gauge Metadata information to the transport. * - * @param aqsSessionId The {@link PerfSession#aqsSessionId()} ()} to which the collected Gauge Metrics + * @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. * @return true if GaugeMetadata was logged, false otherwise. */ - public boolean logGaugeMetadata(String aqsSessionId, ApplicationProcessState appState) { + public boolean logGaugeMetadata(String sessionId, ApplicationProcessState appState) { if (gaugeMetadataManager != null) { GaugeMetric gaugeMetric = GaugeMetric.newBuilder() - .setSessionId(aqsSessionId) + .setSessionId(sessionId) .setGaugeMetadata(getGaugeMetadata()) .build(); transportManager.log(gaugeMetric, appState); 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 618dff747ff..81c6d0c8a16 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 @@ -144,8 +144,7 @@ public void testUpdatePerfSessionStartsCollectingGaugesIfSessionIsVerbose() { testSessionManager.setApplicationContext(mockApplicationContext); verify(mockGaugeManager, times(1)).initializeGaugeMetadataManager(mockApplicationContext); - verify(mockGaugeManager, times(1)) - .startCollectingGauges(newSession, ApplicationProcessState.FOREGROUND); + verify(mockGaugeManager, times(1)).startCollectingGauges(newSession); } @Test 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 61ae0c57132..9fe884cf656 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 @@ -43,6 +43,7 @@ import com.google.testing.timing.FakeScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -124,9 +125,10 @@ public void setUp() { } @Test + @Ignore // b/394127311 public void testStartCollectingGaugesStartsCollectingMetricsInBackgroundState() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); verify(fakeCpuGaugeCollector) .startCollecting( eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_BG_MS), @@ -138,9 +140,10 @@ public void testStartCollectingGaugesStartsCollectingMetricsInBackgroundState() } @Test + @Ignore // b/394127311 public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession); verify(fakeCpuGaugeCollector) .startCollecting( eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), @@ -155,8 +158,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() public void testStartCollectingGaugesDoesNotStartCollectingMetricsWithUnknownApplicationProcessState() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges( - fakeSession, ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN); + testGaugeManager.startCollectingGauges(fakeSession); verify(fakeCpuGaugeCollector, never()) .startCollecting(ArgumentMatchers.anyLong(), ArgumentMatchers.nullable(Timer.class)); verify(fakeMemoryGaugeCollector, never()) @@ -164,12 +166,13 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test + @Ignore // b/394127311 public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithValidFrequencyInBackground() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyBackgroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); // Verify that Cpu metric collection is not started verify(fakeCpuGaugeCollector, never()) @@ -182,7 +185,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyBackgroundMs(); PerfSession fakeSession2 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // Verify that Cpu metric collection is not started verify(fakeCpuGaugeCollector, never()) @@ -194,12 +197,13 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test + @Ignore // b/394127311 public void startCollectingGaugesOnBackground_invalidMemoryCaptureMs_onlyDisableMemoryCollection() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyBackgroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); // Verify that Memory metric collection is not started verify(fakeMemoryGaugeCollector, never()) @@ -212,7 +216,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyBackgroundMs(); PerfSession fakeSession2 = createTestSession(2); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // Verify that Memory metric collection is not started verify(fakeMemoryGaugeCollector, never()) @@ -224,11 +228,12 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test + @Ignore // b/394127311 public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithValidFrequency() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); // Verify that Cpu metric collection is not started verify(fakeCpuGaugeCollector, never()) @@ -241,7 +246,7 @@ public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithV // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); PerfSession fakeSession2 = createTestSession(2); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // Verify that Cpu metric collection is not started verify(fakeCpuGaugeCollector, never()) @@ -253,12 +258,13 @@ public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithV } @Test + @Ignore // b/394127311 public void startCollectingGaugesOnForeground_invalidMemoryCaptureMs_onlyDisableMemoryCollection() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); // Verify that Memory metric collection is not started verify(fakeMemoryGaugeCollector, never()) @@ -271,7 +277,7 @@ public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithV // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession2 = createTestSession(2); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // Verify that Memory metric collection is not started verify(fakeMemoryGaugeCollector, never()) @@ -285,42 +291,43 @@ public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithV @Test public void testStartCollectingGaugesDoesNotStartAJobToConsumeMetricsWithUnknownAppState() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges( - fakeSession, ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN); + testGaugeManager.startCollectingGauges(fakeSession); assertThat(fakeScheduledExecutorService.isEmpty()).isTrue(); } @Test + @Ignore // b/394127311 public void stopCollectingCPUMetrics_invalidCPUCaptureFrequency_appInForegrounf() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); PerfSession fakeSession2 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); } @Test + @Ignore // b/394127311 public void stopCollectingGauges_invalidMemoryCollectionFrequency_appInForeground() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); // PASS 2: Test with -ve value doReturn(-25L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession2 = createTestSession(2); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); } @@ -331,7 +338,7 @@ public void stopCollectingGauges_invalidGaugeCollectionFrequency_appInForeground doReturn(0L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession1 = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); assertThat(fakeScheduledExecutorService.isEmpty()).isTrue(); // PASS 2: Test with -ve value @@ -339,17 +346,18 @@ public void stopCollectingGauges_invalidGaugeCollectionFrequency_appInForeground doReturn(-25L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession2 = createTestSession(2); - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); assertThat(fakeScheduledExecutorService.isEmpty()).isTrue(); } @Test + @Ignore // b/394127311 public void startCollectingGauges_validGaugeCollectionFrequency_appInForeground() { doReturn(25L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); doReturn(15L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS)) @@ -357,9 +365,10 @@ public void startCollectingGauges_validGaugeCollectionFrequency_appInForeground( } @Test + @Ignore // b/394127311 public void testStartCollectingGaugesStartsAJobToConsumeTheGeneratedMetrics() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS)) @@ -391,10 +400,11 @@ public void testStartCollectingGaugesStartsAJobToConsumeTheGeneratedMetrics() { } @Test + @Ignore // b/394127311 public void testStopCollectingGaugesStopsCollectingAllGaugeMetrics() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); verify(fakeCpuGaugeCollector) .startCollecting(eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_BG_MS), ArgumentMatchers.any()); @@ -405,9 +415,10 @@ public void testStopCollectingGaugesStopsCollectingAllGaugeMetrics() { } @Test + @Ignore // b/394127311 public void testStopCollectingGaugesCreatesOneLastJobToConsumeAnyPendingMetrics() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); assertThat(fakeScheduledExecutorService.isEmpty()).isFalse(); testGaugeManager.stopCollectingGauges(); @@ -433,10 +444,11 @@ public void testStopCollectingGaugesCreatesOneLastJobToConsumeAnyPendingMetrics( } @Test + @Ignore // b/394127311 public void testGaugeManagerClearsTheQueueEachRun() { PerfSession fakeSession = createTestSession(1); - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); fakeCpuGaugeCollector.cpuMetricReadings.add(createFakeCpuMetricReading(200, 100)); fakeCpuGaugeCollector.cpuMetricReadings.add(createFakeCpuMetricReading(300, 400)); @@ -465,11 +477,12 @@ public void testGaugeManagerClearsTheQueueEachRun() { } @Test + @Ignore // b/394127311 public void testStartingGaugeManagerWithNewSessionIdButSameAppState() { PerfSession fakeSession1 = createTestSession(1); // Start collecting Gauges. - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); CpuMetricReading fakeCpuMetricReading1 = createFakeCpuMetricReading(200, 100); fakeCpuGaugeCollector.cpuMetricReadings.add(fakeCpuMetricReading1); AndroidMemoryReading fakeMemoryMetricReading1 = @@ -494,7 +507,7 @@ public void testStartingGaugeManagerWithNewSessionIdButSameAppState() { PerfSession fakeSession2 = createTestSession(2); // Start collecting gauges for new session, but same app state. - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // The next sweep conducted by GaugeManager still associates metrics to old sessionId and state. fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask(); @@ -523,11 +536,12 @@ public void testStartingGaugeManagerWithNewSessionIdButSameAppState() { } @Test + @Ignore // b/394127311 public void testStartGaugeManagerWithSameSessionIdButDifferentAppState() { PerfSession fakeSession = createTestSession(1); // Start collecting Gauges. - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession); CpuMetricReading fakeCpuMetricReading1 = createFakeCpuMetricReading(200, 100); fakeCpuGaugeCollector.cpuMetricReadings.add(fakeCpuMetricReading1); AndroidMemoryReading fakeMemoryMetricReading1 = @@ -550,7 +564,7 @@ public void testStartGaugeManagerWithSameSessionIdButDifferentAppState() { fakeMemoryGaugeCollector.memoryMetricReadings.add(fakeMemoryMetricReading2); // Start collecting gauges for same session, but new app state - testGaugeManager.startCollectingGauges(fakeSession, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession); // The next sweep conducted by GaugeManager still associates metrics to old sessionId and state. fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask(); @@ -579,11 +593,12 @@ public void testStartGaugeManagerWithSameSessionIdButDifferentAppState() { } @Test + @Ignore // b/394127311 public void testStartGaugeManagerWithNewSessionIdAndNewAppState() { PerfSession fakeSession1 = createTestSession(1); // Start collecting Gauges. - testGaugeManager.startCollectingGauges(fakeSession1, ApplicationProcessState.BACKGROUND); + testGaugeManager.startCollectingGauges(fakeSession1); CpuMetricReading fakeCpuMetricReading1 = createFakeCpuMetricReading(200, 100); fakeCpuGaugeCollector.cpuMetricReadings.add(fakeCpuMetricReading1); AndroidMemoryReading fakeMemoryMetricReading1 = @@ -608,7 +623,7 @@ public void testStartGaugeManagerWithNewSessionIdAndNewAppState() { PerfSession fakeSession2 = createTestSession(2); // Start collecting gauges for new session and new app state - testGaugeManager.startCollectingGauges(fakeSession2, ApplicationProcessState.FOREGROUND); + testGaugeManager.startCollectingGauges(fakeSession2); // The next sweep conducted by GaugeManager still associates metrics to old sessionId and state. fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask(); From 1b108a266f6894b584c6d7bcca7ad3ed6bc20945 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Mon, 5 May 2025 17:07:41 -0400 Subject: [PATCH 02/17] Only set application process state in updateAppState handler --- .../com/google/firebase/perf/session/gauges/GaugeManager.java | 1 - 1 file changed, 1 deletion(-) 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 8a769902ec9..2c2ef06f2a9 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 @@ -207,7 +207,6 @@ public void stopCollectingGauges() { TimeUnit.MILLISECONDS); this.session = null; - this.applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; } /** From 4767a34348f036bb100bf8fb30c0d3f5e6334251 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 13:57:26 -0400 Subject: [PATCH 03/17] Implement an atomic counter to log gauges --- .../session/gauges/CpuGaugeCollector.java | 3 +- .../perf/session/gauges/GaugeCounter.kt | 26 ++++++++++++ .../perf/session/gauges/GaugeManager.java | 41 +++++++++++-------- .../session/gauges/MemoryGaugeCollector.java | 1 + 4 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index ceb636d56b3..4fda7856b8d 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -76,7 +76,7 @@ public class CpuGaugeCollector { private final String procFileName; private final long clockTicksPerSecond; - @Nullable private ScheduledFuture cpuMetricCollectorJob = null; + @Nullable private ScheduledFuture cpuMetricCollectorJob = null; private long cpuMetricCollectionRateMs = UNSET_CPU_METRIC_COLLECTION_RATE; // TODO(b/258263016): Migrate to go/firebase-android-executors @@ -166,6 +166,7 @@ private synchronized void scheduleCpuMetricCollectionWithRate( CpuMetricReading currCpuReading = syncCollectCpuMetric(referenceTime); if (currCpuReading != null) { cpuMetricReadings.add(currCpuReading); + GaugeCounter.Companion.getInstance().incrementCounter(); } }, /* initialDelay */ 0, 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 new file mode 100644 index 00000000000..dceebeddac2 --- /dev/null +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt @@ -0,0 +1,26 @@ +package com.google.firebase.perf.session.gauges + +import java.util.concurrent.atomic.AtomicInteger + + +class GaugeCounter private constructor() { + private val counter = AtomicInteger(0) + private val gaugeManager: GaugeManager = GaugeManager.getInstance() + + fun incrementCounter() { + val metricsCount = counter.incrementAndGet() + + if (metricsCount >= MAX_METRIC_COUNT && gaugeManager.logGaugeMetrics()) { + counter.set(0) + } + } + + fun resetCounter() { + counter.set(0) + } + + companion object { + val instance: GaugeCounter by lazy(LazyThreadSafetyMode.SYNCHRONIZED) { GaugeCounter() } + const val MAX_METRIC_COUNT = 25 + } +} 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 2c2ef06f2a9..f0b93dd4b70 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 @@ -59,7 +59,6 @@ public class GaugeManager extends AppStateUpdateHandler { private final TransportManager transportManager; @Nullable private GaugeMetadataManager gaugeMetadataManager; - @Nullable private ScheduledFuture gaugeManagerDataCollectionJob = null; @Nullable private PerfSession session = null; private ApplicationProcessState applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; @@ -183,30 +182,36 @@ public void stopCollectingGauges() { return; } - // This is needed, otherwise the Runnable might use a stale value. - final String sessionIdForScheduledTask = session.sessionId(); - final ApplicationProcessState applicationProcessStateForScheduledTask = applicationProcessState; - cpuGaugeCollector.get().stopCollecting(); memoryGaugeCollector.get().stopCollecting(); - if (gaugeManagerDataCollectionJob != null) { - gaugeManagerDataCollectionJob.cancel(false); + logGaugeMetrics(); + GaugeCounter.Companion.getInstance().resetCounter(); + this.session = null; + } + + protected boolean logGaugeMetrics() { + if (session == null) { + logger.warn("Attempted to log Gauge Metrics when session was null."); + return false; } - // Flush any data that was collected for this session one last time. + logExistingGaugeMetrics(session.sessionId(), applicationProcessState); + return true; + } + + private void logExistingGaugeMetrics(String sessionId, ApplicationProcessState applicationProcessState) { + // Flush any data that was collected and attach it to the given session and app state. @SuppressWarnings("FutureReturnValueIgnored") ScheduledFuture unusedFuture = - gaugeManagerExecutor - .get() - .schedule( - () -> { - syncFlush(sessionIdForScheduledTask, applicationProcessStateForScheduledTask); - }, - TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, - TimeUnit.MILLISECONDS); - - this.session = null; + gaugeManagerExecutor + .get() + .schedule( + () -> { + syncFlush(sessionId, applicationProcessState); + }, + TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, + TimeUnit.MILLISECONDS); } /** diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index a7b4b40002a..febf6c4ade1 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -129,6 +129,7 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( AndroidMemoryReading memoryReading = syncCollectMemoryMetric(referenceTime); if (memoryReading != null) { memoryMetricReadings.add(memoryReading); + GaugeCounter.Companion.getInstance().incrementCounter(); } }, /* initialDelay */ 0, From 35f775398c3d4caa769ffde18957bb01b321e656 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 16:05:44 -0400 Subject: [PATCH 04/17] Remove the identical methods in PerfSession --- .../metrics/NetworkRequestMetricBuilder.java | 8 +++---- .../google/firebase/perf/metrics/Trace.java | 4 ++-- .../firebase/perf/session/PerfSession.java | 8 +------ .../firebase/perf/session/SessionManager.java | 21 ++++++++----------- .../perf/session/gauges/GaugeManager.java | 5 ++--- .../perf/session/PerfSessionTest.java | 6 +++--- .../perf/session/SessionManagerTest.java | 2 +- 7 files changed, 22 insertions(+), 32 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilder.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilder.java index 1e04744d1b2..6afbb2a4de9 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilder.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilder.java @@ -224,7 +224,7 @@ public NetworkRequestMetricBuilder setCustomAttributes(Map attri * point depending upon the current {@link PerfSession} verbosity. * * @see GaugeManager#collectGaugeMetricOnce(Timer) - * @see PerfSession#isGaugeAndEventCollectionEnabled() + * @see PerfSession#isVerbose() */ public NetworkRequestMetricBuilder setRequestStartTimeMicros(long time) { SessionManager sessionManager = SessionManager.getInstance(); @@ -234,7 +234,7 @@ public NetworkRequestMetricBuilder setRequestStartTimeMicros(long time) { builder.setClientStartTimeUs(time); updateSession(perfSession); - if (perfSession.isGaugeAndEventCollectionEnabled()) { + if (perfSession.isVerbose()) { gaugeManager.collectGaugeMetricOnce(perfSession.getTimer()); } @@ -265,12 +265,12 @@ public long getTimeToResponseInitiatedMicros() { * point depending upon the current {@link PerfSession} Verbosity. * * @see GaugeManager#collectGaugeMetricOnce(Timer) - * @see PerfSession#isGaugeAndEventCollectionEnabled() + * @see PerfSession#isVerbose() */ public NetworkRequestMetricBuilder setTimeToResponseCompletedMicros(long time) { builder.setTimeToResponseCompletedUs(time); - if (SessionManager.getInstance().perfSession().isGaugeAndEventCollectionEnabled()) { + if (SessionManager.getInstance().perfSession().isVerbose()) { gaugeManager.collectGaugeMetricOnce(SessionManager.getInstance().perfSession().getTimer()); } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/Trace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/Trace.java index 91e5f44b4a0..6e9cc6fa47a 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/Trace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/Trace.java @@ -233,7 +233,7 @@ public void start() { updateSession(perfSession); - if (perfSession.isGaugeAndEventCollectionEnabled()) { + if (perfSession.isVerbose()) { gaugeManager.collectGaugeMetricOnce(perfSession.getTimer()); } } @@ -259,7 +259,7 @@ public void stop() { if (!name.isEmpty()) { transportManager.log(new TraceMetricBuilder(this).build(), getAppState()); - if (SessionManager.getInstance().perfSession().isGaugeAndEventCollectionEnabled()) { + if (SessionManager.getInstance().perfSession().isVerbose()) { gaugeManager.collectGaugeMetricOnce( SessionManager.getInstance().perfSession().getTimer()); } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java index a89c8987896..6b2c8433af7 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java @@ -76,16 +76,10 @@ public Timer getTimer() { * Enables/Disables the gauge and event collection for the system. */ public void setGaugeAndEventCollectionEnabled(boolean enabled) { + // TODO(b/394127311): Explore deleting this method. isGaugeAndEventCollectionEnabled = enabled; } - /* - * Returns if gauge and event collection is enabled for the system. - */ - public boolean isGaugeAndEventCollectionEnabled() { - return isGaugeAndEventCollectionEnabled; - } - /** Returns if the current session is verbose or not. */ public boolean isVerbose() { return isGaugeAndEventCollectionEnabled; 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 85da11bc6ac..277e3a730f1 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 @@ -21,7 +21,6 @@ import com.google.firebase.perf.application.AppStateMonitor; 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.perf.v1.GaugeMetadata; import com.google.firebase.perf.v1.GaugeMetric; import java.lang.ref.WeakReference; @@ -37,7 +36,6 @@ public class SessionManager { private static final SessionManager instance = new SessionManager(); private final GaugeManager gaugeManager; - private final AppStateMonitor appStateMonitor; private final Set> clients = new HashSet<>(); private PerfSession perfSession; @@ -50,22 +48,21 @@ public static SessionManager getInstance() { /** Returns the currently active PerfSession. */ public final PerfSession perfSession() { FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "Access perf session from manger without aqs ready"); + perfSession, "PerfSession.perfSession()"); return perfSession; } private SessionManager() { // session should quickly updated by session subscriber. - this(GaugeManager.getInstance(), PerfSession.createWithId(null), AppStateMonitor.getInstance()); + this(GaugeManager.getInstance(), PerfSession.createWithId(null)); } @VisibleForTesting public SessionManager( - GaugeManager gaugeManager, PerfSession perfSession, AppStateMonitor appStateMonitor) { + GaugeManager gaugeManager, PerfSession perfSession) { this.gaugeManager = gaugeManager; this.perfSession = perfSession; - this.appStateMonitor = appStateMonitor; } /** @@ -84,7 +81,7 @@ public void setApplicationContext(final Context appContext) { public void stopGaugeCollectionIfSessionRunningTooLong() { FirebaseSessionsEnforcementCheck.checkSession( perfSession, - "Session is not ready while trying to stopGaugeCollectionIfSessionRunningTooLong"); + "SessionManager.stopGaugeCollectionIfSessionRunningTooLong"); if (perfSession.isSessionRunningTooLong()) { gaugeManager.stopCollectingGauges(); @@ -123,7 +120,7 @@ public void updatePerfSession(PerfSession perfSession) { } // Start of stop the gauge data collection. - startOrStopCollectingGauges(appStateMonitor.getAppState()); + startOrStopCollectingGauges(); } /** @@ -133,7 +130,7 @@ public void updatePerfSession(PerfSession perfSession) { * this does not reset the perfSession. */ public void initializeGaugeCollection() { - startOrStopCollectingGauges(ApplicationProcessState.FOREGROUND); + startOrStopCollectingGauges(); } /** @@ -160,11 +157,11 @@ public void unregisterForSessionUpdates(WeakReference client } } - private void startOrStopCollectingGauges(ApplicationProcessState appState) { + private void startOrStopCollectingGauges() { FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "Session is not ready while trying to startOrStopCollectingGauges"); + perfSession, "startOrStopCollectingGauges"); - if (perfSession.isGaugeAndEventCollectionEnabled()) { + if (perfSession.isVerbose()) { gaugeManager.startCollectingGauges(perfSession); } else { gaugeManager.stopCollectingGauges(); 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 f0b93dd4b70..84b1bf9c2b4 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 @@ -102,12 +102,11 @@ public void initializeGaugeMetadataManager(Context appContext) { public void onUpdateAppState(ApplicationProcessState applicationProcessState) { this.applicationProcessState = applicationProcessState; - if (session == null) { - // If the session is null, it means there's no gauges being collected. + if (session == null || !session.isVerbose()) { return; } - stopCollectingGauges(); + // If it's a verbose session, start collecting gauges for the new app state. startCollectingGauges(this.applicationProcessState, session.getTimer()); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java index 6fe8e1a959c..f7c8d400483 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/session/PerfSessionTest.java @@ -178,21 +178,21 @@ public void testPerfSessionConversionWithoutVerbosity() { public void testPerfSessionsCreateDisabledGaugeCollectionWhenVerboseSessionForceDisabled() { forceNonVerboseSession(); PerfSession testPerfSession = createTestSession(1); - assertThat(testPerfSession.isGaugeAndEventCollectionEnabled()).isFalse(); + assertThat(testPerfSession.isVerbose()).isFalse(); } @Test public void testPerfSessionsCreateDisabledGaugeCollectionWhenSessionsFeatureDisabled() { forceSessionsFeatureDisabled(); PerfSession testPerfSession = createTestSession(1); - assertThat(testPerfSession.isGaugeAndEventCollectionEnabled()).isFalse(); + assertThat(testPerfSession.isVerbose()).isFalse(); } @Test public void testPerfSessionsCreateEnablesGaugeCollectionWhenVerboseSessionForceEnabled() { forceVerboseSession(); PerfSession testPerfSession = PerfSession.createWithId(testSessionId(1)); - assertThat(testPerfSession.isGaugeAndEventCollectionEnabled()).isTrue(); + assertThat(testPerfSession.isVerbose()).isTrue(); } @Test 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 81c6d0c8a16..95bb13e7c4d 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 @@ -73,7 +73,7 @@ public void testInstanceCreation() { @Test public void setApplicationContext_initializeGaugeMetadataManager() throws ExecutionException, InterruptedException { - when(mockPerfSession.isGaugeAndEventCollectionEnabled()).thenReturn(true); + when(mockPerfSession.isVerbose()).thenReturn(true); InOrder inOrder = Mockito.inOrder(mockGaugeManager); SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); From 1f290441e21662fae3c0f1a6040437fecca71944 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 16:10:23 -0400 Subject: [PATCH 05/17] Update PerfSession --- .../firebase/perf/session/PerfSession.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java index 6b2c8433af7..94c2ad74a0d 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/PerfSession.java @@ -40,9 +40,7 @@ public static PerfSession createWithId(@Nullable String aqsSessionId) { if (sessionId == null) { sessionId = FirebaseSessionsHelperKt.createLegacySessionId(); } - PerfSession session = new PerfSession(sessionId, new Clock()); - session.setGaugeAndEventCollectionEnabled(session.shouldCollectGaugesAndEvents()); - return session; + return new PerfSession(sessionId, new Clock()); } /** Creates a PerfSession with the provided {@code sessionId} and {@code clock}. */ @@ -50,6 +48,7 @@ public static PerfSession createWithId(@Nullable String aqsSessionId) { public PerfSession(String sessionId, Clock clock) { this.sessionId = sessionId; creationTime = clock.getTime(); + isGaugeAndEventCollectionEnabled = shouldCollectGaugesAndEvents(); } private PerfSession(@NonNull Parcel in) { @@ -72,14 +71,6 @@ public Timer getTimer() { return creationTime; } - /* - * Enables/Disables the gauge and event collection for the system. - */ - public void setGaugeAndEventCollectionEnabled(boolean enabled) { - // TODO(b/394127311): Explore deleting this method. - isGaugeAndEventCollectionEnabled = enabled; - } - /** Returns if the current session is verbose or not. */ public boolean isVerbose() { return isGaugeAndEventCollectionEnabled; @@ -146,6 +137,7 @@ public static com.google.firebase.perf.v1.PerfSession[] buildAndSort( } /** If true, Session Gauge collection is enabled. */ + @VisibleForTesting public boolean shouldCollectGaugesAndEvents() { ConfigResolver configResolver = ConfigResolver.getInstance(); return configResolver.isPerformanceMonitoringEnabled() @@ -153,6 +145,14 @@ public boolean shouldCollectGaugesAndEvents() { < configResolver.getSessionsSamplingRate() * 100); } + /* + * Enables/Disables whether the session is verbose or not. + */ + @VisibleForTesting + public void setGaugeAndEventCollectionEnabled(boolean enabled) { + isGaugeAndEventCollectionEnabled = enabled; + } + /** * Describes the kinds of special objects contained in this Parcelable's marshalled * representation. Please refer to From 8900e3fc89ba22a2f1a51383b6915cf711b719d5 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 16:34:38 -0400 Subject: [PATCH 06/17] Update comments --- .../perf/session/gauges/GaugeManager.java | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) 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 84b1bf9c2b4..22aa5deb556 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 @@ -116,12 +116,12 @@ public static synchronized GaugeManager getInstance() { } /** - * Starts the collection of available gauges for the given {@code sessionId} and {@code - * applicationProcessState}. The collected Gauge Metrics will be flushed at regular intervals. + * Starts the collection of available gauges for the given {@link PerfSession}. + * The collected Gauge Metrics will be flushed by {@link GaugeCounter} * *

GaugeManager can only collect gauges for one session at a time, and if this method is called * again with the same or new sessionId while it's already collecting gauges, all future gauges - * will then be associated with the same or new sessionId and applicationProcessState. + * will then be associated with the same or new sessionId. * * @param session The {@link PerfSession} to which the collected gauges will be associated with. * @note: This method is NOT thread safe - {@link this.startCollectingGauges()} and {@link @@ -132,8 +132,15 @@ public void startCollectingGauges(PerfSession session) { stopCollectingGauges(); } + ApplicationProcessState gaugeCollectionApplicationProcessState = applicationProcessState; + if (gaugeCollectionApplicationProcessState == ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN) { + logger.warn("Start collecting gauges with APPLICATION_PROCESS_STATE_UNKNOWN"); + // Since the application process state is unknown, collect gauges at the foreground frequency. + gaugeCollectionApplicationProcessState = ApplicationProcessState.FOREGROUND; + } + long collectionFrequency = - startCollectingGauges(this.applicationProcessState, session.getTimer()); + startCollectingGauges(gaugeCollectionApplicationProcessState, session.getTimer()); if (collectionFrequency == INVALID_GAUGE_COLLECTION_FREQUENCY) { logger.warn("Invalid gauge collection frequency. Unable to start collecting Gauges."); return; @@ -185,10 +192,17 @@ public void stopCollectingGauges() { memoryGaugeCollector.get().stopCollecting(); logGaugeMetrics(); + + // TODO(b/394127311): There might be a race condition where a final metric is collected, but + // isn't uploaded. GaugeCounter.Companion.getInstance().resetCounter(); this.session = null; } + /** + * Logs the existing GaugeMetrics to Firelog, associates it with the current {@link PerfSession} + * and {@link ApplicationProcessState}. + */ protected boolean logGaugeMetrics() { if (session == null) { logger.warn("Attempted to log Gauge Metrics when session was null."); From fbd31de850c9518e405a8f95836f98935e393293 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 17:31:10 -0400 Subject: [PATCH 07/17] Changes in GaugeCounter --- .../firebase/perf/session/SessionManager.java | 13 ++---- .../perf/session/gauges/GaugeCounter.kt | 33 +++++++------- .../perf/session/gauges/GaugeManager.java | 44 +++++++++++-------- 3 files changed, 48 insertions(+), 42 deletions(-) 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 277e3a730f1..5f01a50a032 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 @@ -18,7 +18,6 @@ import android.content.Context; import androidx.annotation.Keep; import androidx.annotation.VisibleForTesting; -import com.google.firebase.perf.application.AppStateMonitor; import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck; import com.google.firebase.perf.session.gauges.GaugeManager; import com.google.firebase.perf.v1.GaugeMetadata; @@ -47,8 +46,7 @@ public static SessionManager getInstance() { /** Returns the currently active PerfSession. */ public final PerfSession perfSession() { - FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "PerfSession.perfSession()"); + FirebaseSessionsEnforcementCheck.checkSession(perfSession, "PerfSession.perfSession()"); return perfSession; } @@ -59,8 +57,7 @@ private SessionManager() { } @VisibleForTesting - public SessionManager( - GaugeManager gaugeManager, PerfSession perfSession) { + public SessionManager(GaugeManager gaugeManager, PerfSession perfSession) { this.gaugeManager = gaugeManager; this.perfSession = perfSession; } @@ -80,8 +77,7 @@ public void setApplicationContext(final Context appContext) { */ public void stopGaugeCollectionIfSessionRunningTooLong() { FirebaseSessionsEnforcementCheck.checkSession( - perfSession, - "SessionManager.stopGaugeCollectionIfSessionRunningTooLong"); + perfSession, "SessionManager.stopGaugeCollectionIfSessionRunningTooLong"); if (perfSession.isSessionRunningTooLong()) { gaugeManager.stopCollectingGauges(); @@ -158,8 +154,7 @@ public void unregisterForSessionUpdates(WeakReference client } private void startOrStopCollectingGauges() { - FirebaseSessionsEnforcementCheck.checkSession( - perfSession, "startOrStopCollectingGauges"); + FirebaseSessionsEnforcementCheck.checkSession(perfSession, "startOrStopCollectingGauges"); if (perfSession.isVerbose()) { gaugeManager.startCollectingGauges(perfSession); 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 dceebeddac2..1fe57071d81 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 @@ -2,25 +2,28 @@ package com.google.firebase.perf.session.gauges import java.util.concurrent.atomic.AtomicInteger - +/** + * [GaugeCounter] is a threadsafe counter for gauge metrics. If the metrics count exceeds + * [MAX_METRIC_COUNT], it attempts to log the metrics to Firelog. + */ class GaugeCounter private constructor() { - private val counter = AtomicInteger(0) - private val gaugeManager: GaugeManager = GaugeManager.getInstance() + private val counter = AtomicInteger(0) + private val gaugeManager: GaugeManager = GaugeManager.getInstance() - fun incrementCounter() { - val metricsCount = counter.incrementAndGet() + fun incrementCounter() { + val metricsCount = counter.incrementAndGet() - if (metricsCount >= MAX_METRIC_COUNT && gaugeManager.logGaugeMetrics()) { - counter.set(0) - } + if (metricsCount >= MAX_METRIC_COUNT) { + gaugeManager.logGaugeMetrics() } + } - fun resetCounter() { - counter.set(0) - } + fun decrementCounter() { + counter.decrementAndGet() + } - companion object { - val instance: GaugeCounter by lazy(LazyThreadSafetyMode.SYNCHRONIZED) { GaugeCounter() } - const val MAX_METRIC_COUNT = 25 - } + companion object { + val instance: GaugeCounter by lazy(LazyThreadSafetyMode.SYNCHRONIZED) { GaugeCounter() } + const val MAX_METRIC_COUNT = 25 + } } 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 22aa5deb556..fa21b72e1b7 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 @@ -49,7 +49,6 @@ public class GaugeManager extends AppStateUpdateHandler { // 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 INVALID_GAUGE_COLLECTION_FREQUENCY = -1; private final Lazy gaugeManagerExecutor; @@ -59,6 +58,7 @@ public class GaugeManager extends AppStateUpdateHandler { private final TransportManager transportManager; @Nullable private GaugeMetadataManager gaugeMetadataManager; + @Nullable private ScheduledFuture gaugeManagerDataCollectionJob = null; @Nullable private PerfSession session = null; private ApplicationProcessState applicationProcessState = ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN; @@ -133,7 +133,8 @@ public void startCollectingGauges(PerfSession session) { } ApplicationProcessState gaugeCollectionApplicationProcessState = applicationProcessState; - if (gaugeCollectionApplicationProcessState == ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN) { + if (gaugeCollectionApplicationProcessState + == ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN) { logger.warn("Start collecting gauges with APPLICATION_PROCESS_STATE_UNKNOWN"); // Since the application process state is unknown, collect gauges at the foreground frequency. gaugeCollectionApplicationProcessState = ApplicationProcessState.FOREGROUND; @@ -192,20 +193,24 @@ public void stopCollectingGauges() { memoryGaugeCollector.get().stopCollecting(); logGaugeMetrics(); - - // TODO(b/394127311): There might be a race condition where a final metric is collected, but - // isn't uploaded. - GaugeCounter.Companion.getInstance().resetCounter(); this.session = null; } /** * Logs the existing GaugeMetrics to Firelog, associates it with the current {@link PerfSession} * and {@link ApplicationProcessState}. + * + * @return true if a new data collection job is started, false otherwise. */ protected boolean logGaugeMetrics() { if (session == null) { - logger.warn("Attempted to log Gauge Metrics when session was null."); + logger.debug("Attempted to log Gauge Metrics when session was null."); + return false; + } + + // If there's an existing gaugeManagerDataCollectionJob, this is a no-op. + if (gaugeManagerDataCollectionJob != null && !gaugeManagerDataCollectionJob.isDone()) { + logger.debug("Attempted to start an additional gauge logging operation."); return false; } @@ -213,18 +218,18 @@ protected boolean logGaugeMetrics() { return true; } - private void logExistingGaugeMetrics(String sessionId, ApplicationProcessState applicationProcessState) { + private void logExistingGaugeMetrics( + String sessionId, ApplicationProcessState applicationProcessState) { // Flush any data that was collected and attach it to the given session and app state. - @SuppressWarnings("FutureReturnValueIgnored") - ScheduledFuture unusedFuture = - gaugeManagerExecutor - .get() - .schedule( - () -> { - syncFlush(sessionId, applicationProcessState); - }, - TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, - TimeUnit.MILLISECONDS); + gaugeManagerDataCollectionJob = + gaugeManagerExecutor + .get() + .schedule( + () -> { + syncFlush(sessionId, applicationProcessState); + }, + TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS, + TimeUnit.MILLISECONDS); } /** @@ -236,16 +241,19 @@ private void logExistingGaugeMetrics(String sessionId, ApplicationProcessState a */ private void syncFlush(String sessionId, ApplicationProcessState appState) { GaugeMetric.Builder gaugeMetricBuilder = GaugeMetric.newBuilder(); + GaugeCounter gaugeCounter = GaugeCounter.Companion.getInstance(); // Adding CPU metric readings. while (!cpuGaugeCollector.get().cpuMetricReadings.isEmpty()) { gaugeMetricBuilder.addCpuMetricReadings(cpuGaugeCollector.get().cpuMetricReadings.poll()); + gaugeCounter.decrementCounter(); } // Adding Memory metric readings. while (!memoryGaugeCollector.get().memoryMetricReadings.isEmpty()) { gaugeMetricBuilder.addAndroidMemoryReadings( memoryGaugeCollector.get().memoryMetricReadings.poll()); + gaugeCounter.decrementCounter(); } // Adding Session ID info. From b7ea8d661920e4c58136c5cb54cb7efdfbfb5de4 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 17:37:04 -0400 Subject: [PATCH 08/17] Fix onUpdateAppState --- .../google/firebase/perf/session/gauges/GaugeManager.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 fa21b72e1b7..065e0a731d7 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 @@ -102,12 +102,14 @@ public void initializeGaugeMetadataManager(Context appContext) { public void onUpdateAppState(ApplicationProcessState applicationProcessState) { this.applicationProcessState = applicationProcessState; - if (session == null || !session.isVerbose()) { + if (session == null) { return; } - // If it's a verbose session, start collecting gauges for the new app state. - startCollectingGauges(this.applicationProcessState, session.getTimer()); + if (session.isVerbose()) { + // If it's a verbose session, start collecting gauges for the new app state. + startCollectingGauges(this.applicationProcessState, session.getTimer()); + } } /** Returns the singleton instance of this class. */ From 34a3d31f90ccb8d910c3b3bdf95a7e3eb0f746a8 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 17:45:39 -0400 Subject: [PATCH 09/17] Comments --- .../firebase/perf/session/gauges/GaugeManager.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 065e0a731d7..0ed73678688 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 @@ -102,14 +102,13 @@ public void initializeGaugeMetadataManager(Context appContext) { public void onUpdateAppState(ApplicationProcessState applicationProcessState) { this.applicationProcessState = applicationProcessState; - if (session == null) { + if (session == null || !session.isVerbose()) { return; } - if (session.isVerbose()) { - // If it's a verbose session, start collecting gauges for the new app state. - startCollectingGauges(this.applicationProcessState, session.getTimer()); - } + // If it's a verbose session, start collecting gauges for the new app state. + // This + startCollectingGauges(this.applicationProcessState, session.getTimer()); } /** Returns the singleton instance of this class. */ @@ -153,7 +152,9 @@ public void startCollectingGauges(PerfSession session) { } /** - * Starts the collection of available Gauges for the given {@code appState}. + * Starts the collection of available Gauges for the given {@code appState}. If it's being + * collected for a different app state, it stops that prior to starting it for the given + * {@code appState}. * * @param appState The app state to which the collected gauges are associated. * @param referenceTime The time off which the system time is calculated when collecting gauges. From f6e68031b46d17ef2b570c8d79486aef2e8911ef Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Tue, 6 May 2025 17:52:34 -0400 Subject: [PATCH 10/17] Comment --- .../com/google/firebase/perf/session/gauges/GaugeManager.java | 1 - 1 file changed, 1 deletion(-) 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 0ed73678688..a3d9351b626 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 @@ -107,7 +107,6 @@ public void onUpdateAppState(ApplicationProcessState applicationProcessState) { } // If it's a verbose session, start collecting gauges for the new app state. - // This startCollectingGauges(this.applicationProcessState, session.getTimer()); } From e925a538bd515f47456839ed60c0b6501065430a Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 09:43:19 -0400 Subject: [PATCH 11/17] Unit test build failures --- .../NetworkRequestMetricBuilderTest.java | 2 +- .../firebase/perf/metrics/TraceTest.java | 2 +- .../perf/session/SessionManagerTest.java | 24 +++++++------------ 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilderTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilderTest.java index 61b3823741d..4a90184936c 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilderTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilderTest.java @@ -242,7 +242,7 @@ public void testSessionIdNotAddedIfPerfSessionIsNull() { int numberOfSessionIds = metricBuilder.getSessions().size(); - new SessionManager(mock(GaugeManager.class), null, mock(AppStateMonitor.class)); + new SessionManager(mock(GaugeManager.class), null); assertThat(metricBuilder.getSessions()).hasSize(numberOfSessionIds); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/TraceTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/TraceTest.java index 0be443031f2..ca86a1a86b6 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/TraceTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/TraceTest.java @@ -1032,7 +1032,7 @@ public void testSessionIdNotAddedIfPerfSessionIsNull() { int numberOfSessionIds = trace.getSessions().size(); - new SessionManager(mock(GaugeManager.class), null, mock(AppStateMonitor.class)); + new SessionManager(mock(GaugeManager.class), null); assertThat(trace.getSessions()).hasSize(numberOfSessionIds); 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 95bb13e7c4d..ab8cce7aab5 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 @@ -75,8 +75,7 @@ public void setApplicationContext_initializeGaugeMetadataManager() throws ExecutionException, InterruptedException { when(mockPerfSession.isVerbose()).thenReturn(true); InOrder inOrder = Mockito.inOrder(mockGaugeManager); - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession); testSessionManager.setApplicationContext(mockApplicationContext); inOrder.verify(mockGaugeManager).initializeGaugeMetadataManager(any()); @@ -90,8 +89,7 @@ public void setApplicationContext_initializeGaugeMetadataManager() public void testUpdatePerfSessionMakesGaugeManagerStopCollectingGaugesIfSessionIsNonVerbose() { forceNonVerboseSession(); - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession); testSessionManager.updatePerfSession(createTestSession(1)); verify(mockGaugeManager).stopCollectingGauges(); @@ -101,8 +99,7 @@ public void testUpdatePerfSessionMakesGaugeManagerStopCollectingGaugesIfSessionI public void testUpdatePerfSessionMakesGaugeManagerStopCollectingGaugesWhenSessionsDisabled() { forceSessionsFeatureDisabled(); - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, createTestSession(1), mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, createTestSession(1)); testSessionManager.updatePerfSession(createTestSession(2)); verify(mockGaugeManager).stopCollectingGauges(); @@ -114,8 +111,7 @@ public void testSessionIdDoesNotUpdateIfPerfSessionRunsTooLong() { when(mockClock.getTime()).thenReturn(mockTimer); PerfSession session = new PerfSession(testSessionId(1), mockClock); - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, session, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, session); assertThat(session.isSessionRunningTooLong()).isFalse(); @@ -138,8 +134,7 @@ public void testUpdatePerfSessionStartsCollectingGaugesIfSessionIsVerbose() { PerfSession newSession = createTestSession(2); newSession.setGaugeAndEventCollectionEnabled(true); - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, previousSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, previousSession); testSessionManager.updatePerfSession(newSession); testSessionManager.setApplicationContext(mockApplicationContext); @@ -149,8 +144,7 @@ public void testUpdatePerfSessionStartsCollectingGaugesIfSessionIsVerbose() { @Test public void testPerfSession_sessionAwareObjects_doesntNotifyIfNotRegistered() { - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession); FakeSessionAwareObject spySessionAwareObjectOne = spy(new FakeSessionAwareObject()); FakeSessionAwareObject spySessionAwareObjectTwo = spy(new FakeSessionAwareObject()); @@ -165,8 +159,7 @@ public void testPerfSession_sessionAwareObjects_doesntNotifyIfNotRegistered() { @Test public void testPerfSession_sessionAwareObjects_NotifiesIfRegistered() { - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession); FakeSessionAwareObject spySessionAwareObjectOne = spy(new FakeSessionAwareObject()); FakeSessionAwareObject spySessionAwareObjectTwo = spy(new FakeSessionAwareObject()); @@ -185,8 +178,7 @@ public void testPerfSession_sessionAwareObjects_NotifiesIfRegistered() { @Test public void testPerfSession_sessionAwareObjects_DoesNotNotifyIfUnregistered() { - SessionManager testSessionManager = - new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor); + SessionManager testSessionManager = new SessionManager(mockGaugeManager, mockPerfSession); FakeSessionAwareObject spySessionAwareObjectOne = spy(new FakeSessionAwareObject()); FakeSessionAwareObject spySessionAwareObjectTwo = spy(new FakeSessionAwareObject()); From b5f6bf44aa8bce7d86990ea15bececf17e5470d3 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 09:56:30 -0400 Subject: [PATCH 12/17] unit test --- .../firebase/perf/session/gauges/GaugeManagerTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 9fe884cf656..77b87f7ecaa 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 @@ -156,13 +156,15 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() @Test public void - testStartCollectingGaugesDoesNotStartCollectingMetricsWithUnknownApplicationProcessState() { + testStartCollectingGaugesStartCollectingMetricsWithUnknownApplicationProcessStateInForegroundState() { PerfSession fakeSession = createTestSession(1); testGaugeManager.startCollectingGauges(fakeSession); + // @see + // com.google.firebase.perf.config.ConfigurationConstants.SessionsCpuCaptureFrequencyForegroundMs verify(fakeCpuGaugeCollector, never()) - .startCollecting(ArgumentMatchers.anyLong(), ArgumentMatchers.nullable(Timer.class)); + .startCollecting(ArgumentMatchers.eq(100L), ArgumentMatchers.nullable(Timer.class)); verify(fakeMemoryGaugeCollector, never()) - .startCollecting(ArgumentMatchers.anyLong(), ArgumentMatchers.nullable(Timer.class)); + .startCollecting(ArgumentMatchers.eq(100L), ArgumentMatchers.nullable(Timer.class)); } @Test From f991f175c603b8b8f03ae5764ed7a4e2340a083e Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 12:05:37 -0400 Subject: [PATCH 13/17] copyright --- .../firebase/perf/session/gauges/GaugeCounter.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 1fe57071d81..0bb26d3a6f8 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 @@ -1,3 +1,17 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package com.google.firebase.perf.session.gauges import java.util.concurrent.atomic.AtomicInteger From 2c531e3d6e6ec39e72a6d0718fd5298bd1a7c902 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 15:59:19 -0400 Subject: [PATCH 14/17] Code review --- .../perf/session/gauges/CpuGaugeCollector.java | 2 +- .../firebase/perf/session/gauges/GaugeCounter.kt | 10 +++------- .../firebase/perf/session/gauges/GaugeManager.java | 2 +- .../perf/session/gauges/MemoryGaugeCollector.java | 2 +- .../firebase/perf/session/gauges/GaugeManagerTest.java | 10 ++++++---- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java index 4fda7856b8d..907cf604062 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/CpuGaugeCollector.java @@ -166,7 +166,7 @@ private synchronized void scheduleCpuMetricCollectionWithRate( CpuMetricReading currCpuReading = syncCollectCpuMetric(referenceTime); if (currCpuReading != null) { cpuMetricReadings.add(currCpuReading); - GaugeCounter.Companion.getInstance().incrementCounter(); + GaugeCounter.INSTANCE.incrementCounter(); } }, /* initialDelay */ 0, 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 0bb26d3a6f8..35e299c27f1 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 @@ -17,10 +17,11 @@ package com.google.firebase.perf.session.gauges import java.util.concurrent.atomic.AtomicInteger /** - * [GaugeCounter] is a threadsafe counter for gauge metrics. If the metrics count exceeds + * [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. */ -class GaugeCounter private constructor() { +object GaugeCounter { + private const val MAX_METRIC_COUNT = 25 private val counter = AtomicInteger(0) private val gaugeManager: GaugeManager = GaugeManager.getInstance() @@ -35,9 +36,4 @@ class GaugeCounter private constructor() { fun decrementCounter() { counter.decrementAndGet() } - - companion object { - val instance: GaugeCounter by lazy(LazyThreadSafetyMode.SYNCHRONIZED) { GaugeCounter() } - const val MAX_METRIC_COUNT = 25 - } } 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 a3d9351b626..fe36654e8f7 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 @@ -243,7 +243,7 @@ private void logExistingGaugeMetrics( */ private void syncFlush(String sessionId, ApplicationProcessState appState) { GaugeMetric.Builder gaugeMetricBuilder = GaugeMetric.newBuilder(); - GaugeCounter gaugeCounter = GaugeCounter.Companion.getInstance(); + GaugeCounter gaugeCounter = GaugeCounter.INSTANCE; // Adding CPU metric readings. while (!cpuGaugeCollector.get().cpuMetricReadings.isEmpty()) { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java index febf6c4ade1..99016400c61 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/MemoryGaugeCollector.java @@ -129,7 +129,7 @@ private synchronized void scheduleMemoryMetricCollectionWithRate( AndroidMemoryReading memoryReading = syncCollectMemoryMetric(referenceTime); if (memoryReading != null) { memoryMetricReadings.add(memoryReading); - GaugeCounter.Companion.getInstance().incrementCounter(); + GaugeCounter.INSTANCE.incrementCounter(); } }, /* initialDelay */ 0, 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 77b87f7ecaa..004969ed979 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 @@ -159,12 +159,14 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() testStartCollectingGaugesStartCollectingMetricsWithUnknownApplicationProcessStateInForegroundState() { PerfSession fakeSession = createTestSession(1); testGaugeManager.startCollectingGauges(fakeSession); - // @see - // com.google.firebase.perf.config.ConfigurationConstants.SessionsCpuCaptureFrequencyForegroundMs verify(fakeCpuGaugeCollector, never()) - .startCollecting(ArgumentMatchers.eq(100L), ArgumentMatchers.nullable(Timer.class)); + .startCollecting( + eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), + ArgumentMatchers.nullable(Timer.class)); verify(fakeMemoryGaugeCollector, never()) - .startCollecting(ArgumentMatchers.eq(100L), ArgumentMatchers.nullable(Timer.class)); + .startCollecting( + eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), + ArgumentMatchers.nullable(Timer.class)); } @Test From c167b9074c0b32434780f8750670637a978d2926 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 16:00:21 -0400 Subject: [PATCH 15/17] unit test --- .../google/firebase/perf/session/gauges/GaugeManagerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 004969ed979..7252aeb7121 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 @@ -159,11 +159,11 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() testStartCollectingGaugesStartCollectingMetricsWithUnknownApplicationProcessStateInForegroundState() { PerfSession fakeSession = createTestSession(1); testGaugeManager.startCollectingGauges(fakeSession); - verify(fakeCpuGaugeCollector, never()) + verify(fakeCpuGaugeCollector) .startCollecting( eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), ArgumentMatchers.nullable(Timer.class)); - verify(fakeMemoryGaugeCollector, never()) + verify(fakeMemoryGaugeCollector) .startCollecting( eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), ArgumentMatchers.nullable(Timer.class)); From 75aaba44fe4ebb038ac246de7333ed9e1f01e72a Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 16:00:52 -0400 Subject: [PATCH 16/17] unit test --- .../google/firebase/perf/session/gauges/GaugeManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7252aeb7121..497097a524d 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 @@ -165,7 +165,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() ArgumentMatchers.nullable(Timer.class)); verify(fakeMemoryGaugeCollector) .startCollecting( - eq(DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS), + eq(DEFAULT_MEMORY_GAUGE_COLLECTION_FREQUENCY_FG_MS), ArgumentMatchers.nullable(Timer.class)); } From 6363a808f2c2a39351370c30ef55e84ed7a37cc9 Mon Sep 17 00:00:00 2001 From: Tejas Deshpande Date: Wed, 7 May 2025 16:14:50 -0400 Subject: [PATCH 17/17] Add TODO --- .../perf/session/gauges/GaugeManagerTest.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) 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 497097a524d..9ec998bd65f 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 @@ -170,7 +170,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithValidFrequencyInBackground() { // PASS 1: Test with 0 @@ -201,7 +201,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void startCollectingGaugesOnBackground_invalidMemoryCaptureMs_onlyDisableMemoryCollection() { // PASS 1: Test with 0 @@ -232,7 +232,7 @@ public void testStartCollectingGaugesStartsCollectingMetricsInForegroundState() } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithValidFrequency() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); @@ -262,7 +262,7 @@ public void stopCollectingCPUMetric_invalidCPUCaptureFrequency_OtherMetricsWithV } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void startCollectingGaugesOnForeground_invalidMemoryCaptureMs_onlyDisableMemoryCollection() { // PASS 1: Test with 0 @@ -300,7 +300,7 @@ public void testStartCollectingGaugesDoesNotStartAJobToConsumeMetricsWithUnknown } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void stopCollectingCPUMetrics_invalidCPUCaptureFrequency_appInForegrounf() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); @@ -318,7 +318,7 @@ public void stopCollectingCPUMetrics_invalidCPUCaptureFrequency_appInForegrounf( } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void stopCollectingGauges_invalidMemoryCollectionFrequency_appInForeground() { // PASS 1: Test with 0 doReturn(0L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); @@ -355,7 +355,7 @@ public void stopCollectingGauges_invalidGaugeCollectionFrequency_appInForeground } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void startCollectingGauges_validGaugeCollectionFrequency_appInForeground() { doReturn(25L).when(mockConfigResolver).getSessionsCpuCaptureFrequencyForegroundMs(); doReturn(15L).when(mockConfigResolver).getSessionsMemoryCaptureFrequencyForegroundMs(); @@ -369,7 +369,7 @@ public void startCollectingGauges_validGaugeCollectionFrequency_appInForeground( } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStartCollectingGaugesStartsAJobToConsumeTheGeneratedMetrics() { PerfSession fakeSession = createTestSession(1); testGaugeManager.startCollectingGauges(fakeSession); @@ -404,7 +404,7 @@ public void testStartCollectingGaugesStartsAJobToConsumeTheGeneratedMetrics() { } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStopCollectingGaugesStopsCollectingAllGaugeMetrics() { PerfSession fakeSession = createTestSession(1); @@ -419,7 +419,7 @@ public void testStopCollectingGaugesStopsCollectingAllGaugeMetrics() { } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStopCollectingGaugesCreatesOneLastJobToConsumeAnyPendingMetrics() { PerfSession fakeSession = createTestSession(1); testGaugeManager.startCollectingGauges(fakeSession); @@ -448,7 +448,7 @@ public void testStopCollectingGaugesCreatesOneLastJobToConsumeAnyPendingMetrics( } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testGaugeManagerClearsTheQueueEachRun() { PerfSession fakeSession = createTestSession(1); @@ -481,7 +481,7 @@ public void testGaugeManagerClearsTheQueueEachRun() { } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStartingGaugeManagerWithNewSessionIdButSameAppState() { PerfSession fakeSession1 = createTestSession(1); @@ -540,7 +540,7 @@ public void testStartingGaugeManagerWithNewSessionIdButSameAppState() { } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStartGaugeManagerWithSameSessionIdButDifferentAppState() { PerfSession fakeSession = createTestSession(1); @@ -597,7 +597,7 @@ public void testStartGaugeManagerWithSameSessionIdButDifferentAppState() { } @Test - @Ignore // b/394127311 + @Ignore // TODO(b/394127311): Fix public void testStartGaugeManagerWithNewSessionIdAndNewAppState() { PerfSession fakeSession1 = createTestSession(1);