Telemetry fix backport

changelog/unreleased/bugfixes/7757.md

@@ -0,0 +1 @@
+- Resolved an issue where a crash could occur if telemetry sending settings were changed after creating `MapboxNavigation`.

libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt

@@ -92,7 +92,7 @@ import com.mapbox.navigation.core.routealternatives.RouteAlternativesRequestCall
 import com.mapbox.navigation.core.routeoptions.RouteOptionsUpdater
 import com.mapbox.navigation.core.routerefresh.RouteRefreshController
 import com.mapbox.navigation.core.routerefresh.RouteRefreshControllerProvider
-import com.mapbox.navigation.core.telemetry.MapboxNavigationTelemetry
+import com.mapbox.navigation.core.telemetry.TelemetryWrapper
 import com.mapbox.navigation.core.telemetry.events.FeedbackEvent
 import com.mapbox.navigation.core.telemetry.events.FeedbackHelper
 import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata
@@ -122,8 +122,6 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonObserver
 import com.mapbox.navigation.core.trip.session.eh.GraphAccessor
 import com.mapbox.navigation.core.trip.session.eh.RoadObjectMatcher
 import com.mapbox.navigation.core.trip.session.eh.RoadObjectsStore
-import com.mapbox.navigation.metrics.MapboxMetricsReporter
-import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate
 import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator
 import com.mapbox.navigation.navigator.internal.NavigatorLoader
 import com.mapbox.navigation.navigator.internal.router.RouterInterfaceAdapter
