diff --git a/app/metrics.yaml b/app/metrics.yaml index ed353053f..21386fce5 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -6176,3 +6176,31 @@ search_terms: notification_emails: - android-probes@mozilla.com expires: "2022-11-01" + group_size_distribution: + type: custom_distribution + description: | + The distribution of search term tab group sizes. Rather than reporting + individual sizes directly as integers, it is currently desired to + report the sizes according to certain size ranges. + The "buckets" for reporting group sizes will be mapped as follows: + * 2 tabs -> 1 + * 3-5 tabs -> 2 + * 6-10 tabs -> 3 + * 11+ tabs -> 4 + Where the reported number will be 1, 2, 3, or 4, accordingly. + As an example, say a user has three groups of sizes 3, 6, and 15. The + app will report 2, 3, and 4 when this metric is tracked. + range_min: 1 + range_max: 4 + bucket_count: 5 + histogram_type: linear + unit: tab_group_size_code + bugs: + - https://github.com/mozilla-mobile/fenix/issues/22410 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/22479 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: "2022-12-01" diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index 969123de3..df664cf8c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -670,6 +670,8 @@ sealed class Event { get() = hashMapOf(SearchTerms.averageTabsPerGroupKeys.count to averageSize.toString()) } + data class SearchTermGroupSizeDistribution(val groupSizes: List) : Event() + object JumpBackInGroupTapped : Event() sealed class Search diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 9803efdba..c8dfe8ca2 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -904,6 +904,9 @@ private val Event.wrapper: EventWrapper<*>? { SearchTerms.averageTabsPerGroup.record(it) }, { SearchTerms.averageTabsPerGroupKeys.valueOf(it) } ) + is Event.SearchTermGroupSizeDistribution -> EventWrapper( + { SearchTerms.groupSizeDistribution.accumulateSamples(this.groupSizes.toLongArray()) }, + ) is Event.JumpBackInGroupTapped -> EventWrapper( { SearchTerms.jumpBackInGroupTapped.record(it) } ) 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 df584e229..729aaa68f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -40,6 +40,7 @@ import org.mozilla.fenix.databinding.FragmentTabTrayDialogBinding import org.mozilla.fenix.databinding.TabsTrayTabCounter2Binding import org.mozilla.fenix.databinding.TabstrayMultiselectItemsBinding import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.HomeScreenViewModel @@ -128,6 +129,11 @@ class TabsTrayFragment : AppCompatDialogFragment() { initialState = TabsTrayState( mode = initialMode, focusGroupTabId = args.focusGroupTabId + ), + middlewares = listOf( + TabsTrayMiddleware( + metrics = requireContext().metrics + ) ) ) } @@ -230,7 +236,6 @@ class TabsTrayFragment : AppCompatDialogFragment() { feature = TabsFeature( tabsTray = TabSorter( requireContext().settings(), - requireContext().components.analytics.metrics, tabsTrayStore ), store = requireContext().components.core.store, diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayMiddleware.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayMiddleware.kt new file mode 100644 index 000000000..9fcfaae65 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayMiddleware.kt @@ -0,0 +1,71 @@ +/* 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.tabstray + +import androidx.annotation.VisibleForTesting +import mozilla.components.lib.state.Middleware +import mozilla.components.lib.state.MiddlewareContext +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController + +/** + * [Middleware] that reacts to various [TabsTrayAction]s. + * + * @property metrics reference to the configured [MetricController] to record general page load events. + */ +class TabsTrayMiddleware( + private val metrics: MetricController +) : Middleware { + + private var shouldReportInactiveTabMetrics: Boolean = true + private var shouldReportSearchGroupMetrics: Boolean = true + + override fun invoke( + context: MiddlewareContext, + next: (TabsTrayAction) -> Unit, + action: TabsTrayAction + ) { + next(action) + + when (action) { + is TabsTrayAction.UpdateInactiveTabs -> { + if (shouldReportInactiveTabMetrics) { + shouldReportInactiveTabMetrics = false + metrics.track(Event.InactiveTabsCountUpdate(action.tabs.size)) + metrics.track(Event.TabsTrayHasInactiveTabs(action.tabs.size)) + } + } + is TabsTrayAction.UpdateSearchGroupTabs -> { + if (shouldReportSearchGroupMetrics) { + shouldReportSearchGroupMetrics = false + + metrics.track(Event.SearchTermGroupCount(action.groups.size)) + + if (action.groups.isNotEmpty()) { + val tabsPerGroup = action.groups.map { it.tabs.size } + val averageTabsPerGroup = tabsPerGroup.average() + metrics.track(Event.AverageTabsPerSearchTermGroup(averageTabsPerGroup)) + + val tabGroupSizeMapping = tabsPerGroup.map { generateTabGroupSizeMappedValue(it) } + metrics.track(Event.SearchTermGroupSizeDistribution(tabGroupSizeMapping)) + } + } + } + } + } + + @Suppress("MagicNumber") + @VisibleForTesting + /** + * This follows the logic outlined in metrics.yaml for "search_terms.group_size_distribution" + */ + internal fun generateTabGroupSizeMappedValue(size: Int): Long = + when (size) { + 2 -> 1L + in 3..5 -> 2L + in 6..10 -> 3L + else -> 4L + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt index 7a932d987..1a72abec9 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt @@ -7,8 +7,6 @@ package org.mozilla.fenix.tabstray.browser import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabsTray import mozilla.components.feature.tabs.tabstray.TabsFeature -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.maxActiveTime import org.mozilla.fenix.ext.toSearchGroup import org.mozilla.fenix.tabstray.TabsTrayAction @@ -23,10 +21,8 @@ import org.mozilla.fenix.utils.Settings */ class TabSorter( private val settings: Settings, - private val metrics: MetricController, private val tabsTrayStore: TabsTrayStore? = null ) : TabsTray { - private var shouldReportMetrics: Boolean = true private val groupsSet = mutableSetOf() override fun updateTabs(tabs: List, selectedTabId: String?) { @@ -52,23 +48,6 @@ class TabSorter( // Normal tabs. val totalNormalTabs = (normalTabs + remainderTabs) tabsTrayStore?.dispatch(TabsTrayAction.UpdateNormalTabs(totalNormalTabs)) - - // TODO move this to a middleware in the TabsTrayStore. - if (shouldReportMetrics) { - shouldReportMetrics = false - - metrics.track(Event.InactiveTabsCountUpdate(inactiveTabs.size)) - - if (settings.inactiveTabsAreEnabled) { - metrics.track(Event.TabsTrayHasInactiveTabs(inactiveTabs.size)) - } - - if (groups.isNotEmpty()) { - val averageTabsPerGroup = groups.map { it.tabs.size }.average() - metrics.track(Event.AverageTabsPerSearchTermGroup(averageTabsPerGroup)) - } - metrics.track(Event.SearchTermGroupCount(groups.size)) - } } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayMiddlewareTest.kt new file mode 100644 index 000000000..a024729fd --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayMiddlewareTest.kt @@ -0,0 +1,107 @@ +/* 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.tabstray + +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import mozilla.components.support.test.libstate.ext.waitUntilIdle +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.tabstray.browser.TabGroup + +class TabsTrayMiddlewareTest { + + private lateinit var store: TabsTrayStore + private lateinit var tabsTrayMiddleware: TabsTrayMiddleware + private lateinit var metrics: MetricController + + @Before + fun setUp() { + metrics = mockk(relaxed = true) + tabsTrayMiddleware = TabsTrayMiddleware( + metrics + ) + store = TabsTrayStore( + middlewares = listOf(tabsTrayMiddleware), + initialState = TabsTrayState() + ) + } + + @Test + fun `WHEN search term groups are updated AND there is at least one group THEN report the average tabs per group`() { + store.dispatch(TabsTrayAction.UpdateSearchGroupTabs(generateSearchTermTabGroupsForAverage())) + store.waitUntilIdle() + verify { metrics.track(Event.AverageTabsPerSearchTermGroup(5.0)) } + } + + @Test + fun `WHEN search term groups are updated AND there is at least one group THEN report the distribution of tab sizes`() { + store.dispatch(TabsTrayAction.UpdateSearchGroupTabs(generateSearchTermTabGroupsForDistribution())) + store.waitUntilIdle() + verify { metrics.track(Event.SearchTermGroupSizeDistribution(listOf(3L, 2L, 1L, 4L))) } + } + + @Test + fun `WHEN search term groups are updated THEN report the count of search term tab groups`() { + store.dispatch(TabsTrayAction.UpdateSearchGroupTabs(emptyList())) + store.waitUntilIdle() + verify { metrics.track(Event.SearchTermGroupCount(0)) } + } + + @Test + fun `WHEN inactive tabs are updated THEN report the count of inactive tabs`() { + store.dispatch(TabsTrayAction.UpdateInactiveTabs(emptyList())) + store.waitUntilIdle() + verify { metrics.track(Event.TabsTrayHasInactiveTabs(0)) } + verify { metrics.track(Event.InactiveTabsCountUpdate(0)) } + } + + @Test + fun testGenerateTabGroupSizeMappedValue() { + assertEquals(1L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(2)) + assertEquals(2L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(3)) + assertEquals(2L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(4)) + assertEquals(2L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(5)) + assertEquals(3L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(6)) + assertEquals(3L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(7)) + assertEquals(3L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(8)) + assertEquals(3L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(9)) + assertEquals(3L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(10)) + assertEquals(4L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(11)) + assertEquals(4L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(12)) + assertEquals(4L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(20)) + assertEquals(4L, tabsTrayMiddleware.generateTabGroupSizeMappedValue(50)) + } + + private fun generateSearchTermTabGroupsForAverage(): List { + val group1 = TabGroup("", mockk(relaxed = true), 0L) + val group2 = TabGroup("", mockk(relaxed = true), 0L) + val group3 = TabGroup("", mockk(relaxed = true), 0L) + + every { group1.tabs.size } returns 8 + every { group2.tabs.size } returns 4 + every { group3.tabs.size } returns 3 + + return listOf(group1, group2, group3) + } + + private fun generateSearchTermTabGroupsForDistribution(): List { + val group1 = TabGroup("", mockk(relaxed = true), 0L) + val group2 = TabGroup("", mockk(relaxed = true), 0L) + val group3 = TabGroup("", mockk(relaxed = true), 0L) + val group4 = TabGroup("", mockk(relaxed = true), 0L) + + every { group1.tabs.size } returns 8 + every { group2.tabs.size } returns 4 + every { group3.tabs.size } returns 2 + every { group4.tabs.size } returns 12 + + return listOf(group1, group2, group3, group4) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt index 603cd0953..c3be18796 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/TabSorterTest.kt @@ -6,20 +6,16 @@ package org.mozilla.fenix.tabstray.browser import io.mockk.every import io.mockk.mockk -import io.mockk.verify import mozilla.components.browser.state.state.createTab import mozilla.components.support.test.libstate.ext.waitUntilIdle import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.utils.Settings class TabSorterTest { private val settings: Settings = mockk() - private val metrics: MetricController = mockk() private var inactiveTimestamp = 0L private val tabsTrayStore = TabsTrayStore() @@ -27,12 +23,11 @@ class TabSorterTest { fun setUp() { every { settings.inactiveTabsAreEnabled }.answers { true } every { settings.searchTermTabGroupsAreEnabled }.answers { true } - every { metrics.track(any()) }.answers { } // do nothing } @Test fun `WHEN updated with one normal tab THEN adapter have only one normal tab and no header`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -50,7 +45,7 @@ class TabSorterTest { @Test fun `WHEN updated with one normal tab and two search term tab THEN adapter have normal tab and a search group`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -80,7 +75,7 @@ class TabSorterTest { @Test fun `WHEN updated with one normal tab, one inactive tab and two search term tab THEN adapter have normal tab, inactive tab and a search group`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -117,7 +112,7 @@ class TabSorterTest { @Test fun `WHEN inactive tabs is off THEN adapter have no inactive tab`() { every { settings.inactiveTabsAreEnabled }.answers { false } - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -154,7 +149,7 @@ class TabSorterTest { @Test fun `WHEN search term tabs is off THEN adapter have no search term group`() { every { settings.searchTermTabGroupsAreEnabled }.answers { false } - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -192,7 +187,7 @@ class TabSorterTest { fun `WHEN both inactive tabs and search term tabs are off THEN adapter have only normal tabs`() { every { settings.inactiveTabsAreEnabled }.answers { false } every { settings.searchTermTabGroupsAreEnabled }.answers { false } - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -227,7 +222,7 @@ class TabSorterTest { @Test fun `WHEN only one search term tab THEN there is no search group`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -248,7 +243,7 @@ class TabSorterTest { @Test fun `WHEN remove second last one search term tab THEN search group is kept even if there's only one tab`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) + val tabSorter = TabSorter(settings, tabsTrayStore) tabSorter.updateTabs( listOf( @@ -286,37 +281,4 @@ class TabSorterTest { assertEquals(tabsTrayStore.state.searchTermGroups.size, 1) assertEquals(tabsTrayStore.state.normalTabs.size, 0) } - - @Test - fun `GIVEN the inactive tabs feature is enabled WHEN the tray is opened THEN we report the number of inactive tabs`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) - - tabSorter.updateTabs( - listOf( - createTab(url = "url", id = "tab1", createdAt = inactiveTimestamp), - createTab(url = "url", id = "tab2", createdAt = inactiveTimestamp), - ), - selectedTabId = "tab2" - ) - tabsTrayStore.waitUntilIdle() - - verify { metrics.track(Event.InactiveTabsCountUpdate(2)) } - } - - @Test - fun `GIVEN the inactive tabs feature is disabled WHEN the tray is opened THEN we report 0 for the number of inactive tabs`() { - val tabSorter = TabSorter(settings, metrics, tabsTrayStore) - every { settings.inactiveTabsAreEnabled }.answers { false } - - tabSorter.updateTabs( - listOf( - createTab(url = "url", id = "tab1", createdAt = inactiveTimestamp), - createTab(url = "url", id = "tab2", createdAt = inactiveTimestamp), - ), - selectedTabId = "tab2" - ) - tabsTrayStore.waitUntilIdle() - - verify { metrics.track(Event.InactiveTabsCountUpdate(0)) } - } }