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

@@ -86,71 +86,73 @@ events:
     notification_emails:
     - fenix-core@mozilla.com
     expires: "2020-03-01"
-  ss_menu_opened:
-    type: event
+  total_uri_count:
+    type: counter
     description: >
-      A user opened the search shortcut menu in the search view by pressing the shortcuts button
+      A counter of URIs visited by the user in the current session, including page reloads. This does not include background page requests and URIs from embedded pages or private browsing.
+    send_in_pings:
+    - baseline
     bugs:
-    - 793
+    - 1301
     data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
+    - https://github.com/mozilla-mobile/fenix/pull/1785
     notification_emails:
     - fenix-core@mozilla.com
     expires: "2020-03-01"
-  ss_menu_closed:
+  preference_toggled:
     type: event
     description: >
-      A user closed the search shortcut menu in the search view by pressing the shortcuts button
+      A user toggled a preference switch in settings
+    extra_keys:
+      preference_key:
+        description: "The preference key for the switch preference the user toggled. We currently track:
+          show_search_suggestions, show_visited_sites_bookmarks, remote_debugging, telemetry, tracking_protection"
+      enabled:
+        description: "Whether or not the preference is *now* enabled"
     bugs:
-    - 793
+    - 975
     data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
+    - https://github.com/mozilla-mobile/fenix/pull/1896
     notification_emails:
     - fenix-core@mozilla.com
     expires: "2020-03-01"
-  ss_selected:
+
+search_shortcuts:
+  opened:
     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"
+      A user opened the search shortcut menu in the search view by pressing the shortcuts button
     bugs:
-    - 793
+      - 793
     data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
+      - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
     notification_emails:
-    - fenix-core@mozilla.com
+      - fenix-core@mozilla.com
     expires: "2020-03-01"
-  total_uri_count:
-    type: counter
+  closed:
+    type: event
     description: >
-      A counter of URIs visited by the user in the current session, including page reloads. This does not include background page requests and URIs from embedded pages or private browsing.
-    send_in_pings:
-    - baseline
+      A user closed the search shortcut menu in the search view by pressing the shortcuts button
     bugs:
-    - 1301
+      - 793
     data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1785
+      - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
     notification_emails:
-    - fenix-core@mozilla.com
+      - fenix-core@mozilla.com
     expires: "2020-03-01"
-  preference_toggled:
+  selected:
     type: event
     description: >
-      A user toggled a preference switch in settings
+      A user selected a search shortcut engine to use
     extra_keys:
-      preference_key:
-        description: "The preference key for the switch preference the user toggled. We currently track:
-          show_search_suggestions, show_visited_sites_bookmarks, remote_debugging, telemetry, tracking_protection"
-      enabled:
-        description: "Whether or not the preference is *now* enabled"
+      engine:
+        description: "The name of the built-in search engine the user selected as a string"
     bugs:
-    - 975
+      - 793
     data_reviews:
-    - https://github.com/mozilla-mobile/fenix/pull/1896
+      - https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449
     notification_emails:
-    - fenix-core@mozilla.com
+      - fenix-core@mozilla.com
     expires: "2020-03-01"
 
 crash_reporter:

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<T : Enum<T>>(
 
     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<T : Enum<T>>(
     }
 }
 
-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<NoExtraKeys>(
+            { SearchShortcuts.opened.record(it) }
+        )
+        is Event.SearchShortcutMenuClosed -> EventWrapper<NoExtraKeys>(
+            { SearchShortcuts.closed.record(it) }
+        )
+        is Event.SearchShortcutSelected -> EventWrapper(
+            { SearchShortcuts.selected.record(it) },
+            { SearchShortcuts.selectedKeys.valueOf(it) }
+        )
+        is Event.ReaderModeAvailable -> EventWrapper<NoExtraKeys>(
+            { ReaderMode.available.record(it) }
+        )
         is Event.FindInPageOpened -> EventWrapper<NoExtraKeys>(
             { 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 {

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)
         }
     }
 

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<String, String>?
-            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<String, String>?
+        override val extras: Map<Events.preferenceToggledKeys, String>?
             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<Events.appOpenedKeys, String>?
+            get() = hashMapOf(Events.appOpenedKeys.source to source.name)
+    }
+
     data class CollectionSaveButtonPressed(val fromScreen: String) : Event() {
-        override val extras: Map<String, String>?
-            get() = mapOf("from_screen" to fromScreen)
+        override val extras: Map<Collections.saveButtonKeys, String>?
+            get() = mapOf(Collections.saveButtonKeys.fromScreen to fromScreen)
     }
 
     data class CollectionSaved(val tabsOpenCount: Int, val tabsSelectedCount: Int) : Event() {
-        override val extras: Map<String, String>?
+        override val extras: Map<Collections.savedKeys, String>?
             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<String, String>?
+        override val extras: Map<Collections.tabsAddedKeys, String>?
             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<String, String>?
-            get() = mapOf("item" to item)
+        override val extras: Map<Library.selectedItemKeys, String>?
+            get() = mapOf(Library.selectedItemKeys.item to item)
     }
 
     data class ErrorPageVisited(val errorType: ErrorType) : Event() {
-        override val extras: Map<String, String>?
-            get() = mapOf("error_type" to errorType.name)
+        override val extras: Map<ErrorPage.visitedErrorKeys, String>?
+            get() = mapOf(ErrorPage.visitedErrorKeys.errorType to errorType.name)
     }
 
     data class SearchBarTapped(val source: Source) : Event() {
         enum class Source { HOME, BROWSER }
-        override val extras: Map<String, String>?
-            get() = mapOf("source" to source.name)
+        override val extras: Map<Events.searchBarTappedKeys, String>?
+            get() = mapOf(Events.searchBarTappedKeys.source to source.name)
     }
 
     data class EnteredUrl(val autoCompleted: Boolean) : Event() {
-        override val extras: Map<String, String>?
-            get() = mapOf("autocomplete" to autoCompleted.toString())
+        override val extras: Map<Events.enteredUrlKeys, String>?
+            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<String, String>?
-            get() = mapOf("source" to eventSource.sourceLabel)
+        override val extras: Map<Events.performedSearchKeys, String>?
+            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<String, String>?
-            get() = mapOf("engine" to engine)
+        override val extras: Map<SearchShortcuts.selectedKeys, String>?
+            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<String, String>?
-            get() = mapOf("named" to item)
+        override val extras: Map<ContextMenu.itemTappedKeys, String>?
+            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<String, String>?
-            get() = mapOf("crash_submitted" to crashSubmitted.toString())
+        override val extras: Map<CrashReporter.closedKeys, String>?
+            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<String, String>?
-            get() = mapOf("item" to item.toString().toLowerCase())
+        override val extras: Map<Events.browserMenuActionKeys, String>?
+            get() = mapOf(Events.browserMenuActionKeys.item to item.toString().toLowerCase())
     }
 
     sealed class Search
 
-    open val extras: Map<String, String>?
+    internal open val extras: Map<*, String>?
         get() = null
 }