From 0d4eceed56eda7a1ee416b4a5ce9b75f49a8d912 Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Mon, 19 Aug 2019 15:25:48 -0700 Subject: [PATCH] For #2706: Refactor Glean to reduce errors (#4753) * For #2706: Adds recording for untracked events * For #2706: Adds key alignment to Metrics --- app/metrics.yaml | 74 ++++++++------- .../components/metrics/GleanMetricsService.kt | 27 +++++- .../metrics/LeanplumMetricsService.kt | 8 +- .../fenix/components/metrics/Metrics.kt | 95 ++++++++++--------- .../fenix/onboarding/FenixOnboarding.kt | 5 +- docs/metrics.md | 62 ++++++------ docs/mma.md | 7 +- 7 files changed, 161 insertions(+), 117 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 3845454aa..7d9973322 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -86,42 +86,6 @@ events: notification_emails: - fenix-core@mozilla.com expires: "2020-03-01" - ss_menu_opened: - type: event - description: > - A user opened the search shortcut menu in the search view by pressing the shortcuts button - bugs: - - 793 - data_reviews: - - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 - notification_emails: - - fenix-core@mozilla.com - expires: "2020-03-01" - ss_menu_closed: - type: event - description: > - A user closed the search shortcut menu in the search view by pressing the shortcuts button - bugs: - - 793 - data_reviews: - - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 - notification_emails: - - fenix-core@mozilla.com - expires: "2020-03-01" - ss_selected: - type: event - description: > - A user selected a search shortcut engine to use - extra_keys: - engine: - description: "The name of the built-in search engine the user selected as a string" - bugs: - - 793 - data_reviews: - - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 - notification_emails: - - fenix-core@mozilla.com - expires: "2020-03-01" total_uri_count: type: counter description: > @@ -153,6 +117,44 @@ events: - fenix-core@mozilla.com expires: "2020-03-01" +search_shortcuts: + opened: + type: event + description: > + A user opened the search shortcut menu in the search view by pressing the shortcuts button + bugs: + - 793 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + closed: + type: event + description: > + A user closed the search shortcut menu in the search view by pressing the shortcuts button + bugs: + - 793 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + selected: + type: event + description: > + A user selected a search shortcut engine to use + extra_keys: + engine: + description: "The name of the built-in search engine the user selected as a string" + bugs: + - 793 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449 + notification_emails: + - fenix-core@mozilla.com + expires: "2020-03-01" + crash_reporter: opened: type: event 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 c1024bd50..1492b1839 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 @@ -31,6 +31,7 @@ import org.mozilla.fenix.GleanMetrics.QrScanner import org.mozilla.fenix.GleanMetrics.QuickActionSheet import org.mozilla.fenix.GleanMetrics.ReaderMode import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine +import org.mozilla.fenix.GleanMetrics.SearchShortcuts import org.mozilla.fenix.GleanMetrics.SearchWidget import org.mozilla.fenix.GleanMetrics.SyncAccount import org.mozilla.fenix.GleanMetrics.SyncAuth @@ -48,7 +49,7 @@ private class EventWrapper>( fun track(event: Event) { val extras = if (keyMapper != null) { - event.extras?.mapKeys { keyMapper.invoke(it.key.asCamelCase) } + event.extras?.mapKeys { keyMapper.invoke(it.key.toString().asCamelCase) } } else { null } @@ -57,7 +58,7 @@ private class EventWrapper>( } } -private val Event.wrapper +private val Event.wrapper: EventWrapper<*>? get() = when (this) { is Event.OpenedApp -> EventWrapper( { Events.appOpened.record(it) }, @@ -78,6 +79,19 @@ private val Event.wrapper }, { Events.performedSearchKeys.valueOf(it) } ) + is Event.SearchShortcutMenuOpened -> EventWrapper( + { SearchShortcuts.opened.record(it) } + ) + is Event.SearchShortcutMenuClosed -> EventWrapper( + { SearchShortcuts.closed.record(it) } + ) + is Event.SearchShortcutSelected -> EventWrapper( + { SearchShortcuts.selected.record(it) }, + { SearchShortcuts.selectedKeys.valueOf(it) } + ) + is Event.ReaderModeAvailable -> EventWrapper( + { ReaderMode.available.record(it) } + ) is Event.FindInPageOpened -> EventWrapper( { FindInPage.opened.record(it) } ) @@ -304,8 +318,13 @@ private val Event.wrapper { SearchWidget.voiceButton.record(it) } ) - // Don't track other events with Glean - else -> null + // Don't record other events in Glean: + is Event.AddBookmark -> null + is Event.OpenedBookmark -> null + is Event.OpenedAppFirstRun -> null + is Event.InteractWithSearchURLArea -> null + is Event.ClearedPrivateData -> null + is Event.DismissedOnboarding -> null } class GleanMetricsService(private val context: Context) : MetricsService { diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt index f91509718..12db8c7e7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/LeanplumMetricsService.kt @@ -11,7 +11,6 @@ import com.leanplum.LeanplumActivityHelper import com.leanplum.annotations.Parser import com.leanplum.internal.LeanplumInternal import org.mozilla.fenix.BuildConfig -import org.mozilla.fenix.ext.components import org.mozilla.fenix.utils.Settings import java.util.UUID @@ -29,6 +28,7 @@ private val Event.name: String? is Event.SyncAuthSignOut -> "E_Sign_Out_FxA" is Event.FXANewSignup -> "E_New_Sign_Up_FxA" is Event.ClearedPrivateData -> "E_Cleared_Private_Data" + is Event.DismissedOnboarding -> "E_Dismissed_Onboarding" // Do not track other events in Leanplum else -> "" @@ -99,8 +99,12 @@ class LeanplumMetricsService(private val application: Application) : MetricsServ } override fun track(event: Event) { + val leanplumExtras = event.extras?.map { + it.key.toString() to it.value + }?.toMap() + event.name?.also { - Leanplum.track(it, event.extras) + Leanplum.track(it, leanplumExtras) } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt index b5d520098..baa522b34 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt @@ -13,18 +13,19 @@ import mozilla.components.support.base.facts.FactProcessor import mozilla.components.support.base.facts.Facts import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.BuildConfig +import org.mozilla.fenix.GleanMetrics.Collections +import org.mozilla.fenix.GleanMetrics.ContextMenu +import org.mozilla.fenix.GleanMetrics.CrashReporter +import org.mozilla.fenix.GleanMetrics.ErrorPage +import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.Library +import org.mozilla.fenix.GleanMetrics.SearchShortcuts import org.mozilla.fenix.R -import java.lang.IllegalArgumentException import java.util.Locale sealed class Event { - data class OpenedApp(val source: Source) : Event() { - enum class Source { APP_ICON, LINK, CUSTOM_TAB } - override val extras: Map? - get() = hashMapOf("source" to source.name) - } - + // Interaction Events object OpenedAppFirstRun : Event() object InteractWithSearchURLArea : Event() object FXANewSignup : Event() @@ -91,6 +92,13 @@ sealed class Event { object CollectionRenamePressed : Event() object SearchWidgetNewTabPressed : Event() object SearchWidgetVoiceSearchPressed : Event() + object FindInPageOpened : Event() + object FindInPageClosed : Event() + object FindInPageNext : Event() + object FindInPagePrevious : Event() + object FindInPageSearchCommitted : Event() + + // Interaction events with extras data class PreferenceToggled(val preferenceKey: String, val enabled: Boolean, val context: Context) : Event() { private val switchPreferenceTelemetryAllowList = listOf( @@ -101,10 +109,10 @@ sealed class Event { context.getString(R.string.pref_key_tracking_protection) ) - override val extras: Map? + override val extras: Map? get() = mapOf( - "preference_key" to preferenceKey, - "enabled" to enabled.toString() + Events.preferenceToggledKeys.preferenceKey to preferenceKey, + Events.preferenceToggledKeys.enabled to enabled.toString() ) init { @@ -113,47 +121,52 @@ sealed class Event { } } - // Interaction Events + data class OpenedApp(val source: Source) : Event() { + enum class Source { APP_ICON, LINK, CUSTOM_TAB } + override val extras: Map? + get() = hashMapOf(Events.appOpenedKeys.source to source.name) + } + data class CollectionSaveButtonPressed(val fromScreen: String) : Event() { - override val extras: Map? - get() = mapOf("from_screen" to fromScreen) + override val extras: Map? + get() = mapOf(Collections.saveButtonKeys.fromScreen to fromScreen) } data class CollectionSaved(val tabsOpenCount: Int, val tabsSelectedCount: Int) : Event() { - override val extras: Map? + override val extras: Map? get() = mapOf( - "tabs_open" to tabsOpenCount.toString(), - "tabs_selected" to tabsSelectedCount.toString() + Collections.savedKeys.tabsOpen to tabsOpenCount.toString(), + Collections.savedKeys.tabsSelected to tabsSelectedCount.toString() ) } data class CollectionTabsAdded(val tabsOpenCount: Int, val tabsSelectedCount: Int) : Event() { - override val extras: Map? + override val extras: Map? get() = mapOf( - "tabs_open" to tabsOpenCount.toString(), - "tabs_selected" to tabsSelectedCount.toString() + Collections.tabsAddedKeys.tabsOpen to tabsOpenCount.toString(), + Collections.tabsAddedKeys.tabsSelected to tabsSelectedCount.toString() ) } data class LibrarySelectedItem(val item: String) : Event() { - override val extras: Map? - get() = mapOf("item" to item) + override val extras: Map? + get() = mapOf(Library.selectedItemKeys.item to item) } data class ErrorPageVisited(val errorType: ErrorType) : Event() { - override val extras: Map? - get() = mapOf("error_type" to errorType.name) + override val extras: Map? + get() = mapOf(ErrorPage.visitedErrorKeys.errorType to errorType.name) } data class SearchBarTapped(val source: Source) : Event() { enum class Source { HOME, BROWSER } - override val extras: Map? - get() = mapOf("source" to source.name) + override val extras: Map? + get() = mapOf(Events.searchBarTappedKeys.source to source.name) } data class EnteredUrl(val autoCompleted: Boolean) : Event() { - override val extras: Map? - get() = mapOf("autocomplete" to autoCompleted.toString()) + override val extras: Map? + get() = mapOf(Events.enteredUrlKeys.autocomplete to autoCompleted.toString()) } data class PerformedSearch(val eventSource: EventSource) : Event() { @@ -197,25 +210,19 @@ sealed class Event { get() = "${source.descriptor}.$label" } - override val extras: Map? - get() = mapOf("source" to eventSource.sourceLabel) + override val extras: Map? + get() = mapOf(Events.performedSearchKeys.source to eventSource.sourceLabel) } // Track only built-in engine selection. Do not track user-added engines! data class SearchShortcutSelected(val engine: String) : Event() { - override val extras: Map? - get() = mapOf("engine" to engine) + override val extras: Map? + get() = mapOf(SearchShortcuts.selectedKeys.engine to engine) } - object FindInPageOpened : Event() - object FindInPageClosed : Event() - object FindInPageNext : Event() - object FindInPagePrevious : Event() - object FindInPageSearchCommitted : Event() - class ContextMenuItemTapped private constructor(val item: String) : Event() { - override val extras: Map? - get() = mapOf("named" to item) + override val extras: Map? + get() = mapOf(ContextMenu.itemTappedKeys.named to item) companion object { fun create(context_item: String) = allowList[context_item]?.let { ContextMenuItemTapped(it) } @@ -234,8 +241,8 @@ sealed class Event { object CrashReporterOpened : Event() data class CrashReporterClosed(val crashSubmitted: Boolean) : Event() { - override val extras: Map? - get() = mapOf("crash_submitted" to crashSubmitted.toString()) + override val extras: Map? + get() = mapOf(CrashReporter.closedKeys.crashSubmitted to crashSubmitted.toString()) } data class BrowserMenuItemTapped(val item: Item) : Event() { @@ -245,13 +252,13 @@ sealed class Event { SAVE_TO_COLLECTION } - override val extras: Map? - get() = mapOf("item" to item.toString().toLowerCase()) + override val extras: Map? + get() = mapOf(Events.browserMenuActionKeys.item to item.toString().toLowerCase()) } sealed class Search - open val extras: Map? + internal open val extras: Map<*, String>? get() = null } diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt b/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt index 2ee112163..dbcfb42e5 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/FenixOnboarding.kt @@ -7,7 +7,6 @@ package org.mozilla.fenix.onboarding import android.content.Context import android.content.SharedPreferences import androidx.core.content.edit -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components class FenixOnboarding(context: Context) { @@ -23,7 +22,9 @@ class FenixOnboarding(context: Context) { fun finish() { onboardingPrefs.onboardedVersion = CURRENT_ONBOARDING_VERSION - metrics.track(Event.DismissedOnboarding) + + // To be fixed in #4824 + // metrics.track(Event.DismissedOnboarding) } fun userHasBeenOnboarded() = onboardingPrefs.onboardedVersion == CURRENT_ONBOARDING_VERSION diff --git a/docs/metrics.md b/docs/metrics.md index b90ed7b8e..af036bca9 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -85,34 +85,6 @@ Private Tab, Share, Report Site Issue, Back/Forward button, Reload Button 2020-03-01 - - ss_menu_opened - event - A user opened the search shortcut menu in the search view by pressing the shortcuts button - link - - 2020-03-01 - - - ss_menu_closed - event - A user closed the search shortcut menu in the search view by pressing the shortcuts button - link - - 2020-03-01 - - - ss_selected - event - A user selected a search shortcut engine to use - link - - - -
engineThe name of the built-in search engine the user selected as a string
- - 2020-03-01 - total_uri_count counter @@ -140,6 +112,40 @@ tracking_protection +## search_shortcuts +
+
+    
+        
+        
+        
+        
+        
+        
+    
+    
+        
+        
+        
+        
+        
+        
+    
+    
+        
+        
+        
+        
+        
+        
+    
+
openedeventA user opened the search shortcut menu in the search view by pressing the shortcuts buttonlink2020-03-01
closedevent A user closed the search shortcut menu in the search view by pressing the shortcuts buttonlink2020-03-01
selectedeventA user selected a search shortcut engine to uselink
+            
+                
+            
engineThe name of the built-in search engine the user selected as a string

+        		2020-03-01

+
+ ## crash_reporter 
diff --git a/docs/mma.md b/docs/mma.md
index 3d7d1cd62..1cbd38217 100644
--- a/docs/mma.md
+++ b/docs/mma.md
@@ -220,7 +220,12 @@ Here is the list of current Events sent, which can be found here in the code bas
     `E_Cleared_Private_Data`
     The user cleared one or many types of private data
     #4626
-   
+  
+  
+    `E_Dismissed_Onboarding`
+    The user finished onboarding. Could be triggered by pressing "start browsing," opening settings, or invoking a search.
+    #3459
+  
 
 
 Deep links