Skip to content
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

Support Room kotlin gencode #630

Merged
merged 9 commits into from
Jan 15, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

### Fixes

- Support Room kotlin codegen ([#630](https://github.com/getsentry/sentry-android-gradle-plugin/pull/630))
- Make sentry-cli path calculation configuration-cache compatible ([#631](https://github.com/getsentry/sentry-android-gradle-plugin/pull/631))
- This will prevent build from failing when e.g. switching branches with stale configuration cache

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ 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.androidx.room.visitor.AndroidXRoomDaoVisitor
import io.sentry.android.gradle.instrumentation.androidx.room.visitor.InstrumentableMethodsCollectingVisitor
import io.sentry.android.gradle.instrumentation.androidx.room.visitor.RoomQueryVisitor
import io.sentry.android.gradle.instrumentation.androidx.room.visitor.RoomQueryWithTransactionVisitor
import io.sentry.android.gradle.instrumentation.androidx.room.visitor.RoomTransactionVisitor
import io.sentry.android.gradle.util.info
import org.objectweb.asm.ClassVisitor
import org.objectweb.asm.MethodVisitor
Expand Down Expand Up @@ -84,32 +82,13 @@ class RoomMethod(
apiVersion: Int,
originalVisitor: MethodVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters
): MethodVisitor = when (type) {
RoomMethodType.TRANSACTION -> RoomTransactionVisitor(
className,
apiVersion,
methodNode,
originalVisitor,
instrumentableContext.access,
instrumentableContext.descriptor
)
RoomMethodType.QUERY -> RoomQueryVisitor(
className,
apiVersion,
methodNode,
originalVisitor,
instrumentableContext.access,
instrumentableContext.descriptor
)
RoomMethodType.QUERY_WITH_TRANSACTION -> RoomQueryWithTransactionVisitor(
className,
apiVersion,
methodNode,
originalVisitor,
instrumentableContext.access,
instrumentableContext.descriptor
)
}
): MethodVisitor = AndroidXRoomDaoVisitor(
className,
apiVersion,
originalVisitor,
instrumentableContext.access,
instrumentableContext.descriptor
)

override fun isInstrumentable(data: MethodContext): Boolean {
return data.name == fqName && data.descriptor == methodNode.desc &&
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package io.sentry.android.gradle.instrumentation.androidx.room.visitor

import io.sentry.android.gradle.instrumentation.AbstractSpanAddingMethodVisitor
import io.sentry.android.gradle.instrumentation.SpanOperations
import org.objectweb.asm.Label
import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Opcodes

class AndroidXRoomDaoVisitor(
private val className: String,
api: Int,
private val originalVisitor: MethodVisitor,
access: Int,
descriptor: String?
) : AbstractSpanAddingMethodVisitor(
api = api,
originalVisitor = originalVisitor,
access = access,
descriptor = descriptor
) {
private val childIfNullStatusOk = Label()
private val childIfNullFinallyPositive = Label()
private val childIfNullFinallyNegative = Label()
private val startSpanIfNull = Label()
private var finallyVisitCount = 0

override fun visitCode() {
super.visitCode()
originalVisitor.visitStartSpan(startSpanIfNull) {
visitLdcInsn(SpanOperations.DB_SQL_ROOM)
visitLdcInsn(className)
}
originalVisitor.visitLabel(startSpanIfNull)
}

override fun visitMethodInsn(
opcode: Int,
owner: String?,
name: String?,
descriptor: String?,
isInterface: Boolean
) {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
// The backend binds the exception to the current span, via tracing without performance, so we don't need any special try/catch management.
if (opcode == Opcodes.INVOKEVIRTUAL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm, does INVOKEVIRTUAL work with the CLOSE method? It used to be INVOKEINTERFACE before

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch!
I updated the instrumentation to take the method type into consideration and updated the condition for the close method

when (name) {
SET_TRANSACTION_SUCCESSFUL -> {
// the original method wants to return, but we intervene here to set status
originalVisitor.visitSetStatus(status = "OK", gotoIfNull = childIfNullStatusOk)
originalVisitor.visitLabel(childIfNullStatusOk)
}
CLOSE, END_TRANSACTION -> {
// room's finally block ends here, we add our code to finish the span

// we visit finally block 2 times - one for the positive path in control flow (try) one for negative (catch)
// hence we need to use different labels
val visitCount = ++finallyVisitCount
val label = if (visitCount == 1) {
childIfNullFinallyPositive
} else {
childIfNullFinallyNegative
}
originalVisitor.visitFinallyBlock(gotoIfNull = label)
originalVisitor.visitLabel(label)
}
}
}
}

companion object {
private const val CLOSE = "close"
private const val END_TRANSACTION = "endTransaction"
private const val SET_TRANSACTION_SUCCESSFUL = "setTransactionSuccessful"
}
}

This file was deleted.

Loading
Loading