For #2706: Refactor Glean to reduce errors (#4753)

* For #2706: Adds recording for untracked events * For #2706: Adds key alignment to Metrics
2019-08-19 15:25:48 -07:00 · 2019-08-19 15:25:48 -07:00 · 0d4eceed56
parent 4731977f5d
commit 0d4eceed56
7 changed files with 161 additions and 117 deletions
--- 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
--- 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<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 {
--- 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)
        }
    }

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

--- 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
--- a/docs/metrics.md
+++ b/docs/metrics.md
@ -85,34 +85,6 @@ Private Tab, Share, Report Site Issue, Back/Forward button, Reload Button</td></
        </td>
        <td>2020-03-01</td>
    </tr>
-    <tr>
-        <td>ss_menu_opened</td>
-        <td>event</td>
-        <td>A user opened the search shortcut menu in the search view by pressing the shortcuts button</td>
-        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
-        <td></td>
-        <td>2020-03-01</td>
-    </tr>
-    <tr>
-        <td>ss_menu_closed</td>
-        <td>event</td>
-        <td> A user closed the search shortcut menu in the search view by pressing the shortcuts button</td>
-        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
-        <td></td>
-        <td>2020-03-01</td>
-    </tr>
-    <tr>
-        <td>ss_selected</td>
-        <td>event</td>
-        <td>A user selected a search shortcut engine to use</td>
-        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
-        <td>
-            <table>
-                <tr><td>engine</td><td>The name of the built-in search engine the user selected as a string</td></tr>
-            </table>
-        </td>
-        <td>2020-03-01</td>
-    </tr>
    <tr>
        <td>total_uri_count</td>
        <td>counter</td>
@ -140,6 +112,40 @@ tracking_protection</td>
 </table>
 </pre>

+## search_shortcuts
+<pre>
+<table style="width: 100%">
+    <tr>
+        <td>opened</td>
+        <td>event</td>
+        <td>A user opened the search shortcut menu in the search view by pressing the shortcuts button</td>
+        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
+        <td></td>
+        <td>2020-03-01</td>
+    </tr>
+    <tr>
+        <td>closed</td>
+        <td>event</td>
+        <td> A user closed the search shortcut menu in the search view by pressing the shortcuts button</td>
+        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
+        <td></td>
+        <td>2020-03-01</td>
+    </tr>
+    <tr>
+        <td>selected</td>
+        <td>event</td>
+        <td>A user selected a search shortcut engine to use</td>
+        <td><a href="https://github.com/mozilla-mobile/fenix/pull/1202#issuecomment-476870449">link</a></td>
+        <td>
+            <table>
+                <tr><td>engine</td><td>The name of the built-in search engine the user selected as a string</td></tr>
+            </table>
+        </td>
+        <td>2020-03-01</td>
+    </tr>
+</table>
+</pre>
+
 ## crash_reporter

 <pre>
--- 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
    <td>`E_Cleared_Private_Data`</td>
    <td>The user cleared one or many types of private data</td>
    <td><a href="https://github.com/mozilla-mobile/fenix/pull/4626#issuecomment-519691332">#4626</a></td>
-  </tr> 
+  </tr>
+  <tr>
+    <td>`E_Dismissed_Onboarding`</td>
+    <td>The user finished onboarding. Could be triggered by pressing "start browsing," opening settings, or invoking a search.</td>
+    <td><a href="https://github.com/mozilla-mobile/fenix/pull/3459#issuecomment-502191109">#3459</a></td>
+  </tr>
 </table>

 Deep links