Skip to content

Commit 00bc5e7

Browse files
DzmitryFomchynDzmitryFomchyn
authored
Dynamic telemetry initialisation/deinitialisation based on user preferences (#7755)
* Dynamic telemetry initialisation/deinitialisation based on user preference * Fix tests * TODO issue; Changelog * Fail only in debug mode * Don't observe telemetry state changes for now * Address PR comments; Add tests
1 parent 9e5e7ce commit 00bc5e7

File tree

10 files changed

+692
-135
lines changed

10 files changed

+692
-135
lines changed

‎changelog/unreleased/bugfixes/7755.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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‎

Lines changed: 29 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ import com.mapbox.navigation.core.routerefresh.RouteRefreshController
9696
import com.mapbox.navigation.core.routerefresh.RouteRefreshControllerProvider
9797
import com.mapbox.navigation.core.sensor.SensorData
9898
import com.mapbox.navigation.core.sensor.UpdateExternalSensorDataCallback
99-
import com.mapbox.navigation.core.telemetry.MapboxNavigationTelemetry
99+
import com.mapbox.navigation.core.telemetry.TelemetryWrapper
100100
import com.mapbox.navigation.core.telemetry.events.FeedbackEvent
101101
import com.mapbox.navigation.core.telemetry.events.FeedbackHelper
102102
import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata
@@ -126,8 +126,6 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonObserver
126126
import com.mapbox.navigation.core.trip.session.eh.GraphAccessor
127127
import com.mapbox.navigation.core.trip.session.eh.RoadObjectMatcher
128128
import com.mapbox.navigation.core.trip.session.eh.RoadObjectsStore
129-
import com.mapbox.navigation.metrics.MapboxMetricsReporter
130-
import com.mapbox.navigation.metrics.internal.TelemetryUtilsDelegate
131129
import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator
132130
import com.mapbox.navigation.navigator.internal.NavigatorLoader
133131
import com.mapbox.navigation.navigator.internal.router.RouterInterfaceAdapter
@@ -255,11 +253,15 @@ private const val MAPBOX_NOTIFICATION_ACTION_CHANNEL = "notificationActionButton
255253
class MapboxNavigation @VisibleForTesting internal constructor(
256254
val navigationOptions: NavigationOptions,
257255
private val threadController: ThreadController,
256+
private val telemetryWrapper: TelemetryWrapper = TelemetryWrapper(),
258257
) {
259258

260-
constructor(navigationOptions: NavigationOptions) : this(navigationOptions, ThreadController())
259+
constructor(navigationOptions: NavigationOptions) : this(
260+
navigationOptions,
261+
ThreadController(),
262+
TelemetryWrapper(),
263+
)
261264

262-
private val accessToken: String? = navigationOptions.accessToken
263265
private val mainJobController = threadController.getMainScopeAndRootJob()
264266
private val directionsSession: DirectionsSession
265267
private var navigator: MapboxNativeNavigator
@@ -473,7 +475,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
473475
val result = MapboxModuleProvider.createModule<Router>(MapboxModuleType.NavigationRouter) {
474476
paramsProvider(
475477
ModuleParams.NavigationRouter(
476-
accessToken
478+
navigationOptions.accessToken
477479
?: throw RuntimeException(MAPBOX_NAVIGATION_TOKEN_EXCEPTION_ROUTER),
478480
nativeRouter,
479481
threadController,
@@ -568,25 +570,11 @@ class MapboxNavigation @VisibleForTesting internal constructor(
568570
arrivalProgressObserver
569571
)
570572

571-
ifNonNull(accessToken) { token ->
572-
runInTelemetryContext { telemetry ->
573-
logD(
574-
"MapboxMetricsReporter.init from MapboxNavigation main",
575-
telemetry.LOG_CATEGORY
576-
)
577-
MapboxMetricsReporter.init(
578-
navigationOptions.applicationContext,
579-
token,
580-
obtainUserAgent()
581-
)
582-
MapboxMetricsReporter.toggleLogging(navigationOptions.isDebugLoggingEnabled)
583-
telemetry.initialize(
584-
this,
585-
navigationOptions,
586-
MapboxMetricsReporter,
587-
)
588-
}
589-
}
573+
telemetryWrapper.initialize(
574+
mapboxNavigation = this,
575+
navigationOptions = navigationOptions,
576+
userAgent = obtainUserAgent(),
577+
)
590578

591579
val routeOptionsProvider = RouteOptionsUpdater()
592580

@@ -1298,9 +1286,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
12981286
@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
12991287
routeRefreshController.destroy()
13001288
routesPreviewController.unregisterAllRoutesPreviewObservers()
1301-
runInTelemetryContext { telemetry ->
1302-
telemetry.destroy(this@MapboxNavigation)
1303-
}
1289+
telemetryWrapper.destroy()
13041290
threadController.cancelAllNonUICoroutines()
13051291
threadController.cancelAllUICoroutines()
13061292
ifNonNull(reachabilityObserverId) {
@@ -1756,17 +1742,15 @@ class MapboxNavigation @VisibleForTesting internal constructor(
17561742
feedbackMetadata: FeedbackMetadata?,
17571743
userFeedbackCallback: UserFeedbackCallback?,
17581744
) {
1759-
runInTelemetryContext { telemetry ->
1760-
telemetry.postUserFeedback(
1761-
feedbackType,
1762-
description,
1763-
feedbackSource,
1764-
screenshot,
1765-
feedbackSubType,
1766-
feedbackMetadata,
1767-
userFeedbackCallback,
1768-
)
1769-
}
1745+
telemetryWrapper.postUserFeedback(
1746+
feedbackType,
1747+
description,
1748+
feedbackSource,
1749+
screenshot,
1750+
feedbackSubType,
1751+
feedbackMetadata,
1752+
userFeedbackCallback,
1753+
)
17701754
}
17711755

17721756
@ExperimentalPreviewMapboxNavigationAPI
@@ -1775,13 +1759,11 @@ class MapboxNavigation @VisibleForTesting internal constructor(
17751759
@CustomEvent.Type customEventType: String,
17761760
customEventVersion: String,
17771761
) {
1778-
runInTelemetryContext { telemetry ->
1779-
telemetry.postCustomEvent(
1780-
payload = payload,
1781-
customEventType = customEventType,
1782-
customEventVersion = customEventVersion
1783-
)
1784-
}
1762+
telemetryWrapper.postCustomEvent(
1763+
payload = payload,
1764+
customEventType = customEventType,
1765+
customEventVersion = customEventVersion
1766+
)
17851767
}
17861768

17871769
/**
@@ -1794,9 +1776,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
17941776
*/
17951777
@ExperimentalPreviewMapboxNavigationAPI
17961778
fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper =
1797-
runInTelemetryContext { telemetry ->
1798-
telemetry.provideFeedbackMetadataWrapper()
1799-
} ?: throw java.lang.IllegalStateException(
1779+
telemetryWrapper.provideFeedbackMetadataWrapper() ?: throw java.lang.IllegalStateException(
18001780
"To get FeedbackMetadataWrapper Telemetry must be enabled"
18011781
)
18021782

@@ -2170,14 +2150,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
21702150
}
21712151
}
21722152

2173-
private inline fun <T> runInTelemetryContext(func: (MapboxNavigationTelemetry) -> T): T? {
2174-
return if (TelemetryUtilsDelegate.getEventsCollectionState()) {
2175-
func(MapboxNavigationTelemetry)
2176-
} else {
2177-
null
2178-
}
2179-
}
2180-
21812153
private fun obtainUserAgent(): String {
21822154
return "$MAPBOX_NAVIGATION_USER_AGENT_BASE/${BuildConfig.MAPBOX_NAVIGATION_VERSION_NAME}"
21832155
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package com.mapbox.navigation.core
22

33
import androidx.annotation.UiThread
4+
import androidx.annotation.VisibleForTesting
45
import com.mapbox.navigation.base.options.NavigationOptions
6+
import com.mapbox.navigation.core.telemetry.TelemetryWrapper
7+
import com.mapbox.navigation.utils.internal.ThreadController
58

69
/**
710
* Singleton responsible for ensuring there is only one MapboxNavigation instance.
@@ -33,6 +36,22 @@ object MapboxNavigationProvider {
3336
return mapboxNavigation!!
3437
}
3538

39+
@VisibleForTesting
40+
internal fun create(
41+
navigationOptions: NavigationOptions,
42+
threadController: ThreadController = ThreadController(),
43+
telemetryWrapper: TelemetryWrapper = TelemetryWrapper()
44+
): MapboxNavigation {
45+
mapboxNavigation?.onDestroy()
46+
mapboxNavigation = MapboxNavigation(
47+
navigationOptions,
48+
threadController,
49+
telemetryWrapper,
50+
)
51+
52+
return mapboxNavigation!!
53+
}
54+
3655
/**
3756
* Retrieve MapboxNavigation instance. Should be called after [create].
3857
*

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,10 @@ The class must be initialized before any telemetry events are reported.
151151
Attempting to use telemetry before initialization is called will throw an exception.
152152
Initialization may be called multiple times, the call is idempotent.
153153
The class has two public methods, postUserFeedback() and initialize().
154+
155+
TODO(NAVAND-1820) refactor this class. It's hard to test because of statics.
154156
*/
155-
internal object MapboxNavigationTelemetry {
157+
internal object MapboxNavigationTelemetry : SdkTelemetry {
156158
internal const val LOG_CATEGORY = "MapboxNavigationTelemetry"
157159

158160
private const val ONE_SECOND = 1000
@@ -354,10 +356,14 @@ internal object MapboxNavigationTelemetry {
354356
log("Valid initialization")
355357
}
356358

357-
fun destroy(mapboxNavigation: MapboxNavigation) {
359+
override fun destroy(mapboxNavigation: MapboxNavigation) {
358360
telemetryStop()
361+
362+
// TODO(NAVAND-1820) MapboxMetricsReporter is destroyed here,
363+
// but initialized separately from MapboxNavigationTelemetry
359364
log("MapboxMetricsReporter disable")
360365
MapboxMetricsReporter.disable()
366+
361367
mapboxNavigation.run {
362368
unregisterLocationObserver(locationsCollector)
363369
unregisterRouteProgressObserver(routeProgressObserver)
@@ -421,7 +427,7 @@ internal object MapboxNavigationTelemetry {
421427
userFeedbackCallbacks.remove(userFeedbackCallback)
422428
}
423429

424-
fun postCustomEvent(
430+
override fun postCustomEvent(
425431
payload: String,
426432
customEventType: String,
427433
customEventVersion: String
@@ -437,7 +443,7 @@ internal object MapboxNavigationTelemetry {
437443
}
438444

439445
@ExperimentalPreviewMapboxNavigationAPI
440-
fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper {
446+
override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper {
441447
(telemetryState as? NavTelemetryState.Running)?.sessionMetadata?.let { sessionMetadata ->
442448
return FeedbackMetadataWrapper(
443449
sessionMetadata.navigatorSessionIdentifier,
@@ -462,7 +468,7 @@ internal object MapboxNavigationTelemetry {
462468
}
463469

464470
@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
465-
fun postUserFeedback(
471+
override fun postUserFeedback(
466472
feedbackType: String,
467473
description: String,
468474
@FeedbackEvent.Source feedbackSource: String,

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package com.mapbox.navigation.core.telemetry
2+
3+
import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
4+
import com.mapbox.navigation.core.MapboxNavigation
5+
import com.mapbox.navigation.core.internal.telemetry.CustomEvent
6+
import com.mapbox.navigation.core.internal.telemetry.UserFeedbackCallback
7+
import com.mapbox.navigation.core.telemetry.events.FeedbackEvent
8+
import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata
9+
import com.mapbox.navigation.core.telemetry.events.FeedbackMetadataWrapper
10+
11+
internal interface SdkTelemetry {
12+
13+
fun postCustomEvent(
14+
payload: String,
15+
@CustomEvent.Type customEventType: String,
16+
customEventVersion: String,
17+
)
18+
19+
@ExperimentalPreviewMapboxNavigationAPI
20+
fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper?
21+
22+
@ExperimentalPreviewMapboxNavigationAPI
23+
fun postUserFeedback(
24+
feedbackType: String,
25+
description: String,
26+
@FeedbackEvent.Source feedbackSource: String,
27+
screenshot: String?,
28+
feedbackSubType: Array<String>?,
29+
feedbackMetadata: FeedbackMetadata?,
30+
userFeedbackCallback: UserFeedbackCallback?,
31+
)
32+
33+
fun destroy(mapboxNavigation: MapboxNavigation)
34+
35+
companion object {
36+
37+
val EMPTY = object : SdkTelemetry {
38+
override fun postCustomEvent(
39+
payload: String,
40+
customEventType: String,
41+
customEventVersion: String,
42+
) {
43+
// do nothing
44+
}
45+
46+
@ExperimentalPreviewMapboxNavigationAPI
47+
override fun provideFeedbackMetadataWrapper(): FeedbackMetadataWrapper? = null
48+
49+
@ExperimentalPreviewMapboxNavigationAPI
50+
override fun postUserFeedback(
51+
feedbackType: String,
52+
description: String,
53+
feedbackSource: String,
54+
screenshot: String?,
55+
feedbackSubType: Array<String>?,
56+
feedbackMetadata: FeedbackMetadata?,
57+
userFeedbackCallback: UserFeedbackCallback?,
58+
) {
59+
// do nothing
60+
}
61+
62+
override fun destroy(mapboxNavigation: MapboxNavigation) {
63+
// do nothing
64+
}
65+
}
66+
}
67+
}

0 commit comments

Comments
 (0)