diff --git a/CHANGELOG.md b/CHANGELOG.md index 79616655acf..8c65c25112a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,7 @@ - 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, 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)) + - When `profilesSampler` throws, disable profiling instead of falling back to `profilesSampleRate` or inheriting the parent's profiling decision. Trace sampling is unchanged ([#6164](https://github.com/getsentry/sentry-java/pull/6164)) - 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 729de9bf185..bb6e5d9f11f 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -26,16 +26,18 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont } Double profilesSampleRate = null; + boolean profilesSamplerFailed = false; if (options.getProfilesSampler() != null) { try { profilesSampleRate = options.getProfilesSampler().sample(samplingContext); } catch (Throwable t) { + profilesSamplerFailed = true; options .getLogger() .log(SentryLevel.ERROR, "Error in the 'ProfilesSamplerCallback' callback.", t); } } - if (profilesSampleRate == null) { + if (profilesSampleRate == null && !profilesSamplerFailed) { profilesSampleRate = options.getProfilesSampleRate(); } Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate, sampleRand); @@ -69,6 +71,13 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont final TracesSamplingDecision parentSamplingDecision = samplingContext.getTransactionContext().getParentSamplingDecision(); if (parentSamplingDecision != null) { + if (profilesSamplerFailed) { + return SampleRateUtils.backfilledSampleRand( + new TracesSamplingDecision( + parentSamplingDecision.getSampled(), + parentSamplingDecision.getSampleRate(), + parentSamplingDecision.getSampleRand())); + } return SampleRateUtils.backfilledSampleRand(parentSamplingDecision); } diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 9b3c60cd996..9b8f0666854 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -194,7 +194,7 @@ class TracesSamplerTest { @Test fun `when profilesSampler returns null and parentSampled is set sampler uses it as a sampling decision`() { - val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSamplerCallback = null) + val sampler = fixture.getSut(tracesSampleRate = 1.0, profilesSamplerCallback = { null }) val transactionContextParentSampled = TransactionContext("name", "op") transactionContextParentSampled.setParentSampled(true, true) val samplingDecision = @@ -225,7 +225,7 @@ class TracesSamplerTest { fixture.getSut( tracesSampleRate = 1.0, profilesSampleRate = 0.2, - profilesSamplerCallback = null, + profilesSamplerCallback = { null }, ) val samplingDecision = sampler.sample( @@ -358,18 +358,92 @@ class TracesSamplerTest { } @Test - 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") + fun `when profilesSampler throws then static profile rates are ignored`() { + for (profilesSampleRate in listOf(null, 0.0, 1.0)) { + val sampler = + fixture.getSut( + tracesSampleRate = 1.0, + profilesSampleRate = profilesSampleRate, + profilesSamplerCallback = { + throw IllegalStateException("faulty ProfilesSamplerCallback") + }, + ) + val decision = + sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) + + assertThat(decision.sampled).isTrue() + assertThat(decision.sampleRate).isEqualTo(1.0) + assertThat(decision.sampleRand).isEqualTo(0.0) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() + } + } + + @Test + fun `when profilesSampler throws then tracesSampler still determines trace sampling`() { val sampler = fixture.getSut( - tracesSampleRate = 1.0, + tracesSampleRate = 0.0, profilesSampleRate = 1.0, - profilesSamplerCallback = { throw exception }, + tracesSamplerCallback = { 0.5 }, + profilesSamplerCallback = { throw IllegalStateException("faulty ProfilesSamplerCallback") }, ) val decision = - sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) - assertTrue(decision.profileSampled) - assertEquals(0.0, decision.sampleRand) + sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.1, null)) + + assertThat(decision.sampled).isTrue() + assertThat(decision.sampleRate).isEqualTo(0.5) + assertThat(decision.sampleRand).isEqualTo(0.1) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() + } + + @Test + fun `when profilesSampler throws then parent trace sampling is preserved without profiling`() { + val sampler = + fixture.getSut( + tracesSampleRate = 1.0, + profilesSampleRate = 1.0, + profilesSamplerCallback = { throw IllegalStateException("faulty ProfilesSamplerCallback") }, + ) + for (sampled in listOf(true, false)) { + val sampleRand = if (sampled) 0.1 else 0.9 + val parentDecision = TracesSamplingDecision(sampled, 0.5, sampleRand, true, 1.0) + val transactionContext = + TransactionContext(SentryId(), SpanId(), SpanId(), parentDecision, null) + + val decision = sampler.sample(SamplingContext(transactionContext, null, sampleRand, null)) + + assertThat(decision.sampled).isEqualTo(sampled) + assertThat(decision.sampleRate).isEqualTo(0.5) + assertThat(decision.sampleRand).isEqualTo(sampleRand) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() + assertThat(parentDecision.profileSampled).isEqualTo(sampled) + assertThat(parentDecision.profileSampleRate).isEqualTo(1.0) + } + } + + @Test + fun `when both samplers throw then tracing and profiling are disabled despite a sampled parent`() { + val sampler = + fixture.getSut( + tracesSampleRate = 0.0, + profilesSampleRate = 1.0, + tracesSamplerCallback = { throw IllegalStateException("faulty TracesSamplerCallback") }, + profilesSamplerCallback = { throw IllegalStateException("faulty ProfilesSamplerCallback") }, + ) + val parentDecision = TracesSamplingDecision(true, 0.5, true, 1.0) + val transactionContext = + TransactionContext(SentryId(), SpanId(), SpanId(), parentDecision, null) + + val decision = sampler.sample(SamplingContext(transactionContext, null, 0.9, null)) + + assertThat(decision.sampled).isFalse() + assertThat(decision.sampleRate).isNull() + assertThat(decision.sampleRand).isEqualTo(0.9) + assertThat(decision.profileSampled).isFalse() + assertThat(decision.profileSampleRate).isNull() } @Test