From 1ffcaaff57e87241c75fa4c987f2f24b2ee299d9 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Fri, 2 Sep 2022 16:44:48 -0700 Subject: [PATCH 1/9] log _experiment_as_ttid --- .../firebase/perf/config/ConfigResolver.java | 33 +++++++++ .../perf/config/ConfigurationConstants.java | 35 ++++++++++ .../firebase/perf/metrics/AppStartTrace.java | 69 +++++++++++++++++-- 3 files changed, 131 insertions(+), 6 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java index 44e5d35fe07..589ffdd5dfb 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java @@ -24,6 +24,7 @@ import com.google.firebase.perf.config.ConfigurationConstants.CollectionDeactivated; import com.google.firebase.perf.config.ConfigurationConstants.CollectionEnabled; import com.google.firebase.perf.config.ConfigurationConstants.FragmentSamplingRate; +import com.google.firebase.perf.config.ConfigurationConstants.ExperimentTTID; import com.google.firebase.perf.config.ConfigurationConstants.LogSourceName; import com.google.firebase.perf.config.ConfigurationConstants.NetworkEventCountBackground; import com.google.firebase.perf.config.ConfigurationConstants.NetworkEventCountForeground; @@ -767,6 +768,38 @@ public float getFragmentSamplingRate() { return config.getDefault(); } + /** Returns if _experiment_as_ttid should be captured. */ + public boolean getIsExperimentTTIDEnabled() { + // Order of precedence is: + // 1. If the value exists in Android Manifest, return this value. + // 2. If the value exists through Firebase Remote Config, cache and return this value. + // 3. If the value exists in device cache, return this value. + // 4. Otherwise, return default value. + ExperimentTTID config = ExperimentTTID.getInstance(); + + // 1. Reads value in Android Manifest (it is set by developers during build time). + Optional metadataValue = getMetadataBoolean(config); + if (metadataValue.isAvailable()) { + return metadataValue.get(); + } + + // 2. Reads value from Firebase Remote Config, saves this value in cache layer if valid. + Optional rcValue = getRemoteConfigBoolean(config); + if (rcValue.isAvailable()) { + deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get()); + return rcValue.get(); + } + + // 3. Reads value from cache layer. + Optional deviceCacheValue = getDeviceCacheBoolean(config); + if (deviceCacheValue.isAvailable()) { + return deviceCacheValue.get(); + } + + // 4. Returns default value if there is no valid value from above approaches. + return config.getDefault(); + } + // endregion // Helper functions for interaction with Metadata layer. diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationConstants.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationConstants.java index f14ba7c71a0..bcc7ac6b511 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationConstants.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigurationConstants.java @@ -661,4 +661,39 @@ protected String getMetadataFlag() { return "fragment_sampling_percentage"; } } + + protected static final class ExperimentTTID extends ConfigurationFlag { + private static ExperimentTTID instance; + + private ExperimentTTID() { + super(); + } + + protected static synchronized ExperimentTTID getInstance() { + if (instance == null) { + instance = new ExperimentTTID(); + } + return instance; + } + + @Override + protected Boolean getDefault() { + return false; + } + + @Override + protected String getRemoteConfigFlag() { + return "fpr_experiment_as_ttid"; + } + + @Override + protected String getDeviceCacheFlag() { + return "com.google.firebase.perf.ExperimentTTID"; + } + + @Override + protected String getMetadataFlag() { + return "experiment_as_ttid"; + } + } } diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index 3aa363e1338..db4bb1a3a5d 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -18,11 +18,16 @@ import android.app.Application; import android.app.Application.ActivityLifecycleCallbacks; import android.content.Context; +import android.os.Build; import android.os.Bundle; +import android.os.Process; +import android.view.View; + import androidx.annotation.Keep; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.google.android.gms.common.util.VisibleForTesting; +import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.logging.AndroidLogger; import com.google.firebase.perf.provider.FirebasePerfProvider; import com.google.firebase.perf.session.PerfSession; @@ -30,11 +35,14 @@ import com.google.firebase.perf.transport.TransportManager; import com.google.firebase.perf.util.Clock; import com.google.firebase.perf.util.Constants; +import com.google.firebase.perf.util.FirstDrawDoneListener; import com.google.firebase.perf.util.Timer; import com.google.firebase.perf.v1.ApplicationProcessState; import com.google.firebase.perf.v1.TraceMetric; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; @@ -89,6 +97,7 @@ public class AppStartTrace implements ActivityLifecycleCallbacks { private Timer onCreateTime = null; private Timer onStartTime = null; private Timer onResumeTime = null; + private Timer firstDrawDone = null; private PerfSession startSession; private boolean isStartedFromBackground = false; @@ -139,7 +148,7 @@ static AppStartTrace getInstance(TransportManager transportManager, Clock clock) MAX_POOL_SIZE, /* keepAliveTime= */ MAX_LATENCY_BEFORE_UI_INIT + 10, TimeUnit.SECONDS, - new LinkedBlockingQueue<>(1))); + new LinkedBlockingQueue<>())); } } } @@ -178,6 +187,32 @@ public synchronized void unregisterActivityLifecycleCallbacks() { isRegisteredForLifecycleCallbacks = false; } + /** + * Gets the timetamp that marks the beginning of app start, currently defined as the beginning of + * BIND_APPLICATION. Fallback to class-load time of {@link FirebasePerfProvider} when API < 24. + * + * @return {@link Timer} at the beginning of app start by Fireperf definition. + */ + private static Timer getStartTimer() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + return Timer.ofElapsedRealtime(Process.getStartElapsedRealtime()); + } + return FirebasePerfProvider.getAppStartTime(); + } + + private void recordFirstDrawDone() { + if (firstDrawDone != null) { + return; + } + this.firstDrawDone = clock.getTime(); + executorService.execute(() -> this.logColdStart(getStartTimer(), this.firstDrawDone, this.startSession)); + + if (isRegisteredForLifecycleCallbacks) { + // After AppStart trace is queued to be logged, we can unregister this callback. + unregisterActivityLifecycleCallbacks(); + } + } + @Override public synchronized void onActivityCreated(Activity activity, Bundle savedInstanceState) { if (isStartedFromBackground || onCreateTime != null // An activity already called onCreate() @@ -207,11 +242,20 @@ public synchronized void onActivityStarted(Activity activity) { @Override public synchronized void onActivityResumed(Activity activity) { if (isStartedFromBackground - || onResumeTime != null // An activity already called onResume() || isTooLateToInitUI) { return; } + // Shadow-launch experiment of new app start time + if (ConfigResolver.getInstance().getIsExperimentTTIDEnabled()) { + View rootView = activity.findViewById(android.R.id.content); + FirstDrawDoneListener.registerForNextDraw(rootView, this::recordFirstDrawDone); + } + + if (onResumeTime != null) { // An activity already called onResume() + return; + } + appStartActivity = new WeakReference(activity); onResumeTime = clock.getTime(); @@ -227,11 +271,24 @@ public synchronized void onActivityResumed(Activity activity) { // Log the app start trace in a non-main thread. executorService.execute(this::logAppStartTrace); + } - if (isRegisteredForLifecycleCallbacks) { - // After AppStart trace is logged, we can unregister this callback. - unregisterActivityLifecycleCallbacks(); - } + private void logColdStart(Timer start, Timer end, PerfSession session) { + TraceMetric.Builder metric = + TraceMetric.newBuilder() + .setName("_experiment_as_ttid") + .setClientStartTimeUs(start.getMicros()) + .setDurationUs(start.getDurationMicros(end)); + + TraceMetric.Builder subtrace = + TraceMetric.newBuilder() + .setName("_experiment_classLoadTime") + .setClientStartTimeUs(FirebasePerfProvider.getAppStartTime().getMicros()) + .setDurationUs(FirebasePerfProvider.getAppStartTime().getDurationMicros(end)); + + metric.addSubtraces(subtrace).addPerfSessions(this.startSession.build()); + + transportManager.log(metric.build(), ApplicationProcessState.FOREGROUND_BACKGROUND); } private void logAppStartTrace() { From ddb9f8cd9f9e682c816ee98f6f0b255899544c11 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Wed, 7 Sep 2022 10:45:16 -0700 Subject: [PATCH 2/9] send event and RRC mitigation --- .../firebase/perf/config/ConfigResolver.java | 22 +++++++++++-------- .../firebase/perf/metrics/AppStartTrace.java | 17 ++++++-------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java index 589ffdd5dfb..89e149ba83a 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java @@ -23,8 +23,8 @@ import com.google.firebase.perf.BuildConfig; import com.google.firebase.perf.config.ConfigurationConstants.CollectionDeactivated; import com.google.firebase.perf.config.ConfigurationConstants.CollectionEnabled; -import com.google.firebase.perf.config.ConfigurationConstants.FragmentSamplingRate; import com.google.firebase.perf.config.ConfigurationConstants.ExperimentTTID; +import com.google.firebase.perf.config.ConfigurationConstants.FragmentSamplingRate; import com.google.firebase.perf.config.ConfigurationConstants.LogSourceName; import com.google.firebase.perf.config.ConfigurationConstants.NetworkEventCountBackground; import com.google.firebase.perf.config.ConfigurationConstants.NetworkEventCountForeground; @@ -115,11 +115,21 @@ public void setMetadataBundle(ImmutableBundle bundle) { /** Default API to call for whether performance monitoring is currently silent. */ public boolean isPerformanceMonitoringEnabled() { + saveExperimentFlagToDeviceCache(); Boolean isPerformanceCollectionEnabled = getIsPerformanceCollectionEnabled(); return (isPerformanceCollectionEnabled == null || isPerformanceCollectionEnabled == true) && getIsServiceCollectionEnabled(); } + // TODO: remove after _experiment_as_ttid is finished + private void saveExperimentFlagToDeviceCache() { + ExperimentTTID config = ExperimentTTID.getInstance(); + Optional rcValue = getRemoteConfigBoolean(config); + if (rcValue.isAvailable()) { + deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get()); + } + } + /** Returns whether developers have enabled Firebase Performance event collection. */ @Nullable public Boolean getIsPerformanceCollectionEnabled() { @@ -772,7 +782,7 @@ public float getFragmentSamplingRate() { public boolean getIsExperimentTTIDEnabled() { // Order of precedence is: // 1. If the value exists in Android Manifest, return this value. - // 2. If the value exists through Firebase Remote Config, cache and return this value. + // 2. Cannot read value from RC because it's not initalized yet // 3. If the value exists in device cache, return this value. // 4. Otherwise, return default value. ExperimentTTID config = ExperimentTTID.getInstance(); @@ -783,13 +793,7 @@ public boolean getIsExperimentTTIDEnabled() { return metadataValue.get(); } - // 2. Reads value from Firebase Remote Config, saves this value in cache layer if valid. - Optional rcValue = getRemoteConfigBoolean(config); - if (rcValue.isAvailable()) { - deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get()); - return rcValue.get(); - } - + // 2. Cannot read value from RC because it's not initialized yet. // 3. Reads value from cache layer. Optional deviceCacheValue = getDeviceCacheBoolean(config); if (deviceCacheValue.isAvailable()) { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index db4bb1a3a5d..16b4b1d7ae9 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -22,7 +22,6 @@ import android.os.Bundle; import android.os.Process; import android.view.View; - import androidx.annotation.Keep; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -41,8 +40,6 @@ import com.google.firebase.perf.v1.TraceMetric; import java.lang.ref.WeakReference; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; @@ -205,7 +202,8 @@ private void recordFirstDrawDone() { return; } this.firstDrawDone = clock.getTime(); - executorService.execute(() -> this.logColdStart(getStartTimer(), this.firstDrawDone, this.startSession)); + executorService.execute( + () -> this.logColdStart(getStartTimer(), this.firstDrawDone, this.startSession)); if (isRegisteredForLifecycleCallbacks) { // After AppStart trace is queued to be logged, we can unregister this callback. @@ -241,8 +239,7 @@ public synchronized void onActivityStarted(Activity activity) { @Override public synchronized void onActivityResumed(Activity activity) { - if (isStartedFromBackground - || isTooLateToInitUI) { + if (isStartedFromBackground || isTooLateToInitUI) { return; } @@ -281,10 +278,10 @@ private void logColdStart(Timer start, Timer end, PerfSession session) { .setDurationUs(start.getDurationMicros(end)); TraceMetric.Builder subtrace = - TraceMetric.newBuilder() - .setName("_experiment_classLoadTime") - .setClientStartTimeUs(FirebasePerfProvider.getAppStartTime().getMicros()) - .setDurationUs(FirebasePerfProvider.getAppStartTime().getDurationMicros(end)); + TraceMetric.newBuilder() + .setName("_experiment_classLoadTime") + .setClientStartTimeUs(FirebasePerfProvider.getAppStartTime().getMicros()) + .setDurationUs(FirebasePerfProvider.getAppStartTime().getDurationMicros(end)); metric.addSubtraces(subtrace).addPerfSessions(this.startSession.build()); From f8b4b4dd8d5d69b675331f8ec1c6d685c5104bee Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Wed, 14 Sep 2022 17:01:54 -0400 Subject: [PATCH 3/9] add RC wip --- firebase-perf/dev-app/dev-app.gradle | 1 + .../firebase/perf/metrics/AppStartTrace.java | 6 +- .../perf/metrics/AppStartTraceTest.java | 60 +++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/firebase-perf/dev-app/dev-app.gradle b/firebase-perf/dev-app/dev-app.gradle index 8f50c4b4a94..fc9a2fcf478 100644 --- a/firebase-perf/dev-app/dev-app.gradle +++ b/firebase-perf/dev-app/dev-app.gradle @@ -16,6 +16,7 @@ apply plugin: 'com.android.application' // Firebase performance plugin, the relevant dependency is specified in the "buildSrc/build-src.gradle" apply plugin: 'com.google.firebase.firebase-perf' apply plugin: com.google.firebase.gradle.plugins.ci.device.FirebaseTestLabPlugin +apply plugin: 'com.google.gms.google-services' firebaseTestLab { device 'model=Pixel2,version=28,locale=en,orientation=portrait' diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java index 16b4b1d7ae9..89f0ac78ad2 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java @@ -74,6 +74,7 @@ public class AppStartTrace implements ActivityLifecycleCallbacks { private boolean isRegisteredForLifecycleCallbacks = false; private final TransportManager transportManager; private final Clock clock; + private final ConfigResolver configResolver; private Context appContext; /** * The first time onCreate() of any activity is called, the activity is saved as launchActivity. @@ -140,6 +141,7 @@ static AppStartTrace getInstance(TransportManager transportManager, Clock clock) new AppStartTrace( transportManager, clock, + ConfigResolver.getInstance(), new ThreadPoolExecutor( CORE_POOL_SIZE, MAX_POOL_SIZE, @@ -155,9 +157,11 @@ static AppStartTrace getInstance(TransportManager transportManager, Clock clock) AppStartTrace( @NonNull TransportManager transportManager, @NonNull Clock clock, + @NonNull ConfigResolver configResolver, @NonNull ExecutorService executorService) { this.transportManager = transportManager; this.clock = clock; + this.configResolver = configResolver; this.executorService = executorService; } @@ -244,7 +248,7 @@ public synchronized void onActivityResumed(Activity activity) { } // Shadow-launch experiment of new app start time - if (ConfigResolver.getInstance().getIsExperimentTTIDEnabled()) { + if (configResolver.getIsExperimentTTIDEnabled()) { View rootView = activity.findViewById(android.R.id.content); FirstDrawDoneListener.registerForNextDraw(rootView, this::recordFirstDrawDone); } diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java index c1f975754e5..6e92eade05f 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/metrics/AppStartTraceTest.java @@ -14,19 +14,27 @@ package com.google.firebase.perf.metrics; +import static com.google.common.truth.Truth.assertThat; import static com.google.firebase.perf.util.TimerTest.getElapsedRealtimeMicros; +import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; +import static org.robolectric.Shadows.shadowOf; import android.app.Activity; import android.content.pm.ProviderInfo; import android.os.Bundle; +import android.os.Looper; +import android.os.Process; +import android.os.SystemClock; +import android.view.View; import androidx.test.core.app.ApplicationProvider; import com.google.firebase.perf.FirebasePerformanceTestBase; +import com.google.firebase.perf.config.ConfigResolver; import com.google.firebase.perf.provider.FirebasePerfProvider; import com.google.firebase.perf.session.SessionManager; import com.google.firebase.perf.transport.TransportManager; @@ -36,6 +44,7 @@ import com.google.firebase.perf.v1.ApplicationProcessState; import com.google.firebase.perf.v1.TraceMetric; import com.google.testing.timing.FakeScheduledExecutorService; +import java.time.Duration; import java.util.concurrent.TimeUnit; import org.junit.After; import org.junit.Assert; @@ -48,13 +57,18 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.LooperMode; +import org.robolectric.shadows.ShadowSystemClock; /** Unit tests for {@link AppStartTrace}. */ @RunWith(RobolectricTestRunner.class) +@LooperMode(LooperMode.Mode.PAUSED) public class AppStartTraceTest extends FirebasePerformanceTestBase { @Mock private Clock clock; @Mock private TransportManager transportManager; + @Mock private ConfigResolver configResolver; @Mock private Activity activity1; @Mock private Activity activity2; @Mock private Bundle bundle; @@ -94,7 +108,8 @@ public void reset() { @Test public void testLaunchActivity() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - AppStartTrace trace = new AppStartTrace(transportManager, clock, fakeExecutorService); + AppStartTrace trace = + new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); // first activity goes through onCreate()->onStart()->onResume() state change. currentTime = 1; trace.onActivityCreated(activity1, bundle); @@ -171,7 +186,8 @@ private void verifyFinalState( @Test public void testInterleavedActivity() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - AppStartTrace trace = new AppStartTrace(transportManager, clock, fakeExecutorService); + AppStartTrace trace = + new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); // first activity onCreate() currentTime = 1; trace.onActivityCreated(activity1, bundle); @@ -206,7 +222,8 @@ public void testInterleavedActivity() { @Test public void testDelayedAppStart() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - AppStartTrace trace = new AppStartTrace(transportManager, clock, fakeExecutorService); + AppStartTrace trace = + new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); // Delays activity creation after 1 minute from app start time. currentTime = appStart.getMicros() + TimeUnit.MINUTES.toMicros(1) + 1; trace.onActivityCreated(activity1, bundle); @@ -227,7 +244,8 @@ public void testDelayedAppStart() { @Test public void testStartFromBackground() { FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); - AppStartTrace trace = new AppStartTrace(transportManager, clock, fakeExecutorService); + AppStartTrace trace = + new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); trace.setIsStartFromBackground(); trace.onActivityCreated(activity1, bundle); Assert.assertNull(trace.getOnCreateTime()); @@ -261,4 +279,38 @@ public void testFirebasePerfProviderOnAttachInfo_initializesGaugeCollection() { Assert.assertEquals(oldSessionId, SessionManager.getInstance().perfSession().sessionId()); verify(mockPerfSession, times(2)).isGaugeAndEventCollectionEnabled(); } + + @Test + @Config(sdk = 26) + public void timeToInitialDisplay_isLogged() { + when(clock.getTime()).thenCallRealMethod(); // Use robolectric shadows to manipulate time + long appStartTime = TimeUnit.MILLISECONDS.toMicros(Process.getStartElapsedRealtime()); + View testView = new View(ApplicationProvider.getApplicationContext()); + when(activity1.findViewById(android.R.id.content)).thenReturn(testView); + when(configResolver.getIsExperimentTTIDEnabled()).thenReturn(true); + FakeScheduledExecutorService fakeExecutorService = new FakeScheduledExecutorService(); + AppStartTrace trace = + new AppStartTrace(transportManager, clock, configResolver, fakeExecutorService); + + ShadowSystemClock.advanceBy(Duration.ofMillis(1000)); + long resumeTime = TimeUnit.NANOSECONDS.toMicros(SystemClock.elapsedRealtimeNanos()); + trace.onActivityCreated(activity1, bundle); + trace.onActivityStarted(activity1); + trace.onActivityResumed(activity1); + fakeExecutorService.runAll(); + verify(transportManager, times(1)) + .log(isA(TraceMetric.class), isA(ApplicationProcessState.class)); + ShadowSystemClock.advanceBy(Duration.ofMillis(1000)); + long drawTime = TimeUnit.NANOSECONDS.toMicros(SystemClock.elapsedRealtimeNanos()); + testView.getViewTreeObserver().dispatchOnDraw(); + shadowOf(Looper.getMainLooper()).idle(); + fakeExecutorService.runNext(); + verify(transportManager, times(2)) + .log(traceArgumentCaptor.capture(), isA(ApplicationProcessState.class)); + + TraceMetric ttid = traceArgumentCaptor.getValue(); + assertThat(ttid.getName()).isEqualTo("_experiment_as_ttid"); + assertThat(ttid.getDurationUs()).isNotEqualTo(resumeTime - appStartTime); + assertThat(ttid.getDurationUs()).isEqualTo(drawTime - appStartTime); + } } From d1de285bf31fd79c7bed36881f4609ab74540e6d Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Fri, 16 Sep 2022 17:32:39 -0400 Subject: [PATCH 4/9] modified save to cache when RC fetches --- .../firebase/perf/config/ConfigResolver.java | 10 ------- .../perf/config/RemoteConfigManager.java | 30 +++++++++++++------ .../perf/config/RemoteConfigManagerTest.java | 8 +++-- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java index 89e149ba83a..e51a792da78 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java @@ -115,21 +115,11 @@ public void setMetadataBundle(ImmutableBundle bundle) { /** Default API to call for whether performance monitoring is currently silent. */ public boolean isPerformanceMonitoringEnabled() { - saveExperimentFlagToDeviceCache(); Boolean isPerformanceCollectionEnabled = getIsPerformanceCollectionEnabled(); return (isPerformanceCollectionEnabled == null || isPerformanceCollectionEnabled == true) && getIsServiceCollectionEnabled(); } - // TODO: remove after _experiment_as_ttid is finished - private void saveExperimentFlagToDeviceCache() { - ExperimentTTID config = ExperimentTTID.getInstance(); - Optional rcValue = getRemoteConfigBoolean(config); - if (rcValue.isAvailable()) { - deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get()); - } - } - /** Returns whether developers have enabled Firebase Performance event collection. */ @Nullable public Boolean getIsPerformanceCollectionEnabled() { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java index 0986e123c16..237eca959ee 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java @@ -14,6 +14,8 @@ package com.google.firebase.perf.config; +import static com.google.firebase.perf.config.ConfigurationConstants.ExperimentTTID; + import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager.NameNotFoundException; @@ -53,6 +55,7 @@ public class RemoteConfigManager { private static final long MIN_APP_START_CONFIG_FETCH_DELAY_MS = 5000; private static final int RANDOM_APP_START_CONFIG_FETCH_DELAY_MS = 25000; + private final DeviceCacheManager cache; private final ConcurrentHashMap allRcConfigMap; private final Executor executor; private final long appStartTimeInMs; @@ -65,29 +68,25 @@ public class RemoteConfigManager { private RemoteConfigManager() { this( + DeviceCacheManager.getInstance(), new ThreadPoolExecutor( /* corePoolSize= */ 0, /* maximumPoolSize= */ 1, /* keepAliveTime= */ 0L, TimeUnit.SECONDS, new LinkedBlockingQueue()), - /* firebaseRemoteConfig= */ null // set once FirebaseRemoteConfig is initialized - ); - } - - RemoteConfigManager(Executor executor, FirebaseRemoteConfig firebaseRemoteConfig) { - this( - executor, - firebaseRemoteConfig, + /* firebaseRemoteConfig= */ null, // set once FirebaseRemoteConfig is initialized MIN_APP_START_CONFIG_FETCH_DELAY_MS + new Random().nextInt(RANDOM_APP_START_CONFIG_FETCH_DELAY_MS)); } @VisibleForTesting RemoteConfigManager( + DeviceCacheManager cache, Executor executor, FirebaseRemoteConfig firebaseRemoteConfig, long appStartConfigFetchDelayInMs) { + this.cache = cache; this.executor = executor; this.firebaseRemoteConfig = firebaseRemoteConfig; this.allRcConfigMap = @@ -315,7 +314,7 @@ private void triggerRemoteConfigFetchIfNecessary() { return; } if (allRcConfigMap.isEmpty()) { // Initial fetch. - syncConfigValues(firebaseRemoteConfig.getAll()); + allRcConfigMap.putAll(firebaseRemoteConfig.getAll()); } if (shouldFetchAndActivateRemoteConfigValues()) { triggerFirebaseRemoteConfigFetchAndActivateOnSuccessfulFetch(); @@ -342,6 +341,19 @@ protected void syncConfigValues(Map newlyFetc allRcConfigMap.remove(existingKey); } } + + // Save to device cache upon successful RC fetchAndActivate + ExperimentTTID flag = ExperimentTTID.getInstance(); + FirebaseRemoteConfigValue rcValue = allRcConfigMap.get(flag.getRemoteConfigFlag()); + if (rcValue != null) { + try { + cache.setValue(flag.getDeviceCacheFlag(), rcValue.asBoolean()); + } catch (Exception exception) { + logger.debug("ExperimentTTID remote config flag has invalid value, expected boolean."); + } + } else { + logger.debug("ExperimentTTID remote config flag does not exist."); + } } @VisibleForTesting diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java index 9855d3cfe62..e06187fa9f0 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java @@ -52,6 +52,7 @@ public final class RemoteConfigManagerTest extends FirebasePerformanceTestBase { @Mock private FirebaseRemoteConfig mockFirebaseRemoteConfig; @Mock private RemoteConfigComponent mockFirebaseRemoteConfigComponent; @Mock private Provider mockFirebaseRemoteConfigProvider; + @Mock private DeviceCacheManager mockCacheManager; private FakeScheduledExecutorService fakeExecutor; @@ -889,10 +890,13 @@ private RemoteConfigManager setupTestRemoteConfigManager( when(mockFirebaseRemoteConfig.getAll()).thenReturn(configs); if (initializeFrc) { return new RemoteConfigManager( - fakeExecutor, mockFirebaseRemoteConfig, appStartConfigFetchDelayInMs); + mockCacheManager, fakeExecutor, mockFirebaseRemoteConfig, appStartConfigFetchDelayInMs); } else { return new RemoteConfigManager( - fakeExecutor, /* firebaseRemoteConfig= */ null, appStartConfigFetchDelayInMs); + mockCacheManager, + fakeExecutor, + /* firebaseRemoteConfig= */ null, + appStartConfigFetchDelayInMs); } } From 2c36e1fcb2a6ab74f5c18556219804a335a55340 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Fri, 16 Sep 2022 17:40:58 -0400 Subject: [PATCH 5/9] dev-app manifest override --- firebase-perf/dev-app/dev-app.gradle | 1 - firebase-perf/dev-app/src/main/AndroidManifest.xml | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/firebase-perf/dev-app/dev-app.gradle b/firebase-perf/dev-app/dev-app.gradle index fc9a2fcf478..8f50c4b4a94 100644 --- a/firebase-perf/dev-app/dev-app.gradle +++ b/firebase-perf/dev-app/dev-app.gradle @@ -16,7 +16,6 @@ apply plugin: 'com.android.application' // Firebase performance plugin, the relevant dependency is specified in the "buildSrc/build-src.gradle" apply plugin: 'com.google.firebase.firebase-perf' apply plugin: com.google.firebase.gradle.plugins.ci.device.FirebaseTestLabPlugin -apply plugin: 'com.google.gms.google-services' firebaseTestLab { device 'model=Pixel2,version=28,locale=en,orientation=portrait' diff --git a/firebase-perf/dev-app/src/main/AndroidManifest.xml b/firebase-perf/dev-app/src/main/AndroidManifest.xml index 643b50fb03e..9b68574b890 100644 --- a/firebase-perf/dev-app/src/main/AndroidManifest.xml +++ b/firebase-perf/dev-app/src/main/AndroidManifest.xml @@ -38,6 +38,9 @@ + Date: Tue, 20 Sep 2022 14:38:14 -0400 Subject: [PATCH 6/9] unit test for RCc cache saving --- .../perf/config/RemoteConfigManagerTest.java | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java index e06187fa9f0..3f1289a06d9 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java @@ -25,6 +25,7 @@ import androidx.annotation.NonNull; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; +import com.google.common.util.concurrent.MoreExecutors; import com.google.firebase.inject.Provider; import com.google.firebase.perf.FirebasePerformanceTestBase; import com.google.firebase.perf.provider.FirebasePerfProvider; @@ -36,6 +37,7 @@ import com.google.testing.timing.FakeScheduledExecutorService; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; @@ -48,12 +50,17 @@ public final class RemoteConfigManagerTest extends FirebasePerformanceTestBase { private static final String FIREPERF_FRC_NAMESPACE_NAME = "fireperf"; + private static final FirebaseRemoteConfigValue TRUE_VALUE = + new RemoteConfigValueImplForTest("true"); + private static final FirebaseRemoteConfigValue FALSE_VALUE = + new RemoteConfigValueImplForTest("false"); @Mock private FirebaseRemoteConfig mockFirebaseRemoteConfig; @Mock private RemoteConfigComponent mockFirebaseRemoteConfigComponent; @Mock private Provider mockFirebaseRemoteConfigProvider; - @Mock private DeviceCacheManager mockCacheManager; + private DeviceCacheManager cacheManager; + private ExecutorService directExecutor; private FakeScheduledExecutorService fakeExecutor; @Before @@ -61,6 +68,8 @@ public void setUp() { initMocks(this); fakeExecutor = new FakeScheduledExecutorService(); + directExecutor = MoreExecutors.newDirectExecutorService(); + cacheManager = new DeviceCacheManager(directExecutor); when(mockFirebaseRemoteConfigProvider.get()).thenReturn(mockFirebaseRemoteConfigComponent); when(mockFirebaseRemoteConfigComponent.get(FIREPERF_FRC_NAMESPACE_NAME)) @@ -231,6 +240,29 @@ public void getString_validFrcValue_updatesWithNewValue() { assertThat(testRemoteConfigManager.getString("some_key3").isAvailable()).isFalse(); } + @Test + public void syncConfigValues_savesNewlyFetchedValueToDeviceCache() { + Map configs = new HashMap<>(); + ConfigurationConstants.ExperimentTTID flag = + ConfigurationConstants.ExperimentTTID.getInstance(); + configs.put(flag.getRemoteConfigFlag(), TRUE_VALUE); + RemoteConfigManager testRemoteConfigManager = setupTestRemoteConfigManager(configs); + + assertThat(cacheManager.getBoolean(flag.getDeviceCacheFlag()).isAvailable()).isFalse(); + + configs.put(flag.getRemoteConfigFlag(), FALSE_VALUE); + testRemoteConfigManager.syncConfigValues(configs); + + assertThat(cacheManager.getBoolean(flag.getDeviceCacheFlag()).isAvailable()).isTrue(); + assertThat(cacheManager.getBoolean(flag.getDeviceCacheFlag()).get()).isFalse(); + + configs.put(flag.getRemoteConfigFlag(), TRUE_VALUE); + testRemoteConfigManager.syncConfigValues(configs); + + assertThat(cacheManager.getBoolean(flag.getDeviceCacheFlag()).isAvailable()).isTrue(); + assertThat(cacheManager.getBoolean(flag.getDeviceCacheFlag()).get()).isTrue(); + } + @Test public void getRemoteConfigValueOrDefaultLong_invalidFrcValue_returnsDefaultValue() { Map configs = createDefaultRcConfigMap(); @@ -890,10 +922,10 @@ private RemoteConfigManager setupTestRemoteConfigManager( when(mockFirebaseRemoteConfig.getAll()).thenReturn(configs); if (initializeFrc) { return new RemoteConfigManager( - mockCacheManager, fakeExecutor, mockFirebaseRemoteConfig, appStartConfigFetchDelayInMs); + cacheManager, fakeExecutor, mockFirebaseRemoteConfig, appStartConfigFetchDelayInMs); } else { return new RemoteConfigManager( - mockCacheManager, + cacheManager, fakeExecutor, /* firebaseRemoteConfig= */ null, appStartConfigFetchDelayInMs); From e3d7a6551008fb77206ce945a37f1481d53f3b04 Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Thu, 22 Sep 2022 12:39:20 -0400 Subject: [PATCH 7/9] better name and comments --- firebase-perf/dev-app/src/main/AndroidManifest.xml | 2 +- .../google/firebase/perf/config/ConfigResolver.java | 2 +- .../firebase/perf/config/ConfigurationConstants.java | 4 ++-- .../google/firebase/perf/metrics/AppStartTrace.java | 10 ++++++++-- .../firebase/perf/metrics/AppStartTraceTest.java | 9 +++++++-- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/firebase-perf/dev-app/src/main/AndroidManifest.xml b/firebase-perf/dev-app/src/main/AndroidManifest.xml index 9b68574b890..c225f4fb472 100644 --- a/firebase-perf/dev-app/src/main/AndroidManifest.xml +++ b/firebase-perf/dev-app/src/main/AndroidManifest.xml @@ -39,7 +39,7 @@ android:name="fragment_sampling_percentage" android:value="100.0" /> Date: Thu, 22 Sep 2022 12:55:36 -0400 Subject: [PATCH 8/9] better formatting remoteconfigmanagertest --- .../firebase/perf/config/RemoteConfigManagerTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java index 3f1289a06d9..af1b971d9a1 100644 --- a/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java +++ b/firebase-perf/src/test/java/com/google/firebase/perf/config/RemoteConfigManagerTest.java @@ -37,7 +37,6 @@ import com.google.testing.timing.FakeScheduledExecutorService; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; @@ -60,7 +59,6 @@ public final class RemoteConfigManagerTest extends FirebasePerformanceTestBase { @Mock private Provider mockFirebaseRemoteConfigProvider; private DeviceCacheManager cacheManager; - private ExecutorService directExecutor; private FakeScheduledExecutorService fakeExecutor; @Before @@ -68,8 +66,8 @@ public void setUp() { initMocks(this); fakeExecutor = new FakeScheduledExecutorService(); - directExecutor = MoreExecutors.newDirectExecutorService(); - cacheManager = new DeviceCacheManager(directExecutor); + // DeviceCacheManager initialization requires immediate blocking task execution in its executor + cacheManager = new DeviceCacheManager(MoreExecutors.newDirectExecutorService()); when(mockFirebaseRemoteConfigProvider.get()).thenReturn(mockFirebaseRemoteConfigComponent); when(mockFirebaseRemoteConfigComponent.get(FIREPERF_FRC_NAMESPACE_NAME)) From 0ae206b9bf3cfbe6767ba2970677398e57c4be4d Mon Sep 17 00:00:00 2001 From: Leo Zhan Date: Tue, 27 Sep 2022 15:44:21 -0400 Subject: [PATCH 9/9] better comments and added local RC lookup back --- .../google/firebase/perf/config/ConfigResolver.java | 10 ++++++++-- .../firebase/perf/config/RemoteConfigManager.java | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java index 57f40fe51a5..65e527e373c 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java @@ -772,7 +772,7 @@ public float getFragmentSamplingRate() { public boolean getIsExperimentTTIDEnabled() { // Order of precedence is: // 1. If the value exists in Android Manifest, return this value. - // 2. Cannot read value from RC because it's not initalized yet + // 2. If the value exists through Firebase Remote Config, cache and return this value. // 3. If the value exists in device cache, return this value. // 4. Otherwise, return default value. ExperimentTTID config = ExperimentTTID.getInstance(); @@ -783,7 +783,13 @@ public boolean getIsExperimentTTIDEnabled() { return metadataValue.get(); } - // 2. Cannot read value from RC because it's not initialized yet. + // 2. Reads value from Firebase Remote Config, saves this value in cache layer if valid. + Optional rcValue = getRemoteConfigBoolean(config); + if (rcValue.isAvailable()) { + deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get()); + return rcValue.get(); + } + // 3. Reads value from cache layer. Optional deviceCacheValue = getDeviceCacheBoolean(config); if (deviceCacheValue.isAvailable()) { diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java index 237eca959ee..7c27b59c3d9 100644 --- a/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java +++ b/firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java @@ -342,7 +342,9 @@ protected void syncConfigValues(Map newlyFetc } } - // Save to device cache upon successful RC fetchAndActivate + // TODO: remove after experiment is over and experiment RC flag is no longer needed + // Save ExperimentTTID flag to device cache upon successful RC fetchAndActivate, because reading + // is done quite early and it is possible that RC isn't initialized yet. ExperimentTTID flag = ExperimentTTID.getInstance(); FirebaseRemoteConfigValue rcValue = allRcConfigMap.get(flag.getRemoteConfigFlag()); if (rcValue != null) {