diff --git a/CHANGELOG.md b/CHANGELOG.md index bb28d7ad52..4af90e40ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,7 @@ - Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140)) - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value. + - Report attached profiles dropped by transaction callback errors as `callback_error` in client reports and `OnDiscardCallback` ([#6166](https://github.com/getsentry/sentry-java/pull/6166)) - 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. - Skip replay capture when `beforeErrorSampling` throws, while still sending the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index e409603f80..75b2b47441 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -624,7 +624,8 @@ private SentryMetricsEvent processMetricsEvent( private @Nullable SentryTransaction processTransaction( @NotNull SentryTransaction transaction, final @NotNull Hint hint, - final @NotNull List eventProcessors) { + final @NotNull List eventProcessors, + final boolean hasProfile) { for (final EventProcessor processor : eventProcessors) { final int spanCountBeforeProcessor = transaction.getSpans().size(); try { @@ -645,6 +646,11 @@ private SentryMetricsEvent processMetricsEvent( .getClientReportRecorder() .recordLostEvent( DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeProcessor + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile); + } return null; } } @@ -1061,7 +1067,9 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint transaction = applyScope(transaction, scope, HintUtils.hasType(hint, Cached.class)); if (transaction != null && scope != null) { - transaction = processTransaction(transaction, hint, scope.getEventProcessors()); + transaction = + processTransaction( + transaction, hint, scope.getEventProcessors(), profilingTraceData != null); } if (transaction == null) { @@ -1070,7 +1078,9 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint } if (transaction != null) { - transaction = processTransaction(transaction, hint, options.getEventProcessors()); + transaction = + processTransaction( + transaction, hint, options.getEventProcessors(), profilingTraceData != null); } if (transaction == null) { @@ -1078,7 +1088,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint return SentryId.EMPTY_ID; } - transaction = executeBeforeSendTransaction(transaction, hint); + transaction = executeBeforeSendTransaction(transaction, hint, profilingTraceData != null); if (transaction == null) { options @@ -1672,7 +1682,7 @@ private void sortBreadcrumbsByDate( } private @Nullable SentryTransaction executeBeforeSendTransaction( - @NotNull SentryTransaction transaction, final @NotNull Hint hint) { + @NotNull SentryTransaction transaction, final @NotNull Hint hint, final boolean hasProfile) { final SentryOptions.BeforeSendTransactionCallback beforeSendTransaction = options.getBeforeSendTransaction(); if (beforeSendTransaction != null) { @@ -1693,6 +1703,11 @@ private void sortBreadcrumbsByDate( .getClientReportRecorder() .recordLostEvent( DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeCallback + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile); + } return null; } diff --git a/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt b/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt index 59aaaa0a1a..6b7934e368 100644 --- a/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt @@ -100,6 +100,35 @@ class SentryClientInternalEventProcessorTest(private val onScope: Boolean) { assertFailureLoggedWithoutLoss("transaction") } + @Test + fun `SDK transaction processor failure keeps attached profile without reporting loss`() { + val transaction = SentryTransaction(fixture.sentryTracer) + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + + val id = + fixture + .getSut() + .captureTransaction( + transaction, + fixture.sentryTracer.traceContext(), + scope, + null, + fixture.profilingTraceData, + ) + + assertThat(id).isEqualTo(transaction.eventId) + verify(fixture.transport) + .send( + check { + assertThat(it.items.map { item -> item.header.type }) + .containsExactly(SentryItemType.Transaction, SentryItemType.Profile) + }, + anyOrNull(), + ) + assertFailureLoggedWithoutLoss("transaction") + } + @Test fun `SDK feedback processor failure keeps feedback and runs remaining callbacks`() { val feedback = Feedback("message") diff --git a/sentry/src/test/java/io/sentry/SentryClientTransactionProfileTest.kt b/sentry/src/test/java/io/sentry/SentryClientTransactionProfileTest.kt new file mode 100644 index 0000000000..dc85ca3817 --- /dev/null +++ b/sentry/src/test/java/io/sentry/SentryClientTransactionProfileTest.kt @@ -0,0 +1,84 @@ +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.protocol.SentryTransaction +import kotlin.test.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions +import org.mockito.kotlin.whenever + +@RunWith(Parameterized::class) +class SentryClientTransactionProfileTest( + private val callback: String, + private val hasProfile: Boolean, +) { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "callback={0}, hasProfile={1}") + fun data(): List> = + listOf("scope processor", "options processor", "beforeSendTransaction").flatMap { callback -> + listOf(false, true).map { hasProfile -> arrayOf(callback, hasProfile) } + } + } + + @Test + fun `transaction callback failure reports attached profile exactly once`() { + val fixture = SentryClientTest.Fixture() + val options = fixture.sentryOptions + options.eventProcessors.clear() + val scope = Scope(options) + val onDiscard = mock() + options.onDiscard = onDiscard + val failure = IllegalStateException("callback failed") + if (callback == "beforeSendTransaction") { + options.setBeforeSendTransaction { _, _ -> throw failure } + } else { + val processor = mock() + whenever(processor.process(any(), any())).thenThrow(failure) + if (callback == "scope processor") { + scope.addEventProcessor(processor) + } else { + options.addEventProcessor(processor) + } + } + + val id = + fixture + .getSut() + .captureTransaction( + SentryTransaction(fixture.sentryTracer), + fixture.sentryTracer.traceContext(), + scope, + null, + if (hasProfile) fixture.profilingTraceData else null, + ) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(fixture.transport, never()).send(any(), anyOrNull()) + val expected = + mutableListOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 2), + ) + if (hasProfile) { + expected.add( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Profile.category, 1) + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Profile, 1) + } + assertClientReport(options.clientReportRecorder, expected) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 2) + verifyNoMoreInteractions(onDiscard) + } +}