Lazy uuid generation for SentryId and SpanId (#3770)

* lazy uuid generation for SentryId and SpanId * add changelog * add tests for lazy init, rework SentryId to cache string result * fix changelog * Update sentry/src/main/java/io/sentry/protocol/SentryId.java Co-authored-by: Stefano <stefano.siano@sentry.io> * Update sentry/src/main/java/io/sentry/protocol/SentryId.java Co-authored-by: Stefano <stefano.siano@sentry.io> * Update sentry/src/main/java/io/sentry/protocol/SentryId.java Co-authored-by: Stefano <stefano.siano@sentry.io> * Update sentry/src/main/java/io/sentry/protocol/SentryId.java Co-authored-by: Stefano <stefano.siano@sentry.io> * Update sentry/src/main/java/io/sentry/protocol/SentryId.java Co-authored-by: Stefano <stefano.siano@sentry.io> * Format code * Add object comparison to SpanFrameMetricsCollector only check for time difference of .equals if objects are not the same --------- Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com> Co-authored-by: Stefano <stefano.siano@sentry.io> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
getsentry · Oct 29, 2024 · d2c4d5e · d2c4d5e
1 parent 57b2da5
commit d2c4d5e
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 48 deletions.
diff --git 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
@@ -45,17 +45,19 @@ public class SpanFrameMetricsCollector
   private final @NotNull SortedSet<ISpan> 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
@@ -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
@@ -1,52 +1,57 @@
 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<String> 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
   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
 
   @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
@@ -5,69 +5,62 @@
 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;
 import org.jetbrains.annotations.NotNull;
 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<String> 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
   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
@@ -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
@@ -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> { 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> { 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`<Any> { 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
@@ -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> { 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> { 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`<Any> { 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))
+        }
+    }
+}