Skip to content

Commit 6b54fa2

Browse files
tejasdtejasd
committed
Update Gauge logging to better match the implementation on main (#6992)
- Update frequency of Gauge logging to better match the existing implementation. - Update GaugeMetadata logging to only log metadata if it's a verbose session.
1 parent e298094 commit 6b54fa2

File tree

6 files changed

+92
-48
lines changed

6 files changed

+92
-48
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
package com.google.firebase.perf.session
1818

1919
import com.google.firebase.perf.logging.FirebaseSessionsEnforcementCheck
20-
import com.google.firebase.perf.session.gauges.GaugeManager
21-
import com.google.firebase.perf.v1.ApplicationProcessState
2220
import com.google.firebase.sessions.api.SessionSubscriber
2321

2422
class FirebasePerformanceSessionSubscriber(override val isDataCollectionEnabled: Boolean) :
@@ -33,7 +31,5 @@ class FirebasePerformanceSessionSubscriber(override val isDataCollectionEnabled:
3331

3432
val updatedSession = PerfSession.createWithId(sessionDetails.sessionId)
3533
SessionManager.getInstance().updatePerfSession(updatedSession)
36-
GaugeManager.getInstance()
37-
.logGaugeMetadata(updatedSession.sessionId(), ApplicationProcessState.FOREGROUND)
3834
}
3935
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ public void updatePerfSession(PerfSession perfSession) {
115115
}
116116
}
117117

118+
// Log gauge metadata.
119+
logGaugeMetadataIfCollectionEnabled();
120+
118121
// Start of stop the gauge data collection.
119122
startOrStopCollectingGauges();
120123
}
@@ -153,6 +156,14 @@ public void unregisterForSessionUpdates(WeakReference<SessionAwareObject> client
153156
}
154157
}
155158

