fix(core): Order breadcrumbs by their own timestamp (JAVA-579)

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

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.
+   *
+   * <p>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<String, Object> 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 breadcrumb = new Breadcrumb(timestamp);
+      Breadcrumb breadcrumb = Breadcrumb.deserialized(timestamp);
       breadcrumb.message = message;
       breadcrumb.type = type;
       if (data != null) {

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()
   }