Sunset telemetry service #351
Conversation
This makes MapboxTelemetry free of circular dependencies
| new HandlerThread("LocationSettingsChangeThread"), | ||
| applicationContext.getSharedPreferences(MAPBOX_SHARED_PREFERENCES, Context.MODE_PRIVATE), | ||
| // Provide empty token as it is not available yet | ||
| new MapboxTelemetry(applicationContext, "", LOCATION_COLLECTOR_USER_AGENT)); |
There was a problem hiding this comment.
I am confused here as MapboxTelemetry is the API class we provide for MapSdk and AutoSDK, they will create a instance of MapboxTelemetry to push gesture events etc, but we create another instance here.
There was a problem hiding this comment.
MapboxTelemetry is no longer coupled with location collection and hence becomes responsible for eventing only. You can treat LocationCollectionClient as producer of location events, which are delivered to the api-events endpoint via MapboxTelemetry.
|
|
||
| import static com.mapbox.android.telemetry.location.LocationCollectionClient.DEFAULT_SESSION_ROTATION_INTERVAL_HOURS; | ||
|
|
||
| public class MapboxTelemetryInitProvider extends ContentProvider { |
There was a problem hiding this comment.
Why do we need a ContentProvider here? Just for init?
There was a problem hiding this comment.
yes, it's a hacky way to execute your code at app startup
Changing implementation details around how session id is managed due to limitation with what can be shared with location pending intent.
Codecov Report
@@ Coverage Diff @@
## master #351 +/- ##
============================================
+ Coverage 54.67% 56.73% +2.06%
+ Complexity 516 491 -25
============================================
Files 107 102 -5
Lines 3367 3190 -177
Branches 227 219 -8
============================================
- Hits 1841 1810 -31
+ Misses 1435 1293 -142
+ Partials 91 87 -4 |
| new HandlerThread("LocationSettingsChangeThread"), | ||
| new SessionIdentifier(defaultInterval), | ||
| applicationContext.getSharedPreferences(MAPBOX_SHARED_PREFERENCES, Context.MODE_PRIVATE), |
There was a problem hiding this comment.
We should use TelemetryUtils.obtainSharedPreferences here
There was a problem hiding this comment.
It is intentional: TelemetryUtils should not be public, the plan is to downgrade its visibility to package private at some point. Hence i prefer to avoid creating any dependencies on it going forward.
There was a problem hiding this comment.
Ok, but keep in mind we may have strict mode issue here.
There was a problem hiding this comment.
Are you suggesting to access it on the background thread?
There was a problem hiding this comment.
Yeah, my thought is that we could create a sharedPreference later and put all background threads in it.
There was a problem hiding this comment.
i need shared preferences at startup but i will make sure there's no impact on cold start by deferring init to background thread.
api-events returns 402 when applicationState is an empty string.
This refactor migrates location traces collection feature from being backed by Android
Service.Telemetry location traces collection relies on
Serviceto do background work, this approach no longer works on apps that target Android 8.0 (API level 26) or higher due to OS limitations imposed on what apps can do while running in the background: https://developer.android.com/about/versions/oreo/background.Refactor objectives: