From 71d977c258eb556c86613f83b8308c3f0c09f348 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 10 Sep 2026 10:24:52 +0200 Subject: [PATCH 1/3] 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 f447fc31512..9745c631992 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -202,6 +202,43 @@ Two more rules that catch most of the rest: Prefer `AutoClosableReentrantLock` over `synchronized`, matching the rest of the codebase. +### 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 22973b0d937..1b8d0590de3 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 b5b000f2805..95c044799f3 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 2cb037c0e09..bd6b52e85c4 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; } From e7b4a4220948a2e27aafa012a7a4f25cfbf0ceb0 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 6 Nov 2026 13:03:59 +0100 Subject: [PATCH 2/3] docs(time): Shorten the Measuring Time section in AGENTS.md Name the legacy clock interfaces instead of listing their implementations, which also covers ICurrentDateProvider. Drop the table's notes column and the history of why the old types remain, and cut the two rules to a sentence each. The getInstance() Javadoc already carries the detail where callers see it. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 54 ++++++++++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9745c631992..eafac499bfd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -204,40 +204,26 @@ Prefer `AutoClosableReentrantLock` over `synchronized`, matching the rest of the ### 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. +**New code that reads a clock uses `io.sentry.time`**, not `System.currentTimeMillis()`, +`System.nanoTime()`, `SystemClock`, `DateUtils.getCurrentDateTime()`, a `SentryDateProvider`, or an +`ICurrentDateProvider`. Those are legacy, not a precedent to follow. + +Pick the type by what the number is for: + +| You need | Use | +|---|---| +| An instant that leaves the process (event, breadcrumb, session) | `EpochClock` → `Timestamp`. Never subtract two of them | +| How long something took | `Stopwatch` | +| Whether a window has elapsed (TTL, backoff, timeout) | `Deadline` | +| Several instants compared with each other (spans of a transaction) | `AnchoredClock` | +| A raw tick, when none of the above fits | `MonotonicTicker` | + +Get the clock from `options.getEpochClock()` or `options.getMonotonicTicker()`, not a +`getInstance()`: on Android the ticker keeps counting through device suspend, and `System.nanoTime()` +does not. Take it as a constructor parameter; tests use `TestMonotonicTicker` instead of sleeping. + +If none of these fits, propose a new type, and the clock bug it makes impossible, before writing +call sites against it. Don't inline raw tick arithmetic as a stopgap. ### Testing Requirements - Write comprehensive unit tests for new features From a5cf7b0721ec82bd2034fe6aa855f259367e28d0 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 6 Nov 2026 13:08:45 +0100 Subject: [PATCH 3/3] docs(time): Shorten the getInstance() Javadoc on the clock singletons Drop the paragraph on when getInstance() may still be called directly. Only the options accessors and test support do, so it restated the code. Keep the one reason that matters: JavaMonotonicTicker stops counting while an Android device is suspended. Co-Authored-By: Claude Opus 5.5 --- .../core/internal/time/AndroidMonotonicTicker.java | 9 +-------- .../java/io/sentry/time/JavaMonotonicTicker.java | 12 +++--------- .../main/java/io/sentry/time/SystemEpochClock.java | 8 ++------ 3 files changed, 6 insertions(+), 23 deletions(-) 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 1b8d0590de3..06deaa5a48a 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,14 +16,7 @@ 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. - */ + /** Prefer {@link io.sentry.SentryOptions#getMonotonicTicker()}; on Android it returns this. */ 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 95c044799f3..fab92fd1b9a 100644 --- a/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java +++ b/sentry/src/main/java/io/sentry/time/JavaMonotonicTicker.java @@ -10,15 +10,9 @@ 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. + * Prefer {@link io.sentry.SentryOptions#getMonotonicTicker()}, which picks the platform's ticker. + * On Android this one stops counting during device suspend, so intervals spanning a deep sleep + * come out short. */ 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 bd6b52e85c4..b11eb18fb72 100644 --- a/sentry/src/main/java/io/sentry/time/SystemEpochClock.java +++ b/sentry/src/main/java/io/sentry/time/SystemEpochClock.java @@ -25,12 +25,8 @@ 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. + * Prefer {@link io.sentry.SentryOptions#getEpochClock()}, which is where a platform would + * substitute its own wall clock, the way Android already substitutes the ticker. */ public static @NotNull EpochClock getInstance() { return instance;