From 9320f73263c5e74f578aeffe7d74b73214882bc7 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 24 Sep 2026 15:46:36 +0200 Subject: [PATCH 1/3] fix(core): [Callback Errors 5] Handle tracesSampler failures Inherit the parent sampling decision when tracesSampler throws. Without a parent decision, leave the trace unsampled instead of applying the static tracesSampleRate, which can override the failed sampling policy. Keep normal null-result fallback, profiling callbacks, and catch types unchanged. Cover parent inheritance, backfilling, and static-rate bypass. Refs #6081 Co-Authored-By: Claude --- CHANGELOG.md | 1 + .../main/java/io/sentry/TracesSampler.java | 5 +- .../test/java/io/sentry/TracesSamplerTest.kt | 78 +++++++++++++++++-- 3 files changed, 75 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17952821933..1b643f449e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,7 @@ - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value. - Drop telemetry and record `callback_error` when a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues without a `callback_error` client report. - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb. + - When `tracesSampler` throws, inherit the parent sampling decision or leave the trace unsampled if there is no parent decision, instead of falling back to `tracesSampleRate`. - Disable URL caching when reading `META-INF/MANIFEST.MF` files during version detection so that the SDK no longer keeps jar file handles open for the life of the process ([#6124](https://github.com/getsentry/sentry-java/pull/6124) - Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003)) diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 5430b9242ac..e981bb6f66f 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -39,11 +39,13 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont } Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate, sampleRand); + boolean tracesSamplerFailed = false; if (options.getTracesSampler() != null) { Double samplerResult = null; try { samplerResult = options.getTracesSampler().sample(samplingContext); } catch (Throwable t) { + tracesSamplerFailed = true; options .getLogger() .log(SentryLevel.ERROR, "Error in the 'TracesSamplerCallback' callback.", t); @@ -64,7 +66,8 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont return SampleRateUtils.backfilledSampleRand(parentSamplingDecision); } - final @Nullable Double tracesSampleRateFromOptions = options.getTracesSampleRate(); + final @Nullable Double tracesSampleRateFromOptions = + tracesSamplerFailed ? null : options.getTracesSampleRate(); final @NotNull Double downsampleFactor = Math.pow(2, options.getBackpressureMonitor().getDownsampleFactor()); final @Nullable Double downsampledTracesSampleRate = diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 4266061353c..bb257a9c46e 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -1,5 +1,7 @@ package io.sentry +import com.google.common.truth.Truth.assertThat +import io.sentry.protocol.SentryId import io.sentry.util.SentryRandom import kotlin.test.Test import kotlin.test.assertEquals @@ -175,7 +177,7 @@ class TracesSamplerTest { @Test fun `when tracesSampler returns null and parentSampled is set sampler uses it as a sampling decision`() { - val sampler = fixture.getSut(tracesSamplerCallback = null) + val sampler = fixture.getSut(tracesSamplerCallback = { null }) val transactionContextParentSampled = TransactionContext("name", "op") transactionContextParentSampled.parentSampled = true val samplingDecision = @@ -204,7 +206,7 @@ class TracesSamplerTest { @Test fun `when tracesSampler returns null and tracesSampleRate is set sampler uses it as a sampling decision`() { - val sampler = fixture.getSut(tracesSampleRate = 0.2, tracesSamplerCallback = null) + val sampler = fixture.getSut(tracesSampleRate = 0.2, tracesSamplerCallback = { null }) val samplingDecision = sampler.sample( SamplingContext(TransactionContext("name", "op"), CustomSamplingContext(), 0.1, null) @@ -381,14 +383,74 @@ class TracesSamplerTest { } @Test - fun `when a tracesSampleRate and a TracesSamplerCallback is set but the callback throws an exception then tracing should still be enabled`() { + fun `when tracesSampler throws without a parent then static rates are ignored`() { val exception = Exception("faulty TracesSamplerCallback") + for (tracesSampleRate in listOf(null, 0.0, 1.0)) { + val sampler = + fixture.getSut( + tracesSampleRate = tracesSampleRate, + profilesSampleRate = 1.0, + tracesSamplerCallback = { throw exception }, + ) + val decision = + sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) + + assertThat(decision.sampled).isFalse() + assertThat(decision.sampleRate).isNull() + assertThat(decision.sampleRand).isEqualTo(0.0) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() + } + } + + @Test + fun `when tracesSampler throws then a sampled parent decision is inherited`() { val sampler = - fixture.getSut(tracesSampleRate = 1.0, tracesSamplerCallback = { throw exception }) - val decision = - sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) - assertTrue(decision.sampled) - assertEquals(0.0, decision.sampleRand) + fixture.getSut( + tracesSampleRate = 0.0, + tracesSamplerCallback = { throw IllegalStateException("faulty TracesSamplerCallback") }, + ) + val parentDecision = TracesSamplingDecision(true, 0.5, 0.1, true, 0.5) + val transactionContext = + TransactionContext(SentryId(), SpanId(), SpanId(), parentDecision, null) + + val decision = sampler.sample(SamplingContext(transactionContext, null, 0.1, null)) + + assertThat(decision).isSameInstanceAs(parentDecision) + } + + @Test + fun `when tracesSampler throws then an unsampled parent decision is inherited`() { + val sampler = + fixture.getSut( + tracesSampleRate = 1.0, + profilesSampleRate = 1.0, + tracesSamplerCallback = { throw IllegalStateException("faulty TracesSamplerCallback") }, + ) + val parentDecision = TracesSamplingDecision(false, 0.5, 0.9) + val transactionContext = + TransactionContext(SentryId(), SpanId(), SpanId(), parentDecision, null) + + val decision = sampler.sample(SamplingContext(transactionContext, null, 0.9, null)) + + assertThat(decision).isSameInstanceAs(parentDecision) + } + + @Test + fun `when tracesSampler throws then a parent decision without sampleRand is backfilled`() { + val sampler = + fixture.getSut( + tracesSampleRate = 0.0, + tracesSamplerCallback = { throw IllegalStateException("faulty TracesSamplerCallback") }, + ) + val transactionContext = TransactionContext("name", "op") + transactionContext.parentSampled = true + + val decision = sampler.sample(SamplingContext(transactionContext, null, 0.1, null)) + + assertThat(decision.sampled).isTrue() + assertThat(decision.sampleRate).isNull() + assertThat(decision.sampleRand).isNotNull() } @Test From 60d1e152d1217bde586e8cd76d3fe6e0f8d33d88 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 24 Sep 2026 15:49:21 +0200 Subject: [PATCH 2/3] changelog Link the tracesSampler failure fallback entry to #6163 in the callback error handling stack. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b643f449e1..79f7b506447 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,7 +126,7 @@ - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value. - Drop telemetry and record `callback_error` when a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues without a `callback_error` client report. - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb. - - When `tracesSampler` throws, inherit the parent sampling decision or leave the trace unsampled if there is no parent decision, instead of falling back to `tracesSampleRate`. + - When `tracesSampler` throws, inherit the parent sampling decision or leave the trace unsampled if there is no parent decision, instead of falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)) - Disable URL caching when reading `META-INF/MANIFEST.MF` files during version detection so that the SDK no longer keeps jar file handles open for the life of the process ([#6124](https://github.com/getsentry/sentry-java/pull/6124) - Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003)) From fcbf95f0dd5708937bde0e18f77b9e4b2eb6bd64 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 25 Sep 2026 10:46:32 +0200 Subject: [PATCH 3/3] fix(core): Drop transactions when tracesSampler fails Replace parent-decision fallback after tracesSampler errors with an unsampled decision and callback_error reports for the transaction and root span. Preserve deliberate null-result fallback and existing catches. Report callback errors directly in the sampler and retain existing sample_rate and backpressure accounting, accepting duplicate loss reports instead of adding discard-reason propagation. Cover core and OpenTelemetry paths with regression tests and update the changelog. Refs #6163 Co-Authored-By: Claude --- CHANGELOG.md | 2 +- .../src/test/kotlin/SentrySamplerTest.kt | 176 ++++++++++++++++++ .../main/java/io/sentry/TracesSampler.java | 15 +- sentry/src/test/java/io/sentry/ScopesTest.kt | 115 ++++++++++++ .../test/java/io/sentry/TracesSamplerTest.kt | 64 ++++++- 5 files changed, 359 insertions(+), 13 deletions(-) create mode 100644 sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySamplerTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 79f7b506447..79616655acf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,7 +126,7 @@ - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value. - Drop telemetry and record `callback_error` when a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues without a `callback_error` client report. - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb. - - When `tracesSampler` throws, inherit the parent sampling decision or leave the trace unsampled if there is no parent decision, instead of falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)) + - When `tracesSampler` throws, drop the transaction and record `callback_error` instead of inheriting the parent sampling decision or falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)) - Disable URL caching when reading `META-INF/MANIFEST.MF` files during version detection so that the SDK no longer keeps jar file handles open for the life of the process ([#6124](https://github.com/getsentry/sentry-java/pull/6124) - Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003)) diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySamplerTest.kt b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySamplerTest.kt new file mode 100644 index 00000000000..8a8ec0c8e0d --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySamplerTest.kt @@ -0,0 +1,176 @@ +package io.sentry.opentelemetry + +import com.google.common.truth.Truth.assertThat +import io.opentelemetry.api.OpenTelemetry +import io.opentelemetry.api.common.Attributes +import io.opentelemetry.api.trace.Span +import io.opentelemetry.api.trace.SpanKind +import io.opentelemetry.api.trace.TraceFlags +import io.opentelemetry.api.trace.TraceState +import io.opentelemetry.context.Context +import io.opentelemetry.sdk.trace.SdkTracerProvider +import io.opentelemetry.sdk.trace.samplers.Sampler +import io.opentelemetry.sdk.trace.samplers.SamplingDecision +import io.sentry.DataCategory +import io.sentry.IScopes +import io.sentry.SamplingContext +import io.sentry.SentryOptions +import io.sentry.SentryTraceHeader +import io.sentry.SpanId +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.clientreport.DiscardReason +import io.sentry.protocol.SentryId +import kotlin.test.AfterTest +import kotlin.test.Test +import org.mockito.AdditionalAnswers.delegatesTo +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions +import org.mockito.kotlin.whenever + +class SentrySamplerTest { + private val onDiscard = mock() + private val options = + SentryOptions().apply { + tracesSampleRate = 1.0 + profilesSampleRate = 1.0 + tracesSampler = SentryOptions.TracesSamplerCallback { throw IllegalStateException("sampler") } + this.onDiscard = this@SentrySamplerTest.onDiscard + } + private val scopes = mock().also { whenever(it.options).thenReturn(options) } + private val sampler = SentrySampler(scopes) + + @AfterTest + fun tearDown() { + SentryWeakSpanStorage.getInstance().clear() + } + + @Test + fun `throwing tracesSampler drops root and reports callback errors alongside sample rate losses`() { + for (parentSampled in listOf(null, false, true)) { + val traceId = SentryId() + val context = + if (parentSampled == null) Context.root() + else + Context.root() + .with( + SentryOtelKeys.SENTRY_TRACE_KEY, + SentryTraceHeader(traceId, SpanId(), parentSampled), + ) + val result = + sampler.shouldSample( + context, + traceId.toString(), + "root", + SpanKind.INTERNAL, + Attributes.empty(), + emptyList(), + ) as SentrySamplingResult + + assertThat(result.decision).isEqualTo(SamplingDecision.RECORD_ONLY) + assertThat(result.sentryDecision.sampled).isFalse() + assertThat(result.sentryDecision.profileSampled).isFalse() + } + + verify(onDiscard, times(3)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard, times(3)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + verify(onDiscard, times(3)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1) + verify(onDiscard, times(3)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + } + + @Test + fun `children of failed sampling decisions retain sample rate accounting`() { + val rootResult = + sampler.shouldSample( + Context.root(), + SentryId().toString(), + "root", + SpanKind.INTERNAL, + Attributes.empty(), + emptyList(), + ) as SentrySamplingResult + val restored = OtelSamplingUtil.extractSamplingDecision(rootResult.attributes)!! + assertThat(restored.sampled).isFalse() + assertThat(restored.sampleRand).isEqualTo(rootResult.sentryDecision.sampleRand) + + val parentContext = + io.opentelemetry.api.trace.SpanContext.create( + SentryId().toString(), + SpanId().toString(), + TraceFlags.getDefault(), + TraceState.getDefault(), + ) + val parent = + mock().also { + whenever(it.samplingDecision).thenReturn(restored) + } + SentryWeakSpanStorage.getInstance().storeSentrySpan(parentContext, parent) + val childResult = + sampler.shouldSample( + Span.wrap(parentContext).storeInContext(Context.root()), + parentContext.traceId, + "child", + SpanKind.INTERNAL, + Attributes.empty(), + emptyList(), + ) as SentrySamplingResult + + assertThat(childResult.decision).isEqualTo(SamplingDecision.RECORD_ONLY) + assertThat(childResult.sentryDecision.sampled).isFalse() + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1) + verify(onDiscard, times(2)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + } + + @Test + fun `Sentry API sampling failure reports before forwarding through span factory`() { + val context = TransactionContext("root", "op") + context.samplingDecision = + options.internalTracesSampler.sample(SamplingContext(context, null, 0.0, null)) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + val recordingSampler = mock(defaultAnswer = delegatesTo(sampler)) + SdkTracerProvider.builder().setSampler(recordingSampler).build().use { provider -> + val openTelemetry = + mock().also { + whenever(it.tracerProvider).thenReturn(provider) + } + OtelSpanFactory(openTelemetry).createTransaction(context, scopes, TransactionOptions(), null) + val attributes = argumentCaptor() + verify(recordingSampler).shouldSample(any(), any(), any(), any(), attributes.capture(), any()) + val restored = OtelSamplingUtil.extractSamplingDecision(attributes.firstValue)!! + assertThat(restored.sampled).isFalse() + assertThat(restored.profileSampled).isFalse() + } + + verifyNoMoreInteractions(onDiscard) + } + + @Test + fun `null tracesSampler result uses normal sample rate accounting`() { + options.tracesSampler = SentryOptions.TracesSamplerCallback { null } + options.tracesSampleRate = 0.0 + val result = + sampler.shouldSample( + Context.root(), + SentryId().toString(), + "root", + SpanKind.INTERNAL, + Attributes.empty(), + emptyList(), + ) as SentrySamplingResult + + assertThat(result.sentryDecision.sampled).isFalse() + verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + } +} diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index e981bb6f66f..729de9bf185 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.clientreport.DiscardReason; import io.sentry.util.Objects; import io.sentry.util.SampleRateUtils; import org.jetbrains.annotations.ApiStatus; @@ -39,16 +40,21 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont } Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate, sampleRand); - boolean tracesSamplerFailed = false; if (options.getTracesSampler() != null) { - Double samplerResult = null; + final Double samplerResult; try { samplerResult = options.getTracesSampler().sample(samplingContext); } catch (Throwable t) { - tracesSamplerFailed = true; options .getLogger() .log(SentryLevel.ERROR, "Error in the 'TracesSamplerCallback' callback.", t); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Span); + return new TracesSamplingDecision(false, null, sampleRand, false, null); } if (samplerResult != null) { return new TracesSamplingDecision( @@ -66,8 +72,7 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont return SampleRateUtils.backfilledSampleRand(parentSamplingDecision); } - final @Nullable Double tracesSampleRateFromOptions = - tracesSamplerFailed ? null : options.getTracesSampleRate(); + final @Nullable Double tracesSampleRateFromOptions = options.getTracesSampleRate(); final @NotNull Double downsampleFactor = Math.pow(2, options.getBackpressureMonitor().getDownsampleFactor()); final @Nullable Double downsampledTracesSampleRate = diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index e6db21bc791..50a05025ea2 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -1660,6 +1660,121 @@ class ScopesTest { ) } + @Test + fun `tracesSampler failures report callback errors and retain normal sampling loss accounting`() { + for (parentSampled in listOf(null, false, true)) { + for (downsampleFactor in listOf(0, 1)) { + val onDiscard = mock() + val profiler = mock() + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + tracesSampleRate = 1.0 + profilesSampleRate = 1.0 + tracesSampler = SentryOptions.TracesSamplerCallback { + throw IllegalStateException("sampler") + } + this.onDiscard = onDiscard + setTransactionProfiler(profiler) + backpressureMonitor = + mock().also { + whenever(it.downsampleFactor).thenReturn(downsampleFactor) + } + } + val scopes = createScopes(options) + val client = createSentryClientMock() + scopes.bindClient(client) + val context = + TransactionContext("name", "op").apply { + setParentSampled(parentSampled, true) + } + + val transaction = scopes.startTransaction(context) + assertThat(transaction.isSampled).isFalse() + assertThat(transaction.isProfileSampled).isFalse() + transaction.startChild("child").finish() + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + transaction.finish() + transaction.finish() + + verify(client, never()) + .captureTransaction(any(), anyOrNull(), any(), anyOrNull(), anyOrNull()) + verify(profiler, never()).start() + val samplingReason = + if (downsampleFactor > 0) DiscardReason.BACKPRESSURE else DiscardReason.SAMPLE_RATE + verify(onDiscard).execute(samplingReason, DataCategory.Transaction, 1) + verify(onDiscard).execute(samplingReason, DataCategory.Span, 1) + verifyNoMoreInteractions(onDiscard) + assertClientReport( + options.clientReportRecorder, + listOf( + DiscardedEvent( + DiscardReason.CALLBACK_ERROR.reason, + DataCategory.Transaction.category, + 1, + ), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 1), + DiscardedEvent(samplingReason.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(samplingReason.reason, DataCategory.Span.category, 1), + ), + ) + } + } + } + + @Test + fun `tracesSampler failure does not affect subsequent successful sampling`() { + var fail = true + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + tracesSampler = SentryOptions.TracesSamplerCallback { + if (fail) throw IllegalStateException("sampler") else 1.0 + } + } + val scopes = createScopes(options) + val client = createSentryClientMock() + scopes.bindClient(client) + scopes.startTransaction("failed", "op").finish() + fail = false + val transaction = scopes.startTransaction("successful", "op") + transaction.finish() + + assertThat(transaction.isSampled).isTrue() + verify(client).captureTransaction(any(), anyOrNull(), any(), anyOrNull(), anyOrNull()) + assertClientReport( + options.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 1), + DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Span.category, 1), + ), + ) + } + + @Test + fun `null tracesSampler results still use normal sampling loss accounting`() { + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + tracesSampleRate = 0.0 + tracesSampler = SentryOptions.TracesSamplerCallback { null } + } + val scopes = createScopes(options) + scopes.startTransaction("name", "op").finish() + + assertClientReport( + options.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Span.category, 1), + ), + ) + } + @Test fun `transactions lost due to sampling caused by backpressure are recorded as lost`() { val options = SentryOptions() diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index bb257a9c46e..9b3c60cd996 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -1,6 +1,9 @@ package io.sentry import com.google.common.truth.Truth.assertThat +import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport +import io.sentry.clientreport.DiscardReason +import io.sentry.clientreport.DiscardedEvent import io.sentry.protocol.SentryId import io.sentry.util.SentryRandom import kotlin.test.Test @@ -404,7 +407,7 @@ class TracesSamplerTest { } @Test - fun `when tracesSampler throws then a sampled parent decision is inherited`() { + fun `when tracesSampler throws then a sampled parent decision is ignored`() { val sampler = fixture.getSut( tracesSampleRate = 0.0, @@ -416,11 +419,17 @@ class TracesSamplerTest { val decision = sampler.sample(SamplingContext(transactionContext, null, 0.1, null)) - assertThat(decision).isSameInstanceAs(parentDecision) + assertThat(decision.sampled).isFalse() + assertThat(decision.sampleRate).isNull() + assertThat(decision.sampleRand).isEqualTo(0.1) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() + assertThat(parentDecision.sampled).isTrue() + assertThat(parentDecision.profileSampled).isTrue() } @Test - fun `when tracesSampler throws then an unsampled parent decision is inherited`() { + fun `when tracesSampler throws then an unsampled parent decision is ignored`() { val sampler = fixture.getSut( tracesSampleRate = 1.0, @@ -433,11 +442,15 @@ class TracesSamplerTest { val decision = sampler.sample(SamplingContext(transactionContext, null, 0.9, null)) - assertThat(decision).isSameInstanceAs(parentDecision) + assertThat(decision.sampled).isFalse() + assertThat(decision.sampleRate).isNull() + assertThat(decision.sampleRand).isEqualTo(0.9) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() } @Test - fun `when tracesSampler throws then a parent decision without sampleRand is backfilled`() { + fun `when tracesSampler throws then a parent decision without sampleRand is ignored`() { val sampler = fixture.getSut( tracesSampleRate = 0.0, @@ -448,9 +461,46 @@ class TracesSamplerTest { val decision = sampler.sample(SamplingContext(transactionContext, null, 0.1, null)) - assertThat(decision.sampled).isTrue() + assertThat(decision.sampled).isFalse() assertThat(decision.sampleRate).isNull() - assertThat(decision.sampleRand).isNotNull() + assertThat(decision.sampleRand).isEqualTo(0.1) + } + + @Test + fun `tracesSampler failure reports callback errors immediately`() { + val options = + SentryOptions().apply { + tracesSampler = SentryOptions.TracesSamplerCallback { + throw IllegalStateException("sampler") + } + } + val decision = + TracesSampler(options) + .sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) + + assertThat(decision.sampled).isFalse() + assertClientReport( + options.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 1), + ), + ) + } + + @Test + fun `explicit sampling decisions bypass a throwing tracesSampler without reporting losses`() { + val options = + SentryOptions().apply { + tracesSampler = SentryOptions.TracesSamplerCallback { + throw IllegalStateException("sampler") + } + } + val context = TransactionContext("name", "op", TracesSamplingDecision(true)) + val decision = TracesSampler(options).sample(SamplingContext(context, null, 0.1, null)) + + assertThat(decision.sampled).isTrue() + assertClientReport(options.clientReportRecorder, emptyList()) } @Test