Skip to content

Commit 154d94e

Browse files
adinauerclaude
adinauer
and
claude
committed
fix(core): Sync trace sampler handling into profile sampling
Merge the updated tracesSampler failure behavior from #6163 into #6164. Preserve profile-sampler isolation and update the combined-failure test to expect tracing and profiling to be disabled, even with a sampled parent. Refs #6163 Refs #6164 Co-Authored-By: Claude <noreply@anthropic.com>
2 parents aa15164 + fcbf95f commit 154d94e

5 files changed

Lines changed: 363 additions & 18 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@
126126
- Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value.
127127
- 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.
128128
- Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb.
129-
- 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))
129+
- 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))
130130
- 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))
131131
- 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)
132132
- Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003))

‎sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/SentrySamplerTest.kt‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
package io.sentry.opentelemetry
2+
3+
import com.google.common.truth.Truth.assertThat
4+
import io.opentelemetry.api.OpenTelemetry
5+
import io.opentelemetry.api.common.Attributes
6+
import io.opentelemetry.api.trace.Span
7+
import io.opentelemetry.api.trace.SpanKind
8+
import io.opentelemetry.api.trace.TraceFlags
9+
import io.opentelemetry.api.trace.TraceState
10+
import io.opentelemetry.context.Context
11+
import io.opentelemetry.sdk.trace.SdkTracerProvider
12+
import io.opentelemetry.sdk.trace.samplers.Sampler
13+
import io.opentelemetry.sdk.trace.samplers.SamplingDecision
14+
import io.sentry.DataCategory
15+
import io.sentry.IScopes
16+
import io.sentry.SamplingContext
17+
import io.sentry.SentryOptions
18+
import io.sentry.SentryTraceHeader
19+
import io.sentry.SpanId
20+
import io.sentry.TransactionContext
21+
import io.sentry.TransactionOptions
22+
import io.sentry.clientreport.DiscardReason
23+
import io.sentry.protocol.SentryId
24+
import kotlin.test.AfterTest
25+
import kotlin.test.Test
26+
import org.mockito.AdditionalAnswers.delegatesTo
27+
import org.mockito.kotlin.any
28+
import org.mockito.kotlin.argumentCaptor
29+
import org.mockito.kotlin.mock
30+
import org.mockito.kotlin.times
31+
import org.mockito.kotlin.verify
32+
import org.mockito.kotlin.verifyNoMoreInteractions
33+
import org.mockito.kotlin.whenever
34+
35+
class SentrySamplerTest {
36+
private val onDiscard = mock<SentryOptions.OnDiscardCallback>()
37+
private val options =
38+
SentryOptions().apply {
39+
tracesSampleRate = 1.0
40+
profilesSampleRate = 1.0
41+
tracesSampler = SentryOptions.TracesSamplerCallback { throw IllegalStateException("sampler") }
42+
this.onDiscard = this@SentrySamplerTest.onDiscard
43+
}
44+
private val scopes = mock<IScopes>().also { whenever(it.options).thenReturn(options) }
45+
private val sampler = SentrySampler(scopes)
46+
47+
@AfterTest
48+
fun tearDown() {
49+
SentryWeakSpanStorage.getInstance().clear()
50+
}
51+
52+
@Test
53+
fun `throwing tracesSampler drops root and reports callback errors alongside sample rate losses`() {
54+
for (parentSampled in listOf(null, false, true)) {
55+
val traceId = SentryId()
56+
val context =
57+
if (parentSampled == null) Context.root()
58+
else
59+
Context.root()
60+
.with(
61+
SentryOtelKeys.SENTRY_TRACE_KEY,
62+
SentryTraceHeader(traceId, SpanId(), parentSampled),
63+
)
64+
val result =
65+
sampler.shouldSample(
66+
context,
67+
traceId.toString(),
68+
"root",
69+
SpanKind.INTERNAL,
70+
Attributes.empty(),
71+
emptyList(),
72+
) as SentrySamplingResult
73+
74+
assertThat(result.decision).isEqualTo(SamplingDecision.RECORD_ONLY)
75+
assertThat(result.sentryDecision.sampled).isFalse()
76+
assertThat(result.sentryDecision.profileSampled).isFalse()
77+
}
78+
79+
verify(onDiscard, times(3)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1)
80+
verify(onDiscard, times(3)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1)
81+
verify(onDiscard, times(3)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1)
82+
verify(onDiscard, times(3)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1)
83+
verifyNoMoreInteractions(onDiscard)
84+
}
85+
86+
@Test
87+
fun `children of failed sampling decisions retain sample rate accounting`() {
88+
val rootResult =
89+
sampler.shouldSample(
90+
Context.root(),
91+
SentryId().toString(),
92+
"root",
93+
SpanKind.INTERNAL,
94+
Attributes.empty(),
95+
emptyList(),
96+
) as SentrySamplingResult
97+
val restored = OtelSamplingUtil.extractSamplingDecision(rootResult.attributes)!!
98+
assertThat(restored.sampled).isFalse()
99+
assertThat(restored.sampleRand).isEqualTo(rootResult.sentryDecision.sampleRand)
100+
101+
val parentContext =
102+
io.opentelemetry.api.trace.SpanContext.create(
103+
SentryId().toString(),
104+
SpanId().toString(),
105+
TraceFlags.getDefault(),
106+
TraceState.getDefault(),
107+
)
108+
val parent =
109+
mock<IOtelSpanWrapper>().also {
110+
whenever(it.samplingDecision).thenReturn(restored)
111+
}
112+
SentryWeakSpanStorage.getInstance().storeSentrySpan(parentContext, parent)
113+
val childResult =
114+
sampler.shouldSample(
115+
Span.wrap(parentContext).storeInContext(Context.root()),
116+
parentContext.traceId,
117+
"child",
118+
SpanKind.INTERNAL,
119+
Attributes.empty(),
120+
emptyList(),
121+
) as SentrySamplingResult
122+
123+
assertThat(childResult.decision).isEqualTo(SamplingDecision.RECORD_ONLY)
124+
assertThat(childResult.sentryDecision.sampled).isFalse()
125+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1)
126+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1)
127+
verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1)
128+
verify(onDiscard, times(2)).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1)
129+
verifyNoMoreInteractions(onDiscard)
130+
}
131+
132+
@Test
133+
fun `Sentry API sampling failure reports before forwarding through span factory`() {
134+
val context = TransactionContext("root", "op")
135+
context.samplingDecision =
136+
options.internalTracesSampler.sample(SamplingContext(context, null, 0.0, null))
137+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1)
138+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1)
139+
verifyNoMoreInteractions(onDiscard)
140+
val recordingSampler = mock<Sampler>(defaultAnswer = delegatesTo(sampler))
141+
SdkTracerProvider.builder().setSampler(recordingSampler).build().use { provider ->
142+
val openTelemetry =
143+
mock<OpenTelemetry>().also {
144+
whenever(it.tracerProvider).thenReturn(provider)
145+
}
146+
OtelSpanFactory(openTelemetry).createTransaction(context, scopes, TransactionOptions(), null)
147+
val attributes = argumentCaptor<Attributes>()
148+
verify(recordingSampler).shouldSample(any(), any(), any(), any(), attributes.capture(), any())
149+
val restored = OtelSamplingUtil.extractSamplingDecision(attributes.firstValue)!!
150+
assertThat(restored.sampled).isFalse()
151+
assertThat(restored.profileSampled).isFalse()
152+
}
153+
154+
verifyNoMoreInteractions(onDiscard)
155+
}
156+
157+
@Test
158+
fun `null tracesSampler result uses normal sample rate accounting`() {
159+
options.tracesSampler = SentryOptions.TracesSamplerCallback { null }
160+
options.tracesSampleRate = 0.0
161+
val result =
162+
sampler.shouldSample(
163+
Context.root(),
164+
SentryId().toString(),
165+
"root",
166+
SpanKind.INTERNAL,
167+
Attributes.empty(),
168+
emptyList(),
169+
) as SentrySamplingResult
170+
171+
assertThat(result.sentryDecision.sampled).isFalse()
172+
verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Transaction, 1)
173+
verify(onDiscard).execute(DiscardReason.SAMPLE_RATE, DataCategory.Span, 1)
174+
verifyNoMoreInteractions(onDiscard)
175+
}
176+
}

