diff --git a/app/metrics.yaml b/app/metrics.yaml index 182e37e78b..98beb7d45f 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2564,6 +2564,25 @@ metrics: metadata: tags: - Experiments + search_page_load_time: + type: timing_distribution + time_unit: millisecond + lifetime: application + description: The time that it takes to load the Search content. + send_in_pings: + - metrics + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1842604 + data_reviews: + - https://github.com/mozilla-mobile/firefox-android/pull/2889 + data_sensitivity: + - technical + notification_emails: + - android-probes@mozilla.com + expires: 128 + metadata: + tags: + - Experiments customize_home: most_visited_sites: diff --git a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt index f67c3ab05b..5720b34538 100644 --- a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt @@ -20,6 +20,7 @@ import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext import mozilla.components.support.base.log.logger.Logger +import mozilla.telemetry.glean.internal.TimerId import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.Config import org.mozilla.fenix.GleanMetrics.Events @@ -27,20 +28,29 @@ import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components +import org.mozilla.fenix.nimbus.FxNimbus import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics +private const val PROGRESS_COMPLETE = 100 + /** * [Middleware] to record telemetry in response to [BrowserAction]s. * * @property settings reference to the application [Settings]. * @property metrics [MetricController] to pass events that have been mapped from actions + * @property nimbusSearchEngine The Nimbus search engine. + * @property searchState Map that stores the [TabSessionState.id] & [TimerId]. + * @property timerId The [TimerId] for the [Metrics.searchPageLoadTime]. */ class TelemetryMiddleware( private val context: Context, private val settings: Settings, private val metrics: MetricController, private val crashReporting: CrashReporting? = null, + private val nimbusSearchEngine: String = FxNimbus.features.searchExtraParams.value().searchEngine, + private val searchState: MutableMap = mutableMapOf(), + private val timerId: TimerId = Metrics.searchPageLoadTime.start(), ) : Middleware { private val logger = Logger("TelemetryMiddleware") @@ -54,11 +64,33 @@ class TelemetryMiddleware( // Pre process actions when (action) { + is ContentAction.UpdateIsSearchAction -> { + if (action.isSearch && nimbusSearchEngine.isNotEmpty() && + (action.searchEngineName == nimbusSearchEngine) + ) { + searchState[action.sessionId] = timerId + } + } is ContentAction.UpdateLoadingStateAction -> { context.state.findTab(action.sessionId)?.let { tab -> + val hasFinishedLoading = tab.content.loading && !action.loading + // Record UriOpened event when a non-private page finishes loading - if (tab.content.loading && !action.loading) { + if (hasFinishedLoading) { Events.normalAndPrivateUriCount.add() + + val progressCompleted = tab.content.progress == PROGRESS_COMPLETE + if (progressCompleted) { + searchState[action.sessionId]?.let { + Metrics.searchPageLoadTime.stopAndAccumulate(it) + } + } else { + searchState[action.sessionId]?.let { + Metrics.searchPageLoadTime.cancel(it) + } + } + + searchState.remove(action.sessionId) } } } diff --git a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt index b1bece8b92..8bc0bc7f04 100644 --- a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt @@ -22,6 +22,7 @@ import mozilla.components.support.base.android.Clock import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.telemetry.glean.internal.TimerId import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -44,6 +45,8 @@ import org.mozilla.fenix.utils.Settings import org.robolectric.shadows.ShadowLooper import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics +private const val SEARCH_ENGINE_NAME = "Test" + @RunWith(FenixRobolectricTestRunner::class) class TelemetryMiddlewareTest { @@ -60,12 +63,21 @@ class TelemetryMiddlewareTest { private val clock = FakeClock() private val metrics: MetricController = mockk() + private val searchState: MutableMap = mutableMapOf() + private val timerId = Metrics.searchPageLoadTime.start() @Before fun setUp() { Clock.delegate = clock settings = Settings(testContext) - telemetryMiddleware = TelemetryMiddleware(testContext, settings, metrics) + telemetryMiddleware = TelemetryMiddleware( + context = testContext, + settings = settings, + metrics = metrics, + nimbusSearchEngine = SEARCH_ENGINE_NAME, + searchState = searchState, + timerId = timerId, + ) store = BrowserStore( middleware = listOf(telemetryMiddleware) + EngineMiddleware.create(engine = mockk()), initialState = BrowserState(), @@ -79,6 +91,117 @@ class TelemetryMiddlewareTest { Clock.reset() } + @Test + fun `WHEN action is UpdateIsSearchAction & all valid args THEN searchState is updated with session id and timer id`() { + assertTrue(searchState.isEmpty()) + + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, true, SEARCH_ENGINE_NAME)) + .joinBlocking() + + assertEquals(1, searchState.size) + assertEquals(mutableMapOf(sessionId to timerId), searchState) + } + + @Test + fun `WHEN action is UpdateIsSearchAction & action is not search THEN searchState is not updated`() { + assertTrue(searchState.isEmpty()) + + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, false, SEARCH_ENGINE_NAME)) + .joinBlocking() + + assertTrue(searchState.isEmpty()) + } + + @Test + fun `WHEN action is UpdateIsSearchAction & search engine name is empty THEN searchState is not updated`() { + assertTrue(searchState.isEmpty()) + + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, true, "")) + .joinBlocking() + + assertTrue(searchState.isEmpty()) + } + + @Test + fun `WHEN action is UpdateIsSearchAction & search engine name is different to Nimbus THEN searchState is not updated`() { + assertTrue(searchState.isEmpty()) + + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, true, "$SEARCH_ENGINE_NAME 2")) + .joinBlocking() + + assertTrue(searchState.isEmpty()) + } + + @Test + fun `WHEN action is UpdateLoadingStateAction & progress completed THEN telemetry is added & searchState is empty`() { + assertNull(Metrics.searchPageLoadTime.testGetValue()) + + // Start searchState + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, true, SEARCH_ENGINE_NAME)) + .joinBlocking() + + assertEquals(1, searchState.size) + assertEquals(mutableMapOf(sessionId to timerId), searchState) + + // Update hasFinishedLoading + val tab = createTab( + id = sessionId, + url = "https://mozilla.org", + ).let { it.copy(content = it.content.copy(progress = 100)) } + + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() + val count = Events.normalAndPrivateUriCount.testGetValue()!! + assertEquals(1, count) + + // Finish searchState + assertNotNull(Metrics.searchPageLoadTime.testGetValue()) + assertTrue(searchState.isEmpty()) + } + + @Test + fun `WHEN action is UpdateLoadingStateAction & progress not completed THEN no telemetry & searchState is empty`() { + assertNull(Metrics.searchPageLoadTime.testGetValue()) + + // Start searchState + val sessionId = "1235" + store.dispatch(ContentAction.UpdateIsSearchAction(sessionId, true, SEARCH_ENGINE_NAME)) + .joinBlocking() + + assertEquals(1, searchState.size) + assertEquals(mutableMapOf(sessionId to timerId), searchState) + + // Update hasFinishedLoading + val tab = createTab( + id = sessionId, + url = "https://mozilla.org", + ).let { it.copy(content = it.content.copy(progress = 50)) } + + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking() + assertNull(Events.normalAndPrivateUriCount.testGetValue()) + + store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking() + val count = Events.normalAndPrivateUriCount.testGetValue()!! + assertEquals(1, count) + + // Finish searchState + assertNull(Metrics.searchPageLoadTime.testGetValue()) + assertTrue(searchState.isEmpty()) + } + @Test fun `WHEN a tab is added THEN the open tab count is updated`() { assertEquals(0, settings.openTabsCount)