Auto install and instrument sentry-okhttp instead of sentry-android-o…

…khttp on v7+ (#724) * removed sentry-android-okhttp module from autoInstall if Sentry v7+ * add sentry-okhttp module to autoInstall if Sentry v7+ * changed OkHttp instrumentation to use sentry-okhttp instead of sentry-android-okhttp if Sentry v7+
getsentry · Jun 21, 2024 · a50beb1 · a50beb1
1 parent bf1acca
commit a50beb1
Show file tree

Hide file tree

Showing 21 changed files with 306 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- Auto install and instrument sentry-okhttp instead of sentry-android-okhttp on v7+ ([#724](https://github.com/getsentry/sentry-android-gradle-plugin/pull/724))
 - Fix source bundles with configuration cache on AGP 8+ ([#725](https://github.com/getsentry/sentry-android-gradle-plugin/pull/725)) 
 
 ## 4.8.0

diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt
@@ -29,7 +29,7 @@ object LibsVersion {
     const val JUNIT = "4.13.2"
     const val ASM = "9.4" // compatibility matrix -> https://developer.android.com/reference/tools/gradle-api/7.1/com/android/build/api/instrumentation/InstrumentationContext#apiversion
     const val SQLITE = "2.1.0"
-    const val SENTRY = "6.31.0"
+    const val SENTRY = "7.0.0"
 }
 
 object Libs {
@@ -50,6 +50,7 @@ object Libs {
     const val SENTRY = "io.sentry:sentry:${LibsVersion.SENTRY}"
     const val SENTRY_ANDROID = "io.sentry:sentry-android:${LibsVersion.SENTRY}"
     const val SENTRY_ANDROID_OKHTTP = "io.sentry:sentry-android-okhttp:${LibsVersion.SENTRY}"
+    const val SENTRY_OKHTTP = "io.sentry:sentry-okhttp:${LibsVersion.SENTRY}"
 
     // test
     val MOCKITO_KOTLIN = "com.nhaarman.mockitokotlin2:mockito-kotlin:2.2.0"

diff --git a/plugin-build/build.gradle.kts b/plugin-build/build.gradle.kts
@@ -96,6 +96,7 @@ dependencies {
     testRuntimeOnly(files(androidSdkPath))
     testImplementation(Libs.SENTRY_ANDROID)
     testImplementationAar(Libs.SENTRY_ANDROID_OKHTTP)
+    testImplementationAar(Libs.SENTRY_OKHTTP)
 
     // Needed to read contents from APK/Source Bundles
     testImplementation(Libs.ARSC_LIB)

diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/autoinstall/AbstractInstallStrategy.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/autoinstall/AbstractInstallStrategy.kt
@@ -1,6 +1,7 @@
 package io.sentry.android.gradle.autoinstall
 
 import io.sentry.android.gradle.util.SemVer
+import io.sentry.android.gradle.util.debug
 import io.sentry.android.gradle.util.info
 import io.sentry.android.gradle.util.warn
 import org.gradle.api.artifacts.ComponentMetadataContext
@@ -19,6 +20,8 @@ abstract class AbstractInstallStrategy : ComponentMetadataRule {
 
     protected open val minSupportedSentryVersion: SemVer = SemVer(0, 0, 0)
 
+    protected open val maxSupportedSentryVersion: SemVer = SemVer(0, 0, 0)
+
     override fun execute(context: ComponentMetadataContext) {
         val autoInstallState = AutoInstallState.getInstance()
         if (!autoInstallState.enabled) {
@@ -55,9 +58,30 @@ abstract class AbstractInstallStrategy : ComponentMetadataRule {
                 val sentrySemVersion = SemVer.parse(autoInstallState.sentryVersion)
                 if (sentrySemVersion < minSupportedSentryVersion) {
                     logger.warn {
-                        "$sentryModuleId won't be installed because the current version is " +
-                            "lower than the minimum supported sentry version " +
-                            "($autoInstallState.sentryVersion)"
+                        "$sentryModuleId won't be installed because the current sentry version " +
+                            "is lower than the minimum supported sentry version " +
+                            "($minSupportedSentryVersion)"
+                    }
+                    return
+                }
+            } catch (ex: IllegalArgumentException) {
+                logger.warn {
+                    "$sentryModuleId won't be installed because the provided " +
+                        "sentry version(${autoInstallState.sentryVersion}) could not be " +
+                        "processed as a semantic version."
+                }
+                return
+            }
+        }
+
+        if (maxSupportedSentryVersion.major > 0) {
+            try {
+                val sentrySemVersion = SemVer.parse(autoInstallState.sentryVersion)
+                if (sentrySemVersion > maxSupportedSentryVersion) {
+                    logger.debug {
+                        "$sentryModuleId won't be installed because the current sentry version " +
+                            "is higher than the maximum supported sentry version " +
+                            "($maxSupportedSentryVersion)"
                     }
                     return
                 }

diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/autoinstall/AutoInstall.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/autoinstall/AutoInstall.kt
@@ -7,6 +7,7 @@ import io.sentry.android.gradle.autoinstall.jdbc.JdbcInstallStrategy
 import io.sentry.android.gradle.autoinstall.kotlin.KotlinExtensionsInstallStrategy
 import io.sentry.android.gradle.autoinstall.log4j2.Log4j2InstallStrategy
 import io.sentry.android.gradle.autoinstall.logback.LogbackInstallStrategy
+import io.sentry.android.gradle.autoinstall.okhttp.AndroidOkHttpInstallStrategy
 import io.sentry.android.gradle.autoinstall.okhttp.OkHttpInstallStrategy
 import io.sentry.android.gradle.autoinstall.override.WarnOnOverrideStrategy
 import io.sentry.android.gradle.autoinstall.quartz.QuartzInstallStrategy
@@ -25,6 +26,7 @@ import org.gradle.api.artifacts.DependencySet
 internal const val SENTRY_GROUP = "io.sentry"
 
 private val strategies = listOf(
+    AndroidOkHttpInstallStrategy.Registrar,
     OkHttpInstallStrategy.Registrar,
     SQLiteInstallStrategy.Registrar,
     TimberInstallStrategy.Registrar,

diff --git a/...c/main/kotlin/io/sentry/android/gradle/autoinstall/okhttp/AndroidOkHttpInstallStrategy.kt b/...c/main/kotlin/io/sentry/android/gradle/autoinstall/okhttp/AndroidOkHttpInstallStrategy.kt
@@ -0,0 +1,43 @@
+package io.sentry.android.gradle.autoinstall.okhttp
+
+import io.sentry.android.gradle.SentryPlugin
+import io.sentry.android.gradle.autoinstall.AbstractInstallStrategy
+import io.sentry.android.gradle.autoinstall.InstallStrategyRegistrar
+import io.sentry.android.gradle.util.SemVer
+import javax.inject.Inject
+import org.gradle.api.artifacts.dsl.ComponentMetadataHandler
+import org.slf4j.Logger
+
+abstract class AndroidOkHttpInstallStrategy : AbstractInstallStrategy {
+
+    constructor(logger: Logger) : super() {
+        this.logger = logger
+    }
+
+    @Suppress("unused") // used by Gradle
+    @Inject // inject is needed to avoid Gradle error
+    constructor() : this(SentryPlugin.logger)
+
+    override val sentryModuleId: String get() = SENTRY_ANDROID_OKHTTP_ID
+
+    override val minSupportedThirdPartyVersion: SemVer get() = MIN_SUPPORTED_VERSION
+
+    override val minSupportedSentryVersion: SemVer get() = SemVer(4, 4, 0)
+
+    override val maxSupportedSentryVersion: SemVer get() = SemVer(6, 9999, 9999)
+
+    companion object Registrar : InstallStrategyRegistrar {
+        private const val OKHTTP_GROUP = "com.squareup.okhttp3"
+        private const val OKHTTP_ID = "okhttp"
+        internal const val SENTRY_ANDROID_OKHTTP_ID = "sentry-android-okhttp"
+
+        private val MIN_SUPPORTED_VERSION = SemVer(3, 13, 0)
+
+        override fun register(component: ComponentMetadataHandler) {
+            component.withModule(
+                "$OKHTTP_GROUP:$OKHTTP_ID",
+                AndroidOkHttpInstallStrategy::class.java
+            ) {}
+        }
+    }
+}
diff --git a/...uild/src/main/kotlin/io/sentry/android/gradle/autoinstall/okhttp/OkHttpInstallStrategy.kt b/...uild/src/main/kotlin/io/sentry/android/gradle/autoinstall/okhttp/OkHttpInstallStrategy.kt
@@ -23,12 +23,12 @@ abstract class OkHttpInstallStrategy : AbstractInstallStrategy {
 
     override val minSupportedThirdPartyVersion: SemVer get() = MIN_SUPPORTED_VERSION
 
-    override val minSupportedSentryVersion: SemVer get() = SemVer(4, 4, 0)
+    override val minSupportedSentryVersion: SemVer get() = SemVer(7, 0, 0)
 
     companion object Registrar : InstallStrategyRegistrar {
         private const val OKHTTP_GROUP = "com.squareup.okhttp3"
         private const val OKHTTP_ID = "okhttp"
-        internal const val SENTRY_OKHTTP_ID = "sentry-android-okhttp"
+        internal const val SENTRY_OKHTTP_ID = "sentry-okhttp"
 
         private val MIN_SUPPORTED_VERSION = SemVer(3, 13, 0)
 

diff --git a/...d/src/main/kotlin/io/sentry/android/gradle/autoinstall/override/WarnOnOverrideStrategy.kt b/...d/src/main/kotlin/io/sentry/android/gradle/autoinstall/override/WarnOnOverrideStrategy.kt
@@ -67,6 +67,7 @@ abstract class WarnOnOverrideStrategy : ComponentMetadataRule {
             SentryModules.SENTRY_ANDROID_FRAGMENT,
             SentryModules.SENTRY_ANDROID_NAVIGATION,
             SentryModules.SENTRY_ANDROID_TIMBER,
+            SentryModules.SENTRY_OKHTTP,
             SentryModules.SENTRY_KOTLIN_EXTENSIONS,
             SentryModules.SENTRY_GRAPHQL,
             SentryModules.SENTRY_JDBC,

diff --git a/...src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt b/...src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt
@@ -23,6 +23,8 @@ import io.sentry.android.gradle.instrumentation.util.isMinifiedClass
 import io.sentry.android.gradle.instrumentation.wrap.WrappingInstrumentable
 import io.sentry.android.gradle.services.SentryModulesService
 import io.sentry.android.gradle.util.SemVer
+import io.sentry.android.gradle.util.SentryModules
+import io.sentry.android.gradle.util.SentryVersions
 import io.sentry.android.gradle.util.info
 import java.io.File
 import org.gradle.api.internal.artifacts.DefaultModuleIdentifier
@@ -85,6 +87,11 @@ abstract class SpanAddingClassVisitorFactory :
                 "okhttp"
             )
             val okHttpVersion = externalModules.getOrDefault(okHttpModule, SemVer())
+            val sentryOkhttpVersion = sentryModules.getOrDefault(
+                SentryModules.SENTRY_OKHTTP,
+                SemVer()
+            )
+            val useSentryAndroidOkHttp = sentryOkhttpVersion < SentryVersions.VERSION_OKHTTP
 
             SentryPlugin.logger.info { "Read sentry modules: $sentryModules" }
 
@@ -104,10 +111,10 @@ abstract class SpanAddingClassVisitorFactory :
                         sentryModulesService.isNewDatabaseInstrEnabled() ||
                             sentryModulesService.isOldDatabaseInstrEnabled()
                     },
-                    OkHttpEventListener(okHttpVersion).takeIf {
+                    OkHttpEventListener(useSentryAndroidOkHttp, okHttpVersion).takeIf {
                         sentryModulesService.isOkHttpListenerInstrEnabled()
                     },
-                    OkHttp().takeIf {
+                    OkHttp(useSentryAndroidOkHttp).takeIf {
                         sentryModulesService.isOkHttpInstrEnabled()
                     },
                     WrappingInstrumentable().takeIf {

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
@@ -10,7 +10,7 @@ import io.sentry.android.gradle.instrumentation.okhttp.visitor.ResponseWithInter
 import org.objectweb.asm.ClassVisitor
 import org.objectweb.asm.MethodVisitor
 
-class OkHttp : ClassInstrumentable {
+class OkHttp(private val useSentryAndroidOkHttp: Boolean) : ClassInstrumentable {
     override val fqName: String get() = "RealCall"
 
     override fun getVisitor(
@@ -22,7 +22,7 @@ class OkHttp : ClassInstrumentable {
         apiVersion = apiVersion,
         classVisitor = originalVisitor,
         className = fqName.substringAfterLast('.'),
-        methodInstrumentables = listOf(ResponseWithInterceptorChain()),
+        methodInstrumentables = listOf(ResponseWithInterceptorChain(useSentryAndroidOkHttp)),
         parameters = parameters
     )
 
@@ -33,7 +33,9 @@ class OkHttp : ClassInstrumentable {
             data.currentClassData.className == "okhttp3.RealCall"
 }
 
-class ResponseWithInterceptorChain : MethodInstrumentable {
+class ResponseWithInterceptorChain(
+    private val useSentryAndroidOkHttp: Boolean
+) : MethodInstrumentable {
     override val fqName: String get() = "getResponseWithInterceptorChain"
 
     override fun getVisitor(
@@ -46,7 +48,8 @@ class ResponseWithInterceptorChain : MethodInstrumentable {
         originalVisitor = originalVisitor,
         access = instrumentableContext.access,
         name = instrumentableContext.name,
-        descriptor = instrumentableContext.descriptor
+        descriptor = instrumentableContext.descriptor,
+        useSentryAndroidOkHttp = useSentryAndroidOkHttp
     )
 
     override fun isInstrumentable(data: MethodContext): Boolean {

diff --git a/...ld/src/main/kotlin/io/sentry/android/gradle/instrumentation/okhttp/OkHttpEventListener.kt b/...ld/src/main/kotlin/io/sentry/android/gradle/instrumentation/okhttp/OkHttpEventListener.kt
@@ -11,7 +11,10 @@ import io.sentry.android.gradle.util.SemVer
 import org.objectweb.asm.ClassVisitor
 import org.objectweb.asm.MethodVisitor
 
-class OkHttpEventListener(private val okHttpVersion: SemVer) : ClassInstrumentable {
+class OkHttpEventListener(
+    private val useSentryAndroidOkHttp: Boolean,
+    private val okHttpVersion: SemVer
+) : ClassInstrumentable {
     override val fqName: String get() = "okhttp3.OkHttpClient"
 
     override fun getVisitor(
@@ -23,12 +26,18 @@ class OkHttpEventListener(private val okHttpVersion: SemVer) : ClassInstrumentab
         apiVersion = apiVersion,
         classVisitor = originalVisitor,
         className = fqName.substringAfterLast('.'),
-        methodInstrumentables = listOf(OkHttpEventListenerMethodInstrumentable(okHttpVersion)),
+        methodInstrumentables = listOf(
+            OkHttpEventListenerMethodInstrumentable(
+                useSentryAndroidOkHttp,
+                okHttpVersion
+            )
+        ),
         parameters = parameters
     )
 }
 
 class OkHttpEventListenerMethodInstrumentable(
+    private val useSentryAndroidOkHttp: Boolean,
     private val okHttpVersion: SemVer
 ) : MethodInstrumentable {
     override val fqName: String get() = "<init>"
@@ -42,7 +51,8 @@ class OkHttpEventListenerMethodInstrumentable(
         apiVersion = apiVersion,
         originalVisitor = originalVisitor,
         instrumentableContext = instrumentableContext,
-        okHttpVersion = okHttpVersion
+        okHttpVersion = okHttpVersion,
+        useSentryAndroidOkHttp = useSentryAndroidOkHttp
     )
 
     override fun isInstrumentable(data: MethodContext): Boolean {

diff --git a/.../sentry/android/gradle/instrumentation/okhttp/visitor/OkHttpEventListenerMethodVisitor.kt b/.../sentry/android/gradle/instrumentation/okhttp/visitor/OkHttpEventListenerMethodVisitor.kt
@@ -10,7 +10,8 @@ class OkHttpEventListenerMethodVisitor(
     apiVersion: Int,
     originalVisitor: MethodVisitor,
     instrumentableContext: MethodContext,
-    private val okHttpVersion: SemVer
+    private val okHttpVersion: SemVer,
+    private val useSentryAndroidOkHttp: Boolean
 ) : AdviceAdapter(
     apiVersion,
     originalVisitor,
@@ -19,6 +20,12 @@ class OkHttpEventListenerMethodVisitor(
     instrumentableContext.descriptor
 ) {
 
+    private val sentryOkHttpEventListener = if (useSentryAndroidOkHttp) {
+        "io/sentry/android/okhttp/SentryOkHttpEventListener"
+    } else {
+        "io/sentry/okhttp/SentryOkHttpEventListener"
+    }
+
     override fun onMethodEnter() {
         super.onMethodEnter()
         // Add the following call at the beginning of the constructor with the Builder parameter:
@@ -28,7 +35,7 @@ class OkHttpEventListenerMethodVisitor(
         visitVarInsn(Opcodes.ALOAD, 1)
 
         // Let's declare the SentryOkHttpEventListener variable
-        visitTypeInsn(Opcodes.NEW, "io/sentry/android/okhttp/SentryOkHttpEventListener")
+        visitTypeInsn(Opcodes.NEW, sentryOkHttpEventListener)
 
         // The SentryOkHttpEventListener constructor, which is called later, will consume the
         //  element without pushing anything back to the stack (<init> returns void).
@@ -62,7 +69,7 @@ class OkHttpEventListenerMethodVisitor(
         // Call SentryOkHttpEventListener constructor passing "eventListenerFactory" as parameter
         visitMethodInsn(
             Opcodes.INVOKESPECIAL,
-            "io/sentry/android/okhttp/SentryOkHttpEventListener",
+            sentryOkHttpEventListener,
             "<init>",
             "(Lokhttp3/EventListener\$Factory;)V",
             false

diff --git a/...ndroid/gradle/instrumentation/okhttp/visitor/ResponseWithInterceptorChainMethodVisitor.kt b/...ndroid/gradle/instrumentation/okhttp/visitor/ResponseWithInterceptorChainMethodVisitor.kt
@@ -8,6 +8,7 @@ import org.objectweb.asm.commons.GeneratorAdapter
 import org.objectweb.asm.commons.Method
 
 class ResponseWithInterceptorChainMethodVisitor(
+    private val useSentryAndroidOkHttp: Boolean,
     api: Int,
     private val originalVisitor: MethodVisitor,
     access: Int,
@@ -17,6 +18,12 @@ class ResponseWithInterceptorChainMethodVisitor(
 
     private var shouldInstrument = false
 
+    private val sentryOkInterceptor = if (useSentryAndroidOkHttp) {
+        Types.SENTRY_ANDROID_OKHTTP_INTERCEPTOR
+    } else {
+        Types.SENTRY_OKHTTP_INTERCEPTOR
+    }
+
     override fun visitMethodInsn(
         opcode: Int,
         owner: String?,
@@ -69,7 +76,7 @@ class ResponseWithInterceptorChainMethodVisitor(
         storeLocal(interceptorIndex)
         loadLocal(interceptorIndex)
         checkCast(Types.OKHTTP_INTERCEPTOR)
-        instanceOf(Types.SENTRY_OKHTTP_INTERCEPTOR)
+        instanceOf(sentryOkInterceptor)
         ifZCmp(EQ, whileLabel)
         loadLocal(interceptorIndex)
         val ifLabel = Label()
@@ -83,10 +90,10 @@ class ResponseWithInterceptorChainMethodVisitor(
 
         originalVisitor.visitVarInsn(Opcodes.ALOAD, 1)
         checkCast(Types.COLLECTION)
-        newInstance(Types.SENTRY_OKHTTP_INTERCEPTOR)
+        newInstance(sentryOkInterceptor)
         dup()
         val sentryOkHttpCtor = Method.getMethod("void <init> ()")
-        invokeConstructor(Types.SENTRY_OKHTTP_INTERCEPTOR, sentryOkHttpCtor)
+        invokeConstructor(sentryOkInterceptor, sentryOkHttpCtor)
         val addInterceptor = Method.getMethod("boolean add (Object)")
         invokeInterface(Types.COLLECTION, addInterceptor)
         pop()

diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/Types.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/Types.kt
@@ -19,6 +19,8 @@ object Types {
 
     // OKHTTP
     val OKHTTP_INTERCEPTOR = Type.getType("Lokhttp3/Interceptor;")
-    val SENTRY_OKHTTP_INTERCEPTOR =
+    val SENTRY_ANDROID_OKHTTP_INTERCEPTOR =
         Type.getType("Lio/sentry/android/okhttp/SentryOkHttpInterceptor;")
+    val SENTRY_OKHTTP_INTERCEPTOR =
+        Type.getType("Lio/sentry/okhttp/SentryOkHttpInterceptor;")
 }