From d56b4a2b9222d6a90202c306e4601e667266ed93 Mon Sep 17 00:00:00 2001 From: Codrut Topliceanu <60002907+codrut-topliceanu@users.noreply.github.com> Date: Thu, 11 Feb 2021 18:31:01 +0200 Subject: [PATCH] For #17418 - Added telemetry for Google Default Top Site (#17637) * For #17418 - Adds channel "ts" to TrackKey This is used to track if the `InContentTelemetry` is a result of the user using the Google Top Site. It looks for `&channel=ts` within the uri. * For #17418 - Adds TopSite PerformedSearch back in * For #17418 - Check now looks for equality with GOOGLE_URL * For #17418 - Adds test for topSite changes --- app/metrics.yaml | 2 +- .../mozilla/fenix/components/metrics/Event.kt | 4 +- .../fenix/components/metrics/MetricsUtils.kt | 6 ++- .../SessionControlController.kt | 25 +++++++++ .../telemetry/incontent/InContentTelemetry.kt | 5 +- .../telemetry/incontent/TrackKeyInfo.kt | 8 ++- .../DefaultSessionControlControllerTest.kt | 51 ++++++++++++++++++- .../incontent/InContentTelemetryTest.kt | 9 ++++ docs/metrics.md | 2 +- 9 files changed, 103 insertions(+), 9 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 4c9a2bd0e..3eced0a56 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -910,7 +910,7 @@ metrics: https://github.com/mozilla-mobile/fenix/issues/1607) the value will be `custom`. - `source` will be: `action`, `suggestion`, `widget` or `shortcut` + `source` will be: `action`, `suggestion`, `widget`, `shortcut`, `topsite` (depending on the source from which the search started). Also added the `other` option for the source but it should never enter on this case. send_in_pings: 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 a5b94eec2..974e6b83a 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 @@ -456,6 +456,7 @@ sealed class Event { data class Action(override val engineSource: EngineSource) : EventSource(engineSource) data class Widget(override val engineSource: EngineSource) : EventSource(engineSource) data class Shortcut(override val engineSource: EngineSource) : EventSource(engineSource) + data class TopSite(override val engineSource: EngineSource) : EventSource(engineSource) data class Other(override val engineSource: EngineSource) : EventSource(engineSource) private val label: String @@ -464,6 +465,7 @@ sealed class Event { is Action -> "action" is Widget -> "widget" is Shortcut -> "shortcut" + is TopSite -> "topsite" is Other -> "other" } @@ -475,7 +477,7 @@ sealed class Event { } enum class SearchAccessPoint { - SUGGESTION, ACTION, WIDGET, SHORTCUT, NONE + SUGGESTION, ACTION, WIDGET, SHORTCUT, TOPSITE, NONE } override val extras: Map? diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt index 18b53961a..8857290c3 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt @@ -17,7 +17,6 @@ import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint -import org.mozilla.fenix.ext.components import java.io.IOException import java.security.NoSuchAlgorithmException import java.security.spec.InvalidKeySpecException @@ -58,6 +57,11 @@ object MetricsUtils { engineSource ) ) + SearchAccessPoint.TOPSITE -> Event.PerformedSearch( + Event.PerformedSearch.EventSource.TopSite( + engineSource + ) + ) SearchAccessPoint.NONE -> Event.PerformedSearch( Event.PerformedSearch.EventSource.Other( engineSource diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index a181d1f69..1fa618487 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -6,12 +6,14 @@ package org.mozilla.fenix.home.sessioncontrol import android.view.LayoutInflater import android.widget.EditText +import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.browser.state.selector.getNormalOrPrivateTabs +import mozilla.components.browser.state.state.searchEngines import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine @@ -378,6 +380,20 @@ class DefaultSessionControlController( metrics.track(Event.PocketTopSiteClicked) } + if (SupportUtils.GOOGLE_URL.equals(url, true)) { + val availableEngines = getAvailableSearchEngines() + + val searchAccessPoint = Event.PerformedSearch.SearchAccessPoint.TOPSITE + val event = + availableEngines.firstOrNull { engine -> engine.suggestUrl?.contains(url) == true } + ?.let { searchEngine -> + searchAccessPoint.let { sap -> + MetricsUtils.createSearchEvent(searchEngine, store, sap) + } + } + event?.let { activity.metrics.track(it) } + } + addTabUseCase.invoke( url = appendSearchAttributionToUrlIfNeeded(url), selectTab = true, @@ -386,6 +402,15 @@ class DefaultSessionControlController( activity.openToBrowser(BrowserDirection.FromHome) } + @VisibleForTesting + internal fun getAvailableSearchEngines() = activity + .components + .core + .store + .state + .search + .searchEngines + /** * Append a search attribution query to any provided search engine URL based on the * user's current region. diff --git a/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetry.kt b/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetry.kt index 1be9e832c..f2ad23fea 100644 --- a/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetry.kt +++ b/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetry.kt @@ -59,8 +59,9 @@ class InContentTelemetry(private val metrics: MetricController) : BaseSearchTele // For Bing if it didn't have a valid cookie and for all the other search engines if (resultNotComputedFromCookies(trackKey) && hasValidCode(code, provider)) { + val channel = uri.getQueryParameter(CHANNEL_KEY) val type = getSapType(provider.followOnParams, paramSet) - trackKey = TrackKeyInfo(provider.name, type, code) + trackKey = TrackKeyInfo(provider.name, type, code, channel) } } @@ -145,5 +146,7 @@ class InContentTelemetry(private val metrics: MetricController) : BaseSearchTele private const val SEARCH_TYPE_ORGANIC = "organic" private const val SEARCH_TYPE_SAP = "sap" private const val SEARCH_TYPE_SAP_FOLLOW_ON = "sap-follow-on" + + private const val CHANNEL_KEY = "channel" } } diff --git a/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/TrackKeyInfo.kt b/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/TrackKeyInfo.kt index 73473943f..b67ec1522 100644 --- a/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/TrackKeyInfo.kt +++ b/app/src/main/java/org/mozilla/fenix/search/telemetry/incontent/TrackKeyInfo.kt @@ -9,11 +9,15 @@ import java.util.Locale internal data class TrackKeyInfo( var providerName: String, var type: String, - var code: String? + var code: String?, + var channel: String? = null ) { fun createTrackKey(): String { return "${providerName.toLowerCase(Locale.ROOT)}.in-content" + ".${type.toLowerCase(Locale.ROOT)}" + - ".${code?.toLowerCase(Locale.ROOT) ?: "none"}" + ".${code?.toLowerCase(Locale.ROOT) ?: "none"}" + + if (!channel?.toLowerCase(Locale.ROOT).isNullOrBlank()) + ".${channel?.toLowerCase(Locale.ROOT)}" + else "" } } diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index 16275ed79..6025ba8b3 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -8,6 +8,9 @@ import androidx.navigation.NavController import androidx.navigation.NavDirections import io.mockk.every import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.spyk +import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineDispatcher @@ -21,6 +24,7 @@ import mozilla.components.browser.state.state.ReaderState import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.recover.RecoverableTab +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.feature.session.SessionUseCases @@ -39,6 +43,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.Analytics import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.Event.PerformedSearch.EngineSource import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.components @@ -86,6 +91,16 @@ class DefaultSessionControlControllerTest { type = SearchEngine.Type.BUNDLED, resultUrls = listOf("https://example.org/?q={searchTerms}") ) + + private val googleSearchEngine = SearchEngine( + id = "googleTest", + name = "Google Test Engine", + icon = mockk(relaxed = true), + type = SearchEngine.Type.BUNDLED, + resultUrls = listOf("https://www.google.com/?q={searchTerms}"), + suggestUrl = "https://www.google.com/" + ) + private lateinit var store: BrowserStore private lateinit var controller: DefaultSessionControlController @@ -117,7 +132,7 @@ class DefaultSessionControlControllerTest { val restoreUseCase: TabsUseCases.RestoreUseCase = mockk(relaxed = true) - controller = DefaultSessionControlController( + controller = spyk(DefaultSessionControlController( activity = activity, store = store, settings = settings, @@ -136,7 +151,7 @@ class DefaultSessionControlControllerTest { showDeleteCollectionPrompt = showDeleteCollectionPrompt, showTabTray = showTabTray, handleSwipedItemDeletionCancel = handleSwipedItemDeletionCancel - ) + )) } @After @@ -392,6 +407,7 @@ class DefaultSessionControlControllerTest { @Test fun handleSelectGoogleDefaultTopSiteUS() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("US", "US"))).joinBlocking() @@ -412,6 +428,7 @@ class DefaultSessionControlControllerTest { @Test fun handleSelectGoogleDefaultTopSiteXX() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("DE", "FR"))).joinBlocking() @@ -429,9 +446,36 @@ class DefaultSessionControlControllerTest { verify { activity.openToBrowser(BrowserDirection.FromHome) } } + @Test + fun handleSelectGoogleDefaultTopSite_EventPerformedSearchTopSite() { + val topSiteUrl = SupportUtils.GOOGLE_URL + val engineSource = EngineSource.Default(googleSearchEngine, false) + every { controller.getAvailableSearchEngines() } returns listOf(googleSearchEngine) + try { + mockkStatic("mozilla.components.browser.state.state.SearchStateKt") + + every { any().selectedOrDefaultSearchEngine } returns googleSearchEngine + + controller.handleSelectTopSite(topSiteUrl, TopSite.Type.DEFAULT) + + verify { + metrics.track( + Event.PerformedSearch( + Event.PerformedSearch.EventSource.TopSite( + engineSource + ) + ) + ) + } + } finally { + unmockkStatic(SearchState::class) + } + } + @Test fun handleSelectGooglePinnedTopSiteUS() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("US", "US"))).joinBlocking() @@ -452,6 +496,7 @@ class DefaultSessionControlControllerTest { @Test fun handleSelectGooglePinnedTopSiteXX() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("DE", "FR"))).joinBlocking() @@ -472,6 +517,7 @@ class DefaultSessionControlControllerTest { @Test fun handleSelectGoogleFrecentTopSiteUS() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("US", "US"))).joinBlocking() @@ -492,6 +538,7 @@ class DefaultSessionControlControllerTest { @Test fun handleSelectGoogleFrecentTopSiteXX() { val topSiteUrl = SupportUtils.GOOGLE_URL + every { controller.getAvailableSearchEngines() } returns listOf(searchEngine) store.dispatch(SearchAction.SetRegionAction(RegionState("DE", "FR"))).joinBlocking() diff --git a/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt b/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt index b491df390..36e30f6cd 100644 --- a/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/telemetry/incontent/InContentTelemetryTest.kt @@ -109,6 +109,15 @@ class InContentTelemetryTest { verify { metrics.track(Event.SearchInContent("google.in-content.sap-follow-on.firefox-b-m")) } } + @Test + fun `track google sap-follow-on and topSite metric`() { + val url = "https://www.google.com/search?q=aaa&client=firefox-b-m&channel=ts&oq=random" + + telemetry.trackPartnerUrlTypeMetric(url, listOf()) + + verify { metrics.track(Event.SearchInContent("google.in-content.sap-follow-on.firefox-b-m.ts")) } + } + @Test fun `track baidu sap-follow-on metric`() { val url = "https://www.baidu.com/from=844b/s?wd=aaa&tn=34046034_firefox&oq=random" diff --git a/docs/metrics.md b/docs/metrics.md index d06c57488..36f60ebaa 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -298,7 +298,7 @@ The following metrics are added to the ping: | metrics.mobile_bookmarks_count |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |A counter that indicates how many bookmarks a user has in the mobile folder. This value will only be set if the user has at least *one* bookmark. If they have 0, this ping will not get sent, resulting in a null value. To disambiguate between a failed `mobile_bookmarks_count` ping and 0 bookmarks, please see `has_mobile_bookmarks`. |[1](https://github.com/mozilla-mobile/fenix/pull/16942), [2](https://github.com/mozilla-mobile/fenix/pull/16942#issuecomment-742794701)||2021-08-01 |2 | | metrics.mozilla_products |[string_list](https://mozilla.github.io/glean/book/user/metrics/string_list.html) |A list of all the Mozilla products installed on device. We currently scan for: Firefox, Firefox Beta, Firefox Aurora, Firefox Nightly, Firefox Fdroid, Firefox Lite, Reference Browser, Reference Browser Debug, Fenix, Focus, and Lockwise. |[1](https://github.com/mozilla-mobile/fenix/pull/1953/), [2](https://github.com/mozilla-mobile/fenix/pull/5216), [3](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |1, 2 | | metrics.recently_used_pwa_count |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |A counter that indicates how many PWAs a user has recently used. Threshold for "recency" set in HomeActivity#PWA_RECENTLY_USED_THRESHOLD. Currently we are not told by the OS when a PWA is removed by the user, so we use the "recently used" heuristic to judge how many PWAs are still active, as a proxy for "installed". This value will only be set if the user has at least *one* recently used PWA. If they have 0, this metric will not be sent, resulting in a null value during analysis on the server-side. To disambiguate between a failed `recently_used_pwa_count` metric and 0 recent PWAs, please see `has_recent_pwas`. |[1](https://github.com/mozilla-mobile/fenix/pull/11982#pullrequestreview-437963817), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | -| metrics.search_count |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |The labels for this counter are `.`. If the search engine is bundled with Fenix `search-engine-name` will be the name of the search engine. If it's a custom search engine (defined: https://github.com/mozilla-mobile/fenix/issues/1607) the value will be `custom`. `source` will be: `action`, `suggestion`, `widget` or `shortcut` (depending on the source from which the search started). Also added the `other` option for the source but it should never enter on this case. |[1](https://github.com/mozilla-mobile/fenix/pull/1677), [2](https://github.com/mozilla-mobile/fenix/pull/5216), [3](https://github.com/mozilla-mobile/fenix/pull/7310), [4](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |1, 2 | +| metrics.search_count |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |The labels for this counter are `.`. If the search engine is bundled with Fenix `search-engine-name` will be the name of the search engine. If it's a custom search engine (defined: https://github.com/mozilla-mobile/fenix/issues/1607) the value will be `custom`. `source` will be: `action`, `suggestion`, `widget`, `shortcut`, `topsite` (depending on the source from which the search started). Also added the `other` option for the source but it should never enter on this case. |[1](https://github.com/mozilla-mobile/fenix/pull/1677), [2](https://github.com/mozilla-mobile/fenix/pull/5216), [3](https://github.com/mozilla-mobile/fenix/pull/7310), [4](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |1, 2 | | metrics.search_widget_installed |[boolean](https://mozilla.github.io/glean/book/user/metrics/boolean.html) |Whether or not the search widget is installed |[1](https://github.com/mozilla-mobile/fenix/pull/10958), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 | | metrics.tab_view_setting |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |A string that indicates the setting for tab view: GRID, LIST |[1](https://github.com/mozilla-mobile/fenix/pull/15811#issuecomment-706402952)||2021-08-01 |2 | | metrics.tabs_open_count |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |A counter that indicates how many NORMAL tabs a user has open. This value will only be set if the user has at least *one* open tab. If they have 0, this ping will not get sent, resulting in a null value. To disambiguate between a failed `tabs_open_count` ping and 0 open tabs, please see `has_open_tabs`. |[1](https://github.com/mozilla-mobile/fenix/pull/12024), [2](https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068)||2021-08-01 |2 |