Expose public API Telemetry #11503
Expose public API Telemetry #11503
Conversation
| @@ -30,10 +30,20 @@ public static void initialize() { | |||
| } | |||
|
|
|||
| private static class EventsHolder { | |||
There was a problem hiding this comment.
Can we rename this one to TelemetryHolder?
There was a problem hiding this comment.
We should expose the updateDebugLoggingEnabled and updateSessionIdRotationInterval methods to developers.
| } | ||
|
|
||
| static MapboxTelemetry obtainTelemetry() { | ||
| return EventsHolder.INSTANCE.telemetry; | ||
| } | ||
|
|
||
| public static void enable(){ |
There was a problem hiding this comment.
This could be package private because we're currently giving the option to opt in through the AttributionDialogManager.
There was a problem hiding this comment.
That is for opt'ing out as an end user, not the developer, the previous telem integration provided a way to disable telem programmatically.
There was a problem hiding this comment.
Capturing from internal convo that disabling can be done through a manifest tag, will remove the api.
| EventsHolder.INSTANCE.telemetry.enable(); | ||
| } | ||
|
|
||
| public static void disable() { |
There was a problem hiding this comment.
This could be package private because we're currently giving the option to opt out through the AttributionDialogManager.
tobrun
force-pushed
the
11493-telem-public-api
branch
2 times, most recently
from
e274ff1
to
318db23
Compare
| @@ -46,7 +46,8 @@ public static synchronized Mapbox getInstance(@NonNull Context context, @NonNull | |||
| Context appContext = context.getApplicationContext(); | |||
| INSTANCE = new Mapbox(appContext, accessToken); | |||
|
|
|||
| Events.initialize(); | |||
| Telemetry.initialize(); | |||
| Telemetry.updateDebugLoggingEnabled(true); | |||
There was a problem hiding this comment.
Are we enabling logging by default?
There was a problem hiding this comment.
this should have gone into the testapp not the sdk
There was a problem hiding this comment.
What do you mean? This is the SDK 🤔
| import com.mapbox.android.telemetry.TelemetryEnabler; | ||
| import com.mapbox.mapboxsdk.BuildConfig; | ||
| import com.mapbox.mapboxsdk.Mapbox; | ||
|
|
||
| public class Events { | ||
| public class Telemetry { |
There was a problem hiding this comment.
Out of curiosity, why did we change the class name?
There was a problem hiding this comment.
Events doesn't expose the purpose of the class in context of the Maps sdk, it's too generic, Telemetry is clear and straightforward.
tobrun
force-pushed
the
11493-telem-public-api
branch
from
318db23
to
b10157a
Compare
Closes #11493