fix: Okhttp v3 was not instrumented (#351)

getsentry · Jul 18, 2022 · 29752ad · 29752ad
1 parent ded781e
commit 29752ad
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 ### Fixes
 
 - Skip jar processing on AGP < 7.1.2 for signed multi release jars ([#334](https://github.com/getsentry/sentry-android-gradle-plugin/pull/334))
+- Fix `OkHttp` version `3.x` was not instrumented ([#351](https://github.com/getsentry/sentry-android-gradle-plugin/pull/351))
 
 ## 3.1.2
 

diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/okhttp/OkHttp.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/okhttp/OkHttp.kt
@@ -11,7 +11,7 @@ import org.objectweb.asm.ClassVisitor
 import org.objectweb.asm.MethodVisitor
 
 class OkHttp : ClassInstrumentable {
-    override val fqName: String get() = "okhttp3.internal.connection.RealCall"
+    override val fqName: String get() = "RealCall"
 
     override fun getVisitor(
         instrumentableContext: ClassContext,
@@ -25,6 +25,12 @@ class OkHttp : ClassInstrumentable {
         methodInstrumentables = listOf(ResponseWithInterceptorChain()),
         parameters = parameters
     )
+
+    // OkHttp has renamed the class in v4, hence we are looking for both old and new package names
+    // https://github.com/square/okhttp/commit/3d3b0f64005f7d2dd7cde80a9eaf665f8df86fb6#diff-e46bb5c1117393fbfb8cd1496fc4a2dcfcd6fcf70d065c50be83ce9215b2ec7b
+    override fun isInstrumentable(data: ClassContext): Boolean =
+        data.currentClassData.className == "okhttp3.internal.connection.RealCall" ||
+            data.currentClassData.className == "okhttp3.RealCall"
 }
 
 class ResponseWithInterceptorChain : MethodInstrumentable {

diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginTest.kt
@@ -234,6 +234,31 @@ class SentryPluginTest(
         }
     }
 
+    @Test
+    fun `instruments okhttp v3`() {
+        applyTracingInstrumentation(features = setOf(InstrumentationFeature.OKHTTP), debug = true)
+        appBuildFile.appendText(
+            // language=Groovy
+            """
+
+            dependencies {
+              implementation 'com.squareup.okhttp3:okhttp:3.14.9'
+              implementation 'io.sentry:sentry-android-okhttp:5.5.0'
+            }
+            """.trimIndent()
+        )
+
+        runner
+            .appendArguments(":app:assembleDebug")
+            .build()
+
+        // since it's an integration test, we just test that the log file was created for the class
+        // meaning our CommonClassVisitor has visited and instrumented it
+        val debugOutput =
+            testProjectDir.root.resolve("app/build/tmp/sentry/RealCall-instrumentation.log")
+        assertTrue { debugOutput.exists() && debugOutput.length() > 0 }
+    }
+
     private fun applyUploadNativeSymbols() {
         appBuildFile.writeText(
             // language=Groovy
@@ -275,7 +300,8 @@ class SentryPluginTest(
 
     private fun applyTracingInstrumentation(
         tracingInstrumentation: Boolean = true,
-        features: Set<InstrumentationFeature> = setOf()
+        features: Set<InstrumentationFeature> = setOf(),
+        debug: Boolean = false
     ) {
         appBuildFile.writeText(
             // language=Groovy
@@ -292,7 +318,9 @@ class SentryPluginTest(
                 sentry {
                   autoUploadProguardMapping = false
                   tracingInstrumentation {
+                    forceInstrumentDependencies = true
                     enabled = $tracingInstrumentation
+                    debug = $debug
                     features = ${
             features.joinToString(
                 prefix = "[",