chore(android-sqlite): Skip wrapping SupportSQLiteDriver bridge to avoid duplicate spans#5514
Open
0xadam-brown wants to merge 3 commits into
Open
Conversation
…oid duplicate spans
SentrySQLiteDriver.create() now recognizes the Room 2.7+ androidx.sqlite.driver.SupportSQLiteDriver bridge adapter and returns it unwrapped. That lets us protect against the one known vector where using both SentrySQLiteDriver and SentrySupportSQLiteOpenHelper with the same db table is allowed under either the Room or SQLDelight APIs:
```kotlin
// AVOID — this configuration produces duplicate spans for every SQL statement.
// Step 1: Developer wraps their open helper with Sentry, either manually or
// via the Sentry Android Gradle Plugin.
val sentryWrappedHelper: SupportSQLiteOpenHelper =
SentrySupportSQLiteOpenHelper.create(
FrameworkSQLiteOpenHelperFactory().create(configuration)
)
// Step 2: Developer builds the compat driver around that wrapped helper.
val driver: SQLiteDriver = SupportSQLiteDriver(sentryWrappedHelper)
// Step 3: Developer (wrongly!) wraps the driver with Sentry as well. All
// spans will now be duplicated.
val sentryWrappedDriver: SQLiteDriver = SentrySQLiteDriver.create(driver)
Room.databaseBuilder(context, MyDb::class.java, "mydb")
.setDriver(sentryWrappedDriver)
.build()
```
This commit lets us avoid step 3 by no-op'ing if a developer tries to pass a SupportSQLiteDriver to SentrySQLiteDriver.create().
… sample app to exercise duplicate-span guard Replace the two-way integration switch with a three-way segmented control and manually construct the SupportSQLiteDriver stack so reviewers can confirm a single helper-layer span per statement when users access their db files through the SupportSQLiteDriver API.
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 804f413. Configure here.
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 50412b5 | 313.85 ms | 353.56 ms | 39.71 ms |
| 3f03258 | 321.26 ms | 361.84 ms | 40.58 ms |
| f6e3213 | 291.28 ms | 357.63 ms | 66.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 50412b5 | 0 B | 0 B | 0 B |
| 3f03258 | 0 B | 0 B | 0 B |
| f6e3213 | 0 B | 0 B | 0 B |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

📜 Description
PR protects against generating duplicate
db.sql.queryspans when using theandroidx.sqlite.driver.SupportSQLiteDriverbridge.💡 Motivation and Context
The bridge is the sole Room (or SQLDelight) API that leaves open the possibility of using
SentrySQLiteDriverandSentrySupportSQLiteOpenHelpertogether with the same db file – a possibility that would lead to redundant spans.Example misuse this PR guards against
Addresses JAVA-275
[1] I've covered the no-span edge case via documentation: It's possible to use
SupportSQLiteDriverwithout wrapping the open helper passed via its constructor, resulting in no spans being produced. That a temporary concern, asSupportSQLiteDrivergoes away with Room 3.0+; the blast radius should also be limited, as most of our users rely on the Sentry Android Gradle Plugin to auto-wrap the open helper. I've flagged the issue in the CHANGELOG and theSentrySQLiteDriverKDoc.[2] Support driver has open helper span semantics: Users who rely on
SupportSQLiteDriverwill have their spans auto-generated via the open helper, and so the span semantics will match its behavior rather than that of SQLiteDriver (for more details about SQLiteDriver's span policy, see here). (Wrapping the SQLiteDriver would've required reflection, which I wanted to avoid for performance reasons and to make it easier to eventually lift the driver into KMP common.)[3] Changes in this PR are needed if we want to use ASM auto-wrapping: I've a forthcoming PR that introduces ASM auto-wrapping of
SQLiteDrivervia the Sentry Android Gradle Plugin. The approach in this PR makes that auto-wrapping safe, as SAGP will always convert theSQLiteDriverpassed toRoomDatabase.Builder.setDriver(SQLiteDriver)intoSentrySQLiteDriver.create(SQLiteDriver). If the provided driver is aSupportSQLiteDriver, this PR ensuresSentrySQLiteDriver.create(SQLiteDriver)will return it unwrapped.💚 How did you test it?
[1] Created a new "Bridge" section in the Android sample app that wires up the
SupportSQLiteDriverand provides it with both aSentrySupportSQLiteOpenHelperand aSentrySQLiteDriver:[2] Verified via the Sentry UI that spans are not duplicated:
Baseline: Existing behavior of SentrySupportSQLiteOpenHelper
This PR: Behavior of SupportSQLiteDriver
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps