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; }