diff --git a/CHANGELOG.md b/CHANGELOG.md index 0036c65807e..95f947f594d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Drop Apollo 5 spans when `beforeSpan` throws without disrupting the GraphQL request ([#6238](https://github.com/getsentry/sentry-java/pull/6238)) + ### Features - Add support for Android Navigation 3 through the new `sentry-android-navigation3` library ([#6233](https://github.com/getsentry/sentry-java/pull/6233)) @@ -60,6 +64,16 @@ ### Fixes +- 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 Android screenshot or view hierarchy capture when its capture callback throws, while retaining the error event ([#6167](https://github.com/getsentry/sentry-java/pull/6167)) + - Drop spans when `beforeSpan` throws in OkHttp, OpenFeign, GraphQL, Ktor, or Apollo, without disrupting the request. Report lost sampled spans as `callback_error` in client reports and `OnDiscardCallback` ([#6167](https://github.com/getsentry/sentry-java/pull/6167)) + - Skip replay capture when `beforeErrorSampling` throws, while still sending the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)) + - 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-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index efd776cabb8..e8b95794ea0 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -233,7 +233,7 @@ public final class io/sentry/android/core/AppState$LifecycleObserver : androidx/ public fun onStop (Landroidx/lifecycle/LifecycleOwner;)V } -public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor { +public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor, io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Landroid/content/Context;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -401,7 +401,7 @@ public class io/sentry/android/core/PerfettoProfiler { public fun endAndCollect (Ljava/util/function/Consumer;)V } -public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;Z)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -680,7 +680,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java index 400a3293bcb..483259fa8dd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java @@ -52,6 +52,7 @@ import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; import io.sentry.hints.NativeCrashExit; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.Contexts; import io.sentry.protocol.DebugImage; @@ -89,7 +90,8 @@ */ @ApiStatus.Internal @WorkerThread -public final class ApplicationExitInfoEventProcessor implements BackfillingEventProcessor { +public final class ApplicationExitInfoEventProcessor + implements BackfillingEventProcessor, SentryEventProcessor { private final @NotNull Context context; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 84e953ee69e..c7ea0e27e4f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -8,6 +8,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.OperatingSystem; import io.sentry.protocol.SentryException; @@ -32,7 +33,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -final class DefaultAndroidEventProcessor implements EventProcessor { +final class DefaultAndroidEventProcessor implements SentryEventProcessor { @TestOnly final Context context; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 7f81ea1b738..142bb1f4752 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -7,7 +7,6 @@ import static io.sentry.android.core.ActivityLifecycleIntegration.STANDALONE_APP_START_OP; import static io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.ISentryLifecycleToken; import io.sentry.MeasurementUnit; @@ -20,6 +19,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.MeasurementValue; import io.sentry.protocol.SentryId; @@ -36,7 +36,7 @@ import org.jetbrains.annotations.Nullable; /** Event Processor responsible for adding Android metrics to transactions */ -final class PerformanceAndroidEventProcessor implements EventProcessor { +final class PerformanceAndroidEventProcessor implements SentryEventProcessor { private static final String APP_METRICS_ORIGIN = "auto.ui"; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java index 8c4b1feef7c..6932b6be3f6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java @@ -8,7 +8,6 @@ import android.graphics.Bitmap; import android.view.View; import io.sentry.Attachment; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryLevel; @@ -18,6 +17,7 @@ import io.sentry.android.replay.util.MaskRenderer; import io.sentry.android.replay.util.ViewsKt; import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryTransaction; import io.sentry.util.HintUtils; import io.sentry.util.Objects; @@ -34,7 +34,7 @@ * captured. */ @ApiStatus.Internal -public final class ScreenshotEventProcessor implements EventProcessor { +public final class ScreenshotEventProcessor implements SentryEventProcessor { private final @NotNull SentryAndroidOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; @@ -112,7 +112,17 @@ private boolean isMaskingEnabled() { final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback = options.getBeforeScreenshotCaptureCallback(); if (beforeCaptureCallback != null) { - if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + try { + if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + return event; + } + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.", + e); return event; } } else if (shouldDebounce) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 6d21edb3d0c..00753ee4c90 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -7,7 +7,6 @@ import android.view.ViewGroup; import android.view.Window; import io.sentry.Attachment; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.ILogger; import io.sentry.ISerializer; @@ -18,6 +17,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.ClassUtil; import io.sentry.android.core.internal.util.Debouncer; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.internal.viewhierarchy.ViewHierarchyExporter; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.ViewHierarchy; @@ -37,7 +37,7 @@ /** ViewHierarchyEventProcessor responsible for taking a snapshot of the current view hierarchy. */ @ApiStatus.Internal -public final class ViewHierarchyEventProcessor implements EventProcessor { +public final class ViewHierarchyEventProcessor implements SentryEventProcessor { private final @NotNull SentryAndroidOptions options; private final @NotNull Debouncer debouncer; @@ -91,7 +91,17 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback = options.getBeforeViewHierarchyCaptureCallback(); if (beforeCaptureCallback != null) { - if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + try { + if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + return event; + } + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeViewHierarchyCapture callback threw an exception. Skipping view hierarchy capture.", + e); return event; } } else if (shouldDebounce) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt index b8e223f08e9..635d7a308c5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt @@ -29,11 +29,14 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Attachment import io.sentry.Hint +import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel import io.sentry.TypeCheckHint.ANDROID_ACTIVITY import io.sentry.protocol.SentryException import io.sentry.util.thread.IThreadChecker @@ -48,6 +51,7 @@ import kotlin.test.assertSame import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.Robolectric.buildActivity import org.robolectric.Shadows.shadowOf @@ -310,11 +314,38 @@ class ScreenshotEventProcessorTest { assertNull(hint.screenshot) } + @Test + fun `when capture callback throws, skips screenshot and retains event`() { + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + val logger = mock() + fixture.options.isDebug = true + fixture.options.setLogger(logger) + val failure = IllegalStateException("callback failed") + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure } + val processor = fixture.getSut(true) + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + val hint = Hint() + + assertThat(processor.process(event, hint)).isSameInstanceAs(event) + assertThat(hint.screenshot).isNull() + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.", + failure, + ) + + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true } + val nextHint = Hint() + assertThat(processor.process(event, nextHint)).isSameInstanceAs(event) + assertThat(nextHint.screenshot).isNotNull() + } + @Test fun `when capture callback returns true, a screenshot should be captured`() { CurrentActivityHolder.getInstance().setActivity(fixture.activity) - fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true } + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true } val processor = fixture.getSut(true) val event = SentryEvent().apply { exceptions = listOf(SentryException()) } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index 4d908fcac1c..6f19035a8d9 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -5,11 +5,13 @@ import android.view.View import android.view.ViewGroup import android.view.Window import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Hint import io.sentry.JsonSerializable import io.sentry.JsonSerializer import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel import io.sentry.TypeCheckHint import io.sentry.protocol.SentryException import io.sentry.util.thread.IThreadChecker @@ -342,6 +344,31 @@ class ViewHierarchyEventProcessorTest { assertNull(hint.viewHierarchy) } + @Test + fun `when capture callback throws, skips view hierarchy and retains event`() { + fixture.options.isDebug = true + fixture.options.setLogger(fixture.logger) + val failure = IllegalStateException("callback failed") + fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> throw failure } + val processor = fixture.getSut(true) + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + val hint = Hint() + + assertThat(processor.process(event, hint)).isSameInstanceAs(event) + assertThat(hint.viewHierarchy).isNull() + verify(fixture.logger) + .log( + SentryLevel.ERROR, + "The beforeViewHierarchyCapture callback threw an exception. Skipping view hierarchy capture.", + failure, + ) + + fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true } + val nextHint = Hint() + assertThat(processor.process(event, nextHint)).isSameInstanceAs(event) + assertThat(nextHint.viewHierarchy).isNotNull() + } + @Test fun `when capture callback returns true, a view hierarchy should be captured`() { fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true } diff --git a/sentry-apollo-3/build.gradle.kts b/sentry-apollo-3/build.gradle.kts index 357d5224495..6711b165dd9 100644 --- a/sentry-apollo-3/build.gradle.kts +++ b/sentry-apollo-3/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.apollo3.kotlin) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.kotlinx.coroutines) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 7aa8d693943..53e48657393 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -10,6 +10,7 @@ import com.apollographql.apollo3.network.http.HttpInterceptor import com.apollographql.apollo3.network.http.HttpInterceptorChain import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.IScopes import io.sentry.ISpan @@ -23,6 +24,7 @@ import io.sentry.SpanDataConvention.HTTP_METHOD_KEY import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE +import io.sentry.clientreport.DiscardReason import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Request @@ -216,6 +218,7 @@ constructor( span.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, it) } if (beforeSpan != null) { + val wasSampled = span.isSampled == true try { val result = beforeSpan.execute(span, request, response) if (result == null) { @@ -223,6 +226,13 @@ constructor( span.spanContext.sampled = false } } catch (e: Throwable) { + span.spanContext.sampled = false + if (wasSampled) { + scopes.options.clientReportRecorder.recordLostEvent( + DiscardReason.CALLBACK_ERROR, + DataCategory.Span, + ) + } scopes.options.logger.log( SentryLevel.ERROR, "An error occurred while executing beforeSpan on ApolloInterceptor", diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt index 8316f6c0f33..49d7d790a53 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt @@ -7,12 +7,16 @@ import com.apollographql.apollo3.exception.ApolloException import com.apollographql.apollo3.exception.ApolloHttpException import com.apollographql.apollo3.network.http.HttpInterceptor import com.apollographql.apollo3.network.http.HttpInterceptorChain +import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS import io.sentry.SentryTraceHeader @@ -25,6 +29,7 @@ import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import io.sentry.W3CTraceparentHeader import io.sentry.apollo3.SentryApollo3HttpInterceptor.BeforeSpanCallback +import io.sentry.clientreport.DiscardReason import io.sentry.mockServerRequestTimeoutMillis import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryTransaction @@ -47,6 +52,7 @@ import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryApollo3InterceptorTest { @@ -291,7 +297,10 @@ class SentryApollo3InterceptorTest { @Test fun `returning null in beforeSpan callback drops span`() { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard executeQuery(fixture.getSut(beforeSpan = { _, _, _ -> null })) + verifyNoMoreInteractions(onDiscard) verify(fixture.scopes) .captureTransaction( @@ -303,16 +312,68 @@ class SentryApollo3InterceptorTest { } @Test - fun `when customizer throws, exception is handled`() { - executeQuery(fixture.getSut(beforeSpan = { _, _, _ -> throw RuntimeException() })) + fun `reports callback errors only for sampled spans`(): Unit = runBlocking { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + val tx = SentryTracer(TransactionContext("op", "desc"), fixture.scopes) + tx.spanContext.sampled = sampled + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + error("callback failed") + } + ) + + assertThat(sut.query(LaunchDetailsQuery("83")).execute().data).isNotNull() + tx.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking { + val failure = IllegalStateException("callback failed") + val logger = mock() + fixture.options.isDebug = true + fixture.options.setLogger(logger) + val tx = + SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes) + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + val response = sut.query(LaunchDetailsQuery("83")).execute() + assertThat(response.data).isNotNull() + val span = tx.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + tx.finish() verify(fixture.scopes) .captureTransaction( - check { assertEquals(1, it.spans.size) }, + check { assertThat(it.spans).isEmpty() }, anyOrNull(), anyOrNull(), anyOrNull(), ) + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "An error occurred while executing beforeSpan on ApolloInterceptor", + failure, + ) } @Test diff --git a/sentry-apollo-4/build.gradle.kts b/sentry-apollo-4/build.gradle.kts index 078d26b8ff8..9fb024a01db 100644 --- a/sentry-apollo-4/build.gradle.kts +++ b/sentry-apollo-4/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.apollo4.kotlin) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.kotlinx.coroutines) testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.mockito.kotlin) diff --git a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt index 437edff82d7..aea5d6691c4 100644 --- a/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt +++ b/sentry-apollo-4/src/main/java/io/sentry/apollo4/SentryApollo4HttpInterceptor.kt @@ -8,6 +8,7 @@ import com.apollographql.apollo.network.http.HttpInterceptor import com.apollographql.apollo.network.http.HttpInterceptorChain import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.IScopes import io.sentry.ISpan @@ -21,6 +22,7 @@ import io.sentry.SpanDataConvention.HTTP_METHOD_KEY import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE +import io.sentry.clientreport.DiscardReason import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Request @@ -215,6 +217,7 @@ constructor( span.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, it) } if (beforeSpan != null) { + val wasSampled = span.isSampled == true try { val result = beforeSpan.execute(span, request, response) if (result == null) { @@ -222,6 +225,13 @@ constructor( span.spanContext.sampled = false } } catch (e: Throwable) { + span.spanContext.sampled = false + if (wasSampled) { + scopes.options.clientReportRecorder.recordLostEvent( + DiscardReason.CALLBACK_ERROR, + DataCategory.Span, + ) + } scopes.options.logger.log( SentryLevel.ERROR, "An error occurred while executing beforeSpan in ApolloInterceptor", diff --git a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt index d92cefe9772..210cab90327 100644 --- a/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt +++ b/sentry-apollo-4/src/test/java/io/sentry/apollo4/SentryApollo4HttpInterceptorTest.kt @@ -10,12 +10,16 @@ import com.apollographql.apollo.exception.ApolloException import com.apollographql.apollo.exception.ApolloHttpException import com.apollographql.apollo.network.http.HttpInterceptor import com.apollographql.apollo.network.http.HttpInterceptorChain +import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS import io.sentry.SentryTraceHeader @@ -29,6 +33,7 @@ import io.sentry.TransactionContext import io.sentry.W3CTraceparentHeader import io.sentry.apollo4.SentryApollo4HttpInterceptor.BeforeSpanCallback import io.sentry.apollo4.generated.LaunchDetailsQuery +import io.sentry.clientreport.DiscardReason import io.sentry.mockServerRequestTimeoutMillis import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryTransaction @@ -52,6 +57,7 @@ import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryApollo4HttpInterceptorTestWithV4Implementation : @@ -305,7 +311,10 @@ abstract class SentryApollo4HttpInterceptorTest( @Test fun `returning null in beforeSpan callback drops span`() { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard executeQuery(fixture.getSut(beforeSpan = { _, _, _ -> null })) + verifyNoMoreInteractions(onDiscard) verify(fixture.scopes) .captureTransaction( @@ -317,16 +326,68 @@ abstract class SentryApollo4HttpInterceptorTest( } @Test - fun `when customizer throws, exception is handled`() { - executeQuery(fixture.getSut(beforeSpan = { _, _, _ -> throw RuntimeException() })) + fun `reports callback errors only for sampled spans`(): Unit = runBlocking { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + val tx = SentryTracer(TransactionContext("op", "desc"), fixture.scopes) + tx.spanContext.sampled = sampled + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + error("callback failed") + } + ) + + assertThat(executeQueryImplementation(sut.query(LaunchDetailsQuery("83"))).data).isNotNull() + tx.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking { + val failure = IllegalStateException("callback failed") + val logger = mock() + fixture.options.isDebug = true + fixture.options.setLogger(logger) + val tx = + SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes) + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + val response = executeQueryImplementation(sut.query(LaunchDetailsQuery("83"))) + assertThat(response.data).isNotNull() + val span = tx.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + tx.finish() verify(fixture.scopes) .captureTransaction( - check { assertEquals(1, it.spans.size) }, + check { assertThat(it.spans).isEmpty() }, anyOrNull(), anyOrNull(), anyOrNull(), ) + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "An error occurred while executing beforeSpan in ApolloInterceptor", + failure, + ) } @Test diff --git a/sentry-apollo-5/src/main/java/io/sentry/apollo5/SentryApollo5HttpInterceptor.kt b/sentry-apollo-5/src/main/java/io/sentry/apollo5/SentryApollo5HttpInterceptor.kt index 7240f2d87a4..f74d0fec111 100644 --- a/sentry-apollo-5/src/main/java/io/sentry/apollo5/SentryApollo5HttpInterceptor.kt +++ b/sentry-apollo-5/src/main/java/io/sentry/apollo5/SentryApollo5HttpInterceptor.kt @@ -8,6 +8,7 @@ import com.apollographql.apollo.network.http.HttpInterceptor import com.apollographql.apollo.network.http.HttpInterceptorChain import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.IScopes import io.sentry.ISpan @@ -21,6 +22,7 @@ import io.sentry.SpanDataConvention.HTTP_METHOD_KEY import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE +import io.sentry.clientreport.DiscardReason import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Request @@ -199,6 +201,7 @@ constructor( span.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, it) } if (beforeSpan != null) { + val wasSampled = span.isSampled == true try { val result = beforeSpan.execute(span, request, response) if (result == null) { @@ -207,6 +210,13 @@ constructor( } } catch (e: Throwable) { ExceptionUtils.rethrowIfFatal(e) + span.spanContext.sampled = false + if (wasSampled) { + scopes.options.clientReportRecorder.recordLostEvent( + DiscardReason.CALLBACK_ERROR, + DataCategory.Span, + ) + } scopes.options.logger.log( SentryLevel.ERROR, "An error occurred while executing beforeSpan in ApolloInterceptor", diff --git a/sentry-apollo-5/src/test/java/io/sentry/apollo5/SentryApollo5HttpInterceptorTest.kt b/sentry-apollo-5/src/test/java/io/sentry/apollo5/SentryApollo5HttpInterceptorTest.kt index ce0df3a55a0..399667a7647 100644 --- a/sentry-apollo-5/src/test/java/io/sentry/apollo5/SentryApollo5HttpInterceptorTest.kt +++ b/sentry-apollo-5/src/test/java/io/sentry/apollo5/SentryApollo5HttpInterceptorTest.kt @@ -14,6 +14,7 @@ import com.apollographql.apollo.network.http.HttpInterceptorChain import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.IScopes import io.sentry.ITransaction @@ -32,6 +33,7 @@ import io.sentry.TransactionContext import io.sentry.W3CTraceparentHeader import io.sentry.apollo5.SentryApollo5HttpInterceptor.BeforeSpanCallback import io.sentry.apollo5.generated.LaunchDetailsQuery +import io.sentry.clientreport.DiscardReason import io.sentry.mockServerRequestTimeoutMillis import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryTransaction @@ -40,6 +42,7 @@ import java.util.concurrent.TimeUnit import kotlin.reflect.KSuspendFunction1 import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -60,6 +63,7 @@ import org.mockito.kotlin.doAnswer 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 class SentryApollo5HttpInterceptorTestWithV5Implementation : @@ -421,16 +425,83 @@ abstract class SentryApollo5HttpInterceptorTest( } @Test - fun `when customizer throws, exception is handled`() { - executeQuery(fixture.getSut(beforeSpan = { _, _, _ -> throw RuntimeException() })) + fun `reports callback errors only for sampled spans`(): Unit = runBlocking { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + val tx = SentryTracer(TransactionContext("op", "desc"), fixture.scopes) + tx.spanContext.sampled = sampled + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + + assertThat(executeQueryImplementation(sut.query(LaunchDetailsQuery("83"))).data).isNotNull() + tx.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + @Test + fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking { + val failure = IllegalStateException("callback failed") + val tx = + SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes) + whenever(fixture.scopes.span).thenReturn(tx) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + + val response = executeQueryImplementation(sut.query(LaunchDetailsQuery("83"))) + assertThat(response.data).isNotNull() + val span = tx.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + tx.finish() verify(fixture.scopes) .captureTransaction( - check { assertEquals(1, it.spans.size) }, + check { assertThat(it.spans).isEmpty() }, anyOrNull(), anyOrNull(), anyOrNull(), ) + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + } + + @Test + fun `fatal error from beforeSpan is not swallowed`(): Unit = runBlocking { + val failure = OutOfMemoryError("callback failed") + val tx = + SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes) + whenever(fixture.scopes.span).thenReturn(tx) + val request = HttpRequest.Builder(HttpMethod.Post, "https://example.com/graphql").build() + val response = HttpResponse.Builder(200).build() + val chain = + object : HttpInterceptorChain { + override suspend fun proceed(request: HttpRequest): HttpResponse = response + } + val interceptor = + SentryApollo5HttpInterceptor( + fixture.scopes, + beforeSpan = { _, _, _ -> throw failure }, + captureFailedRequests = false, + ) + + val thrown = assertFailsWith { interceptor.intercept(request, chain) } + + assertThat(thrown).isSameInstanceAs(failure) } @Test diff --git a/sentry-apollo/build.gradle.kts b/sentry-apollo/build.gradle.kts index 570214e60b8..b3cb3cff953 100644 --- a/sentry-apollo/build.gradle.kts +++ b/sentry-apollo/build.gradle.kts @@ -35,6 +35,7 @@ dependencies { testImplementation(libs.apollo2.coroutines) testImplementation(libs.apollo2.runtime) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.kotlinx.coroutines) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index cb7df6472dd..3524d32dc1f 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -14,6 +14,7 @@ import com.apollographql.apollo.interceptor.ApolloInterceptorChain import com.apollographql.apollo.request.RequestHeaders import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.IScopes import io.sentry.ISpan @@ -24,6 +25,7 @@ import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE +import io.sentry.clientreport.DiscardReason import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import io.sentry.util.SpanUtils import io.sentry.util.TracingUtils @@ -175,9 +177,17 @@ class SentryApolloInterceptor( ) { var newSpan: ISpan? = span if (beforeSpan != null) { + val wasSampled = span.isSampled == true try { newSpan = beforeSpan.execute(span, request, response) } catch (e: Exception) { + span.spanContext.sampled = false + if (wasSampled) { + scopes.options.clientReportRecorder.recordLostEvent( + DiscardReason.CALLBACK_ERROR, + DataCategory.Span, + ) + } scopes.options.logger.log( SentryLevel.ERROR, "An error occurred while executing beforeSpan on ApolloInterceptor", diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index d43fe40c9e4..498b4663cac 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -3,12 +3,16 @@ package io.sentry.apollo import com.apollographql.apollo.ApolloClient import com.apollographql.apollo.coroutines.await import com.apollographql.apollo.exception.ApolloException +import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -17,6 +21,7 @@ import io.sentry.SpanStatus import io.sentry.TraceContext import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext +import io.sentry.clientreport.DiscardReason import io.sentry.mockServerRequestTimeoutMillis import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryTransaction @@ -39,6 +44,7 @@ import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryApolloInterceptorTest { @@ -229,7 +235,10 @@ class SentryApolloInterceptorTest { @Test fun `when beforeSpan callback returns null, span is dropped`() { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard executeQuery(fixture.getSut { _, _, _ -> null }) + verifyNoMoreInteractions(onDiscard) verify(fixture.scopes) .captureTransaction( @@ -241,16 +250,62 @@ class SentryApolloInterceptorTest { } @Test - fun `when customizer throws, exception is handled`() { - executeQuery(fixture.getSut { _, _, _ -> throw RuntimeException() }) + fun `reports callback errors only for sampled spans`(): Unit = runBlocking { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + val tx = SentryTracer(TransactionContext("op", "desc"), fixture.scopes) + tx.spanContext.sampled = sampled + whenever(fixture.scopes.span).thenReturn(tx) + val sut = fixture.getSut { span, _, _ -> + span.spanContext.sampled = false + error("callback failed") + } + + assertThat(sut.query(LaunchDetailsQuery.builder().id("83").build()).await().data).isNotNull() + tx.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + @Test + fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking { + val failure = IllegalStateException("callback failed") + val logger = mock() + fixture.options.isDebug = true + fixture.options.setLogger(logger) + val tx = + SentryTracer(TransactionContext("op", "desc", TracesSamplingDecision(true)), fixture.scopes) + whenever(fixture.scopes.span).thenReturn(tx) + val sut = fixture.getSut { span, _, _ -> + span.description = "partially modified" + throw failure + } + + val response = sut.query(LaunchDetailsQuery.builder().id("83").build()).await() + assertThat(response.data).isNotNull() + val span = tx.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + tx.finish() verify(fixture.scopes) .captureTransaction( - check { assertEquals(1, it.spans.size) }, + check { assertThat(it.spans).isEmpty() }, anyOrNull(), anyOrNull(), anyOrNull(), ) + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "An error occurred while executing beforeSpan on ApolloInterceptor", + failure, + ) } @Test diff --git a/sentry-graphql-22/build.gradle.kts b/sentry-graphql-22/build.gradle.kts index 32db28fae8f..def6708f672 100644 --- a/sentry-graphql-22/build.gradle.kts +++ b/sentry-graphql-22/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.graphql.java22) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) testImplementation(libs.okhttp) diff --git a/sentry-graphql-22/src/test/kotlin/io/sentry/graphql22/SentryInstrumentationTest.kt b/sentry-graphql-22/src/test/kotlin/io/sentry/graphql22/SentryInstrumentationTest.kt index c684688299e..2331ab17aaf 100644 --- a/sentry-graphql-22/src/test/kotlin/io/sentry/graphql22/SentryInstrumentationTest.kt +++ b/sentry-graphql-22/src/test/kotlin/io/sentry/graphql22/SentryInstrumentationTest.kt @@ -1,5 +1,6 @@ package io.sentry.graphql22 +import com.google.common.truth.Truth.assertThat import graphql.GraphQL import graphql.GraphQLContext import graphql.execution.ExecutionContextBuilder @@ -18,17 +19,22 @@ import graphql.schema.GraphQLScalarType import graphql.schema.idl.RuntimeWiring import graphql.schema.idl.SchemaGenerator import graphql.schema.idl.SchemaParser +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.Sentry +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import io.sentry.clientreport.DiscardReason import io.sentry.graphql.ExceptionReporter import io.sentry.graphql.NoOpSubscriptionHandler import io.sentry.graphql.SentryGraphqlInstrumentation import io.sentry.graphql.SentrySubscriptionHandler import java.lang.RuntimeException +import java.util.concurrent.CompletableFuture import kotlin.random.Random import kotlin.test.Test import kotlin.test.assertEquals @@ -38,6 +44,8 @@ import kotlin.test.assertTrue import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryInstrumentationTest { @@ -48,9 +56,11 @@ class SentryInstrumentationTest { fun getSut( isTransactionActive: Boolean = true, dataFetcherThrows: Boolean = false, + async: Boolean = false, beforeSpan: SentryGraphqlInstrumentation.BeforeSpanCallback? = null, ): GraphQL { - whenever(scopes.options).thenReturn(SentryOptions()) + whenever(scopes.options) + .thenReturn(SentryOptions().apply { dsn = "https://key@sentry.io/proj" }) activeSpan = SentryTracer(TransactionContext("name", "op"), scopes) val schema = """ @@ -66,7 +76,10 @@ class SentryInstrumentationTest { val graphQLSchema = SchemaGenerator() - .makeExecutableSchema(SchemaParser().parse(schema), buildRuntimeWiring(dataFetcherThrows)) + .makeExecutableSchema( + SchemaParser().parse(schema), + buildRuntimeWiring(dataFetcherThrows, async), + ) val graphQL = GraphQL.newGraphQL(graphQLSchema) .instrumentation( @@ -83,14 +96,15 @@ class SentryInstrumentationTest { return graphQL } - private fun buildRuntimeWiring(dataFetcherThrows: Boolean) = + private fun buildRuntimeWiring(dataFetcherThrows: Boolean, async: Boolean) = RuntimeWiring.newRuntimeWiring() .type("Query") { it.dataFetcher("shows") { if (dataFetcherThrows) { throw RuntimeException("error") } else { - listOf(Show(Random.nextInt()), Show(Random.nextInt())) + val shows = listOf(Show(Random.nextInt()), Show(Random.nextInt())) + if (async) CompletableFuture.completedFuture(shows) else shows } } } @@ -152,6 +166,9 @@ class SentryInstrumentationTest { fixture.getSut( beforeSpan = SentryGraphqlInstrumentation.BeforeSpanCallback { _, _, _ -> null } ) + val onDiscard = mock() + fixture.scopes.options.onDiscard = onDiscard + fixture.activeSpan.spanContext.sampled = true withMockScopes { val result = sut.execute("{ shows { id } }") @@ -162,6 +179,103 @@ class SentryInstrumentationTest { assertEquals("graphql", span.operation) assertEquals("Query.shows", span.description) assertNotNull(span.isSampled) { assertFalse(it) } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `reports callback errors only for sampled spans`() { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + fixture.activeSpan.spanContext.sampled = sampled + fixture.scopes.options.onDiscard = onDiscard + + withMockScopes { + assertThat(sut.execute("{ shows { id } }").errors).isEmpty() + fixture.activeSpan.finish() + } + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves result`() { + val failure = IllegalStateException("callback failed") + val logger = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + fixture.scopes.options.isDebug = true + fixture.scopes.options.setLogger(logger) + + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).isEmpty() + assertThat(result.getData>()).containsKey("shows") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryGraphqlInstrumentation. Dropping span.", + failure, + ) + } + } + + @Test + fun `when beforeSpan throws, drops async span and preserves result`() { + val sut = + fixture.getSut( + async = true, + beforeSpan = { _, _, _ -> + throw IllegalStateException("callback failed") + }, + ) + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).isEmpty() + assertThat(result.getData>()).containsKey("shows") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + } + } + + @Test + fun `when beforeSpan throws, preserves data fetcher error`() { + val sut = + fixture.getSut( + dataFetcherThrows = true, + beforeSpan = { _, _, _ -> + throw IllegalStateException("callback failed") + }, + ) + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).hasSize(1) + assertThat(result.errors.single().message).contains("error") + assertThat(result.errors.single().message).doesNotContain("callback failed") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR) } } diff --git a/sentry-graphql-core/src/main/java/io/sentry/graphql/SentryGraphqlInstrumentation.java b/sentry-graphql-core/src/main/java/io/sentry/graphql/SentryGraphqlInstrumentation.java index c316774c045..34044aea435 100644 --- a/sentry-graphql-core/src/main/java/io/sentry/graphql/SentryGraphqlInstrumentation.java +++ b/sentry-graphql-core/src/main/java/io/sentry/graphql/SentryGraphqlInstrumentation.java @@ -17,14 +17,17 @@ import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLOutputType; import io.sentry.Breadcrumb; +import io.sentry.DataCategory; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISpan; import io.sentry.NoOpScopes; import io.sentry.Sentry; +import io.sentry.SentryLevel; import io.sentry.SpanOptions; import io.sentry.SpanStatus; import io.sentry.TypeCheckHint; +import io.sentry.clientreport.DiscardReason; import io.sentry.util.StringUtils; import java.util.Arrays; import java.util.List; @@ -283,7 +286,26 @@ private void finish( final @NotNull DataFetchingEnvironment environment, final @Nullable Object result) { if (beforeSpan != null) { - final ISpan newSpan = beforeSpan.execute(span, environment, result); + final boolean wasSampled = Boolean.TRUE.equals(span.isSampled()); + ISpan newSpan = span; + try { + newSpan = beforeSpan.execute(span, environment, result); + } catch (Exception e) { + span.getSpanContext().setSampled(false); + if (wasSampled) { + scopesFromContext(environment.getGraphQlContext()) + .getOptions() + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Span); + } + scopesFromContext(environment.getGraphQlContext()) + .getOptions() + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryGraphqlInstrumentation. Dropping span.", + e); + } if (newSpan == null) { // span is dropped span.getSpanContext().setSampled(false); diff --git a/sentry-graphql/build.gradle.kts b/sentry-graphql/build.gradle.kts index d92dc52c6d7..f996e0310a1 100644 --- a/sentry-graphql/build.gradle.kts +++ b/sentry-graphql/build.gradle.kts @@ -34,6 +34,7 @@ dependencies { testImplementation(kotlin(Config.kotlinStdLib)) testImplementation(libs.graphql.java17) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) testImplementation(libs.okhttp) diff --git a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt index 972b091a226..2652694b8f9 100644 --- a/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt +++ b/sentry-graphql/src/test/kotlin/io/sentry/graphql/SentryInstrumentationTest.kt @@ -1,5 +1,6 @@ package io.sentry.graphql +import com.google.common.truth.Truth.assertThat import graphql.GraphQL import graphql.GraphQLContext import graphql.execution.ExecutionContextBuilder @@ -18,13 +19,18 @@ import graphql.schema.GraphQLScalarType import graphql.schema.idl.RuntimeWiring import graphql.schema.idl.SchemaGenerator import graphql.schema.idl.SchemaParser +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.Sentry +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import io.sentry.clientreport.DiscardReason import java.lang.RuntimeException +import java.util.concurrent.CompletableFuture import kotlin.random.Random import kotlin.test.Test import kotlin.test.assertEquals @@ -34,6 +40,8 @@ import kotlin.test.assertTrue import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryInstrumentationTest { @@ -44,9 +52,11 @@ class SentryInstrumentationTest { fun getSut( isTransactionActive: Boolean = true, dataFetcherThrows: Boolean = false, + async: Boolean = false, beforeSpan: SentryGraphqlInstrumentation.BeforeSpanCallback? = null, ): GraphQL { - whenever(scopes.options).thenReturn(SentryOptions()) + whenever(scopes.options) + .thenReturn(SentryOptions().apply { dsn = "https://key@sentry.io/proj" }) activeSpan = SentryTracer(TransactionContext("name", "op"), scopes) val schema = """ @@ -62,7 +72,10 @@ class SentryInstrumentationTest { val graphQLSchema = SchemaGenerator() - .makeExecutableSchema(SchemaParser().parse(schema), buildRuntimeWiring(dataFetcherThrows)) + .makeExecutableSchema( + SchemaParser().parse(schema), + buildRuntimeWiring(dataFetcherThrows, async), + ) val graphQL = GraphQL.newGraphQL(graphQLSchema) .instrumentation( @@ -79,14 +92,15 @@ class SentryInstrumentationTest { return graphQL } - private fun buildRuntimeWiring(dataFetcherThrows: Boolean) = + private fun buildRuntimeWiring(dataFetcherThrows: Boolean, async: Boolean) = RuntimeWiring.newRuntimeWiring() .type("Query") { it.dataFetcher("shows") { if (dataFetcherThrows) { throw RuntimeException("error") } else { - listOf(Show(Random.nextInt()), Show(Random.nextInt())) + val shows = listOf(Show(Random.nextInt()), Show(Random.nextInt())) + if (async) CompletableFuture.completedFuture(shows) else shows } } } @@ -148,6 +162,9 @@ class SentryInstrumentationTest { fixture.getSut( beforeSpan = SentryGraphqlInstrumentation.BeforeSpanCallback { _, _, _ -> null } ) + val onDiscard = mock() + fixture.scopes.options.onDiscard = onDiscard + fixture.activeSpan.spanContext.sampled = true withMockScopes { val result = sut.execute("{ shows { id } }") @@ -158,6 +175,103 @@ class SentryInstrumentationTest { assertEquals("graphql", span.operation) assertEquals("Query.shows", span.description) assertNotNull(span.isSampled) { assertFalse(it) } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `reports callback errors only for sampled spans`() { + for (sampled in listOf(true, false, null)) { + val onDiscard = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + fixture.activeSpan.spanContext.sampled = sampled + fixture.scopes.options.onDiscard = onDiscard + + withMockScopes { + assertThat(sut.execute("{ shows { id } }").errors).isEmpty() + fixture.activeSpan.finish() + } + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves result`() { + val failure = IllegalStateException("callback failed") + val logger = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + fixture.scopes.options.isDebug = true + fixture.scopes.options.setLogger(logger) + + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).isEmpty() + assertThat(result.getData>()).containsKey("shows") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryGraphqlInstrumentation. Dropping span.", + failure, + ) + } + } + + @Test + fun `when beforeSpan throws, drops async span and preserves result`() { + val sut = + fixture.getSut( + async = true, + beforeSpan = { _, _, _ -> + throw IllegalStateException("callback failed") + }, + ) + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).isEmpty() + assertThat(result.getData>()).containsKey("shows") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + } + } + + @Test + fun `when beforeSpan throws, preserves data fetcher error`() { + val sut = + fixture.getSut( + dataFetcherThrows = true, + beforeSpan = { _, _, _ -> + throw IllegalStateException("callback failed") + }, + ) + withMockScopes { + val result = sut.execute("{ shows { id } }") + assertThat(result.errors).hasSize(1) + assertThat(result.errors.single().message).contains("error") + assertThat(result.errors.single().message).doesNotContain("callback failed") + val span = fixture.activeSpan.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + assertThat(span.status).isEqualTo(SpanStatus.INTERNAL_ERROR) } } diff --git a/sentry-ktor-client/build.gradle.kts b/sentry-ktor-client/build.gradle.kts index a1eb9150d6f..cbee84179fb 100644 --- a/sentry-ktor-client/build.gradle.kts +++ b/sentry-ktor-client/build.gradle.kts @@ -35,6 +35,7 @@ dependencies { testImplementation(projects.sentryTestSupport) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) testImplementation(libs.ktor.client.core) diff --git a/sentry-ktor-client/src/main/java/io/sentry/ktorClient/SentryKtorClientPlugin.kt b/sentry-ktor-client/src/main/java/io/sentry/ktorClient/SentryKtorClientPlugin.kt index 95cdfb5fdae..609ed239961 100644 --- a/sentry-ktor-client/src/main/java/io/sentry/ktorClient/SentryKtorClientPlugin.kt +++ b/sentry-ktor-client/src/main/java/io/sentry/ktorClient/SentryKtorClientPlugin.kt @@ -9,6 +9,7 @@ import io.ktor.util.* import io.ktor.util.pipeline.* import io.sentry.BaggageHeader import io.sentry.BuildConfig +import io.sentry.DataCategory import io.sentry.HttpStatusCodeRange import io.sentry.IScopes import io.sentry.ISpan @@ -16,8 +17,10 @@ import io.sentry.ScopesAdapter import io.sentry.Sentry import io.sentry.SentryDate import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SpanStatus +import io.sentry.clientreport.DiscardReason import io.sentry.kotlin.SentryContext import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import io.sentry.util.Platform @@ -186,7 +189,28 @@ public val SentryKtorClientPlugin: ClientPlugin = var result: ISpan? = span if (beforeSpan != null) { - result = beforeSpan.execute(span, request) + val wasSampled = span.isSampled == true + result = + try { + beforeSpan.execute(span, request) + } catch (e: Exception) { + span.spanContext.sampled = false + if (wasSampled) { + (if (forceScopes) scopes else Sentry.getCurrentScopes()) + .options + .clientReportRecorder + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Span) + } + (if (forceScopes) scopes else Sentry.getCurrentScopes()) + .options + .logger + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryKtorClientPlugin. Dropping span.", + e, + ) + null + } } if (result == null) { diff --git a/sentry-ktor-client/src/test/java/io/sentry/ktorClient/SentryKtorClientPluginTest.kt b/sentry-ktor-client/src/test/java/io/sentry/ktorClient/SentryKtorClientPluginTest.kt index 38ffe609b2c..4aadfe823a0 100644 --- a/sentry-ktor-client/src/test/java/io/sentry/ktorClient/SentryKtorClientPluginTest.kt +++ b/sentry-ktor-client/src/test/java/io/sentry/ktorClient/SentryKtorClientPluginTest.kt @@ -1,17 +1,21 @@ package io.sentry.ktorClient +import com.google.common.truth.Truth.assertThat import io.ktor.client.HttpClient import io.ktor.client.engine.HttpClientEngine import io.ktor.client.engine.java.Java import io.ktor.client.request.get import io.ktor.client.request.post import io.ktor.client.request.setBody +import io.ktor.client.statement.bodyAsText import io.ktor.http.ContentType import io.ktor.http.contentType import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.HttpStatusCodeRange +import io.sentry.ILogger import io.sentry.IScope import io.sentry.IScopes import io.sentry.KeyValueCollectionBehavior @@ -19,6 +23,7 @@ import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryEvent +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -26,6 +31,7 @@ import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.W3CTraceparentHeader +import io.sentry.clientreport.DiscardReason import io.sentry.exception.SentryHttpClientException import io.sentry.mockServerRequestTimeoutMillis import java.util.concurrent.TimeUnit @@ -46,6 +52,7 @@ import org.mockito.kotlin.doAnswer 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 class SentryKtorClientPluginTest { @@ -112,6 +119,7 @@ class SentryKtorClientPluginTest { return HttpClient(httpClientEngine) { install(SentryKtorClientPlugin) { this.scopes = this@Fixture.scopes + this.beforeSpan = beforeSpan this.captureFailedRequests = captureFailedRequests this.failedRequestTargets = failedRequestTargets this.failedRequestStatusCodes = failedRequestStatusCodes @@ -434,6 +442,90 @@ class SentryKtorClientPluginTest { ) } + @Test + fun `reports callback errors only for sampled spans`(): Unit = runBlocking { + for (sampled in listOf(true, false, null)) { + val fixture = Fixture() + val onDiscard = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + fixture.sentryTracer.spanContext.sampled = sampled + fixture.options.onDiscard = onDiscard + + sut.use { sut.get(fixture.server.url("/hello").toString()) } + fixture.sentryTracer.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves response`(): Unit = runBlocking { + val failure = IllegalStateException("callback failed") + val logger = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _ -> + span.description = "partially modified" + throw failure + } + ) + fixture.options.isDebug = true + fixture.options.setLogger(logger) + sut.use { + val response = sut.get(fixture.server.url("/hello").toString()) + assertThat(response.status.value).isEqualTo(201) + assertThat(response.bodyAsText()).isEqualTo("success") + } + val span = fixture.sentryTracer.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + assertThat(span.status).isEqualTo(SpanStatus.OK) + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryKtorClientPlugin. Dropping span.", + failure, + ) + } + + @Test + fun `beforeSpan can drop span`(): Unit = runBlocking { + val sut = fixture.getSut(beforeSpan = { _, _ -> null }) + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + fixture.sentryTracer.spanContext.sampled = true + sut.use { sut.get(fixture.server.url("/hello").toString()) } + verifyNoMoreInteractions(onDiscard) + val span = fixture.sentryTracer.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + } + + @Test + fun `beforeSpan can modify span`(): Unit = runBlocking { + val sut = + fixture.getSut( + beforeSpan = { span, _ -> + span.description = "changed" + span + } + ) + sut.use { sut.get(fixture.server.url("/hello").toString()) } + val span = fixture.sentryTracer.children.single() + assertThat(span.description).isEqualTo("changed") + assertThat(span.isFinished).isTrue() + } + @Test fun `creates a span around the request`(): Unit = runBlocking { val sut = fixture.getSut() diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt index 71a43590e52..2f07a1de240 100644 --- a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt @@ -2,6 +2,7 @@ package io.sentry.okhttp import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.HttpStatusCodeRange import io.sentry.ILogger @@ -9,6 +10,7 @@ import io.sentry.IScopes import io.sentry.ISpan import io.sentry.ScopesAdapter import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS import io.sentry.SentryReplayOptions import io.sentry.SpanDataConvention @@ -17,6 +19,7 @@ import io.sentry.SpanStatus import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.TypeCheckHint.SENTRY_REPLAY_NETWORK_DETAILS +import io.sentry.clientreport.DiscardReason import io.sentry.okhttp.SentryOkHttpInterceptor.BeforeSpanCallback import io.sentry.transport.CurrentDateProvider import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion @@ -368,7 +371,25 @@ public open class SentryOkHttpInterceptor( return } if (beforeSpan != null) { - val result = beforeSpan.execute(span, request, response) + val wasSampled = span.isSampled == true + val result = + try { + beforeSpan.execute(span, request, response) + } catch (e: Exception) { + span.spanContext.sampled = false + if (wasSampled) { + scopes.options.clientReportRecorder.recordLostEvent( + DiscardReason.CALLBACK_ERROR, + DataCategory.Span, + ) + } + scopes.options.logger.log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryOkHttpInterceptor. Dropping span.", + e, + ) + null + } if (result == null) { // span is dropped span.spanContext.sampled = false diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt index 6c474928a39..59a8f121f59 100644 --- a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt @@ -2,16 +2,20 @@ package io.sentry.okhttp +import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory import io.sentry.Hint import io.sentry.HttpStatusCodeRange +import io.sentry.ILogger import io.sentry.IScope import io.sentry.IScopes import io.sentry.KeyValueCollectionBehavior import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.Sentry +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -21,6 +25,7 @@ import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TypeCheckHint import io.sentry.W3CTraceparentHeader +import io.sentry.clientreport.DiscardReason import io.sentry.exception.SentryHttpClientException import io.sentry.mockServerRequestTimeoutMillis import io.sentry.util.network.NetworkRequestData @@ -28,6 +33,7 @@ import java.io.IOException import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -53,6 +59,7 @@ import org.mockito.kotlin.doAnswer 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 class SentryOkHttpInterceptorTest { @@ -437,12 +444,111 @@ class SentryOkHttpInterceptorTest { @Test fun `customizer can drop the span`() { val sut = fixture.getSut(beforeSpan = { _, _, _ -> null }) + val onDiscard = mock() + fixture.options.onDiscard = onDiscard + fixture.sentryTracer.spanContext.sampled = true sut.newCall(getRequest()).execute() + verifyNoMoreInteractions(onDiscard) val httpClientSpan = fixture.sentryTracer.children.first() assertTrue(httpClientSpan.isFinished) assertNotNull(httpClientSpan.spanContext.sampled) { assertFalse(it) } } + @Test + fun `reports callback errors only for sampled spans`() { + for (sampled in listOf(true, false, null)) { + val fixture = Fixture() + val onDiscard = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + fixture.sentryTracer.spanContext.sampled = sampled + fixture.options.onDiscard = onDiscard + + sut.newCall(Request.Builder().url(fixture.server.url("/hello")).build()).execute().close() + fixture.sentryTracer.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves response`() { + val failure = IllegalStateException("callback failed") + val logger = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + fixture.options.isDebug = true + fixture.options.setLogger(logger) + + sut.newCall(getRequest()).execute().use { response -> + assertThat(response.code).isEqualTo(201) + assertThat(response.body!!.string()).isEqualTo("success") + } + val span = fixture.sentryTracer.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryOkHttpInterceptor. Dropping span.", + failure, + ) + } + + @Test + fun `when beforeSpan throws, event listener still finishes span`() { + val sut = + fixture.getSut( + beforeSpan = { _, _, _ -> throw IllegalStateException("callback failed") }, + eventListener = SentryOkHttpEventListener(fixture.scopes), + ) + val call = sut.newCall(getRequest()) + call.execute().use { response -> + assertThat(response.code).isEqualTo(201) + assertThat(SentryOkHttpEventListener.eventMap[call]!!.isEventFinished.get()).isTrue() + } + val span = fixture.sentryTracer.children.first { it.operation == "http.client" } + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + assertThat(SentryOkHttpEventListener.eventMap).doesNotContainKey(call) + } + + @Test + fun `when beforeSpan throws, preserves original request exception`() { + val requestFailure = IOException("request failed") + val sut = + fixture + .getSut( + beforeSpan = { _, _, _ -> + throw IllegalStateException("callback failed") + } + ) + .newBuilder() + .addInterceptor { throw requestFailure } + .build() + + val thrown = assertFailsWith { sut.newCall(getRequest()).execute() } + assertThat(thrown).isSameInstanceAs(requestFailure) + val span = fixture.sentryTracer.children.single() + assertThat(span.throwable).isSameInstanceAs(requestFailure) + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + } + @Test fun `captures failed requests by default`() { val sut = fixture.getSut(httpStatusCode = 500, captureFailedRequests = null) diff --git a/sentry-openfeign/build.gradle.kts b/sentry-openfeign/build.gradle.kts index 3baa85dee26..8b7adc97544 100644 --- a/sentry-openfeign/build.gradle.kts +++ b/sentry-openfeign/build.gradle.kts @@ -31,6 +31,7 @@ dependencies { testImplementation(libs.awaitility.kotlin) testImplementation(libs.feign.core) testImplementation(libs.kotlin.test.junit) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.okhttp.mockwebserver) } diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index 520828c0a75..7ed48d0dc10 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -10,14 +10,17 @@ import io.sentry.BaggageHeader; import io.sentry.Breadcrumb; import io.sentry.BuildConfig; +import io.sentry.DataCategory; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISpan; import io.sentry.SentryIntegrationPackageStorage; +import io.sentry.SentryLevel; import io.sentry.SpanDataConvention; import io.sentry.SpanOptions; import io.sentry.SpanStatus; import io.sentry.W3CTraceparentHeader; +import io.sentry.clientreport.DiscardReason; import io.sentry.util.Objects; import io.sentry.util.SpanUtils; import io.sentry.util.TracingUtils; @@ -95,7 +98,26 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O throw e; } finally { if (beforeSpan != null) { - final ISpan result = beforeSpan.execute(span, request, response); + final boolean wasSampled = Boolean.TRUE.equals(span.isSampled()); + ISpan result = span; + try { + result = beforeSpan.execute(span, request, response); + } catch (Exception e) { + span.getSpanContext().setSampled(false); + if (wasSampled) { + scopes + .getOptions() + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Span); + } + scopes + .getOptions() + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryFeignClient. Dropping span.", + e); + } if (result == null) { // span is dropped diff --git a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt index c25a81f9501..9a2bd98d365 100644 --- a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt +++ b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt @@ -1,15 +1,20 @@ package io.sentry.openfeign +import com.google.common.truth.Truth.assertThat import feign.Client import feign.Feign import feign.FeignException import feign.HeaderMap +import feign.Request import feign.RequestLine import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.DataCategory +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -17,11 +22,14 @@ import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.W3CTraceparentHeader +import io.sentry.clientreport.DiscardReason import io.sentry.mockServerRequestTimeoutMillis +import java.io.IOException import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -35,6 +43,7 @@ import org.mockito.kotlin.check import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class SentryFeignClientTest { @@ -284,6 +293,86 @@ class SentryFeignClientTest { assertTrue(httpClientSpan.throwable is Exception) } + @Test + fun `reports callback errors only for sampled spans`() { + for (sampled in listOf(true, false, null)) { + val fixture = Fixture() + val onDiscard = mock() + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.spanContext.sampled = false + throw IllegalStateException("callback failed") + } + ) + fixture.sentryTracer.spanContext.sampled = sampled + fixture.sentryOptions.onDiscard = onDiscard + + assertThat(sut.getOk()).isEqualTo("success") + fixture.sentryTracer.finish() + + if (sampled == true) { + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 1) + } + verifyNoMoreInteractions(onDiscard) + } + } + + @Test + fun `when beforeSpan throws, drops span and preserves response`() { + val failure = IllegalStateException("callback failed") + val logger = mock() + fixture.sentryOptions.isDebug = true + fixture.sentryOptions.setLogger(logger) + val sut = + fixture.getSut( + beforeSpan = { span, _, _ -> + span.description = "partially modified" + throw failure + } + ) + + assertThat(sut.getOk()).isEqualTo("success") + val span = fixture.sentryTracer.children.single() + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeSpan callback threw an exception in SentryFeignClient. Dropping span.", + failure, + ) + } + + @Test + fun `when beforeSpan throws, preserves original request exception`() { + val requestFailure = IOException("request failed") + val delegate = mock() + whenever(delegate.execute(any(), any())).thenThrow(requestFailure) + whenever(fixture.scopes.span).thenReturn(fixture.sentryTracer) + val sut = + SentryFeignClient(delegate, fixture.scopes) { _, _, _ -> + throw IllegalStateException("callback failed") + } + val request = + Request.create( + Request.HttpMethod.GET, + "https://example.com", + emptyMap>(), + null as ByteArray?, + null, + ) + + val thrown = assertFailsWith { sut.execute(request, Request.Options()) } + assertThat(thrown).isSameInstanceAs(requestFailure) + val span = fixture.sentryTracer.children.single() + assertThat(span.throwable).isSameInstanceAs(requestFailure) + assertThat(span.isSampled).isFalse() + assertThat(span.isFinished).isTrue() + verify(fixture.scopes).addBreadcrumb(any(), anyOrNull()) + } + @Test fun `customizer modifies span`() { val sut = fixture.getSut { span, _, _ -> @@ -310,7 +399,11 @@ class SentryFeignClientTest { @Test fun `customizer can drop the span`() { val sut = fixture.getSut { _, _, _ -> null } + val onDiscard = mock() + fixture.sentryOptions.onDiscard = onDiscard + fixture.sentryTracer.spanContext.sampled = true sut.getOk() + verifyNoMoreInteractions(onDiscard) val httpClientSpan = fixture.sentryTracer.children.first() assertNotNull(httpClientSpan.spanContext.sampled) { assertFalse(it) } } diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api b/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api index 3ed25d1a9cf..b6a5ab2de48 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api +++ b/sentry-opentelemetry/sentry-opentelemetry-core/api/sentry-opentelemetry-core.api @@ -4,7 +4,7 @@ public final class io/sentry/opentelemetry/OpenTelemetryAttributesExtractor { public fun extractUrl (Lio/opentelemetry/api/common/Attributes;Lio/sentry/SentryOptions;)Ljava/lang/String; } -public final class io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun ()V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor.java index cf1e530cd9e..dde593fa755 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryLinkErrorEventProcessor.java @@ -3,7 +3,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanId; import io.opentelemetry.api.trace.TraceId; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISpan; @@ -12,6 +11,7 @@ import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.SpanContext; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryId; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,7 +21,7 @@ * @deprecated this is no longer needed for the latest version of our OpenTelemetry integration. */ @Deprecated -public final class OpenTelemetryLinkErrorEventProcessor implements EventProcessor { +public final class OpenTelemetryLinkErrorEventProcessor implements SentryEventProcessor { private final @NotNull IScopes scopes; 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-opentelemetry/sentry-opentelemetry-otlp/api/sentry-opentelemetry-otlp.api b/sentry-opentelemetry/sentry-opentelemetry-otlp/api/sentry-opentelemetry-otlp.api index 56e80e60ae6..07af57c2e4e 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-otlp/api/sentry-opentelemetry-otlp.api +++ b/sentry-opentelemetry/sentry-opentelemetry-otlp/api/sentry-opentelemetry-otlp.api @@ -1,4 +1,4 @@ -public final class io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun ()V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; diff --git a/sentry-opentelemetry/sentry-opentelemetry-otlp/src/main/java/io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-otlp/src/main/java/io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor.java index ad8b672c7de..0b564bc16d0 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-otlp/src/main/java/io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-otlp/src/main/java/io/sentry/opentelemetry/otlp/OpenTelemetryOtlpEventProcessor.java @@ -3,7 +3,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanId; import io.opentelemetry.api.trace.TraceId; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ScopesAdapter; @@ -12,12 +11,13 @@ import io.sentry.SentryLogEvent; import io.sentry.SentryMetricsEvent; import io.sentry.SpanContext; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryId; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class OpenTelemetryOtlpEventProcessor implements EventProcessor { +public final class OpenTelemetryOtlpEventProcessor implements SentryEventProcessor { private final @NotNull IScopes scopes; diff --git a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java index 1904d2e5cf0..0dea8ee2bfa 100644 --- a/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet-jakarta/src/main/java/io/sentry/servlet/jakarta/SentryRequestHttpServletRequestProcessor.java @@ -1,9 +1,9 @@ package io.sentry.servlet.jakarta; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; @@ -18,7 +18,7 @@ import org.jetbrains.annotations.Nullable; /** Attaches information about HTTP request to {@link SentryEvent}. */ -final class SentryRequestHttpServletRequestProcessor implements EventProcessor { +final class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull HttpServletRequest httpRequest; private final @NotNull SentryOptions options; diff --git a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java index 789ed1b766f..d8a2fcd808c 100644 --- a/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-servlet/src/main/java/io/sentry/servlet/SentryRequestHttpServletRequestProcessor.java @@ -1,9 +1,9 @@ package io.sentry.servlet; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.Request; import io.sentry.util.HttpUtils; import io.sentry.util.Objects; @@ -18,7 +18,7 @@ import org.jetbrains.annotations.Nullable; /** Attaches information about HTTP request to {@link SentryEvent}. */ -final class SentryRequestHttpServletRequestProcessor implements EventProcessor { +final class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull HttpServletRequest httpRequest; private final @NotNull SentryOptions options; diff --git a/sentry-spring-7/api/sentry-spring-7.api b/sentry-spring-7/api/sentry-spring-7.api index c9250b550fd..3b2872afc67 100644 --- a/sentry-spring-7/api/sentry-spring-7.api +++ b/sentry-spring-7/api/sentry-spring-7.api @@ -3,7 +3,7 @@ public final class io/sentry/spring7/BuildConfig { public static final field VERSION_NAME Ljava/lang/String; } -public final class io/sentry/spring7/ContextTagsEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring7/ContextTagsEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -48,7 +48,7 @@ public class io/sentry/spring7/SentryProfilerConfiguration { public fun sentryOpenTelemetryProfilerConverterConfiguration ()Lio/sentry/IProfileConverter; } -public class io/sentry/spring7/SentryRequestHttpServletRequestProcessor : io/sentry/EventProcessor { +public class io/sentry/spring7/SentryRequestHttpServletRequestProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/spring7/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -92,7 +92,7 @@ public class io/sentry/spring7/SentryWebConfiguration { public fun httpServletRequestSentryUserProvider (Lio/sentry/SentryOptions;)Lio/sentry/spring7/HttpServletRequestSentryUserProvider; } -public final class io/sentry/spring7/SpringProfilesEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring7/SpringProfilesEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lorg/springframework/core/env/Environment;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; public fun process (Lio/sentry/SentryReplayEvent;Lio/sentry/Hint;)Lio/sentry/SentryReplayEvent; diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/ContextTagsEventProcessor.java b/sentry-spring-7/src/main/java/io/sentry/spring7/ContextTagsEventProcessor.java index 89fdef8d1b2..812db92d644 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/ContextTagsEventProcessor.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/ContextTagsEventProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring7; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.util.CollectionUtils; import java.util.Map; import org.jetbrains.annotations.NotNull; @@ -14,7 +14,7 @@ * Attaches context tags defined in {@link SentryOptions#getContextTags()} from {@link MDC} to * {@link SentryEvent#getTags()}. */ -public final class ContextTagsEventProcessor implements EventProcessor { +public final class ContextTagsEventProcessor implements SentryEventProcessor { private final SentryOptions options; public ContextTagsEventProcessor(final @NotNull SentryOptions options) { diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java index 2412083812d..680f560fffc 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring7; import com.jakewharton.nopen.annotation.Open; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring7.tracing.TransactionNameProvider; import io.sentry.util.Objects; import jakarta.servlet.http.HttpServletRequest; @@ -12,7 +12,7 @@ /** Attaches transaction name from the HTTP request to {@link SentryEvent}. */ @Open -public class SentryRequestHttpServletRequestProcessor implements EventProcessor { +public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java index bf2c431a179..de766504aff 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java @@ -6,7 +6,6 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Breadcrumb; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -15,6 +14,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.SentryOptions.RequestSize; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring7.tracing.SpringMvcTransactionNameProvider; import io.sentry.spring7.tracing.TransactionNameProvider; import io.sentry.util.Objects; @@ -140,7 +140,7 @@ private static boolean shouldCacheMimeType(String contentType) { } } - static final class RequestBodyExtractingEventProcessor implements EventProcessor { + static final class RequestBodyExtractingEventProcessor implements SentryEventProcessor { private final @NotNull RequestPayloadExtractor requestPayloadExtractor = new RequestPayloadExtractor(); private final @NotNull HttpServletRequest request; diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SpringProfilesEventProcessor.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SpringProfilesEventProcessor.java index 100bdd6a38b..d22d15fe767 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SpringProfilesEventProcessor.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SpringProfilesEventProcessor.java @@ -1,10 +1,10 @@ package io.sentry.spring7; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryReplayEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.Spring; import org.jetbrains.annotations.NotNull; @@ -15,7 +15,7 @@ * Attaches the list of active Spring profiles (an empty list if only the default profile is active) * to the {@link io.sentry.TraceContext} associated with the event. */ -public final class SpringProfilesEventProcessor implements EventProcessor { +public final class SpringProfilesEventProcessor implements SentryEventProcessor { private final @NotNull Environment environment; @Override diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 24b9af7e14b..be94039b754 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -3,7 +3,7 @@ public final class io/sentry/spring/jakarta/BuildConfig { public static final field VERSION_NAME Ljava/lang/String; } -public final class io/sentry/spring/jakarta/ContextTagsEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring/jakarta/ContextTagsEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -48,7 +48,7 @@ public class io/sentry/spring/jakarta/SentryProfilerConfiguration { public fun sentryOpenTelemetryProfilerConverterConfiguration ()Lio/sentry/IProfileConverter; } -public class io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor : io/sentry/EventProcessor { +public class io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/spring/jakarta/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -92,7 +92,7 @@ public class io/sentry/spring/jakarta/SentryWebConfiguration { public fun httpServletRequestSentryUserProvider (Lio/sentry/SentryOptions;)Lio/sentry/spring/jakarta/HttpServletRequestSentryUserProvider; } -public final class io/sentry/spring/jakarta/SpringProfilesEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring/jakarta/SpringProfilesEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lorg/springframework/core/env/Environment;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; public fun process (Lio/sentry/SentryReplayEvent;Lio/sentry/Hint;)Lio/sentry/SentryReplayEvent; diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/ContextTagsEventProcessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/ContextTagsEventProcessor.java index 94f49d83190..dd5b3684fa4 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/ContextTagsEventProcessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/ContextTagsEventProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring.jakarta; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.util.CollectionUtils; import java.util.Map; import org.jetbrains.annotations.NotNull; @@ -14,7 +14,7 @@ * Attaches context tags defined in {@link SentryOptions#getContextTags()} from {@link MDC} to * {@link SentryEvent#getTags()}. */ -public final class ContextTagsEventProcessor implements EventProcessor { +public final class ContextTagsEventProcessor implements SentryEventProcessor { private final SentryOptions options; public ContextTagsEventProcessor(final @NotNull SentryOptions options) { diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java index 91b27ddeac7..b18b5a914e7 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring.jakarta; import com.jakewharton.nopen.annotation.Open; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.jakarta.tracing.TransactionNameProvider; import io.sentry.util.Objects; import jakarta.servlet.http.HttpServletRequest; @@ -12,7 +12,7 @@ /** Attaches transaction name from the HTTP request to {@link SentryEvent}. */ @Open -public class SentryRequestHttpServletRequestProcessor implements EventProcessor { +public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java index c549223e559..3ce21ed4ce5 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java @@ -6,7 +6,6 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Breadcrumb; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -15,6 +14,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.SentryOptions.RequestSize; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.jakarta.tracing.SpringMvcTransactionNameProvider; import io.sentry.spring.jakarta.tracing.TransactionNameProvider; import io.sentry.util.Objects; @@ -140,7 +140,7 @@ private static boolean shouldCacheMimeType(String contentType) { } } - static final class RequestBodyExtractingEventProcessor implements EventProcessor { + static final class RequestBodyExtractingEventProcessor implements SentryEventProcessor { private final @NotNull RequestPayloadExtractor requestPayloadExtractor = new RequestPayloadExtractor(); private final @NotNull HttpServletRequest request; diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringProfilesEventProcessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringProfilesEventProcessor.java index 48957e88507..48ea9910f32 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringProfilesEventProcessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringProfilesEventProcessor.java @@ -1,10 +1,10 @@ package io.sentry.spring.jakarta; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryReplayEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.Spring; import org.jetbrains.annotations.NotNull; @@ -15,7 +15,7 @@ * Attaches the list of active Spring profiles (an empty list if only the default profile is active) * to the {@link io.sentry.TraceContext} associated with the event. */ -public final class SpringProfilesEventProcessor implements EventProcessor { +public final class SpringProfilesEventProcessor implements SentryEventProcessor { private final @NotNull Environment environment; @Override diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 4e1bea84288..7c8ff495f3b 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -3,7 +3,7 @@ public final class io/sentry/spring/BuildConfig { public static final field VERSION_NAME Ljava/lang/String; } -public final class io/sentry/spring/ContextTagsEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring/ContextTagsEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -48,7 +48,7 @@ public class io/sentry/spring/SentryProfilerConfiguration { public fun sentryOpenTelemetryProfilerConverterConfiguration ()Lio/sentry/IProfileConverter; } -public class io/sentry/spring/SentryRequestHttpServletRequestProcessor : io/sentry/EventProcessor { +public class io/sentry/spring/SentryRequestHttpServletRequestProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/spring/tracing/TransactionNameProvider;Ljavax/servlet/http/HttpServletRequest;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -92,7 +92,7 @@ public class io/sentry/spring/SentryWebConfiguration { public fun httpServletRequestSentryUserProvider (Lio/sentry/SentryOptions;)Lio/sentry/spring/HttpServletRequestSentryUserProvider; } -public final class io/sentry/spring/SpringProfilesEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/spring/SpringProfilesEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lorg/springframework/core/env/Environment;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; public fun process (Lio/sentry/SentryReplayEvent;Lio/sentry/Hint;)Lio/sentry/SentryReplayEvent; diff --git a/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java index 41ff04d0c49..52685b0a700 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/ContextTagsEventProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.util.CollectionUtils; import java.util.Map; import org.jetbrains.annotations.NotNull; @@ -14,7 +14,7 @@ * Attaches context tags defined in {@link SentryOptions#getContextTags()} from {@link MDC} to * {@link SentryEvent#getTags()}. */ -public final class ContextTagsEventProcessor implements EventProcessor { +public final class ContextTagsEventProcessor implements SentryEventProcessor { private final SentryOptions options; public ContextTagsEventProcessor(final @NotNull SentryOptions options) { diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java index 426571ba6e1..829e39b8508 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java @@ -1,9 +1,9 @@ package io.sentry.spring; import com.jakewharton.nopen.annotation.Open; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.tracing.TransactionNameProvider; import io.sentry.util.Objects; import javax.servlet.http.HttpServletRequest; @@ -12,7 +12,7 @@ /** Attaches transaction name from the HTTP request to {@link SentryEvent}. */ @Open -public class SentryRequestHttpServletRequestProcessor implements EventProcessor { +public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java b/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java index 3fe8ab9e13f..be893b67a5a 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java @@ -6,7 +6,6 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Breadcrumb; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -15,6 +14,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.SentryOptions.RequestSize; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.tracing.SpringMvcTransactionNameProvider; import io.sentry.spring.tracing.TransactionNameProvider; import io.sentry.util.Objects; @@ -140,7 +140,7 @@ private static boolean shouldCacheMimeType(String contentType) { } } - static final class RequestBodyExtractingEventProcessor implements EventProcessor { + static final class RequestBodyExtractingEventProcessor implements SentryEventProcessor { private final @NotNull RequestPayloadExtractor requestPayloadExtractor = new RequestPayloadExtractor(); private final @NotNull HttpServletRequest request; diff --git a/sentry-spring/src/main/java/io/sentry/spring/SpringProfilesEventProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/SpringProfilesEventProcessor.java index 30b6483fd31..c233a7e9824 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SpringProfilesEventProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SpringProfilesEventProcessor.java @@ -1,10 +1,10 @@ package io.sentry.spring; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryReplayEvent; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.Spring; import org.jetbrains.annotations.NotNull; @@ -15,7 +15,7 @@ * Attaches the list of active Spring profiles (an empty list if only the default profile is active) * to the {@link io.sentry.TraceContext} associated with the event. */ -public final class SpringProfilesEventProcessor implements EventProcessor { +public final class SpringProfilesEventProcessor implements SentryEventProcessor { private final @NotNull Environment environment; @Override diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 01b068ee689..400b602aa14 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -461,7 +461,7 @@ public final class io/sentry/DateUtils { public static fun toUtilDateNotNull (Lio/sentry/SentryDate;)Ljava/util/Date; } -public final class io/sentry/DeduplicateMultithreadedEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/DeduplicateMultithreadedEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -511,7 +511,7 @@ public final class io/sentry/DsnUtil { public static fun urlContainsDsnHost (Lio/sentry/SentryOptions;Ljava/lang/String;)Z } -public final class io/sentry/DuplicateEventDetectionEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/DuplicateEventDetectionEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -1483,7 +1483,7 @@ public final class io/sentry/KeyValueCollectionBehavior$Mode : java/lang/Enum { public static fun values ()[Lio/sentry/KeyValueCollectionBehavior$Mode; } -public final class io/sentry/MainEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/MainEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/SentryOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -5148,6 +5148,7 @@ public final class io/sentry/clientreport/DiscardReason : java/lang/Enum { public static final field BACKPRESSURE Lio/sentry/clientreport/DiscardReason; public static final field BEFORE_SEND Lio/sentry/clientreport/DiscardReason; public static final field CACHE_OVERFLOW Lio/sentry/clientreport/DiscardReason; + public static final field CALLBACK_ERROR Lio/sentry/clientreport/DiscardReason; public static final field EVENT_PROCESSOR Lio/sentry/clientreport/DiscardReason; public static final field NETWORK_ERROR Lio/sentry/clientreport/DiscardReason; public static final field QUEUE_OVERFLOW Lio/sentry/clientreport/DiscardReason; @@ -5446,6 +5447,9 @@ public final class io/sentry/internal/eventprocessor/EventProcessorAndOrder : ja public fun getOrder ()Ljava/lang/Long; } +public abstract interface class io/sentry/internal/eventprocessor/SentryEventProcessor : io/sentry/EventProcessor { +} + public abstract interface class io/sentry/internal/gestures/GestureTargetLocator { public abstract fun locate (Ljava/lang/Object;FFLio/sentry/internal/gestures/UiElement$Type;)Lio/sentry/internal/gestures/UiElement; } diff --git a/sentry/src/main/java/io/sentry/BackfillingEventProcessor.java b/sentry/src/main/java/io/sentry/BackfillingEventProcessor.java index 2d8d7bc5575..7a936be511f 100644 --- a/sentry/src/main/java/io/sentry/BackfillingEventProcessor.java +++ b/sentry/src/main/java/io/sentry/BackfillingEventProcessor.java @@ -4,5 +4,8 @@ * Marker interface for event processors that process events that have to be backfilled, i.e. * currently stored in-memory data (like Scope or SentryOptions) is irrelevant, because the event * happened in the past. + * + *

SDK-owned implementations must also implement {@link + * io.sentry.internal.eventprocessor.SentryEventProcessor}. */ public interface BackfillingEventProcessor extends EventProcessor {} diff --git a/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java b/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java index b5869a63796..7053259c61c 100644 --- a/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java +++ b/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.hints.EventDropReason; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryException; import io.sentry.util.HintUtils; import java.util.Collections; @@ -14,7 +15,7 @@ * multiple threads. This can be the case for OutOfMemory errors or CursorWindowAllocationException, * basically any error related to allocating memory when it's low. */ -public final class DeduplicateMultithreadedEventProcessor implements EventProcessor { +public final class DeduplicateMultithreadedEventProcessor implements SentryEventProcessor { private final @NotNull Map processedEvents = Collections.synchronizedMap(new HashMap<>()); diff --git a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java index b46eb91746b..c3f02e03141 100644 --- a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java +++ b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import java.util.ArrayList; import java.util.Collections; import java.util.IdentityHashMap; @@ -11,7 +12,7 @@ import org.jetbrains.annotations.Nullable; /** Deduplicates events containing throwable that has been already processed. */ -public final class DuplicateEventDetectionEventProcessor implements EventProcessor { +public final class DuplicateEventDetectionEventProcessor implements SentryEventProcessor { private final @NotNull Map capturedObjects = Collections.synchronizedMap(new WeakHashMap<>()); private final @NotNull SentryOptions options; diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 79b45a30c65..25ec4b6966a 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -2,6 +2,7 @@ import io.sentry.hints.AbnormalExit; import io.sentry.hints.Cached; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.DebugMeta; import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryException; @@ -16,7 +17,7 @@ import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class MainEventProcessor implements EventProcessor { +public final class MainEventProcessor implements SentryEventProcessor { private final @NotNull SentryOptions options; private final @NotNull SentryThreadFactory sentryThreadFactory; diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 54e8b893555..42aef2821ac 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -472,12 +472,9 @@ public Queue getBreadcrumbs() { .getLogger() .log( SentryLevel.ERROR, - "The BeforeBreadcrumbCallback callback threw an exception. Exception details will be added to the breadcrumb.", + "The BeforeBreadcrumb callback threw an exception. Dropping breadcrumb.", e); - - if (e.getMessage() != null) { - breadcrumb.setData("sentry:message", e.getMessage()); - } + return null; } return breadcrumb; } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 012587eaa59..c0758a4efd4 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -8,6 +8,7 @@ import io.sentry.hints.Cached; import io.sentry.hints.DiskFlushNotification; import io.sentry.hints.TransactionEnd; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.logger.ILoggerBatchProcessor; import io.sentry.metrics.IMetricsBatchProcessor; import io.sentry.protocol.Contexts; @@ -160,9 +161,6 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul if (event == null) { options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); } } @@ -235,9 +233,14 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul .getLogger() .log( SentryLevel.ERROR, - "The beforeErrorSampling callback threw an exception. Proceeding with replay capture.", + "The beforeErrorSampling callback threw an exception. Skipping replay capture.", e); - shouldCaptureReplay = true; + if (!SentryId.EMPTY_ID.equals(options.getReplayController().getReplayId())) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay); + } + shouldCaptureReplay = false; } } if (shouldCaptureReplay) { @@ -334,9 +337,6 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin if (event == null) { options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSendReplay"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Replay); } } @@ -501,6 +501,12 @@ private SentryEvent processEvent( e, "An exception occurred while processing event by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Error); + return null; + } } if (event == null) { @@ -519,6 +525,24 @@ private SentryEvent processEvent( return event; } + private void recordLostLogEvent( + final @NotNull DiscardReason reason, final @NotNull SentryLogEvent event) { + options.getClientReportRecorder().recordLostEvent(reason, DataCategory.LogItem); + final long numberOfBytes = + JsonSerializationUtils.byteSizeOf(options.getSerializer(), options.getLogger(), event); + options.getClientReportRecorder().recordLostEvent(reason, DataCategory.LogByte, numberOfBytes); + } + + private void recordLostMetricsEvent( + final @NotNull DiscardReason reason, final @NotNull SentryMetricsEvent event) { + options.getClientReportRecorder().recordLostEvent(reason, DataCategory.TraceMetric); + final long numberOfBytes = + JsonSerializationUtils.byteSizeOf(options.getSerializer(), options.getLogger(), event); + options + .getClientReportRecorder() + .recordLostEvent(reason, DataCategory.TraceMetricByte, numberOfBytes); + } + @Nullable private SentryLogEvent processLogEvent( @NotNull SentryLogEvent event, final @NotNull List eventProcessors) { @@ -534,6 +558,10 @@ private SentryLogEvent processLogEvent( e, "An exception occurred while processing log event by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + recordLostLogEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor); + return null; + } } if (event == null) { @@ -543,16 +571,7 @@ private SentryLogEvent processLogEvent( SentryLevel.DEBUG, "Log event was dropped by a processor: %s", processor.getClass().getName()); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.LogItem); - final long logEventNumberOfBytes = - JsonSerializationUtils.byteSizeOf( - options.getSerializer(), options.getLogger(), eventBeforeProcessor); - options - .getClientReportRecorder() - .recordLostEvent( - DiscardReason.EVENT_PROCESSOR, DataCategory.LogByte, logEventNumberOfBytes); + recordLostLogEvent(DiscardReason.EVENT_PROCESSOR, eventBeforeProcessor); break; } } @@ -576,6 +595,10 @@ private SentryMetricsEvent processMetricsEvent( e, "An exception occurred while processing metrics event by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + recordLostMetricsEvent(DiscardReason.CALLBACK_ERROR, eventBeforeProcessor); + return null; + } } if (event == null) { @@ -585,18 +608,7 @@ private SentryMetricsEvent processMetricsEvent( SentryLevel.DEBUG, "Metrics event was dropped by a processor: %s", processor.getClass().getName()); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.TraceMetric); - final long metricsEventNumberOfBytes = - JsonSerializationUtils.byteSizeOf( - options.getSerializer(), options.getLogger(), eventBeforeProcessor); - options - .getClientReportRecorder() - .recordLostEvent( - DiscardReason.EVENT_PROCESSOR, - DataCategory.TraceMetricByte, - metricsEventNumberOfBytes); + recordLostMetricsEvent(DiscardReason.EVENT_PROCESSOR, eventBeforeProcessor); break; } } @@ -606,7 +618,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 { @@ -619,6 +632,21 @@ private SentryMetricsEvent processMetricsEvent( e, "An exception occurred while processing transaction by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction); + options + .getClientReportRecorder() + .recordLostEvent( + DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeProcessor + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile); + } + return null; + } } final int spanCountAfterProcessor = transaction == null ? 0 : transaction.getSpans().size(); @@ -637,6 +665,11 @@ private SentryMetricsEvent processMetricsEvent( .getClientReportRecorder() .recordLostEvent( DiscardReason.EVENT_PROCESSOR, DataCategory.Span, spanCountBeforeProcessor + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Profile); + } break; } else if (spanCountAfterProcessor < spanCountBeforeProcessor) { // If the callback removed some spans, we report it @@ -672,6 +705,12 @@ private SentryReplayEvent processReplayEvent( e, "An exception occurred while processing replay event by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay); + return null; + } } if (replayEvent == null) { @@ -706,6 +745,12 @@ private SentryEvent processFeedbackEvent( e, "An exception occurred while processing feedback event by processor: %s", processor.getClass().getName()); + if (!(processor instanceof SentryEventProcessor)) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback); + return null; + } } if (feedbackEvent == null) { @@ -1021,7 +1066,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) { @@ -1030,7 +1077,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) { @@ -1038,35 +1087,13 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint return SentryId.EMPTY_ID; } - final int spanCountBeforeCallback = transaction.getSpans().size(); - transaction = executeBeforeSendTransaction(transaction, hint); - final int spanCountAfterCallback = transaction == null ? 0 : transaction.getSpans().size(); + transaction = executeBeforeSendTransaction(transaction, hint, profilingTraceData != null); if (transaction == null) { options .getLogger() .log(SentryLevel.DEBUG, "Transaction was dropped by beforeSendTransaction."); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Transaction); - // If we drop a transaction, we are also dropping all its spans (+1 for the root span) - options - .getClientReportRecorder() - .recordLostEvent( - DiscardReason.BEFORE_SEND, DataCategory.Span, spanCountBeforeCallback + 1); return SentryId.EMPTY_ID; - } else if (spanCountAfterCallback < spanCountBeforeCallback) { - // If the callback removed some spans, we report it - final int droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback; - options - .getLogger() - .log( - SentryLevel.DEBUG, - "%d spans were dropped by beforeSendTransaction.", - droppedSpanCount); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Span, droppedSpanCount); } try { @@ -1245,9 +1272,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint if (event == null) { options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Feedback); } } @@ -1345,21 +1369,10 @@ public void captureLog(@Nullable SentryLogEvent logEvent, @Nullable IScope scope } if (logEvent != null) { - final @NotNull SentryLogEvent tmpLogEvent = logEvent; logEvent = executeBeforeSendLog(logEvent); if (logEvent == null) { options.getLogger().log(SentryLevel.DEBUG, "Log Event was dropped by beforeSendLog"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.LogItem); - final @NotNull long logEventNumberOfBytes = - JsonSerializationUtils.byteSizeOf( - options.getSerializer(), options.getLogger(), tmpLogEvent); - options - .getClientReportRecorder() - .recordLostEvent( - DiscardReason.BEFORE_SEND, DataCategory.LogByte, logEventNumberOfBytes); return; } @@ -1408,23 +1421,12 @@ public void captureMetric( } if (metricsEvent != null) { - final @NotNull SentryMetricsEvent tmpMetricsEvent = metricsEvent; metricsEvent = executeBeforeSendMetric(metricsEvent, hint); if (metricsEvent == null) { options .getLogger() .log(SentryLevel.DEBUG, "Metrics Event was dropped by beforeSendMetrics"); - options - .getClientReportRecorder() - .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.TraceMetric); - final long metricsEventNumberOfBytes = - JsonSerializationUtils.byteSizeOf( - options.getSerializer(), options.getLogger(), tmpMetricsEvent); - options - .getClientReportRecorder() - .recordLostEvent( - DiscardReason.BEFORE_SEND, DataCategory.TraceMetricByte, metricsEventNumberOfBytes); return; } @@ -1662,21 +1664,28 @@ private void sortBreadcrumbsByDate( .getLogger() .log( SentryLevel.ERROR, - "The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.", + "The beforeSend callback threw an exception. Dropping event.", e); - - // drop event in case of an error in beforeSend due to PII concerns - event = null; + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Error); + return null; + } + if (event == null) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Error); } } return event; } 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) { + final int spanCountBeforeCallback = transaction.getSpans().size(); try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) { transaction = beforeSendTransaction.execute(transaction, hint); } catch (Throwable e) { @@ -1684,11 +1693,50 @@ private void sortBreadcrumbsByDate( .getLogger() .log( SentryLevel.ERROR, - "The BeforeSendTransaction callback threw an exception. It will be added as breadcrumb and continue.", + "The beforeSendTransaction callback threw an exception. Dropping transaction.", e); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction); + options + .getClientReportRecorder() + .recordLostEvent( + DiscardReason.CALLBACK_ERROR, DataCategory.Span, spanCountBeforeCallback + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Profile); + } + return null; + } - // drop transaction in case of an error in beforeSend due to PII concerns - transaction = null; + if (transaction == null) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Transaction); + options + .getClientReportRecorder() + .recordLostEvent( + DiscardReason.BEFORE_SEND, DataCategory.Span, spanCountBeforeCallback + 1); + if (hasProfile) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Profile); + } + } else { + final int spanCountAfterCallback = transaction.getSpans().size(); + if (spanCountAfterCallback < spanCountBeforeCallback) { + final int droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback; + options + .getLogger() + .log( + SentryLevel.DEBUG, + "%d spans were dropped by beforeSendTransaction.", + droppedSpanCount); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Span, droppedSpanCount); + } } } return transaction; @@ -1703,10 +1751,19 @@ private void sortBreadcrumbsByDate( } catch (Throwable e) { options .getLogger() - .log(SentryLevel.ERROR, "The BeforeSendFeedback callback threw an exception.", e); - - // drop feedback in case of an error in beforeSend due to PII concerns - event = null; + .log( + SentryLevel.ERROR, + "The beforeSendFeedback callback threw an exception. Dropping feedback.", + e); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback); + return null; + } + if (event == null) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Feedback); } } return event; @@ -1723,11 +1780,17 @@ private void sortBreadcrumbsByDate( .getLogger() .log( SentryLevel.ERROR, - "The BeforeSendReplay callback threw an exception. It will be added as breadcrumb and continue.", + "The beforeSendReplay callback threw an exception. Dropping replay event.", e); - - // drop event in case of an error in beforeSend due to PII concerns - event = null; + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.CALLBACK_ERROR, DataCategory.Replay); + return null; + } + if (event == null) { + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Replay); } } return event; @@ -1737,6 +1800,7 @@ private void sortBreadcrumbsByDate( final SentryOptions.Logs.BeforeSendLogCallback beforeSendLog = options.getLogs().getBeforeSend(); if (beforeSendLog != null) { + final @NotNull SentryLogEvent eventBeforeCallback = event; try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) { event = beforeSendLog.execute(event); } catch (Throwable e) { @@ -1744,11 +1808,13 @@ private void sortBreadcrumbsByDate( .getLogger() .log( SentryLevel.ERROR, - "The BeforeSendLog callback threw an exception. Dropping log event.", + "The beforeSendLog callback threw an exception. Dropping log event.", e); - - // drop event in case of an error in beforeSendLog due to PII concerns - event = null; + recordLostLogEvent(DiscardReason.CALLBACK_ERROR, eventBeforeCallback); + return null; + } + if (event == null) { + recordLostLogEvent(DiscardReason.BEFORE_SEND, eventBeforeCallback); } } return event; @@ -1759,6 +1825,7 @@ private void sortBreadcrumbsByDate( final SentryOptions.Metrics.BeforeSendMetricCallback beforeSendMetric = options.getMetrics().getBeforeSend(); if (beforeSendMetric != null) { + final @NotNull SentryMetricsEvent eventBeforeCallback = event; try (final @NotNull ISentryLifecycleToken ignored = SentryCallbackReentrancyGuard.enter()) { event = beforeSendMetric.execute(event, hint); } catch (Throwable e) { @@ -1766,11 +1833,13 @@ private void sortBreadcrumbsByDate( .getLogger() .log( SentryLevel.ERROR, - "The BeforeSendMetric callback threw an exception. Dropping metrics event.", + "The beforeSendMetric callback threw an exception. Dropping metrics event.", e); - - // drop event in case of an error in beforeSendMetric due to PII concerns - event = null; + recordLostMetricsEvent(DiscardReason.CALLBACK_ERROR, eventBeforeCallback); + return null; + } + if (event == null) { + recordLostMetricsEvent(DiscardReason.BEFORE_SEND, eventBeforeCallback); } } return event; diff --git a/sentry/src/main/java/io/sentry/SentryRuntimeEventProcessor.java b/sentry/src/main/java/io/sentry/SentryRuntimeEventProcessor.java index ca19a9ca74d..013cb7ac2b1 100644 --- a/sentry/src/main/java/io/sentry/SentryRuntimeEventProcessor.java +++ b/sentry/src/main/java/io/sentry/SentryRuntimeEventProcessor.java @@ -1,12 +1,13 @@ package io.sentry; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryRuntime; import io.sentry.protocol.SentryTransaction; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** Attaches Java vendor and version to events and transactions. */ -final class SentryRuntimeEventProcessor implements EventProcessor { +final class SentryRuntimeEventProcessor implements SentryEventProcessor { private final @Nullable String javaVersion; private final @Nullable String javaVendor; diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index 5430b9242ac..bb6e5d9f11f 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; @@ -25,28 +26,37 @@ 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); if (options.getTracesSampler() != null) { - Double samplerResult = null; + final Double samplerResult; try { samplerResult = options.getTracesSampler().sample(samplingContext); } catch (Throwable t) { 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( @@ -61,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/main/java/io/sentry/clientreport/DiscardReason.java b/sentry/src/main/java/io/sentry/clientreport/DiscardReason.java index 98f25386a5f..4fc424dfa1a 100644 --- a/sentry/src/main/java/io/sentry/clientreport/DiscardReason.java +++ b/sentry/src/main/java/io/sentry/clientreport/DiscardReason.java @@ -8,6 +8,7 @@ public enum DiscardReason { SEND_ERROR("send_error"), SAMPLE_RATE("sample_rate"), BEFORE_SEND("before_send"), + CALLBACK_ERROR("callback_error"), EVENT_PROCESSOR("event_processor"), // also for ignored exceptions BACKPRESSURE("backpressure"); diff --git a/sentry/src/main/java/io/sentry/internal/eventprocessor/SentryEventProcessor.java b/sentry/src/main/java/io/sentry/internal/eventprocessor/SentryEventProcessor.java new file mode 100644 index 00000000000..160dfe0fd78 --- /dev/null +++ b/sentry/src/main/java/io/sentry/internal/eventprocessor/SentryEventProcessor.java @@ -0,0 +1,8 @@ +package io.sentry.internal.eventprocessor; + +import io.sentry.EventProcessor; +import org.jetbrains.annotations.ApiStatus; + +/** Marker interface for event processors implemented by the Sentry SDK. */ +@ApiStatus.Internal +public interface SentryEventProcessor extends EventProcessor {} diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index be6a22516c0..69f19b6d421 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/sentry/src/test/java/io/sentry/ScopeTest.kt @@ -1,6 +1,8 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.SentryLevel.WARNING +import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport import io.sentry.protocol.Request import io.sentry.protocol.SentryId import io.sentry.protocol.User @@ -334,16 +336,44 @@ class ScopeTest { } @Test - fun `when adding breadcrumb, executeBreadcrumb will be executed and throw, but breadcrumb will be added`() { - val exception = Exception("test") + fun `when beforeBreadcrumb throws, breadcrumb is dropped without notifying observers`() { + val observer = mock() + val options = + SentryOptions().apply { + setBeforeBreadcrumb { _, _ -> throw Exception("test") } + addScopeObserver(observer) + } - val options = SentryOptions().apply { setBeforeBreadcrumb { _, _ -> throw exception } } + val scope = Scope(options) + val breadcrumb = Breadcrumb() + scope.addBreadcrumb(breadcrumb) + + assertThat(scope.breadcrumbs).isEmpty() + assertThat(breadcrumb.data).doesNotContainKey("sentry:message") + verifyNoInteractions(observer) + assertClientReport(options.clientReportRecorder, emptyList()) + } + + @Test + fun `when beforeBreadcrumb throws, later breadcrumbs can still be added`() { + var invocationCount = 0 + val options = + SentryOptions().apply { + setBeforeBreadcrumb { breadcrumb, _ -> + invocationCount++ + if (invocationCount == 1) { + throw Exception("test") + } + breadcrumb + } + } val scope = Scope(options) - val actual = Breadcrumb() - scope.addBreadcrumb(actual) + scope.addBreadcrumb(Breadcrumb("dropped")) + scope.addBreadcrumb(Breadcrumb("kept")) - assertEquals("test", actual.data["sentry:message"]) + assertThat(invocationCount).isEqualTo(2) + assertThat(scope.breadcrumbs.single().message).isEqualTo("kept") } @Test diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 13a57ebe70a..ba0ec629b6e 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -1,5 +1,6 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.backpressure.IBackpressureMonitor import io.sentry.cache.EnvelopeCache import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport @@ -236,22 +237,23 @@ class ScopesTest { } @Test - fun `when beforeSend throws an exception, breadcrumb adds an entry to the data field with exception message`() { - val exception = Exception("test") - + fun `when beforeBreadcrumb throws an exception, breadcrumb is dropped`() { val options = SentryOptions() options.cacheDirPath = file.absolutePath options.beforeBreadcrumb = SentryOptions.BeforeBreadcrumbCallback { _: Breadcrumb, _: Any? -> - throw exception + throw Exception("test") } options.dsn = "https://key@sentry.io/proj" options.setSerializer(mock()) val sut = createScopes(options) - val actual = Breadcrumb() - sut.addBreadcrumb(actual) + val breadcrumb = Breadcrumb() + sut.addBreadcrumb(breadcrumb) - assertEquals("test", actual.data["sentry:message"]) + var breadcrumbs: Queue? = null + sut.configureScope { breadcrumbs = it.breadcrumbs } + assertThat(breadcrumbs).isEmpty() + assertThat(breadcrumb.data).doesNotContainKey("sentry:message") } @Test @@ -1658,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/SentryClientInternalEventProcessorTest.kt b/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt new file mode 100644 index 00000000000..6b7934e3685 --- /dev/null +++ b/sentry/src/test/java/io/sentry/SentryClientInternalEventProcessorTest.kt @@ -0,0 +1,258 @@ +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.internal.eventprocessor.SentryEventProcessor +import io.sentry.protocol.Feedback +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.check +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.same +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever + +@RunWith(Parameterized::class) +class SentryClientInternalEventProcessorTest(private val onScope: Boolean) { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "onScope={0}") + fun data(): List> = listOf(arrayOf(false), arrayOf(true)) + } + + private val fixture = SentryClientTest.Fixture() + private val options = fixture.sentryOptions + private val scope = Scope(options) + private val processor = mock() + private val nextProcessor = mock() + private val onDiscard = mock() + private val logger = mock() + private val failure = IllegalStateException("SDK processor failed") + + init { + options.eventProcessors.clear() + options.onDiscard = onDiscard + options.setLogger(logger) + options.logs.isEnabled = true + options.metrics.isEnabled = true + if (onScope) { + scope.addEventProcessor(processor) + scope.addEventProcessor(nextProcessor) + } else { + options.addEventProcessor(processor) + options.addEventProcessor(nextProcessor) + } + } + + @Test + fun `SDK event processor failure keeps event and runs remaining callbacks`() { + val event = SentryEvent() + val beforeSend = mock() + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] } + options.beforeSend = beforeSend + + val id = fixture.getSut().captureEvent(event, scope) + + assertThat(id).isEqualTo(event.eventId) + verify(nextProcessor).process(same(event), any()) + verify(beforeSend).execute(same(event), any()) + verify(fixture.transport) + .send(check { assertThat(it.header.eventId).isEqualTo(id) }, anyOrNull()) + assertFailureLoggedWithoutLoss("event") + } + + @Test + fun `SDK transaction processor failure keeps transaction and spans`() { + val transaction = SentryTransaction(fixture.sentryTracer) + val beforeSend = mock() + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] } + options.beforeSendTransaction = beforeSend + + val id = fixture.getSut().captureTransaction(transaction, scope, null) + + assertThat(id).isEqualTo(transaction.eventId) + verify(nextProcessor).process(same(transaction), any()) + verify(beforeSend).execute(same(transaction), any()) + verify(fixture.transport) + .send( + check { + val sent = it.items.first().getTransaction(options.serializer)!! + assertThat(sent.eventId).isEqualTo(id) + assertThat(sent.spans).hasSize(1) + }, + anyOrNull(), + ) + 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") + val beforeSend = mock() + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] } + options.beforeSendFeedback = beforeSend + + val id = fixture.getSut().captureFeedback(feedback, null, scope) + + assertThat(id).isNotEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor) + .process( + check { + assertThat(it.contexts.feedback).isSameInstanceAs(feedback) + }, + any(), + ) + verify(beforeSend).execute(check { assertThat(it.eventId).isEqualTo(id) }, any()) + verify(fixture.transport) + .send(check { assertThat(it.header.eventId).isEqualTo(id) }, anyOrNull()) + assertFailureLoggedWithoutLoss("feedback event") + } + + @Test + fun `SDK log processor failure keeps log and runs remaining callbacks`() { + val event = SentryLogEvent(SentryId(), SentryNanotimeDate(), "message", SentryLogLevel.WARN) + val beforeSend = mock() + whenever(processor.process(any())).thenThrow(failure) + whenever(nextProcessor.process(any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any())).thenAnswer { it.arguments[0] } + options.logs.beforeSend = beforeSend + + fixture.getSut().captureLog(event, scope) + + verify(nextProcessor).process(same(event)) + verify(beforeSend).execute(same(event)) + verify(fixture.loggerBatchProcessor).add(same(event)) + assertFailureLoggedWithoutLoss("log event") + } + + @Test + fun `SDK metric processor failure keeps metric and runs remaining callbacks`() { + val event = SentryMetricsEvent(SentryId(), SentryNanotimeDate(), "name", "gauge", 123.0) + val beforeSend = mock() + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] } + options.metrics.beforeSend = beforeSend + + fixture.getSut().captureMetric(event, scope, null) + + verify(nextProcessor).process(same(event), any()) + verify(beforeSend).execute(same(event), any()) + verify(fixture.metricsBatchProcessor).add(same(event)) + assertFailureLoggedWithoutLoss("metrics event") + } + + @Test + fun `SDK processor returning null still drops event as event_processor`() { + whenever(processor.process(any(), any())).thenReturn(null) + + val id = fixture.getSut().captureEvent(SentryEvent(), scope) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor, never()).process(any(), any()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + options.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Error.category, 1)), + ) + } + + @Test + fun `customer processor failure after SDK processor failure still drops event`() { + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())) + .thenThrow(IllegalArgumentException("customer")) + val beforeSend = mock() + options.beforeSend = beforeSend + + val id = fixture.getSut().captureEvent(SentryEvent(), scope) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor).process(any(), any()) + verifyNoInteractions(beforeSend) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + options.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Error.category, 1)), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Error, 1) + } + + @Test + fun `spans removed before SDK processor failure retain event_processor accounting`() { + val transaction = SentryTransaction(fixture.sentryTracer) + whenever(processor.process(any(), any())).doAnswer { + transaction.spans.clear() + throw failure + } + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + + val id = fixture.getSut().captureTransaction(transaction, scope, null) + + assertThat(id).isEqualTo(transaction.eventId) + verify(nextProcessor).process(same(transaction), any()) + verify(fixture.transport).send(any(), anyOrNull()) + assertClientReport( + options.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Span.category, 1)), + ) + } + + private fun assertFailureLoggedWithoutLoss(item: String) { + verify(logger) + .log( + eq(SentryLevel.ERROR), + same(failure), + eq("An exception occurred while processing $item by processor: %s"), + eq(processor.javaClass.name), + ) + assertClientReport(options.clientReportRecorder, emptyList()) + verifyNoInteractions(onDiscard) + } +} diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index d9ed1df0f2d..e3bc59c549a 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1,5 +1,6 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.Scope.IWithPropagationContext import io.sentry.SentryLevel.WARNING import io.sentry.Session.State.Crashed @@ -13,6 +14,7 @@ import io.sentry.hints.Backfillable import io.sentry.hints.Cached import io.sentry.hints.DiskFlushNotification import io.sentry.hints.TransactionEnd +import io.sentry.internal.eventprocessor.SentryEventProcessor import io.sentry.logger.ILoggerBatchProcessor import io.sentry.logger.ILoggerBatchProcessorFactory import io.sentry.metrics.IMetricsBatchProcessor @@ -272,9 +274,10 @@ class SentryClientTest { @Test fun `when beforeSend throws an exception, event is dropped`() { val exception = Exception("test") + val onDiscardMock = mock() - exception.stackTrace.toString() fixture.sentryOptions.setBeforeSend { _, _ -> throw exception } + fixture.sentryOptions.onDiscard = onDiscardMock val sut = fixture.getSut() val actual = SentryEvent() val id = sut.captureEvent(actual) @@ -283,8 +286,9 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, - listOf(DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Error.category, 1)), + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Error.category, 1)), ) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Error, 1) } @Test @@ -429,9 +433,10 @@ class SentryClientTest { fun `when beforeSendLog throws an exception, log is dropped`() { val scope = createScope() val exception = Exception("test") + val onDiscardMock = mock() - exception.stackTrace.toString() fixture.sentryOptions.logs.setBeforeSend { _ -> throw exception } + fixture.sentryOptions.onDiscard = onDiscardMock val sut = fixture.getSut() sut.captureLog( SentryLogEvent(SentryId(), SentryNanotimeDate(), "message", SentryLogLevel.WARN), @@ -441,10 +446,12 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, listOf( - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.LogItem.category, 1), - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.LogByte.category, 109), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.LogItem.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.LogByte.category, 109), ), ) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.LogItem, 1) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.LogByte, 109) } @Test @@ -480,6 +487,48 @@ class SentryClientTest { ) } + @Test + fun `throwing log processor drops log and stops callbacks`() { + val scope = createScope() + val logEvent = SentryLogEvent(SentryId(), SentryNanotimeDate(), "message", SentryLogLevel.WARN) + val logEventNumberOfBytes = + JsonSerializationUtils.byteSizeOf( + fixture.sentryOptions.serializer, + fixture.sentryOptions.logger, + logEvent, + ) + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any())) + .thenThrow(IllegalStateException("test")) + scope.addEventProcessor(throwingProcessor) + scope.addEventProcessor(nextProcessor) + fixture.sentryOptions.logs.beforeSend = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + fixture.getSut().captureLog(logEvent, scope) + + verify(nextProcessor, never()).process(any()) + verify(beforeSend, never()).execute(any()) + verify(fixture.loggerBatchProcessor, never()).add(any()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.LogItem.category, 1), + DiscardedEvent( + DiscardReason.CALLBACK_ERROR.reason, + DataCategory.LogByte.category, + logEventNumberOfBytes, + ), + ), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.LogItem, 1) + verify(onDiscard) + .execute(DiscardReason.CALLBACK_ERROR, DataCategory.LogByte, logEventNumberOfBytes) + } + @Test fun `when beforeSendLog is returns new instance, new instance is sent`() { val scope = createScope() @@ -542,9 +591,10 @@ class SentryClientTest { fun `when beforeSendMetric throws an exception, metric is dropped`() { val scope = createScope() val exception = Exception("test") + val onDiscardMock = mock() - exception.stackTrace.toString() - fixture.sentryOptions.metrics.setBeforeSend { _, hint -> throw exception } + fixture.sentryOptions.metrics.setBeforeSend { _, _ -> throw exception } + fixture.sentryOptions.onDiscard = onDiscardMock val sut = fixture.getSut() sut.captureMetric( SentryMetricsEvent(SentryId(), SentryNanotimeDate(), "name", "gauge", 123.0), @@ -555,14 +605,16 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, listOf( - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.TraceMetric.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.TraceMetric.category, 1), DiscardedEvent( - DiscardReason.BEFORE_SEND.reason, + DiscardReason.CALLBACK_ERROR.reason, DataCategory.TraceMetricByte.category, 120, ), ), ) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.TraceMetric, 1) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.TraceMetricByte, 120) } @Test @@ -598,6 +650,56 @@ class SentryClientTest { ) } + @Test + fun `throwing metric processor drops metric and stops callbacks`() { + val scope = createScope() + val metricsEvent = SentryMetricsEvent(SentryId(), SentryNanotimeDate(), "name", "gauge", 123.0) + val metricsEventNumberOfBytes = + JsonSerializationUtils.byteSizeOf( + fixture.sentryOptions.serializer, + fixture.sentryOptions.logger, + metricsEvent, + ) + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any(), anyOrNull())) + .thenThrow(IllegalStateException("test")) + scope.addEventProcessor(throwingProcessor) + scope.addEventProcessor(nextProcessor) + fixture.sentryOptions.metrics.beforeSend = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + fixture.getSut().captureMetric(metricsEvent, scope, null) + + verify(nextProcessor, never()).process(any(), anyOrNull()) + verify(beforeSend, never()).execute(any(), anyOrNull()) + verify(fixture.metricsBatchProcessor, never()).add(any()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent( + DiscardReason.CALLBACK_ERROR.reason, + DataCategory.TraceMetric.category, + 1, + ), + DiscardedEvent( + DiscardReason.CALLBACK_ERROR.reason, + DataCategory.TraceMetricByte.category, + metricsEventNumberOfBytes, + ), + ), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.TraceMetric, 1) + verify(onDiscard) + .execute( + DiscardReason.CALLBACK_ERROR, + DataCategory.TraceMetricByte, + metricsEventNumberOfBytes, + ) + } + @Test fun `when beforeSendMetric is returns new instance, new instance is sent`() { val scope = createScope() @@ -1245,6 +1347,42 @@ class SentryClientTest { ) } + @Test + fun `throwing transaction processor drops transaction and stops callbacks`() { + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any(), anyOrNull())) + .thenThrow(IllegalStateException("test")) + fixture.sentryOptions.addEventProcessor(throwingProcessor) + fixture.sentryOptions.addEventProcessor(nextProcessor) + fixture.sentryOptions.beforeSendTransaction = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + val id = + fixture + .getSut() + .captureTransaction( + SentryTransaction(fixture.sentryTracer), + fixture.sentryTracer.traceContext(), + ) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor, never()).process(any(), anyOrNull()) + verify(beforeSend, never()).execute(any(), anyOrNull()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 2), + ), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 2) + } + @Test fun `transaction dropped by ignoredTransactions is recorded`() { fixture.sentryOptions.setIgnoredTransactions(listOf("a-transaction")) @@ -1563,13 +1701,14 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, listOf( - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Transaction.category, 1), - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Span.category, 2), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Span.category, 2), ), ) - verify(onDiscardMock, times(1)).execute(DiscardReason.BEFORE_SEND, DataCategory.Transaction, 1) - verify(onDiscardMock).execute(DiscardReason.BEFORE_SEND, DataCategory.Span, 2) + verify(onDiscardMock, times(1)) + .execute(DiscardReason.CALLBACK_ERROR, DataCategory.Transaction, 1) + verify(onDiscardMock).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Span, 2) } @Test @@ -1929,10 +2068,29 @@ class SentryClientTest { } @Test - fun `exception thrown by an event processor is handled gracefully`() { - fixture.sentryOptions.addEventProcessor(eventProcessorThrows()) - val sut = fixture.getSut() - sut.captureEvent(SentryEvent()) + fun `exception thrown by an event processor drops event and stops callbacks`() { + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any(), anyOrNull())) + .thenThrow(IllegalStateException("test")) + fixture.sentryOptions.addEventProcessor(throwingProcessor) + fixture.sentryOptions.addEventProcessor(nextProcessor) + fixture.sentryOptions.beforeSend = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + val id = fixture.getSut().captureEvent(SentryEvent()) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor, never()).process(any(), anyOrNull()) + verify(beforeSend, never()).execute(any(), anyOrNull()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Error.category, 1)), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Error, 1) } @Test @@ -3526,6 +3684,74 @@ class SentryClientTest { verify(onDiscardMock, times(1)).execute(DiscardReason.EVENT_PROCESSOR, DataCategory.Replay, 1) } + @Test + fun `throwing replay processor drops replay and stops callbacks`() { + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any(), anyOrNull())) + .thenThrow(IllegalStateException("test")) + fixture.sentryOptions.addEventProcessor(throwingProcessor) + fixture.sentryOptions.addEventProcessor(nextProcessor) + fixture.sentryOptions.beforeSendReplay = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + val id = fixture.getSut().captureReplayEvent(createReplayEvent(), createScope(), null) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor, never()).process(any(), anyOrNull()) + verify(beforeSend, never()).execute(any(), anyOrNull()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Replay.category, 1)), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Replay, 1) + } + + @Test + fun `throwing SDK replay processor keeps replay and runs remaining callbacks`() { + val processor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + val logger = mock() + val failure = IllegalStateException("SDK processor failed") + val replay = createReplayEvent() + whenever(processor.process(any(), any())).thenThrow(failure) + whenever(nextProcessor.process(any(), any())).thenAnswer { it.arguments[0] } + whenever(beforeSend.execute(any(), any())).thenAnswer { it.arguments[0] } + fixture.sentryOptions.addEventProcessor(processor) + fixture.sentryOptions.addEventProcessor(nextProcessor) + fixture.sentryOptions.beforeSendReplay = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + fixture.sentryOptions.setLogger(logger) + + val id = fixture.getSut().captureReplayEvent(replay, createScope(), null) + + assertThat(id).isEqualTo(replay.eventId) + verify(nextProcessor).process(eq(replay), any()) + verify(beforeSend).execute(eq(replay), any()) + verify(fixture.transport) + .send( + check { + assertThat(it.header.eventId).isEqualTo(id) + assertThat(it.items.first().header.type).isEqualTo(SentryItemType.ReplayVideo) + }, + anyOrNull(), + ) + verify(logger) + .log( + eq(SentryLevel.ERROR), + eq(failure), + eq("An exception occurred while processing replay event by processor: %s"), + eq(processor.javaClass.name), + ) + assertClientReport(fixture.sentryOptions.clientReportRecorder, emptyList()) + verifyNoInteractions(onDiscard) + } + @Test fun `calls captureReplay on replay controller for error events`() { var called = false @@ -3747,24 +3973,78 @@ class SentryClientTest { } @Test - fun `beforeErrorSampling throwing exception proceeds with captureReplay`() { - var called = false - fixture.sentryOptions.setReplayController( - object : ReplayController by NoOpReplayController.getInstance() { - override fun captureReplay(isTerminating: Boolean?): SentryId { - called = true - return SentryId.EMPTY_ID + fun `beforeErrorSampling throwing exception skips replay but still sends errors and crashes`() { + val replayController = mock() + whenever(replayController.replayId).thenReturn(SentryId()) + whenever(replayController.captureReplay(anyOrNull())).thenReturn(SentryId.EMPTY_ID) + fixture.sentryOptions.setReplayController(replayController) + val logger = mock() + fixture.sentryOptions.setLogger(logger) + val exception = RuntimeException("test") + fixture.sentryOptions.sessionReplay.beforeErrorSampling = + SentryReplayOptions.BeforeErrorSamplingCallback { _, _ -> + throw exception + } + val sut = fixture.getSut() + val events = + listOf(true, false).map { handled -> + SentryEvent().apply { + exceptions = + listOf( + SentryException().apply { mechanism = Mechanism().apply { isHandled = handled } } + ) } } + + events.forEach { event -> + assertThat(sut.captureEvent(event)).isEqualTo(event.eventId) + } + + verify(replayController, never()).captureReplay(anyOrNull()) + val envelopes = argumentCaptor() + verify(fixture.transport, times(2)).send(envelopes.capture(), anyOrNull()) + assertThat(envelopes.allValues.map { getEventFromData(it.items.first().data).eventId }) + .containsExactlyElementsIn(events.map { it.eventId }) + .inOrder() + verify(logger, times(2)) + .log( + SentryLevel.ERROR, + "The beforeErrorSampling callback threw an exception. Skipping replay capture.", + exception, + ) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Replay.category, 2)), ) + } + + @Test + fun `beforeErrorSampling throwing exception does not prevent replay capture for subsequent events`() { + val replayController = mock() + whenever(replayController.replayId).thenReturn(SentryId.EMPTY_ID) + whenever(replayController.captureReplay(anyOrNull())).thenReturn(SentryId.EMPTY_ID) + fixture.sentryOptions.setReplayController(replayController) + var invocations = 0 fixture.sentryOptions.sessionReplay.beforeErrorSampling = SentryReplayOptions.BeforeErrorSamplingCallback { _, _ -> - throw RuntimeException("test") + invocations++ + if (invocations == 1) { + throw RuntimeException("test") + } + true } val sut = fixture.getSut() sut.captureEvent(SentryEvent().apply { exceptions = listOf(SentryException()) }) - assertTrue(called) + verify(replayController, never()).captureReplay(anyOrNull()) + + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + assertThat(sut.captureEvent(event)).isEqualTo(event.eventId) + + assertThat(invocations).isEqualTo(2) + verify(replayController).captureReplay(false) + verify(fixture.transport, times(2)).send(any(), anyOrNull()) + assertClientReport(fixture.sentryOptions.clientReportRecorder, emptyList()) } @Test @@ -3881,10 +4161,10 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, - listOf(DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Replay.category, 1)), + listOf(DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Replay.category, 1)), ) - verify(onDiscardMock, times(1)).execute(DiscardReason.BEFORE_SEND, DataCategory.Replay, 1) + verify(onDiscardMock, times(1)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Replay, 1) } // endregion @@ -4061,10 +4341,12 @@ class SentryClientTest { assertClientReport( fixture.sentryOptions.clientReportRecorder, - listOf(DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Feedback.category, 1)), + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Feedback.category, 1) + ), ) - verify(onDiscardMock, times(1)).execute(DiscardReason.BEFORE_SEND, DataCategory.Feedback, 1) + verify(onDiscardMock, times(1)).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback, 1) } @Test @@ -4086,6 +4368,34 @@ class SentryClientTest { verify(onDiscardMock, times(1)).execute(DiscardReason.EVENT_PROCESSOR, DataCategory.Feedback, 1) } + @Test + fun `throwing feedback processor drops feedback and stops callbacks`() { + val throwingProcessor = mock() + val nextProcessor = mock() + val beforeSend = mock() + val onDiscard = mock() + whenever(throwingProcessor.process(any(), anyOrNull())) + .thenThrow(IllegalStateException("test")) + fixture.sentryOptions.addEventProcessor(throwingProcessor) + fixture.sentryOptions.addEventProcessor(nextProcessor) + fixture.sentryOptions.beforeSendFeedback = beforeSend + fixture.sentryOptions.onDiscard = onDiscard + + val id = fixture.getSut().captureFeedback(Feedback("message"), null, createScope()) + + assertThat(id).isEqualTo(SentryId.EMPTY_ID) + verify(nextProcessor, never()).process(any(), anyOrNull()) + verify(beforeSend, never()).execute(any(), anyOrNull()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.CALLBACK_ERROR.reason, DataCategory.Feedback.category, 1) + ), + ) + verify(onDiscard).execute(DiscardReason.CALLBACK_ERROR, DataCategory.Feedback, 1) + } + // endregion private fun givenScopeWithStartedSession( @@ -4352,14 +4662,6 @@ class SentryClientTest { override fun timestamp(): Long? = null } - private fun eventProcessorThrows(): EventProcessor { - return object : EventProcessor { - override fun process(event: SentryEvent, hint: Hint): SentryEvent? { - throw Throwable() - } - } - } - private class BackfillableHint : Backfillable { override fun shouldEnrich(): Boolean = false } 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 00000000000..24761837516 --- /dev/null +++ b/sentry/src/test/java/io/sentry/SentryClientTransactionProfileTest.kt @@ -0,0 +1,135 @@ +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) + } + + @Test + fun `intentional transaction drop 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 discardReason = + if (callback == "beforeSendTransaction") { + options.setBeforeSendTransaction { _, _ -> null } + DiscardReason.BEFORE_SEND + } else { + val processor = mock() + whenever(processor.process(any(), any())).thenReturn(null) + if (callback == "scope processor") { + scope.addEventProcessor(processor) + } else { + options.addEventProcessor(processor) + } + DiscardReason.EVENT_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.reason, DataCategory.Transaction.category, 1), + DiscardedEvent(discardReason.reason, DataCategory.Span.category, 2), + ) + if (hasProfile) { + expected.add(DiscardedEvent(discardReason.reason, DataCategory.Profile.category, 1)) + verify(onDiscard).execute(discardReason, DataCategory.Profile, 1) + } + assertClientReport(options.clientReportRecorder, expected) + verify(onDiscard).execute(discardReason, DataCategory.Transaction, 1) + verify(onDiscard).execute(discardReason, DataCategory.Span, 2) + verifyNoMoreInteractions(onDiscard) + } +} diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 4266061353c..9b8f0666854 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -1,5 +1,10 @@ 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 import kotlin.test.assertEquals @@ -175,7 +180,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 = @@ -189,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 = @@ -204,7 +209,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) @@ -220,7 +225,7 @@ class TracesSamplerTest { fixture.getSut( tracesSampleRate = 1.0, profilesSampleRate = 0.2, - profilesSamplerCallback = null, + profilesSamplerCallback = { null }, ) val samplingDecision = sampler.sample( @@ -353,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 @@ -381,14 +460,121 @@ 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 ignored`() { val sampler = - fixture.getSut(tracesSampleRate = 1.0, tracesSamplerCallback = { throw exception }) + 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.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 ignored`() { + 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.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 ignored`() { + 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).isFalse() + assertThat(decision.sampleRate).isNull() + 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 = - sampler.sample(SamplingContext(TransactionContext("name", "op"), null, 0.0, null)) - assertTrue(decision.sampled) - assertEquals(0.0, decision.sampleRand) + 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 diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index b89b1894f32..165f3c6c030 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -1,5 +1,6 @@ package io.sentry.clientreport +import com.google.common.truth.Truth.assertThat import io.sentry.Attachment import io.sentry.CheckIn import io.sentry.CheckInStatus @@ -64,6 +65,11 @@ class ClientReportTest { lateinit var clientReportRecorder: ClientReportRecorder lateinit var testHelper: ClientReportTestHelper + @Test + fun `callback error has expected discard reason`() { + assertThat(DiscardReason.CALLBACK_ERROR.reason).isEqualTo("callback_error") + } + @Test fun `lost envelope can be recorded`() { givenClientReportRecorder() diff --git a/sentry/src/test/java/io/sentry/internal/eventprocessor/SentryEventProcessorTest.kt b/sentry/src/test/java/io/sentry/internal/eventprocessor/SentryEventProcessorTest.kt new file mode 100644 index 00000000000..5c94108a6ae --- /dev/null +++ b/sentry/src/test/java/io/sentry/internal/eventprocessor/SentryEventProcessorTest.kt @@ -0,0 +1,21 @@ +package io.sentry.internal.eventprocessor + +import com.google.common.truth.Truth.assertThat +import io.sentry.EventProcessor +import io.sentry.SentryOptions +import kotlin.test.Test + +class SentryEventProcessorTest { + @Test + fun `default SDK event processors are marked as Sentry processors`() { + assertThat(SentryOptions().eventProcessors).isNotEmpty() + assertThat(SentryOptions().eventProcessors.all { it is SentryEventProcessor }).isTrue() + } + + @Test + fun `customer event processors are not marked as Sentry processors`() { + val processor = object : EventProcessor {} + + assertThat(processor).isNotInstanceOf(SentryEventProcessor::class.java) + } +}