Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto install and instrument sentry-okhttp instead of sentry-android-okhttp on v7+ #724

Merged
merged 6 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion buildSrc/src/main/java/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"
Expand Down
1 change: 1 addition & 0 deletions plugin-build/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
) {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" }

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -22,7 +22,7 @@ class OkHttp : ClassInstrumentable {
apiVersion = apiVersion,
classVisitor = originalVisitor,
className = fqName.substringAfterLast('.'),
methodInstrumentables = listOf(ResponseWithInterceptorChain()),
methodInstrumentables = listOf(ResponseWithInterceptorChain(useSentryAndroidOkHttp)),
parameters = parameters
)

Expand All @@ -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(
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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>"
Expand All @@ -42,7 +51,8 @@ class OkHttpEventListenerMethodInstrumentable(
apiVersion = apiVersion,
originalVisitor = originalVisitor,
instrumentableContext = instrumentableContext,
okHttpVersion = okHttpVersion
okHttpVersion = okHttpVersion,
useSentryAndroidOkHttp = useSentryAndroidOkHttp
)

override fun isInstrumentable(data: MethodContext): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand All @@ -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).
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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?,
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;")
}
Loading
Loading