From dc851689e543dcf525ea1bd52644b650250f49b0 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 12 Feb 2025 17:50:48 +0100 Subject: [PATCH] Propagate sampling random value (#4153) * wip * wip2 * wip3 * format + api * cleanup * revert change to demo * make baggage final again * remove ObjectToString suppressing * remove outdated comment * remove getPropagationContext from Scopes again * remove noop baggage and propagation context again * fix outbox sender; test; cleanup api file * review changes * fix test for old span processor * review feedback * Format code * changelog --------- Co-authored-by: Sentry Github Bot --- CHANGELOG.md | 2 + .../api/sentry-opentelemetry-bootstrap.api | 1 + .../InternalSemanticAttributes.java | 2 + .../sentry/opentelemetry/OtelSpanFactory.java | 2 + .../opentelemetry/OtelSamplingUtil.java | 7 +- .../opentelemetry/OtelSentryPropagator.java | 24 +-- .../OtelSentrySpanProcessor.java | 7 +- .../sentry/opentelemetry/OtelSpanWrapper.java | 24 ++- .../sentry/opentelemetry/SentrySampler.java | 3 +- .../opentelemetry/SentrySamplingResult.java | 1 + .../opentelemetry/SentrySpanExporter.java | 1 + .../test/kotlin/SentrySpanProcessorTest.kt | 6 +- sentry/api/sentry.api | 21 ++- sentry/src/main/java/io/sentry/Baggage.java | 82 ++++++++-- .../java/io/sentry/CombinedScopeView.java | 1 + .../src/main/java/io/sentry/OutboxSender.java | 11 +- .../java/io/sentry/PropagationContext.java | 27 ++-- .../main/java/io/sentry/SamplingContext.java | 21 +++ sentry/src/main/java/io/sentry/Scopes.java | 21 ++- sentry/src/main/java/io/sentry/Sentry.java | 4 +- .../src/main/java/io/sentry/SentryTracer.java | 30 ++-- .../src/main/java/io/sentry/TraceContext.java | 46 +++++- .../main/java/io/sentry/TracesSampler.java | 42 ++--- .../io/sentry/TracesSamplingDecision.java | 24 ++- .../java/io/sentry/TransactionContext.java | 30 ++-- .../java/io/sentry/util/SampleRateUtils.java | 38 +++++ .../java/io/sentry/util/TracingUtils.java | 79 ++++++++-- sentry/src/test/java/io/sentry/BaggageTest.kt | 61 +++++++- .../test/java/io/sentry/JsonSerializerTest.kt | 8 +- .../test/java/io/sentry/OutboxSenderTest.kt | 59 +++++++ .../java/io/sentry/PropagationContextTest.kt | 43 +++++ .../sentry/TraceContextSerializationTest.kt | 3 +- .../test/java/io/sentry/TracesSamplerTest.kt | 119 ++++++++------ .../java/io/sentry/TransactionContextTest.kt | 30 ++++ .../java/io/sentry/util/SampleRateUtilTest.kt | 61 ++++++++ .../java/io/sentry/util/TracingUtilsTest.kt | 148 ++++++++++++++++-- .../envelope-transaction-with-sample-rand.txt | 3 + .../json/sentry_envelope_header.json | 1 + .../src/test/resources/json/trace_state.json | 1 + 39 files changed, 881 insertions(+), 213 deletions(-) create mode 100644 sentry/src/test/java/io/sentry/PropagationContextTest.kt create mode 100644 sentry/src/test/resources/envelope-transaction-with-sample-rand.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index d2254534c9..f681818c9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ - Expose new `withSentryObservableEffect` method overload that accepts `SentryNavigationListener` as a parameter ([#4143](https://github.com/getsentry/sentry-java/pull/4143)) - This allows sharing the same `SentryNavigationListener` instance across fragments and composables to preserve the trace - (Internal) Add API to filter native debug images based on stacktrace addresses ([#4089](https://github.com/getsentry/sentry-java/pull/4089)) +- Propagate sampling random value ([#4153](https://github.com/getsentry/sentry-java/pull/4153)) + - The random value used for sampling traces is now sent to Sentry and attached to the `baggage` header on outgoing requests ### Fixes diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api index df7db47c65..adb976adc0 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/api/sentry-opentelemetry-bootstrap.api @@ -20,6 +20,7 @@ public final class io/sentry/opentelemetry/InternalSemanticAttributes { public static final field PROFILE_SAMPLED Lio/opentelemetry/api/common/AttributeKey; public static final field PROFILE_SAMPLE_RATE Lio/opentelemetry/api/common/AttributeKey; public static final field SAMPLED Lio/opentelemetry/api/common/AttributeKey; + public static final field SAMPLE_RAND Lio/opentelemetry/api/common/AttributeKey; public static final field SAMPLE_RATE Lio/opentelemetry/api/common/AttributeKey; public fun ()V } diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/InternalSemanticAttributes.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/InternalSemanticAttributes.java index cb64d7bfff..4795401266 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/InternalSemanticAttributes.java +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/InternalSemanticAttributes.java @@ -8,6 +8,8 @@ public final class InternalSemanticAttributes { public static final AttributeKey SAMPLED = AttributeKey.booleanKey("sentry.sampled"); public static final AttributeKey SAMPLE_RATE = AttributeKey.doubleKey("sentry.sample_rate"); + public static final AttributeKey SAMPLE_RAND = + AttributeKey.doubleKey("sentry.sample_rand"); public static final AttributeKey PARENT_SAMPLED = AttributeKey.booleanKey("sentry.parent_sampled"); public static final AttributeKey PROFILE_SAMPLED = diff --git a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanFactory.java b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanFactory.java index 547463dcdc..7a51c3f337 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanFactory.java +++ b/sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanFactory.java @@ -136,6 +136,8 @@ public OtelSpanFactory() { spanBuilder.setAttribute(InternalSemanticAttributes.SAMPLED, samplingDecision.getSampled()); spanBuilder.setAttribute( InternalSemanticAttributes.SAMPLE_RATE, samplingDecision.getSampleRate()); + spanBuilder.setAttribute( + InternalSemanticAttributes.SAMPLE_RAND, samplingDecision.getSampleRand()); spanBuilder.setAttribute( InternalSemanticAttributes.PROFILE_SAMPLED, samplingDecision.getProfileSampled()); spanBuilder.setAttribute( diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSamplingUtil.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSamplingUtil.java index 01f414f902..324f06b539 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSamplingUtil.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSamplingUtil.java @@ -14,13 +14,18 @@ public final class OtelSamplingUtil { final @Nullable Boolean sampled = attributes.get(InternalSemanticAttributes.SAMPLED); if (sampled != null) { final @Nullable Double sampleRate = attributes.get(InternalSemanticAttributes.SAMPLE_RATE); + final @Nullable Double sampleRand = attributes.get(InternalSemanticAttributes.SAMPLE_RAND); final @Nullable Boolean profileSampled = attributes.get(InternalSemanticAttributes.PROFILE_SAMPLED); final @Nullable Double profileSampleRate = attributes.get(InternalSemanticAttributes.PROFILE_SAMPLE_RATE); return new TracesSamplingDecision( - sampled, sampleRate, profileSampled == null ? false : profileSampled, profileSampleRate); + sampled, + sampleRate, + sampleRand, + profileSampled == null ? false : profileSampled, + profileSampleRate); } else { return null; } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java index c5802df245..fc2e3d426b 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java @@ -13,12 +13,12 @@ import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.IScopes; -import io.sentry.PropagationContext; import io.sentry.ScopesAdapter; import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.exception.InvalidSentryTraceHeaderException; +import io.sentry.util.TracingUtils; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -73,12 +73,17 @@ public void inject(final Context context, final C carrier, final TextMapSett return; } - final @NotNull SentryTraceHeader sentryTraceHeader = sentrySpan.toSentryTrace(); - setter.set(carrier, sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - final @Nullable BaggageHeader baggageHeader = - sentrySpan.toBaggageHeader(Collections.emptyList()); - if (baggageHeader != null) { - setter.set(carrier, baggageHeader.getName(), baggageHeader.getValue()); + // TODO can we use traceIfAllowed? do we have the URL here? need to access span attrs + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.trace(scopes, Collections.emptyList(), sentrySpan); + + if (tracingHeaders != null) { + final @NotNull SentryTraceHeader sentryTraceHeader = tracingHeaders.getSentryTraceHeader(); + setter.set(carrier, sentryTraceHeader.getName(), sentryTraceHeader.getValue()); + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + setter.set(carrier, baggageHeader.getName(), baggageHeader.getValue()); + } } } @@ -125,11 +130,6 @@ public Context extract( .getLogger() .log(SentryLevel.DEBUG, "Continuing Sentry trace %s", sentryTraceHeader.getTraceId()); - final @NotNull PropagationContext propagationContext = - PropagationContext.fromHeaders( - scopes.getOptions().getLogger(), sentryTraceString, baggageString); - scopesToUse.getIsolationScope().setPropagationContext(propagationContext); - return modifiedContext; } catch (InvalidSentryTraceHeaderException e) { scopes diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java index 521bc9020c..37293569ae 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java @@ -70,15 +70,10 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri baggage = baggageFromContext; } - final @Nullable Boolean baggageMutable = - otelSpan.getAttribute(InternalSemanticAttributes.BAGGAGE_MUTABLE); final @Nullable String baggageString = otelSpan.getAttribute(InternalSemanticAttributes.BAGGAGE); if (baggageString != null) { baggage = Baggage.fromHeader(baggageString); - if (baggageMutable == true) { - baggage.freeze(); - } } final @Nullable Boolean sampled = isSampled(otelSpan, samplingDecision); @@ -87,6 +82,8 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri new PropagationContext( new SentryId(traceId), sentrySpanId, sentryParentSpanId, baggage, sampled); + baggage = propagationContext.getBaggage(); + updatePropagationContext(scopes, propagationContext); } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java index 090d317485..34f2d2a4d7 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java @@ -64,7 +64,6 @@ public final class OtelSpanWrapper implements IOtelSpanWrapper { private final @NotNull Contexts contexts = new Contexts(); private @Nullable String transactionName; private @Nullable TransactionNameSource transactionNameSource; - private final @Nullable Baggage baggage; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); private final @NotNull Map data = new ConcurrentHashMap<>(); @@ -86,17 +85,12 @@ public OtelSpanWrapper( this.scopes = Objects.requireNonNull(scopes, "scopes are required"); this.span = new WeakReference<>(span); this.startTimestamp = startTimestamp; - - if (parentSpan != null) { - this.baggage = parentSpan.getSpanContext().getBaggage(); - } else if (baggage != null) { - this.baggage = baggage; - } else { - this.baggage = null; - } - + final @Nullable Baggage baggageToUse = + baggage != null + ? baggage + : (parentSpan != null ? parentSpan.getSpanContext().getBaggage() : null); this.context = - new OtelSpanContext(span, samplingDecision, parentSpan, parentSpanId, this.baggage); + new OtelSpanContext(span, samplingDecision, parentSpan, parentSpanId, baggageToUse); } @Override @@ -207,15 +201,16 @@ public OtelSpanWrapper( @Override public @Nullable TraceContext traceContext() { if (scopes.getOptions().isTraceSampling()) { + final @Nullable Baggage baggage = context.getBaggage(); if (baggage != null) { - updateBaggageValues(); + updateBaggageValues(baggage); return baggage.toTraceContext(); } } return null; } - private void updateBaggageValues() { + private void updateBaggageValues(final @NotNull Baggage baggage) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { if (baggage != null && baggage.isMutable()) { final AtomicReference replayIdAtomicReference = new AtomicReference<>(); @@ -238,8 +233,9 @@ private void updateBaggageValues() { @Override public @Nullable BaggageHeader toBaggageHeader(@Nullable List thirdPartyBaggageHeaders) { if (scopes.getOptions().isTraceSampling()) { + final @Nullable Baggage baggage = context.getBaggage(); if (baggage != null) { - updateBaggageValues(); + updateBaggageValues(baggage); return BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); } } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySampler.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySampler.java index 6f35fcb9c5..8949932129 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySampler.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySampler.java @@ -95,7 +95,8 @@ public SamplingResult shouldSample( scopes .getOptions() .getInternalTracesSampler() - .sample(new SamplingContext(transactionContext, null)); + .sample( + new SamplingContext(transactionContext, null, propagationContext.getSampleRand())); if (!sentryDecision.getSampled()) { scopes diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySamplingResult.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySamplingResult.java index 69acf52134..e29601faf5 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySamplingResult.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySamplingResult.java @@ -29,6 +29,7 @@ public Attributes getAttributes() { return Attributes.builder() .put(InternalSemanticAttributes.SAMPLED, sentryDecision.getSampled()) .put(InternalSemanticAttributes.SAMPLE_RATE, sentryDecision.getSampleRate()) + .put(InternalSemanticAttributes.SAMPLE_RAND, sentryDecision.getSampleRand()) .put(InternalSemanticAttributes.PROFILE_SAMPLED, sentryDecision.getProfileSampled()) .put(InternalSemanticAttributes.PROFILE_SAMPLE_RATE, sentryDecision.getProfileSampleRate()) .build(); diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java index 4d2e7545c6..693b94fe38 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanExporter.java @@ -62,6 +62,7 @@ public final class SentrySpanExporter implements SpanExporter { InternalSemanticAttributes.BAGGAGE_MUTABLE.getKey(), InternalSemanticAttributes.SAMPLED.getKey(), InternalSemanticAttributes.SAMPLE_RATE.getKey(), + InternalSemanticAttributes.SAMPLE_RAND.getKey(), InternalSemanticAttributes.PROFILE_SAMPLED.getKey(), InternalSemanticAttributes.PROFILE_SAMPLE_RATE.getKey(), InternalSemanticAttributes.PARENT_SAMPLED.getKey(), diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySpanProcessorTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySpanProcessorTest.kt index 053f80e537..f7bcd22656 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySpanProcessorTest.kt +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySpanProcessorTest.kt @@ -409,15 +409,15 @@ class SentrySpanProcessorTest { assertEquals("1", it.baggage?.sampleRate) assertEquals("HTTP GET", it.baggage?.transaction) assertEquals("502f25099c204a2fbf4cb16edc5975d1", it.baggage?.publicKey) + assertFalse(it.baggage!!.isMutable) } else { assertNotNull(it.baggage) assertNull(it.baggage?.traceId) assertNull(it.baggage?.sampleRate) assertNull(it.baggage?.transaction) assertNull(it.baggage?.publicKey) - assertFalse(it.baggage!!.isMutable) + assertTrue(it.baggage!!.isMutable) } - assertFalse(it.baggage!!.isMutable) }, check { assertNotNull(it.startTimestamp) @@ -434,7 +434,7 @@ class SentrySpanProcessorTest { assertEquals(otelSpan.spanContext.traceId, it.traceId.toString()) assertNull(it.parentSpanId) assertNull(it.parentSamplingDecision) - assertNull(it.baggage) + assertNotNull(it.baggage) }, check { assertNotNull(it.startTimestamp) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 662d87c265..eb63ba38c4 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -32,7 +32,7 @@ public abstract interface class io/sentry/BackfillingEventProcessor : io/sentry/ public final class io/sentry/Baggage { public fun (Lio/sentry/Baggage;)V public fun (Lio/sentry/ILogger;)V - public fun (Ljava/util/Map;Ljava/lang/String;ZLio/sentry/ILogger;)V + public fun (Ljava/util/Map;Ljava/lang/String;ZZLio/sentry/ILogger;)V public fun freeze ()V public static fun fromEvent (Lio/sentry/SentryEvent;Lio/sentry/SentryOptions;)Lio/sentry/Baggage; public static fun fromHeader (Ljava/lang/String;)Lio/sentry/Baggage; @@ -46,6 +46,8 @@ public final class io/sentry/Baggage { public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; public fun getReplayId ()Ljava/lang/String; + public fun getSampleRand ()Ljava/lang/String; + public fun getSampleRandDouble ()Ljava/lang/Double; public fun getSampleRate ()Ljava/lang/String; public fun getSampleRateDouble ()Ljava/lang/Double; public fun getSampled ()Ljava/lang/String; @@ -55,11 +57,14 @@ public final class io/sentry/Baggage { public fun getUnknown ()Ljava/util/Map; public fun getUserId ()Ljava/lang/String; public fun isMutable ()Z + public fun isShouldFreeze ()Z public fun set (Ljava/lang/String;Ljava/lang/String;)V public fun setEnvironment (Ljava/lang/String;)V public fun setPublicKey (Ljava/lang/String;)V public fun setRelease (Ljava/lang/String;)V public fun setReplayId (Ljava/lang/String;)V + public fun setSampleRand (Ljava/lang/String;)V + public fun setSampleRandDouble (Ljava/lang/Double;)V public fun setSampleRate (Ljava/lang/String;)V public fun setSampled (Ljava/lang/String;)V public fun setTraceId (Ljava/lang/String;)V @@ -78,6 +83,7 @@ public final class io/sentry/Baggage$DSCKeys { public static final field RELEASE Ljava/lang/String; public static final field REPLAY_ID Ljava/lang/String; public static final field SAMPLED Ljava/lang/String; + public static final field SAMPLE_RAND 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; @@ -1947,10 +1953,10 @@ public final class io/sentry/PropagationContext { public static fun fromHeaders (Lio/sentry/SentryTraceHeader;Lio/sentry/Baggage;Lio/sentry/SpanId;)Lio/sentry/PropagationContext; public fun getBaggage ()Lio/sentry/Baggage; public fun getParentSpanId ()Lio/sentry/SpanId; + public fun getSampleRand ()Ljava/lang/Double; public fun getSpanId ()Lio/sentry/SpanId; public fun getTraceId ()Lio/sentry/protocol/SentryId; public fun isSampled ()Ljava/lang/Boolean; - public fun setBaggage (Lio/sentry/Baggage;)V public fun setParentSpanId (Lio/sentry/SpanId;)V public fun setSampled (Ljava/lang/Boolean;)V public fun setSpanId (Lio/sentry/SpanId;)V @@ -2007,7 +2013,9 @@ public final class io/sentry/RequestDetails { public final class io/sentry/SamplingContext { public fun (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;)V + public fun (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Ljava/lang/Double;)V public fun getCustomSamplingContext ()Lio/sentry/CustomSamplingContext; + public fun getSampleRand ()Ljava/lang/Double; public fun getTransactionContext ()Lio/sentry/TransactionContext; } @@ -3634,6 +3642,7 @@ public final class io/sentry/TraceContext : io/sentry/JsonSerializable, io/sentr public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; public fun getReplayId ()Lio/sentry/protocol/SentryId; + public fun getSampleRand ()Ljava/lang/String; public fun getSampleRate ()Ljava/lang/String; public fun getSampled ()Ljava/lang/String; public fun getTraceId ()Lio/sentry/protocol/SentryId; @@ -3656,6 +3665,7 @@ public final class io/sentry/TraceContext$JsonKeys { public static final field RELEASE Ljava/lang/String; public static final field REPLAY_ID Ljava/lang/String; public static final field SAMPLED Ljava/lang/String; + public static final field SAMPLE_RAND 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; @@ -3672,8 +3682,11 @@ public final class io/sentry/TracesSamplingDecision { public fun (Ljava/lang/Boolean;)V public fun (Ljava/lang/Boolean;Ljava/lang/Double;)V public fun (Ljava/lang/Boolean;Ljava/lang/Double;Ljava/lang/Boolean;Ljava/lang/Double;)V + public fun (Ljava/lang/Boolean;Ljava/lang/Double;Ljava/lang/Double;)V + public fun (Ljava/lang/Boolean;Ljava/lang/Double;Ljava/lang/Double;Ljava/lang/Boolean;Ljava/lang/Double;)V public fun getProfileSampleRate ()Ljava/lang/Double; public fun getProfileSampled ()Ljava/lang/Boolean; + public fun getSampleRand ()Ljava/lang/Double; public fun getSampleRate ()Ljava/lang/Double; public fun getSampled ()Ljava/lang/Boolean; } @@ -6275,6 +6288,8 @@ public final class io/sentry/util/Random : java/io/Serializable { public final class io/sentry/util/SampleRateUtils { public fun ()V + public static fun backfilledSampleRand (Lio/sentry/TracesSamplingDecision;)Lio/sentry/TracesSamplingDecision; + public static fun backfilledSampleRand (Ljava/lang/Double;Ljava/lang/Double;Ljava/lang/Boolean;)Ljava/lang/Double; public static fun isValidProfilesSampleRate (Ljava/lang/Double;)Z public static fun isValidSampleRate (Ljava/lang/Double;)Z public static fun isValidTracesSampleRate (Ljava/lang/Double;)Z @@ -6310,6 +6325,8 @@ public final class io/sentry/util/StringUtils { public final class io/sentry/util/TracingUtils { public fun ()V + public static fun ensureBaggage (Lio/sentry/Baggage;Lio/sentry/TracesSamplingDecision;)Lio/sentry/Baggage; + public static fun ensureBaggage (Lio/sentry/Baggage;Ljava/lang/Boolean;Ljava/lang/Double;Ljava/lang/Double;)Lio/sentry/Baggage; public static fun isIgnored (Ljava/util/List;Ljava/lang/String;)Z public static fun maybeUpdateBaggage (Lio/sentry/IScope;Lio/sentry/SentryOptions;)Lio/sentry/PropagationContext; public static fun startNewTrace (Lio/sentry/IScopes;)V diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 05a59d7053..71149a81da 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -35,6 +35,7 @@ public final class Baggage { final @NotNull Map keyValues; final @Nullable String thirdPartyHeader; private boolean mutable; + private boolean shouldFreeze; final @NotNull ILogger logger; @NotNull @@ -85,7 +86,7 @@ public static Baggage fromHeader( final @NotNull ILogger logger) { final @NotNull Map keyValues = new HashMap<>(); final @NotNull List thirdPartyKeyValueStrings = new ArrayList<>(); - boolean mutable = true; + boolean shouldFreeze = false; if (headerValue != null) { try { @@ -103,7 +104,20 @@ public static Baggage fromHeader( final String valueDecoded = decode(value); keyValues.put(keyDecoded, valueDecoded); - mutable = false; + + // Without ignoring SAMPLE_RAND here, we'd be freezing baggage that we're transporting + // via OTel span attributes. + // This is done when a transaction is created via Sentry API. + // In that case Baggage is created before the OTel span is created and we put it on + // the span attributes. + // It does however only contain the sample random value as its only value. + // The OTel code then uses it to create a propagation context from it and ends up + // freezing it, + // preventing outgoing requests (to other systems or Sentry) from adding info to + // baggage and only then freeze it. + if (!DSCKeys.SAMPLE_RAND.equalsIgnoreCase(key)) { + shouldFreeze = true; + } } catch (Throwable e) { logger.log( SentryLevel.ERROR, @@ -123,7 +137,12 @@ public static Baggage fromHeader( thirdPartyKeyValueStrings.isEmpty() ? null : StringUtils.join(",", thirdPartyKeyValueStrings); - return new Baggage(keyValues, thirdPartyHeader, mutable, logger); + /* + can't freeze Baggage right away as we might have to backfill sampleRand + also we don't receive sentry-trace header here or in ctor so we can't + backfill then freeze here unless we pass sentry-trace header. + */ + return new Baggage(keyValues, thirdPartyHeader, true, shouldFreeze, logger); } @ApiStatus.Internal @@ -140,6 +159,7 @@ public static Baggage fromEvent( // we don't persist sample rate baggage.setSampleRate(null); baggage.setSampled(null); + baggage.setSampleRand(null); final @Nullable Object replayId = event.getContexts().get(REPLAY_ID); if (replayId != null && !replayId.toString().equals(SentryId.EMPTY_ID.toString())) { baggage.setReplayId(replayId.toString()); @@ -152,12 +172,17 @@ public static Baggage fromEvent( @ApiStatus.Internal public Baggage(final @NotNull ILogger logger) { - this(new HashMap<>(), null, true, logger); + this(new HashMap<>(), null, true, false, logger); } @ApiStatus.Internal public Baggage(final @NotNull Baggage baggage) { - this(baggage.keyValues, baggage.thirdPartyHeader, baggage.mutable, baggage.logger); + this( + baggage.keyValues, + baggage.thirdPartyHeader, + baggage.mutable, + baggage.shouldFreeze, + baggage.logger); } @ApiStatus.Internal @@ -165,11 +190,13 @@ public Baggage( final @NotNull Map keyValues, final @Nullable String thirdPartyHeader, boolean isMutable, + boolean shouldFreeze, final @NotNull ILogger logger) { this.keyValues = keyValues; this.logger = logger; - this.mutable = isMutable; this.thirdPartyHeader = thirdPartyHeader; + this.mutable = isMutable; + this.shouldFreeze = shouldFreeze; } @ApiStatus.Internal @@ -182,6 +209,11 @@ public boolean isMutable() { return mutable; } + @ApiStatus.Internal + public boolean isShouldFreeze() { + return shouldFreeze; + } + @Nullable public String getThirdPartyHeader() { return thirdPartyHeader; @@ -335,6 +367,21 @@ public void setSampleRate(final @Nullable String sampleRate) { set(DSCKeys.SAMPLE_RATE, sampleRate); } + @ApiStatus.Internal + public @Nullable String getSampleRand() { + return get(DSCKeys.SAMPLE_RAND); + } + + @ApiStatus.Internal + public void setSampleRand(final @Nullable String sampleRand) { + set(DSCKeys.SAMPLE_RAND, sampleRand); + } + + @ApiStatus.Internal + public void setSampleRandDouble(final @Nullable Double sampleRand) { + setSampleRand(sampleRateToString(sampleRand)); + } + @ApiStatus.Internal public void setSampled(final @Nullable String sampled) { set(DSCKeys.SAMPLED, sampled); @@ -445,12 +492,20 @@ private static boolean isHighQualityTransactionName( @ApiStatus.Internal public @Nullable Double getSampleRateDouble() { - final String sampleRateString = getSampleRate(); - if (sampleRateString != null) { + return toDouble(getSampleRate()); + } + + @ApiStatus.Internal + public @Nullable Double getSampleRandDouble() { + return toDouble(getSampleRand()); + } + + private @Nullable Double toDouble(final @Nullable String stringValue) { + if (stringValue != null) { try { - double sampleRate = Double.parseDouble(sampleRateString); - if (SampleRateUtils.isValidTracesSampleRate(sampleRate, false)) { - return sampleRate; + double doubleValue = Double.parseDouble(stringValue); + if (SampleRateUtils.isValidTracesSampleRate(doubleValue, false)) { + return doubleValue; } } catch (NumberFormatException e) { return null; @@ -477,7 +532,8 @@ public TraceContext toTraceContext() { getTransaction(), getSampleRate(), getSampled(), - replayIdString == null ? null : new SentryId(replayIdString)); + replayIdString == null ? null : new SentryId(replayIdString), + getSampleRand()); traceContext.setUnknown(getUnknown()); return traceContext; } else { @@ -494,6 +550,7 @@ public static final class DSCKeys { public static final String ENVIRONMENT = "sentry-environment"; public static final String TRANSACTION = "sentry-transaction"; public static final String SAMPLE_RATE = "sentry-sample_rate"; + public static final String SAMPLE_RAND = "sentry-sample_rand"; public static final String SAMPLED = "sentry-sampled"; public static final String REPLAY_ID = "sentry-replay_id"; @@ -506,6 +563,7 @@ public static final class DSCKeys { ENVIRONMENT, TRANSACTION, SAMPLE_RATE, + SAMPLE_RAND, SAMPLED, REPLAY_ID); } diff --git a/sentry/src/main/java/io/sentry/CombinedScopeView.java b/sentry/src/main/java/io/sentry/CombinedScopeView.java index 3523afa4d3..129066450f 100644 --- a/sentry/src/main/java/io/sentry/CombinedScopeView.java +++ b/sentry/src/main/java/io/sentry/CombinedScopeView.java @@ -426,6 +426,7 @@ public void setPropagationContext(@NotNull PropagationContext propagationContext getDefaultWriteScope().setPropagationContext(propagationContext); } + @ApiStatus.Internal @Override public @NotNull PropagationContext getPropagationContext() { return getDefaultWriteScope().getPropagationContext(); diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 4e223da03d..cbe4f6ee00 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -244,7 +244,16 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN "Invalid sample rate parsed from TraceContext: %s", sampleRateString); } else { - return new TracesSamplingDecision(true, sampleRate); + final @Nullable String sampleRandString = traceContext.getSampleRand(); + if (sampleRandString != null) { + final Double sampleRand = Double.parseDouble(sampleRandString); + if (SampleRateUtils.isValidTracesSampleRate(sampleRand, false)) { + return new TracesSamplingDecision(true, sampleRate, sampleRand); + } + } + + return SampleRateUtils.backfilledSampleRand( + new TracesSamplingDecision(true, sampleRate)); } } catch (Exception e) { logger.log( diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java index b0debc2a9d..791cb1d3d3 100644 --- a/sentry/src/main/java/io/sentry/PropagationContext.java +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -2,6 +2,7 @@ import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.SentryId; +import io.sentry.util.TracingUtils; import java.util.Arrays; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -56,7 +57,7 @@ public static PropagationContext fromHeaders( private @Nullable Boolean sampled; - private @Nullable Baggage baggage; + private final @NotNull Baggage baggage; public PropagationContext() { this(new SentryId(), new SpanId(), null, null, null); @@ -67,18 +68,10 @@ public PropagationContext(final @NotNull PropagationContext propagationContext) propagationContext.getTraceId(), propagationContext.getSpanId(), propagationContext.getParentSpanId(), - cloneBaggage(propagationContext.getBaggage()), + propagationContext.getBaggage(), propagationContext.isSampled()); } - private static @Nullable Baggage cloneBaggage(final @Nullable Baggage baggage) { - if (baggage != null) { - return new Baggage(baggage); - } - - return null; - } - public PropagationContext( final @NotNull SentryId traceId, final @NotNull SpanId spanId, @@ -88,7 +81,7 @@ public PropagationContext( this.traceId = traceId; this.spanId = spanId; this.parentSpanId = parentSpanId; - this.baggage = baggage; + this.baggage = TracingUtils.ensureBaggage(baggage, sampled, null, null); this.sampled = sampled; } @@ -116,14 +109,10 @@ public void setParentSpanId(final @Nullable SpanId parentSpanId) { this.parentSpanId = parentSpanId; } - public @Nullable Baggage getBaggage() { + public @NotNull Baggage getBaggage() { return baggage; } - public void setBaggage(final @Nullable Baggage baggage) { - this.baggage = baggage; - } - public @Nullable Boolean isSampled() { return sampled; } @@ -145,4 +134,10 @@ public void setSampled(final @Nullable Boolean sampled) { spanContext.setOrigin("auto"); return spanContext; } + + public @NotNull Double getSampleRand() { + final @Nullable Double sampleRand = baggage.getSampleRandDouble(); + // should never be null since we ensure it in ctor + return sampleRand == null ? 0.0 : sampleRand; + } } diff --git a/sentry/src/main/java/io/sentry/SamplingContext.java b/sentry/src/main/java/io/sentry/SamplingContext.java index 60944fbd43..711c03e21c 100644 --- a/sentry/src/main/java/io/sentry/SamplingContext.java +++ b/sentry/src/main/java/io/sentry/SamplingContext.java @@ -1,6 +1,8 @@ package io.sentry; import io.sentry.util.Objects; +import io.sentry.util.SentryRandom; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -11,13 +13,28 @@ public final class SamplingContext { private final @NotNull TransactionContext transactionContext; private final @Nullable CustomSamplingContext customSamplingContext; + private final @NotNull Double sampleRand; + @Deprecated + @SuppressWarnings("InlineMeSuggester") + /** + * @deprecated creating a SamplingContext is something only the SDK should do + */ public SamplingContext( final @NotNull TransactionContext transactionContext, final @Nullable CustomSamplingContext customSamplingContext) { + this(transactionContext, customSamplingContext, SentryRandom.current().nextDouble()); + } + + @ApiStatus.Internal + public SamplingContext( + final @NotNull TransactionContext transactionContext, + final @Nullable CustomSamplingContext customSamplingContext, + final @NotNull Double sampleRand) { this.transactionContext = Objects.requireNonNull(transactionContext, "transactionContexts is required"); this.customSamplingContext = customSamplingContext; + this.sampleRand = sampleRand; } public @Nullable CustomSamplingContext getCustomSamplingContext() { @@ -27,4 +44,8 @@ public SamplingContext( public @NotNull TransactionContext getTransactionContext() { return transactionContext; } + + public @NotNull Double getSampleRand() { + return sampleRand; + } } diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 2b0d510368..14025a1d77 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -857,8 +857,10 @@ public void flush(long timeoutMillis) { SentryLevel.INFO, "Tracing is disabled and this 'startTransaction' returns a no-op."); transaction = NoOpTransaction.getInstance(); } else { + final Double sampleRand = getSampleRand(transactionContext); final SamplingContext samplingContext = - new SamplingContext(transactionContext, transactionOptions.getCustomSamplingContext()); + new SamplingContext( + transactionContext, transactionOptions.getCustomSamplingContext(), sampleRand); final @NotNull TracesSampler tracesSampler = getOptions().getInternalTracesSampler(); @NotNull TracesSamplingDecision samplingDecision = tracesSampler.sample(samplingContext); transactionContext.setSamplingDecision(samplingDecision); @@ -894,6 +896,18 @@ public void flush(long timeoutMillis) { return transaction; } + private @NotNull Double getSampleRand(final @NotNull TransactionContext transactionContext) { + final @Nullable Baggage baggage = transactionContext.getBaggage(); + if (baggage != null) { + final @Nullable Double sampleRandFromBaggageMaybe = baggage.getSampleRandDouble(); + if (sampleRandFromBaggageMaybe != null) { + return sampleRandFromBaggageMaybe; + } + } + + return getCombinedScopeView().getPropagationContext().getSampleRand(); + } + @Override @ApiStatus.Internal public void setSpanContext( @@ -963,7 +977,10 @@ public void reportFullyDisplayed() { PropagationContext.fromHeaders(getOptions().getLogger(), sentryTrace, baggageHeaders); configureScope( (scope) -> { - scope.setPropagationContext(propagationContext); + scope.withPropagationContext( + oldPropagationContext -> { + scope.setPropagationContext(propagationContext); + }); }); if (getOptions().isTracingEnabled()) { return TransactionContext.fromPropagationContext(propagationContext); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 94007e42c5..bd5f296b7c 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -22,6 +22,7 @@ import io.sentry.util.InitUtil; import io.sentry.util.LoadClass; import io.sentry.util.Platform; +import io.sentry.util.SentryRandom; import io.sentry.util.thread.IThreadChecker; import io.sentry.util.thread.NoOpThreadChecker; import io.sentry.util.thread.ThreadChecker; @@ -458,7 +459,8 @@ private static void handleAppStartProfilingConfig( final @NotNull SentryOptions options) { TransactionContext appStartTransactionContext = new TransactionContext("app.launch", "profile"); appStartTransactionContext.setForNextAppStart(true); - SamplingContext appStartSamplingContext = new SamplingContext(appStartTransactionContext, null); + SamplingContext appStartSamplingContext = + new SamplingContext(appStartTransactionContext, null, SentryRandom.current().nextDouble()); return options.getInternalTracesSampler().sample(appStartSamplingContext); } diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 65901a2e1a..36329db7a7 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -46,7 +46,6 @@ public final class SentryTracer implements ITransaction { private final @NotNull AtomicBoolean isIdleFinishTimerRunning = new AtomicBoolean(false); private final @NotNull AtomicBoolean isDeadlineTimerRunning = new AtomicBoolean(false); - private final @NotNull Baggage baggage; private @NotNull TransactionNameSource transactionNameSource; private final @NotNull Instrumenter instrumenter; private final @NotNull Contexts contexts = new Contexts(); @@ -81,12 +80,6 @@ public SentryTracer( this.transactionNameSource = context.getTransactionNameSource(); this.transactionOptions = transactionOptions; - if (context.getBaggage() != null) { - this.baggage = context.getBaggage(); - } else { - this.baggage = new Baggage(scopes.getOptions().getLogger()); - } - // We are currently sending the performance data only in profiles, but we are always sending // performance measurements. if (transactionPerformanceCollector != null) { @@ -642,14 +635,16 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) @Override public @Nullable TraceContext traceContext() { if (scopes.getOptions().isTraceSampling()) { - updateBaggageValues(); - return baggage.toTraceContext(); - } else { - return null; + final @Nullable Baggage baggage = getSpanContext().getBaggage(); + if (baggage != null) { + updateBaggageValues(baggage); + return baggage.toTraceContext(); + } } + return null; } - private void updateBaggageValues() { + private void updateBaggageValues(final @NotNull Baggage baggage) { try (final @NotNull ISentryLifecycleToken ignored = tracerLock.acquire()) { if (baggage.isMutable()) { final AtomicReference replayId = new AtomicReference<>(); @@ -672,12 +667,13 @@ private void updateBaggageValues() { @Override public @Nullable BaggageHeader toBaggageHeader(@Nullable List thirdPartyBaggageHeaders) { if (scopes.getOptions().isTraceSampling()) { - updateBaggageValues(); - - return BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); - } else { - return null; + final @Nullable Baggage baggage = getSpanContext().getBaggage(); + if (baggage != null) { + updateBaggageValues(baggage); + return BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); + } } + return null; } private boolean hasAllChildrenFinished() { diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index bb32022f60..b10954f528 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -19,6 +19,7 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { private final @Nullable String userId; private final @Nullable String transaction; private final @Nullable String sampleRate; + private final @Nullable String sampleRand; private final @Nullable String sampled; private final @Nullable SentryId replayId; @@ -29,6 +30,11 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { this(traceId, publicKey, null, null, null, null, null, null, null); } + @SuppressWarnings("InlineMeSuggester") + /** + * @deprecated please use the constructor than also takes sampleRand + */ + @Deprecated TraceContext( @NotNull SentryId traceId, @NotNull String publicKey, @@ -39,6 +45,30 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { @Nullable String sampleRate, @Nullable String sampled, @Nullable SentryId replayId) { + this( + traceId, + publicKey, + release, + environment, + userId, + transaction, + sampleRate, + sampled, + replayId, + null); + } + + TraceContext( + @NotNull SentryId traceId, + @NotNull String publicKey, + @Nullable String release, + @Nullable String environment, + @Nullable String userId, + @Nullable String transaction, + @Nullable String sampleRate, + @Nullable String sampled, + @Nullable SentryId replayId, + @Nullable String sampleRand) { this.traceId = traceId; this.publicKey = publicKey; this.release = release; @@ -48,6 +78,7 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { this.sampleRate = sampleRate; this.sampled = sampled; this.replayId = replayId; + this.sampleRand = sampleRand; } @SuppressWarnings("UnusedMethod") @@ -88,6 +119,10 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return sampleRate; } + public @Nullable String getSampleRand() { + return sampleRand; + } + public @Nullable String getSampled() { return sampled; } @@ -117,6 +152,7 @@ public static final class JsonKeys { public static final String USER_ID = "user_id"; public static final String TRANSACTION = "transaction"; public static final String SAMPLE_RATE = "sample_rate"; + public static final String SAMPLE_RAND = "sample_rand"; public static final String SAMPLED = "sampled"; public static final String REPLAY_ID = "replay_id"; } @@ -142,6 +178,9 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger if (sampleRate != null) { writer.name(TraceContext.JsonKeys.SAMPLE_RATE).value(sampleRate); } + if (sampleRand != null) { + writer.name(TraceContext.JsonKeys.SAMPLE_RAND).value(sampleRand); + } if (sampled != null) { writer.name(TraceContext.JsonKeys.SAMPLED).value(sampled); } @@ -171,6 +210,7 @@ public static final class Deserializer implements JsonDeserializer String userId = null; String transaction = null; String sampleRate = null; + String sampleRand = null; String sampled = null; SentryId replayId = null; @@ -199,6 +239,9 @@ public static final class Deserializer implements JsonDeserializer case TraceContext.JsonKeys.SAMPLE_RATE: sampleRate = reader.nextStringOrNull(); break; + case TraceContext.JsonKeys.SAMPLE_RAND: + sampleRand = reader.nextStringOrNull(); + break; case TraceContext.JsonKeys.SAMPLED: sampled = reader.nextStringOrNull(); break; @@ -229,7 +272,8 @@ public static final class Deserializer implements JsonDeserializer transaction, sampleRate, sampled, - replayId); + replayId, + sampleRand); traceContext.setUnknown(unknown); reader.endObject(); return traceContext; diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 3ce28ab745..b3da8d63cc 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -1,35 +1,27 @@ package io.sentry; import io.sentry.util.Objects; -import io.sentry.util.Random; -import io.sentry.util.SentryRandom; +import io.sentry.util.SampleRateUtils; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal public final class TracesSampler { private final @NotNull SentryOptions options; - private final @Nullable Random random; public TracesSampler(final @NotNull SentryOptions options) { - this(Objects.requireNonNull(options, "options are required"), null); - } - - @TestOnly - TracesSampler(final @NotNull SentryOptions options, final @Nullable Random random) { - this.options = options; - this.random = random; + this.options = Objects.requireNonNull(options, "options are required"); } @SuppressWarnings("deprecation") @NotNull public TracesSamplingDecision sample(final @NotNull SamplingContext samplingContext) { + final @NotNull Double sampleRand = samplingContext.getSampleRand(); final TracesSamplingDecision samplingContextSamplingDecision = samplingContext.getTransactionContext().getSamplingDecision(); if (samplingContextSamplingDecision != null) { - return samplingContextSamplingDecision; + return SampleRateUtils.backfilledSampleRand(samplingContextSamplingDecision); } Double profilesSampleRate = null; @@ -45,7 +37,7 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont if (profilesSampleRate == null) { profilesSampleRate = options.getProfilesSampleRate(); } - Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate); + Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate, sampleRand); if (options.getTracesSampler() != null) { Double samplerResult = null; @@ -58,14 +50,18 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont } if (samplerResult != null) { return new TracesSamplingDecision( - sample(samplerResult), samplerResult, profilesSampled, profilesSampleRate); + sample(samplerResult, sampleRand), + samplerResult, + sampleRand, + profilesSampled, + profilesSampleRate); } } final TracesSamplingDecision parentSamplingDecision = samplingContext.getTransactionContext().getParentSamplingDecision(); if (parentSamplingDecision != null) { - return parentSamplingDecision; + return SampleRateUtils.backfilledSampleRand(parentSamplingDecision); } final @Nullable Double tracesSampleRateFromOptions = options.getTracesSampleRate(); @@ -76,23 +72,17 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont if (downsampledTracesSampleRate != null) { return new TracesSamplingDecision( - sample(downsampledTracesSampleRate), + sample(downsampledTracesSampleRate, sampleRand), downsampledTracesSampleRate, + sampleRand, profilesSampled, profilesSampleRate); } - return new TracesSamplingDecision(false, null, false, null); + return new TracesSamplingDecision(false, null, sampleRand, false, null); } - private boolean sample(final @NotNull Double aDouble) { - return !(aDouble < getRandom().nextDouble()); - } - - private Random getRandom() { - if (random == null) { - return SentryRandom.current(); - } - return random; + private boolean sample(final @NotNull Double sampleRate, final @NotNull Double sampleRand) { + return !(sampleRate < sampleRand); } } diff --git a/sentry/src/main/java/io/sentry/TracesSamplingDecision.java b/sentry/src/main/java/io/sentry/TracesSamplingDecision.java index 8010537d5c..e9e9a7a490 100644 --- a/sentry/src/main/java/io/sentry/TracesSamplingDecision.java +++ b/sentry/src/main/java/io/sentry/TracesSamplingDecision.java @@ -7,6 +7,7 @@ public final class TracesSamplingDecision { private final @NotNull Boolean sampled; private final @Nullable Double sampleRate; + private final @Nullable Double sampleRand; private final @NotNull Boolean profileSampled; private final @Nullable Double profileSampleRate; @@ -15,16 +16,33 @@ public TracesSamplingDecision(final @NotNull Boolean sampled) { } public TracesSamplingDecision(final @NotNull Boolean sampled, final @Nullable Double sampleRate) { - this(sampled, sampleRate, false, null); + this(sampled, sampleRate, null, false, null); } public TracesSamplingDecision( final @NotNull Boolean sampled, final @Nullable Double sampleRate, + final @Nullable Double sampleRand) { + this(sampled, sampleRate, sampleRand, false, null); + } + + public TracesSamplingDecision( + final @NotNull Boolean sampled, + final @Nullable Double sampleRate, + final @NotNull Boolean profileSampled, + final @Nullable Double profileSampleRate) { + this(sampled, sampleRate, null, profileSampled, profileSampleRate); + } + + public TracesSamplingDecision( + final @NotNull Boolean sampled, + final @Nullable Double sampleRate, + final @Nullable Double sampleRand, final @NotNull Boolean profileSampled, final @Nullable Double profileSampleRate) { this.sampled = sampled; this.sampleRate = sampleRate; + this.sampleRand = sampleRand; // A profile can be sampled only if the transaction is sampled this.profileSampled = sampled && profileSampled; this.profileSampleRate = profileSampleRate; @@ -38,6 +56,10 @@ public TracesSamplingDecision( return sampleRate; } + public @Nullable Double getSampleRand() { + return sampleRand; + } + public @NotNull Boolean getProfileSampled() { return profileSampled; } diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index aec0b8927d..866c1c00da 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -3,6 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; +import io.sentry.util.TracingUtils; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -20,24 +21,14 @@ public final class TransactionContext extends SpanContext { @ApiStatus.Internal public static TransactionContext fromPropagationContext( final @NotNull PropagationContext propagationContext) { - @Nullable Boolean parentSampled = propagationContext.isSampled(); - TracesSamplingDecision samplingDecision = - parentSampled == null ? null : new TracesSamplingDecision(parentSampled); - - @Nullable Baggage baggage = propagationContext.getBaggage(); - - if (baggage != null) { - baggage.freeze(); - - Double sampleRate = baggage.getSampleRateDouble(); - if (parentSampled != null) { - if (sampleRate != null) { - samplingDecision = new TracesSamplingDecision(parentSampled.booleanValue(), sampleRate); - } else { - samplingDecision = new TracesSamplingDecision(parentSampled.booleanValue()); - } - } - } + final @Nullable Boolean parentSampled = propagationContext.isSampled(); + final @NotNull Baggage baggage = propagationContext.getBaggage(); + final @Nullable Double sampleRate = baggage.getSampleRateDouble(); + final @Nullable TracesSamplingDecision samplingDecision = + parentSampled == null + ? null + : new TracesSamplingDecision( + parentSampled, sampleRate, propagationContext.getSampleRand()); return new TransactionContext( propagationContext.getTraceId(), @@ -90,6 +81,7 @@ public TransactionContext( this.name = Objects.requireNonNull(name, "name is required"); this.transactionNameSource = transactionNameSource; this.setSamplingDecision(samplingDecision); + this.baggage = TracingUtils.ensureBaggage(null, samplingDecision); } @ApiStatus.Internal @@ -103,7 +95,7 @@ public TransactionContext( this.name = DEFAULT_TRANSACTION_NAME; this.parentSamplingDecision = parentSamplingDecision; this.transactionNameSource = DEFAULT_NAME_SOURCE; - this.baggage = baggage; + this.baggage = TracingUtils.ensureBaggage(baggage, parentSamplingDecision); } public @NotNull String getName() { diff --git a/sentry/src/main/java/io/sentry/util/SampleRateUtils.java b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java index ed011ff842..225ce58a3b 100644 --- a/sentry/src/main/java/io/sentry/util/SampleRateUtils.java +++ b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java @@ -1,6 +1,8 @@ package io.sentry.util; +import io.sentry.TracesSamplingDecision; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal @@ -23,6 +25,42 @@ public static boolean isValidProfilesSampleRate(@Nullable Double profilesSampleR return isValidRate(profilesSampleRate, true); } + public static @NotNull Double backfilledSampleRand( + final @Nullable Double sampleRand, + final @Nullable Double sampleRate, + final @Nullable Boolean sampled) { + if (sampleRand != null) { + return sampleRand; + } + + double newSampleRand = SentryRandom.current().nextDouble(); + if (sampleRate != null && sampled != null) { + if (sampled) { + return newSampleRand * sampleRate; + } else { + return sampleRate + (newSampleRand * (1 - sampleRate)); + } + } + + return newSampleRand; + } + + public static @NotNull TracesSamplingDecision backfilledSampleRand( + final @NotNull TracesSamplingDecision samplingDecision) { + if (samplingDecision.getSampleRand() != null) { + return samplingDecision; + } + + final @NotNull Double sampleRand = + backfilledSampleRand(null, samplingDecision.getSampleRate(), samplingDecision.getSampled()); + return new TracesSamplingDecision( + samplingDecision.getSampled(), + samplingDecision.getSampleRate(), + sampleRand, + samplingDecision.getProfileSampled(), + samplingDecision.getProfileSampleRate()); + } + private static boolean isValidRate(final @Nullable Double rate, final boolean allowNull) { if (rate == null) { return allowNull; diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 16655be634..8673b358a9 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -6,9 +6,11 @@ import io.sentry.IScope; import io.sentry.IScopes; import io.sentry.ISpan; +import io.sentry.NoOpLogger; import io.sentry.PropagationContext; import io.sentry.SentryOptions; import io.sentry.SentryTraceHeader; +import io.sentry.TracesSamplingDecision; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -57,12 +59,9 @@ public static void startNewTrace(final @NotNull IScopes scopes) { if (returnValue.propagationContext != null) { final @NotNull PropagationContext propagationContext = returnValue.propagationContext; - final @Nullable Baggage baggage = propagationContext.getBaggage(); - @Nullable BaggageHeader baggageHeader = null; - if (baggage != null) { - baggageHeader = - BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); - } + final @NotNull Baggage baggage = propagationContext.getBaggage(); + final @NotNull BaggageHeader baggageHeader = + BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); return new TracingHeaders( new SentryTraceHeader( @@ -80,11 +79,7 @@ public static void startNewTrace(final @NotNull IScopes scopes) { final @NotNull IScope scope, final @NotNull SentryOptions sentryOptions) { return scope.withPropagationContext( propagationContext -> { - @Nullable Baggage baggage = propagationContext.getBaggage(); - if (baggage == null) { - baggage = new Baggage(sentryOptions.getLogger()); - propagationContext.setBaggage(baggage); - } + @NotNull Baggage baggage = propagationContext.getBaggage(); if (baggage.isMutable()) { baggage.setValuesFromScope(scope, sentryOptions); baggage.freeze(); @@ -152,4 +147,66 @@ public static boolean isIgnored( return false; } + + /** + * Ensures a non null baggage instance is present by creating a new Baggage instance if null is + * passed in. + * + *

Also ensures there is a sampleRand value present on the baggage if it is still mutable. If + * the baggage should be frozen, it also takes care of freezing it. + * + * @param incomingBaggage a nullable baggage instance, if null a new one will be created + * @param decision a TracesSamplingDecision for potentially backfilling sampleRand to match that + * decision + * @return previous baggage instance or a new one + */ + @ApiStatus.Internal + public static @NotNull Baggage ensureBaggage( + final @Nullable Baggage incomingBaggage, final @Nullable TracesSamplingDecision decision) { + final @Nullable Boolean decisionSampled = decision == null ? null : decision.getSampled(); + final @Nullable Double decisionSampleRate = decision == null ? null : decision.getSampleRate(); + final @Nullable Double decisionSampleRand = decision == null ? null : decision.getSampleRand(); + + return ensureBaggage(incomingBaggage, decisionSampled, decisionSampleRate, decisionSampleRand); + } + + /** + * Ensures a non null baggage instance is present by creating a new Baggage instance if null is + * passed in. + * + *

Also ensures there is a sampleRand value present on the baggage if it is still mutable. If + * the baggage should be frozen, it also takes care of freezing it. + * + * @param incomingBaggage a nullable baggage instance, if null a new one will be created + * @param decisionSampled sampled decision for potential backfilling + * @param decisionSampleRate sampleRate for potential backfilling + * @param decisionSampleRand sampleRand to be used if none in baggage + * @return previous baggage instance or a new one + */ + @ApiStatus.Internal + public static @NotNull Baggage ensureBaggage( + final @Nullable Baggage incomingBaggage, + final @Nullable Boolean decisionSampled, + final @Nullable Double decisionSampleRate, + final @Nullable Double decisionSampleRand) { + final @NotNull Baggage baggage = + incomingBaggage == null ? new Baggage(NoOpLogger.getInstance()) : incomingBaggage; + + if (baggage.getSampleRand() == null) { + final @Nullable Double baggageSampleRate = baggage.getSampleRateDouble(); + final @Nullable Double sampleRateMaybe = + baggageSampleRate == null ? decisionSampleRate : baggageSampleRate; + final @NotNull Double sampleRand = + SampleRateUtils.backfilledSampleRand( + decisionSampleRand, sampleRateMaybe, decisionSampled); + baggage.setSampleRandDouble(sampleRand); + } + if (baggage.isMutable()) { + if (baggage.isShouldFreeze()) { + baggage.freeze(); + } + } + + return baggage; + } } diff --git a/sentry/src/test/java/io/sentry/BaggageTest.kt b/sentry/src/test/java/io/sentry/BaggageTest.kt index 8beae33668..fedd782624 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -8,7 +8,9 @@ import java.util.UUID import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue class BaggageTest { @@ -340,8 +342,9 @@ class BaggageTest { } @Test - fun `setting values if header contains sentry values has no effect`() { + fun `setting values on frozen baggage has no effect`() { val baggage = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction", logger) + baggage.freeze() baggage.traceId = "b" baggage.traceId = "c" @@ -535,6 +538,62 @@ class BaggageTest { assertEquals("abc", traceContext.unknown!!["anewkey"]) } + @Test + fun `header with sentry values is marked for freezing`() { + val baggage = + Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") + assertTrue(baggage.isShouldFreeze) + } + + @Test + fun `header with sentry sample rand only is not marked for freezing`() { + val baggage = + Baggage.fromHeader("sentry-sample_rand=0.3") + assertFalse(baggage.isShouldFreeze) + } + + @Test + fun `header without sentry values is not marked for freezing`() { + val baggage = + Baggage.fromHeader("a=b,c=d") + assertFalse(baggage.isShouldFreeze) + } + + @Test + fun `sample rate can be retrieved as double`() { + val baggage = Baggage.fromHeader("a=b,c=d") + baggage.sampleRate = "0.1" + assertEquals(0.1, baggage.sampleRateDouble) + } + + @Test + fun `sample rand can be retrieved as double`() { + val baggage = Baggage.fromHeader("a=b,c=d") + baggage.sampleRand = "0.1" + assertEquals(0.1, baggage.sampleRandDouble) + } + + @Test + fun `sample rand can be set as double`() { + val baggage = Baggage.fromHeader("a=b,c=d") + baggage.sampleRandDouble = 0.1 + assertEquals("0.1", baggage.sampleRand) + } + + @Test + fun `broken sample rand returns null double`() { + val baggage = Baggage.fromHeader("a=b,c=d") + baggage.sampleRand = "a0.1" + assertNull(baggage.sampleRandDouble) + } + + @Test + fun `broken sample rate returns null double`() { + val baggage = Baggage.fromHeader("a=b,c=d") + baggage.sampleRate = "a0.1" + assertNull(baggage.sampleRateDouble) + } + /** * token = 1*tchar * tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index e9d12ff4b1..8e3140faa1 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -456,16 +456,16 @@ class JsonSerializerTest { @Test fun `serializes trace context`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "transaction", "0.5", "true", SentryId("3367f5196c494acaae85bbbd535379aa"))) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","transaction":"transaction","sample_rate":"0.5","sampled":"true","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "transaction", "0.5", "true", SentryId("3367f5196c494acaae85bbbd535379aa"), "0.25")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","transaction":"transaction","sample_rate":"0.5","sample_rand":"0.25","sampled":"true","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } @Test fun `serializes trace context with user having null id`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, "transaction", "0.6", "false", SentryId("3367f5196c494acaae85bbbd535379aa"))) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6","sampled":"false","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, "transaction", "0.6", "false", SentryId("3367f5196c494acaae85bbbd535379aa"), "0.3")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6","sample_rand":"0.3","sampled":"false","replay_id":"3367f5196c494acaae85bbbd535379aa"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 8a1850e7dd..9136494ddf 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -23,6 +23,7 @@ import java.util.Date import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertTrue class OutboxSenderTest { @@ -141,6 +142,63 @@ class OutboxSenderTest { whenever(fixture.scopes.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, 0.021) + + val sentryTracer = SentryTracer(transactionContext, fixture.scopes) + 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-rand.txt") + assertTrue(File(path).exists()) + + val hints = HintUtils.createWithTypeCheckHint(mock()) + sut.processEnvelopeFile(path, hints) + + verify(fixture.scopes).captureTransaction( + check { + assertEquals(expected, it) + assertTrue(it.isSampled) + assertEquals(0.00000021, it.samplingDecision?.sampleRate) + assertEquals(0.021, it.samplingDecision?.sampleRand) + 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("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 `backfills sampleRand`() { + fixture.envelopeReader = EnvelopeReader(JsonSerializer(fixture.options)) + whenever(fixture.options.maxSpans).thenReturn(1000) + whenever(fixture.scopes.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 @@ -170,6 +228,7 @@ class OutboxSenderTest { assertEquals(expected, it) assertTrue(it.isSampled) assertEquals(0.00000021, it.samplingDecision?.sampleRate) + assertNotNull(it.samplingDecision?.sampleRand) assertTrue(it.samplingDecision!!.sampled) }, check { diff --git a/sentry/src/test/java/io/sentry/PropagationContextTest.kt b/sentry/src/test/java/io/sentry/PropagationContextTest.kt new file mode 100644 index 0000000000..39fffe9f89 --- /dev/null +++ b/sentry/src/test/java/io/sentry/PropagationContextTest.kt @@ -0,0 +1,43 @@ +package io.sentry + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +class PropagationContextTest { + + @Test + fun `freezes baggage with sentry values`() { + val propagationContext = PropagationContext.fromHeaders( + NoOpLogger.getInstance(), + "2722d9f6ec019ade60c776169d9a8904-cedf5b7571cb4972-1", + "sentry-trace_id=a,sentry-transaction=sentryTransaction" + ) + assertFalse(propagationContext.baggage.isMutable) + assertTrue(propagationContext.baggage.isShouldFreeze) + } + + @Test + fun `does not freeze baggage without sentry values`() { + val propagationContext = PropagationContext.fromHeaders( + NoOpLogger.getInstance(), + "2722d9f6ec019ade60c776169d9a8904-cedf5b7571cb4972-1", + "a=b" + ) + assertTrue(propagationContext.baggage.isMutable) + assertFalse(propagationContext.baggage.isShouldFreeze) + } + + @Test + fun `creates new baggage if none passed`() { + val propagationContext = PropagationContext.fromHeaders( + NoOpLogger.getInstance(), + "2722d9f6ec019ade60c776169d9a8904-cedf5b7571cb4972-1", + null as? String? + ) + assertNotNull(propagationContext.baggage) + assertTrue(propagationContext.baggage.isMutable) + assertFalse(propagationContext.baggage.isShouldFreeze) + } +} diff --git a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt index 8b00df543d..ce0e4a4ae4 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -24,7 +24,8 @@ class TraceContextSerializationTest { "0252ec25-cd0a-4230-bd2f-936a4585637e", "0.00000021", "true", - SentryId("3367f5196c494acaae85bbbd535379aa") + SentryId("3367f5196c494acaae85bbbd535379aa"), + "0.00000012" ) } private val fixture = Fixture() diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 06eb60aece..0fbc8e2f67 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -1,31 +1,25 @@ package io.sentry -import io.sentry.util.Random import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue class TracesSamplerTest { class Fixture { internal fun getSut( - randomResult: Double? = null, tracesSampleRate: Double? = null, profilesSampleRate: Double? = null, tracesSamplerCallback: SentryOptions.TracesSamplerCallback? = null, profilesSamplerCallback: SentryOptions.ProfilesSamplerCallback? = null, logger: ILogger? = null ): TracesSampler { - val random = mock() - if (randomResult != null) { - whenever(random.nextDouble()).thenReturn(randomResult) - } val options = SentryOptions() if (tracesSampleRate != null) { options.tracesSampleRate = tracesSampleRate @@ -43,7 +37,7 @@ class TracesSamplerTest { options.isDebug = true options.setLogger(logger) } - return TracesSampler(options, random) + return TracesSampler(options) } } @@ -51,103 +45,115 @@ 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, profilesSampleRate = 0.2) - val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + val sampler = fixture.getSut(tracesSampleRate = 0.2, profilesSampleRate = 0.2) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.9)) assertFalse(samplingDecision.sampled) assertEquals(0.2, samplingDecision.sampleRate) + assertEquals(0.9, samplingDecision.sampleRand) } @Test fun `when tracesSampleRate is set and random returns lower number returns true`() { - val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 0.2, profilesSampleRate = 0.2) - val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + val sampler = fixture.getSut(tracesSampleRate = 0.2, profilesSampleRate = 0.2) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.1)) assertTrue(samplingDecision.sampled) assertEquals(0.2, samplingDecision.sampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when profilesSampleRate is set and random returns greater number returns false`() { - val sampler = fixture.getSut(randomResult = 0.9, tracesSampleRate = 1.0, profilesSampleRate = 0.2) - val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSampleRate = 0.2) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.9)) assertTrue(samplingDecision.sampled) assertFalse(samplingDecision.profileSampled) assertEquals(0.2, samplingDecision.profileSampleRate) + assertEquals(0.9, samplingDecision.sampleRand) } @Test fun `when profilesSampleRate is set and random returns lower number returns true`() { - val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 1.0, profilesSampleRate = 0.2) - val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSampleRate = 0.2) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.1)) assertTrue(samplingDecision.sampled) assertTrue(samplingDecision.profileSampled) assertEquals(0.2, samplingDecision.profileSampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when trace is not sampled, profile is not sampled`() { - val sampler = fixture.getSut(randomResult = 0.3, tracesSampleRate = 0.0, profilesSampleRate = 1.0) - val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null)) + val sampler = fixture.getSut(tracesSampleRate = 0.0, profilesSampleRate = 1.0) + val samplingDecision = sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.3)) assertFalse(samplingDecision.sampled) assertFalse(samplingDecision.profileSampled) assertEquals(1.0, samplingDecision.profileSampleRate) + assertEquals(0.3, samplingDecision.sampleRand) } @Test fun `when tracesSampleRate is not set, tracesSampler is set and random returns lower number returns true`() { val sampler = fixture.getSut( - randomResult = 0.1, tracesSamplerCallback = { 0.2 }, profilesSamplerCallback = { 0.2 } ) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertEquals(0.2, samplingDecision.sampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when profilesSampleRate is not set, profilesSampler is set and random returns lower number returns true`() { - val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 1.0, profilesSamplerCallback = { 0.2 }) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSamplerCallback = { 0.2 }) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertTrue(samplingDecision.profileSampled) assertEquals(0.2, samplingDecision.profileSampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when tracesSampleRate is not set, tracesSampler is set and random returns greater number returns false`() { - val sampler = fixture.getSut(randomResult = 0.9, tracesSamplerCallback = { 0.2 }) + val sampler = fixture.getSut(tracesSamplerCallback = { 0.2 }) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.9 ) ) assertFalse(samplingDecision.sampled) assertEquals(0.2, samplingDecision.sampleRate) + assertEquals(0.9, samplingDecision.sampleRand) } @Test fun `when profilesSampleRate is not set, profilesSampler is set and random returns greater number returns false`() { - val sampler = fixture.getSut(randomResult = 0.9, tracesSampleRate = 1.0, profilesSamplerCallback = { 0.2 }) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSamplerCallback = { 0.2 }) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.9 ) ) assertTrue(samplingDecision.sampled) assertFalse(samplingDecision.profileSampled) assertEquals(0.2, samplingDecision.profileSampleRate) + assertEquals(0.9, samplingDecision.sampleRand) } @Test @@ -158,11 +164,13 @@ class TracesSamplerTest { val samplingDecision = sampler.sample( SamplingContext( transactionContextParentSampled, - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertNull(samplingDecision.sampleRate) + assertNotNull(samplingDecision.sampleRand) } @Test @@ -173,34 +181,39 @@ class TracesSamplerTest { val samplingDecision = sampler.sample( SamplingContext( transactionContextParentSampled, - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertTrue(samplingDecision.profileSampled) assertNull(samplingDecision.profileSampleRate) + assertNotNull(samplingDecision.sampleRand) } @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, tracesSamplerCallback = null) + val sampler = fixture.getSut(tracesSampleRate = 0.2, tracesSamplerCallback = null) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertEquals(0.2, samplingDecision.sampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when profilesSampler returns null and profilesSampleRate is set sampler uses it as a sampling decision`() { - val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 1.0, profilesSampleRate = 0.2, profilesSamplerCallback = null) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSampleRate = 0.2, profilesSamplerCallback = null) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) @@ -210,29 +223,33 @@ class TracesSamplerTest { @Test fun `when tracesSampleRate is not set, and tracesSampler is not set returns false`() { - val sampler = fixture.getSut(randomResult = 0.1) + val sampler = fixture.getSut() val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertFalse(samplingDecision.sampled) assertNull(samplingDecision.sampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test fun `when profilesSampleRate is not set, and profilesSampler is not set returns false`() { - val sampler = fixture.getSut(randomResult = 0.1, tracesSampleRate = 1.0) + val sampler = fixture.getSut(tracesSampleRate = 1.0) val samplingDecision = sampler.sample( SamplingContext( TransactionContext("name", "op"), - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecision.sampled) assertFalse(samplingDecision.profileSampled) assertNull(samplingDecision.profileSampleRate) + assertEquals(0.1, samplingDecision.sampleRand) } @Test @@ -243,11 +260,13 @@ class TracesSamplerTest { val samplingDecision = sampler.sample( SamplingContext( transactionContextParentNotSampled, - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertFalse(samplingDecision.sampled) assertNull(samplingDecision.sampleRate) + assertNotNull(samplingDecision.sampleRand) assertFalse(samplingDecision.profileSampled) assertNull(samplingDecision.profileSampleRate) @@ -256,11 +275,13 @@ class TracesSamplerTest { val samplingDecisionParentSampled = sampler.sample( SamplingContext( transactionContextParentSampled, - CustomSamplingContext() + CustomSamplingContext(), + 0.1 ) ) assertTrue(samplingDecisionParentSampled.sampled) assertNull(samplingDecisionParentSampled.sampleRate) + assertNotNull(samplingDecisionParentSampled.sampleRand) assertTrue(samplingDecisionParentSampled.profileSampled) assertNull(samplingDecisionParentSampled.profileSampleRate) } @@ -288,27 +309,30 @@ class TracesSamplerTest { val transactionContextNotSampled = TransactionContext("name", "op") transactionContextNotSampled.sampled = false val samplingDecision = - sampler.sample(SamplingContext(transactionContextNotSampled, CustomSamplingContext())) + sampler.sample(SamplingContext(transactionContextNotSampled, CustomSamplingContext(), 0.1)) assertFalse(samplingDecision.sampled) assertNull(samplingDecision.sampleRate) + assertNotNull(samplingDecision.sampleRand) assertFalse(samplingDecision.profileSampled) assertNull(samplingDecision.profileSampleRate) val transactionContextSampled = TransactionContext("name", "op") transactionContextSampled.setSampled(true, true) val samplingDecisionContextSampled = - sampler.sample(SamplingContext(transactionContextSampled, CustomSamplingContext())) + sampler.sample(SamplingContext(transactionContextSampled, CustomSamplingContext(), 0.1)) assertTrue(samplingDecisionContextSampled.sampled) assertNull(samplingDecisionContextSampled.sampleRate) + assertNotNull(samplingDecisionContextSampled.sampleRand) assertTrue(samplingDecisionContextSampled.profileSampled) assertNull(samplingDecisionContextSampled.profileSampleRate) val transactionContextUnsampledWithProfile = TransactionContext("name", "op") transactionContextUnsampledWithProfile.setSampled(false, true) val samplingDecisionContextUnsampledWithProfile = - sampler.sample(SamplingContext(transactionContextUnsampledWithProfile, CustomSamplingContext())) + sampler.sample(SamplingContext(transactionContextUnsampledWithProfile, CustomSamplingContext(), 0.1)) assertFalse(samplingDecisionContextUnsampledWithProfile.sampled) assertNull(samplingDecisionContextUnsampledWithProfile.sampleRate) + assertNotNull(samplingDecisionContextUnsampledWithProfile.sampleRand) assertFalse(samplingDecisionContextUnsampledWithProfile.profileSampled) assertNull(samplingDecisionContextUnsampledWithProfile.profileSampleRate) } @@ -326,7 +350,7 @@ class TracesSamplerTest { logger = logger ) val decision = sampler.sample( - SamplingContext(TransactionContext("name", "op"), null) + SamplingContext(TransactionContext("name", "op"), null, 0.1) ) assertFalse(decision.profileSampled) verify(logger).log(eq(SentryLevel.ERROR), any(), eq(exception)) @@ -336,7 +360,6 @@ class TracesSamplerTest { fun `when a profilingRate and a ProfilesSamplerCallback is set but the callback throws an exception then profiling should still be enabled`() { val exception = Exception("faulty ProfilesSamplerCallback") val sampler = fixture.getSut( - randomResult = 0.0, tracesSampleRate = 1.0, profilesSampleRate = 1.0, profilesSamplerCallback = { @@ -344,9 +367,10 @@ class TracesSamplerTest { } ) val decision = sampler.sample( - SamplingContext(TransactionContext("name", "op"), null) + SamplingContext(TransactionContext("name", "op"), null, 0.0) ) assertTrue(decision.profileSampled) + assertEquals(0.0, decision.sampleRand) } @Test @@ -361,9 +385,10 @@ class TracesSamplerTest { logger = logger ) val decision = sampler.sample( - SamplingContext(TransactionContext("name", "op"), null) + SamplingContext(TransactionContext("name", "op"), null, 0.1) ) assertFalse(decision.sampled) + assertEquals(0.1, decision.sampleRand) verify(logger).log(eq(SentryLevel.ERROR), any(), eq(exception)) } @@ -371,15 +396,15 @@ class TracesSamplerTest { fun `when a tracesSampleRate and a TracesSamplerCallback is set but the callback throws an exception then tracing should still be enabled`() { val exception = Exception("faulty TracesSamplerCallback") val sampler = fixture.getSut( - randomResult = 0.0, tracesSampleRate = 1.0, tracesSamplerCallback = { throw exception } ) val decision = sampler.sample( - SamplingContext(TransactionContext("name", "op"), null) + SamplingContext(TransactionContext("name", "op"), null, 0.0) ) assertTrue(decision.sampled) + assertEquals(0.0, decision.sampleRand) } } diff --git a/sentry/src/test/java/io/sentry/TransactionContextTest.kt b/sentry/src/test/java/io/sentry/TransactionContextTest.kt index d6b715bd84..8a66870bc2 100644 --- a/sentry/src/test/java/io/sentry/TransactionContextTest.kt +++ b/sentry/src/test/java/io/sentry/TransactionContextTest.kt @@ -1,10 +1,12 @@ package io.sentry import io.sentry.protocol.SentryId +import io.sentry.protocol.TransactionNameSource import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -91,4 +93,32 @@ class TransactionContextTest { context.isForNextAppStart = true assertTrue(context.isForNextAppStart) } + + @Test + fun `when passing null baggage creates a new one`() { + val context = TransactionContext(SentryId(), SpanId(), null, null, null) + assertNotNull(context.baggage) + assertNotNull(context.baggage?.sampleRand) + } + + @Test + fun `when passing null baggage creates a new one and uses parent sampling decision`() { + val context = TransactionContext(SentryId(), SpanId(), null, TracesSamplingDecision(true, 0.1, 0.2), null) + assertNotNull(context.baggage) + assertEquals("0.2", context.baggage?.sampleRand) + } + + @Test + fun `when using few param ctor creates a new baggage`() { + val context = TransactionContext("name", "op") + assertNotNull(context.baggage) + assertNotNull(context.baggage?.sampleRand) + } + + @Test + fun `when using few param ctor creates a new baggage and uses sampling decision`() { + val context = TransactionContext("name", TransactionNameSource.CUSTOM, "op", TracesSamplingDecision(true, 0.1, 0.2)) + assertNotNull(context.baggage) + assertEquals("0.2", context.baggage?.sampleRand) + } } diff --git a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt index e5c81bc70e..8576e9b10f 100644 --- a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt +++ b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt @@ -1,7 +1,10 @@ package io.sentry.util +import io.sentry.TracesSamplingDecision import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertTrue class SampleRateUtilTest { @@ -130,4 +133,62 @@ class SampleRateUtilTest { fun `accepts null profiles sample rate`() { assertTrue(SampleRateUtils.isValidProfilesSampleRate(null)) } + + @Test + fun `fills sample rand on decision if missing`() { + val decision = SampleRateUtils.backfilledSampleRand(TracesSamplingDecision(true)) + assertNotNull(decision.sampleRand) + } + + @Test + fun `keeps sample rand on decision if present`() { + val decision = SampleRateUtils.backfilledSampleRand(TracesSamplingDecision(true, 0.1, 0.5)) + assertEquals(0.5, decision.sampleRand) + } + + @Test + fun `uses sampleRand and does not backfill`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(0.3, null, null) + assertEquals(0.3, sampleRand) + } + + @Test + fun `backfills sampleRand if missing`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(null, null, null) + assertNotNull(sampleRand) + assertTrue(sampleRand >= 0) + assertTrue(sampleRand < 1) + } + + @Test + fun `backfills sampleRand if missing with sampled true`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(null, null, true) + assertNotNull(sampleRand) + assertTrue(sampleRand >= 0) + assertTrue(sampleRand < 1) + } + + @Test + fun `backfills sampleRand if missing with sampled false`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(null, null, false) + assertNotNull(sampleRand) + assertTrue(sampleRand >= 0) + assertTrue(sampleRand < 1) + } + + @Test + fun `backfills sampleRand if missing with sampled true below sample rate`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(null, 0.0001, true) + assertNotNull(sampleRand) + assertTrue(sampleRand >= 0) + assertTrue(sampleRand < 0.0001) + } + + @Test + fun `backfills sampleRand if missing with sampled false above sample rate`() { + val sampleRand = SampleRateUtils.backfilledSampleRand(null, 0.9999, false) + assertNotNull(sampleRand) + assertTrue(sampleRand >= 0.9999) + assertTrue(sampleRand < 1) + } } diff --git a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt index e5403f54d9..ed378ee960 100644 --- a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt @@ -2,15 +2,19 @@ package io.sentry.util import io.sentry.Baggage import io.sentry.IScopes +import io.sentry.NoOpLogger import io.sentry.NoOpSpan +import io.sentry.PropagationContext import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.Span +import io.sentry.SpanId import io.sentry.SpanOptions import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext +import io.sentry.protocol.SentryId import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.doAnswer @@ -21,6 +25,7 @@ import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertSame import kotlin.test.assertTrue class TracingUtilsTest { @@ -129,7 +134,7 @@ class TracingUtilsTest { @Test fun `returns headers if allowed from scope without span leaving frozen baggage alone`() { - fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET").also { it.freeze() } + fixture.scope.propagationContext = PropagationContext(SentryId(), SpanId(), null, Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET").also { it.freeze() }, true) fixture.setup() val headers = TracingUtils.traceIfAllowed(fixture.scopes, "https://sentry.io/hello", fixture.preExistingBaggage, null) @@ -208,23 +213,11 @@ class TracingUtilsTest { assertNotEquals(propagationContextBefore.spanId, fixture.scope.propagationContext.spanId) } - @Test - fun `creates new baggage if none present`() { - fixture.setup() - assertNull(fixture.scope.propagationContext.baggage) - - TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) - - assertNotNull(fixture.scope.propagationContext.baggage) - assertEquals(fixture.scope.propagationContext.traceId.toString(), fixture.scope.propagationContext.baggage!!.traceId) - assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) - } - @Test fun `updates mutable baggage`() { fixture.setup() // not frozen because it doesn't contain sentry-* keys - fixture.scope.propagationContext.baggage = Baggage.fromHeader(fixture.preExistingBaggage) + fixture.scope.propagationContext = PropagationContext(SentryId(), SpanId(), null, Baggage.fromHeader(fixture.preExistingBaggage), true) TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) @@ -236,11 +229,136 @@ class TracingUtilsTest { fun `does not change frozen baggage`() { fixture.setup() // frozen automatically because it contains sentry-* keys - fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.scope.propagationContext = PropagationContext(SentryId(), SpanId(), null, Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET"), true) TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) assertEquals("2722d9f6ec019ade60c776169d9a8904", fixture.scope.propagationContext.baggage!!.traceId) assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) } + + @Test + fun `returns baggage if passed in`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + } + + @Test + fun `crates new baggage if null passed in that has sampleRand set and is mutable`() { + val baggage = TracingUtils.ensureBaggage( + null, + null as? TracesSamplingDecision? + ) + assertNotNull(baggage) + assertNotNull(baggage.sampleRand) + assertTrue(baggage.isMutable) + assertFalse(baggage.isShouldFreeze) + } + + @Test + fun `backfills sampleRand on passed in baggage if missing`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + assertNotNull(baggage.sampleRand) + assertTrue(baggage.isMutable) + } + + @Test + fun `keeps sampleRand on passed in baggage if present`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + incomingBaggage.sampleRand = "0.3" + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + assertEquals("0.3", baggage.sampleRand) + assertTrue(baggage.isMutable) + } + + @Test + fun `does not backfill sampleRand on passed in baggage if frozen`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + incomingBaggage.freeze() + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + assertNull(baggage.sampleRand) + assertFalse(baggage.isMutable) + } + + @Test + fun `freezes passed in baggage if should be frozen`() { + // markes as shouldFreeze=true due to sentry values being present in header + val incomingBaggage = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + assertNotNull(baggage.sampleRand) + assertFalse(baggage.isMutable) + } + + @Test + fun `does not freeze passed in baggage if should not be frozen`() { + // markes as shouldFreeze=false due to no sentry values being present in header + val incomingBaggage = Baggage.fromHeader("a=b,c=d") + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + null as? TracesSamplingDecision? + ) + assertSame(incomingBaggage, baggage) + assertNotNull(baggage.sampleRand) + assertTrue(baggage.isMutable) + } + + @Test + fun `uses sample rand if passed in`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + TracesSamplingDecision(true, null, 0.123) + ) + assertSame(incomingBaggage, baggage) + assertEquals("0.123", baggage.sampleRand) + } + + @Test + fun `uses sample rate and sampled flag true if passed in`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + TracesSamplingDecision(true, 0.0001, null) + ) + assertSame(incomingBaggage, baggage) + val sampleRand = baggage.sampleRandDouble + assertNotNull(sampleRand) + assertTrue(sampleRand < 0.0001) + assertTrue(sampleRand >= 0.0) + } + + @Test + fun `uses sample rate and sampled flag false if passed in`() { + val incomingBaggage = Baggage(NoOpLogger.getInstance()) + val baggage = TracingUtils.ensureBaggage( + incomingBaggage, + TracesSamplingDecision(false, 0.9999, null) + ) + assertSame(incomingBaggage, baggage) + val sampleRand = baggage.sampleRandDouble + assertNotNull(sampleRand) + assertTrue(sampleRand < 1.0) + assertTrue(sampleRand >= 0.9999) + } } diff --git a/sentry/src/test/resources/envelope-transaction-with-sample-rand.txt b/sentry/src/test/resources/envelope-transaction-with-sample-rand.txt new file mode 100644 index 0000000000..1a6b3120b8 --- /dev/null +++ b/sentry/src/test/resources/envelope-transaction-with-sample-rand.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","transaction":"tx1","sample_rate":"0.00000021","sample_rand":"0.021"}} +{"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 626e9cbbc2..23580aab66 100644 --- a/sentry/src/test/resources/json/sentry_envelope_header.json +++ b/sentry/src/test/resources/json/sentry_envelope_header.json @@ -26,6 +26,7 @@ "user_id": "c052c566-6619-45f5-a61f-172802afa39a", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", "sample_rate": "0.00000021", + "sample_rand": "0.00000012", "sampled": "true", "replay_id": "3367f5196c494acaae85bbbd535379aa" }, diff --git a/sentry/src/test/resources/json/trace_state.json b/sentry/src/test/resources/json/trace_state.json index db745e5213..a5eabb3583 100644 --- a/sentry/src/test/resources/json/trace_state.json +++ b/sentry/src/test/resources/json/trace_state.json @@ -6,6 +6,7 @@ "user_id": "c052c566-6619-45f5-a61f-172802afa39a", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", "sample_rate": "0.00000021", + "sample_rand": "0.00000012", "sampled": "true", "replay_id": "3367f5196c494acaae85bbbd535379aa" }