Skip to content

fix(tracing): End timed-out transactions when the timeout is due - #6091

Open
runningcode wants to merge 8 commits into
mainfrom
no/clamp-late-transaction-timeouts
Open

fix(tracing): End timed-out transactions when the timeout is due#6091
runningcode wants to merge 8 commits into
mainfrom
no/clamp-late-transaction-timeouts

Conversation

@runningcode

@runningcode runningcode commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📜 Description

in #5752 we see that an app can go in to doze during startup leading to very long startup transactions or spans.
The issue is that when the device goes in to doze, the timer is also paused and only fires when the device wakes up leading to hours long startup spans.

To fix this we essentially set the length of the transaction to the timeout duration. This is a PRODUCT DECISION! So feel free to disagree with this. Another alternate is to discard the transaction (which is also easier code-wise). Data-wise that might make more sense too!

Designwise, I added a Timestamp.getSentryDate() to bridge the gap between the new APIs and the existing. Otherwise we need to make a lot of breaking changes to the Span object. I'm also open to that, but we should save that for v9.

💡 Motivation and Context

#5752

See also this other PR which tries to solve the same problem by discarding the transaction: #5755

💚 How did you test it?

Well we can't really test the doze feature here. We're just cutting off everything that exceeds the deadline.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

🤖 Generated with Claude Code

runningcode and others added 2 commits September 10, 2026 15:56
…AVA-642)

The idle and deadline timers run on a thread that is frozen while the device
is in deep sleep or the process is cached, so a timeout scheduled for 30s can
fire hours later. Both callbacks stamped the spans with the wake-up time,
turning an app start the user walked away from into a multi-hour transaction.

Record when each timeout falls due at scheduling time and stamp with that
instead, whenever the timer runs late. The deadline path passes the clamped
timestamp down to forceFinish, which stamps every child before the root
finishes and so otherwise defeats trimEnd.

Fixes #5752

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 10, 2026

Copy link
Copy Markdown

JAVA-642

@sentry

sentry Bot commented Sep 10, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.56.0 (1) release

⚙️ sentry-android Build Distribution Settings

runningcode and others added 6 commits September 10, 2026 16:03
Whether a timeout expired was decided by comparing a fresh wall-clock reading
against the projected due date. That is a duration measured from two
independent wall-clock readings, which the clock can lengthen, shorten or make
negative. A backward step while the timer waits made an expired timeout look
pending, so the transaction was stamped with the wake-up time again; a forward
step truncated a transaction whose timeout had not expired.

Measure expiry on io.sentry.time.Deadline, which runs on the monotonic ticker
and, on Android, on CLOCK_BOOTTIME so the interval includes deep sleep. The
instant to end at stays a wall-clock timestamp, projected once from the same
reading that sets the deadline.

An expired timeout now always ends the transaction at the instant it fell due,
including an on-time fire, which drops the scheduler jitter from the reported
duration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tracer's timeout logic now works in io.sentry.time terms throughout: the
duration on a Deadline, the instant as a Timestamp read from the options'
EpochClock. Timestamp.toSentryDate bridges to the type the span API still
takes, at the single point of use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FixedEpochClock, TestMonotonicTicker and DeferredExecutorService already
existed; the tests were hand-rolling a date-provider fake and waiting on the
real timer. Driving the ticker and the timer directly also removes the race
between the test advancing its clock and the 20ms timer actually firing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rename Timestamp.toSentryDate to getSentryDate so Kotlin callers reach it as
expiry?.expiredAt()?.sentryDate, and note at the call sites what the bridge is
for and where the tracer reaches into options for its clocks.

Also drops a meaningless @NotNull on forceFinish's void return.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The paragraph on which class holds the instant restated the field types. What
is left is the part the code cannot say: why a late timer is possible at all,
and why only a monotonic deadline can tell.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@runningcode runningcode added the sanity-check PR needs a lightweight review for obvious issues label Sep 10, 2026
@runningcode
runningcode marked this pull request as ready for review September 10, 2026 16:22
@runningcode runningcode changed the title fix(tracing): End timed-out transactions when the timeout fell due fix(tracing): End timed-out transactions when the timeout is due Sep 10, 2026
Comment on lines 124 to 130
if (idleTimeout != null) {
cancelIdleTimer();
isIdleFinishTimerRunning.set(true);
idleExpiry = expiryIn(idleTimeout);

try {
idleTimeoutFuture =

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A race condition in scheduleFinish() can cause an old timer to use a new idleExpiry value, resulting in an incorrect transaction finish time.
Severity: MEDIUM

Suggested Fix

To prevent the race condition, avoid using a shared, mutable idleExpiry field. Instead, the Expiry object should be captured by the timer's callback at the time of scheduling. Pass the specific Expiry instance to the onIdleTimeoutReached runnable, ensuring it uses the correct value and is not affected by subsequent calls to scheduleFinish().

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry/src/main/java/io/sentry/SentryTracer.java#L124-L130

Potential issue: A race condition exists when rescheduling an idle transaction's finish
timer. When `scheduleFinish()` is called, it overwrites the class field `idleExpiry`. If
a previously scheduled timer's callback, `onIdleTimeoutReached()`, executes after this
field is overwritten but before the old timer is successfully canceled, it will read the
new `idleExpiry` value. This causes the deadline check to fail, and the transaction
finishes with the current time instead of its originally intended timeout time,
defeating the purpose of using a monotonic clock for late-firing timers.

Also affects:

  • sentry/src/main/java/io/sentry/SentryTracer.java:148~154

Did we get this right? 👍 / 👎 to inform future reviews.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit edd56b0. Configure here.


final @NotNull SentryDate finishTimestamp = scopes.getOptions().getDateProvider().now();
final @NotNull SentryDate finishTimestamp =
finishDate != null ? finishDate : scopes.getOptions().getDateProvider().now();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Child timestamps ignore timeout cutoff

Medium Severity

When a late timeout backdates the transaction, children that already finished keep timestamps after that cutoff and extend past the parent. Unfinished children started after the due instant are ended at that earlier time, so their finish precedes their start.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit edd56b0. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sanity-check PR needs a lightweight review for obvious issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant