Don't send events if telemetry is disabled

[VysotskiVadim]

TelemetryUtilsDelegate.getEventsCollectionState() can change value in runtime and some of our customers do change it.

The switch between true to false while mapbox navigation is alive caused 2 problems:

1. MapboxMetricsReporter logged events when telemetry is turned on. This caused the SDK to always get error.
2. MapboxNavigation didn't stop telemetry in MapboxNavigation.onDestroy

Now telemetry always works, but it doesn't send events if disabled

[VysotskiVadim]

@RingerJK , @LukasPaczos , do you guys think that we need to reconsider approach with telemetry initialisation/destruction as we now know that customers can disable telemetry in runtime using this

mapbox-navigation-android/libnavigation-metrics/src/main/java/com/mapbox/navigation/metrics/internal/TelemetryUtilsDelegate.kt

Line 9 in a2bf593

fun setEventsCollectionState(enableCollection: Boolean) {

?

[LukasPaczos]

@VysotskiVadim we should also refactor EventsServiceObserver to not log the whole events as they could contain routes and other big objects. We should perhaps only log types or other relevant but concise info.

[RingerJK]

we should refactor EventsServiceObserver that it's not log if EventsService is disabled as well

[codecov]

Codecov Report

Merging #6512 (fa03557) into main (934e668) will decrease coverage by 0.02%.
The diff coverage is 14.28%.

@@             Coverage Diff              @@
##               main    #6512      +/-   ##
============================================
- Coverage     70.33%   70.31%   -0.03%     
  Complexity     4909     4909              
============================================
  Files           717      718       +1     
  Lines         27951    27959       +8     
  Branches       3295     3297       +2     
============================================
  Hits          19660    19660              
- Misses         7013     7021       +8     
  Partials       1278     1278              

Impacted Files	Coverage Δ
...apbox/navigation/base/internal/metric/ValueUtil.kt	0.00% <0.00%> (ø)
...mapbox/navigation/metrics/MapboxMetricsReporter.kt	46.55% <16.66%> (-5.38%)	⬇️

[dzinad]

I didn't see the corresponding addition of flag check in MapboxMetricsReporter. Is this intentional?
Same for toggleLogging and disable.

[VysotskiVadim]

sorry, didn't get this one

[LukasPaczos]

I think that's okay in this context. We shouldn't if-check on init because someone could flip the global setting back at any point in time. And the TelemetryUtils.setEventsCollectionState also disables the telemetry service under the hood so we don't need to check for that.

[LukasPaczos]

All the processing in MapboxNavigationTelemetry would now be running even if telemetry collection is disabled. Could we improve that control to avoid the unnecessary processing?

[VysotskiVadim]

that's a fair point. I see following options:

1. Keep as is

pros:

• telemetry events are sent whenever the SDK client turns off and on telemetry globally
• simple code

cons:

• MapboxNavigationTelemetry works but events aren't sent when telemetry is disabled

2. Initiaize MapboxNavigationTelemetry only when telemetry is turned on, always disable on destroy

pros:

• MapboxNavigationTelemetry doesn't work when telemetry is disabled globally
• simple code

cons:

• telemetry events won't be sent if telemetry was turned off globally during initialization and then turned on in runtime

3. keep current telemetry state (enabled/disabled) on the sdk side, observe it and start/stop MapboxNavigationTelemetry

pros:

• telemetry events are sent whenever the SDK client turns off and on telemetry globally
• MapboxNavigationTelemetry doesn't work when telemetry is disabled globally

cons:

• complex code

I proffer the option 1

It doesn't seem that MapboxNavigationTelemetry consumes a lot of resources while working, so I think option 1 is a reasonable tradeoff for us. @LukasPaczos , WDYT?

[LukasPaczos]

I'd be in favor of option 2 that matches the existing behavior (avoid processing) and we already have NAVAND-355 that tracks the runtime enable/disable changes which we can tackle separately.

[VysotskiVadim]

Ok, let's keep current telemetry initialisation as is and improve it in NAVAND-355. Reverted cba98ab