Skip to content

Commit 7c34b37

Browse files
stefanosianostefanosiano
authored
Fix lazy select queries instrumentation (#3604)
* added SentryCrossProcessCursor wrapper * SQLiteSpanManager now wraps CrossProcessCursors to start a span only when the cursor is filled with data
1 parent ae2294f commit 7c34b37

File tree

5 files changed

+214
-3
lines changed

5 files changed

+214
-3
lines changed

‎CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Fixes
6+
7+
- Fix lazy select queries instrumentation ([#3604](https://github.com/getsentry/sentry-java/pull/3604))
8+
39
## 7.13.0
410

511
### Features

‎sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SQLiteSpanManager.kt

+18-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
package io.sentry.android.sqlite
22

3+
import android.database.CrossProcessCursor
34
import android.database.SQLException
45
import io.sentry.HubAdapter
56
import io.sentry.IHub
7+
import io.sentry.ISpan
8+
import io.sentry.Instrumenter
69
import io.sentry.SentryIntegrationPackageStorage
710
import io.sentry.SentryStackTraceFactory
811
import io.sentry.SpanDataConvention
@@ -27,16 +30,28 @@ internal class SQLiteSpanManager(
2730
* @param operation The sql operation to execute.
2831
* In case of an error the surrounding span will have its status set to INTERNAL_ERROR
2932
*/
30-
@Suppress("TooGenericExceptionCaught")
33+
@Suppress("TooGenericExceptionCaught", "UNCHECKED_CAST")
3134
@Throws(SQLException::class)
3235
fun <T> performSql(sql: String, operation: () -> T): T {
33-
val span = hub.span?.startChild("db.sql.query", sql)
34-
span?.spanContext?.origin = TRACE_ORIGIN
36+
val startTimestamp = hub.getOptions().dateProvider.now()
37+
var span: ISpan? = null
3538
return try {
3639
val result = operation()
40+
/*
41+
* SQLiteCursor - that extends CrossProcessCursor - executes the query lazily, when one of
42+
* getCount() or onMove() is called. In this case we don't have to start the span here.
43+
* Otherwise we start the span with the timestamp taken before the operation started.
44+
*/
45+
if (result is CrossProcessCursor) {
46+
return SentryCrossProcessCursor(result, this, sql) as T
47+
}
48+
span = hub.span?.startChild("db.sql.query", sql, startTimestamp, Instrumenter.SENTRY)
49+
span?.spanContext?.origin = TRACE_ORIGIN
3750
span?.status = SpanStatus.OK
3851
result
3952
} catch (e: Throwable) {
53+
span = hub.span?.startChild("db.sql.query", sql, startTimestamp, Instrumenter.SENTRY)
54+
span?.spanContext?.origin = TRACE_ORIGIN
4055
span?.status = SpanStatus.INTERNAL_ERROR
4156
span?.throwable = e
4257
throw e

‎sentry-android-sqlite/src/main/java/io/sentry/android/sqlite/SentryCrossProcessCursor.kt

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package io.sentry.android.sqlite
2+
3+
import android.database.CrossProcessCursor
4+
import android.database.CursorWindow
5+
6+
/*
7+
* SQLiteCursor executes the query lazily, when one of getCount() and onMove() is called.
8+
* Also, by docs, fillWindow() can be used to fill the cursor with data.
9+
* So we wrap these methods to create a span.
10+
* SQLiteCursor is never used directly in the code, but only the Cursor interface.
11+
* This means we can use CrossProcessCursor - that extends Cursor - as wrapper, since
12+
* CrossProcessCursor is an interface and we can use Kotlin delegation.
13+
*/
14+
internal class SentryCrossProcessCursor(
15+
private val delegate: CrossProcessCursor,
16+
private val spanManager: SQLiteSpanManager,
17+
private val sql: String
18+
) : CrossProcessCursor by delegate {
19+
// We have to start the span only the first time, regardless of how many times its methods get called.
20+
private var isSpanStarted = false
21+
22+
override fun getCount(): Int {
23+
if (isSpanStarted) {
24+
return delegate.count
25+
}
26+
isSpanStarted = true
27+
return spanManager.performSql(sql) {
28+
delegate.count
29+
}
30+
}
31+
32+
override fun onMove(oldPosition: Int, newPosition: Int): Boolean {
33+
if (isSpanStarted) {
34+
return delegate.onMove(oldPosition, newPosition)
35+
}
36+
isSpanStarted = true
37+
return spanManager.performSql(sql) {
38+
delegate.onMove(oldPosition, newPosition)
39+
}
40+
}
41+
42+
override fun fillWindow(position: Int, window: CursorWindow?) {
43+
if (isSpanStarted) {
44+
return delegate.fillWindow(position, window)
45+
}
46+
isSpanStarted = true
47+
return spanManager.performSql(sql) {
48+
delegate.fillWindow(position, window)
49+
}
50+
}
51+
}

‎sentry-android-sqlite/src/test/java/io/sentry/android/sqlite/SQLiteSpanManagerTest.kt

+15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.sentry.android.sqlite
22

3+
import android.database.CrossProcessCursor
34
import android.database.SQLException
45
import io.sentry.IHub
56
import io.sentry.SentryIntegrationPackageStorage
@@ -15,6 +16,7 @@ import org.mockito.kotlin.whenever
1516
import kotlin.test.Test
1617
import kotlin.test.assertEquals
1718
import kotlin.test.assertFalse
19+
import kotlin.test.assertIs
1820
import kotlin.test.assertNotNull
1921
import kotlin.test.assertNull
2022
import kotlin.test.assertTrue
@@ -140,4 +142,17 @@ class SQLiteSpanManagerTest {
140142

141143
assertEquals(span.data[SpanDataConvention.DB_SYSTEM_KEY], "in-memory")
142144
}
145+
146+
@Test
147+
fun `when performSql returns a CrossProcessCursor, does not start a span and returns a SentryCrossProcessCursor`() {
148+
val sut = fixture.getSut()
149+
150+
// When performSql returns a CrossProcessCursor
151+
val result = sut.performSql("sql") { mock<CrossProcessCursor>() }
152+
153+
// Returns a SentryCrossProcessCursor
154+
assertIs<SentryCrossProcessCursor>(result)
155+
// And no span is started
156+
assertNull(fixture.sentryTracer.children.firstOrNull())
157+
}
143158
}

‎sentry-android-sqlite/src/test/java/io/sentry/android/sqlite/SentryCrossProcessCursorTest.kt

+124
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package io.sentry.android.sqlite
2+
3+
import android.database.CrossProcessCursor
4+
import io.sentry.IHub
5+
import io.sentry.ISpan
6+
import io.sentry.SentryOptions
7+
import io.sentry.SentryTracer
8+
import io.sentry.SpanStatus
9+
import io.sentry.TransactionContext
10+
import org.mockito.kotlin.any
11+
import org.mockito.kotlin.eq
12+
import org.mockito.kotlin.mock
13+
import org.mockito.kotlin.verify
14+
import org.mockito.kotlin.whenever
15+
import kotlin.test.Test
16+
import kotlin.test.assertEquals
17+
import kotlin.test.assertNotNull
18+
import kotlin.test.assertTrue
19+
20+
class SentryCrossProcessCursorTest {
21+
private class Fixture {
22+
private val hub = mock<IHub>()
23+
private val spanManager = SQLiteSpanManager(hub)
24+
val mockCursor = mock<CrossProcessCursor>()
25+
lateinit var options: SentryOptions
26+
lateinit var sentryTracer: SentryTracer
27+
28+
fun getSut(sql: String, isSpanActive: Boolean = true): SentryCrossProcessCursor {
29+
options = SentryOptions().apply {
30+
dsn = "https://[email protected]/proj"
31+
}
32+
whenever(hub.options).thenReturn(options)
33+
sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)
34+
35+
if (isSpanActive) {
36+
whenever(hub.span).thenReturn(sentryTracer)
37+
}
38+
return SentryCrossProcessCursor(mockCursor, spanManager, sql)
39+
}
40+
}
41+
42+
private val fixture = Fixture()
43+
44+
@Test
45+
fun `all calls are propagated to the delegate`() {
46+
val sql = "sql"
47+
val cursor = fixture.getSut(sql)
48+
49+
cursor.onMove(0, 1)
50+
verify(fixture.mockCursor).onMove(eq(0), eq(1))
51+
52+
cursor.count
53+
verify(fixture.mockCursor).count
54+
55+
cursor.fillWindow(0, mock())
56+
verify(fixture.mockCursor).fillWindow(eq(0), any())
57+
58+
// Let's verify other methods are delegated, even if not explicitly
59+
cursor.close()
60+
verify(fixture.mockCursor).close()
61+
62+
cursor.getString(1)
63+
verify(fixture.mockCursor).getString(eq(1))
64+
}
65+
66+
@Test
67+
fun `getCount creates a span if a span is running`() {
68+
val sql = "execute"
69+
val sut = fixture.getSut(sql)
70+
assertEquals(0, fixture.sentryTracer.children.size)
71+
sut.count
72+
val span = fixture.sentryTracer.children.firstOrNull()
73+
assertSqlSpanCreated(sql, span)
74+
}
75+
76+
@Test
77+
fun `getCount does not create a span if no span is running`() {
78+
val sut = fixture.getSut("execute", isSpanActive = false)
79+
sut.count
80+
assertEquals(0, fixture.sentryTracer.children.size)
81+
}
82+
83+
@Test
84+
fun `onMove creates a span if a span is running`() {
85+
val sql = "execute"
86+
val sut = fixture.getSut(sql)
87+
assertEquals(0, fixture.sentryTracer.children.size)
88+
sut.onMove(0, 5)
89+
val span = fixture.sentryTracer.children.firstOrNull()
90+
assertSqlSpanCreated(sql, span)
91+
}
92+
93+
@Test
94+
fun `onMove does not create a span if no span is running`() {
95+
val sut = fixture.getSut("execute", isSpanActive = false)
96+
sut.onMove(0, 5)
97+
assertEquals(0, fixture.sentryTracer.children.size)
98+
}
99+
100+
@Test
101+
fun `fillWindow creates a span if a span is running`() {
102+
val sql = "execute"
103+
val sut = fixture.getSut(sql)
104+
assertEquals(0, fixture.sentryTracer.children.size)
105+
sut.fillWindow(0, mock())
106+
val span = fixture.sentryTracer.children.firstOrNull()
107+
assertSqlSpanCreated(sql, span)
108+
}
109+
110+
@Test
111+
fun `fillWindow does not create a span if no span is running`() {
112+
val sut = fixture.getSut("execute", isSpanActive = false)
113+
sut.fillWindow(0, mock())
114+
assertEquals(0, fixture.sentryTracer.children.size)
115+
}
116+
117+
private fun assertSqlSpanCreated(sql: String, span: ISpan?) {
118+
assertNotNull(span)
119+
assertEquals("db.sql.query", span.operation)
120+
assertEquals(sql, span.description)
121+
assertEquals(SpanStatus.OK, span.status)
122+
assertTrue(span.isFinished)
123+
}
124+
}

0 commit comments

Comments
 (0)