From 864c8cb28d8e034476fae707896e00bb07ed3a0e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 10 Sep 2026 10:24:52 +0200 Subject: [PATCH] docs(time): Steer new measurements at the io.sentry.time clock APIs Document on each singleton clock's getInstance() that callers should read the ticker or wall clock off the options object instead, so they get the per-platform implementation rather than pinning themselves to one. Calling JavaMonotonicTicker.getInstance() on Android is the trap worth naming: it stops counting through a device suspend, so intervals spanning a deep sleep come out short. Add an AGENTS.md section so new measurement code reaches for io.sentry.time rather than DateUtils.getCurrentDateTime() or a SentryDateProvider, picks the abstraction by what the number is for, and treats adding a new abstraction as a deliberate step rather than a way around an awkward call site. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 37 +++++++++++++++++++ .../internal/time/AndroidMonotonicTicker.java | 8 ++++ .../io/sentry/time/JavaMonotonicTicker.java | 11 ++++++ .../java/io/sentry/time/SystemEpochClock.java | 8 ++++ 4 files changed, 64 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1027e513c4..a56b907093 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -155,6 +155,43 @@ Apply that pattern only where a broad catch is genuinely unavoidable — an entr arbitrary user code or third-party callbacks. Everywhere else, name the exception types. Say in the PR description why the broad catch is necessary. +### Measuring Time + +**All new code that reads a clock uses `io.sentry.time`.** Do not reach for +`DateUtils.getCurrentDateTime()`, `System.currentTimeMillis()`, `System.nanoTime()`, +`SystemClock`, or any `SentryDateProvider` implementation (`SentryAutoDateProvider`, +`SentryNanotimeDateProvider`, `SentryInstantDateProvider`, `SentryAndroidDateProvider`). Those +remain only because the `SentryDate` protocol types still flow through the event pipeline; they +are legacy, not a precedent to follow. + +Pick the abstraction by what the number is *for*, not by what is convenient to call: + +| You need | Use | Notes | +|---|---|---| +| An instant that leaves the process (event, breadcrumb, session) | `EpochClock` → `Timestamp` | Serialize it. Never subtract two of them | +| How long something took | `Stopwatch` | | +| Whether a window has elapsed (TTL, cache expiry, backoff, timeout) | `Deadline` | Also gives you `remaining(unit)` for scheduling | +| Several instants that will be compared with each other (spans of a transaction, samples of a chunk) | `AnchoredClock` | One wall-clock read, the rest projected off ticks, so gaps are real elapsed time | +| A raw monotonic tick, when none of the above fits | `MonotonicTicker` | | + +Two rules on top of that: + +1. **Get the clock from the options object** — `options.getEpochClock()` and + `options.getMonotonicTicker()` — not from `SystemEpochClock.getInstance()`, + `JavaMonotonicTicker.getInstance()`, or `AndroidMonotonicTicker.getInstance()`. The accessor is + what resolves to the correct per-platform implementation: on Android the ticker is + `CLOCK_BOOTTIME`, which keeps counting through a device suspend, while `System.nanoTime()` does + not. Take the clock as a constructor parameter and keep it in a field; `sentry-test-support` + provides `TestMonotonicTicker` so tests advance time instead of sleeping. +2. **If no abstraction fits, stop and think hard before adding one.** Adding a type here is a + deliberate act, not a shortcut around an awkward call site. First re-read the existing types — + most "missing" cases turn out to be a `Deadline` or a `Stopwatch` described in different words. + If one is genuinely missing, work out what invariant it exists to enforce (each of these types + exists to make one class of clock bug unrepresentable — negative durations, unit mix-ups, + wrap-unsafe comparisons, a tick escaping into serialized output), name that invariant in its + Javadoc, and propose it before writing call sites against it. Do not inline raw tick arithmetic + at a call site as a stopgap. + ### Testing Requirements - Write comprehensive unit tests for new features - Android modules require both unit tests and instrumented tests where applicable diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/time/AndroidMonotonicTicker.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/time/AndroidMonotonicTicker.java index 22973b0d93..1b8d0590de 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/time/AndroidMonotonicTicker.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/time/AndroidMonotonicTicker.java @@ -16,6 +16,14 @@ public final class AndroidMonotonicTicker implements MonotonicTicker { private static final AndroidMonotonicTicker instance = new AndroidMonotonicTicker(); + /** + * Prefer {@link io.sentry.SentryOptions#getMonotonicTicker()} over this: on Android it already + * returns this ticker, and it keeps the call site compiling on the JVM too. + * + *

Call this directly only where there is no options object to ask — the {@code + * SentryAndroidOptions} override that supplies it, or a test that means this implementation + * specifically. + */ public static @NotNull MonotonicTicker getInstance() { return instance; } diff --git a/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java b/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java index b5b000f280..95c044799f 100644 --- a/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java +++ b/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java @@ -9,6 +9,17 @@ public final class JavaMonotonicTicker implements MonotonicTicker { private static final JavaMonotonicTicker instance = new JavaMonotonicTicker(); + /** + * Prefer {@link io.sentry.SentryOptions#getMonotonicTicker()} over this: it resolves to the + * ticker that is right for the platform the SDK is running on. + * + *

Android overrides that accessor with a {@code CLOCK_BOOTTIME}-backed ticker. This one stops + * counting while the device is suspended, so measuring against it there silently under-reports + * every interval that spans a deep sleep. + * + *

Call this directly only where there is no options object to ask — the default that {@code + * SentryOptions} itself returns, or a test that means this implementation specifically. + */ public static @NotNull MonotonicTicker getInstance() { return instance; } diff --git a/sentry/src/main/java/io/sentry/time/SystemEpochClock.java b/sentry/src/main/java/io/sentry/time/SystemEpochClock.java index 2cb037c0e0..bd6b52e85c 100644 --- a/sentry/src/main/java/io/sentry/time/SystemEpochClock.java +++ b/sentry/src/main/java/io/sentry/time/SystemEpochClock.java @@ -24,6 +24,14 @@ public final class SystemEpochClock implements EpochClock { private static final SystemEpochClock instance = new SystemEpochClock(); + /** + * Prefer {@link io.sentry.SentryOptions#getEpochClock()} over this: that accessor is where a + * platform-specific wall clock would be substituted, the way {@code SentryAndroidOptions} already + * substitutes the ticker. Reading it keeps a call site from being pinned to this implementation. + * + *

Call this directly only where there is no options object to ask — the default that {@code + * SentryOptions} itself returns, or a test that means this implementation specifically. + */ public static @NotNull EpochClock getInstance() { return instance; }