chore(samples-android): Adapt SQLite demo screen to SAGP build mode#5568
Open
0xadam-brown wants to merge 1 commit into
Open
chore(samples-android): Adapt SQLite demo screen to SAGP build mode#55680xadam-brown wants to merge 1 commit into
0xadam-brown wants to merge 1 commit into
Conversation
Exposes a `BuildConfig.USE_SAGP` property from the recently introduced -PuseSagp flag ([#5538](#5538)). Lets us update the SQLite screen in the Android sample app so that it swizzles between auto-instrumenting vs manually wrapping `SQLiteDriver`, depending on the whether SAGP was applied to the build.
0xadam-brown
requested review from
adinauer,
markushi,
romtsn and
runningcode
as code owners
📲 Install BuildsAndroid
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+744
to
+746
| SqlDemo.DRIVER_DIRECT -> SAGP_DIRECT_DRIVER_MESSAGE | ||
| else -> null | ||
| } |
There was a problem hiding this comment.
Bug: The sagpDisabledReason function incorrectly leaves the BRIDGE_DIRECT button enabled in SAGP mode, even though its underlying direct SQLiteDriver usage is not instrumented, misleading testers.
Severity: LOW
Suggested Fix
Update the when expression within the sagpDisabledReason function to also handle the SqlDemo.BRIDGE_DIRECT case when BuildConfig.USE_SAGP is true. This will ensure the button for this demo is disabled, correctly reflecting that direct SQLiteDriver usage is not auto-instrumented by SAGP.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/sqlite/SQLiteActivity.kt#L744-L746
Potential issue: When the sample app is built with `USE_SAGP = true`, the
`sagpDisabledReason` function only disables the button for `DRIVER_DIRECT` usage. It
fails to also disable the `BRIDGE_DIRECT` button. The `BRIDGE_DIRECT` case also uses
`SQLiteDriver` directly without Room, a scenario which the Sentry Android Gradle Plugin
(SAGP) does not instrument. This results in a misleading UI where the `BRIDGE_DIRECT`
button is enabled and its subtitle suggests 'SAGP auto-wrap', but clicking it produces
no Sentry spans, creating a false negative for testers.
Did we get this right? 👍 / 👎 to inform future reviews.
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a416a65 | 333.78 ms | 410.37 ms | 76.59 ms |
| d15471f | 304.55 ms | 408.43 ms | 103.87 ms |
| ca6b6d8 | 380.45 ms | 460.38 ms | 79.93 ms |
| c8125f3 | 397.65 ms | 485.14 ms | 87.49 ms |
| b936425 | 302.69 ms | 372.86 ms | 70.17 ms |
| ed33deb | 334.19 ms | 362.30 ms | 28.11 ms |
| cf708bd | 408.35 ms | 458.98 ms | 50.63 ms |
| eb95ded | 317.51 ms | 369.08 ms | 51.57 ms |
| a5ab36f | 320.47 ms | 389.77 ms | 69.30 ms |
| 80fd6ad | 321.06 ms | 375.79 ms | 54.73 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a416a65 | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| ca6b6d8 | 0 B | 0 B | 0 B |
| c8125f3 | 1.58 MiB | 2.10 MiB | 532.32 KiB |
| b936425 | 0 B | 0 B | 0 B |
| ed33deb | 1.58 MiB | 2.13 MiB | 559.52 KiB |
| cf708bd | 1.58 MiB | 2.11 MiB | 539.71 KiB |
| eb95ded | 0 B | 0 B | 0 B |
| a5ab36f | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| 80fd6ad | 0 B | 0 B | 0 B |
runningcode
approved these changes
Jun 19, 2026
runningcode
left a comment
runningcode
left a comment
Contributor
There was a problem hiding this comment.
looks great! love the debugging UI with the indication of how the app was built!
| versionCode = 2 | ||
| versionName = project.version.toString() | ||
|
|
||
| buildConfigField( |
Contributor
There was a problem hiding this comment.
nit: i thought they created a new API that takes a provider here instead of eagerly resolving. not a big deal here since it doesn't matter either way for performance.
| } | ||
|
|
||
| @androidx.compose.runtime.Composable | ||
| @Composable |
Contributor
There was a problem hiding this comment.
thanks for fixing! im surprised spotless doesn't have a check for this.
| private val SECTION_HEADER_HEIGHT = 28.dp | ||
|
|
||
| private const val SAGP_DIRECT_DRIVER_MESSAGE = | ||
| "SAGP doesn't auto-instrument SQLiteDriver for direct use" |
Contributor
There was a problem hiding this comment.
this is because it isn't supported right? If so I suggest this to differentiate from it being disabled.
Suggested change
| "SAGP doesn't auto-instrument SQLiteDriver for direct use" | |
| "SAGP doesn't support auto-instrument SQLiteDriver for direct use" |
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
2 participants
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
Exposes a
BuildConfig.USE_SAGPproperty from the recently introduced -PuseSagp flag (#5538).Also updates the SQLite screen in the Android sample app so that it swizzles between auto-instrumenting vs manually wrapping
SQLiteDriver, depending on the whether SAGP was applied to the build.💡 Motivation and Context
Lets us test end-to-end our
SentrySQLiteDriverand its auto-wiring via the SAGP.JAVA-275, GRADLE-107
Screenshots
Note the new pill that tells us whether or not the sample app was built via the SAGP. The SAGP auto-instruments
SQLiteDriverwhen used with Room 2 and Room 3, but not when used directly – hence the buttons' enabled vs disabled states. (SQLDelight doesn't supportSQLiteDriverwhatsoever.)💚 How did you test it?
Manually via the samples app:
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps