-
-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor android sqlite instrumentation (SDK side) #2722
Refactor android sqlite instrumentation (SDK side) #2722
Conversation
started wrappers around SupportSQLiteOpenHelper, SupportSQLiteStatement and SupportSQLiteDatabase
…rtSQLiteOpenHelper and SentrySupportSQLiteDatabase wrappers
…current sagp values
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
16cd2b6 | 243.02 ms | 349.69 ms | 106.67 ms |
46b1782 | 387.72 ms | 458.74 ms | 71.02 ms |
1707044 | 338.80 ms | 384.79 ms | 46.00 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
16cd2b6 | 1.72 MiB | 2.28 MiB | 570.95 KiB |
46b1782 | 1.72 MiB | 2.28 MiB | 570.44 KiB |
1707044 | 1.72 MiB | 2.28 MiB | 570.44 KiB |
Previous results on branch: feat/refactor-android-sqlite-instrumentation
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7ce8206 | 337.78 ms | 363.26 ms | 25.48 ms |
8684d91 | 322.21 ms | 362.98 ms | 40.77 ms |
81c74ea | 271.52 ms | 363.18 ms | 91.66 ms |
18caa2b | 335.94 ms | 411.35 ms | 75.41 ms |
69fdf80 | 286.24 ms | 330.40 ms | 44.16 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7ce8206 | 1.72 MiB | 2.28 MiB | 570.95 KiB |
8684d91 | 1.72 MiB | 2.28 MiB | 570.44 KiB |
81c74ea | 1.72 MiB | 2.28 MiB | 571.51 KiB |
18caa2b | 1.72 MiB | 2.28 MiB | 570.44 KiB |
69fdf80 | 1.72 MiB | 2.28 MiB | 570.95 KiB |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2722 +/- ##
============================================
- Coverage 81.12% 81.09% -0.03%
- Complexity 4453 4470 +17
============================================
Files 346 347 +1
Lines 16420 16475 +55
Branches 2226 2232 +6
============================================
+ Hits 13320 13361 +41
- Misses 2173 2181 +8
- Partials 927 933 +6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great job. I guess we can talk later about how to release this new artifact (I think it's the first time for you to ship a new artifact, right?)
sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SQLiteSpanManager.kt
Outdated
Show resolved
Hide resolved
sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SentrySupportSQLiteDatabase.kt
Outdated
Show resolved
Hide resolved
sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SentrySupportSQLiteOpenHelper.kt
Outdated
Show resolved
Hide resolved
sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SentrySupportSQLiteOpenHelper.kt
Show resolved
Hide resolved
sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SentrySupportSQLiteStatement.kt
Outdated
Show resolved
Hide resolved
...ry-android-sqlite/src/test/java/io/sentry/android/sqlite/SentrySupportSQLiteStatementTest.kt
Outdated
Show resolved
Hide resolved
updated unit tests added javadoc added @JvmStatic to SentrySupportSQLiteOpenHelper.create added wrapper around SentrySupportSQLiteOpenHelper.readableDatabase removed okhttp references
📜 Description
We're adding wrappers around Android's sqlite classes, specifically
SupportSQLiteOpenHelper
,SupportSQLiteStatement
andSupportSQLiteDatabase
.This is the first part of the database instrumentation refactoring.
💡 Motivation and Context
We want to refactor the database instrumentation moving as much as we can to the Java side, decreasing the need of bytecode manipulation.
Here is the tracking issue
💚 How did you test it?
Unit tests
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps