For #24450 - Remove Event.wrapper for ErrorPage metrics

app/metrics.yaml

@@ -2527,6 +2527,7 @@ error_page:
       A user encountered an error page
     extra_keys:
       error_type:
+        type: string
         description: "The error type of the error page encountered"
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/1242

app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt

@@ -12,7 +12,7 @@ import mozilla.components.browser.errorpages.ErrorPages
 import mozilla.components.browser.errorpages.ErrorType
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.request.RequestInterceptor
-import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.GleanMetrics.ErrorPage
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.isOnline
 import org.mozilla.fenix.helpers.TestHelper.appContext
@@ -80,7 +80,7 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor {
         val improvedErrorType = improveErrorType(errorType)
         val riskLevel = getRiskLevel(improvedErrorType)
 
-        context.components.analytics.metrics.track(Event.ErrorPageVisited(improvedErrorType))
+        ErrorPage.visitedError.record(ErrorPage.VisitedErrorExtra(improvedErrorType.name))
 
         val errorPageUri = ErrorPages.createUrlEncodedErrorPage(
             context = context,

app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt

@@ -12,7 +12,7 @@ import mozilla.components.browser.errorpages.ErrorPages
 import mozilla.components.browser.errorpages.ErrorType
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.request.RequestInterceptor
-import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.GleanMetrics.ErrorPage
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.isOnline
 import java.lang.ref.WeakReference
@@ -63,7 +63,7 @@ class AppRequestInterceptor(
         val improvedErrorType = improveErrorType(errorType)
         val riskLevel = getRiskLevel(improvedErrorType)
 
-        context.components.analytics.metrics.track(Event.ErrorPageVisited(improvedErrorType))
+        ErrorPage.visitedError.record(ErrorPage.VisitedErrorExtra(improvedErrorType.name))
 
         val errorPageUri = ErrorPages.createUrlEncodedErrorPage(
             context = context,

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

@@ -5,7 +5,6 @@
 package org.mozilla.fenix.components.metrics
 
 import android.content.Context
-import mozilla.components.browser.errorpages.ErrorType
 import mozilla.components.browser.state.search.SearchEngine
 import mozilla.components.feature.top.sites.TopSite
 import org.mozilla.fenix.GleanMetrics.Addons
@@ -13,7 +12,6 @@ import org.mozilla.fenix.GleanMetrics.AppTheme
 import org.mozilla.fenix.GleanMetrics.Autoplay
 import org.mozilla.fenix.GleanMetrics.Collections
 import org.mozilla.fenix.GleanMetrics.ContextMenu
-import org.mozilla.fenix.GleanMetrics.ErrorPage
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.History
 import org.mozilla.fenix.GleanMetrics.Logins
@@ -395,11 +393,6 @@ sealed class Event {
             )
     }
 
-    data class ErrorPageVisited(val errorType: ErrorType) : Event() {
-        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 }
 

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

@@ -21,7 +21,6 @@ import org.mozilla.fenix.GleanMetrics.ContextualMenu
 import org.mozilla.fenix.GleanMetrics.CreditCards
 import org.mozilla.fenix.GleanMetrics.CustomTab
 import org.mozilla.fenix.GleanMetrics.CustomizeHome
-import org.mozilla.fenix.GleanMetrics.ErrorPage
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser
 import org.mozilla.fenix.GleanMetrics.History
@@ -202,10 +201,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.NormalAndPrivateUriOpened -> EventWrapper<NoExtraKeys>(
             { Events.normalAndPrivateUriCount.add(1) }
         )
-        is Event.ErrorPageVisited -> EventWrapper(
-            { ErrorPage.visitedError.record(it) },
-            { ErrorPage.visitedErrorKeys.valueOf(it) }
-        )
         is Event.SyncAuthOpened -> EventWrapper<NoExtraKeys>(
             { SyncAuth.opened.record(it) }
         )

app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt

@@ -15,13 +15,16 @@ import mozilla.components.browser.errorpages.ErrorPages
 import mozilla.components.browser.errorpages.ErrorType
 import mozilla.components.concept.engine.request.RequestInterceptor
 import mozilla.components.support.test.robolectric.testContext
+import mozilla.telemetry.glean.testing.GleanTestRule
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.AppRequestInterceptor.Companion.HIGH_RISK_ERROR_PAGES
 import org.mozilla.fenix.AppRequestInterceptor.Companion.LOW_AND_MEDIUM_RISK_ERROR_PAGES
+import org.mozilla.fenix.GleanMetrics.ErrorPage
 import org.mozilla.fenix.components.Services
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.isOnline
@@ -30,6 +33,9 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 @RunWith(FenixRobolectricTestRunner::class)
 class AppRequestInterceptorTest {
 
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
     private lateinit var interceptor: RequestInterceptor
     private lateinit var navigationController: NavController
 
@@ -156,7 +162,6 @@ class AppRequestInterceptorTest {
 
     @Test
     fun `onErrorRequest results in correct error page for low risk level error`() {
-        every { testContext.components.analytics } returns mockk(relaxed = true)
         setOf(
             ErrorType.UNKNOWN,
             ErrorType.ERROR_NET_INTERRUPT,
@@ -185,12 +190,17 @@ class AppRequestInterceptorTest {
             )
 
             assertEquals(expectedPage, actualPage)
+            // Check if the error metric was recorded
+            assertEquals(true, ErrorPage.visitedError.testHasValue())
+            assertEquals(
+                error.name,
+                ErrorPage.visitedError.testGetValue().last().extra?.get("error_type")
+            )
         }
     }
 
     @Test
     fun `onErrorRequest results in correct error page for medium risk level error`() {
-        every { testContext.components.analytics } returns mockk(relaxed = true)
         setOf(
             ErrorType.ERROR_SECURITY_BAD_CERT,
             ErrorType.ERROR_SECURITY_SSL,
@@ -203,12 +213,17 @@ class AppRequestInterceptorTest {
             )
 
             assertEquals(expectedPage, actualPage)
+            // Check if the error metric was recorded
+            assertEquals(true, ErrorPage.visitedError.testHasValue())
+            assertEquals(
+                error.name,
+                ErrorPage.visitedError.testGetValue().last().extra?.get("error_type")
+            )
         }
     }
 
     @Test
     fun `onErrorRequest results in correct error page for high risk level error`() {
-        every { testContext.components.analytics } returns mockk(relaxed = true)
         setOf(
             ErrorType.ERROR_SAFEBROWSING_HARMFUL_URI,
             ErrorType.ERROR_SAFEBROWSING_MALWARE_URI,
@@ -222,6 +237,12 @@ class AppRequestInterceptorTest {
             )
 
             assertEquals(expectedPage, actualPage)
+            // Check if the error metric was recorded
+            assertEquals(true, ErrorPage.visitedError.testHasValue())
+            assertEquals(
+                error.name,
+                ErrorPage.visitedError.testGetValue().last().extra?.get("error_type")
+            )
         }
     }