From 414a54ed028829fe617849dffb87c10354293ba9 Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Tue, 5 Apr 2022 10:16:09 -0700 Subject: [PATCH] for #24549: add telemetry for recent synced tab --- app/metrics.yaml | 85 +++++++++++++++++++ .../org/mozilla/fenix/home/HomeFragment.kt | 4 +- .../RecentSyncedTabFeature.kt | 24 ++++++ .../controller/RecentSyncedTabController.kt | 10 ++- .../recentsyncedtabs/view/RecentSyncedTab.kt | 2 + .../fenix/tabstray/TabsTrayFragment.kt | 11 +++ app/src/main/res/navigation/nav_graph.xml | 7 +- .../mozilla/fenix/components/AppStoreTest.kt | 3 +- .../RecentSyncedTabFeatureTest.kt | 55 ++++++++++++ .../DefaultRecentSyncedTabControllerTest.kt | 55 +++++++++++- 10 files changed, 249 insertions(+), 7 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index e09866fe3..79cf9cee4 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -3957,6 +3957,20 @@ tabs_tray: tab_count: description: The number of selected tabs added to colelction. type: quantity + access_point: + type: labeled_counter + description: | + The area that the tabs tray was accessed from. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + notification_emails: + - android-probes@mozilla.com + data_sensitivity: + - interaction + expires: 114 + collections: renamed: @@ -7570,3 +7584,74 @@ downloads: metadata: tags: - Download + +recent_synced_tabs: + recent_synced_tab_shown: + type: labeled_counter + description: | + Counts impressions of a recent synced tab on the homepage, labeled by the + device type the tab originates from. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + notification_emails: + - android-probes@mozilla.com + data_sensitivity: + - interaction + expires: 114 + recent_synced_tab_time_to_load: + type: timing_distribution + time_unit: millisecond + description: | + Measures the amount of time between the beginning of a sync and the end. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: 114 + recent_synced_tab_opened: + type: labeled_counter + description: | + Counts the number of times a recent synced tab is opened, labeled by the + device type the tab originates from. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + notification_emails: + - android-probes@mozilla.com + data_sensitivity: + - interaction + expires: 114 + show_all_synced_tabs_clicked: + type: counter + description: | + Counts how many times "show all synced tabs" button has been clicked. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + notification_emails: + - android-probes@mozilla.com + data_sensitivity: + - interaction + expires: 114 + latest_synced_tab_is_stale: + type: counter + description: | + Counts how often the loading placeholder is shown and the resulting tab + is the same as it was before the load. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/24549 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/24671 + notification_emails: + - android-probes@mozilla.com + data_sensitivity: + - interaction + expires: 114 diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 26e1d3d22..467c60fa7 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -123,6 +123,7 @@ import org.mozilla.fenix.perf.MarkersFragmentLifecycleCallbacks import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.settings.SupportUtils.SumoTopic.HELP import org.mozilla.fenix.settings.deletebrowsingdata.deleteAndQuit +import org.mozilla.fenix.tabstray.TabsTrayAccessPoint import org.mozilla.fenix.theme.ThemeManager import org.mozilla.fenix.utils.Settings.Companion.TOP_SITES_PROVIDER_MAX_THRESHOLD import org.mozilla.fenix.utils.ToolbarPopupWindow @@ -175,7 +176,7 @@ class HomeFragment : Fragment() { context = requireContext(), storage = requireComponents.backgroundServices.syncedTabsStorage, accountManager = requireComponents.backgroundServices.accountManager, - lifecycleOwner = viewLifecycleOwner + lifecycleOwner = viewLifecycleOwner, ) } @@ -364,6 +365,7 @@ class HomeFragment : Fragment() { recentSyncedTabController = DefaultRecentSyncedTabController( addNewTabUseCase = requireComponents.useCases.tabsUseCases.addTab, navController = findNavController(), + accessPoint = TabsTrayAccessPoint.HomeRecentSyncedTab, ), recentBookmarksController = DefaultRecentBookmarksController( activity = activity, diff --git a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt index 6fd3ac923..961542809 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.home.recentsyncedtabs import android.content.Context import androidx.lifecycle.LifecycleOwner import mozilla.components.browser.storage.sync.SyncedDeviceTabs +import mozilla.components.concept.sync.DeviceType import mozilla.components.feature.syncedtabs.SyncedTabsFeature import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage import mozilla.components.feature.syncedtabs.view.SyncedTabsView @@ -14,6 +15,8 @@ import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.support.base.feature.LifecycleAwareFeature import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction +import mozilla.telemetry.glean.GleanTimerId +import org.mozilla.fenix.GleanMetrics.RecentSyncedTabs /** * Delegate to handle layout updates and dispatch actions related to the recent synced tab. @@ -45,7 +48,12 @@ class RecentSyncedTabFeature( override var listener: SyncedTabsView.Listener? = null + private var syncStartId: GleanTimerId? = null + private var lastSyncedTab: RecentSyncedTab? = null + override fun startLoading() { + syncStartId?.let { RecentSyncedTabs.recentSyncedTabTimeToLoad.cancel(it) } + syncStartId = RecentSyncedTabs.recentSyncedTabTimeToLoad.start() store.dispatch( AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Loading) ) @@ -59,14 +67,17 @@ class RecentSyncedTabFeature( val tab = it.tabs.firstOrNull()?.active() ?: return RecentSyncedTab( deviceDisplayName = it.device.displayName, + deviceType = it.device.deviceType, title = tab.title, url = tab.url, iconUrl = tab.iconUrl ) } ?: return + recordMetrics(syncedTab, lastSyncedTab, syncStartId) store.dispatch( AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(syncedTab)) ) + lastSyncedTab = syncedTab } // UI will either not be displayed if not authenticated (DefaultPresenter.start), @@ -84,6 +95,18 @@ class RecentSyncedTabFeature( override fun stop() { syncedTabsFeature.stop() } + + private fun recordMetrics( + tab: RecentSyncedTab, + lastSyncedTab: RecentSyncedTab?, + syncStartId: GleanTimerId? + ) { + RecentSyncedTabs.recentSyncedTabShown[tab.deviceType.name].add() + RecentSyncedTabs.recentSyncedTabTimeToLoad.stopAndAccumulate(syncStartId) + if (tab == lastSyncedTab) { + RecentSyncedTabs.latestSyncedTabIsStale.add() + } + } } /** @@ -116,6 +139,7 @@ sealed class RecentSyncedTabState { */ data class RecentSyncedTab( val deviceDisplayName: String, + val deviceType: DeviceType, val title: String, val url: String, val iconUrl: String?, diff --git a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt index 4617d598a..151f985b0 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/controller/RecentSyncedTabController.kt @@ -6,11 +6,13 @@ package org.mozilla.fenix.home.recentsyncedtabs.controller import androidx.navigation.NavController import mozilla.components.feature.tabs.TabsUseCases +import org.mozilla.fenix.GleanMetrics.RecentSyncedTabs import org.mozilla.fenix.R import org.mozilla.fenix.home.HomeFragmentDirections import org.mozilla.fenix.home.recentsyncedtabs.RecentSyncedTab import org.mozilla.fenix.home.recentsyncedtabs.interactor.RecentSyncedTabInteractor import org.mozilla.fenix.tabstray.Page +import org.mozilla.fenix.tabstray.TabsTrayAccessPoint /** * An interface that handles the view manipulation of the recent synced tabs in the Home screen. @@ -36,15 +38,21 @@ interface RecentSyncedTabController { class DefaultRecentSyncedTabController( private val addNewTabUseCase: TabsUseCases.AddNewTabUseCase, private val navController: NavController, + private val accessPoint: TabsTrayAccessPoint, ) : RecentSyncedTabController { override fun handleRecentSyncedTabClick(tab: RecentSyncedTab) { + RecentSyncedTabs.recentSyncedTabOpened[tab.deviceType.name].add() addNewTabUseCase.invoke(tab.url) navController.navigate(R.id.browserFragment) } override fun handleSyncedTabShowAllClicked() { + RecentSyncedTabs.showAllSyncedTabsClicked.add() navController.navigate( - HomeFragmentDirections.actionGlobalTabsTrayFragment(page = Page.SyncedTabs) + HomeFragmentDirections.actionGlobalTabsTrayFragment( + page = Page.SyncedTabs, + accessPoint = accessPoint + ) ) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt index 9b697678a..de77351f0 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt @@ -35,6 +35,7 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import mozilla.components.concept.sync.DeviceType import org.mozilla.fenix.R import org.mozilla.fenix.compose.PrimaryText import org.mozilla.fenix.compose.SecondaryText @@ -206,6 +207,7 @@ private fun TextLinePlaceHolder() { private fun LoadedRecentSyncedTab() { val tab = RecentSyncedTab( deviceDisplayName = "Firefox on MacBook", + deviceType = DeviceType.DESKTOP, title = "This is a long site title", url = "https://mozilla.org", iconUrl = "https://mozilla.org", diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt index 4e319fb79..a19f729f3 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -66,6 +66,14 @@ import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsIntegration import org.mozilla.fenix.utils.allowUndo import kotlin.math.max +/** + * The action or screen that was used to navigate to the Tabs Tray. + */ +enum class TabsTrayAccessPoint { + None, + HomeRecentSyncedTab +} + @Suppress("TooManyFunctions", "LargeClass") class TabsTrayFragment : AppCompatDialogFragment() { @@ -127,6 +135,9 @@ class TabsTrayFragment : AppCompatDialogFragment() { ) val args by navArgs() + args.accessPoint.takeIf { it != TabsTrayAccessPoint.None }?.let { + TabsTray.accessPoint[it.name].add() + } val initialMode = if (args.enterMultiselect) { TabsTrayState.Mode.Select(emptySet()) } else { diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index f47d25fa5..8d419b10b 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -165,8 +165,11 @@ + app:argType="org.mozilla.fenix.tabstray.Page" /> + ()) } just runs + + controller.handleRecentSyncedTabClick(tab) + + assertEquals(1, RecentSyncedTabs.recentSyncedTabOpened[deviceType.name].testGetValue()) + } + + @Test + fun `WHEN synced tab show all clicked THEN metric counter is incremented`() { + every { navController.navigate(any()) } just runs + + controller.handleSyncedTabShowAllClicked() + + assertEquals(1, RecentSyncedTabs.showAllSyncedTabsClicked.testGetValue()) + } }