@@ -251,11 +249,15 @@ private const val MAPBOX_NOTIFICATION_ACTION_CHANNEL = "notificationActionButton
 class MapboxNavigation @VisibleForTesting internal constructor(
     val navigationOptions: NavigationOptions,
     private val threadController: ThreadController,
+    private val telemetryWrapper: TelemetryWrapper = TelemetryWrapper(),
 ) {
 
-    constructor(navigationOptions: NavigationOptions) : this(navigationOptions, ThreadController())
+    constructor(navigationOptions: NavigationOptions) : this(
+        navigationOptions,
+        ThreadController(),
+        TelemetryWrapper(),
+    )
 
-    private val accessToken: String? = navigationOptions.accessToken
     private val mainJobController = threadController.getMainScopeAndRootJob()
     private val directionsSession: DirectionsSession
     private var navigator: MapboxNativeNavigator
@@ -469,7 +471,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         val result = MapboxModuleProvider.createModule<Router>(MapboxModuleType.NavigationRouter) {
             paramsProvider(
                 ModuleParams.NavigationRouter(
-                    accessToken
+                    navigationOptions.accessToken
                         ?: throw RuntimeException(MAPBOX_NAVIGATION_TOKEN_EXCEPTION_ROUTER),
                     nativeRouter,
                     threadController,
@@ -564,25 +566,11 @@ class MapboxNavigation @VisibleForTesting internal constructor(
             arrivalProgressObserver
         )
 
-        ifNonNull(accessToken) { token ->
-            runInTelemetryContext { telemetry ->
-                logD(
-                    "MapboxMetricsReporter.init from MapboxNavigation main",
-                    telemetry.LOG_CATEGORY
-                )
-                MapboxMetricsReporter.init(
-                    navigationOptions.applicationContext,
-                    token,
-                    obtainUserAgent()
-                )
-                MapboxMetricsReporter.toggleLogging(navigationOptions.isDebugLoggingEnabled)
-                telemetry.initialize(
-                    this,
-                    navigationOptions,
-                    MapboxMetricsReporter,
-                )
-            }
-        }
+        telemetryWrapper.initialize(
+            mapboxNavigation = this,
+            navigationOptions = navigationOptions,
+            userAgent = obtainUserAgent(),
+        )
 
         val routeOptionsProvider = RouteOptionsUpdater()
 
@@ -1294,9 +1282,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         @OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
         routeRefreshController.destroy()
         routesPreviewController.unregisterAllRoutesPreviewObservers()
-        runInTelemetryContext { telemetry ->
-            telemetry.destroy(this@MapboxNavigation)
-        }
+        telemetryWrapper.destroy()
         threadController.cancelAllNonUICoroutines()
         threadController.cancelAllUICoroutines()
         ifNonNull(reachabilityObserverId) {
@@ -1751,17 +1737,15 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         feedbackMetadata: FeedbackMetadata?,
         userFeedbackCallback: UserFeedbackCallback?,
     ) {
-        runInTelemetryContext { telemetry ->
-            telemetry.postUserFeedback(
-                feedbackType,
-                description,
-                feedbackSource,
-                screenshot,
-                feedbackSubType,
-                feedbackMetadata,
-                userFeedbackCallback,
-            )
-        }
+        telemetryWrapper.postUserFeedback(
+            feedbackType,
+            description,
+            feedbackSource,
+            screenshot,
+            feedbackSubType,
+            feedbackMetadata,
+            userFeedbackCallback,
+        )
     }
 
     @ExperimentalPreviewMapboxNavigationAPI
@@ -1770,13 +1754,11 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         @CustomEvent.Type customEventType: String,
         customEventVersion: String,
     ) {
-        runInTelemetryContext { telemetry ->
-            telemetry.postCustomEvent(
-                payload = payload,
-                customEventType = customEventType,
-                customEventVersion = customEventVersion
-            )
-        }
+        telemetryWrapper.postCustomEvent(
+            payload = payload,
+            customEventType = customEventType,
+            customEventVersion = customEventVersion
+        )
     }
 
     /**
@@ -1789,9 +1771,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
      */
     @ExperimentalPreviewMapboxNavigationAPI
     fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper =
-        runInTelemetryContext { telemetry ->
-            telemetry.provideFeedbackMetadataWrapper()
-        } ?: throw java.lang.IllegalStateException(
+        telemetryWrapper.provideFeedbackMetadataWrapper() ?: throw java.lang.IllegalStateException(
             "To get FeedbackMetadataWrapper Telemetry must be enabled"
         )
 
@@ -2130,14 +2110,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         }
     }
 
-    private inline fun <T> runInTelemetryContext(func: (MapboxNavigationTelemetry) -> T): T? {
-        return if (TelemetryUtilsDelegate.getEventsCollectionState()) {
-            func(MapboxNavigationTelemetry)
-        } else {
-            null
-        }
-    }
-
     private fun obtainUserAgent(): String {
         return "$MAPBOX_NAVIGATION_USER_AGENT_BASE/${BuildConfig.MAPBOX_NAVIGATION_VERSION_NAME}"
     }

libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigationProvider.kt

@@ -1,7 +1,10 @@
 package com.mapbox.navigation.core
 
 import androidx.annotation.UiThread
+import androidx.annotation.VisibleForTesting
 import com.mapbox.navigation.base.options.NavigationOptions
+import com.mapbox.navigation.core.telemetry.TelemetryWrapper
+import com.mapbox.navigation.utils.internal.ThreadController
 
 /**
  * Singleton responsible for ensuring there is only one MapboxNavigation instance.
@@ -33,6 +36,22 @@ object MapboxNavigationProvider {
         return mapboxNavigation!!
     }
 
+    @VisibleForTesting
+    internal fun create(
+        navigationOptions: NavigationOptions,
+        threadController: ThreadController = ThreadController(),
+        telemetryWrapper: TelemetryWrapper = TelemetryWrapper()
+    ): MapboxNavigation {
+        mapboxNavigation?.onDestroy()
+        mapboxNavigation = MapboxNavigation(
+            navigationOptions,
+            threadController,
+            telemetryWrapper,
+        )
+
+        return mapboxNavigation!!
+    }
+
     /**
      * Retrieve MapboxNavigation instance. Should be called after [create].
      *

libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/MapboxNavigationTelemetry.kt

@@ -151,8 +151,10 @@ The class must be initialized before any telemetry events are reported.
 Attempting to use telemetry before initialization is called will throw an exception.
 Initialization may be called multiple times, the call is idempotent.
 The class has two public methods, postUserFeedback() and initialize().
+
+TODO(NAVAND-1820) refactor this class. It's hard to test because of statics.
  */
-internal object MapboxNavigationTelemetry {
+internal object MapboxNavigationTelemetry : SdkTelemetry {
     internal const val LOG_CATEGORY = "MapboxNavigationTelemetry"
 
     private const val ONE_SECOND = 1000
@@ -354,10 +356,14 @@ internal object MapboxNavigationTelemetry {
         log("Valid initialization")
     }
 
-    fun destroy(mapboxNavigation: MapboxNavigation) {
+    override fun destroy(mapboxNavigation: MapboxNavigation) {
         telemetryStop()
+
+        // TODO(NAVAND-1820) MapboxMetricsReporter is destroyed here,
+        // but initialized separately from MapboxNavigationTelemetry
         log("MapboxMetricsReporter disable")
         MapboxMetricsReporter.disable()
+
         mapboxNavigation.run {
             unregisterLocationObserver(locationsCollector)
             unregisterRouteProgressObserver(routeProgressObserver)
@@ -421,7 +427,7 @@ internal object MapboxNavigationTelemetry {
         userFeedbackCallbacks.remove(userFeedbackCallback)
     }
 
-    fun postCustomEvent(
+    override fun postCustomEvent(
         payload: String,
         customEventType: String,
         customEventVersion: String
@@ -437,7 +443,7 @@ internal object MapboxNavigationTelemetry {
     }
 
     @ExperimentalPreviewMapboxNavigationAPI
-    fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper {
+    override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper {
         (telemetryState as? NavTelemetryState.Running)?.sessionMetadata?.let { sessionMetadata ->
             return FeedbackMetadataWrapper(
                 sessionMetadata.navigatorSessionIdentifier,
@@ -462,7 +468,7 @@ internal object MapboxNavigationTelemetry {
     }
 
     @OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
-    fun postUserFeedback(
+    override fun postUserFeedback(
         feedbackType: String,
         description: String,
         @FeedbackEvent.Source feedbackSource: String,

libnavigation-core/src/main/java/com/mapbox/navigation/core/telemetry/SdkTelemetry.kt

@@ -0,0 +1,67 @@
+package com.mapbox.navigation.core.telemetry
+
+import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
+import com.mapbox.navigation.core.MapboxNavigation
+import com.mapbox.navigation.core.internal.telemetry.CustomEvent
+import com.mapbox.navigation.core.internal.telemetry.UserFeedbackCallback
+import com.mapbox.navigation.core.telemetry.events.FeedbackEvent
+import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata
+import com.mapbox.navigation.core.telemetry.events.FeedbackMetadataWrapper
+
+internal interface SdkTelemetry {
+
+    fun postCustomEvent(
+        payload: String,
+        @CustomEvent.Type customEventType: String,
+        customEventVersion: String,
+    )
+
+    @ExperimentalPreviewMapboxNavigationAPI
+    fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper?
+
+    @ExperimentalPreviewMapboxNavigationAPI
+    fun postUserFeedback(
+        feedbackType: String,
+        description: String,
+        @FeedbackEvent.Source feedbackSource: String,
+        screenshot: String?,
+        feedbackSubType: Array<String>?,
+        feedbackMetadata: FeedbackMetadata?,
+        userFeedbackCallback: UserFeedbackCallback?,
+    )
+
+    fun destroy(mapboxNavigation: MapboxNavigation)
+
+    companion object {
+
+        val EMPTY = object : SdkTelemetry {
+            override fun postCustomEvent(
+                payload: String,
+                customEventType: String,
+                customEventVersion: String,
+            ) {
+                // do nothing
+            }
+
+            @ExperimentalPreviewMapboxNavigationAPI
+            override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper? = null
+
+            @ExperimentalPreviewMapboxNavigationAPI
+            override fun postUserFeedback(
+                feedbackType: String,
+                description: String,
+                feedbackSource: String,
+                screenshot: String?,
+                feedbackSubType: Array<String>?,
+                feedbackMetadata: FeedbackMetadata?,
+                userFeedbackCallback: UserFeedbackCallback?,
+            ) {
+                // do nothing
+            }
+
+            override fun destroy(mapboxNavigation: MapboxNavigation) {
+                // do nothing
+            }
+        }
+    }
+}