Skip to content
Draft
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
8 changes: 8 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ fabric.properties
.idea/**/markdown-navigator.xml
.idea/**/markdown-navigator/

# Additional IDE settings
.idea/AndroidProjectSystem.xml
.idea/deploymentTargetSelector.xml
.idea/markdown.xml

### Gradle ###
.gradle

Expand Down Expand Up @@ -304,6 +309,9 @@ hs_err_pid*

!/gradle/wrapper/gradle-wrapper.jar

# Kotlin compiler sessions
.kotlin/

# Custom
/sqlite-framework
!**/instrumentation/**/*.class
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### Features

- Auto-wrap SQLiteDriver with SentrySQLiteDriver for Room users ([#1281](https://github.com/getsentry/sentry-android-gradle-plugin/issues/1281))
- Gated on `sentry-android-sqlite` >= 8.44.0 and the existing `tracingInstrumentation` `DATABASE` feature

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Update version number once we ship the SentrySQLiteDriver in sentry-android.

- Users of the `androidx.sqlite.driver.SupportSQLiteDriver` bridge are protected from duplicate span creation (the `SupportSQLiteOpenHelper` passed to the bridge constructor is auto-wrapped but the bridge itself isn't)

### Fixes

- Resolve the sentry-cli path as a task input instead of memoizing it in a static field, fixing stale-path build failures when switching branches with the configuration cache enabled ([#1264](https://github.com/getsentry/sentry-android-gradle-plugin/pull/1264))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,19 @@ open class TracingInstrumentationExtension @Inject constructor(objects: ObjectFa

enum class InstrumentationFeature(val integrationName: String) {
/**
* When enabled the SDK will create spans for any CRUD operation performed by
* 'androidx.sqlite.db.SupportSQLiteOpenHelper' and 'androidx.room'. This feature uses bytecode
* manipulation.
* When enabled the SDK will create spans for database operations at two levels:
*
* **SQL execution** (`db.sql.query` spans): wraps the low-level driver so each individual SQL
* statement produces a span. Two mutually exclusive paths:
* - `androidx.sqlite.db.SupportSQLiteOpenHelper` via any `SupportSQLiteOpenHelper.Factory`
* (open-helper path)
* - `androidx.sqlite.SQLiteDriver` via `RoomDatabase.Builder.setDriver` (driver path, Room 2.7+)
*
* **DAO method** (`db.sql.room` spans): wraps each public method on Room's generated `@Dao`
* `_Impl` classes, measuring the full DAO call end-to-end (transaction management, query
* execution, and cursor processing). Pre-Room 2.7 only.
*
* This feature uses bytecode manipulation.
*/
DATABASE("DatabaseInstrumentation"),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.sentry.android.gradle.instrumentation.androidx.compose.ComposeNavigati
import io.sentry.android.gradle.instrumentation.androidx.room.AndroidXRoomDao
import io.sentry.android.gradle.instrumentation.androidx.sqlite.AndroidXSQLiteOpenHelper
import io.sentry.android.gradle.instrumentation.androidx.sqlite.database.AndroidXSQLiteDatabase
import io.sentry.android.gradle.instrumentation.androidx.sqlite.driver.AndroidXSQLiteDriver
import io.sentry.android.gradle.instrumentation.androidx.sqlite.statement.AndroidXSQLiteStatement
import io.sentry.android.gradle.instrumentation.appstart.Application
import io.sentry.android.gradle.instrumentation.appstart.ContentProvider
Expand Down Expand Up @@ -90,6 +91,7 @@ abstract class SpanAddingClassVisitorFactory :
ChainedInstrumentable(
listOfNotNull(
AndroidXSQLiteOpenHelper().takeIf { sentryModulesService.isNewDatabaseInstrEnabled() },
AndroidXSQLiteDriver().takeIf { sentryModulesService.isSQLiteDriverInstrEnabled() },
AndroidXSQLiteDatabase().takeIf { sentryModulesService.isOldDatabaseInstrEnabled() },
AndroidXSQLiteStatement(androidXSqliteFrameWorkVersion).takeIf {
sentryModulesService.isOldDatabaseInstrEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import org.objectweb.asm.tree.MethodNode

class AndroidXRoomDao : ClassInstrumentable {

// Hardcoded to the androidx.room (2.x) annotation. On Room 3.0 the annotation is
// androidx.room3.Dao, so this matcher silently no-ops there. Unlike the owner-agnostic
// SQLiteDriverCallSiteVisitor, the DAO path is not resilient to Room's package rename.
override val fqName: String
get() = "androidx.room.Dao"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package io.sentry.android.gradle.instrumentation.androidx.sqlite.driver

import com.android.build.api.instrumentation.ClassContext
import io.sentry.android.gradle.instrumentation.ClassInstrumentable
import io.sentry.android.gradle.instrumentation.CommonClassVisitor
import io.sentry.android.gradle.instrumentation.MethodContext
import io.sentry.android.gradle.instrumentation.MethodInstrumentable
import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory
import io.sentry.android.gradle.instrumentation.androidx.sqlite.driver.visitor.SQLiteDriverCallSiteVisitor
import io.sentry.android.gradle.instrumentation.util.isSentryClass
import org.objectweb.asm.ClassVisitor
import org.objectweb.asm.MethodVisitor

class AndroidXSQLiteDriver : ClassInstrumentable {

override fun getVisitor(
instrumentableContext: ClassContext,
apiVersion: Int,
originalVisitor: ClassVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters,
): ClassVisitor {
val currentClassName = instrumentableContext.currentClassData.className

return CommonClassVisitor(
apiVersion = apiVersion,
classVisitor = originalVisitor,
className = currentClassName.substringAfterLast('.'),
methodInstrumentables = METHOD_INSTRUMENTABLES,
parameters = parameters,
)
}

// Broad activation: the setDriver call site can appear in any class. The MethodInstrumentable
// and MethodVisitor filter at the call-site level. Per-method overhead is one MethodVisitor
// construction; per-invoke overhead is one visitMethodInsn predicate evaluation.
override fun isInstrumentable(data: ClassContext): Boolean = !data.isSentryClass()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โš ๏ธ Checking every non-Sentry class for instrumentability is the most controversial implementation decision in this PR.

Some existing visitors do likewise, including logging and remapping โ€“ and logging is enabled for users by default. That makes the cost here incremental for most (db instrumentation is also enabled by default). But users who've disabled logging and enabled db instrumentation will be especially impacted.

I've run benchmarks against the DuckDuckGo app (see the PR description for details). They suggest that the impact is within reason (~1.5-2.5s extra for warm builds). We can get smarter if we hear differently, but the lift is non-trivial, so I've opted for the simple approach for now.


companion object {
private val METHOD_INSTRUMENTABLES: List<MethodInstrumentable> =
listOf(SetDriverCallSiteInstrumentable())
}
}

class SetDriverCallSiteInstrumentable : MethodInstrumentable {

override fun getVisitor(
instrumentableContext: MethodContext,
apiVersion: Int,
originalVisitor: MethodVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters,
): MethodVisitor = SQLiteDriverCallSiteVisitor(apiVersion, originalVisitor)

override fun isInstrumentable(data: MethodContext): Boolean = true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package io.sentry.android.gradle.instrumentation.androidx.sqlite.driver.visitor

import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Opcodes

/**
* Wraps the `SQLiteDriver` argument of `setDriver(SQLiteDriver)` calls with
* `SentrySQLiteDriver.create(...)`. Targeted at `RoomDatabase.Builder.setDriver()` but will match
* any `setDriver(Landroidx/sqlite/SQLiteDriver;)...` invocation. Keeps us robust to repackaging of
* the receiver, which has happened with Room.
*
* The SDK protects against duplicate wrappings, allowing the visitor to wrap the driver
* unconditionally.
*
* The injected INVOKESTATIC has net-zero stack effect (pops one SQLiteDriver, pushes one
* SQLiteDriver), so existing stack-map frames remain valid; COMPUTE_MAXS is sufficient.
*/
class SQLiteDriverCallSiteVisitor(apiVersion: Int, originalVisitor: MethodVisitor) :
MethodVisitor(apiVersion, originalVisitor) {

override fun visitMethodInsn(
opcode: Int,
owner: String?,
name: String?,
descriptor: String?,
isInterface: Boolean,
) {
// setDriver is always an instance call; skip name/descriptor checks on other opcodes.
if (opcode != Opcodes.INVOKEVIRTUAL && opcode != Opcodes.INVOKEINTERFACE) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
return
}

// startsWith b/c it lets us account for today's setDriver signature and accommodate (some)
// future extensions. We don't support the driver param at arbitrary locations b/c current
// wrapping logic assumes the driver will be on top of the JVM stack.
if (name == SET_DRIVER && descriptor?.startsWith(SET_DRIVER_DESCRIPTOR_PREFIX) == true) {
super.visitMethodInsn(
Opcodes.INVOKESTATIC,
SENTRY_SQLITE_DRIVER,
CREATE,
SENTRY_CREATE_DESCRIPTOR,
false,
)
}

super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
}

companion object {

private const val CREATE = "create"
private const val SENTRY_CREATE_DESCRIPTOR =
"(Landroidx/sqlite/SQLiteDriver;)Landroidx/sqlite/SQLiteDriver;"
private const val SENTRY_SQLITE_DRIVER = "io/sentry/sqlite/SentrySQLiteDriver"
private const val SET_DRIVER = "setDriver"
private const val SET_DRIVER_DESCRIPTOR_PREFIX = "(Landroidx/sqlite/SQLiteDriver;)"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,17 @@ abstract class SentryModulesService :
features.add("DexGuard")
}

if (isSQLiteDriverInstrEnabled()) {
features.add("SQLiteDriver")
}

return project.provider { features }
}

private fun isInstrumentationEnabled(feature: InstrumentationFeature): Boolean {
return when (feature) {
InstrumentationFeature.DATABASE -> isOldDatabaseInstrEnabled() || isNewDatabaseInstrEnabled()
InstrumentationFeature.DATABASE ->
isOldDatabaseInstrEnabled() || isNewDatabaseInstrEnabled() || isSQLiteDriverInstrEnabled()
InstrumentationFeature.FILE_IO -> isFileIOInstrEnabled()
InstrumentationFeature.OKHTTP -> isOkHttpInstrEnabled()
InstrumentationFeature.COMPOSE -> isComposeInstrEnabled()
Expand All @@ -72,6 +77,12 @@ abstract class SentryModulesService :
sentryModules.isAtLeast(SentryModules.SENTRY_ANDROID_SQLITE, SentryVersions.VERSION_SQLITE) &&
parameters.features.get().contains(InstrumentationFeature.DATABASE)

fun isSQLiteDriverInstrEnabled(): Boolean =
sentryModules.isAtLeast(
SentryModules.SENTRY_ANDROID_SQLITE,
SentryVersions.VERSION_SQLITE_DRIVER,
) && parameters.features.get().contains(InstrumentationFeature.DATABASE)

fun isOldDatabaseInstrEnabled(): Boolean =
!isNewDatabaseInstrEnabled() &&
sentryModules.isAtLeast(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ internal object SentryVersions {
internal val VERSION_LOGCAT = SemVer(6, 17, 0)
internal val VERSION_APP_START = SemVer(7, 1, 0)
internal val VERSION_SQLITE = SemVer(6, 21, 0)
internal val VERSION_SQLITE_DRIVER = SemVer(8, 44, 0)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Update version number once we ship the SentrySQLiteDriver in sentry-android.

internal val VERSION_ANDROID_OKHTTP_LISTENER = SemVer(6, 20, 0)
internal val VERSION_OKHTTP = SemVer(7, 0, 0)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.SQLiteDriver;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

public class FactoryReturn {

public void build() {
new RoomDatabase.Builder().setDriver(provideDriver());
}

private SQLiteDriver provideDriver() {
return new BundledSQLiteDriver();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

/** setDriver argument loaded from an instance field. */
public class FieldLoad {

private BundledSQLiteDriver driver;

public void build() {
new RoomDatabase.Builder().setDriver(driver);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.SQLiteDriver;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

/**
* A {@code checkcast} to {@code SQLiteDriver} sits immediately before the {@code setDriver} call
* site (here forced via an {@code Object}-typed local). This mirrors the bytecode kotlinc emits for
* an inferred-type local and verifies the visitor still inserts the wrap when a {@code checkcast}
* precedes the invocation.
*/
public class InferredLocal {

public void build() {
Object driver = new BundledSQLiteDriver();
new RoomDatabase.Builder().setDriver((SQLiteDriver) driver);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import androidx.room.RoomDatabase
import androidx.sqlite.driver.bundled.BundledSQLiteDriver

/**
* Kotlin reference showing the idiomatic equivalent of [InferredLocal.java]. The compiled `.class`
* used by tests comes from the Java source; this file is kept for documentation only (it shows the
* `val driver = BundledSQLiteDriver()` pattern that makes kotlinc emit a `checkcast` to
* `SQLiteDriver` at the `setDriver` call site).
*/
class InferredLocal {
fun configure(builder: RoomDatabase.Builder<RoomDatabase>) {
val driver = BundledSQLiteDriver()
builder.setDriver(driver)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.db.SupportSQLiteOpenHelper;
import androidx.sqlite.driver.SupportSQLiteDriver;

/** SupportSQLiteDriver bridge constructed inline: a SupportSQLiteDriver wrapping a SupportSQLiteOpenHelper. */
public class InlineBridge {

private SupportSQLiteOpenHelper helper;

public void build() {
new RoomDatabase.Builder().setDriver(new SupportSQLiteDriver(helper));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

/** Driver constructed inline as the setDriver argument. */
public class InlineConstruction {

public void build() {
new RoomDatabase.Builder().setDriver(new BundledSQLiteDriver());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

public class InvokeInterface {

public void build() {
RoomDatabase.SetDriverReceiver receiver = new RoomDatabase.Builder();
receiver.setDriver(new BundledSQLiteDriver());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.SQLiteDriver;
import androidx.sqlite.db.SupportSQLiteOpenHelper;
import androidx.sqlite.driver.SupportSQLiteDriver;

/** Erased-local SupportSQLiteDriver bridge: SQLiteDriver-typed local holding a SupportSQLiteDriver instance. */
public class LocalTypedAsBridge {

private final SupportSQLiteOpenHelper helper;

public LocalTypedAsBridge(SupportSQLiteOpenHelper helper) {
this.helper = helper;
}

public void build() {
SQLiteDriver driver = new SupportSQLiteDriver(helper);
new RoomDatabase.Builder().setDriver(driver);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.example;

import androidx.room.RoomDatabase;
import androidx.sqlite.driver.bundled.BundledSQLiteDriver;

/** Local typed as the concrete BundledSQLiteDriver impl, not the SQLiteDriver interface. */
public class LocalTypedAsImpl {

public void build() {
BundledSQLiteDriver driver = new BundledSQLiteDriver();
new RoomDatabase.Builder().setDriver(driver);
}
}
Loading
Loading