Skip to content

Commit bcc2bb9

Browse files
runningcodeclaude
andauthored
fix(core): Order breadcrumbs by their own timestamp (JAVA-579) (#6097)
* fix(core): Order breadcrumbs by their own timestamp (JAVA-579) Breadcrumb.compareTo ordered purely by a System.nanoTime() reading taken in the constructor. A breadcrumb rebuilt from a serialized one — read back from disk, or handed over by a hybrid SDK — got that reading at parse time, so a breadcrumb recorded yesterday sorted as if it had just happened, and the merged order in CombinedScopeView became parse order. The clone constructor had the same problem: copying a breadcrumb moved it to the end of the order. Order by the recorded timestamp instead, and keep the creation tick only as the tie-breaker it was added for in #3355, since timestamps are millisecond-granular. A deserialized breadcrumb carries no tick, and a clone carries the original's, so neither jumps position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * changelog * ref(core): Keep the creation tick on deserialized breadcrumbs Always fill the tick and let the timestamp comparison carry the fix, so ordering no longer depends on every caller using a stable sort. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ref(core): Sort breadcrumbs by their natural ordering in SentryClient SortBreadcrumbsByDate compared timestamps only, so ties fell through to the sort's stability. Breadcrumb.compareTo now defines that same order with a defined tie-breaker, leaving the comparator a weaker duplicate of it and the codebase with two definitions of breadcrumb order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 56fb8ca commit bcc2bb9

4 files changed

Lines changed: 67 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
### Fixes
3939

4040
- Keep resolving the server name after `Sentry.close()` or a re-init. Closing the SDK shut down the shared hostname cache for the life of the process, so `server_name` silently froze at the value it had last resolved ([#6119](https://github.com/getsentry/sentry-java/pull/6119))
41+
- 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))
4142

4243
### Internal
4344

‎sentry/src/main/java/io/sentry/Breadcrumb.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ public Breadcrumb(final long timestamp) {
7373
}
7474

7575
Breadcrumb(final @NotNull Breadcrumb breadcrumb) {
76-
this.nanos = System.nanoTime();
76+
// A clone stands in for the breadcrumb it was copied from, so it inherits its tie-breaker
77+
// instead of taking a fresh one and sorting after everything recorded since.
78+
this.nanos = breadcrumb.nanos;
7779
this.timestamp = breadcrumb.timestamp;
7880
this.timestampMs = breadcrumb.timestampMs;
7981
this.message = breadcrumb.message;
@@ -832,6 +834,12 @@ public void setUnknown(@Nullable Map<String, Object> unknown) {
832834
@Override
833835
@SuppressWarnings("JavaUtilDate")
834836
public int compareTo(@NotNull Breadcrumb o) {
837+
final int byTimestamp = getTimestamp().compareTo(o.getTimestamp());
838+
if (byTimestamp != 0) {
839+
return byTimestamp;
840+
}
841+
// Timestamps are millisecond-granular, so breadcrumbs recorded in the same millisecond tie.
842+
// nanos is only meaningful within a process run, which is all a tie-breaker has to cover.
835843
return nanos.compareTo(o.nanos);
836844
}
837845

‎sentry/src/main/java/io/sentry/SentryClient.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.ArrayList;
2727
import java.util.Collection;
2828
import java.util.Collections;
29-
import java.util.Comparator;
3029
import java.util.List;
3130
import java.util.Map;
3231
import org.jetbrains.annotations.ApiStatus;
@@ -41,7 +40,6 @@ public final class SentryClient implements ISentryClient {
4140

4241
private final @NotNull SentryOptions options;
4342
private final @NotNull ITransport transport;
44-
private final @NotNull SortBreadcrumbsByDate sortBreadcrumbsByDate = new SortBreadcrumbsByDate();
4543
private final @NotNull ILoggerBatchProcessor loggerBatchProcessor;
4644
private final @NotNull IMetricsBatchProcessor metricsBatchProcessor;
4745

@@ -1660,7 +1658,7 @@ private void sortBreadcrumbsByDate(
16601658

16611659
if (sortedBreadcrumbs != null && !breadcrumbs.isEmpty()) {
16621660
sortedBreadcrumbs.addAll(breadcrumbs);
1663-
Collections.sort(sortedBreadcrumbs, sortBreadcrumbsByDate);
1661+
Collections.sort(sortedBreadcrumbs);
16641662
}
16651663
}
16661664

@@ -1851,13 +1849,4 @@ private boolean sample() {
18511849
}
18521850
return true;
18531851
}
1854-
1855-
private static final class SortBreadcrumbsByDate implements Comparator<Breadcrumb> {
1856-
1857-
@SuppressWarnings({"JdkObsolete", "JavaUtilDate"})
1858-
@Override
1859-
public int compare(final @NotNull Breadcrumb b1, final @NotNull Breadcrumb b2) {
1860-
return b1.getTimestamp().compareTo(b2.getTimestamp());
1861-
}
1862-
}
18631852
}

‎sentry/src/test/java/io/sentry/BreadcrumbTest.kt‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.sentry
22

3+
import com.google.common.truth.Truth.assertThat
4+
import java.io.StringReader
35
import java.util.Date
46
import java.util.concurrent.CountDownLatch
57
import java.util.concurrent.Executors
@@ -365,6 +367,60 @@ class BreadcrumbTest {
365367
}
366368
}
367369

370+
@Test
371+
fun `breadcrumbs sharing a timestamp keep the order they were recorded in`() {
372+
val timestamp = Date(1_600_000_000_000)
373+
val first = Breadcrumb(timestamp).apply { message = "first" }
374+
val second = Breadcrumb(timestamp).apply { message = "second" }
375+
val third = Breadcrumb(timestamp).apply { message = "third" }
376+
377+
val sorted = listOf(third, first, second).sorted().map { it.message }
378+
379+
assertThat(sorted).containsExactly("first", "second", "third").inOrder()
380+
}
381+
382+
@Test
383+
fun `a deserialized breadcrumb is ordered by its own timestamp, not by when it was parsed`() {
384+
val live = Breadcrumb(Date(1_600_000_000_000)).apply { message = "live" }
385+
val restored =
386+
Breadcrumb.fromMap(
387+
mapOf(
388+
Breadcrumb.JsonKeys.TIMESTAMP to DateUtils.getTimestamp(Date(1_500_000_000_000)),
389+
Breadcrumb.JsonKeys.MESSAGE to "restored",
390+
),
391+
SentryOptions(),
392+
)
393+
394+
val sorted = listOf(live, restored).sorted().map { it.message }
395+
396+
assertThat(sorted).containsExactly("restored", "live").inOrder()
397+
}
398+
399+
@Test
400+
fun `a breadcrumb read back from JSON is ordered by its own timestamp, not by when it was parsed`() {
401+
val live = Breadcrumb(Date(1_600_000_000_000)).apply { message = "live" }
402+
val json =
403+
"""{"timestamp":"${DateUtils.getTimestamp(Date(1_500_000_000_000))}","message":"restored"}"""
404+
val restored =
405+
Breadcrumb.Deserializer()
406+
.deserialize(JsonObjectReader(StringReader(json)), NoOpLogger.getInstance())
407+
408+
val sorted = listOf(live, restored).sorted().map { it.message }
409+
410+
assertThat(sorted).containsExactly("restored", "live").inOrder()
411+
}
412+
413+
@Test
414+
fun `cloning a breadcrumb keeps its position among breadcrumbs sharing its timestamp`() {
415+
val timestamp = Date(1_600_000_000_000)
416+
val first = Breadcrumb(timestamp).apply { message = "first" }
417+
val second = Breadcrumb(timestamp).apply { message = "second" }
418+
419+
val sorted = listOf(second, Breadcrumb(first)).sorted().map { it.message }
420+
421+
assertThat(sorted).containsExactly("first", "second").inOrder()
422+
}
423+
368424
class TestKey(val id: Long) {
369425
override fun toString(): String = id.toString()
370426
}

0 commit comments

Comments
 (0)