Skip to content

Commit

Permalink
[QA] Cache parsed Dsn (#3796)
Browse files Browse the repository at this point in the history
* parsed Dsn object is now cached
* updated LazyEvaluator javadoc
* setting a new dsn resets the cached parsed dsn
  • Loading branch information
stefanosiano authored Oct 17, 2024
1 parent 143f91a commit 9182d86
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -5703,6 +5703,7 @@ public final class io/sentry/util/JsonSerializationUtils {
public final class io/sentry/util/LazyEvaluator {
public fun <init> (Lio/sentry/util/LazyEvaluator$Evaluator;)V
public fun getValue ()Ljava/lang/Object;
public fun resetValue ()V
public fun setValue (Ljava/lang/Object;)V
}

Expand Down
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/Baggage.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/DsnUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/RequestDetailsResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
17 changes: 16 additions & 1 deletion sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ public class SentryOptions {
*/
private @Nullable String dsn;

/** Parsed DSN to avoid parsing it every time. */
private final @NotNull LazyEvaluator<Dsn> parsedDsn = new LazyEvaluator<>(() -> new Dsn(dsn));

/** dsnHash is used as a subfolder of cacheDirPath to isolate events when rotating DSNs */
private @Nullable String dsnHash;

Expand Down Expand Up @@ -529,21 +532,33 @@ public void addIntegration(@NotNull Integration integration) {
}

/**
* Returns the DSN
* Returns the DSN.
*
* @return the DSN or null if not set
*/
public @Nullable String getDsn() {
return dsn;
}

/**
* Evaluates and parses the DSN. May throw an exception if the DSN is invalid.
*
* @return the parsed DSN or throws if dsn is invalid
*/
@ApiStatus.Internal
@NotNull
Dsn getParsedDsn() throws IllegalArgumentException {
return parsedDsn.getValue();
}

/**
* Sets the DSN
*
* @param dsn the DSN
*/
public void setDsn(final @Nullable String dsn) {
this.dsn = dsn;
this.parsedDsn.resetValue();

dsnHash = StringUtils.calculateStringHash(this.dsn, logger);
}
Expand Down
13 changes: 12 additions & 1 deletion sentry/src/main/java/io/sentry/util/LazyEvaluator.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public LazyEvaluator(final @NotNull Evaluator<T> 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.
*/
Expand All @@ -48,6 +49,16 @@ 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;
}
}

public interface Evaluator<T> {
@NotNull
T evaluate();
Expand Down
14 changes: 14 additions & 0 deletions sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 9182d86

Please sign in to comment.