Merge pull request #1701 from bugsnag/PLAT-8556-telemetry

Add telemetry controls for internal reports.

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## TBD
 
+### Enhancements
+
+* Added configuration option to control whether internal errors are sent to Bugsnag
+  [#1701](https://github.com/bugsnag/bugsnag-android/pull/1701)
+
 ### Bug fixes
 
 * Fixed Bugsnag interactions with the Google ANR handler on newer versions of Android

bugsnag-android-core/detekt-baseline.xml

@@ -7,10 +7,11 @@
     <ID>LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: SessionTracker, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState )</ID>
     <ID>LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? )</ID>
     <ID>LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? )</ID>
-    <ID>LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceId: String?, memoryTrimState: MemoryTrimState )</ID>
+    <ID>LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceId: String?, internalDeviceId: String?, memoryTrimState: MemoryTrimState )</ID>
     <ID>LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array&lt;String>?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ runtimeVersions: MutableMap&lt;String, Any>? )</ID>
     <ID>LongParameterList:DeviceBuildInfo.kt$DeviceBuildInfo$( val manufacturer: String?, val model: String?, val osVersion: String?, val apiLevel: Int?, val osBuild: String?, val fingerprint: String?, val tags: String?, val brand: String?, val cpuAbis: Array&lt;String>? )</ID>
-    <ID>LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, rootDetector: RootDetector, private val bgTaskService: BackgroundTaskService, private val logger: Logger )</ID>
+    <ID>LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceId: String?, private val internalDeviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, rootDetector: RootDetector, private val bgTaskService: BackgroundTaskService, private val logger: Logger )</ID>
+    <ID>LongParameterList:DeviceIdStore.kt$DeviceIdStore$( context: Context, deviceIdfile: File = File(context.filesDir, "device-id"), deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, internalDeviceIdfile: File = File(context.filesDir, "internal-device-id"), internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, private val sharedPrefMigrator: SharedPrefMigrator, logger: Logger )</ID>
     <ID>LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap&lt;String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? )</ID>
     <ID>LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null )</ID>
     <ID>LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, breadcrumbs: MutableList&lt;Breadcrumb> = mutableListOf(), discardClasses: Set&lt;String> = setOf(), errors: MutableList&lt;Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection&lt;String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList&lt;Thread> = mutableListOf(), user: User = User(), redactionKeys: Set&lt;String>? = null )</ID>
@@ -37,9 +38,10 @@
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exc: Exception) { false }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") }</ID>
-    <ID>SwallowedException:DeviceIdStore.kt$DeviceIdStore$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) }</ID>
+    <ID>SwallowedException:DeviceIdFilePersistence.kt$DeviceIdFilePersistence$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) }</ID>
     <ID>SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null }</ID>
     <ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware</ID>
+    <ID>TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector</ID>
     <ID>TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware</ID>
     <ID>UnnecessaryAbstractClass:DependencyModule.kt$DependencyModule</ID>
     <ID>UnusedPrivateMember:ThreadStateTest.kt$ThreadStateTest$private val configuration = generateImmutableConfig()</ID>

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DataCollectorTest.kt

