Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[QA] Cache parsed Dsn #3796

Merged
merged 5 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind-of brittle, as whoever calls getParsedDsn() first, may receives an exception. On the other hand it was like this before, so all good for now 😃


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() {
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
Loading