Invalidate instrumentation cache when changing instrumentation features

* add features, logcat and startup flags to SpanAddingClassVisitorFactory to allow invalidating cache on change to these flags, add test * format, remove unnecessary comment * fix trailing space error * fix test * add changelog * try different approach to test
getsentry · Sep 3, 2024 · 367444c · 367444c
1 parent 3ea735c
commit 367444c
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Invalidate instrumentation cache when changing instrumentation features ([#753](https://github.com/getsentry/sentry-android-gradle-plugin/pull/753))
+
 ## 4.11.0
 
 ### Fixes

diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/AndroidComponentsConfig.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/AndroidComponentsConfig.kt
@@ -203,7 +203,15 @@ fun AndroidComponentsExtension<*, *, *>.configure(
                     params.logcatMinLevel.setDisallowChanges(
                         extension.tracingInstrumentation.logcat.minLevel
                     )
+
                     params.sentryModulesService.setDisallowChanges(sentryModulesService)
+                    params.features.setDisallowChanges(extension.tracingInstrumentation.features)
+                    params.logcatEnabled.setDisallowChanges(
+                        extension.tracingInstrumentation.logcat.enabled
+                    )
+                    params.appStartEnabled.setDisallowChanges(
+                        extension.tracingInstrumentation.appStart.enabled
+                    )
                     params.tmpDir.set(tmpDir)
                 }
 

diff --git a/...src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt b/...src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt
@@ -5,6 +5,7 @@ import com.android.build.api.instrumentation.ClassContext
 import com.android.build.api.instrumentation.ClassData
 import com.android.build.api.instrumentation.InstrumentationParameters
 import io.sentry.android.gradle.SentryPlugin
+import io.sentry.android.gradle.extensions.InstrumentationFeature
 import io.sentry.android.gradle.instrumentation.androidx.compose.ComposeNavigation
 import io.sentry.android.gradle.instrumentation.androidx.room.AndroidXRoomDao
 import io.sentry.android.gradle.instrumentation.androidx.sqlite.AndroidXSQLiteOpenHelper
@@ -29,6 +30,7 @@ import io.sentry.android.gradle.util.info
 import java.io.File
 import org.gradle.api.internal.artifacts.DefaultModuleIdentifier
 import org.gradle.api.provider.Property
+import org.gradle.api.provider.SetProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Internal
 import org.gradle.api.tasks.Optional
@@ -63,6 +65,15 @@ abstract class SpanAddingClassVisitorFactory :
 
         @get:Internal
         var _instrumentable: ClassInstrumentable?
+
+        @get:Input
+        val features: SetProperty<InstrumentationFeature>
+
+        @get:Input
+        val logcatEnabled: Property<Boolean>
+
+        @get:Input
+        val appStartEnabled: Property<Boolean>
     }
 
     private val instrumentable: ClassInstrumentable

diff --git a/...rc/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/TestSpanAddingParameters.kt b/...rc/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/TestSpanAddingParameters.kt
@@ -1,5 +1,6 @@
 package io.sentry.android.gradle.instrumentation.fakes
 
+import io.sentry.android.gradle.extensions.InstrumentationFeature
 import io.sentry.android.gradle.instrumentation.ClassInstrumentable
 import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory
 import io.sentry.android.gradle.instrumentation.logcat.LogcatLevel
@@ -8,6 +9,7 @@ import java.io.File
 import org.gradle.api.internal.provider.DefaultProperty
 import org.gradle.api.internal.provider.PropertyHost
 import org.gradle.api.provider.Property
+import org.gradle.api.provider.SetProperty
 
 class TestSpanAddingParameters(
     private val debugOutput: Boolean = true,
@@ -32,4 +34,13 @@ class TestSpanAddingParameters(
         get() = DefaultProperty<File>(PropertyHost.NO_OP, File::class.java).convention(inMemoryDir)
 
     override var _instrumentable: ClassInstrumentable? = null
+
+    override val features: SetProperty<InstrumentationFeature>
+        get() = TODO()
+
+    override val logcatEnabled: Property<Boolean>
+        get() = TODO()
+
+    override val appStartEnabled: Property<Boolean>
+        get() = TODO()
 }
diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/integration/SentryPluginTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/integration/SentryPluginTest.kt
@@ -8,6 +8,7 @@ import io.sentry.android.gradle.verifyDependenciesReportAndroid
 import io.sentry.android.gradle.verifyIntegrationList
 import io.sentry.android.gradle.verifyProguardUuid
 import java.io.File
+import java.util.EnumSet
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertNotEquals
@@ -637,6 +638,55 @@ class SentryPluginTest :
         assertTrue { debugOutput.exists() && debugOutput.length() > 0 }
     }
 
+    @Test
+    fun `changing instrumentation features invalidates cache`() {
+        val enumSetInitial = EnumSet.allOf(InstrumentationFeature::class.java)
+        val enumSetWithoutOkHttp =
+            EnumSet.allOf(InstrumentationFeature::class.java) - InstrumentationFeature.OKHTTP
+        val dependencies = setOf(
+            "com.squareup.okhttp3:okhttp:3.14.9",
+            "io.sentry:sentry-android-okhttp:6.6.0"
+        )
+
+        val secondBuildFile = File(appBuildFile.parentFile, "secondBuild")
+        appBuildFile.copyTo(secondBuildFile)
+
+        applyTracingInstrumentation(
+            dependencies = dependencies,
+            features = enumSetInitial,
+            debug = true,
+            forceInstrumentDependencies = false
+        )
+
+        // Enable build cache to be able to test that changing the feature set actually reruns the instrumentation task
+        val buildOne = runner
+            .appendArguments("clean", ":app:assembleDebug", "--info", "--build-cache")
+            .build()
+
+        assertTrue {
+            "[sentry] Instrumentable: ChainedInstrumentable(instrumentables=" +
+                "AndroidXSQLiteDatabase, AndroidXSQLiteStatement, AndroidXRoomDao, " +
+                "OkHttp, WrappingInstrumentable, RemappingInstrumentable)" in buildOne.output
+        }
+
+        appBuildFile.writeText(secondBuildFile.readText())
+
+        applyTracingInstrumentation(
+            dependencies = dependencies,
+            features = enumSetWithoutOkHttp,
+            debug = true,
+            forceInstrumentDependencies = false
+        )
+
+        val buildTwo = runner.build()
+
+        assertTrue {
+            "[sentry] Instrumentable: ChainedInstrumentable(instrumentables=" +
+                "AndroidXSQLiteDatabase, AndroidXSQLiteStatement, AndroidXRoomDao, " +
+                "WrappingInstrumentable, RemappingInstrumentable)" in buildTwo.output
+        }
+    }
+
     @Test
     fun `includes flattened list of dependencies into the APK, excluding non-external deps`() {
         appBuildFile.appendText(
@@ -999,7 +1049,8 @@ class SentryPluginTest :
         dependencies: Set<String> = emptySet(),
         debug: Boolean = false,
         excludes: Set<String> = emptySet(),
-        sdkVersion: String = "7.1.0"
+        sdkVersion: String = "7.1.0",
+        forceInstrumentDependencies: Boolean = true
     ) {
         appBuildFile.appendText(
             // language=Groovy
@@ -1012,7 +1063,7 @@ class SentryPluginTest :
                 sentry {
                   autoUploadProguardMapping = false
                   tracingInstrumentation {
-                    forceInstrumentDependencies = true
+                    forceInstrumentDependencies = $forceInstrumentDependencies
                     enabled = $tracingInstrumentation
                     debug = $debug
                     features = [${features.joinToString { "${it::class.java.canonicalName}.${it.name}" }}]