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 30a78d411d5..4250419e2b9 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 419c13cedd8..4b0760fd17f 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 3f87cf9f236..db727aa1eea 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 8cb5bee0e8c..a1a4c8941b0 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 dd4fe25cd90..7341034af69 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -474,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 @@ -661,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; @@ -1387,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; @@ -1489,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; @@ -1521,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; @@ -1538,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 @@ -1644,11 +1650,19 @@ public final class io/sentry/TraceContext$JsonKeys { 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/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 { public fun ()V public static fun contain (Ljava/util/List;Ljava/lang/String;)Z @@ -1657,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 } @@ -2809,6 +2824,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; @@ -3018,6 +3034,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/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 5351c9d2b4e..48435c7db00 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 2718f8988e5..4ce3e773eb5 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 0c415449bbf..8e4b6e0410f 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 51049b6d4c0..902e76bfb04 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 ea07b7bc9a6..7892d927ae8 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 e941dc7ba53..a05d37c5793 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 5207adea919..1f5889c6697 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 7efce08c304..d879510b841 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 55ab322e02c..83c2fda212e 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -2,6 +2,7 @@ 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; @@ -53,7 +54,8 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { 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(), @@ -62,7 +64,7 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { user != null ? user.getId() : null, user != null ? getSegment(user) : null, transaction.getName(), - sampleRateToString(sentryOptions.getTracesSampleRate())); + sampleRateToString(sampleRate(samplingDecision))); } private static @Nullable String getSegment(final @NotNull User user) { @@ -74,23 +76,21 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { } } - private static @Nullable String sampleRateToString(@Nullable Double sampleRateAsDouble) { - if (sampleRateAsDouble == null) { + private static @Nullable Double sampleRate(@Nullable TracesSamplingDecision samplingDecision) { + if (samplingDecision == null) { return null; } - if (sampleRateAsDouble == null - || sampleRateAsDouble.isNaN() - || sampleRateAsDouble.isInfinite()) { - return null; - } + return samplingDecision.getSampleRate(); + } - if (sampleRateAsDouble < 0.0 || sampleRateAsDouble > 1.0) { + private static @Nullable String sampleRateToString(@Nullable Double sampleRateAsDouble) { + if (!SampleRateUtil.isValidTracesSampleRate(sampleRateAsDouble, false)) { return null; } DecimalFormat df = - new DecimalFormat("#.################", DecimalFormatSymbols.getInstance(Locale.US)); + new DecimalFormat("#.################", DecimalFormatSymbols.getInstance(Locale.ROOT)); return df.format(sampleRateAsDouble); } @@ -141,6 +141,82 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return baggage; } + /** @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; + + private TraceContextUser(final @Nullable String id, final @Nullable String segment) { + this.id = id; + this.segment = segment; + } + + public @Nullable String getId() { + return id; + } + + public @Nullable String getSegment() { + return segment; + } + + // region json + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public static final class JsonKeys { + public static final String ID = "id"; + public static final String SEGMENT = "segment"; + } + + public static final class Deserializer implements JsonDeserializer { + @Override + public @NotNull TraceContextUser deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + reader.beginObject(); + + String id = null; + String segment = null; + Map unknown = null; + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case TraceContextUser.JsonKeys.ID: + id = reader.nextStringOrNull(); + break; + case TraceContextUser.JsonKeys.SEGMENT: + segment = reader.nextStringOrNull(); + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + TraceContextUser traceStateUser = new TraceContextUser(id, segment); + traceStateUser.setUnknown(unknown); + reader.endObject(); + return traceStateUser; + } + } + + // endregion + } + // region json @Nullable @@ -159,6 +235,7 @@ public static final class JsonKeys { public static final String PUBLIC_KEY = "public_key"; 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"; @@ -209,6 +286,7 @@ public static final class Deserializer implements JsonDeserializer String publicKey = null; String release = null; String environment = null; + TraceContextUser user = null; String userId = null; String userSegment = null; String transaction = null; @@ -230,6 +308,9 @@ public static final class Deserializer implements JsonDeserializer case TraceContext.JsonKeys.ENVIRONMENT: environment = reader.nextStringOrNull(); break; + case TraceContext.JsonKeys.USER: + user = reader.nextOrNull(logger, new TraceContextUser.Deserializer()); + break; case TraceContext.JsonKeys.USER_ID: userId = reader.nextStringOrNull(); break; @@ -256,6 +337,14 @@ public static final class Deserializer implements JsonDeserializer if (publicKey == null) { throw missingRequiredFieldException(TraceContext.JsonKeys.PUBLIC_KEY, logger); } + if (user != null) { + if (userId == null) { + userId = user.getId(); + } + if (userSegment == null) { + userSegment = user.getSegment(); + } + } TraceContext traceContext = new TraceContext( traceId, diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 9f14ec95028..c169c18faf7 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 00000000000..3e8d895fb55 --- /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 25296713b75..4b7c1f8da1b 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 fecd9072f85..f2c6f318981 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 00000000000..0035a240be8 --- /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 7358b6de50d..1ced5f9fd36 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -200,7 +200,7 @@ class BaggageTest { baggage.setUserSegment("segmentA") baggage.setSampleRate((1.0 / 3.0).toString()) - assertEquals("sentry-environment=production,sentry-publickey=$publicKey,sentry-release=1.0-rc.1,sentry-samplerate=0.3333333333333333,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 @@ -211,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 5f64108b6ff..5ab9f53b031 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/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 3f14f3cf5cf..4b23fdea90a 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 fe26c5ab748..527a2c9af08 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) @@ -539,19 +539,19 @@ 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())) + assertTrue(it.value.contains("sentry-user_id=userId12345,")) + assertTrue(it.value.contains("sentry-user_segment=pro$".toRegex())) } } @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( @@ -566,7 +566,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() @@ -666,7 +666,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() @@ -692,19 +692,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 a7e1931d4df..da090abe68b 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 30cf6d262e2..ffe43fa5246 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -62,10 +62,20 @@ class TraceContextSerializationTest { 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/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 595d853181b..29d2f69eb75 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 7ca26d1b2cb..9d81d93f7c0 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 00000000000..5cb42681521 --- /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 00000000000..b790648e064 --- /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/trace_state_legacy.json b/sentry/src/test/resources/json/trace_state_legacy.json new file mode 100644 index 00000000000..14f4f904a6d --- /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 00000000000..538dc616718 --- /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" +}