feat(android-sqlite): Add SentrySQLiteDriver (JAVA-275)#5466
feat(android-sqlite): Add SentrySQLiteDriver (JAVA-275)#54660xadam-brown wants to merge 6 commits into
Conversation
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 62b579c | 349.26 ms | 426.26 ms | 77.00 ms |
| d501a7e | 348.06 ms | 431.42 ms | 83.36 ms |
| cf708bd | 434.73 ms | 502.96 ms | 68.22 ms |
| 2195398 | 351.77 ms | 433.22 ms | 81.45 ms |
| cf708bd | 408.35 ms | 458.98 ms | 50.63 ms |
| e2dce0b | 308.96 ms | 360.10 ms | 51.14 ms |
| 5dee26b | 336.02 ms | 402.62 ms | 66.60 ms |
| 4c04bb8 | 333.16 ms | 408.16 ms | 75.00 ms |
| a1eadfa | 345.67 ms | 411.26 ms | 65.59 ms |
| 5b1a06b | 352.27 ms | 413.70 ms | 61.43 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 62b579c | 0 B | 0 B | 0 B |
| d501a7e | 0 B | 0 B | 0 B |
| cf708bd | 1.58 MiB | 2.11 MiB | 539.71 KiB |
| 2195398 | 0 B | 0 B | 0 B |
| cf708bd | 1.58 MiB | 2.11 MiB | 539.71 KiB |
| e2dce0b | 0 B | 0 B | 0 B |
| 5dee26b | 0 B | 0 B | 0 B |
| 4c04bb8 | 0 B | 0 B | 0 B |
| a1eadfa | 0 B | 0 B | 0 B |
| 5b1a06b | 0 B | 0 B | 0 B |
Previous results on branch: feat/support-sqlite-driver
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 703e7ae | 350.72 ms | 415.76 ms | 65.03 ms |
| f8d2380 | 309.09 ms | 365.52 ms | 56.43 ms |
| 2a55b58 | 365.41 ms | 434.64 ms | 69.23 ms |
| 4993a1b | 303.45 ms | 392.65 ms | 89.20 ms |
| 9ed1dfc | 310.92 ms | 361.74 ms | 50.82 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 703e7ae | 0 B | 0 B | 0 B |
| f8d2380 | 0 B | 0 B | 0 B |
| 2a55b58 | 0 B | 0 B | 0 B |
| 4993a1b | 0 B | 0 B | 0 B |
| 9ed1dfc | 0 B | 0 B | 0 B |
a590992 to
0084312
Compare
Introduces support for AndroidX's SQLiteDriver via a new SentrySQLiteDriver wrapper. SentrySQLiteDriver automatically creates spans for each SQL statement it executes, and its data scheme closely tracks that of SentrySupportSQLiteOpenHelper, which it's designed to replace. (Span duration is an important exception; see the SentrySQLiteStatement KDoc for more details.) A key motivation for Google's using SQLiteDriver with Room 2.7+ was Kotlin Multiplatform support. We've been careful to keep the SentrySQLiteDriver KMP-compatible as well, should we one day want to lift it into sentry-kotlin-multiplatform. --- Co-authored-by: Angus Holder <7407345+angusholder@users.noreply.github.com>
4c87cb9 to
4c67ea9
Compare
Method reference `System::nanoTime` compiles to FunctionReferenceImpl, which breaks R8 in the SDK size test app.
markushi
left a comment
There was a problem hiding this comment.
Looks very promising already, left a few comments. Once we've decided what execution phases spans should cover, I'll have a second look.
romtsn
left a comment
There was a problem hiding this comment.
Looks great, nice analysis and research, kudos!
Some more things I'd want to clarify before approving:
- do we actually need to bump
androidx.sqliteversion that we compile against in the module to support this new stuff? - what's the migration path? do we want to keep them side-by-side in the same module, or would it make sense to introduce
sentry-android-sqlite3(or whatever fits best) to keep them separate? would probably also simplify the future KMP work (if ever) - you said you're yet to test it out with SAGP and auto-instrumentation, which is great! I think I would want to also make SAGP instrumentation of the new Driver a part of the success criteria, because this is the main method for our users to get room/sqlite instrumented (obviously it doesn't have to block this PR)
|
Thanks for the excellent feedback / comments @romtsn. Answers to your questions + one question about single vs multiple modules on my end... > do we actually need to bump
Scratch that^^. I've now bumped to 2.6.2 to pick up the introduction Fyi, I'll wait to merge all the PRs together so we make sure they land in the same release. > what's the migration path? do we want to keep them side-by-side in the same module, or would it make sense to introduce u This ended up being an important question (thx!). I propose keeping the driver in the existing Here are the migration steps I had in mind:
// build.gradle in sentry-android-sqlite (if the driver lives with the open helper) or
// sentry-sqlite (if we give the driver its own module)
dependencies {
api("io.sentry:sentry-kmp-sqlite")
}Creating a new module doesn't meaningfully simplify future KMP work because we'll always have to relocate the driver somewhere + redirect from the existing module's build.gradle script. (Eg, if we want to publish the driver in a JAR for non-Android JVM consumers / desktop, we could create a new module then and redirect from sentry-android-sqlite; if we want to go straight for common KMP, we do likewise, just in sentry-kotlin-multiplatform.) See the expandable "migration path to KMP" discussion below for more details.
cc @markushi Step-by-step migration path to KMP: Click to expandMigration path
User experience
KMP migration shape for both paths is identical: no code change so long as we can keep the package name in sentry-kotlin-multiplatform; dependency redirect means only version bump needed on existing coordinate. > you said you're yet to test it out with SAGP and auto-instrumentation, which is great! I think I would want to also make SAGP instrumentation of the new Driver a part of the success criteria, because this is the main method for our users to get room/sqlite instrumented (obviously it doesn't have to block this PR) Good deal. I'll plan to implement SAGP auto-instrumentation of |
- Merge SQLiteSpanHelper + SQLiteSpanRecorder into a single SQLiteSpanInstrumentation class. - DRY out reference to file name path separators.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d4ab6b7. Configure here.
| get() = | ||
| try { | ||
| delegate.hasConnectionPool | ||
| } catch (_: LinkageError) { |
There was a problem hiding this comment.
I see that this is the only reference to a LinkageError in our code base. Do we normally handle these sorts of issues differently?
*** "These sorts of issues" = A compileOnly dependency against an API that's added methods as its versions evolve. In this case, androidx.sqlite 2.6.0 added SQLiteDriver.hasConnectionPool. Because we don't include our own androidx.sqlite dep at runtime, we need to account for users who bring in versions with and without hasConnectionPool.
There was a problem hiding this comment.
I see we had a similar one here: #4597 which we just fixed by bumping the version, but apparently we don't account for older versions, so I guess it just breaks there?

📜 Description
Introduces
SentrySQLiteDriverfor wrapping and instrumentingandroidx.sqlite.SQLiteDriver. Like our existingSentrySupportSQLiteOpenHelper, the new wrapper produces a span per executed SQL statement.Example use:
💡 Motivation and Context
Room 2.7 introduced the
SQLiteDriverAPI as a replacement forSupportSQLiteOpenHelper; Room 3.0+ makes use ofSQLiteDrivermandatory.A key motivation behind Google's introduction of
SQLiteDriverwas Kotlin Multiplatform compatibility. This PR makesSentrySQLiteDriveravailable on Android only, but the wrapper has been packaged so that we can lift it into our KMP module in the future without having to break clients.Addresses JAVA-275.
[1] Unlike
SentrySupportSQLiteOpenHelper, theSentrySQLiteDriveris not automatically wrapped via the sentry-android-gradle-plugin. (We can add byte code support later if we want – or do it now if there's a strong interest.)[2] API is not marked as
@Experimental. (Super small surface area: acreate(SQLiteDriver)constructor; future additions are non-breaking; sufficient alignment withSentrySupportSQLiteOpenHelperdata model. That said, chime in if you think we should add it.)[3] Behavior differences vs
SentrySupportSQLiteHelperClick to expand
Behavior differences
SentrySupportSQLiteOpenHelper(old)SentrySQLiteDriver(new)performSqlcall (incl. app time during cursor materialization)step()db time only – app time between steps excludedgetCount/onMove/fillWindowtimed; later rows untimedstep()contributes to cumulative span timestep()of the cycledb.namederivationdatabaseName(for Room, the builder name, e.g.,"tracks"fromdatabaseBuilder(ctx, MyDb, "tracks")).File(fileName).name— the on-disk filename of the path Room passes todriver.open()(e.g.,"tracks.db").db.namevalues during migration. Both paths report data correctly, but will attribute it to different sources.execSQL("CREATE TABLE …; INSERT …; INSERT …;")produces one span whose description is the full script.prepare(...), so multi-statement scripts must be split by Room (or the caller) into separate prepare/step cycles → one span per statement.[4] Risk of duplicate spans with Room's bridge adapter
SentrySQLiteDriverprotects internally against duplicate span creation should a developer try to double-wrap it or any of its components. Room and SQLDelight ensure that use of the driver andSentrySupportSQLiteOpenHelperare mutually exclusive at the API level in virtually all instances, save for one:Room 2.7+ (but not Room 3.0+) exposes a bridge adapter (
SupportSQLiteDriver) that lets users delegate to aSQLiteDriverfrom an existingSupportSQLiteOpenHelper. Double-wrapping both components is possible if folks aren't mindful.I'll be updating the Sentry Docs to warn users against wrapping both adapter components. Another option is to log an error, as mentioned here.
Updates to Sentry Docs: Click to expand
Avoiding duplicate spans with Room 2.7+
AndroidX ships an adapter class,
SupportSQLiteDriver, that lets developers bridge an existingSupportSQLiteOpenHelperto aSQLiteDriverthat Room 2.7+ accepts. Do not wrap both the open helper and the driver. (Remember that the Sentry Android Gradle Plugin will wrap the open helper for you at the byte code level if enabled.) If you double-wrap, you'll produce duplicate spans for every SQL statement:💚 How did you test it?
Unit tests cover:
SentrySQLiteDriverdelegation and wrappingSentrySQLiteConnectiondelegation and wrappingSentrySQLiteStatementspan creation, cancellation, and success/error taggingSQLiteSpanRecorderspan lifecycle (start/finish/cancel)I also dog-fooded
SentrySQLiteDriveron my own example app via Maven local artifact + I verified spans are displayed in Sentry UI.The legacy open helper didn't have the above, so I followed suit. Let me know if you think any is worth it and I'll be happy to address (eg, real Room db test, which would require a test dependency on androidx.sqlite:sqlite-bundled but wouldn't require Robolectric).
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
SQLiteDrivervia the sentry-android-gradle-plugin at some point (not currently on the roadmap).