Address Roman's comments

 - Merge SQLiteSpanHelper + SQLiteSpanRecorder into a single SQLiteSpanInstrumentation class.
 - DRY out reference to file name path separators.

Author: 0xadam-brown

sentry-android-sqlite/README.md

@@ -13,8 +13,8 @@ Please consult the [Sentry Docs](https://docs.sentry.io/platforms/android/integr
 
 This module is organized as two separate packages:
 
-- **`io.sentry.android.sqlite`**: Android-specific code. Classes here depend on `android.database.*` (e.g., `CrossProcessCursor`, `SQLException`) and/or on `androidx.sqlite.db.*`, the Android-only compatibility layer over the platform's SQLite. The `SentrySupportSQLiteOpenHelper` path and its span helper `SQLiteSpanManager` live here.
-- **`io.sentry.sqlite`**: Code whose contract depends only on the multiplatform `androidx.sqlite.*` interfaces (e.g., `SQLiteDriver` and `SQLiteConnection`). `SentrySQLiteDriver` and its span helper `SQLiteSpanRecorder` live here.
+- **`io.sentry.android.sqlite`**: Android-specific code. Classes here depend on `android.database.*` (e.g., `CrossProcessCursor`, `SQLException`) and/or on `androidx.sqlite.db.*`, the Android-only compatibility layer over the platform's SQLite. The `SentrySupportSQLiteOpenHelper` path and its `SQLiteSpanManager` wrapper live here.
+- **`io.sentry.sqlite`**: Code whose contract depends only on the multiplatform `androidx.sqlite.*` interfaces (e.g., `SQLiteDriver` and `SQLiteConnection`). `SentrySQLiteDriver` and shared span instrumentation via `SQLiteSpanInstrumentation` live here.
 
 The split anticipates the possibility of future Kotlin Multiplatform support. The `androidx.sqlite.*` driver interfaces are defined in the library's `commonMain` source set and are reused by Room across Android, JVM, and native targets. Classes in `io.sentry.sqlite` are written against those portable interfaces and are intended to lift cleanly into a KMP `commonMain` source set if/when the `sentry` core gains multiplatform targets. Classes in `io.sentry.android.sqlite` are Android-only by construction and will stay where they are.
 

sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SQLiteSpanManager.kt

@@ -3,19 +3,17 @@ package io.sentry.android.sqlite
 import android.database.CrossProcessCursor
 import android.database.SQLException
 import io.sentry.IScopes
-import io.sentry.ISpan
 import io.sentry.ScopesAdapter
 import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.SpanStatus
-import io.sentry.sqlite.SQLiteSpanHelper
-import io.sentry.sqlite.dbMetadataFromDatabaseName
+import io.sentry.sqlite.SQLiteSpanInstrumentation
 
 internal class SQLiteSpanManager(
   private val scopes: IScopes = ScopesAdapter.getInstance(),
   databaseName: String? = null,
 ) {
 
-  private val spanHelper = SQLiteSpanHelper(scopes, dbMetadataFromDatabaseName(databaseName))
+  private val spans = SQLiteSpanInstrumentation.fromDatabaseName(databaseName, scopes)
 
   init {
     SentryIntegrationPackageStorage.getInstance().addIntegration("SQLite")
@@ -31,8 +29,8 @@ internal class SQLiteSpanManager(
   @Suppress("TooGenericExceptionCaught", "UNCHECKED_CAST")
   @Throws(SQLException::class)
   fun <T> performSql(sql: String, operation: () -> T): T {
-    val startTimestamp = scopes.getOptions().dateProvider.now()
-    var span: ISpan? = null
+    val startTimestamp = spans.startTimestamp()
+
     return try {
       val result = operation()
       /*
@@ -43,19 +41,11 @@ internal class SQLiteSpanManager(
       if (result is CrossProcessCursor) {
         return SentryCrossProcessCursor(result, this, sql) as T
       }
-      span = spanHelper.startSpan(sql, startTimestamp)
-      span?.status = SpanStatus.OK
+      spans.recordSpan(sql, startTimestamp, SpanStatus.OK)
       result
     } catch (e: Throwable) {
-      span = spanHelper.startSpan(sql, startTimestamp)
-      span?.status = SpanStatus.INTERNAL_ERROR
-      span?.throwable = e
+      spans.recordSpan(sql, startTimestamp, SpanStatus.INTERNAL_ERROR, e)
       throw e
-    } finally {
-      span?.let {
-        spanHelper.applyDataToSpan(it)
-        it.finish()
-      }
     }
   }
 }

sentry-android-sqlite/src/main/java/io/sentry/sqlite/DbMetadata.kt

@@ -33,7 +33,7 @@ internal fun dbMetadataFromFileName(fileName: String): DbMetadata {
     return DbMetadata(name = null, system = DB_SYSTEM_IN_MEMORY)
   }
 
-  val trimmed = fileName.trimEnd('/', '\\')
+  val trimmed = fileName.trimEnd { it in FILE_NAME_PATH_SEPARATORS }
   if (trimmed.isEmpty()) {
     return DbMetadata(name = null, system = DB_SYSTEM_SQLITE)
   }

sentry-android-sqlite/src/main/java/io/sentry/sqlite/SQLiteSpanHelper.kt

@@ -0,0 +1,85 @@
+package io.sentry.sqlite
+
+import io.sentry.IScopes
+import io.sentry.Instrumenter
+import io.sentry.ScopesAdapter
+import io.sentry.SentryDate
+import io.sentry.SentryLongDate
+import io.sentry.SentryStackTraceFactory
+import io.sentry.SpanDataConvention
+import io.sentry.SpanStatus
+
+private const val SQLITE_TRACE_ORIGIN = "auto.db.sqlite"
+
+/** Shared span creation and metadata for SQLite instrumentation. */
+internal class SQLiteSpanInstrumentation(
+  private val scopes: IScopes,
+  private val dbMetadata: DbMetadata,
+) {
+
+  private val stackTraceFactory = SentryStackTraceFactory(scopes.options)
+
+  /**
+   * Returns a start timestamp for a `db.sql.query` span.
+   *
+   * Exposed so callers can capture a wall-clock start before accumulating database time.
+   * Internalizing the start time in [recordSpan] would shift spans to end-of-work on the trace
+   * timeline, which is less desirable.
+   */
+  fun startTimestamp(): SentryDate = scopes.options.dateProvider.now()
+
+  /** Records a `db.sql.query` span from [startTimestamp] to the moment of invocation. */
+  fun recordSpan(
+    sql: String,
+    startTimestamp: SentryDate,
+    status: SpanStatus,
+    throwable: Throwable? = null,
+  ) {
+    recordSpan(sql, startTimestamp, endTimestamp = null, status, throwable)
+  }
+
+  /** Records a `db.sql.query` span from [startTimestamp] to [startTimestamp] + [durationNanos]. */
+  fun recordSpan(
+    sql: String,
+    startTimestamp: SentryDate,
+    durationNanos: Long,
+    status: SpanStatus,
+    throwable: Throwable? = null,
+  ) {
+    val endTimestamp = SentryLongDate(startTimestamp.nanoTimestamp() + durationNanos)
+    recordSpan(sql, startTimestamp, endTimestamp, status, throwable)
+  }
+
+  private fun recordSpan(
+    sql: String,
+    startTimestamp: SentryDate,
+    endTimestamp: SentryDate?,
+    status: SpanStatus,
+    throwable: Throwable?,
+  ) {
+    scopes.span?.startChild("db.sql.query", sql, startTimestamp, Instrumenter.SENTRY)?.apply {
+      spanContext.origin = SQLITE_TRACE_ORIGIN
+      throwable?.let { this.throwable = it }
+
+      val isMainThread = scopes.options.threadChecker.isMainThread
+      setData(SpanDataConvention.BLOCKED_MAIN_THREAD_KEY, isMainThread)
+
+      if (isMainThread) {
+        setData(SpanDataConvention.CALL_STACK_KEY, stackTraceFactory.inAppCallStack)
+      }
+
+      dbMetadata.name?.let { setData(SpanDataConvention.DB_NAME_KEY, it) }
+      setData(SpanDataConvention.DB_SYSTEM_KEY, dbMetadata.system)
+      finish(status, endTimestamp)
+    }
+  }
+
+  companion object {
+
+    fun fromDatabaseName(databaseName: String?, scopes: IScopes = ScopesAdapter.getInstance()) =
+      SQLiteSpanInstrumentation(scopes, dbMetadataFromDatabaseName(databaseName))
+
+    fun fromFileName(fileName: String, scopes: IScopes = ScopesAdapter.getInstance()) =
+      SQLiteSpanInstrumentation(scopes, dbMetadataFromFileName(fileName))
+  }
+}

sentry-android-sqlite/src/main/java/io/sentry/sqlite/SQLiteSpanInstrumentation.kt

@@ -5,12 +5,11 @@ import androidx.sqlite.SQLiteStatement
 
 internal class SentrySQLiteConnection(
   private val delegate: SQLiteConnection,
-  private val spanRecorder: SQLiteSpanRecorder,
+  private val spans: SQLiteSpanInstrumentation,
 ) : SQLiteConnection by delegate {
 
   override fun prepare(sql: String): SQLiteStatement {
     val statement = delegate.prepare(sql)
-    return statement as? SentrySQLiteStatement
-      ?: SentrySQLiteStatement(statement, spanRecorder, sql)
+    return statement as? SentrySQLiteStatement ?: SentrySQLiteStatement(statement, spans, sql)
   }
 }

sentry-android-sqlite/src/main/java/io/sentry/sqlite/SQLiteSpanRecorder.kt

@@ -40,10 +40,10 @@ public class SentrySQLiteDriver private constructor(private val delegate: SQLite
     val connection = delegate.open(fileName)
 
     return try {
-      val spanRecorder = SQLiteSpanRecorder(fileName)
-      // create() ensures delegate is unwrapped, so we don't protect against double-wrapping the
-      // connection.
-      SentrySQLiteConnection(connection, spanRecorder)
+      val spans = SQLiteSpanInstrumentation.fromFileName(fileName)
+      // create() ensures delegate is unwrapped, so we don't need to protect against double-wrapping
+      // the connection.
+      SentrySQLiteConnection(connection, spans)
     } catch (t: Throwable) {
       ScopesAdapter.getInstance()
         .options

sentry-android-sqlite/src/main/java/io/sentry/sqlite/SentrySQLiteConnection.kt

@@ -18,7 +18,7 @@ import io.sentry.SpanStatus
  */
 internal class SentrySQLiteStatement(
   private val delegate: SQLiteStatement,
-  private val spanRecorder: SQLiteSpanRecorder,
+  private val spans: SQLiteSpanInstrumentation,
   private val sql: String,
   private val nanoTimeProvider: () -> Long = { System.nanoTime() },
 ) : SQLiteStatement by delegate {
@@ -37,7 +37,7 @@ internal class SentrySQLiteStatement(
     val beforeNanos = nanoTimeProvider()
     return try {
       if (firstStepTimestamp == null) {
-        firstStepTimestamp = spanRecorder.startTimestamp()
+        firstStepTimestamp = spans.startTimestamp()
       }
 
       stepsComplete = !delegate.step()
@@ -76,6 +76,6 @@ internal class SentrySQLiteStatement(
     val duration = accumulatedDbNanos
     firstStepTimestamp = null
     accumulatedDbNanos = 0L
-    spanRecorder.recordSpan(sql, start, duration, status, throwable)
+    spans.recordSpan(sql, start, duration, status, throwable)
   }
 }

sentry-android-sqlite/src/main/java/io/sentry/sqlite/SentrySQLiteDriver.kt

@@ -16,7 +16,7 @@ import kotlin.test.assertTrue
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
 
-class SQLiteSpanRecorderTest {
+class SQLiteSpanInstrumentationTest {
 
   private class Fixture {
 
@@ -27,14 +27,14 @@ class SQLiteSpanRecorderTest {
     fun getSut(
       isTransactionActive: Boolean = true,
       fileName: String = ":memory:",
-    ): SQLiteSpanRecorder {
+    ): SQLiteSpanInstrumentation {
       options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" }
       whenever(scopes.options).thenReturn(options)
       sentryTracer = SentryTracer(TransactionContext("name", "op"), scopes)
       if (isTransactionActive) {
         whenever(scopes.span).thenReturn(sentryTracer)
       }
-      return SQLiteSpanRecorder(fileName, scopes)
+      return SQLiteSpanInstrumentation.fromFileName(fileName, scopes)
     }
   }
 
@@ -147,10 +147,47 @@ class SQLiteSpanRecorderTest {
   }
 
   @Test
-  fun `recordSpan does not throw if span recording fails`() {
+  fun `recordSpan without a duration finishes the span at the time of invocation`() {
     val sut = fixture.getSut()
-    whenever(fixture.scopes.span).thenThrow(RuntimeException("span unavailable"))
+    val start = sut.startTimestamp()
 
-    sut.recordSpan("SELECT 1", sut.startTimestamp(), 1_000_000, SpanStatus.OK)
+    sut.recordSpan("SELECT 1", start, SpanStatus.OK)
+
+    val span = fixture.sentryTracer.children.first()
+    assertTrue(span.isFinished)
+    assertEquals(SpanStatus.OK, span.status)
+    // Unlike the duration overload, no synthetic end timestamp is supplied; the span finishes at
+    // "now", i.e. at or after its start.
+    assertTrue(span.finishDate!!.nanoTimestamp() >= start.nanoTimestamp())
+  }
+
+  @Test
+  fun `fromFileName sets db name from fileName`() {
+    val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" }
+    whenever(fixture.scopes.options).thenReturn(options)
+    fixture.sentryTracer = SentryTracer(TransactionContext("name", "op"), fixture.scopes)
+    whenever(fixture.scopes.span).thenReturn(fixture.sentryTracer)
+
+    val sut = SQLiteSpanInstrumentation.fromFileName("tracks.db", fixture.scopes)
+    sut.recordSpan("SELECT 1", sut.startTimestamp(), SpanStatus.OK)
+
+    val span = fixture.sentryTracer.children.first()
+    assertEquals("sqlite", span.data[SpanDataConvention.DB_SYSTEM_KEY])
+    assertEquals("tracks.db", span.data[SpanDataConvention.DB_NAME_KEY])
+  }
+
+  @Test
+  fun `fromDatabaseName sets db name from databaseName`() {
+    val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" }
+    whenever(fixture.scopes.options).thenReturn(options)
+    fixture.sentryTracer = SentryTracer(TransactionContext("name", "op"), fixture.scopes)
+    whenever(fixture.scopes.span).thenReturn(fixture.sentryTracer)
+
+    val sut = SQLiteSpanInstrumentation.fromDatabaseName("tracks.db", fixture.scopes)
+    sut.recordSpan("SELECT 1", sut.startTimestamp(), SpanStatus.OK)
+
+    val span = fixture.sentryTracer.children.first()
+    assertEquals("sqlite", span.data[SpanDataConvention.DB_SYSTEM_KEY])
+    assertEquals("tracks.db", span.data[SpanDataConvention.DB_NAME_KEY])
   }
 }