Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ internal class SQLiteSpanManager(
if (result is CrossProcessCursor) {
return SentryCrossProcessCursor(result, this, sql) as T
}
spans.recordSpan(sql, startTimestamp, SpanStatus.OK)
spans.recordCoarseSpan(sql, startTimestamp, SpanStatus.OK)
result
} catch (e: Throwable) {
spans.recordSpan(sql, startTimestamp, SpanStatus.INTERNAL_ERROR, e)
spans.recordCoarseSpan(sql, startTimestamp, SpanStatus.INTERNAL_ERROR, e)
throw e
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package io.sentry.sqlite

import io.sentry.IScopes
import io.sentry.ISpan
import io.sentry.Instrumenter
import io.sentry.ScopesAdapter
import io.sentry.SentryDate
import io.sentry.SentryLongDate
import io.sentry.SentryNanotimeDate
import io.sentry.SentryStackTraceFactory
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
Expand All @@ -28,36 +30,44 @@ internal class SQLiteSpanInstrumentation(
*/
fun startTimestamp(): SentryDate = scopes.options.dateProvider.now()

/** Records a `db.sql.query` span from [startTimestamp] to the moment of invocation. */
/** Records a `db.sql.query` span from [startTimestamp] to [startTimestamp] + [durationNanos]. */
fun recordSpan(
sql: String,
startTimestamp: SentryDate,
durationNanos: Long,
status: SpanStatus,
throwable: Throwable? = null,
) {
recordSpan(sql, startTimestamp, endTimestamp = null, status, throwable)
val parent = scopes.span ?: return
val nanoPrecisionStart = startTimestamp.repairPrecision(baseline = parent.startDate)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repairing the incoming timestamp is the only interesting change this PR makes. (The diff looks a bit bigger because I switched the order of the first and second methods and renamed the second recordCoarseSpan.)

val endTimestamp = SentryLongDate(nanoPrecisionStart.nanoTimestamp() + durationNanos)
parent.recordChild(sql, nanoPrecisionStart, endTimestamp, status, throwable)
}

/** Records a `db.sql.query` span from [startTimestamp] to [startTimestamp] + [durationNanos]. */
fun recordSpan(
/**
* Records a `db.sql.query` span from [startTimestamp] to the moment of invocation.
*
* "Coarse" in that it doesn't ensure nanosecond precision for [SentryNanotimeDate]
* [startTimestamp]s.
*/
fun recordCoarseSpan(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Chime in if you can think of a better name. Not in love with this, but it'll do if it has to...

sql: String,
startTimestamp: SentryDate,
durationNanos: Long,
status: SpanStatus,
throwable: Throwable? = null,
) {
val endTimestamp = SentryLongDate(startTimestamp.nanoTimestamp() + durationNanos)
recordSpan(sql, startTimestamp, endTimestamp, status, throwable)
val parent = scopes.span ?: return
parent.recordChild(sql, startTimestamp, endTimestamp = null, status, throwable)
}

private fun recordSpan(
private fun ISpan.recordChild(
sql: String,
startTimestamp: SentryDate,
endTimestamp: SentryDate?,
status: SpanStatus,
throwable: Throwable?,
) {
scopes.span?.startChild("db.sql.query", sql, startTimestamp, Instrumenter.SENTRY)?.apply {
startChild("db.sql.query", sql, startTimestamp, Instrumenter.SENTRY).apply {
spanContext.origin = SQLITE_TRACE_ORIGIN
throwable?.let { this.throwable = it }

Expand All @@ -74,6 +84,38 @@ internal class SQLiteSpanInstrumentation(
}
}

/**
* Repairs the receiver's [nanoTimestamp][SentryDate.nanoTimestamp] if needed so that it actually
* has nanosecond precision.
*
* Designed for use with spans whose start timestamps are [SentryNanotimeDate]s. Without repair,
* those timestamps will be aligned to the same millisecond at transport, and the Sentry UI will
* arbitrarily reorder them:
* ```
* Parent span ├█████████████┤
* END TRANSACTION ├███┤ 0.18 ms ← (Wrong order)
* BEGIN IMMEDIATE TRANSACTION ├████┤ 0.25 ms
* INSERT INTO `my_db` … ├██┤ 0.10 ms
* ↑
* (All spans share the same ms baseline
* even though their execution was staggered)
* ```
*
* Repair ensures proper ordering and lets the spans stagger:
* ```
* Parent span ├█████████████┤
* BEGIN IMMEDIATE TRANSACTION ├████┤ 0.25 ms
* INSERT INTO `my_db` … ├██┤ 0.10 ms
* END TRANSACTION ├███┤ 0.18 ms
* ```
*/
private fun SentryDate.repairPrecision(baseline: SentryDate?): SentryDate =

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This arguably belongs on SentryDate, especially as the latter's API is already enmeshed in the broken precision of SentryDate.nanoTimestamp. But I struggled to get it to work. I considered:

[1] adding a SentryDate.laterDateByDiff() that returns a SentryLongDate (declared as a SentryDate).

...but that seemed heavyweight for a single call site (new public method, tests against all implementing classes), esp as the only thing it buys us is a replacement for the already minimal line 112. Plus, bumping the minSdkVersion to 26 will let us move off of SentryNanotimeDate, so any investments in it will be short-lived.

[2] adding a repairPrecision() method solely to SentryNanotimeDate (allowing us to relocate all of the repair method).

...but that'd also be a new public API, and it'd mean we'd have to cast at the call site, which would be against the grain of how we typically use SentryDate.

So I left things as you see them, figuring we could shuffle bits around if we ever get a second call site.

Let me know if you have any opinions.

if (baseline is SentryNanotimeDate) {
SentryLongDate(baseline.laterDateNanosTimestampByDiff(this))
} else {
this
}

companion object {

fun fromDatabaseName(databaseName: String?, scopes: IScopes = ScopesAdapter.getInstance()) =
Expand Down
Loading
Loading