From 957117a956428bc7508686f087be80a7563468f5 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 23 Nov 2021 11:51:01 +0100 Subject: [PATCH 01/12] Add new remapping instrumentable --- buildSrc/src/main/java/Dependencies.kt | 2 +- .../SpanAddingClassVisitorFactory.kt | 6 ++- .../remap/RemappingInstrumentable.kt | 38 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index bd0279263..35cdd6559 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -13,7 +13,7 @@ object LibsVersion { const val JUNIT = "4.13.2" const val ASM = "9.2" const val SQLITE = "2.1.0" - const val SENTRY = "5.1.2" + const val SENTRY = "5.4.0" } object Libs { diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt index af2b4b7f6..4670ad4b9 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt @@ -9,12 +9,13 @@ import io.sentry.android.gradle.instrumentation.androidx.room.AndroidXRoomDao import io.sentry.android.gradle.instrumentation.androidx.sqlite.database.AndroidXSQLiteDatabase import io.sentry.android.gradle.instrumentation.androidx.sqlite.statement.AndroidXSQLiteStatement import io.sentry.android.gradle.util.warn -import java.io.File +import io.sentry.android.gradle.instrumentation.remap.RemappingInstrumentable import org.gradle.api.provider.Property import org.gradle.api.tasks.Input import org.gradle.api.tasks.Internal import org.gradle.api.tasks.Optional import org.objectweb.asm.ClassVisitor +import java.io.File @Suppress("UnstableApiUsage") abstract class SpanAddingClassVisitorFactory : @@ -42,7 +43,8 @@ abstract class SpanAddingClassVisitorFactory : private val instrumentables: List = listOf( AndroidXSQLiteDatabase(), AndroidXSQLiteStatement(), - AndroidXRoomDao() + AndroidXRoomDao(), + RemappingInstrumentable() ) } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt new file mode 100644 index 000000000..7bdda7c57 --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt @@ -0,0 +1,38 @@ +@file:Suppress("UnstableApiUsage") + +package io.sentry.android.gradle.instrumentation.remap + +import com.android.build.api.instrumentation.ClassContext +import io.sentry.android.gradle.instrumentation.ClassInstrumentable +import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.commons.ClassRemapper +import org.objectweb.asm.commons.SimpleRemapper + +class RemappingInstrumentable : ClassInstrumentable { + + companion object { + private val mapping = mapOf( + "java/io/FileInputStream" to "io/sentry/instrumentation/file/SentryFileInputStream", + "java/io/FileOutputStream" to "io/sentry/instrumentation/file/SentryFileOutputStream" + ) + } + + override val fqName: String get() = "remapper" + + override fun getVisitor( + instrumentableContext: ClassContext, + apiVersion: Int, + originalVisitor: ClassVisitor, + parameters: SpanAddingClassVisitorFactory.SpanAddingParameters + ): ClassVisitor = + ClassRemapper(originalVisitor, SimpleRemapper(mapping)) + + override fun isInstrumentable(data: ClassContext): Boolean { + return when { + data.currentClassData.className.startsWith("io.sentry") + && !data.currentClassData.className.startsWith("io.sentry.android.roomsample") -> false + else -> true + } + } +} From 7233b4402c94dc981d888942c5ce77ce99d6dfd4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 23 Nov 2021 11:56:47 +0100 Subject: [PATCH 02/12] Update sample --- .../android-room/src/main/AndroidManifest.xml | 6 +- .../io/sentry/android/roomsample/SampleApp.kt | 30 ++++++++++ .../android/roomsample/ui/EditActivity.kt | 6 ++ .../android/roomsample/ui/LyricsActivity.kt | 60 +++++++++++++++++++ .../android/roomsample/ui/MainActivity.kt | 13 ++++ .../sentry/android/roomsample/util/Lyrics.kt | 14 +++++ .../src/main/res/layout/activity_lyrics.xml | 32 ++++++++++ .../src/main/res/layout/track_row.xml | 10 +++- 8 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt create mode 100644 examples/android-room/src/main/java/io/sentry/android/roomsample/util/Lyrics.kt create mode 100644 examples/android-room/src/main/res/layout/activity_lyrics.xml diff --git a/examples/android-room/src/main/AndroidManifest.xml b/examples/android-room/src/main/AndroidManifest.xml index 2a76f2318..32f6a3269 100644 --- a/examples/android-room/src/main/AndroidManifest.xml +++ b/examples/android-room/src/main/AndroidManifest.xml @@ -16,9 +16,13 @@ android:name=".ui.EditActivity" android:theme="@style/Theme.AppCompat.NoActionBar" /> + + + android:value="https://0651689dcd3c48b89e8a8b6c4bc763ad@o447951.ingest.sentry.io/6067820" /> + tracks.forEachIndexed { index, track -> + // add lyrics for every 2nd track + if (index % 2 == 0) { + val dir = File("$filesDir${File.separatorChar}lyrics") + dir.mkdirs() + + val file = File(dir, "${track.id}.txt") + file.writeText(DEFAULT_LYRICS) + } + } + } + } } } diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/EditActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/EditActivity.kt index 4e1983df7..70c86930b 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/EditActivity.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/EditActivity.kt @@ -56,8 +56,14 @@ class EditActivity : ComponentActivity() { } else { if (originalTrack == null) { addNewTrack(name, composer, duration.toLong(), unitPrice.toFloat()) + + val createCount = SampleApp.analytics.getInt("create_count", 0) + 1 + SampleApp.analytics.edit().putInt("create_count", createCount).apply() } else { originalTrack.update(name, composer, duration.toLong(), unitPrice.toFloat()) + + val editCount = SampleApp.analytics.getInt("edit_count", 0) + 1 + SampleApp.analytics.edit().putInt("edit_count", editCount).apply() } transaction.finish(SpanStatus.OK) finish() diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt new file mode 100644 index 000000000..4e1ebec36 --- /dev/null +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt @@ -0,0 +1,60 @@ +package io.sentry.android.roomsample.ui + +import android.annotation.SuppressLint +import android.net.Uri +import android.os.Bundle +import android.widget.EditText +import androidx.activity.ComponentActivity +import androidx.appcompat.widget.Toolbar +import io.sentry.ITransaction +import io.sentry.Sentry +import io.sentry.SpanStatus +import io.sentry.android.roomsample.R +import io.sentry.android.roomsample.data.Track +import java.io.File +import java.io.FileInputStream + +@SuppressLint("SetTextI18n") +class LyricsActivity : ComponentActivity() { + private lateinit var file: File + private lateinit var lyricsInput: EditText + private lateinit var transaction: ITransaction + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setContentView(R.layout.activity_lyrics) + + transaction = Sentry.startTransaction( + "Track Interaction", + "ui.action.lyrics", + true + ) + + lyricsInput = findViewById(R.id.lyrics) + val toolbar = findViewById(R.id.toolbar) + + val track: Track = intent.getSerializableExtra(TRACK_EXTRA_KEY) as Track + toolbar.title = "Lyrics for ${track.name}" + + val dir = File("$filesDir${File.separatorChar}lyrics") + dir.mkdirs() + + file = File(dir, "${track.id}.txt") + if (file.exists()) { + lyricsInput.setText(file.readText()) + } + } + + override fun onBackPressed() { + if (!file.exists()) { + file.createNewFile() + } + file.writeText(lyricsInput.text.toString()) + transaction.finish(SpanStatus.OK) + super.onBackPressed() + } + + companion object { + const val TRACK_EXTRA_KEY = "LyricsActivity.Track" + } +} diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt index f489d1a1d..964571b26 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt @@ -63,6 +63,7 @@ class TrackRow( val deleteButton: View get() = findViewById(R.id.delete_track) val editButton: View get() = findViewById(R.id.edit_track) + val infoButton: View get() = findViewById(R.id.track_info) @SuppressLint("SetTextI18n") fun populate(track: Track) { @@ -106,6 +107,8 @@ class TrackAdapter : RecyclerView.Adapter() { ) runBlocking { SampleApp.database.tracksDao().delete(data[holder.bindingAdapterPosition]) + val deleteCount = SampleApp.analytics.getInt("delete_count", 0) + 1 + SampleApp.analytics.edit().putInt("delete_count", deleteCount).apply() } transaction.finish(SpanStatus.OK) } @@ -119,6 +122,16 @@ class TrackAdapter : RecyclerView.Adapter() { ).putExtra(EditActivity.TRACK_EXTRA_KEY, track) ) } + holder.row.infoButton.setOnClickListener { + val context = holder.row.context + val track = data[holder.bindingAdapterPosition] + context.startActivity( + Intent( + context, + LyricsActivity::class.java + ).putExtra(LyricsActivity.TRACK_EXTRA_KEY, track) + ) + } } override fun onViewRecycled(holder: ViewHolder) { diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/util/Lyrics.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/util/Lyrics.kt new file mode 100644 index 000000000..616e2c716 --- /dev/null +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/util/Lyrics.kt @@ -0,0 +1,14 @@ +package io.sentry.android.roomsample.util + +val DEFAULT_LYRICS = + """ +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi dapibus elit vel commodo varius. Suspendisse eget tempus est. Pellentesque egestas mi vitae massa ultrices vulputate. Aenean tempor nec sem eu congue. Phasellus mollis tellus odio, ut tincidunt arcu ullamcorper at. Nunc quis lorem vel risus auctor ultricies. Quisque ornare congue sagittis. Donec sit amet arcu vitae mi sodales porta vitae et mi. Nullam eu viverra urna, quis elementum risus. In at accumsan justo. Nam hendrerit, lorem ac lacinia semper, diam dolor pulvinar turpis, non tincidunt dolor lacus non quam. Etiam tempus, dui ut rutrum ornare, quam eros feugiat nisi, et blandit nisi lacus nec neque. Mauris aliquam id tellus vel molestie. Quisque ultricies ante nec lacus condimentum, at aliquet diam volutpat. + +Vivamus fermentum eu ante non aliquam. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum interdum semper orci, et auctor enim pharetra vel. Ut porttitor neque in blandit scelerisque. Cras fermentum urna sit amet metus tincidunt, eu porttitor nisi mattis. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Mauris pharetra leo vitae turpis commodo, sit amet cursus nunc varius. Etiam bibendum interdum dolor, a scelerisque massa mattis eu. Suspendisse consequat tortor ac tincidunt vulputate. + +Maecenas non lectus sit amet dui porta dapibus. Etiam vitae velit nibh. Morbi purus urna, cursus convallis feugiat vel, vulputate eget turpis. Vivamus et tincidunt lectus. Nullam id risus est. Sed ullamcorper pellentesque massa, dignissim ultricies urna maximus ac. Vivamus maximus eu ex quis ultricies. Donec facilisis diam arcu, id cursus sapien condimentum ac. Aenean suscipit, nibh ac tempus pharetra, ligula augue bibendum arcu, sed fringilla magna nisl vel felis. + +Cras lacinia efficitur elit, ac sagittis nulla commodo eu. Aliquam a est at augue imperdiet eleifend. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Maecenas luctus est non vulputate porta. Phasellus eros libero, auctor id justo ac, mollis eleifend enim. Suspendisse pulvinar mi lorem, a maximus massa tincidunt ac. Duis mattis nibh a mauris laoreet laoreet. Donec malesuada quis dui feugiat ultricies. Ut at sem consectetur, tincidunt massa in, bibendum metus. + +Sed fermentum eros ac odio venenatis, id varius est mollis. Fusce sollicitudin tellus risus, vel accumsan ante auctor in. Phasellus vel blandit massa. Suspendisse potenti. Donec vitae elementum enim, quis dictum mauris. Nullam consectetur enim at turpis bibendum, consequat facilisis sem ultrices. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam eget nisl id libero tempus fringilla vitae eget urna. Etiam dui neque, vestibulum a mi vulputate, placerat interdum sem. Mauris sit amet aliquam felis. Suspendisse potenti. Nullam odio purus, ultricies in tincidunt sit amet, interdum ac orci. Nulla volutpat auctor velit, sed malesuada urna auctor sed. Suspendisse non sollicitudin tortor. Vivamus vulputate lacinia nisi, pellentesque sagittis sapien. + """.trimIndent() diff --git a/examples/android-room/src/main/res/layout/activity_lyrics.xml b/examples/android-room/src/main/res/layout/activity_lyrics.xml new file mode 100644 index 000000000..bb74f57e3 --- /dev/null +++ b/examples/android-room/src/main/res/layout/activity_lyrics.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + diff --git a/examples/android-room/src/main/res/layout/track_row.xml b/examples/android-room/src/main/res/layout/track_row.xml index c0bd4541c..52e452f75 100644 --- a/examples/android-room/src/main/res/layout/track_row.xml +++ b/examples/android-room/src/main/res/layout/track_row.xml @@ -33,10 +33,18 @@ android:textColor="?android:attr/textColorPrimary" /> + + From b9c774c89a9c5bff69380eadf40af838f0a9983b Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 2 Dec 2021 19:54:07 +0100 Subject: [PATCH 03/12] Switch from mapping to wrapping approach --- .../android-room/src/main/AndroidManifest.xml | 2 +- .../gradle/instrumentation/Instrumentable.kt | 8 +- .../SpanAddingClassVisitorFactory.kt | 4 +- .../sqlite/database/AndroidXSQLiteDatabase.kt | 7 +- .../statement/AndroidXSQLiteStatement.kt | 7 +- .../remap/RemappingInstrumentable.kt | 38 ------- .../instrumentation/wrap/Replacements.kt | 90 ++++++++++++++++ .../wrap/WrappingInstrumentable.kt | 74 +++++++++++++ .../wrap/visitor/WrappingVisitor.kt | 102 ++++++++++++++++++ 9 files changed, 272 insertions(+), 60 deletions(-) delete mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt create mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt create mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt create mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt diff --git a/examples/android-room/src/main/AndroidManifest.xml b/examples/android-room/src/main/AndroidManifest.xml index 32f6a3269..e68bd17bf 100644 --- a/examples/android-room/src/main/AndroidManifest.xml +++ b/examples/android-room/src/main/AndroidManifest.xml @@ -22,7 +22,7 @@ + android:value="https://1053864c67cc410aa1ffc9701bd6f93d@o447951.ingest.sentry.io/5428559" /> { * Class: androidx.sqlite.db.framework.FrameworkSQLiteDatabase * Method: query */ - val fqName: String + val fqName: String get() = "" /** * Provides a visitor for this instrumentable. A visitor can be one of the visitors defined @@ -31,12 +31,6 @@ interface Instrumentable { parameters: SpanAddingClassVisitorFactory.SpanAddingParameters ): Visitor - /** - * Provides children instrumentables that are going to be used when visiting the current - * class/method/field/etc. - */ - val children: List> get() = emptyList() - /** * Defines whether this object is instrumentable or not based on [data] */ diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt index 4670ad4b9..d778f7452 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt @@ -9,7 +9,7 @@ import io.sentry.android.gradle.instrumentation.androidx.room.AndroidXRoomDao import io.sentry.android.gradle.instrumentation.androidx.sqlite.database.AndroidXSQLiteDatabase import io.sentry.android.gradle.instrumentation.androidx.sqlite.statement.AndroidXSQLiteStatement import io.sentry.android.gradle.util.warn -import io.sentry.android.gradle.instrumentation.remap.RemappingInstrumentable +import io.sentry.android.gradle.instrumentation.wrap.WrappingInstrumentable import org.gradle.api.provider.Property import org.gradle.api.tasks.Input import org.gradle.api.tasks.Internal @@ -44,7 +44,7 @@ abstract class SpanAddingClassVisitorFactory : AndroidXSQLiteDatabase(), AndroidXSQLiteStatement(), AndroidXRoomDao(), - RemappingInstrumentable() + WrappingInstrumentable() ) } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/database/AndroidXSQLiteDatabase.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/database/AndroidXSQLiteDatabase.kt index 667e511f7..808e79e83 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/database/AndroidXSQLiteDatabase.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/database/AndroidXSQLiteDatabase.kt @@ -25,14 +25,9 @@ class AndroidXSQLiteDatabase : ClassInstrumentable { apiVersion = apiVersion, classVisitor = originalVisitor, className = fqName.substringAfterLast('.'), - methodInstrumentables = children, + methodInstrumentables = listOf(Query(), ExecSql()), parameters = parameters ) - - override val children: List = listOf( - Query(), - ExecSql() - ) } class Query : MethodInstrumentable { diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/statement/AndroidXSQLiteStatement.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/statement/AndroidXSQLiteStatement.kt index 4f9f39a46..0869bd670 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/statement/AndroidXSQLiteStatement.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/androidx/sqlite/statement/AndroidXSQLiteStatement.kt @@ -26,14 +26,9 @@ class AndroidXSQLiteStatement : ClassInstrumentable { apiVersion = apiVersion, classVisitor = originalVisitor, className = fqName.substringAfterLast('.'), - methodInstrumentables = children, + methodInstrumentables = listOf(ExecuteInsert(), ExecuteUpdateDelete()), parameters = parameters ) - - override val children: List = listOf( - ExecuteInsert(), - ExecuteUpdateDelete() - ) } class ExecuteInsert : MethodInstrumentable { diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt deleted file mode 100644 index 7bdda7c57..000000000 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/remap/RemappingInstrumentable.kt +++ /dev/null @@ -1,38 +0,0 @@ -@file:Suppress("UnstableApiUsage") - -package io.sentry.android.gradle.instrumentation.remap - -import com.android.build.api.instrumentation.ClassContext -import io.sentry.android.gradle.instrumentation.ClassInstrumentable -import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory -import org.objectweb.asm.ClassVisitor -import org.objectweb.asm.commons.ClassRemapper -import org.objectweb.asm.commons.SimpleRemapper - -class RemappingInstrumentable : ClassInstrumentable { - - companion object { - private val mapping = mapOf( - "java/io/FileInputStream" to "io/sentry/instrumentation/file/SentryFileInputStream", - "java/io/FileOutputStream" to "io/sentry/instrumentation/file/SentryFileOutputStream" - ) - } - - override val fqName: String get() = "remapper" - - override fun getVisitor( - instrumentableContext: ClassContext, - apiVersion: Int, - originalVisitor: ClassVisitor, - parameters: SpanAddingClassVisitorFactory.SpanAddingParameters - ): ClassVisitor = - ClassRemapper(originalVisitor, SimpleRemapper(mapping)) - - override fun isInstrumentable(data: ClassContext): Boolean { - return when { - data.currentClassData.className.startsWith("io.sentry") - && !data.currentClassData.className.startsWith("io.sentry.android.roomsample") -> false - else -> true - } - } -} diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt new file mode 100644 index 000000000..4a0e7ad59 --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt @@ -0,0 +1,90 @@ +package io.sentry.android.gradle.instrumentation.wrap + +data class Replacement( + val owner: String, + val name: String, + val descriptor: String +) { + object FileInputStream { + val STRING = Replacement( + "java/io/FileInputStream", + "", + "(Ljava/lang/String;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + "create", + "(Ljava/io/FileInputStream;Ljava/lang/String;)Ljava/io/FileInputStream;" + ) + val FILE = Replacement( + "java/io/FileInputStream", + "", + "(Ljava/io/File;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + "create", + "(Ljava/io/FileInputStream;Ljava/io/File;)Ljava/io/FileInputStream;" + ) + val FILE_DESCRIPTOR = + Replacement( + "java/io/FileInputStream", + "", + "(Ljava/io/FileDescriptor;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + "create", + "(Ljava/io/FileInputStream;Ljava/io/FileDescriptor;)Ljava/io/FileInputStream;" + ) + } + + object FileOutputStream { + val STRING = + Replacement( + "java/io/FileOutputStream", + "", + "(Ljava/lang/String;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/lang/String;)Ljava/io/FileOutputStream;" + ) + val STRING_BOOLEAN = + Replacement( + "java/io/FileOutputStream", + "", + "(Ljava/lang/String;Z)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/lang/String;Z)Ljava/io/FileOutputStream;" + ) + val FILE = Replacement( + "java/io/FileOutputStream", + "", + "(Ljava/io/File;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/io/File;)Ljava/io/FileOutputStream;" + ) + val FILE_BOOLEAN = + Replacement( + "java/io/FileOutputStream", + "", + "(Ljava/io/File;Z)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/io/File;Z)Ljava/io/FileOutputStream;" + ) + val FILE_DESCRIPTOR = + Replacement( + "java/io/FileOutputStream", + "", + "(Ljava/io/FileDescriptor;)V" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/io/FileDescriptor;)Ljava/io/FileOutputStream;" + ) + } +} diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt new file mode 100644 index 000000000..1807973aa --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt @@ -0,0 +1,74 @@ +@file:Suppress("UnstableApiUsage") + +package io.sentry.android.gradle.instrumentation.wrap + +import com.android.build.api.instrumentation.ClassContext +import io.sentry.android.gradle.instrumentation.ClassInstrumentable +import io.sentry.android.gradle.instrumentation.CommonClassVisitor +import io.sentry.android.gradle.instrumentation.MethodContext +import io.sentry.android.gradle.instrumentation.MethodInstrumentable +import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory +import io.sentry.android.gradle.instrumentation.wrap.visitor.WrappingVisitor +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.MethodVisitor + +class WrappingInstrumentable : ClassInstrumentable { + + override fun getVisitor( + instrumentableContext: ClassContext, + apiVersion: Int, + originalVisitor: ClassVisitor, + parameters: SpanAddingClassVisitorFactory.SpanAddingParameters + ): ClassVisitor { + val className = instrumentableContext.currentClassData.className.substringAfterLast('.') + return CommonClassVisitor( + apiVersion = apiVersion, + classVisitor = originalVisitor, + className = className, + methodInstrumentables = listOf(Wrap(className)), + parameters = parameters + ) + } + + override fun isInstrumentable(data: ClassContext): Boolean { + return when { + data.currentClassData.className.startsWith("io.sentry") + && !data.currentClassData.className.startsWith("io.sentry.android.roomsample") -> false + else -> true + } + } +} + +class Wrap(private val className: String) : MethodInstrumentable { + + companion object { + private val replacements = mapOf( + // FileInputStream to SentryFileInputStream + Replacement.FileInputStream.STRING, + Replacement.FileInputStream.FILE, + Replacement.FileInputStream.FILE_DESCRIPTOR, + // FileOutputStream to SentryFileOutputStream + Replacement.FileOutputStream.STRING, + Replacement.FileOutputStream.STRING_BOOLEAN, + Replacement.FileOutputStream.FILE, + Replacement.FileOutputStream.FILE_BOOLEAN, + Replacement.FileOutputStream.FILE_DESCRIPTOR + ) + } + + override fun getVisitor( + instrumentableContext: MethodContext, + apiVersion: Int, + originalVisitor: MethodVisitor, + parameters: SpanAddingClassVisitorFactory.SpanAddingParameters + ): MethodVisitor = + WrappingVisitor( + api = apiVersion, + originalVisitor = originalVisitor, + className = className, + context = instrumentableContext, + replacements = replacements + ) + + override fun isInstrumentable(data: MethodContext): Boolean = true +} diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt new file mode 100644 index 000000000..be7112681 --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt @@ -0,0 +1,102 @@ +package io.sentry.android.gradle.instrumentation.wrap.visitor + +import io.sentry.android.gradle.SentryPlugin +import io.sentry.android.gradle.instrumentation.MethodContext +import io.sentry.android.gradle.instrumentation.wrap.Replacement +import io.sentry.android.gradle.util.info +import io.sentry.android.gradle.util.warn +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes +import org.objectweb.asm.commons.GeneratorAdapter +import org.objectweb.asm.commons.Method + +class WrappingVisitor( + api: Int, + originalVisitor: MethodVisitor, + private val className: String, + private val context: MethodContext, + private val replacements: Map +) : GeneratorAdapter( + api, + originalVisitor, + context.access, + context.name, + context.descriptor +) { + + override fun visitMethodInsn( + opcode: Int, + owner: String, + name: String, + descriptor: String, + isInterface: Boolean + ) { + val methodSig = Replacement(owner, name, descriptor) + val replacement = replacements[methodSig] + when { + opcode == Opcodes.INVOKEDYNAMIC -> { + // we don't instrument invokedynamic, because it's just forwarding to a synthetic method + // which will be instrumented thanks to condition below + SentryPlugin.logger.warn { "INVOKEDYNAMIC skipped from instrumentation for $className.${context.name}" } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + } + replacement != null -> { + val isSuperCallInOverride = opcode == Opcodes.INVOKESPECIAL && + owner != className && + name == context.name && + descriptor == context.descriptor + + if (isSuperCallInOverride) { + SentryPlugin.logger.info { + "$className skipped from instrumentation in overridden method $name.$descriptor" + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + } else { + SentryPlugin.logger.info { + "Wrapping $owner.$name with ${replacement.owner}.${replacement.name} in $className.${context.name}" + } + visitWrapping(replacement, opcode, owner, name, descriptor, isInterface) + } + } + else -> super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + } + } + + private fun GeneratorAdapter.visitWrapping( + replacement: Replacement, + opcode: Int, + owner: String, + name: String, + descriptor: String, + isInterface: Boolean + ) { + // create a new method to figure out the number of arguments + val newMethod = Method(name, descriptor) + + // replicate arguments on stack, so we can later re-use them for our wrapping + val locals = IntArray(newMethod.argumentTypes.size) + for (i in locals.size - 1 downTo 0) { + locals[i] = newLocal(newMethod.argumentTypes[i]) + storeLocal(locals[i]) + } + + // load arguments from stack for the original method call + locals.forEach { + loadLocal(it) + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + + // load arguments from stack for the wrapping method call + locals.forEach { + loadLocal(it) + } + // call wrapping (it's always a static method) + super.visitMethodInsn( + Opcodes.INVOKESTATIC, + replacement.owner, + replacement.name, + replacement.descriptor, + false + ) + } +} From db3df7b8467421e5c996a993149e16aa6c271251 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 2 Dec 2021 21:25:27 +0100 Subject: [PATCH 04/12] Address PR comments --- .../android/roomsample/ui/LyricsActivity.kt | 9 +- .../android/roomsample/ui/MainActivity.kt | 98 +------------------ .../roomsample/ui/list/TrackAdapter.kt | 82 ++++++++++++++++ .../android/roomsample/ui/list/TrackRow.kt | 30 ++++++ 4 files changed, 120 insertions(+), 99 deletions(-) create mode 100644 examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt create mode 100644 examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackRow.kt diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt index 4e1ebec36..81e647308 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt @@ -18,13 +18,12 @@ import java.io.FileInputStream class LyricsActivity : ComponentActivity() { private lateinit var file: File private lateinit var lyricsInput: EditText - private lateinit var transaction: ITransaction override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_lyrics) - transaction = Sentry.startTransaction( + val transaction = Sentry.startTransaction( "Track Interaction", "ui.action.lyrics", true @@ -43,9 +42,15 @@ class LyricsActivity : ComponentActivity() { if (file.exists()) { lyricsInput.setText(file.readText()) } + transaction.finish(SpanStatus.OK) } override fun onBackPressed() { + val transaction = Sentry.getSpan() ?: Sentry.startTransaction( + "Track Interaction", + "ui.action.lyrics_finish", + true + ) if (!file.exists()) { file.createNewFile() } diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt index 964571b26..7f852baaa 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/MainActivity.kt @@ -1,15 +1,7 @@ package io.sentry.android.roomsample.ui -import android.annotation.SuppressLint -import android.content.Context import android.content.Intent import android.os.Bundle -import android.util.AttributeSet -import android.view.LayoutInflater -import android.view.View -import android.view.ViewGroup -import android.widget.LinearLayout -import android.widget.TextView import androidx.activity.ComponentActivity import androidx.appcompat.widget.Toolbar import androidx.lifecycle.lifecycleScope @@ -19,9 +11,8 @@ import io.sentry.Sentry import io.sentry.SpanStatus import io.sentry.android.roomsample.R import io.sentry.android.roomsample.SampleApp -import io.sentry.android.roomsample.data.Track +import io.sentry.android.roomsample.ui.list.TrackAdapter import kotlinx.coroutines.flow.collect -import kotlinx.coroutines.runBlocking class MainActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { @@ -55,90 +46,3 @@ class MainActivity : ComponentActivity() { } } } - -class TrackRow( - context: Context, - attrs: AttributeSet -) : LinearLayout(context, attrs) { - - val deleteButton: View get() = findViewById(R.id.delete_track) - val editButton: View get() = findViewById(R.id.edit_track) - val infoButton: View get() = findViewById(R.id.track_info) - - @SuppressLint("SetTextI18n") - fun populate(track: Track) { - val mins = (track.millis / 1000) / 60 - val secs = (track.millis / 1000) % 60 - - findViewById(R.id.track_name).text = track.name - findViewById(R.id.track_duration).text = "${mins}m ${secs}s" - findViewById(R.id.band_name).text = track.composer - } -} - -class TrackAdapter : RecyclerView.Adapter() { - - private var data: List = listOf() - - fun populate(data: List) { - this.data = data - notifyDataSetChanged() - } - - override fun getItemCount() = data.size - - override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { - return ViewHolder( - LayoutInflater.from(parent.context).inflate( - R.layout.track_row, - parent, - false - ) as TrackRow - ) - } - - override fun onBindViewHolder(holder: ViewHolder, position: Int) { - holder.row.populate(data[position]) - holder.row.deleteButton.setOnClickListener { - val transaction = Sentry.startTransaction( - "Track Interaction", - "ui.action.delete", - true - ) - runBlocking { - SampleApp.database.tracksDao().delete(data[holder.bindingAdapterPosition]) - val deleteCount = SampleApp.analytics.getInt("delete_count", 0) + 1 - SampleApp.analytics.edit().putInt("delete_count", deleteCount).apply() - } - transaction.finish(SpanStatus.OK) - } - holder.row.editButton.setOnClickListener { - val context = holder.row.context - val track = data[holder.bindingAdapterPosition] - context.startActivity( - Intent( - context, - EditActivity::class.java - ).putExtra(EditActivity.TRACK_EXTRA_KEY, track) - ) - } - holder.row.infoButton.setOnClickListener { - val context = holder.row.context - val track = data[holder.bindingAdapterPosition] - context.startActivity( - Intent( - context, - LyricsActivity::class.java - ).putExtra(LyricsActivity.TRACK_EXTRA_KEY, track) - ) - } - } - - override fun onViewRecycled(holder: ViewHolder) { - super.onViewRecycled(holder) - holder.row.deleteButton.setOnClickListener(null) - holder.row.editButton.setOnClickListener(null) - } - - inner class ViewHolder(val row: TrackRow) : RecyclerView.ViewHolder(row) -} diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt new file mode 100644 index 000000000..eb1750a98 --- /dev/null +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt @@ -0,0 +1,82 @@ +package io.sentry.android.roomsample.ui.list + +import android.content.Intent +import android.view.LayoutInflater +import android.view.ViewGroup +import androidx.recyclerview.widget.RecyclerView +import io.sentry.Sentry +import io.sentry.SpanStatus +import io.sentry.android.roomsample.R +import io.sentry.android.roomsample.SampleApp +import io.sentry.android.roomsample.data.Track +import io.sentry.android.roomsample.ui.EditActivity +import io.sentry.android.roomsample.ui.LyricsActivity +import io.sentry.android.roomsample.ui.TrackRow +import kotlinx.coroutines.runBlocking + +class TrackAdapter : RecyclerView.Adapter() { + + private var data: List = listOf() + + fun populate(data: List) { + this.data = data + notifyDataSetChanged() + } + + override fun getItemCount() = data.size + + override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { + return ViewHolder( + LayoutInflater.from(parent.context).inflate( + R.layout.track_row, + parent, + false + ) as TrackRow + ) + } + + override fun onBindViewHolder(holder: ViewHolder, position: Int) { + holder.row.populate(data[position]) + holder.row.deleteButton.setOnClickListener { + val transaction = Sentry.startTransaction( + "Track Interaction", + "ui.action.delete", + true + ) + runBlocking { + SampleApp.database.tracksDao().delete(data[holder.bindingAdapterPosition]) + val deleteCount = SampleApp.analytics.getInt("delete_count", 0) + 1 + SampleApp.analytics.edit().putInt("delete_count", deleteCount).apply() + } + transaction.finish(SpanStatus.OK) + } + holder.row.editButton.setOnClickListener { + val context = holder.row.context + val track = data[holder.bindingAdapterPosition] + context.startActivity( + Intent( + context, + EditActivity::class.java + ).putExtra(EditActivity.TRACK_EXTRA_KEY, track) + ) + } + holder.row.infoButton.setOnClickListener { + val context = holder.row.context + val track = data[holder.bindingAdapterPosition] + context.startActivity( + Intent( + context, + LyricsActivity::class.java + ).putExtra(LyricsActivity.TRACK_EXTRA_KEY, track) + ) + } + } + + override fun onViewRecycled(holder: ViewHolder) { + super.onViewRecycled(holder) + holder.row.deleteButton.setOnClickListener(null) + holder.row.editButton.setOnClickListener(null) + } + + inner class ViewHolder(val row: TrackRow) : RecyclerView.ViewHolder(row) +} diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackRow.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackRow.kt new file mode 100644 index 000000000..bc6aab38f --- /dev/null +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackRow.kt @@ -0,0 +1,30 @@ +package io.sentry.android.roomsample.ui.list + +import android.annotation.SuppressLint +import android.content.Context +import android.util.AttributeSet +import android.view.View +import android.widget.LinearLayout +import android.widget.TextView +import io.sentry.android.roomsample.R +import io.sentry.android.roomsample.data.Track + +class TrackRow( + context: Context, + attrs: AttributeSet +) : LinearLayout(context, attrs) { + + val deleteButton: View get() = findViewById(R.id.delete_track) + val editButton: View get() = findViewById(R.id.edit_track) + val infoButton: View get() = findViewById(R.id.track_info) + + @SuppressLint("SetTextI18n") + fun populate(track: Track) { + val mins = (track.millis / 1000) / 60 + val secs = (track.millis / 1000) % 60 + + findViewById(R.id.track_name).text = track.name + findViewById(R.id.track_duration).text = "${mins}m ${secs}s" + findViewById(R.id.band_name).text = track.composer + } +} From b5f2d9104dbfb78c977e38c1688098606d255449 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 2 Dec 2021 21:37:04 +0100 Subject: [PATCH 05/12] Ktlint --- .../java/io/sentry/android/roomsample/SampleApp.kt | 5 ++--- .../io/sentry/android/roomsample/ui/LyricsActivity.kt | 3 --- .../instrumentation/SpanAddingClassVisitorFactory.kt | 4 ++-- .../gradle/instrumentation/wrap/Replacements.kt | 1 + .../instrumentation/wrap/WrappingInstrumentable.kt | 4 ++-- .../instrumentation/wrap/visitor/WrappingVisitor.kt | 10 +++++++--- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/SampleApp.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/SampleApp.kt index d43338fb2..e49451e85 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/SampleApp.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/SampleApp.kt @@ -6,12 +6,11 @@ import android.content.SharedPreferences import androidx.room.Room import io.sentry.android.roomsample.data.TracksDatabase import io.sentry.android.roomsample.util.DEFAULT_LYRICS +import java.io.File import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext -import java.io.File class SampleApp : Application() { @@ -34,7 +33,7 @@ class SampleApp : Application() { GlobalScope.launch(Dispatchers.IO) { database.tracksDao().all() - .collect { tracks -> + .collect { tracks -> tracks.forEachIndexed { index, track -> // add lyrics for every 2nd track if (index % 2 == 0) { diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt index 81e647308..f2f2ce82a 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/LyricsActivity.kt @@ -1,18 +1,15 @@ package io.sentry.android.roomsample.ui import android.annotation.SuppressLint -import android.net.Uri import android.os.Bundle import android.widget.EditText import androidx.activity.ComponentActivity import androidx.appcompat.widget.Toolbar -import io.sentry.ITransaction import io.sentry.Sentry import io.sentry.SpanStatus import io.sentry.android.roomsample.R import io.sentry.android.roomsample.data.Track import java.io.File -import java.io.FileInputStream @SuppressLint("SetTextI18n") class LyricsActivity : ComponentActivity() { diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt index d778f7452..69a0c6976 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/SpanAddingClassVisitorFactory.kt @@ -8,14 +8,14 @@ import io.sentry.android.gradle.SentryPlugin import io.sentry.android.gradle.instrumentation.androidx.room.AndroidXRoomDao import io.sentry.android.gradle.instrumentation.androidx.sqlite.database.AndroidXSQLiteDatabase import io.sentry.android.gradle.instrumentation.androidx.sqlite.statement.AndroidXSQLiteStatement -import io.sentry.android.gradle.util.warn import io.sentry.android.gradle.instrumentation.wrap.WrappingInstrumentable +import io.sentry.android.gradle.util.warn +import java.io.File import org.gradle.api.provider.Property import org.gradle.api.tasks.Input import org.gradle.api.tasks.Internal import org.gradle.api.tasks.Optional import org.objectweb.asm.ClassVisitor -import java.io.File @Suppress("UnstableApiUsage") abstract class SpanAddingClassVisitorFactory : diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt index 4a0e7ad59..86781fb38 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt @@ -1,3 +1,4 @@ +// ktlint-disable filename package io.sentry.android.gradle.instrumentation.wrap data class Replacement( diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt index 1807973aa..2270eb2e0 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt @@ -32,8 +32,8 @@ class WrappingInstrumentable : ClassInstrumentable { override fun isInstrumentable(data: ClassContext): Boolean { return when { - data.currentClassData.className.startsWith("io.sentry") - && !data.currentClassData.className.startsWith("io.sentry.android.roomsample") -> false + data.currentClassData.className.startsWith("io.sentry") && + !data.currentClassData.className.startsWith("io.sentry.android.roomsample") -> false else -> true } } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt index be7112681..ba5cac413 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt @@ -37,7 +37,9 @@ class WrappingVisitor( opcode == Opcodes.INVOKEDYNAMIC -> { // we don't instrument invokedynamic, because it's just forwarding to a synthetic method // which will be instrumented thanks to condition below - SentryPlugin.logger.warn { "INVOKEDYNAMIC skipped from instrumentation for $className.${context.name}" } + SentryPlugin.logger.warn { + "INVOKEDYNAMIC skipped from instrumentation for $className.${context.name}" + } super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) } replacement != null -> { @@ -48,12 +50,14 @@ class WrappingVisitor( if (isSuperCallInOverride) { SentryPlugin.logger.info { - "$className skipped from instrumentation in overridden method $name.$descriptor" + "$className skipped from instrumentation " + + "in overridden method $name.$descriptor" } super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) } else { SentryPlugin.logger.info { - "Wrapping $owner.$name with ${replacement.owner}.${replacement.name} in $className.${context.name}" + "Wrapping $owner.$name with ${replacement.owner}.${replacement.name} " + + "in $className.${context.name}" } visitWrapping(replacement, opcode, owner, name, descriptor, isInterface) } From fb47d6ec7502c2682ee307d90ebe6a618a26551e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 2 Dec 2021 21:41:20 +0100 Subject: [PATCH 06/12] Fix compile issue --- .../java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt index eb1750a98..59d44fcb7 100644 --- a/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt +++ b/examples/android-room/src/main/java/io/sentry/android/roomsample/ui/list/TrackAdapter.kt @@ -11,7 +11,6 @@ import io.sentry.android.roomsample.SampleApp import io.sentry.android.roomsample.data.Track import io.sentry.android.roomsample.ui.EditActivity import io.sentry.android.roomsample.ui.LyricsActivity -import io.sentry.android.roomsample.ui.TrackRow import kotlinx.coroutines.runBlocking class TrackAdapter : RecyclerView.Adapter() { From c4700f864653b154bff71e4da11d9b5ec5a5ed54 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 2 Dec 2021 22:16:14 +0100 Subject: [PATCH 07/12] lint --- examples/android-room/src/main/res/layout/track_row.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/android-room/src/main/res/layout/track_row.xml b/examples/android-room/src/main/res/layout/track_row.xml index 52e452f75..03af189dd 100644 --- a/examples/android-room/src/main/res/layout/track_row.xml +++ b/examples/android-room/src/main/res/layout/track_row.xml @@ -1,5 +1,5 @@ - - + From 8eae0e7c8c10bdfb1b1f3dd25a2537d84392ada6 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 14 Dec 2021 12:34:04 +0100 Subject: [PATCH 08/12] Change approach to wrapping and add tests --- .../instrumentation/wrap/Replacements.kt | 24 ++ .../wrap/WrappingInstrumentable.kt | 17 +- .../wrap/visitor/WrappingVisitor.kt | 76 ++++-- .../fakes/CapturingTestLogger.kt | 5 + .../wrap/visitor/WrappingVisitorTest.kt | 248 ++++++++++++++++++ .../fileIO/SQLiteCopyOpenHelper.class | Bin 0 -> 9645 bytes .../fileIO/TypefaceCompatUtil.class | Bin 0 -> 6167 bytes 7 files changed, 344 insertions(+), 26 deletions(-) create mode 100644 plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt create mode 100644 plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/SQLiteCopyOpenHelper.class create mode 100644 plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/TypefaceCompatUtil.class diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt index 86781fb38..23fc41183 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/Replacements.kt @@ -88,4 +88,28 @@ data class Replacement( "(Ljava/io/FileOutputStream;Ljava/io/FileDescriptor;)Ljava/io/FileOutputStream;" ) } + + object Context { + val OPEN_FILE_INPUT = + Replacement( + "", + "openFileInput", + "(Ljava/lang/String;)Ljava/io/FileInputStream;" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + "create", + "(Ljava/io/FileInputStream;Ljava/lang/String;)Ljava/io/FileInputStream;" + ) + + val OPEN_FILE_OUTPUT = + Replacement( + "", + "openFileOutput", + "(Ljava/lang/String;I)Ljava/io/FileOutputStream;" + ) to Replacement( + "io/sentry/instrumentation/file/SentryFileOutputStream${'$'}Factory", + "create", + "(Ljava/io/FileOutputStream;Ljava/lang/String;)Ljava/io/FileOutputStream;" + ) + } } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt index 2270eb2e0..1cbfd566b 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt @@ -3,6 +3,7 @@ package io.sentry.android.gradle.instrumentation.wrap import com.android.build.api.instrumentation.ClassContext +import com.android.build.api.instrumentation.ClassData import io.sentry.android.gradle.instrumentation.ClassInstrumentable import io.sentry.android.gradle.instrumentation.CommonClassVisitor import io.sentry.android.gradle.instrumentation.MethodContext @@ -20,12 +21,13 @@ class WrappingInstrumentable : ClassInstrumentable { originalVisitor: ClassVisitor, parameters: SpanAddingClassVisitorFactory.SpanAddingParameters ): ClassVisitor { - val className = instrumentableContext.currentClassData.className.substringAfterLast('.') + val simpleClassName = + instrumentableContext.currentClassData.className.substringAfterLast('.') return CommonClassVisitor( apiVersion = apiVersion, classVisitor = originalVisitor, - className = className, - methodInstrumentables = listOf(Wrap(className)), + className = simpleClassName, + methodInstrumentables = listOf(Wrap(instrumentableContext.currentClassData)), parameters = parameters ) } @@ -39,7 +41,7 @@ class WrappingInstrumentable : ClassInstrumentable { } } -class Wrap(private val className: String) : MethodInstrumentable { +class Wrap(private val classContext: ClassData) : MethodInstrumentable { companion object { private val replacements = mapOf( @@ -53,6 +55,11 @@ class Wrap(private val className: String) : MethodInstrumentable { Replacement.FileOutputStream.FILE, Replacement.FileOutputStream.FILE_BOOLEAN, Replacement.FileOutputStream.FILE_DESCRIPTOR + // TODO: enable, once https://github.com/getsentry/sentry-java/issues/1842 is resolved + // Context.openFileInput to SentryFileInputStream +// Replacement.Context.OPEN_FILE_INPUT, + // Context.openFileOutput to SentryFileOutputStream +// Replacement.Context.OPEN_FILE_OUTPUT ) } @@ -65,7 +72,7 @@ class Wrap(private val className: String) : MethodInstrumentable { WrappingVisitor( api = apiVersion, originalVisitor = originalVisitor, - className = className, + classContext = classContext, context = instrumentableContext, replacements = replacements ) diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt index ba5cac413..f6085437b 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitor.kt @@ -1,21 +1,25 @@ +@file:Suppress("UnstableApiUsage") + package io.sentry.android.gradle.instrumentation.wrap.visitor +import com.android.build.api.instrumentation.ClassData import io.sentry.android.gradle.SentryPlugin import io.sentry.android.gradle.instrumentation.MethodContext import io.sentry.android.gradle.instrumentation.wrap.Replacement import io.sentry.android.gradle.util.info -import io.sentry.android.gradle.util.warn import org.objectweb.asm.MethodVisitor import org.objectweb.asm.Opcodes import org.objectweb.asm.commons.GeneratorAdapter import org.objectweb.asm.commons.Method +import org.slf4j.Logger class WrappingVisitor( api: Int, originalVisitor: MethodVisitor, - private val className: String, + private val classContext: ClassData, private val context: MethodContext, - private val replacements: Map + private val replacements: Map, + private val logger: Logger = SentryPlugin.logger ) : GeneratorAdapter( api, originalVisitor, @@ -24,6 +28,8 @@ class WrappingVisitor( context.descriptor ) { + private val className = classContext.className.replace('.', '/') + override fun visitMethodInsn( opcode: Int, owner: String, @@ -32,13 +38,19 @@ class WrappingVisitor( isInterface: Boolean ) { val methodSig = Replacement(owner, name, descriptor) - val replacement = replacements[methodSig] + val replacement = if (methodSig in replacements) { + replacements[methodSig] + } else { + // try to look up for a replacement without owner (as the owner sometimes can differ) + replacements[methodSig.copy(owner = "")] + } when { opcode == Opcodes.INVOKEDYNAMIC -> { // we don't instrument invokedynamic, because it's just forwarding to a synthetic method // which will be instrumented thanks to condition below - SentryPlugin.logger.warn { - "INVOKEDYNAMIC skipped from instrumentation for $className.${context.name}" + logger.info { + "INVOKEDYNAMIC skipped from instrumentation for" + + " ${className.prettyPrintClassName()}.${context.name}" } super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) } @@ -48,24 +60,44 @@ class WrappingVisitor( name == context.name && descriptor == context.descriptor - if (isSuperCallInOverride) { - SentryPlugin.logger.info { - "$className skipped from instrumentation " + - "in overridden method $name.$descriptor" + val isSuperCallInCtor = opcode == Opcodes.INVOKESPECIAL && + name == "" && + classContext.superClasses.firstOrNull()?.fqName() == owner + + when { + isSuperCallInOverride -> { + // this will be instrumented on the calling side of the overriding class + logger.info { + "${className.prettyPrintClassName()} skipped from instrumentation " + + "in overridden method $name.$descriptor" + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + } + isSuperCallInCtor -> { + // this has to be manually instrumented (e.g. by inheriting our runtime classes) + logger.info { + "${className.prettyPrintClassName()} skipped from instrumentation " + + "in constructor $name.$descriptor" + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) } - super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) - } else { - SentryPlugin.logger.info { - "Wrapping $owner.$name with ${replacement.owner}.${replacement.name} " + - "in $className.${context.name}" + else -> { + logger.info { + "Wrapping $owner.$name with ${replacement.owner}.${replacement.name} " + + "in ${className.prettyPrintClassName()}.${context.name}" + } + visitWrapping(replacement, opcode, owner, name, descriptor, isInterface) } - visitWrapping(replacement, opcode, owner, name, descriptor, isInterface) } } else -> super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) } } + private fun String.prettyPrintClassName() = replace('/', '.') + + private fun String.fqName() = replace('.', '/') + private fun GeneratorAdapter.visitWrapping( replacement: Replacement, opcode: Int, @@ -75,12 +107,12 @@ class WrappingVisitor( isInterface: Boolean ) { // create a new method to figure out the number of arguments - val newMethod = Method(name, descriptor) + val originalMethod = Method(name, descriptor) // replicate arguments on stack, so we can later re-use them for our wrapping - val locals = IntArray(newMethod.argumentTypes.size) + val locals = IntArray(originalMethod.argumentTypes.size) for (i in locals.size - 1 downTo 0) { - locals[i] = newLocal(newMethod.argumentTypes[i]) + locals[i] = newLocal(originalMethod.argumentTypes[i]) storeLocal(locals[i]) } @@ -91,8 +123,10 @@ class WrappingVisitor( super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) // load arguments from stack for the wrapping method call - locals.forEach { - loadLocal(it) + // only load as many as the new method requires (replacement method may have less arguments) + val newMethod = Method(replacement.name, replacement.descriptor) + for (i in 0 until newMethod.argumentTypes.size - 1) { + loadLocal(locals[i]) } // call wrapping (it's always a static method) super.visitMethodInsn( diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/CapturingTestLogger.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/CapturingTestLogger.kt index 8239d185b..f0abd0aa6 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/CapturingTestLogger.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/fakes/CapturingTestLogger.kt @@ -15,4 +15,9 @@ class CapturingTestLogger : BaseTestLogger() { capturedMessage = msg capturedThrowable = throwable } + + override fun info(msg: String, throwable: Throwable?) { + capturedMessage = msg + capturedThrowable = throwable + } } diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt new file mode 100644 index 000000000..14ecda8ce --- /dev/null +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt @@ -0,0 +1,248 @@ +package io.sentry.android.gradle.instrumentation.wrap.visitor + +import com.android.build.api.instrumentation.ClassData +import io.sentry.android.gradle.instrumentation.MethodContext +import io.sentry.android.gradle.instrumentation.fakes.CapturingTestLogger +import io.sentry.android.gradle.instrumentation.fakes.TestClassData +import io.sentry.android.gradle.instrumentation.wrap.Replacement +import kotlin.test.assertEquals +import org.junit.Test +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes + +class WrappingVisitorTest { + + class Fixture { + val logger = CapturingTestLogger() + val visitor = CapturingMethodVisitor() + + fun getSut( + methodContext: MethodContext, + classContext: ClassData = TestClassData("io/sentry/RandomClass"), + replacements: Map = mapOf() + ) = WrappingVisitor( + Opcodes.ASM9, + visitor, + classContext, + methodContext, + replacements, + logger + ) + } + + private val fixture = Fixture() + + @Test + fun `invokedynamic is skipped from instrumentation`() { + val context = MethodContext(Opcodes.ACC_PUBLIC, "test", "()V", null, null) + val methodVisit = MethodVisit( + opcode = Opcodes.INVOKEDYNAMIC, + owner = "java/io/FileInputStream", + name = "", + descriptor = "(Ljava/lang/String;)V", + isInterface = false + ) + fixture.getSut(context) + .visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) + + assertEquals( + fixture.logger.capturedMessage, + "[sentry] INVOKEDYNAMIC skipped from instrumentation for io.sentry.RandomClass.test" + ) + // method visit should remain unchanged + assertEquals(fixture.visitor.methodVisits.size, 1) + assertEquals(fixture.visitor.methodVisits.first(), methodVisit) + } + + @Test + fun `when no replacements found does not modify method visit`() { + val context = MethodContext(Opcodes.ACC_PUBLIC, "test", "()V", null, null) + val methodVisit = MethodVisit( + opcode = Opcodes.INVOKEVIRTUAL, + owner = "java/io/FileInputStream", + name = "", + descriptor = "(Ljava/lang/String;)V", + isInterface = false + ) + fixture.getSut(context) + .visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) + + // method visit should remain unchanged + assertEquals(fixture.visitor.methodVisits.size, 1) + assertEquals(fixture.visitor.methodVisits.first(), methodVisit) + } + + @Test + fun `when replacement found and super call in override does not modify method visit`() { + val context = MethodContext( + Opcodes.ACC_PUBLIC, + "openFileInput", + "(Ljava/lang/String;)Ljava/io/FileInputStream;", + null, + null + ) + val methodVisit = MethodVisit( + opcode = Opcodes.INVOKESPECIAL, + owner = "android/content/Context", + name = "openFileInput", + descriptor = "(Ljava/lang/String;)Ljava/io/FileInputStream;", + isInterface = false + ) + fixture.getSut( + context, + classContext = TestClassData("io/sentry/android/sample/LyricsActivity"), + replacements = mapOf(Replacement.Context.OPEN_FILE_INPUT) + ).visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) + + assertEquals( + fixture.logger.capturedMessage, + "[sentry] io.sentry.android.sample.LyricsActivity skipped from instrumentation " + + "in overridden method openFileInput.(Ljava/lang/String;)Ljava/io/FileInputStream;" + ) + // method visit should remain unchanged + assertEquals(fixture.visitor.methodVisits.size, 1) + assertEquals(fixture.visitor.methodVisits.first(), methodVisit) + } + + @Test + fun `when replacement found and super call in constructor does not modify method visit`() { + val context = MethodContext( + Opcodes.ACC_PUBLIC, + "", + "()V", + null, + null + ) + val methodVisit = MethodVisit( + opcode = Opcodes.INVOKESPECIAL, + owner = "java/io/FileInputStream", + name = "", + descriptor = "(Ljava/lang/String;)V", + isInterface = false + ) + fixture.getSut( + context, + classContext = TestClassData( + "io/sentry/CustomFileInputStream", + superClasses = listOf("java/io/FileInputStream") + ), + replacements = mapOf(Replacement.FileInputStream.STRING) + ).visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) + + assertEquals( + fixture.logger.capturedMessage, + "[sentry] io.sentry.CustomFileInputStream skipped from instrumentation in " + + "constructor .(Ljava/lang/String;)V" + ) + // method visit should remain unchanged + assertEquals(fixture.visitor.methodVisits.size, 1) + assertEquals(fixture.visitor.methodVisits.first(), methodVisit) + } + + @Test + fun `when replacement found modifies method visit`() { + val context = MethodContext( + Opcodes.ACC_PUBLIC, + "test", + "()V", + null, + null + ) + val methodVisit = MethodVisit( + opcode = Opcodes.INVOKESPECIAL, + owner = "java/io/FileInputStream", + name = "", + descriptor = "(Ljava/lang/String;)V", + isInterface = false + ) + fixture.getSut(context, replacements = mapOf(Replacement.FileInputStream.STRING)) + .visitMethodInsn( + methodVisit.opcode, + methodVisit.owner, + methodVisit.name, + methodVisit.descriptor, + methodVisit.isInterface + ) + + assertEquals(fixture.visitor.methodVisits.size, 2) + // store original arguments + assertEquals(fixture.visitor.varVisits[0], VarVisit(Opcodes.ASTORE, 1)) + // load original argument for the original method visit + assertEquals(fixture.visitor.varVisits[1], VarVisit(Opcodes.ALOAD, 1)) + // original method is visited unchanged + assertEquals(fixture.visitor.methodVisits[0], methodVisit) + + // load original argument for the replacement/wrapping method visit + // the target object that we are wrapping will be taken from stack + assertEquals(fixture.visitor.varVisits[2], VarVisit(Opcodes.ALOAD, 1)) + // replacement/wrapping method visited + assertEquals( + fixture.visitor.methodVisits[1], MethodVisit( + opcode = Opcodes.INVOKESTATIC, + owner = "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + name = "create", + descriptor = "(Ljava/io/FileInputStream;Ljava/lang/String;)Ljava/io/FileInputStream;", + isInterface = false + ) + ) + } +} + +data class MethodVisit( + val opcode: Int, + val owner: String, + val name: String, + val descriptor: String, + val isInterface: Boolean +) + +data class VarVisit( + val opcode: Int, + val `var`: Int +) + +class CapturingMethodVisitor : MethodVisitor(Opcodes.ASM9) { + + val methodVisits = mutableListOf() + val varVisits = mutableListOf() + + override fun visitVarInsn(opcode: Int, `var`: Int) { + super.visitVarInsn(opcode, `var`) + varVisits += VarVisit(opcode, `var`) + } + + override fun visitMethodInsn( + opcode: Int, + owner: String, + name: String, + descriptor: String, + isInterface: Boolean + ) { + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface) + methodVisits += MethodVisit(opcode, owner, name, descriptor, isInterface) + } +} diff --git a/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/SQLiteCopyOpenHelper.class b/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/SQLiteCopyOpenHelper.class new file mode 100644 index 0000000000000000000000000000000000000000..bd005964603b3a6924624ffea9a3c4d96dbe06a2 GIT binary patch literal 9645 zcmcIq33y!9b^h;YX5MK0w2d)%EE`6)Y$VyT02^Z@<4v})K#MJTK?XxcnrCV3(ad-j zF9`wC(n7kX&C=K{B@Id4HcR6aOEPLPX^Tk-Z96VhDY15=Bv_J^P<0~{EalvO7pon*pKI= z`CEDXyC9p-%i|ZMd0v_?%8M__i|2y)GQLuYzsFZKd`%v{9>iRHLz;gG;v@J+S@loy z_|F>tMZ-4(_}3un@j?*a!nZYiCxCwo;JZQmJN_d`GT###|Eb~oK`h170sKJv{|exT z(*L(Cdr?-r6vY4Fe}i}#uV{ERh&KF4n%Cyw$9P@C1r2Wma50DlctOLP0fiuLQYxT4 z0ad~M%1halPa40bv>-Of{tb97$TgKR9h7EHK>znt-YesJeg(@!)E% z%wG{y^VF5{@~VKEuc>el{dhWvr__R=s#gm&wJ3-^>S}qoMw$p0s0K|f*3=SBHEL?9 zg5H%(8kx>SG@CWD3W2fCR5E8A%PEArqsdq%6_2$I%TzMgX20rCsPE4wbMY~wD|sZI zjSnS^P03^`7tO^}NfKG)>^#;Mb?4i9Q_0?ZBEj7uGD#obmPw6mBG=rGXl|5~-3Oyb zqHT$2a-?k_mx(7wxbo`qlt?B@rd&|vlG+we7z))Em3XR6p1I%(cR^P&ozIc75gk*w z##)!p#S?P8;d~}zo-pcCt{jMuB%`@}hE&&Fdi6S&X%UZ(_2gbVwk4X24n?!Z9wU>b zb`(^XLgm;NBVmk0bELhZaK!B4L_BA-#fI7j^67LclN;FC&56FWk=$-1(nf|9uXHKW z8VBMd`HZP`g{1{LnN(`5&0$=!j07sjC`$Z5+=x+@eG2|{@nk%=Ug6;;hr$elTjxs@ znZ`Y_spde?P7H=2s=h-RW=MvmxEc=Z3v0b8T#6yog`6V}9gd>dk!@ByI;%;qY9JRKKGYLU+h`M_{S>y8sBbut%JQ5QO(O9e zIlt@SD}M@xB@mrYtvC{txUWK!3z1`OmvD0tVqVNf=`<3dBjW$Z9R?QA6=k32I(1jy z)?>p)TEaOsP;Ey?H-rU_+Dc{maF|$Tb{{ROwdqW1*vMwPjUz^aj&O(xMm)+I7NrUE zC}uG3v|o2iEe~44MT8sBG{Fwz6ouJ5_ z6&4iAEIXLHFk|Jm+YGglTq+_y7je8ha)3ZxDZFmhRJ*P^)H){PQrY@hbuec4_x1HC zL<)q6s#t-Ljf9n&xI0RS?ap05|93iI|ZgxouGj5EHp#jH@}L zKu{!$H0f%K+N!XmIdUMCi9~Jb>7JSVa4vqth>XQYtV(I6uC~z_uN9IrS4FJy4ojrn(yFASoE1u&5E3wx>Hwosr|aTTXx)| zZeqMDSj>tmkr;Ed_QC`JR;=HMI|Y(Od@97nYm6tW~WA$ zJ(oLCAqlR%+|t6iN+UiU-`+a?{=`9v<>mCWX%$zi!tM-y=dUh}pnm857QyCu#rSnE{L1s08E8Z@JZOi7}yh^j65?68y#k%Qb1Mj}Ij*`uN zEctm2JzA(RiuGxoW0sjBnMieRJlo?$c>6xU3xuz!t4q#VZ6ysy|E%cBoYz!HG|l3Q zJ6p>wm?4jypI9pdaw*3#t}U~5#?Fpr`3ykgnqNR@kjBC#!G$01G;QYlo@kPlo?QPW zvX^|5nZAa>JH;a?G?x*h0MMeOK&nrt{`(m3im;(D8^R27Ac=ODj?LK#OV zs~M0Wpn}wc%b~%+cnPthXh0%XxG(|48gTl@8>+xcq@|ZmD>`y_S>sa-@wLwA@Tg(> zTD6t7b{S)7VXm;bj6!GH)tYhqv!HOHLEgHB#J=R#WARzaJ6UNtlUI_mLYK|BL?Pw0 zYw<2wR;)Mi-721ucXr+~tV8+74lys&$`au=4P{e_e9o{UT!5>171)(3V>J3yeR}2< zM5>x0UQ774%mtN)jT?IFqdIz5hI63aBan>(2#mcg&{CrgGGtDZA>1S659o}z+5rxsWc9!C+ z?OQq!c`k6#VU``m-lee0eK`xv%V-o4{-AOfJp38vZUhjL%Ptg#_?gYmewcko?!HBH z2h2HMg=ft1W=u+l_}+trOT${v3$wY8cjlsfLf<3zSDfLlprJ1WtI zYTU+OkZwm9-M9ul{3)mx$GKv=MICR)JNQICxYyK%jNVBdc+9!`c=8H!bU)AQHAfHd zbUt(RE}qYiU*K5hnPlhv)P)zn$Z^j2KK|X$zX$mDF8*0cdl0|Gaf73I1<^%u zD^H>`C7&JGidI&$7f!*J$Dy4?K;ct-@|Y(VO5krtaym<%pnW?2Du8$M973U!tL1ES zghKsURC*xFvq%KZS9kCyuR)W{mDW;MNptWkG?g+B=u>=vY56&v`c?k@S{HsD4e~{V z55jC8_lr>VWEMP$suu4A8Pj;S{JJH-Wx0^4-~F`9-IV_x)X^S=v((!{yocl}pz6qn zA6;hSQ&GyNY9^lpvUk$1R= zZjEb;_u>5{BkE}*Sy7Kq$W1rok)q8$dvmd?0w2Kd*hXDXf?`y^I7&@H9Lt{f60Yww zsaf{)yY}?sgQhzQU}?_}vJ+5hB9`Tv{KRDm>zN>xNUue-IZw;7 zdmNz?(7dPMtytro!rZ|XvT7Vx%%j=owfn-paa`Ftj;l^$QA=z61m??3_;kB}3iX4b zg<<~$7M&AH&!EzJ)jEl**Z5Cyw-$b% zGJr^w@Nri)!^$!|NQw_*JsoW;J?nN%Vh6ozCviVOiud4sk(3zWZ3w9a{dFZoaBlR6E7bn|4Dode?$pBMyWqRxj#+GKZ(C&6nKu~&*O2*Hi?&T zmXb~40{b@^jKvYtm_xogPLh|O+$zW`$UjekRsR@^Zq@M7N+(!7LRudAu4?&=r=OgdU{oEEQ*j>i*%7M(*@|%x zcPo2OVK<+==P_>zZG$qLz;)xeo-fw%$#)*hr8|oo6m__&oyf0i_f28JVCcrMZvt!1 zg}oM?swvzw7+O1lo6m*XLmk{jR_i8lOS?Dh9mo19Y#0oa`o?i=YWKBoza z{;>Z%t`GZ7r)idc%Ski{3kS4YiB&G$Wf5+XILh+S z?S-wO?!x}lwAeak%^qeO3z!)!r1GAmy3TR*6qevKxRJ(Qho`yjvs`%|eKZ>XF9>7y zv#cRLM_@dUBlseL^E?yamuQ7AGhly(A?vFI!Pf}2uj8+1@-N_?j>dzV0*-PeVk+JIfZIs#TP1#TFQJwH59VgPLI-=5t6!ldU!^5~WNLmjHPA}kjx$I|)zj>bA);}~Ri@k)f=_bJNeV;HKU5sJ_M zI%h8;sCap1K&#;a>Qx0Al@}|N4{Mbl9mL&6qC-MmKdZE$_zbPI|ncQ@Hm|?D_oh6l<6u6t>U6d>c88N-Jzi)@0>G7n zSr{-C_@^|gAU6_@^y+JjB!f>R^=@N;NNq5~kzFm}gcX>IPb)a5y+39eeR?pq$4D8B zxgi#dn_<%+rV8de8w>=(nfX9#e>5uah_8Kp;e>*POU62zjw!~V8uyucV}EzIo}{=M z>7^Y#s-VnP5Q+3$%8I|q77)`Z<2oXztYgx;1%LmTJ%X`Bzex`0;XVp0l$@1W53X+> zi0BCkf@=#V4WeD&-ESoIR71j`jAxH<$Hi)zq2Su7ljWES#8ZK$a4ez|3E5_Y(Gv?t z>odzS-3)9@8cuR20O3k>cV&4d=GwwZniCGJ(NmG6kuc)|S#e~-HiBN7ic|PO$oi9{ zOHoG7afrlV(DCRVoy5o>GnSMwvpp?Rui`T*KC9w$Dn75`3o4#d@w_+{PweZ6lU;hm zw6m+A?G3=@ke@yTWRrfzoT?8_#TOUlvZI|`T|tgPG*#27HlOXDU3!bgadVU;t!Gv3J zb;KFSmsxZhm?swQqSukGu;f?^Geg0~_J;P(?U%K$-`JLJ?iyBNm4>h2s|se1KOP$U zjcAupz48tBZu+&fo4r}X*YI@}-_Y<)d`n)wO<-y@d`I5CiB(;FlVHg;kEhs2qFbLwv7fO2e=5 z8x6n3@90-j!z=i`hCkqsV(w2GR^!bY;&`iuKjUQ$A+)IYi-y1AZwg8s|Hji*bpNj5 zANZ$+f8pO64&WvY|G|G*@)MJ-U}X!7ZWN*6cmo8ZvBp(Y%61??8Ba|U_8;A?46{S*gY*HF-1#t zz#Q9>nVm^3%PBk0S(&uw^A_Q(r+X|ltXQTx+Z3GZXiQz|j+>-Q7FDvGnY7YXQUMid z>r>R8T}}1{r73eqC|ok(9^wrr5_*h{wKm-+EdJS{oK6KwcJ<8(#R$7@^q4#M-%*>kE?(gB*{4Q1hLX1sz|E@|dGRWeU`?ad8qwy$pu zbt+hFdp*@Ij=Jn-UANYz9e-Vgb?dTDR&JfpyT;tdJl26ge8OE_8~PKxC-N{N?xooo z&vcx60jO928eeqh5hy3=4l&0G2`|{~W0HW;3yuPP^w!|qQ6}avV|>yQBpIE}rzm=E zUPQmg?>)!KZ@MXu`bC9v#TZxQ8YSow)k>JrfAUdq~r4R;2sD#9WDA1(NCZ@Lp-la>WxyIR!tqqe*6*(#dsaE3)3b$fXP~2$Z;(5&@11M^DuWD90_8 zI{tdC$>pf$vt|q661<6Td2*1_1eKLiZtCS+U)2zDLeEPLVmT2v&-lr=#&SS(!B;u!x~(GX1chRl9%IZ1aU1|sBs0q)SIyj z>#e|+Q)45oTuL1`QCo#ZG|?slqJ?IT9)9bunJ1`Ucu2Pca1b+F3t*Kt4^Y*laLpOs)sP6g`v-Eta%)>@)iuC zq|U?1*?!Nl78wVT*IH9M1b@w`5QBF!#QFSlK`9oY3bh=8bD8Wy&Rt}UHYkfI2VBM9 zPE5xZ%)nOaZKLc~N?t|DZ6wCk86c)(Hd+Zx4ke3u?Yfdc)b*uXeeaK3|J2~}2E zfWZRdLbyk0E{D;p{Bwt*qI3i*T=qNPtSQe36}gr#KZO=J1vDt&Y~sr1pyg+Xo?5)h z*+`6hTs%l#&1tQzDL#il%oRpF&(G01kKg?qa)loA2$CQv&+|{9j7uKkS?BeWelu69 zey?C!q6(;avNStOHSZYZWNr{z9Yy^V^ZUK0Z2L1U`?5qKG3WPtPSv@Yfjknfg#;`` z7~L>977)9Bjuk{p#_jx-@p>G<%^W=}&pQ}+1u>U#JAY-gGqsk$0X-Jk3TSU8cGBK% z>g=ROl)Lqj$}yrI=e`NzncPP2E+#W1lU{0+S(#L6qshvo zhrS*l8@7^1ccpXc7NpaJm#1xHh2;4luHhaz^y_w(Png~c^`C<8EEcHPp(3K9>ojsk z*iQ0P=qkEZ^f)U;Dtg@%07huTJ8?ZtmOQ8v*Bs*HimqqedRlS%Bt~tzSz6qC)T7tr zok5%}-ti+w>JwO5HH`AsTC1PcR?i*Af@svX98qi=N{d){-3>eHCI zTACXr5F}>uJGhMfPxyo+ck@Ivt)4O7555n%`hqnNYyOw(K>gr)69#d zn~N5)VO?p%Si0G@!jlp99IaEG7r%On`+M1eSFbXM*v{n(<^H;d}TRF>p zlsw}DyAZ{4SjQ!Fri3PBw85FrK!At2I>>ZBLYYSi*CUj8l=ONmi%MmgaLNdWO{KCl zm2xo83egUy1a!Y7?f#AI-}|llqvQM(PCI^$1qV=2^&Il6p27tR9dL(^bK1>Do}X!T zB}5hc3%47`IsXJ#9_Qioq~(}>%_zXz$bI{8QISLX7D{w)=Tr_0LeAAfn!SbpH*e)% iIq`112XEo*97^q@>FeOfJMkjkb*Z0!t?$K8k@tT_`UD~X literal 0 HcmV?d00001 From 55c88fbeb493f47d497bc3e3fa2944a9ff44f1fd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 14 Dec 2021 12:46:12 +0100 Subject: [PATCH 09/12] Ktlint --- .../wrap/visitor/WrappingVisitorTest.kt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt index 14ecda8ce..dbb3392ee 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt @@ -1,3 +1,4 @@ +// ktlint-disable experimental:argument-list-wrapping package io.sentry.android.gradle.instrumentation.wrap.visitor import com.android.build.api.instrumentation.ClassData @@ -201,11 +202,12 @@ class WrappingVisitorTest { assertEquals(fixture.visitor.varVisits[2], VarVisit(Opcodes.ALOAD, 1)) // replacement/wrapping method visited assertEquals( - fixture.visitor.methodVisits[1], MethodVisit( - opcode = Opcodes.INVOKESTATIC, - owner = "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", - name = "create", - descriptor = "(Ljava/io/FileInputStream;Ljava/lang/String;)Ljava/io/FileInputStream;", + fixture.visitor.methodVisits[1], + MethodVisit( + Opcodes.INVOKESTATIC, + "io/sentry/instrumentation/file/SentryFileInputStream${'$'}Factory", + "create", + "(Ljava/io/FileInputStream;Ljava/lang/String;)Ljava/io/FileInputStream;", isInterface = false ) ) From 6a24aa80da5736ee3e5a0997e9df37b09778bc92 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 14 Dec 2021 13:15:59 +0100 Subject: [PATCH 10/12] ktlint --- .../wrap/visitor/WrappingVisitorTest.kt | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt index dbb3392ee..5a3aab3b5 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt @@ -1,4 +1,3 @@ -// ktlint-disable experimental:argument-list-wrapping package io.sentry.android.gradle.instrumentation.wrap.visitor import com.android.build.api.instrumentation.ClassData @@ -43,14 +42,13 @@ class WrappingVisitorTest { descriptor = "(Ljava/lang/String;)V", isInterface = false ) - fixture.getSut(context) - .visitMethodInsn( - opcode = methodVisit.opcode, - owner = methodVisit.owner, - name = methodVisit.name, - descriptor = methodVisit.descriptor, - isInterface = methodVisit.isInterface - ) + fixture.getSut(context).visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) assertEquals( fixture.logger.capturedMessage, @@ -71,14 +69,13 @@ class WrappingVisitorTest { descriptor = "(Ljava/lang/String;)V", isInterface = false ) - fixture.getSut(context) - .visitMethodInsn( - opcode = methodVisit.opcode, - owner = methodVisit.owner, - name = methodVisit.name, - descriptor = methodVisit.descriptor, - isInterface = methodVisit.isInterface - ) + fixture.getSut(context).visitMethodInsn( + opcode = methodVisit.opcode, + owner = methodVisit.owner, + name = methodVisit.name, + descriptor = methodVisit.descriptor, + isInterface = methodVisit.isInterface + ) // method visit should remain unchanged assertEquals(fixture.visitor.methodVisits.size, 1) @@ -180,6 +177,7 @@ class WrappingVisitorTest { descriptor = "(Ljava/lang/String;)V", isInterface = false ) + /* ktlint-disable experimental:argument-list-wrapping */ fixture.getSut(context, replacements = mapOf(Replacement.FileInputStream.STRING)) .visitMethodInsn( methodVisit.opcode, @@ -188,6 +186,7 @@ class WrappingVisitorTest { methodVisit.descriptor, methodVisit.isInterface ) + /* ktlint-enable experimental:argument-list-wrapping */ assertEquals(fixture.visitor.methodVisits.size, 2) // store original arguments From fe98adc95083a80a0c556844fb0a4ef71e1b755e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 14 Dec 2021 13:16:13 +0100 Subject: [PATCH 11/12] Bump sentry-android version --- buildSrc/src/main/java/Dependencies.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 35cdd6559..ae05b6769 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -13,7 +13,7 @@ object LibsVersion { const val JUNIT = "4.13.2" const val ASM = "9.2" const val SQLITE = "2.1.0" - const val SENTRY = "5.4.0" + const val SENTRY = "5.5.0" } object Libs { From 305cb2e396866844267cf8a1fba035fb6c069ea0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 14 Dec 2021 13:17:13 +0100 Subject: [PATCH 12/12] Var -> variable rename --- .../instrumentation/wrap/visitor/WrappingVisitorTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt index 5a3aab3b5..76d0b7e26 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/wrap/visitor/WrappingVisitorTest.kt @@ -223,7 +223,7 @@ data class MethodVisit( data class VarVisit( val opcode: Int, - val `var`: Int + val variable: Int ) class CapturingMethodVisitor : MethodVisitor(Opcodes.ASM9) { @@ -231,9 +231,9 @@ class CapturingMethodVisitor : MethodVisitor(Opcodes.ASM9) { val methodVisits = mutableListOf() val varVisits = mutableListOf() - override fun visitVarInsn(opcode: Int, `var`: Int) { - super.visitVarInsn(opcode, `var`) - varVisits += VarVisit(opcode, `var`) + override fun visitVarInsn(opcode: Int, variable: Int) { + super.visitVarInsn(opcode, variable) + varVisits += VarVisit(opcode, variable) } override fun visitMethodInsn(