159+
private void logGaugeMetadataIfCollectionEnabled() {
160+
FirebaseSessionsEnforcementCheck.checkSession(
161+
perfSession, "logGaugeMetadataIfCollectionEnabled");
162+
if (perfSession.isVerbose()) {
163+
gaugeManager.logGaugeMetadata(perfSession.sessionId());
164+
}
165+
}
166+
156167
private void startOrStopCollectingGauges() {
157168
FirebaseSessionsEnforcementCheck.checkSession(perfSession, "startOrStopCollectingGauges");
158169

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,16 @@
1515
package com.google.firebase.perf.session.gauges
1616

1717
import androidx.annotation.VisibleForTesting
18-
import com.google.firebase.perf.logging.AndroidLogger
1918
import java.util.concurrent.atomic.AtomicInteger
2019

2120
/**
22-
* [GaugeCounter] is a thread-safe counter for gauge metrics. If the metrics count exceeds
23-
* [MAX_METRIC_COUNT], it attempts to log the metrics to Firelog.
21+
* [GaugeCounter] is a thread-safe counter for gauge metrics. If the metrics count reaches or
22+
* exceeds [MAX_METRIC_COUNT], it attempts to log the metrics to Firelog.
2423
*/
2524
object GaugeCounter {
26-
private const val MAX_METRIC_COUNT = 25
25+
private const val MAX_METRIC_COUNT = 50
26+
// For debugging explore re-introducing logging.
2727
private val counter = AtomicInteger(0)
28-
private val logger = AndroidLogger.getInstance()
2928

3029
@set:VisibleForTesting(otherwise = VisibleForTesting.NONE)
3130
@set:JvmStatic
@@ -36,16 +35,16 @@ object GaugeCounter {
3635
val metricsCount = counter.incrementAndGet()
3736

3837
if (metricsCount >= MAX_METRIC_COUNT) {
38+
// TODO(b/394127311): There can be rare conditions where there's an attempt to log metrics
39+
// even when it's currently logging them. While this is a no-op, it might be worth
40+
// exploring optimizing it further to prevent additional calls to [GaugeManager].
3941
gaugeManager.logGaugeMetrics()
4042
}
41-
42-
logger.debug("Incremented logger to $metricsCount")
4343
}
4444

4545
@JvmStatic
4646
fun decrementCounter() {
47-
val curr = counter.decrementAndGet()
48-
logger.debug("Decremented logger to $curr")
47+
counter.decrementAndGet()
4948
}
5049

5150
@VisibleForTesting(otherwise = VisibleForTesting.NONE)

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,18 +274,17 @@ private void syncFlush(String sessionId, ApplicationProcessState appState) {
274274
* Log the Gauge Metadata information to the transport.
275275
*
276276
* @param sessionId The {@link PerfSession#sessionId()} ()} to which the collected Gauge Metrics
277-
* should be associated with.
278-
* @param appState The {@link ApplicationProcessState} for which these gauges are collected.
277+
* should be associated with.
279278
* @return true if GaugeMetadata was logged, false otherwise.
280279
*/
281-
public boolean logGaugeMetadata(String sessionId, ApplicationProcessState appState) {
280+
public boolean logGaugeMetadata(String sessionId) {
282281
if (gaugeMetadataManager != null) {
283282
GaugeMetric gaugeMetric =
284283
GaugeMetric.newBuilder()
285284
.setSessionId(sessionId)
286285
.setGaugeMetadata(getGaugeMetadata())
287286
.build();
288-
transportManager.log(gaugeMetric, appState);
287+
transportManager.log(gaugeMetric);
289288
return true;
290289
}
291290
return false;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,6 @@ public void log(final GaugeMetric gaugeMetric) {
356356
* {@link #isAllowedToDispatch(PerfMetric)}).
357357
*/
358358
public void log(final GaugeMetric gaugeMetric, final ApplicationProcessState appState) {
359-
// TODO(b/394127311): This *might* potentially be the right place to get AQS.
360359
executorService.execute(
361360
() -> syncLog(PerfMetric.newBuilder().setGaugeMetric(gaugeMetric), appState));
362361
}

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

Lines changed: 70 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,14 @@ public final class GaugeManagerTest extends FirebasePerformanceTestBase {
6060
// This is a guesstimate of the max amount of time to wait before any pending metrics' collection
6161
// might take.
6262
private static final long TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS = 20;
63-
private static final long APPROX_NUMBER_OF_DATA_POINTS_PER_GAUGE_METRIC = 20;
6463
private static final long DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_BG_MS = 100;
6564
private static final long DEFAULT_CPU_GAUGE_COLLECTION_FREQUENCY_FG_MS = 50;
6665
private static final long DEFAULT_MEMORY_GAUGE_COLLECTION_FREQUENCY_BG_MS = 120;
6766
private static final long DEFAULT_MEMORY_GAUGE_COLLECTION_FREQUENCY_FG_MS = 60;
6867

68+
// See [com.google.firebase.perf.session.gauges.GaugeCounter].
69+
private static final long MAX_GAUGE_COUNTER_LIMIT = 50;
70+
6971
private GaugeManager testGaugeManager = null;
7072
private FakeScheduledExecutorService fakeScheduledExecutorService = null;
7173
private TransportManager mockTransportManager = null;
@@ -340,8 +342,7 @@ public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() {
340342
// There's no job to log the gauges.
341343
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
342344

343-
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
344-
generateMetricsAndIncrementCounter(20);
345+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
345346

346347
// There's still no job to log the gauges.
347348
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -366,7 +367,7 @@ public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() {
366367
int recordedGaugeMetricsCount =
367368
recordedGaugeMetric.getAndroidMemoryReadingsCount()
368369
+ recordedGaugeMetric.getCpuMetricReadingsCount();
369-
assertThat(recordedGaugeMetricsCount).isEqualTo(30);
370+
assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT);
370371

371372
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
372373
}
@@ -383,8 +384,7 @@ public void testGaugeCounterIsDecrementedWhenLogged() {
383384
// There's no job to log the gauges.
384385
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
385386

386-
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
387-
generateMetricsAndIncrementCounter(20);
387+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
388388

389389
// There's still no job to log the gauges.
390390
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -395,9 +395,47 @@ public void testGaugeCounterIsDecrementedWhenLogged() {
395395
assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS))
396396
.isEqualTo(TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS);
397397

398-
assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter + 30);
398+
assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter + MAX_GAUGE_COUNTER_LIMIT);
399+
fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask();
400+
401+
assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter);
402+
}
403+
404+
@Test
405+
public void testDuplicateGaugeLoggingIsAvoided() {
406+
int priorGaugeCounter = GaugeCounter.count();
407+
PerfSession fakeSession = createTestSession(1);
408+
testGaugeManager.setApplicationProcessState(ApplicationProcessState.FOREGROUND);
409+
testGaugeManager.startCollectingGauges(fakeSession);
410+
GaugeCounter.setGaugeManager(testGaugeManager);
411+
412+
// There's no job to log the gauges.
413+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
414+
415+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 20);
416+
417+
// There's still no job to log the gauges.
418+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
419+
420+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT);
421+
422+
assertThat(fakeScheduledExecutorService.isEmpty()).isFalse();
423+
assertThat(fakeScheduledExecutorService.getDelayToNextTask(TimeUnit.MILLISECONDS))
424+
.isEqualTo(TIME_TO_WAIT_BEFORE_FLUSHING_GAUGES_QUEUE_MS);
425+
399426
fakeScheduledExecutorService.simulateSleepExecutingAtMostOneTask();
427+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
400428

429+
GaugeMetric recordedGaugeMetric =
430+
getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
431+
432+
// It flushes all the metrics in the ConcurrentLinkedQueues that were added.
433+
int recordedGaugeMetricsCount =
434+
recordedGaugeMetric.getAndroidMemoryReadingsCount()
435+
+ recordedGaugeMetric.getCpuMetricReadingsCount();
436+
assertThat(recordedGaugeMetricsCount).isEqualTo(2 * MAX_GAUGE_COUNTER_LIMIT - 20);
437+
438+
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
401439
assertThat(GaugeCounter.count()).isEqualTo(priorGaugeCounter);
402440
}
403441

@@ -410,7 +448,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
410448
GaugeCounter.setGaugeManager(testGaugeManager);
411449

412450
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
413-
generateMetricsAndIncrementCounter(10);
451+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
414452

415453
// There's no job to log the gauges.
416454
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -425,7 +463,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
425463
shadowOf(Looper.getMainLooper()).idle();
426464

427465
// Generate additional metrics in the new app state.
428-
generateMetricsAndIncrementCounter(26);
466+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT + 1);
429467

430468
GaugeMetric recordedGaugeMetric =
431469
getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
@@ -434,7 +472,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
434472
int recordedGaugeMetricsCount =
435473
recordedGaugeMetric.getAndroidMemoryReadingsCount()
436474
+ recordedGaugeMetric.getCpuMetricReadingsCount();
437-
assertThat(recordedGaugeMetricsCount).isEqualTo(10);
475+
assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT - 10);
438476

439477
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
440478

@@ -448,7 +486,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
448486
recordedGaugeMetricsCount =
449487
recordedGaugeMetric.getAndroidMemoryReadingsCount()
450488
+ recordedGaugeMetric.getCpuMetricReadingsCount();
451-
assertThat(recordedGaugeMetricsCount).isEqualTo(26);
489+
assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT + 1);
452490

453491
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
454492
}
@@ -462,7 +500,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
462500
GaugeCounter.setGaugeManager(testGaugeManager);
463501

464502
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
465-
generateMetricsAndIncrementCounter(10);
503+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT - 10);
466504

467505
PerfSession updatedPerfSession = createTestSession(2);
468506
updatedPerfSession.setGaugeAndEventCollectionEnabled(true);
@@ -479,7 +517,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
479517
shadowOf(Looper.getMainLooper()).idle();
480518

481519
// Generate metrics for the new session.
482-
generateMetricsAndIncrementCounter(26);
520+
generateMetricsAndIncrementCounter(MAX_GAUGE_COUNTER_LIMIT + 1);
483521

484522
GaugeMetric recordedGaugeMetric =
485523
getLastRecordedGaugeMetric(ApplicationProcessState.BACKGROUND);
@@ -488,7 +526,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
488526
int recordedGaugeMetricsCount =
489527
recordedGaugeMetric.getAndroidMemoryReadingsCount()
490528
+ recordedGaugeMetric.getCpuMetricReadingsCount();
491-
assertThat(recordedGaugeMetricsCount).isEqualTo(10);
529+
assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT - 10);
492530

493531
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
494532

@@ -502,7 +540,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
502540
recordedGaugeMetricsCount =
503541
recordedGaugeMetric.getAndroidMemoryReadingsCount()
504542
+ recordedGaugeMetric.getCpuMetricReadingsCount();
505-
assertThat(recordedGaugeMetricsCount).isEqualTo(26);
543+
assertThat(recordedGaugeMetricsCount).isEqualTo(MAX_GAUGE_COUNTER_LIMIT + 1);
506544

507545
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(2));
508546
}
@@ -559,10 +597,10 @@ public void testLogGaugeMetadataSendDataToTransport() {
559597
when(fakeGaugeMetadataManager.getMaxAppJavaHeapMemoryKb()).thenReturn(1000);
560598
when(fakeGaugeMetadataManager.getMaxEncouragedAppJavaHeapMemoryKb()).thenReturn(800);
561599

562-
testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND);
600+
testGaugeManager.logGaugeMetadata(testSessionId(1));
563601

564602
GaugeMetric recordedGaugeMetric =
565-
getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
603+
getLastRecordedGaugeMetric(ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN);
566604
GaugeMetadata recordedGaugeMetadata = recordedGaugeMetric.getGaugeMetadata();
567605

568606
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
@@ -586,9 +624,7 @@ public void testLogGaugeMetadataDoesNotLogWhenGaugeMetadataManagerNotAvailable()
586624
new Lazy<>(() -> fakeCpuGaugeCollector),
587625
new Lazy<>(() -> fakeMemoryGaugeCollector));
588626

589-
assertThat(
590-
testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
591-
.isFalse();
627+
assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isFalse();
592628
}
593629

594630
@Test
@@ -603,17 +639,13 @@ public void testLogGaugeMetadataLogsAfterApplicationContextIsSet() {
603639
new Lazy<>(() -> fakeCpuGaugeCollector),
604640
new Lazy<>(() -> fakeMemoryGaugeCollector));
605641

606-
assertThat(
607-
testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
608-
.isFalse();
642+
assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isFalse();
609643

610644
testGaugeManager.initializeGaugeMetadataManager(ApplicationProvider.getApplicationContext());
611-
assertThat(
612-
testGaugeManager.logGaugeMetadata(testSessionId(1), ApplicationProcessState.FOREGROUND))
613-
.isTrue();
645+
assertThat(testGaugeManager.logGaugeMetadata(testSessionId(1))).isTrue();
614646

615647
GaugeMetric recordedGaugeMetric =
616-
getLastRecordedGaugeMetric(ApplicationProcessState.FOREGROUND);
648+
getLastRecordedGaugeMetric(ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN);
617649
GaugeMetadata recordedGaugeMetadata = recordedGaugeMetric.getGaugeMetadata();
618650

619651
assertThat(recordedGaugeMetric.getSessionId()).isEqualTo(testSessionId(1));
@@ -638,7 +670,7 @@ private long getMinimumBackgroundCollectionFrequency() {
638670
}
639671

640672
// Simulates the behavior of Cpu and Memory Gauge collector.
641-
private void generateMetricsAndIncrementCounter(int count) {
673+
private void generateMetricsAndIncrementCounter(long count) {
642674
// TODO(b/394127311): Explore actually collecting metrics using the fake Cpu and Memory
643675
// metric collectors.
644676
Random random = new Random();
@@ -679,8 +711,16 @@ private AndroidMemoryReading createFakeAndroidMetricReading(int currentUsedAppJa
679711
private GaugeMetric getLastRecordedGaugeMetric(
680712
ApplicationProcessState expectedApplicationProcessState) {
681713
ArgumentCaptor<GaugeMetric> argMetric = ArgumentCaptor.forClass(GaugeMetric.class);
682-
verify(mockTransportManager, times(1))
683-
.log(argMetric.capture(), eq(expectedApplicationProcessState));
714+
715+
// TODO(b/394127311): Revisit transportManager.log method which is only being called in unit
716+
// tests.
717+
if (expectedApplicationProcessState
718+
== ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN) {
719+
verify(mockTransportManager, times(1)).log(argMetric.capture());
720+
} else {
721+
verify(mockTransportManager, times(1))
722+
.log(argMetric.capture(), eq(expectedApplicationProcessState));
723+
}
684724
reset(mockTransportManager);
685725
// Required after resetting the mock. By default we assume that Transport is initialized.
686726
when(mockTransportManager.isInitialized()).thenReturn(true);

0 commit comments

Comments
 (0)