diff --git a/CHANGELOG.md b/CHANGELOG.md index 15daf073d2..d087e23cff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Add `globalHubMode` to options ([#3805](https://github.com/getsentry/sentry-java/pull/3805)) - `globalHubMode` used to only be a param on `Sentry.init`. To make it easier to be used in e.g. Desktop environments, we now additionally added it as an option on SentryOptions that can also be set via `sentry.properties`. - If both the param on `Sentry.init` and the option are set, the option will win. By default the option is set to `null` meaning whatever is passed to `Sentry.init` takes effect. +- Lazy uuid generation for SentryId and SpanId ([#3770](https://github.com/getsentry/sentry-java/pull/3770)) ### Behavioural Changes diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java index 18427dd589..d4e47ddc80 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SpanFrameMetricsCollector.java @@ -45,17 +45,19 @@ public class SpanFrameMetricsCollector private final @NotNull SortedSet runningSpans = new TreeSet<>( (o1, o2) -> { + if (o1 == o2) { + return 0; + } int timeDiff = o1.getStartDate().compareTo(o2.getStartDate()); if (timeDiff != 0) { return timeDiff; - } else { - // TreeSet uses compareTo to check for duplicates, so ensure that - // two non-equal spans with the same start date are not considered equal - return o1.getSpanContext() - .getSpanId() - .toString() - .compareTo(o2.getSpanContext().getSpanId().toString()); } + // TreeSet uses compareTo to check for duplicates, so ensure that + // two non-equal spans with the same start date are not considered equal + return o1.getSpanContext() + .getSpanId() + .toString() + .compareTo(o2.getSpanContext().getSpanId().toString()); }); // all collected frames, sorted by frame end time diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index fbd8d91d0d..293ea6cb8c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -6151,6 +6151,7 @@ public final class io/sentry/util/SpanUtils { } public final class io/sentry/util/StringUtils { + public static final field PROPER_NIL_UUID Ljava/lang/String; public static fun byteCountToString (J)Ljava/lang/String; public static fun calculateStringHash (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/String; public static fun camelCase (Ljava/lang/String;)Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/SpanId.java b/sentry/src/main/java/io/sentry/SpanId.java index 70608fb7cb..7857919e02 100644 --- a/sentry/src/main/java/io/sentry/SpanId.java +++ b/sentry/src/main/java/io/sentry/SpanId.java @@ -1,26 +1,31 @@ package io.sentry; -import io.sentry.util.Objects; +import static io.sentry.util.StringUtils.PROPER_NIL_UUID; + +import io.sentry.util.LazyEvaluator; import io.sentry.util.StringUtils; import java.io.IOException; +import java.util.Objects; import java.util.UUID; import org.jetbrains.annotations.NotNull; public final class SpanId implements JsonSerializable { - public static final SpanId EMPTY_ID = new SpanId(new UUID(0, 0)); + public static final SpanId EMPTY_ID = new SpanId(PROPER_NIL_UUID); - private final @NotNull String value; + private final @NotNull LazyEvaluator lazyValue; public SpanId(final @NotNull String value) { - this.value = Objects.requireNonNull(value, "value is required"); + Objects.requireNonNull(value, "value is required"); + this.lazyValue = new LazyEvaluator<>(() -> value); } public SpanId() { - this(UUID.randomUUID()); - } - - private SpanId(final @NotNull UUID uuid) { - this(StringUtils.normalizeUUID(uuid.toString()).replace("-", "").substring(0, 16)); + this.lazyValue = + new LazyEvaluator<>( + () -> + StringUtils.normalizeUUID(UUID.randomUUID().toString()) + .replace("-", "") + .substring(0, 16)); } @Override @@ -28,17 +33,17 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; SpanId spanId = (SpanId) o; - return value.equals(spanId.value); + return lazyValue.getValue().equals(spanId.lazyValue.getValue()); } @Override public int hashCode() { - return value.hashCode(); + return lazyValue.getValue().hashCode(); } @Override public String toString() { - return this.value; + return lazyValue.getValue(); } // JsonElementSerializer @@ -46,7 +51,7 @@ public String toString() { @Override public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) throws IOException { - writer.value(value); + writer.value(lazyValue.getValue()); } // JsonElementDeserializer diff --git a/sentry/src/main/java/io/sentry/protocol/SentryId.java b/sentry/src/main/java/io/sentry/protocol/SentryId.java index 109655fdf2..bf1bde0c4d 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryId.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryId.java @@ -5,6 +5,7 @@ import io.sentry.JsonSerializable; import io.sentry.ObjectReader; import io.sentry.ObjectWriter; +import io.sentry.util.LazyEvaluator; import io.sentry.util.StringUtils; import java.io.IOException; import java.util.UUID; @@ -12,28 +13,37 @@ import org.jetbrains.annotations.Nullable; public final class SentryId implements JsonSerializable { - private final @NotNull UUID uuid; public static final SentryId EMPTY_ID = new SentryId(new UUID(0, 0)); + private final @NotNull LazyEvaluator lazyStringValue; + public SentryId() { this((UUID) null); } public SentryId(@Nullable UUID uuid) { - if (uuid == null) { - uuid = UUID.randomUUID(); + if (uuid != null) { + this.lazyStringValue = new LazyEvaluator<>(() -> normalize(uuid.toString())); + } else { + this.lazyStringValue = new LazyEvaluator<>(() -> normalize(UUID.randomUUID().toString())); } - this.uuid = uuid; } public SentryId(final @NotNull String sentryIdString) { - this.uuid = fromStringSentryId(StringUtils.normalizeUUID(sentryIdString)); + final @NotNull String normalized = StringUtils.normalizeUUID(sentryIdString); + if (normalized.length() != 32 && normalized.length() != 36) { + throw new IllegalArgumentException( + "String representation of SentryId has either 32 (UUID no dashes) " + + "or 36 characters long (completed UUID). Received: " + + sentryIdString); + } + this.lazyStringValue = new LazyEvaluator<>(() -> normalize(normalized)); } @Override public String toString() { - return StringUtils.normalizeUUID(uuid.toString()).replace("-", ""); + return lazyStringValue.getValue(); } @Override @@ -41,33 +51,16 @@ public boolean equals(final @Nullable Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; SentryId sentryId = (SentryId) o; - return uuid.compareTo(sentryId.uuid) == 0; + return lazyStringValue.getValue().equals(sentryId.lazyStringValue.getValue()); } @Override public int hashCode() { - return uuid.hashCode(); + return lazyStringValue.getValue().hashCode(); } - private @NotNull UUID fromStringSentryId(@NotNull String sentryIdString) { - if (sentryIdString.length() == 32) { - // expected format, SentryId is a UUID without dashes - sentryIdString = - new StringBuilder(sentryIdString) - .insert(8, "-") - .insert(13, "-") - .insert(18, "-") - .insert(23, "-") - .toString(); - } - if (sentryIdString.length() != 36) { - throw new IllegalArgumentException( - "String representation of SentryId has either 32 (UUID no dashes) " - + "or 36 characters long (completed UUID). Received: " - + sentryIdString); - } - - return UUID.fromString(sentryIdString); + private @NotNull String normalize(@NotNull String uuidString) { + return StringUtils.normalizeUUID(uuidString).replace("-", ""); } // JsonSerializable diff --git a/sentry/src/main/java/io/sentry/util/StringUtils.java b/sentry/src/main/java/io/sentry/util/StringUtils.java index e4bf618501..249940a638 100644 --- a/sentry/src/main/java/io/sentry/util/StringUtils.java +++ b/sentry/src/main/java/io/sentry/util/StringUtils.java @@ -20,8 +20,8 @@ public final class StringUtils { private static final Charset UTF_8 = Charset.forName("UTF-8"); + public static final String PROPER_NIL_UUID = "00000000-0000-0000-0000-000000000000"; private static final String CORRUPTED_NIL_UUID = "0000-0000"; - private static final String PROPER_NIL_UUID = "00000000-0000-0000-0000-000000000000"; private static final @NotNull Pattern PATTERN_WORD_SNAKE_CASE = Pattern.compile("[\\W_]+"); private StringUtils() {} diff --git a/sentry/src/test/java/io/sentry/protocol/SentryIdTest.kt b/sentry/src/test/java/io/sentry/protocol/SentryIdTest.kt index 2aa24ec859..f26f591156 100644 --- a/sentry/src/test/java/io/sentry/protocol/SentryIdTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/SentryIdTest.kt @@ -1,5 +1,11 @@ package io.sentry.protocol +import io.sentry.util.StringUtils +import org.mockito.Mockito +import org.mockito.kotlin.any +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import java.util.* import kotlin.test.Test import kotlin.test.assertEquals @@ -10,4 +16,41 @@ class SentryIdTest { val id = SentryId("0000-0000") assertEquals("00000000000000000000000000000000", id.toString()) } + + @Test + fun `UUID is not generated on initialization`() { + val uuid = UUID.randomUUID() + Mockito.mockStatic(UUID::class.java).use { utils -> + utils.`when` { UUID.randomUUID() }.thenReturn(uuid) + val ignored = SentryId() + utils.verify({ UUID.randomUUID() }, never()) + } + } + + @Test + fun `UUID is generated only once`() { + val uuid = UUID.randomUUID() + Mockito.mockStatic(UUID::class.java).use { utils -> + utils.`when` { UUID.randomUUID() }.thenReturn(uuid) + val sentryId = SentryId() + val uuid1 = sentryId.toString() + val uuid2 = sentryId.toString() + + assertEquals(uuid1, uuid2) + utils.verify({ UUID.randomUUID() }, times(1)) + } + } + + @Test + fun `normalizeUUID is only called once`() { + Mockito.mockStatic(StringUtils::class.java).use { utils -> + utils.`when` { StringUtils.normalizeUUID(any()) }.thenReturn("00000000000000000000000000000000") + val sentryId = SentryId() + val uuid1 = sentryId.toString() + val uuid2 = sentryId.toString() + + assertEquals(uuid1, uuid2) + utils.verify({ StringUtils.normalizeUUID(any()) }, times(1)) + } + } } diff --git a/sentry/src/test/java/io/sentry/protocol/SpanIdTest.kt b/sentry/src/test/java/io/sentry/protocol/SpanIdTest.kt new file mode 100644 index 0000000000..071d402e2f --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/SpanIdTest.kt @@ -0,0 +1,51 @@ +package io.sentry.protocol + +import io.sentry.SpanId +import io.sentry.util.StringUtils +import org.mockito.Mockito +import org.mockito.kotlin.any +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import java.util.* +import kotlin.test.Test +import kotlin.test.assertEquals + +class SpanIdTest { + + @Test + fun `UUID is not generated on initialization`() { + val uuid = UUID.randomUUID() + Mockito.mockStatic(UUID::class.java).use { utils -> + utils.`when` { UUID.randomUUID() }.thenReturn(uuid) + val ignored = SpanId() + utils.verify({ UUID.randomUUID() }, never()) + } + } + + @Test + fun `UUID is generated only once`() { + val uuid = UUID.randomUUID() + Mockito.mockStatic(java.util.UUID::class.java).use { utils -> + utils.`when` { UUID.randomUUID() }.thenReturn(uuid) + val spanId = SpanId() + val uuid1 = spanId.toString() + val uuid2 = spanId.toString() + + assertEquals(uuid1, uuid2) + utils.verify({ UUID.randomUUID() }, times(1)) + } + } + + @Test + fun `normalizeUUID is only called once`() { + Mockito.mockStatic(StringUtils::class.java).use { utils -> + utils.`when` { StringUtils.normalizeUUID(any()) }.thenReturn("00000000000000000000000000000000") + val spanId = SpanId() + val uuid1 = spanId.toString() + val uuid2 = spanId.toString() + + assertEquals(uuid1, uuid2) + utils.verify({ StringUtils.normalizeUUID(any()) }, times(1)) + } + } +}