Skip to content

ref(core): Measure the performance-collection budget on a monotonic ticker (JAVA-579) - #6101

Open
runningcode wants to merge 3 commits into
9.x.xfrom
no/java-579-c6-collection-budget
Open

runningcode wants to merge 3 commits into
9.x.xfrom
no/java-579-c6-collection-budget

Conversation

@runningcode

@runningcode runningcode commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

📜 Description

This switches the DefaultCompositePerformanceCollector to use a monotonic clock for the collection deadline.
Before it seemed like it was using a monotonic clock but SentryNanotimeDate.nanoTimestamp return the epoch millis. 😢

Each CompositeData now holds a Deadline on options.getMonotonicTicker().

Tiny behavior changes:

  • The 30 second boundary is now inclusive: Deadline.hasPassed() is >=, where the old check was >.
  • addDataAndCheckTimeout no longer takes a shared nowNanos, since each transaction owns its own deadline.
  • TestMonotonicTicker's field is now @Volatile, because the collector's timer thread reads a ticker the test thread advances.

💡 Motivation and Context

Clock-usage audit finding §C6.

💚 How did you test it?

The two 30 second tests in DefaultCompositePerformanceCollectorTest now advance a TestMonotonicTicker instead of stubbing dateProvider.now() with a positional sequence of return values.

📝 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

The 100ms sampling loop in this class still runs on java.util.Timer, whose scheduling is wall-clock. Moving it is a scheduling change and gets its own PR.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Sep 11, 2026

Copy link
Copy Markdown

JAVA-579

@sentry

sentry Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

📲 Install Builds

Android

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

⚙️ sentry-android Build Distribution Settings

runningcode and others added 3 commits November 6, 2026 14:31
…icker (JAVA-579)

DefaultCompositePerformanceCollector decided a transaction had been
collecting for 30s by subtracting two options.getDateProvider()
readings. Those are wall-clock on every platform, including Android:
SentryNanotimeDate.nanoTimestamp() returns millisToNanos(unixDateMillis),
not its nanoTime component, so the extra precision that type exists for
never entered this comparison. A device time change therefore ended
collection early or kept it running past the budget.

Each CompositeData now holds a Deadline on a MonotonicTicker. The budget
is not a serialized value, so this only changes when the collector stops.

Two side effects worth review: the 30s boundary is now inclusive, where
the old strict `>` let a sample land exactly at 30s; and
addDataAndCheckTimeout no longer takes the shared clock reading, since
each transaction owns its own deadline. TestMonotonicTicker's field is
now volatile, as the timer thread reads a ticker the test thread
advances.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drop the package-private constructor overload that took a
MonotonicTicker and read options.getMonotonicTicker() instead. The test
overrides getMonotonicTicker() on a SentryOptions subclass, as
SentryOptionsTest already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@runningcode
runningcode force-pushed the no/java-579-c6-collection-budget branch from e46de12 to 88f31b6 Compare November 6, 2026 13:33
@runningcode
runningcode changed the base branch from main to 9.x.x November 6, 2026 13:33
@runningcode runningcode added the next-major This should only be merged in the next major version. label Nov 6, 2026
@runningcode
runningcode marked this pull request as ready for review November 6, 2026 14:16
@runningcode
runningcode requested review from a team and adinauer as code owners November 6, 2026 14:16
@runningcode
runningcode requested review from 0xadam-brown and markushi and removed request for a team November 6, 2026 14:16

This branch has not been deployed

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

Labels

next-major This should only be merged in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant