[full ci] feat: capture breadcrumbs for OkHttp requests

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * Capture breadcrumbs for OkHttp network requests
   [#1358](https://github.com/bugsnag/bugsnag-android/pull/1358)
+  [#1361](https://github.com/bugsnag/bugsnag-android/pull/1361)
 
 * Update project to build using Gradle/AGP 7
   [#1354](https://github.com/bugsnag/bugsnag-android/pull/1354)

bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt

@@ -52,7 +52,8 @@ data class ImmutableConfig(
 
     // results cached here to avoid unnecessary lookups in Client.
     val packageInfo: PackageInfo?,
-    val appInfo: ApplicationInfo?
+    val appInfo: ApplicationInfo?,
+    val redactedKeys: Collection<String>
 ) {
 
     @JvmName("getErrorApiDeliveryParams")
@@ -162,7 +163,8 @@ internal fun convertToImmutableConfig(
         persistenceDirectory = persistenceDir,
         sendLaunchCrashesSynchronously = config.sendLaunchCrashesSynchronously,
         packageInfo = packageInfo,
-        appInfo = appInfo
+        appInfo = appInfo,
+        redactedKeys = config.redactedKeys
     )
 }
 

bugsnag-plugin-android-okhttp/src/main/java/com/bugsnag/android/Redaction.kt

@@ -0,0 +1,12 @@
+package com.bugsnag.android
+
+internal fun Client.redactMap(data: Map<String, String?>): Map<String, String?> {
+    return data.mapValues { entry ->
+        val redactedKeys = config.redactedKeys
+
+        when {
+            redactedKeys.contains(entry.key) -> "[REDACTED]"
+            else -> entry.value
+        }
+    }
+}

bugsnag-plugin-android-okhttp/src/main/java/com/bugsnag/android/okhttp/BugsnagOkHttpPlugin.kt

@@ -1,14 +1,147 @@
 package com.bugsnag.android.okhttp
 
+import androidx.annotation.VisibleForTesting
+import com.bugsnag.android.BreadcrumbType
 import com.bugsnag.android.Client
 import com.bugsnag.android.Plugin
+import com.bugsnag.android.redactMap
+import okhttp3.Call
 import okhttp3.EventListener
+import okhttp3.Request
+import okhttp3.Response
+import java.io.IOException
+import java.util.concurrent.ConcurrentHashMap
 
-class BugsnagOkHttpPlugin : Plugin, EventListener() {
+/**
+ * see https://square.github.io/okhttp/events/
+ */
+// TODO improve docs
+class BugsnagOkHttpPlugin @JvmOverloads constructor(
+    internal val timeProvider: () -> Long = { System.currentTimeMillis() }
+) : Plugin, EventListener() {
+
+    internal val requestMap = ConcurrentHashMap<Call, NetworkRequestMetadata>()
+    private var client: Client? = null
 
     override fun load(client: Client) {
+        this.client = client
     }
 
     override fun unload() {
+        this.client = null
+    }
+
+    override fun callStart(call: Call) {
+        requestMap[call] = NetworkRequestMetadata(timeProvider())
+    }
+
+    override fun canceled(call: Call) {
+        requestMap.remove(call)
+    }
+
+    // TODO confirm that this works well with OkHttp's concept of retries (e.g. redirects)
+    override fun requestBodyEnd(call: Call, byteCount: Long) {
+        requestMap[call]?.requestBodyCount = byteCount
+    }
+
+    override fun responseBodyEnd(call: Call, byteCount: Long) {
+        requestMap[call]?.responseBodyCount = byteCount
+    }
+
+    override fun responseHeadersEnd(call: Call, response: Response) {
+        requestMap[call]?.status = response.code
+    }
+
+    override fun callEnd(call: Call) {
+        requestMap.remove(call)?.let { requestInfo ->
+            client?.apply {
+                leaveBreadcrumb(
+                    "OkHttp call succeeded",
+                    collateMetadata(call, requestInfo, timeProvider()),
+                    BreadcrumbType.REQUEST
+                )
+            }
+        }
+    }
+
+    override fun callFailed(call: Call, ioe: IOException) {
+        requestMap.remove(call)?.let { requestInfo ->
+            client?.apply {
+                leaveBreadcrumb(
+                    "OkHttp call failed",
+                    collateMetadata(call, requestInfo, timeProvider()),
+                    BreadcrumbType.REQUEST
+                )
+            }
+        }
     }
+
+    @VisibleForTesting
+    internal fun Client.collateMetadata(
+        call: Call,
+        info: NetworkRequestMetadata,
+        nowMs: Long
+    ): Map<String, Any> {
+        val request = call.request()
+
+        return mapOf(
+            "method" to request.method,
+            "url" to sanitizeUrl(request),
+            "duration" to nowMs - info.startTime,
+            "urlParams" to buildQueryParams(request),
+
+            // TODO test that these are skipped if a call fails or a request/response has no body.
+            "requestContentLength" to info.requestBodyCount,
+            "responseContentLength" to info.responseBodyCount,
+            "status" to info.status
+        )
+    }
+
+    /**
+     * Constructs a map of query parameters, redacting any sensitive values.
+     */
+    private fun Client.buildQueryParams(request: Request): Map<String, String?> {
+        val url = request.url
+        val params = mutableMapOf<String, String?>()
+
+        url.queryParameterNames.forEach { name ->
+            params[name] = url.queryParameter(name)
+        }
+        return redactMap(params)
+    }
+
+    /**
+     * Sanitizes the URL by removing query params.
+     */
+    private fun sanitizeUrl(request: Request): String {
+        val url = request.url
+        val builder = url.newBuilder()
+
+        url.queryParameterNames.forEach { name ->
+            builder.removeAllQueryParameters(name)
+        }
+        return builder.build().toString()
+    }
+}
+
+/**
+ * Stores stateful information about the in-flight network request, and contains functions
+ * that construct breadcrumb metadata from this information.
+ */
+internal class NetworkRequestMetadata(
+    @JvmField
+    val startTime: Long
+) {
+
+    @JvmField
+    @Volatile
+    var status: Int = -1
+
+    @JvmField
+    @Volatile
+    var requestBodyCount: Long = -1
+
+    @JvmField
+    @Volatile
+    var responseBodyCount: Long = -1
 }

bugsnag-plugin-android-okhttp/src/test/java/com/bugsnag/android/BugsnagOkHttpPluginTest.kt

@@ -0,0 +1,190 @@
+package com.bugsnag.android
+
+import com.bugsnag.android.okhttp.BugsnagOkHttpPlugin
+import okhttp3.Call
+import okhttp3.Protocol
+import okhttp3.Request
+import okhttp3.Response
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.Mockito.`when`
+import org.mockito.Mockito.anyMap
+import org.mockito.Mockito.eq
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.junit.MockitoJUnitRunner
+import java.io.IOException
+import java.util.concurrent.atomic.AtomicLong
+
+@RunWith(MockitoJUnitRunner::class)
+class BugsnagOkHttpPluginTest {
+
+    @Mock
+    lateinit var client: Client
+
+    @Mock
+    lateinit var call: Call
+
+    @Before
+    fun setup() {
+        `when`(client.config).thenReturn(BugsnagTestUtils.generateImmutableConfig())
+    }
+
+    @Test
+    fun unloadedClientCallFailed() {
+        BugsnagOkHttpPlugin().apply {
+            load(client)
+            callStart(call)
+            unload()
+            callFailed(call, IOException())
+        }
+        verify(client, times(0)).leaveBreadcrumb(
+            eq("OkHttp call failed"),
+            anyMap(),
+            eq(BreadcrumbType.REQUEST)
+        )
+    }
+
+    @Test
+    fun unloadedClientCallEnd() {
+        BugsnagOkHttpPlugin().apply {
+            load(client)
+            callStart(call)
+            unload()
+            callEnd(call)
+        }
+        verify(client, times(0)).leaveBreadcrumb(
+            eq("OkHttp call succeeded"),
+            anyMap(),
+            eq(BreadcrumbType.REQUEST)
+        )
+    }
+
+    @Test
+    fun loadedClientCallFailed() {
+        val request = Request.Builder().url("https://example.com?debug=true").build()
+        `when`(call.request()).thenReturn(request)
+
+        val duration = AtomicLong()
+        val plugin = BugsnagOkHttpPlugin { duration.incrementAndGet() }.apply {
+            load(client)
+            callStart(call)
+            callFailed(call, IOException())
+        }
+        val map = mapOf(
+            "method" to "GET",
+            "url" to "https://example.com/",
+            "urlParams" to mapOf(
+                "debug" to "true"
+            ),
+            "duration" to 1L,
+            "requestContentLength" to -1L,
+            "responseContentLength" to -1L,
+            "status" to -1
+        )
+        verify(client, times(1)).leaveBreadcrumb(
+            "OkHttp call failed",
+            map,
+            BreadcrumbType.REQUEST
+        )
+        assertNull(plugin.requestMap[call])
+    }
+
+    @Test
+    fun loadedClientCallEnd() {
+        val request = Request.Builder().url("https://example.com?debug=true").build()
+        `when`(call.request()).thenReturn(request)
+
+        val duration = AtomicLong()
+        val plugin = BugsnagOkHttpPlugin { duration.incrementAndGet() }.apply {
+            load(client)
+            callStart(call)
+            callEnd(call)
+        }
+        val map = mapOf(
+            "method" to "GET",
+            "url" to "https://example.com/",
+            "urlParams" to mapOf(
+                "debug" to "true"
+            ),
+            "duration" to 1L,
+            "requestContentLength" to -1L,
+            "responseContentLength" to -1L,
+            "status" to -1
+        )
+        verify(client, times(1)).leaveBreadcrumb(
+            "OkHttp call succeeded",
+            map,
+            BreadcrumbType.REQUEST
+        )
+        assertNull(plugin.requestMap[call])
+    }
+
+    @Test
+    fun callCancelled() {
+        val plugin = BugsnagOkHttpPlugin()
+        plugin.apply {
+            load(client)
+            assertNull(requestMap[call])
+
+            callStart(call)
+            assertNotNull(requestMap[call])
+            assertTrue(requireNotNull(requestMap[call]).startTime > 0)
+
+            canceled(call)
+            assertNull(requestMap[call])
+        }
+    }
+
+    @Test
+    fun requestContentLength() {
+        val plugin = BugsnagOkHttpPlugin()
+        plugin.apply {
+            load(client)
+            callStart(call)
+            val info = requireNotNull(requestMap[call])
+            assertEquals(-1, info.requestBodyCount)
+            requestBodyEnd(call, 2340)
+            assertEquals(2340, info.requestBodyCount)
+        }
+    }
+
+    @Test
+    fun responseContentLength() {
+        val plugin = BugsnagOkHttpPlugin()
+        plugin.apply {
+            load(client)
+            callStart(call)
+            val info = requireNotNull(requestMap[call])
+            assertEquals(-1, info.responseBodyCount)
+            responseBodyEnd(call, 5092)
+            assertEquals(5092, info.responseBodyCount)
+        }
+    }
+
+    @Test
+    fun responseStatus() {
+        val plugin = BugsnagOkHttpPlugin()
+        val request = Request.Builder().url("https://example.com").build()
+        val response = Response.Builder()
+            .message("Test")
+            .protocol(Protocol.HTTP_2)
+            .request(request).code(200)
+            .build()
+
+        plugin.apply {
+            load(client)
+            callStart(call)
+            val info = requireNotNull(requestMap[call])
+            assertEquals(-1, info.status)
+            responseHeadersEnd(call, response)
+            assertEquals(200, info.status)
+        }
+    }
+}