Skip to content

Commit b14a2f4

Browse files
tejasdtejasd
committed
Remove the logging of GaugeMetadata to allow using AQS (#6678)
Based on the behaviour of AQS w/ Fireperf, an AQS session isn't available when (currently) logging gauge metadata. Changes: - Remove the current logging of gauge metadata - will be re-introduced in a future PR. - Switch Gauge collection from `scheduleAtFixedRate` to `scheduleWithFixedDelay`. As [documented](https://stackoverflow.com/a/78405653), this *should* prevent a potentially large amounts of gauge collection if a process is cached, and then restored during a verbose session - which *should* make it work better w/ AQS. - Remove API restricted behaviour which is no longer relevant.
1 parent 2299b50 commit b14a2f4

File tree

3 files changed

+192
-38
lines changed

3 files changed

+192
-38
lines changed

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

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,23 @@
1919
import androidx.annotation.Keep;
2020
import androidx.annotation.VisibleForTesting;
2121
import com.google.firebase.perf.application.AppStateMonitor;
22-
import com.google.firebase.perf.logging.AndroidLogger;
22+
import com.google.firebase.perf.application.AppStateUpdateHandler;
2323
import com.google.firebase.perf.session.gauges.GaugeManager;
2424
import com.google.firebase.perf.v1.ApplicationProcessState;
2525
import com.google.firebase.perf.v1.GaugeMetadata;
2626
import com.google.firebase.perf.v1.GaugeMetric;
2727
import java.lang.ref.WeakReference;
2828
import java.util.HashSet;
2929
import java.util.Iterator;
30-
import java.util.Objects;
3130
import java.util.Set;
31+
import java.util.UUID;
32+
import java.util.concurrent.ExecutorService;
33+
import java.util.concurrent.Executors;
3234
import java.util.concurrent.Future;
3335

3436
/** Session manager to generate sessionIDs and broadcast to the application. */
3537
@Keep // Needed because of b/117526359.
36-
public class SessionManager {
38+
public class SessionManager extends AppStateUpdateHandler {
3739

3840
@SuppressLint("StaticFieldLeak")
3941
private static final SessionManager instance = new SessionManager();
@@ -57,7 +59,10 @@ public final PerfSession perfSession() {
5759

5860
private SessionManager() {
5961
// Generate a new sessionID for every cold start.
60-
this(GaugeManager.getInstance(), PerfSession.createNewSession(), AppStateMonitor.getInstance());
62+
this(
63+
GaugeManager.getInstance(),
64+
PerfSession.createWithId(UUID.randomUUID().toString()),
65+
AppStateMonitor.getInstance());
6166
}
6267

6368
@VisibleForTesting
@@ -66,15 +71,50 @@ public SessionManager(
6671
this.gaugeManager = gaugeManager;
6772
this.perfSession = perfSession;
6873
this.appStateMonitor = appStateMonitor;
69-
AndroidLogger.getInstance().debug("CFPRS: SessionManager()");
74+
registerForAppState();
7075
}
7176

7277
/**
7378
* Finalizes gauge initialization during cold start. This must be called before app start finishes
7479
* (currently that is before onResume finishes) to ensure gauge collection starts on time.
7580
*/
7681
public void setApplicationContext(final Context appContext) {
77-
gaugeManager.initializeGaugeMetadataManager(appContext, ApplicationProcessState.FOREGROUND);
82+
// TODO(b/258263016): Migrate to go/firebase-android-executors
83+
@SuppressLint("ThreadPoolCreation")
84+
ExecutorService executorService = Executors.newSingleThreadExecutor();
85+
syncInitFuture =
86+
executorService.submit(
87+
() -> {
88+
gaugeManager.initializeGaugeMetadataManager(appContext);
89+
});
90+
}
91+
92+
@Override
93+
public void onUpdateAppState(ApplicationProcessState newAppState) {
94+
super.onUpdateAppState(newAppState);
95+
96+
if (appStateMonitor.isColdStart()) {
97+
// We want the Session to remain unchanged if this is a cold start of the app since we already
98+
// update the PerfSession in FirebasePerfProvider#onAttachInfo().
99+
return;
100+
}
101+
102+
if (newAppState == ApplicationProcessState.FOREGROUND) {
103+
// A new foregrounding of app will force a new sessionID generation.
104+
PerfSession session = PerfSession.createWithId(UUID.randomUUID().toString());
105+
updatePerfSession(session);
106+
} else {
107+
// If the session is running for too long, generate a new session and collect gauges as
108+
// necessary.
109+
if (perfSession.isSessionRunningTooLong()) {
110+
PerfSession session = PerfSession.createWithId(UUID.randomUUID().toString());
111+
updatePerfSession(session);
112+
} else {
113+
// For any other state change of the application, modify gauge collection state as
114+
// necessary.
115+
startOrStopCollectingGauges(newAppState);
116+
}
117+
}
78118
}
79119

80120
/**
@@ -98,7 +138,7 @@ public void stopGaugeCollectionIfSessionRunningTooLong() {
98138
*/
99139
public void updatePerfSession(PerfSession perfSession) {
100140
// Do not update the perf session if it is the exact same sessionId.
101-
if (Objects.equals(perfSession.sessionId(), this.perfSession.sessionId())) {
141+
if (perfSession.sessionId() == this.perfSession.sessionId()) {
102142
return;
103143
}
104144

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

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.firebase.components.Lazy;
2323
import com.google.firebase.perf.config.ConfigResolver;
2424
import com.google.firebase.perf.logging.AndroidLogger;
25-
import com.google.firebase.perf.session.FirebasePerformanceSessionSubscriber;
2625
import com.google.firebase.perf.session.PerfSession;
2726
import com.google.firebase.perf.transport.TransportManager;
2827
import com.google.firebase.perf.util.Timer;
@@ -60,7 +59,7 @@ public class GaugeManager {
6059
private final TransportManager transportManager;
6160

6261
@Nullable private GaugeMetadataManager gaugeMetadataManager;
63-
@Nullable private ScheduledFuture<?> gaugeManagerDataCollectionJob = null;
62+
@Nullable private ScheduledFuture gaugeManagerDataCollectionJob = null;
6463
@Nullable private String sessionId = null;
6564
private ApplicationProcessState applicationProcessState =
6665
ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN;
@@ -95,10 +94,8 @@ private GaugeManager() {
9594
}
9695

9796
/** Initializes GaugeMetadataManager which requires application context. */
98-
public void initializeGaugeMetadataManager(
99-
Context appContext, ApplicationProcessState applicationProcessState) {
97+
public void initializeGaugeMetadataManager(Context appContext) {
10098
this.gaugeMetadataManager = new GaugeMetadataManager(appContext);
101-
this.applicationProcessState = applicationProcessState;
10299
}
103100

104101
/** Returns the singleton instance of this class. */
@@ -209,7 +206,7 @@ public void stopCollectingGauges() {
209206

210207
// Flush any data that was collected for this session one last time.
211208
@SuppressWarnings("FutureReturnValueIgnored")
212-
ScheduledFuture<?> unusedFuture =
209+
ScheduledFuture unusedFuture =
213210
gaugeManagerExecutor
214211
.get()
215212
.schedule(
@@ -245,32 +242,31 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) {
245242
}
246243

247244
// Adding Session ID info.
248-
String aqsSessionId =
249-
FirebasePerformanceSessionSubscriber.Companion.getInstance()
250-
.getAqsMappedToPerfSession(sessionId);
251-
gaugeMetricBuilder.setSessionId(aqsSessionId);
252-
AndroidLogger.getInstance().debug("CFPR syncFlush: " + sessionId + " AQS: " + aqsSessionId);
245+
gaugeMetricBuilder.setSessionId(sessionId);
253246

254247
transportManager.log(gaugeMetricBuilder.build(), appState);
255248
}
256249

257250
/**
258251
* Log the Gauge Metadata information to the transport.
259252
*
260-
* @param aqsSessionId The {@link FirebasePerformanceSessionSubscriber#getAqsMappedToPerfSession(String)} to which the collected Gauge Metrics
253+
* @param sessionId The {@link PerfSession#sessionId()} to which the collected Gauge Metrics
261254
* should be associated with.
255+
* @param appState The {@link ApplicationProcessState} for which these gauges are collected.
262256
* @return true if GaugeMetadata was logged, false otherwise.
263257
*/
264-
public void logGaugeMetadata(String aqsSessionId) {
265-
// TODO(b/394127311): This can now throw an NPE. Explore if there's anything that should be
266-
// verified.
267-
AndroidLogger.getInstance().debug("CFPR logGaugeMetadata: " + aqsSessionId);
268-
GaugeMetric gaugeMetric =
269-
GaugeMetric.newBuilder()
270-
.setSessionId(aqsSessionId)
271-
.setGaugeMetadata(getGaugeMetadata())
272-
.build();
273-
transportManager.log(gaugeMetric, this.applicationProcessState);
258+
public boolean logGaugeMetadata(String sessionId, ApplicationProcessState appState) {
259+
// TODO(b/394127311): Re-introduce logging of metadata for AQS.
260+
if (gaugeMetadataManager != null) {
261+
GaugeMetric gaugeMetric =
262+
GaugeMetric.newBuilder()
263+
.setSessionId(sessionId)
264+
.setGaugeMetadata(getGaugeMetadata())
265+
.build();
266+
transportManager.log(gaugeMetric, appState);
267+
return true;
268+
}
269+
return false;
274270
}
275271

276272
private GaugeMetadata getGaugeMetadata() {

‎firebase-perf/src/test/java/com/google/firebase/perf/session/SessionManagerTest.java

Lines changed: 127 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
import static com.google.common.truth.Truth.assertThat;
1818
import static org.mockito.ArgumentMatchers.any;
19+
import static org.mockito.ArgumentMatchers.anyString;
20+
import static org.mockito.ArgumentMatchers.eq;
21+
import static org.mockito.ArgumentMatchers.nullable;
1922
import static org.mockito.Mockito.mock;
2023
import static org.mockito.Mockito.never;
2124
import static org.mockito.Mockito.spy;
@@ -37,6 +40,7 @@
3740
import org.junit.Before;
3841
import org.junit.Test;
3942
import org.junit.runner.RunWith;
43+
import org.mockito.AdditionalMatchers;
4044
import org.mockito.ArgumentMatchers;
4145
import org.mockito.InOrder;
4246
import org.mockito.Mock;
@@ -79,9 +83,123 @@ public void setApplicationContext_initializeGaugeMetadataManager()
7983
testSessionManager.setApplicationContext(mockApplicationContext);
8084

8185
testSessionManager.getSyncInitFuture().get();
82-
inOrder
83-
.verify(mockGaugeManager)
84-
.initializeGaugeMetadataManager(any(), ApplicationProcessState.FOREGROUND);
86+
inOrder.verify(mockGaugeManager).initializeGaugeMetadataManager(any());
87+
}
88+
89+
@Test
90+
public void testOnUpdateAppStateDoesNothingDuringAppStart() {
91+
String oldSessionId = SessionManager.getInstance().perfSession().sessionId();
92+
93+
assertThat(oldSessionId).isNotNull();
94+
assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId());
95+
96+
AppStateMonitor.getInstance().setIsColdStart(true);
97+
98+
SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.FOREGROUND);
99+
assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId());
100+
}
101+
102+
@Test
103+
public void testOnUpdateAppStateGeneratesNewSessionIdOnForegroundState() {
104+
String oldSessionId = SessionManager.getInstance().perfSession().sessionId();
105+
106+
assertThat(oldSessionId).isNotNull();
107+
assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId());
108+
109+
SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.FOREGROUND);
110+
assertThat(oldSessionId).isNotEqualTo(SessionManager.getInstance().perfSession().sessionId());
111+
}
112+
113+
@Test
114+
public void testOnUpdateAppStateDoesntGenerateNewSessionIdOnBackgroundState() {
115+
String oldSessionId = SessionManager.getInstance().perfSession().sessionId();
116+
117+
assertThat(oldSessionId).isNotNull();
118+
assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId());
119+
120+
SessionManager.getInstance().onUpdateAppState(ApplicationProcessState.BACKGROUND);
121+
assertThat(oldSessionId).isEqualTo(SessionManager.getInstance().perfSession().sessionId());
122+
}
123+
124+
@Test
125+
public void testOnUpdateAppStateGeneratesNewSessionIdOnBackgroundStateIfPerfSessionExpires() {
126+
when(mockPerfSession.isSessionRunningTooLong()).thenReturn(true);
127+
SessionManager testSessionManager =
128+
new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor);
129+
String oldSessionId = testSessionManager.perfSession().sessionId();
130+
131+
assertThat(oldSessionId).isNotNull();
132+
assertThat(oldSessionId).isEqualTo(testSessionManager.perfSession().sessionId());
133+
134+
testSessionManager.onUpdateAppState(ApplicationProcessState.BACKGROUND);
135+
assertThat(oldSessionId).isNotEqualTo(testSessionManager.perfSession().sessionId());
136+
}
137+
138+
@Test
139+
public void
140+
testOnUpdateAppStateDoesntMakeGaugeManagerLogGaugeMetadataOnForegroundStateIfSessionIsNonVerbose() {
141+
forceNonVerboseSession();
142+
143+
SessionManager testSessionManager =
144+
new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor);
145+
testSessionManager.onUpdateAppState(ApplicationProcessState.FOREGROUND);
146+
147+
verify(mockGaugeManager, never())
148+
.logGaugeMetadata(
149+
anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class));
150+
}
151+
152+
@Test
153+
public void
154+
testOnUpdateAppStateDoesntMakeGaugeManagerLogGaugeMetadataOnBackgroundStateEvenIfSessionIsVerbose() {
155+
forceVerboseSession();
156+
157+
SessionManager testSessionManager =
158+
new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor);
159+
testSessionManager.onUpdateAppState(ApplicationProcessState.BACKGROUND);
160+
161+
verify(mockGaugeManager, never())
162+
.logGaugeMetadata(
163+
anyString(), nullable(com.google.firebase.perf.v1.ApplicationProcessState.class));
164+
}
165+
166+
@Test
167+
public void testOnUpdateAppStateMakesGaugeManagerStartCollectingGaugesIfSessionIsVerbose() {
168+
forceVerboseSession();
169+
170+
SessionManager testSessionManager =
171+
new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor);
172+
testSessionManager.onUpdateAppState(ApplicationProcessState.FOREGROUND);
173+
174+
verify(mockGaugeManager)
175+
.startCollectingGauges(AdditionalMatchers.not(eq(mockPerfSession)), any());
176+
}
177+
178+
// LogGaugeData on new perf session when Verbose
179+
// NotLogGaugeData on new perf session when not Verbose
180+
// Mark Session as expired after time limit.
181+
182+
@Test
183+
public void testOnUpdateAppStateMakesGaugeManagerStopCollectingGaugesIfSessionIsNonVerbose() {
184+
forceNonVerboseSession();
185+
186+
SessionManager testSessionManager =
187+
new SessionManager(mockGaugeManager, mockPerfSession, mockAppStateMonitor);
188+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId"));
189+
190+
verify(mockGaugeManager).stopCollectingGauges();
191+
}
192+
193+
@Test
194+
public void testOnUpdateAppStateMakesGaugeManagerStopCollectingGaugesWhenSessionsDisabled() {
195+
forceSessionsFeatureDisabled();
196+
197+
SessionManager testSessionManager =
198+
new SessionManager(
199+
mockGaugeManager, PerfSession.createWithId("testSessionId"), mockAppStateMonitor);
200+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId2"));
201+
202+
verify(mockGaugeManager).stopCollectingGauges();
85203
}
86204

87205
@Test
@@ -118,7 +236,7 @@ public void testPerfSessionExpiredMakesGaugeManagerStopsCollectingGaugesIfSessio
118236
.thenReturn(TimeUnit.HOURS.toMicros(5)); // Default Max Session Length is 4 hours
119237

120238
assertThat(session.isSessionRunningTooLong()).isTrue();
121-
verify(mockGaugeManager, times(0)).logGaugeMetadata(any());
239+
verify(mockGaugeManager, times(0)).logGaugeMetadata(any(), any());
122240
}
123241

124242
@Test
@@ -129,7 +247,7 @@ public void testPerfSession_sessionAwareObjects_doesntNotifyIfNotRegistered() {
129247
FakeSessionAwareObject spySessionAwareObjectOne = spy(new FakeSessionAwareObject());
130248
FakeSessionAwareObject spySessionAwareObjectTwo = spy(new FakeSessionAwareObject());
131249

132-
testSessionManager.updatePerfSession(PerfSession.createNewSession());
250+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId1"));
133251

134252
verify(spySessionAwareObjectOne, never())
135253
.updateSession(ArgumentMatchers.nullable(PerfSession.class));
@@ -148,8 +266,8 @@ public void testPerfSession_sessionAwareObjects_NotifiesIfRegistered() {
148266
testSessionManager.registerForSessionUpdates(new WeakReference<>(spySessionAwareObjectOne));
149267
testSessionManager.registerForSessionUpdates(new WeakReference<>(spySessionAwareObjectTwo));
150268

151-
testSessionManager.updatePerfSession(PerfSession.createNewSession());
152-
testSessionManager.updatePerfSession(PerfSession.createNewSession());
269+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId1"));
270+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId2"));
153271

154272
verify(spySessionAwareObjectOne, times(2))
155273
.updateSession(ArgumentMatchers.nullable(PerfSession.class));
@@ -173,11 +291,11 @@ public void testPerfSession_sessionAwareObjects_DoesNotNotifyIfUnregistered() {
173291
testSessionManager.registerForSessionUpdates(weakSpySessionAwareObjectOne);
174292
testSessionManager.registerForSessionUpdates(weakSpySessionAwareObjectTwo);
175293

176-
testSessionManager.updatePerfSession(PerfSession.createNewSession());
294+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId1"));
177295

178296
testSessionManager.unregisterForSessionUpdates(weakSpySessionAwareObjectOne);
179297
testSessionManager.unregisterForSessionUpdates(weakSpySessionAwareObjectTwo);
180-
testSessionManager.updatePerfSession(PerfSession.createNewSession());
298+
testSessionManager.updatePerfSession(PerfSession.createWithId("testSessionId2"));
181299

182300
verify(spySessionAwareObjectOne, times(1))
183301
.updateSession(ArgumentMatchers.nullable(PerfSession.class));

0 commit comments

Comments
 (0)