[fenix] For https://github.com/mozilla-mobile/fenix/issues/24452 - Remove Event.wrapper for SyncAuth metrics

app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt

@@ -32,17 +32,17 @@ import mozilla.components.service.fxa.manager.SCOPE_SESSION
 import mozilla.components.service.fxa.manager.SCOPE_SYNC
 import mozilla.components.service.fxa.manager.SyncEnginesStorage
 import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider
+import mozilla.components.service.glean.private.NoExtras
 import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage
 import mozilla.components.service.sync.logins.SyncableLoginsStorage
 import mozilla.components.support.utils.RunWhenReadyQueue
 import org.mozilla.fenix.Config
 import org.mozilla.fenix.FeatureFlags
+import org.mozilla.fenix.GleanMetrics.SyncAuth
 import org.mozilla.fenix.R
-import org.mozilla.fenix.perf.StrictModeManager
-import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
+import org.mozilla.fenix.perf.StrictModeManager
 import org.mozilla.fenix.perf.lazyMonitored
 import org.mozilla.fenix.sync.SyncedTabsIntegration
 import org.mozilla.fenix.utils.Settings
@@ -127,7 +127,6 @@ class BackgroundServices(
 
     private val telemetryAccountObserver = TelemetryAccountObserver(
         context.settings(),
-        context.components.analytics.metrics
     )
 
     val accountAbnormalities = AccountAbnormalities(context, crashReporter, strictMode)
@@ -214,42 +213,39 @@ private class AccountManagerReadyObserver(
 @VisibleForTesting(otherwise = PRIVATE)
 internal class TelemetryAccountObserver(
     private val settings: Settings,
-    private val metricController: MetricController
 ) : AccountObserver {
     override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
         settings.signedInFxaAccount = true
         when (authType) {
             // User signed-in into an existing FxA account.
-            AuthType.Signin -> Event.SyncAuthSignIn
+            AuthType.Signin -> SyncAuth.signIn.record(NoExtras())
 
             // User created a new FxA account.
-            AuthType.Signup -> Event.SyncAuthSignUp
+            AuthType.Signup -> SyncAuth.signUp.record(NoExtras())
 
             // User paired to an existing account via QR code scanning.
-            AuthType.Pairing -> Event.SyncAuthPaired
-
-            // User signed-in into an FxA account shared from another locally installed app using the reuse flow.
-            AuthType.MigratedReuse -> Event.SyncAuthFromSharedReuse
-
-            // User signed-in into an FxA account shared from another locally installed app using the copy flow.
-            AuthType.MigratedCopy -> Event.SyncAuthFromSharedCopy
+            AuthType.Pairing -> SyncAuth.paired.record(NoExtras())
 
             // Account Manager recovered a broken FxA auth state, without direct user involvement.
-            AuthType.Recovered -> Event.SyncAuthRecovered
+            AuthType.Recovered -> SyncAuth.recovered.record(NoExtras())
 
             // User signed-in into an FxA account via unknown means.
             // Exact mechanism identified by the 'action' param.
-            is AuthType.OtherExternal -> Event.SyncAuthOtherExternal
+            is AuthType.OtherExternal -> SyncAuth.otherExternal.record(NoExtras())
 
+            // User signed-in into an FxA account shared from another locally installed app using the copy flow.
+            AuthType.MigratedCopy,
+            // User signed-in into an FxA account shared from another locally installed app using the reuse flow.
+            AuthType.MigratedReuse,
             // Account restored from a hydrated state on disk (e.g. during startup).
-            AuthType.Existing -> null
-        }?.let {
-            metricController.track(it)
+            AuthType.Existing -> {
+                // no-op, events not recorded in Glean
+            }
         }
     }
 
     override fun onLoggedOut() {
-        metricController.track(Event.SyncAuthSignOut)
+        SyncAuth.signOut.record(NoExtras())
         settings.signedInFxaAccount = false
     }
 }

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -33,19 +33,6 @@ sealed class Event {
     object CustomTabsActionTapped : Event()
     object CustomTabsMenuOpened : Event()
     object NormalAndPrivateUriOpened : Event()
-    object SyncAuthOpened : Event()
-    object SyncAuthClosed : Event()
-    object SyncAuthSignUp : Event()
-    object SyncAuthSignIn : Event()
-    object SyncAuthSignOut : Event()
-    object SyncAuthScanPairing : Event()
-    object SyncAuthUseEmail : Event()
-    object SyncAuthUseEmailProblem : Event()
-    object SyncAuthPaired : Event()
-    object SyncAuthRecovered : Event()
-    object SyncAuthOtherExternal : Event()
-    object SyncAuthFromSharedReuse : Event()
-    object SyncAuthFromSharedCopy : Event()
     object SyncAccountOpened : Event()
     object SyncAccountSyncNow : Event()
     object SendTab : Event()

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

@@ -39,7 +39,6 @@ import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
 import org.mozilla.fenix.GleanMetrics.SearchTerms
 import org.mozilla.fenix.GleanMetrics.StartOnHome
 import org.mozilla.fenix.GleanMetrics.SyncAccount
-import org.mozilla.fenix.GleanMetrics.SyncAuth
 import org.mozilla.fenix.GleanMetrics.SyncedTabs
 import org.mozilla.fenix.GleanMetrics.Tab
 import org.mozilla.fenix.GleanMetrics.Tabs
@@ -158,39 +157,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.NormalAndPrivateUriOpened -> EventWrapper<NoExtraKeys>(
             { Events.normalAndPrivateUriCount.add(1) }
         )
-        is Event.SyncAuthOpened -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.opened.record(it) }
-        )
-        is Event.SyncAuthClosed -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.closed.record(it) }
-        )
-        is Event.SyncAuthUseEmail -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.useEmail.record(it) }
-        )
-        is Event.SyncAuthUseEmailProblem -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.useEmailProblem.record(it) }
-        )
-        is Event.SyncAuthSignIn -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.signIn.record(it) }
-        )
-        is Event.SyncAuthSignUp -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.signUp.record(it) }
-        )
-        is Event.SyncAuthPaired -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.paired.record(it) }
-        )
-        is Event.SyncAuthOtherExternal -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.otherExternal.record(it) }
-        )
-        is Event.SyncAuthRecovered -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.recovered.record(it) }
-        )
-        is Event.SyncAuthSignOut -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.signOut.record(it) }
-        )
-        is Event.SyncAuthScanPairing -> EventWrapper<NoExtraKeys>(
-            { SyncAuth.scanPairing.record(it) }
-        )
         is Event.SyncAccountOpened -> EventWrapper<NoExtraKeys>(
             { SyncAccount.opened.record(it) }
         )
@@ -777,7 +743,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.DismissedOnboarding -> null
         is Event.AddonInstalled -> null
         is Event.SearchWidgetInstalled -> null
-        is Event.SyncAuthFromSharedReuse, Event.SyncAuthFromSharedCopy -> null
     }
 
 /**

app/src/main/java/org/mozilla/fenix/settings/account/AccountProblemFragment.kt

@@ -14,8 +14,9 @@ import kotlinx.coroutines.launch
 import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.OAuthAccount
+import mozilla.telemetry.glean.private.NoExtras
+import org.mozilla.fenix.GleanMetrics.SyncAuth
 import org.mozilla.fenix.R
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.getPreferenceKey
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.requireComponents
@@ -25,7 +26,7 @@ class AccountProblemFragment : PreferenceFragmentCompat(), AccountObserver {
 
     private val signInClickListener = Preference.OnPreferenceClickListener {
         requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext())
-        requireComponents.analytics.metrics.track(Event.SyncAuthUseEmailProblem)
+        SyncAuth.useEmailProblem.record(NoExtras())
         // TODO The sign-in web content populates session history,
         // so pressing "back" after signing in won't take us back into the settings screen, but rather up the
         // session history stack.

app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt

@@ -20,11 +20,12 @@ import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.support.ktx.android.content.hasCamera
 import mozilla.components.support.ktx.android.content.isPermissionGranted
 import mozilla.components.support.ktx.android.view.hideKeyboard
+import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.Config
+import org.mozilla.fenix.GleanMetrics.SyncAuth
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FenixSnackbar
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.databinding.FragmentTurnOnSyncBinding
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
@@ -63,7 +64,7 @@ class TurnOnSyncFragment : Fragment(), AccountObserver {
     private fun navigateToPairFragment() {
         val directions = TurnOnSyncFragmentDirections.actionTurnOnSyncFragmentToPairFragment()
         requireView().findNavController().navigate(directions)
-        requireComponents.analytics.metrics.track(Event.SyncAuthScanPairing)
+        SyncAuth.scanPairing.record(NoExtras())
     }
 
     private val createAccountClickListener = View.OnClickListener {
@@ -73,7 +74,7 @@ class TurnOnSyncFragment : Fragment(), AccountObserver {
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         requireComponents.backgroundServices.accountManager.register(this, owner = this)
-        requireComponents.analytics.metrics.track(Event.SyncAuthOpened)
+        SyncAuth.opened.record(NoExtras())
 
         // App can be installed on devices with no camera modules. Like Android TV boxes.
         // Let's skip presenting the option to sign in by scanning a qr code in this case
@@ -86,7 +87,7 @@ class TurnOnSyncFragment : Fragment(), AccountObserver {
 
     override fun onDestroy() {
         super.onDestroy()
-        requireComponents.analytics.metrics.track(Event.SyncAuthClosed)
+        SyncAuth.closed.record(NoExtras())
     }
 
     override fun onResume() {
@@ -168,7 +169,7 @@ class TurnOnSyncFragment : Fragment(), AccountObserver {
 
     private fun navigateToPairWithEmail() {
         requireComponents.services.accountsAuthFeature.beginAuthentication(requireContext())
-        requireComponents.analytics.metrics.track(Event.SyncAuthUseEmail)
+        SyncAuth.useEmail.record(NoExtras())
         // TODO The sign-in web content populates session history,
         // so pressing "back" after signing in won't take us back into the settings screen, but rather up the
         // session history stack.

app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt

@@ -17,14 +17,25 @@ import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.support.base.observer.ObserverRegistry
+import mozilla.components.support.test.robolectric.testContext
+import mozilla.telemetry.glean.testing.GleanTestRule
+import org.junit.Assert.assertEquals
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
-import org.mozilla.fenix.components.metrics.Event
+import org.junit.runner.RunWith
+import org.mozilla.fenix.GleanMetrics.SyncAuth
 import org.mozilla.fenix.components.metrics.MetricController
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.utils.Settings
 
+// For gleanTestRule
+@RunWith(FenixRobolectricTestRunner::class)
 class BackgroundServicesTest {
 
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
     @MockK
     private lateinit var metrics: MetricController
 
@@ -40,7 +51,7 @@ class BackgroundServicesTest {
         every { metrics.track(any()) } just Runs
         every { settings.signedInFxaAccount = any() } just Runs
 
-        observer = TelemetryAccountObserver(settings, metrics)
+        observer = TelemetryAccountObserver(settings)
         registry = ObserverRegistry<AccountObserver>().apply { register(observer) }
     }
 
@@ -49,7 +60,9 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.Signin) }
-        verify { metrics.track(Event.SyncAuthSignIn) }
+        assertEquals(true, SyncAuth.signIn.testHasValue())
+        assertEquals(1, SyncAuth.signIn.testGetValue().size)
+        assertEquals(null, SyncAuth.signIn.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
@@ -59,7 +72,9 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.Signup) }
-        verify { metrics.track(Event.SyncAuthSignUp) }
+        assertEquals(true, SyncAuth.signUp.testHasValue())
+        assertEquals(1, SyncAuth.signUp.testGetValue().size)
+        assertEquals(null, SyncAuth.signUp.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
@@ -69,22 +84,31 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.Pairing) }
-        verify { metrics.track(Event.SyncAuthPaired) }
+        assertEquals(true, SyncAuth.paired.testHasValue())
+        assertEquals(1, SyncAuth.paired.testGetValue().size)
+        assertEquals(null, SyncAuth.paired.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
 
     @Test
-    fun `telemetry account observer tracks shared event`() {
+    fun `telemetry account observer tracks shared copy event`() {
         val account = mockk<OAuthAccount>()
 
-        registry.notifyObservers { onAuthenticated(account, AuthType.MigratedReuse) }
-        verify { metrics.track(Event.SyncAuthFromSharedReuse) }
+        registry.notifyObservers { onAuthenticated(account, AuthType.MigratedCopy) }
+        verify { metrics wasNot Called }
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
+    }
 
-        registry.notifyObservers { onAuthenticated(account, AuthType.MigratedCopy) }
-        verify { metrics.track(Event.SyncAuthFromSharedCopy) }
+    @Test
+    fun `telemetry account observer tracks shared reuse event`() {
+        val account = mockk<OAuthAccount>()
+
+        registry.notifyObservers { onAuthenticated(account, AuthType.MigratedReuse) }
+        verify { metrics wasNot Called }
+        verify { settings.signedInFxaAccount = true }
+        confirmVerified(metrics, settings)
     }
 
     @Test
@@ -92,7 +116,9 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.Recovered) }
-        verify { metrics.track(Event.SyncAuthRecovered) }
+        assertEquals(true, SyncAuth.recovered.testHasValue())
+        assertEquals(1, SyncAuth.recovered.testGetValue().size)
+        assertEquals(null, SyncAuth.recovered.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
@@ -102,7 +128,9 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.OtherExternal(null)) }
-        verify { metrics.track(Event.SyncAuthOtherExternal) }
+        assertEquals(true, SyncAuth.otherExternal.testHasValue())
+        assertEquals(1, SyncAuth.otherExternal.testGetValue().size)
+        assertEquals(null, SyncAuth.otherExternal.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
@@ -112,7 +140,9 @@ class BackgroundServicesTest {
         val account = mockk<OAuthAccount>()
 
         registry.notifyObservers { onAuthenticated(account, AuthType.OtherExternal("someAction")) }
-        verify { metrics.track(Event.SyncAuthOtherExternal) }
+        assertEquals(true, SyncAuth.otherExternal.testHasValue())
+        assertEquals(1, SyncAuth.otherExternal.testGetValue().size)
+        assertEquals(null, SyncAuth.otherExternal.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = true }
         confirmVerified(metrics, settings)
     }
@@ -130,7 +160,9 @@ class BackgroundServicesTest {
     @Test
     fun `telemetry account observer tracks sign out event`() {
         registry.notifyObservers { onLoggedOut() }
-        verify { metrics.track(Event.SyncAuthSignOut) }
+        assertEquals(true, SyncAuth.signOut.testHasValue())
+        assertEquals(1, SyncAuth.signOut.testGetValue().size)
+        assertEquals(null, SyncAuth.signOut.testGetValue().single().extra)
         verify { settings.signedInFxaAccount = false }
         confirmVerified(metrics, settings)
     }