‎sentry/src/main/java/io/sentry/TracesSampler.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.sentry;
22

3+
import io.sentry.clientreport.DiscardReason;
34
import io.sentry.util.Objects;
45
import io.sentry.util.SampleRateUtils;
56
import org.jetbrains.annotations.ApiStatus;
@@ -41,16 +42,21 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont
4142
}
4243
Boolean profilesSampled = profilesSampleRate != null && sample(profilesSampleRate, sampleRand);
4344

44-
boolean tracesSamplerFailed = false;
4545
if (options.getTracesSampler() != null) {
46-
Double samplerResult = null;
46+
final Double samplerResult;
4747
try {
4848
samplerResult = options.getTracesSampler().sample(samplingContext);
4949
} catch (Throwable t) {
50-
tracesSamplerFailed = true;
5150
options
5251
.getLogger()
5352
.log(SentryLevel.ERROR, "Error in the 'TracesSamplerCallback' callback.", t);
53+
options
54+
.getClientReportRecorder()
55+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction);
56+
options
57+
.getClientReportRecorder()
58+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Span);
59+
return new TracesSamplingDecision(false, null, sampleRand, false, null);
5460
}
5561
if (samplerResult != null) {
5662
return new TracesSamplingDecision(
@@ -75,8 +81,7 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont
7581
return SampleRateUtils.backfilledSampleRand(parentSamplingDecision);
7682
}
7783

78-
final @Nullable Double tracesSampleRateFromOptions =
79-
tracesSamplerFailed ? null : options.getTracesSampleRate();
84+
final @Nullable Double tracesSampleRateFromOptions = options.getTracesSampleRate();
8085
final @NotNull Double downsampleFactor =
8186
Math.pow(2, options.getBackpressureMonitor().getDownsampleFactor());
8287
final @Nullable Double downsampledTracesSampleRate =

‎sentry/src/test/java/io/sentry/ScopesTest.kt‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,121 @@ class ScopesTest {
16601660
)
16611661
}
16621662