@@ -25,6 +25,7 @@ class DataCollectorTest {
             Mockito.mock(Context::class.java),
             res,
             "fakeDevice",
+            "internalFakeDevice",
             Mockito.mock(DeviceBuildInfo::class.java),
             File("/tmp/javatest"),
             Mockito.mock(RootDetector::class.java),

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt

@@ -14,15 +14,21 @@ import java.util.concurrent.Executors
 internal class DeviceIdStoreTest {
 
     lateinit var file: File
+    lateinit var fileInternal: File
     lateinit var storageDir: File
     lateinit var ctx: Context
     lateinit var prefMigrator: SharedPrefMigrator
 
-    private val uuidProvider = {
-        UUID.fromString("ab0c1482-2ffe-11eb-adc1-0242ac120002")
-    }
-    private val diffUuidProvider = {
-        UUID.fromString("d9901bff-2ffe-11eb-adc1-0242ac120002")
+    private val prefsId = "55670bff-9024-fc0a-b392-0242ac88ccd9"
+    private val ids = listOf<String>(
+        "ab0c1482-2ffe-11eb-adc1-0242ac120002",
+        "d9901bff-2ffe-11eb-adc1-0242ac120002",
+        "103f88a2-2ffe-11eb-adc1-0242ac120002",
+        "8c55319d-2ffe-11eb-adc1-0242ac120002"
+    )
+
+    private fun uuidProvider(index: Int): () -> UUID {
+        return { UUID.fromString(ids[index]) }
     }
 
     @Before
@@ -32,6 +38,12 @@ internal class DeviceIdStoreTest {
         storageDir = ctx.cacheDir
         file = File(storageDir, "device.json")
         file.delete()
+        fileInternal = File(storageDir, "device_internal.json")
+        fileInternal.delete()
+        val context = ApplicationProvider.getApplicationContext<Context>()
+        val prefs =
+            context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE)
+        prefs.edit().remove("install.iud").commit()
     }
 
     /**
@@ -41,32 +53,43 @@ internal class DeviceIdStoreTest {
     fun nonExistentFile() {
         val nonExistentFile = File(storageDir, "foo")
         nonExistentFile.delete()
-        val store = DeviceIdStore(ctx, nonExistentFile, prefMigrator, NoopLogger)
-        val deviceId = store.loadDeviceId(uuidProvider)
-        requireNotNull(deviceId)
-        assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
+        val nonExistentInternalFile = File(storageDir, "foo_internal")
+        nonExistentInternalFile.delete()
+        val store = DeviceIdStore(
+            ctx,
+            nonExistentFile,
+            uuidProvider(0),
+            nonExistentInternalFile,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
+
+        assertEquals(ids[0], store.loadDeviceId()!!)
+        assertEquals(ids[1], store.loadInternalDeviceId()!!)
     }
 
     /**
      * An empty file should be overwritten with a new device ID
      */
     @Test
     fun emptyFile() {
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
-        val deviceId = store.loadDeviceId(uuidProvider)
-        requireNotNull(deviceId)
-        assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
-    }
+        file.delete()
+        assert(file.createNewFile())
+        fileInternal.delete()
+        assert(fileInternal.createNewFile())
+        val store = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(0),
+            fileInternal,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
 
-    /**
-     * A file of the incorrect length should be overwritten with a new device ID
-     */
-    @Test
-    fun incorrectFileLength() {
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
-        val deviceId = store.loadDeviceId(uuidProvider)
-        requireNotNull(deviceId)
-        assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
+        assertEquals(ids[0], store.loadDeviceId()!!)
+        assertEquals(ids[1], store.loadInternalDeviceId()!!)
     }
 
     /**
@@ -75,30 +98,54 @@ internal class DeviceIdStoreTest {
     @Test
     fun invalidFileContents() {
         file.writeText("{\"hamster\": 2}")
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
-        val deviceId = store.loadDeviceId(uuidProvider)
-        requireNotNull(deviceId)
-        assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
+        fileInternal.writeText("{\"hamster\": 2}")
+        val store = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(0),
+            fileInternal,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
+
+        assertEquals(ids[0], store.loadDeviceId()!!)
+        assertEquals(ids[1], store.loadInternalDeviceId()!!)
     }
 
     /**
      * A valid file should not be overwritten with a new device ID
      */
     @Test
     fun validFileContents() {
-        file.writeText("{\"id\": \"24c51482-2ffe-11eb-adc1-0242ac120002\"}")
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
+        file.writeText("{\"id\": \"${ids[0]}\"}")
+        fileInternal.writeText("{\"id\": \"${ids[1]}\"}")
+        val storeB = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(2),
+            fileInternal,
+            uuidProvider(3),
+            prefMigrator,
+            NoopLogger
+        )
+        val storeA = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(0),
+            fileInternal,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
 
         // device ID is loaded without writing file
-        assertEquals(
-            "24c51482-2ffe-11eb-adc1-0242ac120002",
-            store.loadDeviceId(uuidProvider)
-        )
+        assertEquals(ids[0], storeA.loadDeviceId()!!)
+        assertEquals(ids[1], storeA.loadInternalDeviceId()!!)
+
         // same device ID is retrieved as before
-        assertEquals(
-            "24c51482-2ffe-11eb-adc1-0242ac120002",
-            store.loadDeviceId(diffUuidProvider)
-        )
+        assertEquals(ids[0], storeB.loadDeviceId()!!)
+        assertEquals(ids[1], storeB.loadInternalDeviceId()!!)
     }
 
     /**
@@ -111,17 +158,38 @@ internal class DeviceIdStoreTest {
             createNewFile()
             setWritable(false)
         }
-        val store = DeviceIdStore(ctx, nonReadableFile, prefMigrator, NoopLogger)
-        val deviceId = store.loadDeviceId(uuidProvider)
-        assertNull(deviceId)
+        val nonReadableInternalFile = File(storageDir, "foo").apply {
+            delete()
+            createNewFile()
+            setWritable(false)
+        }
+        val store = DeviceIdStore(
+            ctx,
+            nonReadableFile,
+            uuidProvider(0),
+            nonReadableInternalFile,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
+        assertNull(store.loadDeviceId())
+        assertNull(store.loadInternalDeviceId())
     }
 
     /**
      * The device ID store should take out a file lock to prevent concurrent writes to the file.
      */
     @Test(timeout = 2000)
     fun fileLockUsed() {
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
+        val store = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(0),
+            fileInternal,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
 
         // load the device ID on many different background threads.
         // each thread races with each other, but only one should generate a device ID
@@ -133,9 +201,7 @@ internal class DeviceIdStoreTest {
 
         repeat(attempts) {
             executor.submit {
-                val id = store.loadDeviceId(uuidProvider)
-                requireNotNull(id)
-                deviceIds.add(id)
+                deviceIds.add(store.loadDeviceId()!!)
                 latch.countDown()
             }
         }
@@ -151,16 +217,24 @@ internal class DeviceIdStoreTest {
      */
     @Test
     fun sharedPrefMigration() {
-        val store = DeviceIdStore(ctx, file, prefMigrator, NoopLogger)
+        val store = DeviceIdStore(
+            ctx,
+            file,
+            uuidProvider(0),
+            fileInternal,
+            uuidProvider(1),
+            prefMigrator,
+            NoopLogger
+        )
         val context = ApplicationProvider.getApplicationContext<Context>()
 
         val prefs =
             context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE)
-        prefs.edit().putString("install.iud", "55670bff-9024-fc0a-b392-0242ac88ccd9").commit()
+        prefs.edit().putString("install.iud", prefsId).commit()
 
-        val prefDeviceId = SharedPrefMigrator(context).loadDeviceId()
-        val storeDeviceId = store.loadDeviceId()
-        assertEquals("55670bff-9024-fc0a-b392-0242ac88ccd9", storeDeviceId)
+        val prefDeviceId = SharedPrefMigrator(context).loadDeviceId(false)
+        val storeDeviceId = store.loadDeviceId()!!
+        assertEquals(prefsId, storeDeviceId)
         assertEquals(prefDeviceId, storeDeviceId)
     }
 }

bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java

@@ -170,7 +170,8 @@ public Unit invoke(Boolean hasConnection, String networkState) {
 
         DataCollectionModule dataCollectionModule = new DataCollectionModule(contextModule,
                 configModule, systemServiceModule, trackerModule,
-                bgTaskService, connectivity, storageModule.getDeviceId(), memoryTrimState);
+                bgTaskService, connectivity, storageModule.getDeviceId(),
+                storageModule.getInternalDeviceId(), memoryTrimState);
         dataCollectionModule.resolveDependencies(bgTaskService, TaskType.IO);
         appDataCollector = dataCollectionModule.getAppDataCollector();
         deviceDataCollector = dataCollectionModule.getDeviceDataCollector();

bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt

@@ -52,6 +52,7 @@ internal class ConfigInternal(
     var discardClasses: Set<String> = emptySet()
     var enabledReleaseStages: Set<String>? = null
     var enabledBreadcrumbTypes: Set<BreadcrumbType>? = null
+    var telemetry: Set<Telemetry> = setOf(Telemetry.INTERNAL_ERRORS)
     var projectPackages: Set<String> = emptySet()
     var persistenceDirectory: File? = null
 

bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java

@@ -746,6 +746,22 @@ public void setEnabledBreadcrumbTypes(@Nullable Set<BreadcrumbType> enabledBread
         impl.setEnabledBreadcrumbTypes(enabledBreadcrumbTypes);
     }
 
+    @NonNull
+    public Set<Telemetry> getTelemetry() {
+        return impl.getTelemetry();
+    }
+
+    /**
+     * Set which telemetry will be sent to Bugsnag. By default, all telemetry is enabled.
+     *
+     * The following telemetry can be enabled:
+     *
+     * - internal errors: Errors in the Bugsnag SDK itself.
+     */
+    public void setTelemetry(@NonNull Set<Telemetry> telemetry) {
+        impl.setTelemetry(telemetry);
+    }
+
     /**
      * Sets which package names Bugsnag should consider as a part of the
      * running application. We mark stacktrace lines as in-project if they

bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt

@@ -18,6 +18,7 @@ internal class DataCollectionModule(
     bgTaskService: BackgroundTaskService,
     connectivity: Connectivity,
     deviceId: String?,
+    internalDeviceId: String?,
     memoryTrimState: MemoryTrimState
 ) : DependencyModule() {
 
@@ -49,6 +50,7 @@ internal class DataCollectionModule(
             ctx,
             ctx.resources,
             deviceId,
+            internalDeviceId,
             deviceBuildInfo,
             dataDir,
             rootDetector,

bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt

@@ -27,6 +27,7 @@ internal class DeviceDataCollector(
     private val appContext: Context,
     resources: Resources,
     private val deviceId: String?,
+    private val internalDeviceId: String?,
     private val buildInfo: DeviceBuildInfo,
     private val dataDirectory: File,
     rootDetector: RootDetector,
@@ -88,6 +89,19 @@ internal class DeviceDataCollector(
         Date(now)
     )
 
+    fun generateInternalDeviceWithState(now: Long) = DeviceWithState(
+        buildInfo,
+        checkIsRooted(),
+        internalDeviceId,
+        locale,
+        totalMemoryFuture.runCatching { this?.get() }.getOrNull(),
+        runtimeVersions.toMutableMap(),
+        calculateFreeDisk(),
+        calculateFreeMemory(),
+        getOrientationAsString(),
+        Date(now)
+    )
+
     fun getDeviceMetadata(): Map<String, Any?> {
         val map = HashMap<String, Any?>()
         populateBatteryInfo(into = map)