diff --git a/CHANGELOG.md b/CHANGELOG.md index 09622493a2..eeea71a8a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Order breadcrumbs by the timestamp they carry rather than by when they were created in the current process, so breadcrumbs restored from disk or handed over by a hybrid SDK no longer sort as if they had just happened ([#6097](https://github.com/getsentry/sentry-java/pull/6097)) + ## 8.56.0 ### Behavioral Changes diff --git a/sentry/src/main/java/io/sentry/Breadcrumb.java b/sentry/src/main/java/io/sentry/Breadcrumb.java index fff6954ee5..9a91b1630d 100644 --- a/sentry/src/main/java/io/sentry/Breadcrumb.java +++ b/sentry/src/main/java/io/sentry/Breadcrumb.java @@ -26,7 +26,14 @@ public final class Breadcrumb implements JsonUnknown, JsonSerializable, Comparab /** A timestamp representing when the breadcrumb occurred as java.util.Date. */ private @Nullable Date timestamp; - private final @NotNull Long nanos; + /** + * The tick this breadcrumb was created at, used to order breadcrumbs that share a timestamp. + * + *

Null for a breadcrumb rebuilt from a serialized one. A tick is a reading of a counter whose + * origin is this process run, so a tick from an earlier run is a number from an unrelated origin + * rather than a position in this run's order. + */ + private final @Nullable Long creationTick; /** If a message is provided, its rendered as text and the whitespace is preserved. */ private @Nullable String message; @@ -59,21 +66,33 @@ public final class Breadcrumb implements JsonUnknown, JsonSerializable, Comparab * * @param timestamp the timestamp */ - @SuppressWarnings("JavaUtilDate") public Breadcrumb(final @NotNull Date timestamp) { - this.nanos = System.nanoTime(); + this(timestamp, System.nanoTime()); + } + + @SuppressWarnings("JavaUtilDate") + private Breadcrumb(final @NotNull Date timestamp, final @Nullable Long creationTick) { + this.creationTick = creationTick; this.timestamp = timestamp; this.timestampMs = null; } + /** + * A breadcrumb rebuilt from a serialized one — read back from disk, or handed over by a hybrid + * SDK. It carries the timestamp it was serialized with and no creation tick. + */ + static @NotNull Breadcrumb deserialized(final @NotNull Date timestamp) { + return new Breadcrumb(timestamp, null); + } + public Breadcrumb(final long timestamp) { - this.nanos = System.nanoTime(); + this.creationTick = System.nanoTime(); this.timestampMs = timestamp; this.timestamp = null; } Breadcrumb(final @NotNull Breadcrumb breadcrumb) { - this.nanos = System.nanoTime(); + this.creationTick = breadcrumb.creationTick; this.timestamp = breadcrumb.timestamp; this.timestampMs = breadcrumb.timestampMs; this.message = breadcrumb.message; @@ -170,7 +189,7 @@ public static Breadcrumb fromMap( } } - final Breadcrumb breadcrumb = new Breadcrumb(timestamp); + final Breadcrumb breadcrumb = Breadcrumb.deserialized(timestamp); breadcrumb.message = message; breadcrumb.type = type; if (data != null) { @@ -831,8 +850,21 @@ public void setUnknown(@Nullable Map unknown) { @Override @SuppressWarnings("JavaUtilDate") - public int compareTo(@NotNull Breadcrumb o) { - return nanos.compareTo(o.nanos); + public int compareTo(final @NotNull Breadcrumb o) { + final int byTimestamp = getTimestamp().compareTo(o.getTimestamp()); + if (byTimestamp != 0) { + return byTimestamp; + } + // Timestamps are millisecond-granular, so breadcrumbs recorded in the same millisecond tie. + // Creation ticks break the tie in the order they were actually recorded. + if (creationTick == null) { + // Deserialized, so from an earlier process run than anything holding a tick. + return o.creationTick == null ? 0 : -1; + } + if (o.creationTick == null) { + return 1; + } + return creationTick.compareTo(o.creationTick); } public static final class JsonKeys { @@ -941,7 +973,7 @@ public static final class Deserializer implements JsonDeserializer { } } - Breadcrumb breadcrumb = new Breadcrumb(timestamp); + Breadcrumb breadcrumb = Breadcrumb.deserialized(timestamp); breadcrumb.message = message; breadcrumb.type = type; if (data != null) { diff --git a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt index f51acca81c..14742fecb8 100644 --- a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt +++ b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt @@ -1,5 +1,6 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import java.util.Date import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors @@ -365,6 +366,46 @@ class BreadcrumbTest { } } + @Test + fun `breadcrumbs sharing a timestamp keep the order they were recorded in`() { + val timestamp = Date(1_600_000_000_000) + val first = Breadcrumb(timestamp).apply { message = "first" } + val second = Breadcrumb(timestamp).apply { message = "second" } + val third = Breadcrumb(timestamp).apply { message = "third" } + + val sorted = listOf(third, first, second).sorted().map { it.message } + + assertThat(sorted).containsExactly("first", "second", "third").inOrder() + } + + @Test + fun `a deserialized breadcrumb is ordered by its own timestamp, not by when it was parsed`() { + val live = Breadcrumb(Date(1_600_000_000_000)).apply { message = "live" } + val restored = + Breadcrumb.fromMap( + mapOf( + Breadcrumb.JsonKeys.TIMESTAMP to DateUtils.getTimestamp(Date(1_500_000_000_000)), + Breadcrumb.JsonKeys.MESSAGE to "restored", + ), + SentryOptions(), + ) + + val sorted = listOf(live, restored).sorted().map { it.message } + + assertThat(sorted).containsExactly("restored", "live").inOrder() + } + + @Test + fun `cloning a breadcrumb keeps its position among breadcrumbs sharing its timestamp`() { + val timestamp = Date(1_600_000_000_000) + val first = Breadcrumb(timestamp).apply { message = "first" } + val second = Breadcrumb(timestamp).apply { message = "second" } + + val sorted = listOf(second, Breadcrumb(first)).sorted().map { it.message } + + assertThat(sorted).containsExactly("first", "second").inOrder() + } + class TestKey(val id: Long) { override fun toString(): String = id.toString() }