1663+
@Test
1664+
fun `tracesSampler failures report callback errors and retain normal sampling loss accounting`() {
1665+
for (parentSampled in listOf(null, false, true)) {
1666+
for (downsampleFactor in listOf(0, 1)) {
1667+
val onDiscard = mock<SentryOptions.OnDiscardCallback>()
1668+
val profiler = mock<ITransactionProfiler>()
1669+
val options =
1670+
SentryOptions().apply {
1671+
dsn = "https://key@sentry.io/proj"
1672+
tracesSampleRate = 1.0
1673+
profilesSampleRate = 1.0
1674+
tracesSampler = SentryOptions.TracesSamplerCallback {
1675+
throw IllegalStateException("sampler")
1676+
}
1677+
this.onDiscard = onDiscard
1678+
setTransactionProfiler(profiler)
1679+
backpressureMonitor =
1680+
mock<IBackpressureMonitor>().also {
1681+
whenever(it.downsampleFactor).thenReturn(downsampleFactor)
1682+
}
1683+
}
1684+
val scopes = createScopes(options)
1685+
val client = createSentryClientMock()
1686+
scopes.bindClient(client)
1687+
val context =
1688+
TransactionContext("name", "op").apply {
1689+
setParentSampled(parentSampled, true)
1690+
}
1691+
1692+
val transaction = scopes.startTransaction(context)
1693+
assertThat(transaction.isSampled).isFalse()
1694+
assertThat(transaction.isProfileSampled).isFalse()
1695+
transaction.startChild("child").finish()
1696+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1)
1697+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1)
1698+
verifyNoMoreInteractions(onDiscard)
1699+
transaction.finish()
1700+
transaction.finish()
1701+
1702+
verify(client, never())
1703+
.captureTransaction(any(), anyOrNull(), any(), anyOrNull(), anyOrNull())
1704+
verify(profiler, never()).start()
1705+
val samplingReason =
1706+
if (downsampleFactor > 0) DiscardReason.BACKPRESSURE else DiscardReason.SAMPLE_RATE
1707+
verify(onDiscard).execute(samplingReason, DataCategory.Transaction, 1)
1708+
verify(onDiscard).execute(samplingReason, DataCategory.Span, 1)
1709+
verifyNoMoreInteractions(onDiscard)
1710+
assertClientReport(
1711+
options.clientReportRecorder,
1712+
listOf(
1713+
DiscardedEvent(
1714+
DiscardReason.CALLBACK_ERROR.reason,
1715+
DataCategory.Transaction.category,
1716+
1,
1717+
),
1718+
DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 1),
1719+
DiscardedEvent(samplingReason.reason, DataCategory.Transaction.category, 1),
1720+
DiscardedEvent(samplingReason.reason, DataCategory.Span.category, 1),
1721+
),
1722+
)
1723+
}
1724+
}
1725+
}
1726+
1727+
@Test
1728+
fun `tracesSampler failure does not affect subsequent successful sampling`() {
1729+
var fail = true
1730+
val options =
1731+
SentryOptions().apply {
1732+
dsn = "https://key@sentry.io/proj"
1733+
tracesSampler = SentryOptions.TracesSamplerCallback {
1734+
if (fail) throw IllegalStateException("sampler") else 1.0
1735+
}
1736+
}
1737+
val scopes = createScopes(options)
1738+
val client = createSentryClientMock()
1739+
scopes.bindClient(client)
1740+
scopes.startTransaction("failed", "op").finish()
1741+
fail = false
1742+
val transaction = scopes.startTransaction("successful", "op")
1743+
transaction.finish()
1744+
1745+
assertThat(transaction.isSampled).isTrue()
1746+
verify(client).captureTransaction(any(), anyOrNull(), any(), anyOrNull(), anyOrNull())
1747+
assertClientReport(
1748+
options.clientReportRecorder,
1749+
listOf(
1750+
DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1),
1751+
DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 1),
1752+
DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Transaction.category, 1),
1753+
DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Span.category, 1),
1754+
),
1755+
)
1756+
}
1757+
1758+
@Test
1759+
fun `null tracesSampler results still use normal sampling loss accounting`() {
1760+
val options =
1761+
SentryOptions().apply {
1762+
dsn = "https://key@sentry.io/proj"
1763+
tracesSampleRate = 0.0
1764+
tracesSampler = SentryOptions.TracesSamplerCallback { null }
1765+
}
1766+
val scopes = createScopes(options)
1767+
scopes.startTransaction("name", "op").finish()
1768+
1769+
assertClientReport(
1770+
options.clientReportRecorder,
1771+
listOf(
1772+
DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Transaction.category, 1),
1773+
DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Span.category, 1),
1774+
),
1775+
)
1776+
}
1777+
16631778
@Test
16641779
fun `transactions lost due to sampling caused by backpressure are recorded as lost`() {
16651780
val options = SentryOptions()

0 commit comments

Comments
 (0)