Skip to content

Commit dd04244

Browse files
mrobermrober
authored
Add different default sampling rates for when RC fetch failed (#5059)
* Add different defaults for when RC fetch failed * Add changelog entry * Format * Add throttled case to isLastFetchFailed check * Add rc fetch failed defaults for cpu and memory capturing frequency
1 parent 174e7d8 commit dd04244

File tree

8 files changed

+149
-17
lines changed

8 files changed

+149
-17
lines changed

‎firebase-perf/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Unreleased
22

33
* [fixed] Fixed app start trace creation where some measured time could be NULL (#4730).
4+
* [changed] Adjusted default behavior when remote config fetch fails.
45

56

67
# 20.3.2
@@ -357,4 +358,3 @@ updates.
357358

358359
# 16.1.0
359360
* [fixed] Fixed a `SecurityException` crash on certain devices that do not have Google Play Services on them.
360-

‎firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,8 @@ public double getTraceSamplingRate() {
290290
// Order of precedence is:
291291
// 1. If the value exists through Firebase Remote Config, cache and return this value.
292292
// 2. If the value exists in device cache, return this value.
293-
// 3. Otherwise, return default value.
293+
// 3. If the Firebase Remote Config fetch failed, return the default "rc failure" value.
294+
// 4. Otherwise, return default value.
294295
TraceSamplingRate config = TraceSamplingRate.getInstance();
295296

296297
// 1. Reads value from Firebase Remote Config, saves this value in cache layer if valid.
@@ -306,7 +307,12 @@ public double getTraceSamplingRate() {
306307
return deviceCacheValue.get();
307308
}
308309

309-
// 3. Returns default value if there is no valid value from above approaches.
310+
// 3. Check if RC fetch failed.
311+
if (remoteConfigManager.isLastFetchFailed()) {
312+
return config.getDefaultOnRcFetchFail();
313+
}
314+
315+
// 4. Returns default value if there is no valid value from above approaches.
310316
return config.getDefault();
311317
}
312318

@@ -315,7 +321,8 @@ public double getNetworkRequestSamplingRate() {
315321
// Order of precedence is:
316322
// 1. If the value exists through Firebase Remote Config, cache and return this value.
317323
// 2. If the value exists in device cache, return this value.
318-
// 3. Otherwise, return default value.
324+
// 3. If the Firebase Remote Config fetch failed, return the default "rc failure" value.
325+
// 4. Otherwise, return default value.
319326
NetworkRequestSamplingRate config = NetworkRequestSamplingRate.getInstance();
320327

321328
// 1. Reads value from Firebase Remote Config, saves this value in cache layer if valid.
@@ -331,7 +338,12 @@ public double getNetworkRequestSamplingRate() {
331338
return deviceCacheValue.get();
332339
}
333340

334-
// 3. Returns default value if there is no valid value from above approaches.
341+
// 3. Check if RC fetch failed.
342+
if (remoteConfigManager.isLastFetchFailed()) {
343+
return config.getDefaultOnRcFetchFail();
344+
}
345+
346+
// 4. Returns default value if there is no valid value from above approaches.
335347
return config.getDefault();
336348
}
337349

@@ -342,7 +354,8 @@ public double getSessionsSamplingRate() {
342354
// and return this value.
343355
// 2. If the value exists through Firebase Remote Config, cache and return this value.
344356
// 3. If the value exists in device cache, return this value.
345-
// 4. Otherwise, return default value.
357+
// 4. If the Firebase Remote Config fetch failed, return the default "rc failure" value.
358+
// 5. Otherwise, return default value.
346359
SessionsSamplingRate config = SessionsSamplingRate.getInstance();
347360

348361
// 1. Reads value in Android Manifest (it is set by developers during build time).
@@ -368,7 +381,12 @@ public double getSessionsSamplingRate() {
368381
return deviceCacheValue.get();
369382
}
370383

371-
// 4. Returns default value if there is no valid value from above approaches.
384+
// 4. Check if RC fetch failed.
385+
if (remoteConfigManager.isLastFetchFailed()) {
386+
return config.getDefaultOnRcFetchFail();
387+
}
388+
389+
// 5. Returns default value if there is no valid value from above approaches.
372390
return config.getDefault();
373391
}
374392

@@ -381,7 +399,8 @@ public long getSessionsCpuCaptureFrequencyForegroundMs() {
381399
// 1. If the value exists in Android Manifest, return this value.
382400
// 2. If the value exists through Firebase Remote Config, cache and return this value.
383401
// 3. If the value exists in device cache, return this value.
384-
// 4. Otherwise, return default value.
402+
// 4. If the Firebase Remote Config fetch failed, return default failure value.
403+
// 5. Otherwise, return default value.
385404
SessionsCpuCaptureFrequencyForegroundMs config =
386405
SessionsCpuCaptureFrequencyForegroundMs.getInstance();
387406

@@ -404,7 +423,12 @@ public long getSessionsCpuCaptureFrequencyForegroundMs() {
404423
return deviceCacheValue.get();
405424
}
406425

407-
// 4. Returns default value if there is no valid value from above approaches.
426+
// 4. Check if RC fetch failed.
427+
if (remoteConfigManager.isLastFetchFailed()) {
428+
return config.getDefaultOnRcFetchFail();
429+
}
430+
431+
// 5. Returns default value if there is no valid value from above approaches.
408432
return config.getDefault();
409433
}
410434

@@ -453,7 +477,8 @@ public long getSessionsMemoryCaptureFrequencyForegroundMs() {
453477
// 1. If the value exists in Android Manifest, return this value.
454478
// 2. If the value exists through Firebase Remote Config, cache and return this value.
455479
// 3. If the value exists in device cache, return this value.
456-
// 4. Otherwise, return default value.
480+
// 4. If the Firebase Remote Config fetch failed, return default failure value.
481+
// 5. Otherwise, return default value.
457482
SessionsMemoryCaptureFrequencyForegroundMs config =
458483
SessionsMemoryCaptureFrequencyForegroundMs.getInstance();
459484

@@ -476,7 +501,12 @@ public long getSessionsMemoryCaptureFrequencyForegroundMs() {
476501
return deviceCacheValue.get();
477502
}
478503

479-
// 4. Returns default value if there is no valid value from above approaches.
504+
// 4. Check if RC fetch failed.
505+
if (remoteConfigManager.isLastFetchFailed()) {
506+
return config.getDefaultOnRcFetchFail();
507+
}
508+
509+
// 5. Returns default value if there is no valid value from above approaches.
480510
return config.getDefault();
481511
}
482512

‎firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationConstants.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,12 @@ protected Double getDefault() {
152152
return 1.0;
153153
}
154154

155+
@Override
156+
protected Double getDefaultOnRcFetchFail() {
157+
// Reduce the typical default by 2 orders of magnitude.
158+
return getDefault() / 100;
159+
}
160+
155161
@Override
156162
protected String getRemoteConfigFlag() {
157163
return "fpr_vc_trace_sampling_rate";
@@ -185,6 +191,12 @@ protected Double getDefault() {
185191
return 1.0;
186192
}
187193

194+
@Override
195+
protected Double getDefaultOnRcFetchFail() {
196+
// Reduce the typical default by 2 orders of magnitude.
197+
return getDefault() / 100;
198+
}
199+
188200
@Override
189201
protected String getRemoteConfigFlag() {
190202
return "fpr_vc_network_request_sampling_rate";
@@ -218,6 +230,11 @@ protected Long getDefault() {
218230
return 100L;
219231
}
220232

233+
@Override
234+
protected Long getDefaultOnRcFetchFail() {
235+
return getDefault() * 3;
236+
}
237+
221238
@Override
222239
protected String getRemoteConfigFlag() {
223240
return "fpr_session_gauge_cpu_capture_frequency_fg_ms";
@@ -294,6 +311,11 @@ protected Long getDefault() {
294311
return 100L;
295312
}
296313

314+
@Override
315+
protected Long getDefaultOnRcFetchFail() {
316+
return getDefault() * 3;
317+
}
318+
297319
@Override
298320
protected String getRemoteConfigFlag() {
299321
return "fpr_session_gauge_memory_capture_frequency_fg_ms";
@@ -553,6 +575,12 @@ protected Double getDefault() {
553575
return 0.01;
554576
}
555577

578+
@Override
579+
protected Double getDefaultOnRcFetchFail() {
580+
// Reduce the typical default by 2 orders of magnitude.
581+
return getDefault() / 100;
582+
}
583+
556584
@Override
557585
protected String getDeviceCacheFlag() {
558586
return "com.google.firebase.perf.SessionSamplingRate";

‎firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationFlag.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ abstract class ConfigurationFlag<T> {
2727
/* Default value for this flag. */
2828
protected abstract T getDefault();
2929

30+
/* Default value for this flag in the case that RC fetch explicitly failed. */
31+
protected T getDefaultOnRcFetchFail() {
32+
// Same as typical default unless overridden.
33+
return getDefault();
34+
}
35+
3036
/* Configuration flag ID for Firebase Remote Config. */
3137
@Nullable
3238
String getRemoteConfigFlag() {

‎firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ private FirebaseRemoteConfigValue getRemoteConfigValue(String key) {
313313
public boolean isLastFetchFailed() {
314314
return firebaseRemoteConfig == null
315315
|| (firebaseRemoteConfig.getInfo().getLastFetchStatus()
316-
== FirebaseRemoteConfig.LAST_FETCH_STATUS_FAILURE);
316+
== FirebaseRemoteConfig.LAST_FETCH_STATUS_FAILURE)
317+
|| (firebaseRemoteConfig.getInfo().getLastFetchStatus()
318+
== FirebaseRemoteConfig.LAST_FETCH_STATUS_THROTTLED);
317319
}
318320

319321
/**
@@ -344,7 +346,8 @@ private void triggerFirebaseRemoteConfigFetchAndActivateOnSuccessfulFetch() {
344346
.addOnSuccessListener(executor, result -> syncConfigValues(firebaseRemoteConfig.getAll()))
345347
.addOnFailureListener(
346348
executor,
347-
task -> {
349+
ex -> {
350+
logger.debug("Remote config fetch failed: %s", ex);
348351
firebaseRemoteConfigLastFetchTimestampMs = FETCH_NEVER_HAPPENED_TIMESTAMP_MS;
349352
});
350353
}

‎firebase-perf/src/test/java/com/google/firebase/perf/config/ConfigResolverTest.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,18 @@ public void getSessionsCpuCaptureFrequencyForegroundMs_validMetadata_notSaveMeta
647647
assertThat(testConfigResolver.getSessionsCpuCaptureFrequencyForegroundMs()).isEqualTo(100L);
648648
}
649649

650+
@Test
651+
public void
652+
getSessionsCpuCaptureFrequencyForegroundMs_remoteConfigFetchFailed_returnDefaultRCValue() {
653+
when(mockRemoteConfigManager.getLong(SESSIONS_CPU_CAPTURE_FREQUENCY_FG_MS_FRC_KEY))
654+
.thenReturn(Optional.absent());
655+
when(mockDeviceCacheManager.getLong(SESSIONS_CPU_CAPTURE_FREQUENCY_FG_MS_CACHE_KEY))
656+
.thenReturn(Optional.absent());
657+
when(mockRemoteConfigManager.isLastFetchFailed()).thenReturn(true);
658+
659+
assertThat(testConfigResolver.getSessionsCpuCaptureFrequencyForegroundMs()).isEqualTo(300L);
660+
}
661+
650662
@Test
651663
public void
652664
getSessionsCpuCaptureFrequencyForegroundMs_invalidAndroidMetadataBundle_returnRemoteConfigValue() {
@@ -1051,6 +1063,18 @@ public void getSessionsMemoryCaptureFrequencyForegroundMs_validMetadata_notSaveM
10511063
assertThat(testConfigResolver.getSessionsMemoryCaptureFrequencyForegroundMs()).isEqualTo(100L);
10521064
}
10531065

1066+
@Test
1067+
public void
1068+
getSessionsMemoryCaptureFrequencyForegroundMs_remoteConfigFetchFailed_returnDefaultRCValue() {
1069+
when(mockRemoteConfigManager.getLong(SESSIONS_MEMORY_CAPTURE_FREQUENCY_FG_MS_FRC_KEY))
1070+
.thenReturn(Optional.absent());
1071+
when(mockDeviceCacheManager.getLong(SESSIONS_MEMORY_CAPTURE_FREQUENCY_FG_MS_CACHE_KEY))
1072+
.thenReturn(Optional.absent());
1073+
when(mockRemoteConfigManager.isLastFetchFailed()).thenReturn(true);
1074+
1075+
assertThat(testConfigResolver.getSessionsMemoryCaptureFrequencyForegroundMs()).isEqualTo(300L);
1076+
}
1077+
10541078
@Test
10551079
public void
10561080
getSessionsMemoryCaptureFrequencyForegroundMs_invalidAndroidMetadataBundle_returnRemoteConfigValue() {
@@ -1653,6 +1677,17 @@ public void getTraceSamplingRate_noRemoteConfig_returnsDefault() {
16531677
assertThat(testConfigResolver.getTraceSamplingRate()).isEqualTo(1.00);
16541678
}
16551679

1680+
@Test
1681+
public void getTraceSamplingRate_remoteConfigFetchFailed_returnsRCFailureDefault() {
1682+
when(mockRemoteConfigManager.getDouble(TRACE_SAMPLING_RATE_FRC_KEY))
1683+
.thenReturn(Optional.absent());
1684+
when(mockDeviceCacheManager.getDouble(TRACE_SAMPLING_RATE_CACHE_KEY))
1685+
.thenReturn(Optional.absent());
1686+
when(mockRemoteConfigManager.isLastFetchFailed()).thenReturn(true);
1687+
1688+
assertThat(testConfigResolver.getTraceSamplingRate()).isEqualTo(1.00 / 100);
1689+
}
1690+
16561691
@Test
16571692
public void getTraceSamplingRate_noRemoteConfigHasCache_returnsCache() {
16581693
when(mockRemoteConfigManager.getDouble(TRACE_SAMPLING_RATE_FRC_KEY))
@@ -1714,6 +1749,17 @@ public void getNetworkRequestSamplingRate_noRemoteConfig_returnsDefault() {
17141749
assertThat(testConfigResolver.getNetworkRequestSamplingRate()).isEqualTo(1.00);
17151750
}
17161751

1752+
@Test
1753+
public void getNetworkRequestSamplingRate_remoteConfigFetchFailed_returnsRCFailureDefault() {
1754+
when(mockRemoteConfigManager.getDouble(NETWORK_REQUEST_SAMPLING_RATE_FRC_KEY))
1755+
.thenReturn(Optional.absent());
1756+
when(mockDeviceCacheManager.getDouble(NETWORK_REQUEST_SAMPLING_RATE_CACHE_KEY))
1757+
.thenReturn(Optional.absent());
1758+
when(mockRemoteConfigManager.isLastFetchFailed()).thenReturn(true);
1759+
1760+
assertThat(testConfigResolver.getNetworkRequestSamplingRate()).isEqualTo(1.00 / 100);
1761+
}
1762+
17171763
@Test
17181764
public void getNetworkRequestSamplingRate_noRemoteConfigHasCache_returnsCache() {
17191765
when(mockRemoteConfigManager.getDouble(NETWORK_REQUEST_SAMPLING_RATE_FRC_KEY))
@@ -1935,6 +1981,17 @@ public void getSessionsSamplingRate_invalidRemoteConfig_returnCacheValue() {
19351981
verify(mockDeviceCacheManager, never()).setValue(any(), any());
19361982
}
19371983

1984+
@Test
1985+
public void getSessionsSamplingRate_remoteConfigFetchFailed_returnsRCFailureDefault() {
1986+
when(mockDeviceCacheManager.getDouble(SESSION_SAMPLING_RATE_CACHE_KEY))
1987+
.thenReturn(Optional.absent());
1988+
when(mockRemoteConfigManager.getDouble(SESSION_SAMPLING_RATE_FRC_KEY))
1989+
.thenReturn(Optional.absent());
1990+
when(mockRemoteConfigManager.isLastFetchFailed()).thenReturn(true);
1991+
1992+
assertThat(testConfigResolver.getSessionsSamplingRate()).isEqualTo(0.01 / 100);
1993+
}
1994+
19381995
@Test
19391996
public void getSessionsSamplingRate_validCache_returnCacheValue() {
19401997
when(mockDeviceCacheManager.getDouble(SESSION_SAMPLING_RATE_CACHE_KEY))

‎firebase-perf/src/test/java/com/google/firebase/perf/config/ConfigurationConstantsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public void getInstance_SessionsCpuCaptureFrequencyForegroundMs_validateConstant
9191
SessionsCpuCaptureFrequencyForegroundMs.getInstance();
9292

9393
assertThat(configFlag.getDefault()).isEqualTo(100L);
94+
assertThat(configFlag.getDefaultOnRcFetchFail()).isEqualTo(300L);
9495
assertThat(configFlag.getDeviceCacheFlag())
9596
.isEqualTo("com.google.firebase.perf.SessionsCpuCaptureFrequencyForegroundMs");
9697
assertThat(configFlag.getRemoteConfigFlag())
@@ -117,6 +118,7 @@ public void getInstance_SessionsMemoryCaptureFrequencyForegroundMs_validateConst
117118
SessionsMemoryCaptureFrequencyForegroundMs.getInstance();
118119

119120
assertThat(configFlag.getDefault()).isEqualTo(100L);
121+
assertThat(configFlag.getDefaultOnRcFetchFail()).isEqualTo(300L);
120122
assertThat(configFlag.getDeviceCacheFlag())
121123
.isEqualTo("com.google.firebase.perf.SessionsMemoryCaptureFrequencyForegroundMs");
122124
assertThat(configFlag.getRemoteConfigFlag())

‎firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,16 @@ public void isLastFetchFailed_frcIsNonNullAndStatusFailed_returnsTrue() {
812812
}
813813

814814
@Test
815-
public void isLastFetchFailed_frcIsNonNullAndStatusOtherThanFailed_returnsFalse() {
815+
public void isLastFetchFailed_frcIsNonNullAndStatusThrottled_returnsTrue() {
816+
RemoteConfigManager testRemoteConfigManager =
817+
setupTestRemoteConfigManager(createDefaultRcConfigMap());
818+
simulateFirebaseRemoteConfigLastFetchStatus(FirebaseRemoteConfig.LAST_FETCH_STATUS_THROTTLED);
819+
820+
assertThat(testRemoteConfigManager.isLastFetchFailed()).isTrue();
821+
}
822+
823+
@Test
824+
public void isLastFetchFailed_frcIsNonNullAndStatusOtherThanFailedOrThrottled_returnsFalse() {
816825
RemoteConfigManager testRemoteConfigManager =
817826
setupTestRemoteConfigManager(createDefaultRcConfigMap());
818827

@@ -822,9 +831,6 @@ public void isLastFetchFailed_frcIsNonNullAndStatusOtherThanFailed_returnsFalse(
822831

823832
simulateFirebaseRemoteConfigLastFetchStatus(FirebaseRemoteConfig.LAST_FETCH_STATUS_SUCCESS);
824833
assertThat(testRemoteConfigManager.isLastFetchFailed()).isFalse();
825-
826-
simulateFirebaseRemoteConfigLastFetchStatus(FirebaseRemoteConfig.LAST_FETCH_STATUS_THROTTLED);
827-
assertThat(testRemoteConfigManager.isLastFetchFailed()).isFalse();
828834
}
829835

830836
@Test

0 commit comments

Comments
 (0)