From 22029c16186877a937540f9f05715392220acbff Mon Sep 17 00:00:00 2001 From: Lukas Bloder Date: Mon, 7 Oct 2024 10:52:12 +0200 Subject: [PATCH 1/4] lazy uuid generation for SentryId and SpanId --- sentry/api/sentry.api | 1 + sentry/src/main/java/io/sentry/SpanId.java | 31 ++++++++++-------- .../java/io/sentry/protocol/SentryId.java | 32 +++++++++++-------- .../main/java/io/sentry/util/StringUtils.java | 2 +- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e7ec93fd20..fabda50323 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -6374,6 +6374,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..ad2bc45ef6 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 lazyValue; + public SentryId() { this((UUID) null); } public SentryId(@Nullable UUID uuid) { - if (uuid == null) { - uuid = UUID.randomUUID(); + if (uuid != null) { + this.lazyValue = new LazyEvaluator<>(() -> uuid); + } else { + this.lazyValue = new LazyEvaluator<>(UUID::randomUUID); } - this.uuid = uuid; } public SentryId(final @NotNull String sentryIdString) { - this.uuid = fromStringSentryId(StringUtils.normalizeUUID(sentryIdString)); + if (sentryIdString.length() != 32 && sentryIdString.length() != 36) { + throw new IllegalArgumentException( + "String representation of SentryId has either 32 (UUID no dashes) " + + "or 36 characters long (completed UUID). Received: " + + sentryIdString); + } + this.lazyValue = + new LazyEvaluator<>(() -> fromStringSentryId(StringUtils.normalizeUUID(sentryIdString))); } @Override public String toString() { - return StringUtils.normalizeUUID(uuid.toString()).replace("-", ""); + return StringUtils.normalizeUUID(lazyValue.getValue().toString()).replace("-", ""); } @Override @@ -41,12 +51,12 @@ 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 lazyValue.getValue().compareTo(sentryId.lazyValue.getValue()) == 0; } @Override public int hashCode() { - return uuid.hashCode(); + return lazyValue.getValue().hashCode(); } private @NotNull UUID fromStringSentryId(@NotNull String sentryIdString) { @@ -60,12 +70,6 @@ public int hashCode() { .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); } 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() {} From f947f824a43c43fbeaa24fe8d78797ff92b7362d Mon Sep 17 00:00:00 2001 From: Lukas Bloder Date: Mon, 7 Oct 2024 10:59:17 +0200 Subject: [PATCH 2/4] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77de2d5898..14cfe20471 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Use `` to ensure Sentry Android auto init is not easily overwritten - Attach request body for `application/x-www-form-urlencoded` requests in Spring ([#3731](https://github.com/getsentry/sentry-java/pull/3731)) - Previously request body was only attached for `application/json` requests +- Lazy uuid generation for SentryId and SpanId ([#3770](https://github.com/getsentry/sentry-java/pull/3770)) ### Fixes From e6df0ad59ec3c9cb84fa912ac8021ce1af3f22ec Mon Sep 17 00:00:00 2001 From: Lukas Bloder Date: Fri, 11 Oct 2024 13:07:32 +0200 Subject: [PATCH 3/4] add tests for lazy init, rework SentryId to cache string result --- .../java/io/sentry/protocol/SentryId.java | 35 +++++-------- .../java/io/sentry/protocol/SentryIdTest.kt | 43 ++++++++++++++++ .../java/io/sentry/protocol/SpanIdTest.kt | 51 +++++++++++++++++++ 3 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 sentry/src/test/java/io/sentry/protocol/SpanIdTest.kt diff --git a/sentry/src/main/java/io/sentry/protocol/SentryId.java b/sentry/src/main/java/io/sentry/protocol/SentryId.java index ad2bc45ef6..d60f6f3dc8 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryId.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryId.java @@ -16,7 +16,7 @@ public final class SentryId implements JsonSerializable { public static final SentryId EMPTY_ID = new SentryId(new UUID(0, 0)); - private final @NotNull LazyEvaluator lazyValue; + private final @NotNull LazyEvaluator lazyStringValue; public SentryId() { this((UUID) null); @@ -24,26 +24,26 @@ public SentryId() { public SentryId(@Nullable UUID uuid) { if (uuid != null) { - this.lazyValue = new LazyEvaluator<>(() -> uuid); + this.lazyStringValue = new LazyEvaluator<>(() -> uuidToSentryIdString(uuid)); } else { - this.lazyValue = new LazyEvaluator<>(UUID::randomUUID); + this.lazyStringValue = new LazyEvaluator<>(() -> uuidToSentryIdString(UUID.randomUUID())); } } public SentryId(final @NotNull String sentryIdString) { - if (sentryIdString.length() != 32 && sentryIdString.length() != 36) { + 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.lazyValue = - new LazyEvaluator<>(() -> fromStringSentryId(StringUtils.normalizeUUID(sentryIdString))); + this.lazyStringValue = new LazyEvaluator<>(() -> uuidStringToSentryIdString(normalized)); } @Override public String toString() { - return StringUtils.normalizeUUID(lazyValue.getValue().toString()).replace("-", ""); + return lazyStringValue.getValue(); } @Override @@ -51,27 +51,20 @@ 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 lazyValue.getValue().compareTo(sentryId.lazyValue.getValue()) == 0; + return lazyStringValue.getValue().compareTo(sentryId.lazyStringValue.getValue()) == 0; } @Override public int hashCode() { - return lazyValue.getValue().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(); - } + private @NotNull String uuidToSentryIdString(@NotNull UUID uuid) { + return uuidStringToSentryIdString(uuid.toString()); + } - return UUID.fromString(sentryIdString); + private @NotNull String uuidStringToSentryIdString(@NotNull String uuidString) { + return StringUtils.normalizeUUID(uuidString).replace("-", ""); } // JsonSerializable 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)) + } + } +} From f2fba851635cf2a70e1c2380e58a0d17bd6e7b3f Mon Sep 17 00:00:00 2001 From: Lukas Bloder Date: Fri, 18 Oct 2024 09:26:53 +0200 Subject: [PATCH 4/4] fix changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b106f876a2..b029c20a08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +## Features + +- Lazy uuid generation for SentryId and SpanId ([#3770](https://github.com/getsentry/sentry-java/pull/3770)) + ## 8.0.0-beta.1 ### Breaking Changes @@ -45,7 +51,6 @@ - It is now also possible to provide a bean of type `SentryGraphqlInstrumentation.BeforeSpanCallback` which is then used by `SentryInstrumenter` - Emit transaction.data inside contexts.trace.data ([#3735](https://github.com/getsentry/sentry-java/pull/3735)) - Also does not emit `transaction.data` in `exras` anymore -- Lazy uuid generation for SentryId and SpanId ([#3770](https://github.com/getsentry/sentry-java/pull/3770)) ### Fixes