Skip to content

Commit 8a06fde

Browse files
adinaueradinauer
committed
chore(core): Sync processor fixes into breadcrumb stack
Merge #6142 forward into #6143 so SDK-owned processor failures preserve\ntelemetry throughout the stack. Keep the breadcrumb-only diff unchanged\nand retain both changelog entries without rewriting existing history.
2 parents aa17ea5 + 6d07426 commit 8a06fde

4 files changed

Lines changed: 310 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@
124124

125125
- Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140))
126126
- Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value.
127-
- Drop telemetry and record `callback_error` when an event processor throws instead of continuing with a potentially partially processed item.
127+
- 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.
129129
- 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)
130130
- Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003))

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

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.sentry.hints.Cached;
99
import io.sentry.hints.DiskFlushNotification;
1010
import io.sentry.hints.TransactionEnd;
11+
import io.sentry.internal.eventprocessor.SentryEventProcessor;
1112
import io.sentry.logger.ILoggerBatchProcessor;
1213
import io.sentry.logger.NoOpLoggerBatchProcessor;
1314
import io.sentry.metrics.IMetricsBatchProcessor;
@@ -506,10 +507,12 @@ private SentryEvent processEvent(
506507
e,
507508
"An exception occurred while processing event by processor: %s",
508509
processor.getClass().getName());
509-
options
510-
.getClientReportRecorder()
511-
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Error);
512-
return null;
510+
if (!(processor instanceof SentryEventProcessor)) {
511+
options
512+
.getClientReportRecorder()
513+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Error);
514+
return null;
515+
}
513516
}
514517

515518
if (event == null) {
@@ -561,8 +564,10 @@ private SentryLogEvent processLogEvent(
561564
e,
562565
"An exception occurred while processing log event by processor: %s",
563566
processor.getClass().getName());
564-
recordLostLogEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor);
565-
return null;
567+
if (!(processor instanceof SentryEventProcessor)) {
568+
recordLostLogEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor);
569+
return null;
570+
}
566571
}
567572

568573
if (event == null) {
@@ -596,8 +601,10 @@ private SentryMetricsEvent processMetricsEvent(
596601
e,
597602
"An exception occurred while processing metrics event by processor: %s",
598603
processor.getClass().getName());
599-
recordLostMetricsEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor);
600-
return null;
604+
if (!(processor instanceof SentryEventProcessor)) {
605+
recordLostMetricsEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor);
606+
return null;
607+
}
601608
}
602609

603610
if (event == null) {
@@ -630,14 +637,16 @@ private SentryMetricsEvent processMetricsEvent(
630637
e,
631638
"An exception occurred while processing transaction by processor: %s",
632639
processor.getClass().getName());
633-
options
634-
.getClientReportRecorder()
635-
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction);
636-
options
637-
.getClientReportRecorder()
638-
.recordLostEvent(
639-
DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeProcessor + 1);
640-
return null;
640+
if (!(processor instanceof SentryEventProcessor)) {
641+
options
642+
.getClientReportRecorder()
643+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction);
644+
options
645+
.getClientReportRecorder()
646+
.recordLostEvent(
647+
DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeProcessor + 1);
648+
return null;
649+
}
641650
}
642651
final int spanCountAfterProcessor = transaction == null ? 0 : transaction.getSpans().size();
643652

@@ -691,10 +700,12 @@ private SentryReplayEvent processReplayEvent(
691700
e,
692701
"An exception occurred while processing replay event by processor: %s",
693702
processor.getClass().getName());
694-
options
695-
.getClientReportRecorder()
696-
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay);
697-
return null;
703+
if (!(processor instanceof SentryEventProcessor)) {
704+
options
705+
.getClientReportRecorder()
706+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay);
707+
return null;
708+
}
698709
}
699710

700711
if (replayEvent == null) {
@@ -729,10 +740,12 @@ private SentryEvent processFeedbackEvent(
729740
e,
730741
"An exception occurred while processing feedback event by processor: %s",
731742
processor.getClass().getName());
732-
options
733-
.getClientReportRecorder()
734-
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback);
735-
return null;
743+
if (!(processor instanceof SentryEventProcessor)) {
744+
options
745+
.getClientReportRecorder()
746+
.recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback);
747+
return null;
748+
}
736749
}
737750

738751
if (feedbackEvent == null) {

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

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
package io.sentry
2+
3+
import com.google.common.truth.Truth.assertThat
4+
import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport
5+
import io.sentry.clientreport.DiscardReason
6+
import io.sentry.clientreport.DiscardedEvent
7+
import io.sentry.internal.eventprocessor.SentryEventProcessor
8+
import io.sentry.protocol.Feedback
9+
import io.sentry.protocol.SentryId
10+
import io.sentry.protocol.SentryTransaction
11+
import kotlin.test.Test
12+
import org.junit.runner.RunWith
13+
import org.junit.runners.Parameterized
14+
import org.mockito.kotlin.any
15+
import org.mockito.kotlin.anyOrNull
16+
import org.mockito.kotlin.check
17+
import org.mockito.kotlin.doAnswer
18+
import org.mockito.kotlin.eq
19+
import org.mockito.kotlin.mock
20+
import org.mockito.kotlin.never
21+
import org.mockito.kotlin.same
22+
import org.mockito.kotlin.verify
23+
import org.mockito.kotlin.verifyNoInteractions
24+
import org.mockito.kotlin.whenever
25+
26+
@RunWith(Parameterized::class)
27+
class SentryClientInternalEventProcessorTest(private val onScope: Boolean) {
28+
companion object {
29+
@JvmStatic
30+
@Parameterized.Parameters(name = "onScope={0}")
31+
fun data(): List<Array<Boolean>> = listOf(arrayOf(false), arrayOf(true))
32+
}
33+
34+
private val fixture = SentryClientTest.Fixture()
35+
private val options = fixture.sentryOptions
36+
private val scope = Scope(options)
37+
private val processor = mock<SentryEventProcessor>()
38+
private val nextProcessor = mock<EventProcessor>()
39+
private val onDiscard = mock<SentryOptions.OnDiscardCallback>()
40+
private val logger = mock<ILogger>()
41+
private val failure = IllegalStateException("SDK processor failed")
42+
43+
init {
44+
options.eventProcessors.clear()
45+
options.onDiscard = onDiscard
46+
options.setLogger(logger)
47+
options.logs.isEnabled = true
48+
options.metrics.isEnabled = true
49+
if (onScope) {
50+
scope.addEventProcessor(processor)
51+
scope.addEventProcessor(nextProcessor)
52+
} else {
53+
options.addEventProcessor(processor)
54+
options.addEventProcessor(nextProcessor)
55+
}
56+
}
57+
58+
@Test
59+
fun `SDK event processor failure keeps event and runs remaining callbacks`() {
60+
val event = SentryEvent()
61+
val beforeSend = mock<SentryOptions.BeforeSendCallback>()
62+
whenever(processor.process(any<SentryEvent>(), any())).thenThrow(failure)
63+
whenever(nextProcessor.process(any<SentryEvent>(), any())).thenAnswer { it.arguments[0] }
64+
whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] }
65+
options.beforeSend = beforeSend
66+
67+
val id = fixture.getSut().captureEvent(event, scope)
68+
69+
assertThat(id).isEqualTo(event.eventId)
70+
verify(nextProcessor).process(same(event), any())
71+
verify(beforeSend).execute(same(event), any())
72+
verify(fixture.transport)
73+
.send(check { assertThat(it.header.eventId).isEqualTo(id) }, anyOrNull())
74+
assertFailureLoggedWithoutLoss("event")
75+
}
76+
77+
@Test
78+
fun `SDK transaction processor failure keeps transaction and spans`() {
79+
val transaction = SentryTransaction(fixture.sentryTracer)
80+
val beforeSend = mock<SentryOptions.BeforeSendTransactionCallback>()
81+
whenever(processor.process(any<SentryTransaction>(), any())).thenThrow(failure)
82+
whenever(nextProcessor.process(any<SentryTransaction>(), any())).thenAnswer { it.arguments[0] }
83+
whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] }
84+
options.beforeSendTransaction = beforeSend
85+
86+
val id = fixture.getSut().captureTransaction(transaction, scope, null)
87+
88+
assertThat(id).isEqualTo(transaction.eventId)
89+
verify(nextProcessor).process(same(transaction), any())
90+
verify(beforeSend).execute(same(transaction), any())
91+
verify(fixture.transport)
92+
.send(
93+
check {
94+
val sent = it.items.first().getTransaction(options.serializer)!!
95+
assertThat(sent.eventId).isEqualTo(id)
96+
assertThat(sent.spans).hasSize(1)
97+
},
98+
anyOrNull(),
99+
)
100+
assertFailureLoggedWithoutLoss("transaction")
101+
}
102+
103+
@Test
104+
fun `SDK feedback processor failure keeps feedback and runs remaining callbacks`() {
105+
val feedback = Feedback("message")
106+
val beforeSend = mock<SentryOptions.BeforeSendCallback>()
107+
whenever(processor.process(any<SentryEvent>(), any())).thenThrow(failure)
108+
whenever(nextProcessor.process(any<SentryEvent>(), any())).thenAnswer { it.arguments[0] }
109+
whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] }
110+
options.beforeSendFeedback = beforeSend
111+
112+
val id = fixture.getSut().captureFeedback(feedback, null, scope)
113+
114+
assertThat(id).isNotEqualTo(SentryId.EMPTY_ID)
115+
verify(nextProcessor)
116+
.process(
117+
check<SentryEvent> {
118+
assertThat(it.contexts.feedback).isSameInstanceAs(feedback)
119+
},
120+
any(),
121+
)
122+
verify(beforeSend).execute(check { assertThat(it.eventId).isEqualTo(id) }, any())
123+
verify(fixture.transport)
124+
.send(check { assertThat(it.header.eventId).isEqualTo(id) }, anyOrNull())
125+
assertFailureLoggedWithoutLoss("feedback event")
126+
}
127+
128+
@Test
129+
fun `SDK log processor failure keeps log and runs remaining callbacks`() {
130+
val event = SentryLogEvent(SentryId(), SentryNanotimeDate(), "message", SentryLogLevel.WARN)
131+
val beforeSend = mock<SentryOptions.Logs.BeforeSendLogCallback>()
132+
whenever(processor.process(any<SentryLogEvent>())).thenThrow(failure)
133+
whenever(nextProcessor.process(any<SentryLogEvent>())).thenAnswer { it.arguments[0] }
134+
whenever(beforeSend.execute(any())).thenAnswer { it.arguments[0] }
135+
options.logs.beforeSend = beforeSend
136+
137+
fixture.getSut().captureLog(event, scope)
138+
139+
verify(nextProcessor).process(same(event))
140+
verify(beforeSend).execute(same(event))
141+
verify(fixture.loggerBatchProcessor).add(same(event))
142+
assertFailureLoggedWithoutLoss("log event")
143+
}
144+
145+
@Test
146+
fun `SDK metric processor failure keeps metric and runs remaining callbacks`() {
147+
val event = SentryMetricsEvent(SentryId(), SentryNanotimeDate(), "name", "gauge", 123.0)
148+
val beforeSend = mock<SentryOptions.Metrics.BeforeSendMetricCallback>()
149+
whenever(processor.process(any<SentryMetricsEvent>(), any())).thenThrow(failure)
150+
whenever(nextProcessor.process(any<SentryMetricsEvent>(), any())).thenAnswer { it.arguments[0] }
151+
whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] }
152+
options.metrics.beforeSend = beforeSend
153+
154+
fixture.getSut().captureMetric(event, scope, null)
155+
156+
verify(nextProcessor).process(same(event), any())
157+
verify(beforeSend).execute(same(event), any())
158+
verify(fixture.metricsBatchProcessor).add(same(event))
159+
assertFailureLoggedWithoutLoss("metrics event")
160+
}
161+
162+
@Test
163+
fun `SDK processor returning null still drops event as event_processor`() {
164+
whenever(processor.process(any<SentryEvent>(), any())).thenReturn(null)
165+
166+
val id = fixture.getSut().captureEvent(SentryEvent(), scope)
167+
168+
assertThat(id).isEqualTo(SentryId.EMPTY_ID)
169+
verify(nextProcessor, never()).process(any<SentryEvent>(), any())
170+
verify(fixture.transport, never()).send(any(), anyOrNull())
171+
assertClientReport(
172+
options.clientReportRecorder,
173+
listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Error.category, 1)),
174+
)
175+
}
176+
177+
@Test
178+
fun `customer processor failure after SDK processor failure still drops event`() {
179+
whenever(processor.process(any<SentryEvent>(), any())).thenThrow(failure)
180+
whenever(nextProcessor.process(any<SentryEvent>(), any()))
181+
.thenThrow(IllegalArgumentException("customer"))
182+
val beforeSend = mock<SentryOptions.BeforeSendCallback>()
183+
options.beforeSend = beforeSend
184+
185+
val id = fixture.getSut().captureEvent(SentryEvent(), scope)
186+
187+
assertThat(id).isEqualTo(SentryId.EMPTY_ID)
188+
verify(nextProcessor).process(any<SentryEvent>(), any())
189+
verifyNoInteractions(beforeSend)
190+
verify(fixture.transport, never()).send(any(), anyOrNull())
191+
assertClientReport(
192+
options.clientReportRecorder,
193+
listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Error.category, 1)),
194+
)
195+
verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Error, 1)
196+
}
197+
198+
@Test
199+
fun `spans removed before SDK processor failure retain event_processor accounting`() {
200+
val transaction = SentryTransaction(fixture.sentryTracer)
201+
whenever(processor.process(any<SentryTransaction>(), any())).doAnswer {
202+
transaction.spans.clear()
203+
throw failure
204+
}
205+
whenever(nextProcessor.process(any<SentryTransaction>(), any())).thenAnswer { it.arguments[0] }
206+
207+
val id = fixture.getSut().captureTransaction(transaction, scope, null)
208+
209+
assertThat(id).isEqualTo(transaction.eventId)
210+
verify(nextProcessor).process(same(transaction), any())
211+
verify(fixture.transport).send(any(), anyOrNull())
212+
assertClientReport(
213+
options.clientReportRecorder,
214+
listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Span.category, 1)),
215+
)
216+
}
217+
218+
private fun assertFailureLoggedWithoutLoss(item: String) {
219+
verify(logger)
220+
.log(
221+
eq(SentryLevel.ERROR),
222+
same(failure),
223+
eq("An exception occurred while processing $item by processor: %s"),
224+
eq(processor.javaClass.name),
225+
)
226+
assertClientReport(options.clientReportRecorder, emptyList())
227+
verifyNoInteractions(onDiscard)
228+
}
229+
}

0 commit comments

Comments
 (0)