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 1 commit
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
Prev Previous commit
Next Next commit
added tests
fixed formatting
  • Loading branch information
stefanosiano committed Jun 20, 2024
commit 0fbe8c5a3fb76653fb51ff7c63036f403949b2ab
2 changes: 1 addition & 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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ 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
}
Expand All @@ -79,9 +79,9 @@ abstract class AbstractInstallStrategy : ComponentMetadataRule {
val sentrySemVersion = SemVer.parse(autoInstallState.sentryVersion)
if (sentrySemVersion > maxSupportedSentryVersion) {
logger.debug {
"$sentryModuleId won't be installed because the current version is " +
"higher than the maximum supported sentry version " +
"(${autoInstallState.sentryVersion})"
"$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 @@ -87,7 +87,10 @@ abstract class SpanAddingClassVisitorFactory :
"okhttp"
)
val okHttpVersion = externalModules.getOrDefault(okHttpModule, SemVer())
val sentryOkhttpVersion = sentryModules.getOrDefault(SentryModules.SENTRY_OKHTTP, SemVer())
val sentryOkhttpVersion = sentryModules.getOrDefault(
SentryModules.SENTRY_OKHTTP,
SemVer()
)
val useSentryAndroidOkHttp = sentryOkhttpVersion < SentryVersions.VERSION_OKHTTP

SentryPlugin.logger.info { "Read sentry modules: $sentryModules" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ class OkHttp(private val useSentryAndroidOkHttp: Boolean) : ClassInstrumentable
data.currentClassData.className == "okhttp3.RealCall"
}

class ResponseWithInterceptorChain(private val useSentryAndroidOkHttp: Boolean) : MethodInstrumentable {
class ResponseWithInterceptorChain(
private val useSentryAndroidOkHttp: Boolean
) : MethodInstrumentable {
override val fqName: String get() = "getResponseWithInterceptorChain"

override fun getVisitor(
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 useSentryAndroidOkHttp: Boolean, 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,7 +26,12 @@ class OkHttpEventListener(private val useSentryAndroidOkHttp: Boolean, private v
apiVersion = apiVersion,
classVisitor = originalVisitor,
className = fqName.substringAfterLast('.'),
methodInstrumentables = listOf(OkHttpEventListenerMethodInstrumentable(useSentryAndroidOkHttp, okHttpVersion)),
methodInstrumentables = listOf(
OkHttpEventListenerMethodInstrumentable(
useSentryAndroidOkHttp,
okHttpVersion
)
),
parameters = parameters
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ class OkHttpEventListenerMethodVisitor(
instrumentableContext.descriptor
) {

private val sentryOkHttpEventListener = if (useSentryAndroidOkHttp) "io/sentry/android/okhttp/SentryOkHttpEventListener" else "io/sentry/okhttp/SentryOkHttpEventListener"
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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ class ResponseWithInterceptorChainMethodVisitor(

private var shouldInstrument = false

private val sentryOkInterceptor = if (useSentryAndroidOkHttp) Types.SENTRY_ANDROID_OKHTTP_INTERCEPTOR else Types.SENTRY_OKHTTP_INTERCEPTOR
private val sentryOkInterceptor = if (useSentryAndroidOkHttp) {
Types.SENTRY_ANDROID_OKHTTP_INTERCEPTOR
} else {
Types.SENTRY_OKHTTP_INTERCEPTOR
}

override fun visitMethodInsn(
opcode: Int,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,31 @@ abstract class SentryModulesService :
SentryVersions.VERSION_FILE_IO
) && parameters.features.get().contains(InstrumentationFeature.FILE_IO)

fun isOkHttpListenerInstrEnabled(): Boolean = (sentryModules.isAtLeast(
SentryModules.SENTRY_ANDROID_OKHTTP,
SentryVersions.VERSION_ANDROID_OKHTTP_LISTENER
) || sentryModules.isAtLeast(
SentryModules.SENTRY_OKHTTP,
SentryVersions.VERSION_OKHTTP
)) && parameters.features.get().contains(InstrumentationFeature.OKHTTP)

fun isOkHttpInstrEnabled(): Boolean = (sentryModules.isAtLeast(
SentryModules.SENTRY_ANDROID_OKHTTP,
SentryVersions.VERSION_ANDROID_OKHTTP
) || sentryModules.isAtLeast(
SentryModules.SENTRY_OKHTTP,
SentryVersions.VERSION_OKHTTP
)) && parameters.features.get().contains(InstrumentationFeature.OKHTTP)
fun isOkHttpListenerInstrEnabled(): Boolean {
val isSentryAndroidOkHttpListener = sentryModules.isAtLeast(
SentryModules.SENTRY_ANDROID_OKHTTP,
SentryVersions.VERSION_ANDROID_OKHTTP_LISTENER
)
val isSentryOkHttpListener = sentryModules.isAtLeast(
SentryModules.SENTRY_OKHTTP,
SentryVersions.VERSION_OKHTTP
)
return (isSentryAndroidOkHttpListener || isSentryOkHttpListener) &&
parameters.features.get().contains(InstrumentationFeature.OKHTTP)
}

fun isOkHttpInstrEnabled(): Boolean {
val isSentryAndroidOkHttp = sentryModules.isAtLeast(
SentryModules.SENTRY_ANDROID_OKHTTP,
SentryVersions.VERSION_ANDROID_OKHTTP
)
val isSentryOkHttp = sentryModules.isAtLeast(
SentryModules.SENTRY_OKHTTP,
SentryVersions.VERSION_OKHTTP
)
return (isSentryAndroidOkHttp || isSentryOkHttp) &&
parameters.features.get().contains(InstrumentationFeature.OKHTTP)
}

fun isComposeInstrEnabled(): Boolean =
sentryModules.isAtLeast(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import io.sentry.Sentry
import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SpanStatus
import io.sentry.TransactionOptions
import io.sentry.android.gradle.SentryCliProvider
import io.sentry.android.gradle.SentryPlugin
import io.sentry.android.gradle.SentryPlugin.Companion.logger
Expand Down Expand Up @@ -196,7 +197,9 @@ abstract class SentryTelemetryService :

fun startRun(transactionName: String) {
hub.startSession()
transaction = hub.startTransaction(transactionName, "build", true)
val options = TransactionOptions()
options.isBindToScope = true
transaction = hub.startTransaction(transactionName, "build", options)
}

fun endRun() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package io.sentry.android.gradle.autoinstall.okhttp

import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.check
import com.nhaarman.mockitokotlin2.doAnswer
import com.nhaarman.mockitokotlin2.doReturn
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.never
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.android.gradle.autoinstall.AutoInstallState
import io.sentry.android.gradle.instrumentation.fakes.CapturingTestLogger
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import org.gradle.api.Action
import org.gradle.api.artifacts.ComponentMetadataContext
import org.gradle.api.artifacts.ComponentMetadataDetails
import org.gradle.api.artifacts.DirectDependenciesMetadata
import org.gradle.api.artifacts.ModuleVersionIdentifier
import org.gradle.api.artifacts.VariantMetadata
import org.junit.Test
import org.slf4j.Logger

class AndroidOkHttpInstallStrategyTest {
class Fixture {
val logger = CapturingTestLogger()
val dependencies = mock<DirectDependenciesMetadata>()
val metadataDetails = mock<ComponentMetadataDetails>()
val metadataContext = mock<ComponentMetadataContext> {
whenever(it.details).thenReturn(metadataDetails)
val metadata = mock<VariantMetadata>()
doAnswer {
(it.arguments[0] as Action<DirectDependenciesMetadata>).execute(dependencies)
}.whenever(metadata).withDependencies(any<Action<DirectDependenciesMetadata>>())

doAnswer {
// trigger the callback registered in tests
(it.arguments[0] as Action<VariantMetadata>).execute(metadata)
}.whenever(metadataDetails).allVariants(any<Action<VariantMetadata>>())
}

fun getSut(
okHttpVersion: String = "4.9.3",
sentryVersion: String = "5.6.1"
): AndroidOkHttpInstallStrategy {
val id = mock<ModuleVersionIdentifier> {
whenever(it.version).doReturn(okHttpVersion)
}
whenever(metadataDetails.id).thenReturn(id)

with(AutoInstallState.getInstance()) {
this.enabled = true
this.sentryVersion = sentryVersion
}
return AndroidOkHttpInstallStrategyImpl(logger)
}
}

private val fixture = Fixture()

@Test
fun `when okhttp version is unsupported logs a message and does nothing`() {
val sut = fixture.getSut(okHttpVersion = "3.11.0")
sut.execute(fixture.metadataContext)

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-android-okhttp won't be installed because the current " +
"version is lower than the minimum supported version (3.13.0)"
}
verify(fixture.metadataDetails, never()).allVariants(any())
}

@Test
fun `when sentry version is unsupported logs a message and does nothing`() {
val sut = fixture.getSut(sentryVersion = "7.0.0")
sut.execute(fixture.metadataContext)

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-android-okhttp won't be installed because the current " +
"sentry version is higher than the maximum supported sentry version (6.9999.9999)"
}
verify(fixture.metadataDetails, never()).allVariants(any())
}

@Test
fun `installs sentry-android-okhttp with info message`() {
val sut = fixture.getSut()
sut.execute(fixture.metadataContext)

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-android-okhttp was successfully installed with version: 5.6.1"
}
verify(fixture.dependencies).add(
check<String> {
assertEquals("io.sentry:sentry-android-okhttp:5.6.1", it)
}
)
}

private class AndroidOkHttpInstallStrategyImpl(
logger: Logger
) : AndroidOkHttpInstallStrategy(logger)
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class OkHttpInstallStrategyTest {
}

fun getSut(
okHttpVersion: String = "4.9.3"
okHttpVersion: String = "4.9.3",
sentryVersion: String = "7.0.0"
): OkHttpInstallStrategy {
val id = mock<ModuleVersionIdentifier> {
whenever(it.version).doReturn(okHttpVersion)
Expand All @@ -49,7 +50,7 @@ class OkHttpInstallStrategyTest {

with(AutoInstallState.getInstance()) {
this.enabled = true
this.sentryVersion = "5.6.1"
this.sentryVersion = sentryVersion
}
return OkHttpInstallStrategyImpl(logger)
}
Expand All @@ -64,24 +65,37 @@ class OkHttpInstallStrategyTest {

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-android-okhttp won't be installed because the current " +
"[sentry] sentry-okhttp won't be installed because the current " +
"version is lower than the minimum supported version (3.13.0)"
}
verify(fixture.metadataDetails, never()).allVariants(any())
}

@Test
fun `installs sentry-android-okhttp with info message`() {
fun `when sentry version is unsupported logs a message and does nothing`() {
val sut = fixture.getSut(sentryVersion = "6.33.0")
sut.execute(fixture.metadataContext)

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-okhttp won't be installed because the current sentry " +
"version is lower than the minimum supported sentry version (7.0.0)"
}
verify(fixture.metadataDetails, never()).allVariants(any())
}

@Test
fun `installs sentry-okhttp with info message`() {
val sut = fixture.getSut()
sut.execute(fixture.metadataContext)

assertTrue {
fixture.logger.capturedMessage ==
"[sentry] sentry-android-okhttp was successfully installed with version: 5.6.1"
"[sentry] sentry-okhttp was successfully installed with version: 7.0.0"
}
verify(fixture.dependencies).add(
check<String> {
assertEquals("io.sentry:sentry-android-okhttp:5.6.1", it)
assertEquals("io.sentry:sentry-okhttp:7.0.0", it)
}
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,14 @@ class VisitorTest(
ChainedInstrumentable(listOf(WrappingInstrumentable(), RemappingInstrumentable())),
null
),
arrayOf("okhttp/v3", "RealCall", OkHttp(), null),
arrayOf("okhttp/v4", "RealCall", OkHttp(), null),
arrayOf("okhttp/v3", "OkHttpClient", OkHttpEventListener(SemVer(3, 0, 0)), null),
arrayOf("okhttp/v4", "OkHttpClient", OkHttpEventListener(SemVer(4, 0, 0)), null),
arrayOf("okhttp/v3", "RealCall", OkHttp(true), null),
arrayOf("okhttp/v4", "RealCall", OkHttp(true), null),
arrayOf("okhttp/v3", "RealCall", OkHttp(false), null),
arrayOf("okhttp/v4", "RealCall", OkHttp(false), null),
arrayOf("okhttp/v3", "OkHttpClient", OkHttpEventListener(true, SemVer(3, 0, 0)), null),
arrayOf("okhttp/v4", "OkHttpClient", OkHttpEventListener(true, SemVer(4, 0, 0)), null),
arrayOf("okhttp/v3", "OkHttpClient", OkHttpEventListener(false, SemVer(3, 0, 0)), null),
arrayOf("okhttp/v4", "OkHttpClient", OkHttpEventListener(false, SemVer(4, 0, 0)), null),
arrayOf("androidxCompose", "NavHostControllerKt", ComposeNavigation(), null),
arrayOf("logcat", "LogcatTest", Logcat(), null),
arrayOf("appstart", "MyApplication", Application(), null),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,9 @@ class CapturingTestLogger : BaseTestLogger() {
capturedMessage = msg
capturedThrowable = throwable
}

override fun debug(msg: String, throwable: Throwable?) {
capturedMessage = msg
capturedThrowable = throwable
}
}
Loading