From 662049148f0390f4430d804d12cc194feb5b028f Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Oct 2024 11:07:43 +0200 Subject: [PATCH 1/3] parsed Dsn object is now cached --- sentry/api/sentry.api | 1 + sentry/src/main/java/io/sentry/Baggage.java | 6 +++--- sentry/src/main/java/io/sentry/DsnUtil.java | 2 +- .../java/io/sentry/RequestDetailsResolver.java | 2 +- sentry/src/main/java/io/sentry/Sentry.java | 4 ++-- .../src/main/java/io/sentry/SentryOptions.java | 17 +++++++++++++++++ .../main/java/io/sentry/util/LazyEvaluator.java | 6 ++++++ 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 89684c7ae5..bdeee6a6af 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -5704,6 +5704,7 @@ public final class io/sentry/util/JsonSerializationUtils { public final class io/sentry/util/LazyEvaluator { public fun (Lio/sentry/util/LazyEvaluator$Evaluator;)V public fun getValue ()Ljava/lang/Object; + public fun resetValue ()V public fun setValue (Ljava/lang/Object;)V } diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index d736358ee1..0a80b154bf 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -134,7 +134,7 @@ public static Baggage fromEvent( final Baggage baggage = new Baggage(options.getLogger()); final SpanContext trace = event.getContexts().getTrace(); baggage.setTraceId(trace != null ? trace.getTraceId().toString() : null); - baggage.setPublicKey(new Dsn(options.getDsn()).getPublicKey()); + baggage.setPublicKey(options.getParsedDsn().getPublicKey()); baggage.setRelease(event.getRelease()); baggage.setEnvironment(event.getEnvironment()); final User user = event.getUser(); @@ -405,7 +405,7 @@ public void setValuesFromTransaction( final @NotNull SentryOptions sentryOptions, final @Nullable TracesSamplingDecision samplingDecision) { setTraceId(transaction.getSpanContext().getTraceId().toString()); - setPublicKey(new Dsn(sentryOptions.getDsn()).getPublicKey()); + setPublicKey(sentryOptions.getParsedDsn().getPublicKey()); setRelease(sentryOptions.getRelease()); setEnvironment(sentryOptions.getEnvironment()); setUserSegment(user != null ? getSegment(user) : null); @@ -427,7 +427,7 @@ public void setValuesFromScope( final @Nullable User user = scope.getUser(); final @NotNull SentryId replayId = scope.getReplayId(); setTraceId(propagationContext.getTraceId().toString()); - setPublicKey(new Dsn(options.getDsn()).getPublicKey()); + setPublicKey(options.getParsedDsn().getPublicKey()); setRelease(options.getRelease()); setEnvironment(options.getEnvironment()); if (!SentryId.EMPTY_ID.equals(replayId)) { diff --git a/sentry/src/main/java/io/sentry/DsnUtil.java b/sentry/src/main/java/io/sentry/DsnUtil.java index 3c48e4a43e..6cc0dc360b 100644 --- a/sentry/src/main/java/io/sentry/DsnUtil.java +++ b/sentry/src/main/java/io/sentry/DsnUtil.java @@ -23,7 +23,7 @@ public static boolean urlContainsDsnHost(@Nullable SentryOptions options, @Nulla return false; } - final @NotNull Dsn dsn = new Dsn(dsnString); + final @NotNull Dsn dsn = options.getParsedDsn(); final @NotNull URI sentryUri = dsn.getSentryUri(); final @Nullable String dsnHost = sentryUri.getHost(); diff --git a/sentry/src/main/java/io/sentry/RequestDetailsResolver.java b/sentry/src/main/java/io/sentry/RequestDetailsResolver.java index b4474893a2..6083c69e99 100644 --- a/sentry/src/main/java/io/sentry/RequestDetailsResolver.java +++ b/sentry/src/main/java/io/sentry/RequestDetailsResolver.java @@ -21,7 +21,7 @@ public RequestDetailsResolver(final @NotNull SentryOptions options) { @NotNull RequestDetails resolve() { - final Dsn dsn = new Dsn(options.getDsn()); + final Dsn dsn = options.getParsedDsn(); final URI sentryUri = dsn.getSentryUri(); final String envelopeUrl = sentryUri.resolve(sentryUri.getPath() + "/envelope/").toString(); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 08571e151a..7c34720d19 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -381,8 +381,8 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) "DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK."); } - @SuppressWarnings("unused") - final Dsn parsedDsn = new Dsn(dsn); + // This creates the DSN object and performs some checks + options.getParsedDsn(); ILogger logger = options.getLogger(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 0eb3bace91..b17e283bc6 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -82,6 +82,9 @@ public class SentryOptions { */ private @Nullable String dsn; + /** Parsed DSN to avoid parsing it every time. */ + private final @NotNull LazyEvaluator parsedDsn = new LazyEvaluator<>(() -> new Dsn(dsn)); + /** dsnHash is used as a subfolder of cacheDirPath to isolate events when rotating DSNs */ private @Nullable String dsnHash; @@ -537,6 +540,17 @@ public void addIntegration(@NotNull Integration integration) { return dsn; } + /** + * Returns the DSN + * + * @return the DSN or null if not set + */ + @ApiStatus.Internal + @NotNull + Dsn getParsedDsn() { + return parsedDsn.getValue(); + } + /** * Sets the DSN * @@ -544,6 +558,9 @@ public void addIntegration(@NotNull Integration integration) { */ public void setDsn(final @Nullable String dsn) { this.dsn = dsn; + if (!isEnabled() || (dsn != null && dsn.isEmpty())) { + this.parsedDsn.resetValue(); + } dsnHash = StringUtils.calculateStringHash(this.dsn, logger); } diff --git a/sentry/src/main/java/io/sentry/util/LazyEvaluator.java b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java index d540cbe508..e731fd3469 100644 --- a/sentry/src/main/java/io/sentry/util/LazyEvaluator.java +++ b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java @@ -48,6 +48,12 @@ public void setValue(final @Nullable T value) { } } + public void resetValue() { + synchronized (this) { + this.value = null; + } + } + public interface Evaluator { @NotNull T evaluate(); From 91dbdcd04633978975935c70efe7fff1e6603958 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Oct 2024 11:10:02 +0200 Subject: [PATCH 2/3] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f3171a03b..3b903ceb68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- Cache parsed Dsn ([#3796](https://github.com/getsentry/sentry-java/pull/3796)) - fix invalid profiles when the transaction name is empty ([#3747](https://github.com/getsentry/sentry-java/pull/3747)) - Deprecate `enableTracing` option ([#3777](https://github.com/getsentry/sentry-java/pull/3777)) - Vendor `java.util.Random` and replace `java.security.SecureRandom` usages ([#3783](https://github.com/getsentry/sentry-java/pull/3783)) From 0c26bbc4a2285bac2b3231d0d37c2579337039b3 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Oct 2024 14:02:46 +0200 Subject: [PATCH 3/3] updated LazyEvaluator javadoc added test for LazyEvaluator.resetValue setting a new dsn resets the cached parsed dsn --- sentry/src/main/java/io/sentry/SentryOptions.java | 12 +++++------- .../main/java/io/sentry/util/LazyEvaluator.java | 7 ++++++- .../test/java/io/sentry/util/LazyEvaluatorTest.kt | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index b17e283bc6..91a1372fed 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -532,7 +532,7 @@ public void addIntegration(@NotNull Integration integration) { } /** - * Returns the DSN + * Returns the DSN. * * @return the DSN or null if not set */ @@ -541,13 +541,13 @@ public void addIntegration(@NotNull Integration integration) { } /** - * Returns the DSN + * Evaluates and parses the DSN. May throw an exception if the DSN is invalid. * - * @return the DSN or null if not set + * @return the parsed DSN or throws if dsn is invalid */ @ApiStatus.Internal @NotNull - Dsn getParsedDsn() { + Dsn getParsedDsn() throws IllegalArgumentException { return parsedDsn.getValue(); } @@ -558,9 +558,7 @@ Dsn getParsedDsn() { */ public void setDsn(final @Nullable String dsn) { this.dsn = dsn; - if (!isEnabled() || (dsn != null && dsn.isEmpty())) { - this.parsedDsn.resetValue(); - } + this.parsedDsn.resetValue(); dsnHash = StringUtils.calculateStringHash(this.dsn, logger); } diff --git a/sentry/src/main/java/io/sentry/util/LazyEvaluator.java b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java index e731fd3469..8b3a6cce53 100644 --- a/sentry/src/main/java/io/sentry/util/LazyEvaluator.java +++ b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java @@ -25,7 +25,8 @@ public LazyEvaluator(final @NotNull Evaluator evaluator) { } /** - * Executes the evaluator function and caches its result, so that it's called only once. + * Executes the evaluator function and caches its result, so that it's called only once, unless + * resetValue is called. * * @return The result of the evaluator function. */ @@ -48,6 +49,10 @@ public void setValue(final @Nullable T value) { } } + /** + * Resets the internal value and forces the evaluator function to be called the next time + * getValue() is called. + */ public void resetValue() { synchronized (this) { this.value = null; diff --git a/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt b/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt index 8f0e3bc0a7..a238205405 100644 --- a/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt +++ b/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt @@ -38,4 +38,18 @@ class LazyEvaluatorTest { assertEquals(1, evaluator.value) assertEquals(1, fixture.count) } + + @Test + fun `evaluates again after resetValue`() { + val evaluator = fixture.getSut() + assertEquals(0, fixture.count) + assertEquals(1, evaluator.value) + assertEquals(1, evaluator.value) + assertEquals(1, fixture.count) + // Evaluate again, only once + evaluator.resetValue() + assertEquals(2, evaluator.value) + assertEquals(2, evaluator.value) + assertEquals(2, fixture.count) + } }