diff --git a/CHANGELOG.md b/CHANGELOG.md index bfee768fa6..855e5c3cc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,15 @@ ## Unreleased +### Fixes + +- Only send userid in Dynamic Sampling Context if sendDefaultPii is true ([#2147](https://github.com/getsentry/sentry-java/pull/2147)) + ### Features - New package `sentry-android-navigation` for AndroidX Navigation support ([#2136](https://github.com/getsentry/sentry-java/pull/2136)) - New package `sentry-compose` for Jetpack Compose support (Navigation) ([#2136](https://github.com/getsentry/sentry-java/pull/2136)) +- Add sample rate to baggage as well as trace in envelope header and flatten user ([#2135](https://github.com/getsentry/sentry-java/pull/2135)) ## 6.1.4 diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 30a78d411d..4250419e2b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -6,6 +6,7 @@ import com.nhaarman.mockitokotlin2.whenever import io.sentry.Hint import io.sentry.IHub import io.sentry.SentryTracer +import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP import io.sentry.protocol.MeasurementValue @@ -21,7 +22,7 @@ class PerformanceAndroidEventProcessorTest { val options = SentryAndroidOptions() val hub = mock() - val context = TransactionContext("name", "op", true) + val context = TransactionContext("name", "op", TracesSamplingDecision(true)) val tracer = SentryTracer(context, hub) val activityFramesTracker = mock() diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index 419c13cedd..4b0760fd17 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -16,6 +16,7 @@ import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TraceContext +import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import io.sentry.protocol.SentryTransaction import kotlinx.coroutines.launch @@ -200,7 +201,7 @@ class SentryApolloInterceptorTest { private fun executeQuery(sut: ApolloClient = fixture.getSut(), isSpanActive: Boolean = true) = runBlocking { var tx: ITransaction? = null if (isSpanActive) { - tx = SentryTracer(TransactionContext("op", "desc", true), fixture.hub) + tx = SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.hub) whenever(fixture.hub.span).thenReturn(tx) } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt index 3f87cf9f23..db727aa1ee 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt @@ -11,6 +11,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus +import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -28,7 +29,7 @@ class SentrySpanRestTemplateCustomizerTest { val hub = mock() val restTemplate = RestTemplateBuilder().build() var mockServer = MockWebServer() - val transaction = SentryTracer(TransactionContext("aTransaction", "op", true), hub) + val transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) internal val customizer = SentrySpanRestTemplateCustomizer(hub) val url = mockServer.url("/test/123").toString() diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt index 8cb5bee0e8..a1a4c8941b 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt @@ -12,6 +12,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus +import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse @@ -34,7 +35,7 @@ class SentrySpanWebClientCustomizerTest { lateinit var sentryOptions: SentryOptions val hub = mock() var mockServer = MockWebServer() - val transaction = SentryTracer(TransactionContext("aTransaction", "op", true), hub) + val transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) private val customizer = SentrySpanWebClientCustomizer(hub) fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK, throwIOException: Boolean = false, includeMockServerInTracingOrigins: Boolean = true): WebClient { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a55aab4471..6267f40405 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -30,6 +30,7 @@ public final class io/sentry/Baggage { public fun setEnvironment (Ljava/lang/String;)V public fun setPublicKey (Ljava/lang/String;)V public fun setRelease (Ljava/lang/String;)V + public fun setSampleRate (Ljava/lang/String;)V public fun setTraceId (Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUserId (Ljava/lang/String;)V @@ -473,6 +474,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { public abstract fun getEventId ()Lio/sentry/protocol/SentryId; public abstract fun getLatestActiveSpan ()Lio/sentry/Span; public abstract fun getName ()Ljava/lang/String; + public abstract fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public abstract fun getSpans ()Ljava/util/List; public abstract fun isSampled ()Ljava/lang/Boolean; public abstract fun scheduleFinish ()V @@ -660,6 +662,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public fun getLatestActiveSpan ()Lio/sentry/Span; public fun getName ()Ljava/lang/String; public fun getOperation ()Ljava/lang/String; + public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanContext ()Lio/sentry/SpanContext; public fun getSpans ()Ljava/util/List; public fun getStatus ()Lio/sentry/SpanStatus; @@ -1386,6 +1389,7 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction { public fun getLatestActiveSpan ()Lio/sentry/Span; public fun getName ()Ljava/lang/String; public fun getOperation ()Ljava/lang/String; + public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanContext ()Lio/sentry/SpanContext; public fun getSpans ()Ljava/util/List; public fun getStartTimestamp ()Ljava/util/Date; @@ -1488,6 +1492,7 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun getHighPrecisionTimestamp ()Ljava/lang/Double; public fun getOperation ()Ljava/lang/String; public fun getParentSpanId ()Lio/sentry/SpanId; + public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanContext ()Lio/sentry/SpanContext; public fun getSpanId ()Lio/sentry/SpanId; public fun getStartTimestamp ()Ljava/util/Date; @@ -1520,14 +1525,15 @@ public class io/sentry/SpanContext : io/sentry/JsonSerializable, io/sentry/JsonU protected field status Lio/sentry/SpanStatus; protected field tags Ljava/util/Map; public fun (Lio/sentry/SpanContext;)V - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Lio/sentry/SpanStatus;)V - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Ljava/lang/String;Lio/sentry/SpanId;Ljava/lang/Boolean;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;Lio/sentry/SpanStatus;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Ljava/lang/String;Lio/sentry/SpanId;Lio/sentry/TracesSamplingDecision;)V public fun (Ljava/lang/String;)V - public fun (Ljava/lang/String;Ljava/lang/Boolean;)V + public fun (Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V public fun getDescription ()Ljava/lang/String; public fun getOperation ()Ljava/lang/String; public fun getParentSpanId ()Lio/sentry/SpanId; public fun getSampled ()Ljava/lang/Boolean; + public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanId ()Lio/sentry/SpanId; public fun getStatus ()Lio/sentry/SpanStatus; public fun getTags ()Ljava/util/Map; @@ -1537,6 +1543,7 @@ public class io/sentry/SpanContext : io/sentry/JsonSerializable, io/sentry/JsonU public fun setDescription (Ljava/lang/String;)V public fun setOperation (Ljava/lang/String;)V public fun setSampled (Ljava/lang/Boolean;)V + public fun setSamplingDecision (Lio/sentry/TracesSamplingDecision;)V public fun setStatus (Lio/sentry/SpanStatus;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setUnknown (Ljava/util/Map;)V @@ -1619,10 +1626,12 @@ public final class io/sentry/TraceContext : io/sentry/JsonSerializable, io/sentr public fun getEnvironment ()Ljava/lang/String; public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; + public fun getSampleRate ()Ljava/lang/String; public fun getTraceId ()Lio/sentry/protocol/SentryId; public fun getTransaction ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; - public fun getUser ()Lio/sentry/TraceContext$TraceContextUser; + public fun getUserId ()Ljava/lang/String; + public fun getUserSegment ()Ljava/lang/String; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V public fun setUnknown (Ljava/util/Map;)V public fun toBaggage (Lio/sentry/ILogger;)Lio/sentry/Baggage; @@ -1638,31 +1647,20 @@ public final class io/sentry/TraceContext$JsonKeys { public static final field ENVIRONMENT Ljava/lang/String; public static final field PUBLIC_KEY Ljava/lang/String; public static final field RELEASE Ljava/lang/String; + public static final field SAMPLE_RATE Ljava/lang/String; public static final field TRACE_ID Ljava/lang/String; public static final field TRANSACTION Ljava/lang/String; public static final field USER Ljava/lang/String; + public static final field USER_ID Ljava/lang/String; + public static final field USER_SEGMENT Ljava/lang/String; public fun ()V } -public final class io/sentry/TraceContext$TraceContextUser : io/sentry/JsonSerializable, io/sentry/JsonUnknown { - public fun (Lio/sentry/protocol/User;)V - public fun getId ()Ljava/lang/String; - public fun getSegment ()Ljava/lang/String; - public fun getUnknown ()Ljava/util/Map; - public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V - public fun setUnknown (Ljava/util/Map;)V -} - -public final class io/sentry/TraceContext$TraceContextUser$Deserializer : io/sentry/JsonDeserializer { - public fun ()V - public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/TraceContext$TraceContextUser; - public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; -} - -public final class io/sentry/TraceContext$TraceContextUser$JsonKeys { - public static final field ID Ljava/lang/String; - public static final field SEGMENT Ljava/lang/String; - public fun ()V +public final class io/sentry/TracesSamplingDecision { + public fun (Ljava/lang/Boolean;)V + public fun (Ljava/lang/Boolean;Ljava/lang/Double;)V + public fun getSampleRate ()Ljava/lang/Double; + public fun getSampled ()Ljava/lang/Boolean; } public final class io/sentry/TracingOrigins { @@ -1673,10 +1671,11 @@ public final class io/sentry/TracingOrigins { public final class io/sentry/TransactionContext : io/sentry/SpanContext { public fun (Ljava/lang/String;Ljava/lang/String;)V - public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;)V + public fun (Ljava/lang/String;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V public static fun fromSentryTrace (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext; public fun getName ()Ljava/lang/String; public fun getParentSampled ()Ljava/lang/Boolean; + public fun getParentSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun setParentSampled (Ljava/lang/Boolean;)V } @@ -2826,6 +2825,7 @@ public final class io/sentry/protocol/SentryTransaction : io/sentry/SentryBaseEv public fun (Lio/sentry/SentryTracer;)V public fun (Ljava/lang/String;Ljava/lang/Double;Ljava/lang/Double;Ljava/util/List;Ljava/util/Map;)V public fun getMeasurements ()Ljava/util/Map; + public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpans ()Ljava/util/List; public fun getStartTimestamp ()Ljava/lang/Double; public fun getStatus ()Lio/sentry/SpanStatus; @@ -3035,6 +3035,14 @@ public final class io/sentry/util/Platform { public static fun isJvm ()Z } +public final class io/sentry/util/SampleRateUtil { + public fun ()V + public static fun isValidSampleRate (Ljava/lang/Double;)Z + public static fun isValidSampleRate (Ljava/lang/Double;Z)Z + public static fun isValidTracesSampleRate (Ljava/lang/Double;)Z + public static fun isValidTracesSampleRate (Ljava/lang/Double;Z)Z +} + public final class io/sentry/util/StringUtils { public static fun byteCountToString (J)Ljava/lang/String; public static fun calculateStringHash (Ljava/lang/String;Lio/sentry/ILogger;)Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 4ebb46c68b..c93eef61e6 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -153,11 +153,11 @@ private static String decode(final @NotNull String value) throws UnsupportedEnco } public void setTraceId(final @Nullable String traceId) { - set("sentry-traceid", traceId); + set("sentry-trace_id", traceId); } public void setPublicKey(final @Nullable String publicKey) { - set("sentry-publickey", publicKey); + set("sentry-public_key", publicKey); } public void setEnvironment(final @Nullable String environment) { @@ -169,17 +169,21 @@ public void setRelease(final @Nullable String release) { } public void setUserId(final @Nullable String userId) { - set("sentry-userid", userId); + set("sentry-user_id", userId); } public void setUserSegment(final @Nullable String userSegment) { - set("sentry-usersegment", userSegment); + set("sentry-user_segment", userSegment); } public void setTransaction(final @Nullable String transaction) { set("sentry-transaction", transaction); } + public void setSampleRate(final @Nullable String sampleRate) { + set("sentry-sample_rate", sampleRate); + } + public void set(final @NotNull String key, final @Nullable String value) { this.keyValues.put(key, value); } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 5351c9d2b4..48435c7db0 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -730,8 +730,8 @@ public void flush(long timeoutMillis) { } else { final SamplingContext samplingContext = new SamplingContext(transactionContext, customSamplingContext); - boolean samplingDecision = tracesSampler.sample(samplingContext); - transactionContext.setSampled(samplingDecision); + @NotNull TracesSamplingDecision samplingDecision = tracesSampler.sample(samplingContext); + transactionContext.setSamplingDecision(samplingDecision); transaction = new SentryTracer( @@ -745,7 +745,7 @@ public void flush(long timeoutMillis) { // The listener is called only if the transaction exists, as the transaction is needed to // stop it - if (samplingDecision && options.isProfilingEnabled()) { + if (samplingDecision.getSampled() && options.isProfilingEnabled()) { final ITransactionProfiler transactionProfiler = options.getTransactionProfiler(); transactionProfiler.onTransactionStart(transaction); } diff --git a/sentry/src/main/java/io/sentry/ITransaction.java b/sentry/src/main/java/io/sentry/ITransaction.java index 2718f8988e..4ce3e773eb 100644 --- a/sentry/src/main/java/io/sentry/ITransaction.java +++ b/sentry/src/main/java/io/sentry/ITransaction.java @@ -35,6 +35,9 @@ public interface ITransaction extends ISpan { @Nullable Boolean isSampled(); + @Nullable + TracesSamplingDecision getSamplingDecision(); + /** * Returns the latest span that is not finished. * diff --git a/sentry/src/main/java/io/sentry/NoOpTransaction.java b/sentry/src/main/java/io/sentry/NoOpTransaction.java index 0c415449bb..8e4b6e0410 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransaction.java +++ b/sentry/src/main/java/io/sentry/NoOpTransaction.java @@ -136,6 +136,11 @@ public void setTag(@NotNull String key, @NotNull String value) {} return null; } + @Override + public @Nullable TracesSamplingDecision getSamplingDecision() { + return null; + } + @Override public void setData(@NotNull String key, @NotNull Object value) {} diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 51049b6d4c..902e76bfb0 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -13,6 +13,7 @@ import io.sentry.util.HintUtils; import io.sentry.util.LogUtils; import io.sentry.util.Objects; +import io.sentry.util.SampleRateUtil; import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -161,12 +162,17 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN continue; } + // if there is no trace context header we also won't send it to Sentry + final @Nullable TraceContext traceContext = envelope.getHeader().getTraceContext(); if (transaction.getContexts().getTrace() != null) { - // Hint: Set sampled in order for the transaction not to be dropped, as this is a - // transient property. - transaction.getContexts().getTrace().setSampled(true); + // Hint: Set sampling decision in order for the transaction not to be dropped, as this + // is a transient property. + transaction + .getContexts() + .getTrace() + .setSamplingDecision(extractSamplingDecision(traceContext)); } - hub.captureTransaction(transaction, envelope.getHeader().getTraceContext(), hint); + hub.captureTransaction(transaction, traceContext, hint); logItemCaptured(currentItem); if (!waitFlush(hint)) { @@ -216,6 +222,33 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN } } + private @NotNull TracesSamplingDecision extractSamplingDecision( + final @Nullable TraceContext traceContext) { + if (traceContext != null) { + final @Nullable String sampleRateString = traceContext.getSampleRate(); + if (sampleRateString != null) { + try { + final Double sampleRate = Double.parseDouble(sampleRateString); + if (!SampleRateUtil.isValidTracesSampleRate(sampleRate, false)) { + logger.log( + SentryLevel.ERROR, + "Invalid sample rate parsed from TraceContext: %s", + sampleRateString); + } else { + return new TracesSamplingDecision(true, sampleRate); + } + } catch (Exception e) { + logger.log( + SentryLevel.ERROR, + "Unable to parse sample rate from TraceContext: %s", + sampleRateString); + } + } + } + + return new TracesSamplingDecision(true); + } + private void logEnvelopeItemNull(final @NotNull SentryEnvelopeItem item, int itemIndex) { logger.log( SentryLevel.ERROR, diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index ea07b7bc9a..7892d927ae 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -10,6 +10,7 @@ import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.transport.NoOpTransportGate; import io.sentry.util.Platform; +import io.sentry.util.SampleRateUtil; import io.sentry.util.StringUtils; import java.io.File; import java.util.ArrayList; @@ -729,7 +730,7 @@ public void setProxy(@Nullable Proxy proxy) { * @param sampleRate the sample rate */ public void setSampleRate(Double sampleRate) { - if (sampleRate != null && (sampleRate > 1.0 || sampleRate <= 0.0)) { + if (!SampleRateUtil.isValidSampleRate(sampleRate)) { throw new IllegalArgumentException( "The value " + sampleRate @@ -753,7 +754,7 @@ public void setSampleRate(Double sampleRate) { * @param tracesSampleRate the sample rate */ public void setTracesSampleRate(final @Nullable Double tracesSampleRate) { - if (tracesSampleRate != null && (tracesSampleRate > 1.0 || tracesSampleRate < 0.0)) { + if (!SampleRateUtil.isValidTracesSampleRate(tracesSampleRate)) { throw new IllegalArgumentException( "The value " + tracesSampleRate diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index e941dc7ba5..a05d37c579 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -367,7 +367,9 @@ public void finish(@Nullable SpanStatus status) { scope -> { userAtomicReference.set(scope.getUser()); }); - this.traceContext = new TraceContext(this, userAtomicReference.get(), hub.getOptions()); + this.traceContext = + new TraceContext( + this, userAtomicReference.get(), hub.getOptions(), this.getSamplingDecision()); } return this.traceContext; } @@ -502,6 +504,11 @@ public void setData(@NotNull String key, @NotNull Object value) { return this.root.isSampled(); } + @Override + public @Nullable TracesSamplingDecision getSamplingDecision() { + return this.root.getSamplingDecision(); + } + @Override public void setName(@NotNull String name) { if (root.isFinished()) { diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 5207adea91..1f5889c669 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -66,7 +66,8 @@ public final class Span implements ISpan { final @Nullable Date startTimestamp, final @Nullable SpanFinishedCallback spanFinishedCallback) { this.context = - new SpanContext(traceId, new SpanId(), operation, parentSpanId, transaction.isSampled()); + new SpanContext( + traceId, new SpanId(), operation, parentSpanId, transaction.getSamplingDecision()); this.transaction = Objects.requireNonNull(transaction, "transaction is required"); this.hub = Objects.requireNonNull(hub, "hub is required"); this.spanFinishedCallback = spanFinishedCallback; @@ -258,6 +259,10 @@ public boolean isFinished() { return context.getSampled(); } + public @Nullable TracesSamplingDecision getSamplingDecision() { + return context.getSamplingDecision(); + } + @Override public void setThrowable(final @Nullable Throwable throwable) { if (finished.get()) { diff --git a/sentry/src/main/java/io/sentry/SpanContext.java b/sentry/src/main/java/io/sentry/SpanContext.java index 7efce08c30..d879510b84 100644 --- a/sentry/src/main/java/io/sentry/SpanContext.java +++ b/sentry/src/main/java/io/sentry/SpanContext.java @@ -26,8 +26,7 @@ public class SpanContext implements JsonUnknown, JsonSerializable { /** Id of a parent span. */ private final @Nullable SpanId parentSpanId; - /** If trace is sampled. */ - private transient @Nullable Boolean sampled; + private transient @Nullable TracesSamplingDecision samplingDecision; /** Short code identifying the type of operation the span is measuring. */ protected @NotNull String op; @@ -46,8 +45,9 @@ public class SpanContext implements JsonUnknown, JsonSerializable { private @Nullable Map unknown; - public SpanContext(final @NotNull String operation, final @Nullable Boolean sampled) { - this(new SentryId(), new SpanId(), operation, null, sampled); + public SpanContext( + final @NotNull String operation, final @Nullable TracesSamplingDecision samplingDecision) { + this(new SentryId(), new SpanId(), operation, null, samplingDecision); } /** @@ -64,8 +64,8 @@ public SpanContext( final @NotNull SpanId spanId, final @NotNull String operation, final @Nullable SpanId parentSpanId, - final @Nullable Boolean sampled) { - this(traceId, spanId, parentSpanId, operation, null, sampled, null); + final @Nullable TracesSamplingDecision samplingDecision) { + this(traceId, spanId, parentSpanId, operation, null, samplingDecision, null); } @ApiStatus.Internal @@ -75,13 +75,13 @@ public SpanContext( final @Nullable SpanId parentSpanId, final @NotNull String operation, final @Nullable String description, - final @Nullable Boolean sampled, + final @Nullable TracesSamplingDecision samplingDecision, final @Nullable SpanStatus status) { this.traceId = Objects.requireNonNull(traceId, "traceId is required"); this.spanId = Objects.requireNonNull(spanId, "spanId is required"); this.op = Objects.requireNonNull(operation, "operation is required"); this.parentSpanId = parentSpanId; - this.sampled = sampled; + this.samplingDecision = samplingDecision; this.description = description; this.status = status; } @@ -95,7 +95,7 @@ public SpanContext(final @NotNull SpanContext spanContext) { this.traceId = spanContext.traceId; this.spanId = spanContext.spanId; this.parentSpanId = spanContext.parentSpanId; - this.sampled = spanContext.sampled; + this.samplingDecision = spanContext.samplingDecision; this.op = spanContext.op; this.description = spanContext.description; this.status = spanContext.status; @@ -155,13 +155,30 @@ public SpanId getParentSpanId() { return tags; } + public @Nullable TracesSamplingDecision getSamplingDecision() { + return samplingDecision; + } + public @Nullable Boolean getSampled() { - return sampled; + if (samplingDecision == null) { + return null; + } + + return samplingDecision.getSampled(); } @ApiStatus.Internal public void setSampled(final @Nullable Boolean sampled) { - this.sampled = sampled; + if (sampled == null) { + setSamplingDecision(null); + } else { + setSamplingDecision(new TracesSamplingDecision(sampled)); + } + } + + @ApiStatus.Internal + public void setSamplingDecision(final @Nullable TracesSamplingDecision samplingDecision) { + this.samplingDecision = samplingDecision; } // region JsonSerializable diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index 35ea52fad0..28a235797b 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -2,8 +2,12 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import io.sentry.util.SampleRateUtil; import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; +import java.text.DecimalFormat; +import java.text.DecimalFormatSymbols; +import java.util.Locale; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.jetbrains.annotations.ApiStatus; @@ -16,14 +20,16 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { private @NotNull String publicKey; private @Nullable String release; private @Nullable String environment; - private @Nullable TraceContext.TraceContextUser user; + private @Nullable String userId; + private @Nullable String userSegment; private @Nullable String transaction; + private @Nullable String sampleRate; @SuppressWarnings("unused") private @Nullable Map unknown; TraceContext(@NotNull SentryId traceId, @NotNull String publicKey) { - this(traceId, publicKey, null, null, null, null); + this(traceId, publicKey, null, null, null, null, null, null); } TraceContext( @@ -31,27 +37,70 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { @NotNull String publicKey, @Nullable String release, @Nullable String environment, - @Nullable TraceContext.TraceContextUser user, - @Nullable String transaction) { + @Nullable String userId, + @Nullable String userSegment, + @Nullable String transaction, + @Nullable String sampleRate) { this.traceId = traceId; this.publicKey = publicKey; this.release = release; this.environment = environment; - this.user = user; + this.userId = userId; + this.userSegment = userSegment; this.transaction = transaction; + this.sampleRate = sampleRate; } TraceContext( final @NotNull ITransaction transaction, final @Nullable User user, - final @NotNull SentryOptions sentryOptions) { + final @NotNull SentryOptions sentryOptions, + final @Nullable TracesSamplingDecision samplingDecision) { this( transaction.getSpanContext().getTraceId(), new Dsn(sentryOptions.getDsn()).getPublicKey(), sentryOptions.getRelease(), sentryOptions.getEnvironment(), - user != null ? new TraceContextUser(user) : null, - transaction.getName()); + getUserId(sentryOptions, user), + user != null ? getSegment(user) : null, + transaction.getName(), + sampleRateToString(sampleRate(samplingDecision))); + } + + private static @Nullable String getUserId( + final @NotNull SentryOptions options, final @Nullable User user) { + if (options.isSendDefaultPii() && user != null) { + return user.getId(); + } + + return null; + } + + private static @Nullable String getSegment(final @NotNull User user) { + final Map others = user.getOthers(); + if (others != null) { + return others.get("segment"); + } else { + return null; + } + } + + private static @Nullable Double sampleRate(@Nullable TracesSamplingDecision samplingDecision) { + if (samplingDecision == null) { + return null; + } + + return samplingDecision.getSampleRate(); + } + + private static @Nullable String sampleRateToString(@Nullable Double sampleRateAsDouble) { + if (!SampleRateUtil.isValidTracesSampleRate(sampleRateAsDouble, false)) { + return null; + } + + DecimalFormat df = + new DecimalFormat("#.################", DecimalFormatSymbols.getInstance(Locale.ROOT)); + return df.format(sampleRateAsDouble); } public @NotNull SentryId getTraceId() { @@ -70,60 +119,51 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return environment; } - public @Nullable TraceContextUser getUser() { - return user; + public @Nullable String getUserId() { + return userId; + } + + public @Nullable String getUserSegment() { + return userSegment; } public @Nullable String getTransaction() { return transaction; } + public @Nullable String getSampleRate() { + return sampleRate; + } + public @NotNull Baggage toBaggage(@NotNull ILogger logger) { Baggage baggage = new Baggage(logger); baggage.setTraceId(traceId.toString()); baggage.setPublicKey(publicKey); + baggage.setSampleRate(sampleRate); baggage.setRelease(release); baggage.setEnvironment(environment); baggage.setTransaction(transaction); - - final TraceContextUser user = this.user; - if (user != null) { - baggage.setUserId(user.id); - baggage.setUserSegment(user.segment); - } + baggage.setUserId(userId); + baggage.setUserSegment(userSegment); return baggage; } - public static final class TraceContextUser implements JsonUnknown, JsonSerializable { + /** @deprecated only here to support parsing legacy JSON with non flattened user */ + @Deprecated + private static final class TraceContextUser implements JsonUnknown { private @Nullable String id; private @Nullable String segment; @SuppressWarnings("unused") private @Nullable Map unknown; - TraceContextUser(final @Nullable String id, final @Nullable String segment) { + private TraceContextUser(final @Nullable String id, final @Nullable String segment) { this.id = id; this.segment = segment; } - public TraceContextUser(final @Nullable User protocolUser) { - if (protocolUser != null) { - this.id = protocolUser.getId(); - this.segment = getSegment(protocolUser); - } - } - - private static @Nullable String getSegment(final @NotNull User user) { - final Map others = user.getOthers(); - if (others != null) { - return others.get("segment"); - } else { - return null; - } - } - public @Nullable String getId() { return id; } @@ -150,26 +190,6 @@ public static final class JsonKeys { public static final String SEGMENT = "segment"; } - @Override - public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) - throws IOException { - writer.beginObject(); - if (id != null) { - writer.name(TraceContextUser.JsonKeys.ID).value(id); - } - if (segment != null) { - writer.name(TraceContextUser.JsonKeys.SEGMENT).value(segment); - } - if (unknown != null) { - for (String key : unknown.keySet()) { - Object value = unknown.get(key); - writer.name(key); - writer.value(logger, value); - } - } - writer.endObject(); - } - public static final class Deserializer implements JsonDeserializer { @Override public @NotNull TraceContextUser deserialize( @@ -225,7 +245,10 @@ public static final class JsonKeys { public static final String RELEASE = "release"; public static final String ENVIRONMENT = "environment"; public static final String USER = "user"; + public static final String USER_ID = "user_id"; + public static final String USER_SEGMENT = "user_segment"; public static final String TRANSACTION = "transaction"; + public static final String SAMPLE_RATE = "sample_rate"; } @Override @@ -240,14 +263,18 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (environment != null) { writer.name(TraceContext.JsonKeys.ENVIRONMENT).value(environment); } - if (user != null) { - if (user.id != null || user.segment != null || user.unknown != null) { - writer.name(TraceContext.JsonKeys.USER).value(logger, user); - } + if (userId != null) { + writer.name(TraceContext.JsonKeys.USER_ID).value(userId); + } + if (userSegment != null) { + writer.name(TraceContext.JsonKeys.USER_SEGMENT).value(userSegment); } if (transaction != null) { writer.name(TraceContext.JsonKeys.TRANSACTION).value(transaction); } + if (sampleRate != null) { + writer.name(TraceContext.JsonKeys.SAMPLE_RATE).value(sampleRate); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -269,7 +296,10 @@ public static final class Deserializer implements JsonDeserializer String release = null; String environment = null; TraceContextUser user = null; + String userId = null; + String userSegment = null; String transaction = null; + String sampleRate = null; Map unknown = null; while (reader.peek() == JsonToken.NAME) { @@ -290,9 +320,18 @@ public static final class Deserializer implements JsonDeserializer case TraceContext.JsonKeys.USER: user = reader.nextOrNull(logger, new TraceContextUser.Deserializer()); break; + case TraceContext.JsonKeys.USER_ID: + userId = reader.nextStringOrNull(); + break; + case TraceContext.JsonKeys.USER_SEGMENT: + userSegment = reader.nextStringOrNull(); + break; case TraceContext.JsonKeys.TRANSACTION: transaction = reader.nextStringOrNull(); break; + case TraceContext.JsonKeys.SAMPLE_RATE: + sampleRate = reader.nextStringOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); @@ -307,11 +346,27 @@ public static final class Deserializer implements JsonDeserializer if (publicKey == null) { throw missingRequiredFieldException(TraceContext.JsonKeys.PUBLIC_KEY, logger); } - TraceContext traceStateUser = - new TraceContext(traceId, publicKey, release, environment, user, transaction); - traceStateUser.setUnknown(unknown); + if (user != null) { + if (userId == null) { + userId = user.getId(); + } + if (userSegment == null) { + userSegment = user.getSegment(); + } + } + TraceContext traceContext = + new TraceContext( + traceId, + publicKey, + release, + environment, + userId, + userSegment, + transaction, + sampleRate); + traceContext.setUnknown(unknown); reader.endObject(); - return traceStateUser; + return traceContext; } private Exception missingRequiredFieldException(String field, ILogger logger) { diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 9f14ec9502..c169c18faf 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -19,23 +19,34 @@ public TracesSampler(final @NotNull SentryOptions options) { this.random = random; } - boolean sample(final @NotNull SamplingContext samplingContext) { - if (samplingContext.getTransactionContext().getSampled() != null) { - return samplingContext.getTransactionContext().getSampled(); + @NotNull + TracesSamplingDecision sample(final @NotNull SamplingContext samplingContext) { + final TracesSamplingDecision samplingContextSamplingDecision = + samplingContext.getTransactionContext().getSamplingDecision(); + if (samplingContextSamplingDecision != null) { + return samplingContextSamplingDecision; } + if (options.getTracesSampler() != null) { final Double samplerResult = options.getTracesSampler().sample(samplingContext); if (samplerResult != null) { - return sample(samplerResult); + return new TracesSamplingDecision(sample(samplerResult), samplerResult); } } - if (samplingContext.getTransactionContext().getParentSampled() != null) { - return samplingContext.getTransactionContext().getParentSampled(); + + final TracesSamplingDecision parentSamplingDecision = + samplingContext.getTransactionContext().getParentSamplingDecision(); + if (parentSamplingDecision != null) { + return parentSamplingDecision; } - if (options.getTracesSampleRate() != null) { - return sample(options.getTracesSampleRate()); + + final Double tracesSampleRateFromOptions = options.getTracesSampleRate(); + if (tracesSampleRateFromOptions != null) { + return new TracesSamplingDecision( + sample(tracesSampleRateFromOptions), tracesSampleRateFromOptions); } - return false; + + return new TracesSamplingDecision(false); } private boolean sample(final @NotNull Double aDouble) { diff --git a/sentry/src/main/java/io/sentry/TracesSamplingDecision.java b/sentry/src/main/java/io/sentry/TracesSamplingDecision.java new file mode 100644 index 0000000000..3e8d895fb5 --- /dev/null +++ b/sentry/src/main/java/io/sentry/TracesSamplingDecision.java @@ -0,0 +1,29 @@ +package io.sentry; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TracesSamplingDecision { + + private final @NotNull Boolean sampled; + private final @Nullable Double sampleRate; + + public TracesSamplingDecision(@NotNull Boolean sampled, @Nullable Double sampleRate) { + this.sampled = sampled; + this.sampleRate = sampleRate; + } + + public TracesSamplingDecision(@NotNull Boolean sampled) { + this(sampled, null); + } + + public @NotNull Boolean getSampled() { + return sampled; + } + + public @Nullable Double getSampleRate() { + return sampleRate; + } +} diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index 25296713b7..4b7c1f8da1 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -7,7 +7,7 @@ public final class TransactionContext extends SpanContext { private final @NotNull String name; - private @Nullable Boolean parentSampled; + private @Nullable TracesSamplingDecision parentSamplingDecision; /** * Creates {@link TransactionContext} from sentry-trace header. @@ -21,19 +21,24 @@ public final class TransactionContext extends SpanContext { final @NotNull String name, final @NotNull String operation, final @NotNull SentryTraceHeader sentryTrace) { + @Nullable Boolean parentSampled = sentryTrace.isSampled(); return new TransactionContext( name, operation, sentryTrace.getTraceId(), new SpanId(), sentryTrace.getSpanId(), - sentryTrace.isSampled()); + parentSampled == null + ? null + : new TracesSamplingDecision( + parentSampled)); // TODO sampleRate should be retrieved from baggage and passed here + // in the future } public TransactionContext(final @NotNull String name, final @NotNull String operation) { super(operation); this.name = Objects.requireNonNull(name, "name is required"); - this.parentSampled = null; + this.parentSamplingDecision = null; } /** @@ -41,15 +46,15 @@ public TransactionContext(final @NotNull String name, final @NotNull String oper * * @param name - transaction name * @param operation - operation - * @param sampled - sampling decision + * @param samplingDecision - sampling decision */ public TransactionContext( final @NotNull String name, final @NotNull String operation, - final @Nullable Boolean sampled) { + final @Nullable TracesSamplingDecision samplingDecision) { super(operation); this.name = Objects.requireNonNull(name, "name is required"); - this.setSampled(sampled); + this.setSamplingDecision(samplingDecision); } private TransactionContext( @@ -58,10 +63,10 @@ private TransactionContext( final @NotNull SentryId traceId, final @NotNull SpanId spanId, final @Nullable SpanId parentSpanId, - final @Nullable Boolean parentSampled) { + final @Nullable TracesSamplingDecision parentSamplingDecision) { super(traceId, spanId, operation, parentSpanId, null); this.name = Objects.requireNonNull(name, "name is required"); - this.parentSampled = parentSampled; + this.parentSamplingDecision = parentSamplingDecision; } public @NotNull String getName() { @@ -69,10 +74,22 @@ private TransactionContext( } public @Nullable Boolean getParentSampled() { - return parentSampled; + if (parentSamplingDecision == null) { + return null; + } + + return parentSamplingDecision.getSampled(); + } + + public @Nullable TracesSamplingDecision getParentSamplingDecision() { + return parentSamplingDecision; } public void setParentSampled(final @Nullable Boolean parentSampled) { - this.parentSampled = parentSampled; + if (parentSampled == null) { + this.parentSamplingDecision = null; + } else { + this.parentSamplingDecision = new TracesSamplingDecision(parentSampled); + } } } diff --git a/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java b/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java index fecd9072f8..f2c6f31898 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java @@ -12,6 +12,7 @@ import io.sentry.Span; import io.sentry.SpanContext; import io.sentry.SpanStatus; +import io.sentry.TracesSamplingDecision; import io.sentry.util.Objects; import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; @@ -73,7 +74,7 @@ public SentryTransaction(final @NotNull SentryTracer sentryTracer) { tracerContext.getParentSpanId(), tracerContext.getOperation(), tracerContext.getDescription(), - tracerContext.getSampled(), + tracerContext.getSamplingDecision(), tracerContext.getStatus())); for (final Map.Entry tag : tracerContext.getTags().entrySet()) { this.setTag(tag.getKey(), tag.getValue()); @@ -131,8 +132,21 @@ public boolean isFinished() { } public boolean isSampled() { + final @Nullable TracesSamplingDecision samplingDecsion = getSamplingDecision(); + if (samplingDecsion == null) { + return false; + } + + return samplingDecsion.getSampled(); + } + + public @Nullable TracesSamplingDecision getSamplingDecision() { final SpanContext trace = this.getContexts().getTrace(); - return trace != null && Boolean.TRUE.equals(trace.getSampled()); + if (trace == null) { + return null; + } + + return trace.getSamplingDecision(); } public @NotNull Map getMeasurements() { diff --git a/sentry/src/main/java/io/sentry/util/SampleRateUtil.java b/sentry/src/main/java/io/sentry/util/SampleRateUtil.java new file mode 100644 index 0000000000..0035a240be --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/SampleRateUtil.java @@ -0,0 +1,33 @@ +package io.sentry.util; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class SampleRateUtil { + + public static boolean isValidSampleRate(@Nullable Double sampleRate) { + return isValidSampleRate(sampleRate, true); + } + + public static boolean isValidSampleRate(@Nullable Double sampleRate, boolean allowNull) { + if (sampleRate == null) { + return allowNull; + } + + return !(sampleRate.isNaN() || (sampleRate > 1.0 || sampleRate <= 0.0)); + } + + public static boolean isValidTracesSampleRate(@Nullable Double tracesSampleRate) { + return isValidTracesSampleRate(tracesSampleRate, true); + } + + public static boolean isValidTracesSampleRate( + @Nullable Double tracesSampleRate, boolean allowNull) { + if (tracesSampleRate == null) { + return allowNull; + } + + return !(tracesSampleRate.isNaN() || (tracesSampleRate > 1.0 || tracesSampleRate < 0.0)); + } +} diff --git a/sentry/src/test/java/io/sentry/BaggageTest.kt b/sentry/src/test/java/io/sentry/BaggageTest.kt index c44867067f..1ced5f9fd3 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -198,8 +198,9 @@ class BaggageTest { baggage.setTransaction("TX") baggage.setUserId(userId) baggage.setUserSegment("segmentA") + baggage.setSampleRate((1.0 / 3.0).toString()) - assertEquals("sentry-environment=production,sentry-publickey=$publicKey,sentry-release=1.0-rc.1,sentry-traceid=$traceId,sentry-transaction=TX,sentry-userid=$userId,sentry-usersegment=segmentA", baggage.toHeaderString()) + assertEquals("sentry-environment=production,sentry-public_key=$publicKey,sentry-release=1.0-rc.1,sentry-sample_rate=0.3333333333333333,sentry-trace_id=$traceId,sentry-transaction=TX,sentry-user_id=$userId,sentry-user_segment=segmentA", baggage.toHeaderString()) } @Test @@ -210,12 +211,12 @@ class BaggageTest { @Test fun `setting a value multiple times only keeps the last`() { - val baggage = Baggage.fromHeader("sentry-traceid=a", logger) + val baggage = Baggage.fromHeader("sentry-trace_id=a", logger) baggage.setTraceId("b") baggage.setTraceId("c") - assertEquals("sentry-traceid=c", baggage.toHeaderString()) + assertEquals("sentry-trace_id=c", baggage.toHeaderString()) } @Test diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 5f64108b6f..5ab9f53b03 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1204,7 +1204,7 @@ class HubTest { val mockClient = mock() sut.bindClient(mockClient) - val sentryTracer = SentryTracer(TransactionContext("name", "op", true), sut) + val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sentryTracer.finish() val traceContext = sentryTracer.traceContext() verify(mockClient).captureTransaction(any(), eq(traceContext), any(), eq(null), anyOrNull()) @@ -1218,14 +1218,14 @@ class HubTest { it.setTransactionProfiler(mockTransactionProfiler) } // Transaction is not sampled, so it should not be profiled - val contexts = TransactionContext("name", "op", false) + val contexts = TransactionContext("name", "op", TracesSamplingDecision(false)) val transaction = hub.startTransaction(contexts) transaction.finish() verify(mockTransactionProfiler, never()).onTransactionStart(anyOrNull()) verify(mockTransactionProfiler, never()).onTransactionFinish(anyOrNull()) // Transaction is sampled, so it should be profiled - val sampledContexts = TransactionContext("name", "op", true) + val sampledContexts = TransactionContext("name", "op", TracesSamplingDecision(true)) val sampledTransaction = hub.startTransaction(sampledContexts) sampledTransaction.finish() verify(mockTransactionProfiler).onTransactionStart(anyOrNull()) @@ -1239,7 +1239,7 @@ class HubTest { it.isProfilingEnabled = false it.setTransactionProfiler(mockTransactionProfiler) } - val contexts = TransactionContext("name", "op", true) + val contexts = TransactionContext("name", "op", TracesSamplingDecision(true)) val transaction = hub.startTransaction(contexts) transaction.finish() verify(mockTransactionProfiler, never()).onTransactionStart(anyOrNull()) @@ -1257,7 +1257,7 @@ class HubTest { sut.bindClient(mockClient) whenever(mockClient.captureTransaction(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())).thenReturn(SentryId()) - val sentryTracer = SentryTracer(TransactionContext("name", "op", true), sut) + val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sentryTracer.finish() assertEquals(SentryId.EMPTY_ID, sut.lastEventId) } @@ -1272,7 +1272,7 @@ class HubTest { val mockClient = mock() sut.bindClient(mockClient) - val sentryTracer = SentryTracer(TransactionContext("name", "op", true), sut) + val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), sut) sut.captureTransaction(SentryTransaction(sentryTracer), null as TraceContext?) verify(mockClient, never()).captureTransaction(any(), any(), any(), eq(null), anyOrNull()) } @@ -1287,7 +1287,7 @@ class HubTest { val mockClient = mock() sut.bindClient(mockClient) - val sentryTracer = SentryTracer(TransactionContext("name", "op", false), sut) + val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(false)), sut) sentryTracer.finish() val traceContext = sentryTracer.traceContext() verify(mockClient, never()).captureTransaction(any(), eq(traceContext), any(), eq(null), anyOrNull()) @@ -1303,7 +1303,7 @@ class HubTest { val mockClient = mock() sut.bindClient(mockClient) - val sentryTracer = SentryTracer(TransactionContext("name", "op", false), sut) + val sentryTracer = SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(false)), sut) sentryTracer.finish() assertClientReport( @@ -1397,7 +1397,7 @@ class HubTest { it.tracesSampleRate = null it.tracesSampler = null } - val transaction = hub.startTransaction(TransactionContext("name", "op", true)) + val transaction = hub.startTransaction(TransactionContext("name", "op", TracesSamplingDecision(true))) assertTrue(transaction is NoOpTransaction) } //endregion diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index a7c819978f..12f1caf186 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -443,31 +443,23 @@ class JsonSerializerTest { @Test fun `serializes trace context`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", TraceContext.TraceContextUser("userId", "segment"), "transaction")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user":{"id":"userId","segment":"segment"},"transaction":"transaction"}}""" - val json = serializeToString(traceContext) - assertEquals(expected, json) - } - - @Test - fun `serializes trace context with null user`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, "transaction")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "segment", "transaction", "0.5")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction","sample_rate":"0.5"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } @Test fun `serializes trace context with user having null id and segment`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", TraceContext.TraceContextUser(null, null), "transaction")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, null, "transaction", "0.6")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } @Test fun `deserializes trace context`() { - val json = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user":{"id":"userId","segment":"segment"},"transaction":"transaction"}}""" + val json = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction"}}""" val actual = fixture.serializer.deserialize(StringReader(json), SentryEnvelopeHeader::class.java) assertNotNull(actual) { assertNotNull(it.traceContext) { @@ -475,10 +467,8 @@ class JsonSerializerTest { assertEquals("key", it.publicKey) assertEquals("release", it.release) assertEquals("environment", it.environment) - assertNotNull(it.user) { - assertEquals("userId", it.id) - assertEquals("segment", it.segment) - } + assertEquals("userId", it.userId) + assertEquals("segment", it.userSegment) } } } @@ -493,7 +483,8 @@ class JsonSerializerTest { assertEquals("key", it.publicKey) assertEquals("release", it.release) assertEquals("environment", it.environment) - assertNull(it.user) + assertNull(it.userId) + assertNull(it.userSegment) } } } diff --git a/sentry/src/test/java/io/sentry/JsonUnknownSerializationTest.kt b/sentry/src/test/java/io/sentry/JsonUnknownSerializationTest.kt index ad5a1df73b..591424a4e2 100644 --- a/sentry/src/test/java/io/sentry/JsonUnknownSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/JsonUnknownSerializationTest.kt @@ -79,7 +79,6 @@ class JsonUnknownSerializationTest( val skdVersion = givenJsonUnknown(SdkVersion("3e934135-3f2b-49bc-8756-9f025b55143e", "3e31738e-4106-42d0-8be2-4a3a1bc648d3")) val spanContext = givenJsonUnknown(SpanContext("c2fb8fee2e2b49758bcb67cda0f713c7")) val traceContext = givenJsonUnknown(TraceContextSerializationTest.Fixture().getSut()) - val traceContextUser = givenJsonUnknown(TraceContextUserSerializationTest.Fixture().getSut()) val user = givenJsonUnknown(User()) val userFeedback = givenJsonUnknown(UserFeedback(SentryId("c2fb8fee2e2b49758bcb67cda0f713c7"))) @@ -114,7 +113,6 @@ class JsonUnknownSerializationTest( arrayOf(skdVersion, skdVersion, SdkVersion.Deserializer()::deserialize), arrayOf(spanContext, spanContext, SpanContext.Deserializer()::deserialize), arrayOf(traceContext, traceContext, TraceContext.Deserializer()::deserialize), - arrayOf(traceContextUser, traceContextUser, TraceContext.TraceContextUser.Deserializer()::deserialize), arrayOf(user, user, User.Deserializer()::deserialize), arrayOf(userFeedback, userFeedback, UserFeedback.Deserializer()::deserialize) ) diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 3f14f3cf5c..4b23fdea90 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -132,6 +132,63 @@ class OutboxSenderTest { verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any(), any()) } + @Test + fun `restores sampleRate`() { + fixture.envelopeReader = EnvelopeReader(JsonSerializer(fixture.options)) + whenever(fixture.options.maxSpans).thenReturn(1000) + whenever(fixture.hub.options).thenReturn(fixture.options) + whenever(fixture.options.transactionProfiler).thenReturn(NoOpTransactionProfiler.getInstance()) + + val transactionContext = TransactionContext("fixture-name", "http") + transactionContext.description = "fixture-request" + transactionContext.status = SpanStatus.OK + transactionContext.setTag("fixture-tag", "fixture-value") + transactionContext.samplingDecision = TracesSamplingDecision(true, 0.00000021) + + val sentryTracer = SentryTracer(transactionContext, fixture.hub) + val span = sentryTracer.startChild("child") + span.finish(SpanStatus.OK) + sentryTracer.finish() + + val sentryTracerSpy = spy(sentryTracer) + whenever(sentryTracerSpy.eventId).thenReturn(SentryId("3367f5196c494acaae85bbbd535379ac")) + + val expected = SentryTransaction(sentryTracerSpy) + whenever(fixture.serializer.deserialize(any(), eq(SentryTransaction::class.java))).thenReturn(expected) + + val sut = fixture.getSut() + val path = getTempEnvelope(fileName = "envelope-transaction-with-sample-rate.txt") + assertTrue(File(path).exists()) + + val hints = HintUtils.createWithTypeCheckHint(mock()) + sut.processEnvelopeFile(path, hints) + + verify(fixture.hub).captureTransaction( + check { + assertEquals(expected, it) + assertTrue(it.isSampled) + assertEquals(0.00000021, it.samplingDecision?.sampleRate) + assertTrue(it.samplingDecision!!.sampled) + }, + check { + assertEquals("b156a475de54423d9c1571df97ec7eb6", it.traceId.toString()) + assertEquals("key", it.publicKey) + assertEquals("0.00000021", it.sampleRate) + assertEquals("1.0-beta.1", it.release) + assertEquals("prod", it.environment) + assertEquals("usr1", it.userId) + assertEquals("pro", it.userSegment) + assertEquals("tx1", it.transaction) + }, + any() + ) + assertFalse(File(path).exists()) + + // Additionally make sure we have no errors logged + verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any(), any()) + verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any(), any()) + } + @Test fun `when parser is EnvelopeReader and serializer returns SentryEnvelope, event captured, file is deleted `() { fixture.envelopeReader = EnvelopeReader(JsonSerializer(fixture.options)) diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index a38fffe98f..c5bc1a37a6 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -40,10 +40,10 @@ class SentryTracerTest { idleTimeout: Long? = null, trimEnd: Boolean = false, transactionFinishedCallback: TransactionFinishedCallback? = null, - sampled: Boolean? = null + samplingDecision: TracesSamplingDecision? = null ): SentryTracer { optionsConfiguration.configure(options) - return SentryTracer(TransactionContext("name", "op", sampled), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback) + return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback) } } @@ -144,7 +144,7 @@ class SentryTracerTest { val tracer = fixture.getSut(optionsConfiguration = { it.isProfilingEnabled = true it.setTransactionProfiler(transactionProfiler) - }, sampled = true) + }, samplingDecision = TracesSamplingDecision(true)) tracer.finish() verify(transactionProfiler).onTransactionFinish(any()) } @@ -188,7 +188,7 @@ class SentryTracerTest { @Test fun `not sampled spans are filtered out`() { - val tracer = fixture.getSut(sampled = true) + val tracer = fixture.getSut(samplingDecision = TracesSamplingDecision(true)) tracer.startChild("op1") val span = tracer.startChild("op2") span.spanContext.sampled = false @@ -459,7 +459,7 @@ class SentryTracerTest { @Test fun `finishing unfinished spans with the transaction timestamp`() { - val transaction = fixture.getSut(sampled = true) + val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true)) val span = transaction.startChild("op") as Span transaction.startChild("op2") transaction.finish(SpanStatus.INVALID_ARGUMENT) @@ -481,6 +481,7 @@ class SentryTracerTest { fun `returns trace state`() { val transaction = fixture.getSut({ it.isTraceSampling = true + it.isSendDefaultPii = true }) fixture.hub.setUser( User().apply { @@ -495,10 +496,31 @@ class SentryTracerTest { assertEquals("environment", it.environment) assertEquals("release@3.0.0", it.release) assertEquals(transaction.name, it.transaction) - assertNotNull(it.user) { - assertEquals("user-id", it.id) - assertEquals("pro", it.segment) + assertEquals("user-id", it.userId) + assertEquals("pro", it.userSegment) + } + } + + @Test + fun `returns trace state without userId if not send pii`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + }) + fixture.hub.setUser( + User().apply { + id = "user-id" + others = mapOf("segment" to "pro") } + ) + val trace = transaction.traceContext() + assertNotNull(trace) { + assertEquals(transaction.spanContext.traceId, it.traceId) + assertEquals("key", it.publicKey) + assertEquals("environment", it.environment) + assertEquals("release@3.0.0", it.release) + assertEquals(transaction.name, it.transaction) + assertNull(it.userId) + assertEquals("pro", it.userSegment) } } @@ -516,12 +538,44 @@ class SentryTracerTest { val traceAfterUserSet = transaction.traceContext() assertNotNull(traceAfterUserSet) { assertEquals(it, traceBeforeUserSet) - assertNull(it.user) + assertNull(it.userId) + assertNull(it.userSegment) } } @Test fun `returns baggage header`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + it.environment = "production" + it.release = "1.0.99-rc.7" + it.isSendDefaultPii = true + }) + + fixture.hub.setUser( + User().apply { + id = "userId12345" + others = mapOf("segment" to "pro") + } + ) + + val header = transaction.toBaggageHeader() + assertNotNull(header) { + assertEquals("baggage", it.name) + assertNotNull(it.value) + println(it.value) + assertTrue(it.value.contains("sentry-trace_id=[^,]+".toRegex())) + assertTrue(it.value.contains("sentry-public_key=key,")) + assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) + assertTrue(it.value.contains("sentry-environment=production,")) + assertTrue(it.value.contains("sentry-transaction=name,")) + assertTrue(it.value.contains("sentry-user_id=userId12345,")) + assertTrue(it.value.contains("sentry-user_segment=pro$".toRegex())) + } + } + + @Test + fun `returns baggage header without userId if not send pii`() { val transaction = fixture.getSut({ it.isTraceSampling = true it.environment = "production" @@ -540,19 +594,45 @@ class SentryTracerTest { assertEquals("baggage", it.name) assertNotNull(it.value) println(it.value) - assertTrue(it.value.contains("sentry-traceid=[^,]+".toRegex())) - assertTrue(it.value.contains("sentry-publickey=key,")) + assertTrue(it.value.contains("sentry-trace_id=[^,]+".toRegex())) + assertTrue(it.value.contains("sentry-public_key=key,")) assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) assertTrue(it.value.contains("sentry-environment=production,")) assertTrue(it.value.contains("sentry-transaction=name,")) - assertTrue(it.value.contains("sentry-userid=userId12345,")) - assertTrue(it.value.contains("sentry-usersegment=pro$".toRegex())) + assertFalse(it.value.contains("sentry-user_id")) + assertTrue(it.value.contains("sentry-user_segment=pro$".toRegex())) + } + } + + @Test + fun `returns baggage header without userId if send pii and null user`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + it.environment = "production" + it.release = "1.0.99-rc.7" + it.isSendDefaultPii = true + }) + + fixture.hub.setUser(null) + + val header = transaction.toBaggageHeader() + assertNotNull(header) { + assertEquals("baggage", it.name) + assertNotNull(it.value) + println(it.value) + assertTrue(it.value.contains("sentry-trace_id=[^,]+".toRegex())) + assertTrue(it.value.contains("sentry-public_key=key,")) + assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) + assertTrue(it.value.contains("sentry-environment=production,")) + assertTrue(it.value.contains("sentry-transaction=name")) + assertFalse(it.value.contains("sentry-user_id")) + assertFalse(it.value.contains("sentry-user_segment")) } } @Test fun `sets ITransaction data as extra in SentryTransaction`() { - val transaction = fixture.getSut(sampled = true) + val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true)) transaction.setData("key", "val") transaction.finish() verify(fixture.hub).captureTransaction( @@ -567,7 +647,7 @@ class SentryTracerTest { @Test fun `sets Span data as data in SentrySpan`() { - val transaction = fixture.getSut(sampled = true) + val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true)) val span = transaction.startChild("op") span.setData("key", "val") span.finish() @@ -667,7 +747,7 @@ class SentryTracerTest { @Test fun `when trimEnd, trims idle transaction time to the latest child timestamp`() { - val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, sampled = true) + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) val span = transaction.startChild("op") span.finish() @@ -693,19 +773,19 @@ class SentryTracerTest { @Test fun `timer is created if idle timeout is set`() { - val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, sampled = true) + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) assertNotNull(transaction.timer) } @Test fun `timer is not created if idle timeout is not set`() { - val transaction = fixture.getSut(waitForChildren = true, idleTimeout = null, trimEnd = true, sampled = true) + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = null, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) assertNull(transaction.timer) } @Test fun `timer is cancelled on finish`() { - val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, sampled = true) + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) assertNotNull(transaction.timer) transaction.finish(SpanStatus.OK) assertNull(transaction.timer) diff --git a/sentry/src/test/java/io/sentry/SpanTest.kt b/sentry/src/test/java/io/sentry/SpanTest.kt index a7e1931d4d..da090abe68 100644 --- a/sentry/src/test/java/io/sentry/SpanTest.kt +++ b/sentry/src/test/java/io/sentry/SpanTest.kt @@ -112,7 +112,7 @@ class SpanTest { val span = Span( traceId, parentSpanId, SentryTracer( - TransactionContext("name", "op", true), fixture.hub + TransactionContext("name", "op", TracesSamplingDecision(true)), fixture.hub ), "op", fixture.hub ) diff --git a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt index 8d40d1c1f6..ffe43fa524 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -2,10 +2,12 @@ package io.sentry import com.nhaarman.mockitokotlin2.mock import io.sentry.protocol.SentryId +import io.sentry.protocol.User import org.junit.Test import java.io.StringReader import java.io.StringWriter import kotlin.test.assertEquals +import kotlin.test.assertTrue class TraceContextSerializationTest { @@ -17,8 +19,10 @@ class TraceContextSerializationTest { "5d593cac-f833-4845-bb23-4eabdf720da2", "9ee2c92c-401e-4296-b6f0-fb3b13edd9ee", "0666ab02-6364-4135-aa59-02e8128ce052", - TraceContextUserSerializationTest.Fixture().getSut(), - "0252ec25-cd0a-4230-bd2f-936a4585637e" + "c052c566-6619-45f5-a61f-172802afa39a", + "f7d8662b-5551-4ef8-b6a8-090f0561a530", + "0252ec25-cd0a-4230-bd2f-936a4585637e", + "0.00000021" ) } private val fixture = Fixture() @@ -38,6 +42,40 @@ class TraceContextSerializationTest { assertEquals(expectedJson, actualJson) } + @Test + fun `sample rate uses digit dot digit formatting`() { + val traceContext = createTraceContext(0.00000021) + + val json = serialize(traceContext) + assertTrue(json.contains(""""sample_rate":"0.00000021""""), json) + } + + private fun createTraceContext(sRate: Double): TraceContext { + return TraceContext( + SentryTracer(TransactionContext("name", "op"), mock()), + User().apply { + id = "user-id" + others = mapOf("segment" to "pro") + }, + SentryOptions().apply { + dsn = dsnString + environment = "prod" + release = "1.0.17" + tracesSampleRate = sRate + }, + TracesSamplingDecision(sRate > 0.5, sRate) + ) + } + + @Test + fun `can still parse legacy JSON with non flat user`() { + val expectedJson = sanitizedFile("json/trace_state_no_sample_rate.json") + val legacyJson = sanitizedFile("json/trace_state_legacy.json") + val actual = deserialize(legacyJson) + val actualJson = serialize(actual) + assertEquals(expectedJson, actualJson) + } + // Helper private fun sanitizedFile(path: String): String { diff --git a/sentry/src/test/java/io/sentry/TraceContextUserSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextUserSerializationTest.kt deleted file mode 100644 index 6ed6fc6074..0000000000 --- a/sentry/src/test/java/io/sentry/TraceContextUserSerializationTest.kt +++ /dev/null @@ -1,55 +0,0 @@ -package io.sentry - -import com.nhaarman.mockitokotlin2.mock -import org.junit.Test -import java.io.StringReader -import java.io.StringWriter -import kotlin.test.assertEquals - -class TraceContextUserSerializationTest { - - class Fixture { - val logger = mock() - - fun getSut() = TraceContext.TraceContextUser( - "c052c566-6619-45f5-a61f-172802afa39a", - "f7d8662b-5551-4ef8-b6a8-090f0561a530" - ) - } - private val fixture = Fixture() - - @Test - fun serialize() { - val expected = sanitizedFile("json/trace_state_user.json") - val actual = serialize(fixture.getSut()) - assertEquals(expected, actual) - } - - @Test - fun deserialize() { - val expectedJson = sanitizedFile("json/trace_state_user.json") - val actual = deserialize(expectedJson) - val actualJson = serialize(actual) - assertEquals(expectedJson, actualJson) - } - - // Helper - - private fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } - - private fun serialize(jsonSerializable: JsonSerializable): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - jsonSerializable.serialize(jsonWrt, fixture.logger) - return wrt.toString() - } - - private fun deserialize(json: String): TraceContext.TraceContextUser { - val reader = JsonObjectReader(StringReader(json)) - return TraceContext.TraceContextUser.Deserializer().deserialize(reader, fixture.logger) - } -} diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 595d853181..29d2f69eb7 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -4,7 +4,9 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import java.security.SecureRandom import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue class TracesSamplerTest { @@ -30,25 +32,43 @@ class TracesSamplerTest { @Test fun `when tracesSampleRate is set and random returns greater number returns false`() { val sampler = fixture.getSut(randomResult = 0.9, tracesSampleRate = 0.2) - assertFalse(sampler.sample(SamplingContext(TransactionContext("name", "op"), null))) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + assertFalse(samplingDecision.sampled) + assertEquals(0.2, samplingDecision.sampleRate) } @Test fun `when tracesSampleRate is set and random returns lower number returns true`() { val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 0.2) - assertTrue(sampler.sample(SamplingContext(TransactionContext("name", "op"), null))) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + assertTrue(samplingDecision.sampled) + assertEquals(0.2, samplingDecision.sampleRate) } @Test fun `when tracesSampleRate is not set, tracesSampler is set and random returns lower number returns false`() { val sampler = fixture.getSut(randomResult = 0.1, tracesSamplerResult = 0.2) - assertTrue(sampler.sample(SamplingContext(TransactionContext("name", "op"), CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + TransactionContext("name", "op"), + CustomSamplingContext() + ) + ) + assertTrue(samplingDecision.sampled) + assertEquals(0.2, samplingDecision.sampleRate) } @Test fun `when tracesSampleRate is not set, tracesSampler is set and random returns greater number returns false`() { val sampler = fixture.getSut(randomResult = 0.9, tracesSamplerResult = 0.2) - assertFalse(sampler.sample(SamplingContext(TransactionContext("name", "op"), CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + TransactionContext("name", "op"), + CustomSamplingContext() + ) + ) + assertFalse(samplingDecision.sampled) + assertEquals(0.2, samplingDecision.sampleRate) } @Test @@ -56,19 +76,40 @@ class TracesSamplerTest { val sampler = fixture.getSut(tracesSamplerResult = null) val transactionContextParentSampled = TransactionContext("name", "op") transactionContextParentSampled.parentSampled = true - assertTrue(sampler.sample(SamplingContext(transactionContextParentSampled, CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + transactionContextParentSampled, + CustomSamplingContext() + ) + ) + assertTrue(samplingDecision.sampled) + assertNull(samplingDecision.sampleRate) } @Test fun `when tracesSampler returns null and tracesSampleRate is set sampler uses it as a sampling decision`() { val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 0.2, tracesSamplerResult = null) - assertTrue(sampler.sample(SamplingContext(TransactionContext("name", "op"), CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + TransactionContext("name", "op"), + CustomSamplingContext() + ) + ) + assertTrue(samplingDecision.sampled) + assertEquals(0.2, samplingDecision.sampleRate) } @Test fun `when tracesSampleRate is not set, and tracesSampler is not set returns false`() { val sampler = fixture.getSut(randomResult = 0.1) - assertFalse(sampler.sample(SamplingContext(TransactionContext("name", "op"), CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + TransactionContext("name", "op"), + CustomSamplingContext() + ) + ) + assertFalse(samplingDecision.sampled) + assertNull(samplingDecision.sampleRate) } @Test @@ -76,10 +117,25 @@ class TracesSamplerTest { val sampler = fixture.getSut() val transactionContextParentNotSampled = TransactionContext("name", "op") transactionContextParentNotSampled.parentSampled = false - assertFalse(sampler.sample(SamplingContext(transactionContextParentNotSampled, CustomSamplingContext()))) + val samplingDecision = sampler.sample( + SamplingContext( + transactionContextParentNotSampled, + CustomSamplingContext() + ) + ) + assertFalse(samplingDecision.sampled) + assertNull(samplingDecision.sampleRate) + val transactionContextParentSampled = TransactionContext("name", "op") transactionContextParentSampled.parentSampled = true - assertTrue(sampler.sample(SamplingContext(transactionContextParentSampled, CustomSamplingContext()))) + val samplingDecisionParentSampled = sampler.sample( + SamplingContext( + transactionContextParentSampled, + CustomSamplingContext() + ) + ) + assertTrue(samplingDecisionParentSampled.sampled) + assertNull(samplingDecisionParentSampled.sampleRate) } @Test @@ -87,9 +143,16 @@ class TracesSamplerTest { val sampler = fixture.getSut() val transactionContextNotSampled = TransactionContext("name", "op") transactionContextNotSampled.sampled = false - assertFalse(sampler.sample(SamplingContext(transactionContextNotSampled, CustomSamplingContext()))) + val samplingDecision = + sampler.sample(SamplingContext(transactionContextNotSampled, CustomSamplingContext())) + assertFalse(samplingDecision.sampled) + assertNull(samplingDecision.sampleRate) + val transactionContextSampled = TransactionContext("name", "op") transactionContextSampled.sampled = true - assertTrue(sampler.sample(SamplingContext(transactionContextSampled, CustomSamplingContext()))) + val samplingDecisionContextSampled = + sampler.sample(SamplingContext(transactionContextSampled, CustomSamplingContext())) + assertTrue(samplingDecisionContextSampled.sampled) + assertNull(samplingDecisionContextSampled.sampleRate) } } diff --git a/sentry/src/test/java/io/sentry/protocol/SpanContextSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/SpanContextSerializationTest.kt index 7ca26d1b2c..9d81d93f7c 100644 --- a/sentry/src/test/java/io/sentry/protocol/SpanContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/SpanContextSerializationTest.kt @@ -9,6 +9,7 @@ import io.sentry.JsonSerializable import io.sentry.SpanContext import io.sentry.SpanId import io.sentry.SpanStatus +import io.sentry.TracesSamplingDecision import org.junit.Test import java.io.StringReader import java.io.StringWriter @@ -26,7 +27,7 @@ class SpanContextSerializationTest { SpanId("bf6b582d-8ce3-412b-a334-f4c5539b9602"), "e481581d-35a4-4e97-8a1c-b554bf49f23e", SpanId("c7500f2a-d4e6-4f5f-a0f4-6bb67e98d5a2"), - false + TracesSamplingDecision(false) ).apply { description = "c204b6c7-9753-4d45-927d-b19789bfc9a5" status = SpanStatus.RESOURCE_EXHAUSTED diff --git a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt new file mode 100644 index 0000000000..5cb4268152 --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt @@ -0,0 +1,103 @@ +package io.sentry.util + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class SampleRateUtilTest { + + @Test + fun `accepts 0 dot 01 for sample rate`() { + assertTrue(SampleRateUtil.isValidSampleRate(0.01)) + } + + @Test + fun `accepts 1 for sample rate`() { + assertTrue(SampleRateUtil.isValidSampleRate(1.0)) + } + + @Test + fun `rejects 0 for sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(0.0)) + } + + @Test + fun `rejects 1 dot 01 for sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(1.01)) + } + + @Test + fun `rejects negative sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(-0.5)) + } + + @Test + fun `rejects NaN sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(Double.NaN)) + } + + @Test + fun `rejects positive infinite sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(Double.POSITIVE_INFINITY)) + } + + @Test + fun `rejects negative infinite sample rate`() { + assertFalse(SampleRateUtil.isValidSampleRate(Double.NEGATIVE_INFINITY)) + } + + @Test + fun `accepts null sample rate if told so`() { + assertTrue(SampleRateUtil.isValidSampleRate(null, true)) + } + + @Test + fun `rejects null sample rate if told so`() { + assertFalse(SampleRateUtil.isValidSampleRate(null, false)) + } + + @Test + fun `accepts 0 for traces sample rate`() { + assertTrue(SampleRateUtil.isValidTracesSampleRate(0.0)) + } + + @Test + fun `accepts 1 for traces sample rate`() { + assertTrue(SampleRateUtil.isValidTracesSampleRate(1.0)) + } + + @Test + fun `rejects negative traces sample rate`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(-0.5)) + } + + @Test + fun `rejects 1 dot 01 for traces sample rate`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(1.01)) + } + + @Test + fun `rejects NaN traces sample rate`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.NaN)) + } + + @Test + fun `rejects positive infinite traces sample rate`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.POSITIVE_INFINITY)) + } + + @Test + fun `rejects negative infinite traces sample rate`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.NEGATIVE_INFINITY)) + } + + @Test + fun `accepts null traces sample rate if told so`() { + assertTrue(SampleRateUtil.isValidTracesSampleRate(null, true)) + } + + @Test + fun `rejects null traces sample rate if told so`() { + assertFalse(SampleRateUtil.isValidTracesSampleRate(null, false)) + } +} diff --git a/sentry/src/test/resources/envelope-transaction-with-sample-rate.txt b/sentry/src/test/resources/envelope-transaction-with-sample-rate.txt new file mode 100644 index 0000000000..b790648e06 --- /dev/null +++ b/sentry/src/test/resources/envelope-transaction-with-sample-rate.txt @@ -0,0 +1,3 @@ +{"event_id":"3367f5196c494acaae85bbbd535379ac","trace":{"trace_id":"b156a475de54423d9c1571df97ec7eb6","public_key":"key","release":"1.0-beta.1","environment":"prod","user_id":"usr1","user_segment":"pro","transaction":"tx1","sample_rate":"0.00000021"}} +{"type":"transaction","length":640,"content_type":"application/json"} +{"transaction":"a-transaction","type":"transaction","start_timestamp":"2020-10-23T10:24:01.791Z","timestamp":"2020-10-23T10:24:02.791Z","event_id":"3367f5196c494acaae85bbbd535379ac","contexts":{"trace":{"trace_id":"b156a475de54423d9c1571df97ec7eb6","span_id":"0a53026963414893","op":"http","status":"ok"},"custom":{"some-key":"some-value"}},"spans":[{"start_timestamp":"2021-03-05T08:51:12.838Z","timestamp":"2021-03-05T08:51:12.949Z","trace_id":"2b099185293344a5bfdd7ad89ebf9416","span_id":"5b95c29a5ded4281","parent_span_id":"a3b2d1d58b344b07","op":"PersonService.create","description":"desc","status":"aborted","tags":{"name":"value"}}]} diff --git a/sentry/src/test/resources/json/sentry_envelope_header.json b/sentry/src/test/resources/json/sentry_envelope_header.json index 9c42c8a260..3cc31691f2 100644 --- a/sentry/src/test/resources/json/sentry_envelope_header.json +++ b/sentry/src/test/resources/json/sentry_envelope_header.json @@ -23,11 +23,9 @@ "public_key": "5d593cac-f833-4845-bb23-4eabdf720da2", "release": "9ee2c92c-401e-4296-b6f0-fb3b13edd9ee", "environment": "0666ab02-6364-4135-aa59-02e8128ce052", - "user": - { - "id": "c052c566-6619-45f5-a61f-172802afa39a", - "segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530" - }, - "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e" + "user_id": "c052c566-6619-45f5-a61f-172802afa39a", + "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", + "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", + "sample_rate": "0.00000021" } -} \ No newline at end of file +} diff --git a/sentry/src/test/resources/json/trace_state.json b/sentry/src/test/resources/json/trace_state.json index 8a5f837447..ff4a4f86d9 100644 --- a/sentry/src/test/resources/json/trace_state.json +++ b/sentry/src/test/resources/json/trace_state.json @@ -3,10 +3,8 @@ "public_key": "5d593cac-f833-4845-bb23-4eabdf720da2", "release": "9ee2c92c-401e-4296-b6f0-fb3b13edd9ee", "environment": "0666ab02-6364-4135-aa59-02e8128ce052", - "user": - { - "id": "c052c566-6619-45f5-a61f-172802afa39a", - "segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530" - }, - "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e" + "user_id": "c052c566-6619-45f5-a61f-172802afa39a", + "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", + "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", + "sample_rate": "0.00000021" } diff --git a/sentry/src/test/resources/json/trace_state_legacy.json b/sentry/src/test/resources/json/trace_state_legacy.json new file mode 100644 index 0000000000..14f4f904a6 --- /dev/null +++ b/sentry/src/test/resources/json/trace_state_legacy.json @@ -0,0 +1,12 @@ +{ + "trace_id": "65bcd18546c942069ed957b15b4ace7c", + "public_key": "5d593cac-f833-4845-bb23-4eabdf720da2", + "release": "9ee2c92c-401e-4296-b6f0-fb3b13edd9ee", + "environment": "0666ab02-6364-4135-aa59-02e8128ce052", + "user": + { + "id": "c052c566-6619-45f5-a61f-172802afa39a", + "segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530" + }, + "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e" +} diff --git a/sentry/src/test/resources/json/trace_state_no_sample_rate.json b/sentry/src/test/resources/json/trace_state_no_sample_rate.json new file mode 100644 index 0000000000..538dc61671 --- /dev/null +++ b/sentry/src/test/resources/json/trace_state_no_sample_rate.json @@ -0,0 +1,9 @@ +{ + "trace_id": "65bcd18546c942069ed957b15b4ace7c", + "public_key": "5d593cac-f833-4845-bb23-4eabdf720da2", + "release": "9ee2c92c-401e-4296-b6f0-fb3b13edd9ee", + "environment": "0666ab02-6364-4135-aa59-02e8128ce052", + "user_id": "c052c566-6619-45f5-a61f-172802afa39a", + "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", + "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e" +} diff --git a/sentry/src/test/resources/json/trace_state_user.json b/sentry/src/test/resources/json/trace_state_user.json deleted file mode 100644 index e2b4ec9451..0000000000 --- a/sentry/src/test/resources/json/trace_state_user.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "id": "c052c566-6619-45f5-a61f-172802afa39a", - "segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530" -}