diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 49381b187..381b38ffb 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -101,6 +101,7 @@ import org.mozilla.fenix.perf.PerformanceInflater import org.mozilla.fenix.perf.ProfilerMarkers import org.mozilla.fenix.perf.StartupPathProvider import org.mozilla.fenix.perf.StartupTimeline +import org.mozilla.fenix.perf.StartupTypeTelemetry import org.mozilla.fenix.search.SearchDialogFragmentDirections import org.mozilla.fenix.session.PrivateNotificationService import org.mozilla.fenix.settings.SettingsFragmentDirections @@ -173,6 +174,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { private var actionMode: ActionMode? = null private val startupPathProvider = StartupPathProvider() + private lateinit var startupTypeTelemetry: StartupTypeTelemetry final override fun onCreate(savedInstanceState: Bundle?): Unit = PerfStartup.homeActivityOnCreate.measureNoInline { // DO NOT MOVE ANYTHING ABOVE THIS addMarker CALL. @@ -266,6 +268,9 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { startupTelemetryOnCreateCalled(intent.toSafeIntent(), savedInstanceState != null) startupPathProvider.attachOnActivityOnCreate(lifecycle, intent) + startupTypeTelemetry = StartupTypeTelemetry(components.startupStateProvider, startupPathProvider).apply { + attachOnHomeActivityOnCreate(lifecycle) + } components.core.requestInterceptor.setNavigationController(navHost.navController) diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupTypeTelemetry.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupTypeTelemetry.kt new file mode 100644 index 000000000..07b07855a --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupTypeTelemetry.kt @@ -0,0 +1,96 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import androidx.annotation.VisibleForTesting +import androidx.annotation.VisibleForTesting.NONE +import androidx.annotation.VisibleForTesting.PRIVATE +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import mozilla.components.support.base.log.logger.Logger +import org.mozilla.fenix.GleanMetrics.PerfStartup +import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.perf.StartupPathProvider.StartupPath +import org.mozilla.fenix.perf.StartupStateProvider.StartupState + +private val activityClass = HomeActivity::class.java + +private val logger = Logger("StartupTypeTelemetry") + +/** + * Records telemetry for the number of start ups. See the + * [Fenix perf glossary](https://wiki.mozilla.org/index.php?title=Performance/Fenix/Glossary) + * for specific definitions. + * + * This should be a member variable of [HomeActivity] because its data is tied to the lifecycle of an + * Activity. Call [attachOnHomeActivityOnCreate] for this class to work correctly. + * + * N.B.: this class is lightly hardcoded to HomeActivity. + */ +class StartupTypeTelemetry( + private val startupStateProvider: StartupStateProvider, + private val startupPathProvider: StartupPathProvider +) { + + fun attachOnHomeActivityOnCreate(lifecycle: Lifecycle) { + lifecycle.addObserver(StartupTypeLifecycleObserver()) + } + + private fun getTelemetryLabel(startupState: StartupState, startupPath: StartupPath): String { + // We don't use the enum name directly to avoid unintentional changes when refactoring. + val stateLabel = when (startupState) { + StartupState.COLD -> "cold" + StartupState.WARM -> "warm" + StartupState.HOT -> "hot" + StartupState.UNKNOWN -> "unknown" + } + + val pathLabel = when (startupPath) { + StartupPath.MAIN -> "main" + StartupPath.VIEW -> "view" + + // To avoid combinatorial explosion in label names, we bucket NOT_SET into UNKNOWN. + StartupPath.NOT_SET, + StartupPath.UNKNOWN -> "unknown" + } + + return "${stateLabel}_$pathLabel" + } + + @VisibleForTesting(otherwise = NONE) + fun getTestCallbacks() = StartupTypeLifecycleObserver() + + @VisibleForTesting(otherwise = PRIVATE) + fun record() { + val startupState = startupStateProvider.getStartupStateForStartedActivity(activityClass) + val startupPath = startupPathProvider.startupPathForActivity + val label = getTelemetryLabel(startupState, startupPath) + + PerfStartup.startupType[label].add(1) + logger.info("Recorded start up: $label") + } + + @VisibleForTesting(otherwise = PRIVATE) + inner class StartupTypeLifecycleObserver : DefaultLifecycleObserver { + private var shouldRecordStart = false + + override fun onStart(owner: LifecycleOwner) { + shouldRecordStart = true + } + + override fun onResume(owner: LifecycleOwner) { + // We must record in onResume because the StartupStateProvider can only be called for + // STARTED activities and we can't guarantee our onStart is called before its. + // + // We only record if start was called for this resume to avoid recording + // for onPause -> onResume states. + if (shouldRecordStart) { + record() + shouldRecordStart = false + } + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupTypeTelemetryTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupTypeTelemetryTest.kt new file mode 100644 index 000000000..68a92875e --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupTypeTelemetryTest.kt @@ -0,0 +1,140 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import androidx.lifecycle.Lifecycle +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import mozilla.components.support.ktx.kotlin.crossProduct +import mozilla.components.support.test.robolectric.testContext +import mozilla.telemetry.glean.testing.GleanTestRule +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.PerfStartup +import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.perf.StartupPathProvider.StartupPath +import org.mozilla.fenix.perf.StartupStateProvider.StartupState + +private val validTelemetryLabels = run { + val allStates = listOf("cold", "warm", "hot", "unknown") + val allPaths = listOf("main", "view", "unknown") + + allStates.crossProduct(allPaths) { state, path -> "${state}_$path" }.toSet() +} + +private val activityClass = HomeActivity::class.java + +@RunWith(FenixRobolectricTestRunner::class) +class StartupTypeTelemetryTest { + + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + + private lateinit var telemetry: StartupTypeTelemetry + private lateinit var callbacks: StartupTypeTelemetry.StartupTypeLifecycleObserver + @MockK private lateinit var stateProvider: StartupStateProvider + @MockK private lateinit var pathProvider: StartupPathProvider + + @Before + fun setUp() { + MockKAnnotations.init(this) + telemetry = spyk(StartupTypeTelemetry(stateProvider, pathProvider)) + callbacks = telemetry.getTestCallbacks() + } + + @Test + fun `WHEN attach is called THEN it is registered to the lifecycle`() { + val lifecycle = mockk(relaxed = true) + telemetry.attachOnHomeActivityOnCreate(lifecycle) + + verify { lifecycle.addObserver(any()) } + } + + @Test + fun `GIVEN all possible path and state combinations WHEN record telemetry THEN the labels are incremented the appropriate number of times`() { + val allPossibleInputArgs = StartupState.values().toList().crossProduct( + StartupPath.values().toList() + ) { state, path -> + Pair(state, path) + } + + allPossibleInputArgs.forEach { (state, path) -> + every { stateProvider.getStartupStateForStartedActivity(activityClass) } returns state + every { pathProvider.startupPathForActivity } returns path + + telemetry.record() + } + + validTelemetryLabels.forEach { label -> + // Path == NOT_SET gets bucketed with Path == UNKNOWN so we'll increment twice for those. + val expected = if (label.endsWith("unknown")) 2 else 1 + assertEquals("label: $label", expected, PerfStartup.startupType[label].testGetValue()) + } + + // All invalid labels go to a single bucket: let's verify it has no value. + assertFalse(PerfStartup.startupType["__other__"].testHasValue()) + } + + @Test + fun `WHEN record is called THEN telemetry is recorded with the appropriate label`() { + every { stateProvider.getStartupStateForStartedActivity(activityClass) } returns StartupState.COLD + every { pathProvider.startupPathForActivity } returns StartupPath.MAIN + + telemetry.record() + + assertEquals(1, PerfStartup.startupType["cold_main"].testGetValue()) + } + + @Test + fun `GIVEN the activity is launched WHEN onResume is called THEN we record the telemetry`() { + launchApp() + verify(exactly = 1) { telemetry.record() } + } + + @Test + fun `GIVEN the activity is launched WHEN the activity is paused and resumed THEN record is not called`() { + // This part of the test duplicates another test but it's needed to initialize the state of this test. + launchApp() + verify(exactly = 1) { telemetry.record() } + + callbacks.onPause(mockk()) + callbacks.onResume(mockk()) + + verify(exactly = 1) { telemetry.record() } // i.e. this shouldn't be called again. + } + + @Test + fun `GIVEN the activity is launched WHEN the activity is stopped and resumed THEN record is called again`() { + // This part of the test duplicates another test but it's needed to initialize the state of this test. + launchApp() + verify(exactly = 1) { telemetry.record() } + + callbacks.onPause(mockk()) + callbacks.onStop(mockk()) + callbacks.onStart(mockk()) + callbacks.onResume(mockk()) + + verify(exactly = 2) { telemetry.record() } // i.e. this should be called again. + } + + private fun launchApp() { + // What these return isn't important. + every { stateProvider.getStartupStateForStartedActivity(activityClass) } returns StartupState.COLD + every { pathProvider.startupPathForActivity } returns StartupPath.MAIN + + callbacks.onCreate(mockk()) + callbacks.onStart(mockk()) + callbacks.onResume(mockk()) + } +}