diff --git a/CHANGELOG.md b/CHANGELOG.md index 46b2a0687b..ab0b586ef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Add manifest `AutoInit` to integrations list ([#2795](https://github.com/getsentry/sentry-java/pull/2795)) +- Tracing headers (`sentry-trace` and `baggage`) are now attached and passed through even if performance is disabled ([#2788](https://github.com/getsentry/sentry-java/pull/2788)) ### Dependencies diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index f2717b4324..decc43f5be 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -20,6 +20,7 @@ import io.sentry.ITransaction; import io.sentry.Instrumenter; import io.sentry.Integration; +import io.sentry.NoOpTransaction; import io.sentry.Scope; import io.sentry.SentryDate; import io.sentry.SentryLevel; @@ -31,6 +32,7 @@ import io.sentry.protocol.MeasurementValue; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.io.IOException; import java.lang.ref.WeakReference; @@ -122,11 +124,9 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio fullyDisplayedReporter = this.options.getFullyDisplayedReporter(); timeToFullDisplaySpanEnabled = this.options.isEnableTimeToFullDisplayTracing(); - if (this.options.isEnableActivityLifecycleBreadcrumbs() || performanceEnabled) { - application.registerActivityLifecycleCallbacks(this); - this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed."); - addIntegrationToSdkVersion(); - } + application.registerActivityLifecycleCallbacks(this); + this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed."); + addIntegrationToSdkVersion(); } private boolean isPerformanceEnabled(final @NotNull SentryAndroidOptions options) { @@ -176,104 +176,110 @@ private void stopPreviousTransactions() { private void startTracing(final @NotNull Activity activity) { WeakReference weakActivity = new WeakReference<>(activity); - if (performanceEnabled && !isRunningTransaction(activity) && hub != null) { - // as we allow a single transaction running on the bound Scope, we finish the previous ones - stopPreviousTransactions(); - - final String activityName = getActivityName(activity); - - final SentryDate appStartTime = - foregroundImportance ? AppStartState.getInstance().getAppStartTime() : null; - final Boolean coldStart = AppStartState.getInstance().isColdStart(); - - final TransactionOptions transactionOptions = new TransactionOptions(); - if (options.isEnableActivityLifecycleTracingAutoFinish()) { - transactionOptions.setIdleTimeout(options.getIdleTimeout()); - transactionOptions.setTrimEnd(true); - } - transactionOptions.setWaitForChildren(true); - transactionOptions.setTransactionFinishedCallback( - (finishingTransaction) -> { - @Nullable Activity unwrappedActivity = weakActivity.get(); - if (unwrappedActivity != null) { - activityFramesTracker.setMetrics( - unwrappedActivity, finishingTransaction.getEventId()); - } else { - if (options != null) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "Unable to track activity frames as the Activity %s has been destroyed.", - activityName); + if (hub != null && !isRunningTransactionOrTrace(activity)) { + if (!performanceEnabled) { + activitiesWithOngoingTransactions.put(activity, NoOpTransaction.getInstance()); + TracingUtils.startNewTrace(hub); + } else if (performanceEnabled) { + // as we allow a single transaction running on the bound Scope, we finish the previous ones + stopPreviousTransactions(); + + final String activityName = getActivityName(activity); + + final SentryDate appStartTime = + foregroundImportance ? AppStartState.getInstance().getAppStartTime() : null; + final Boolean coldStart = AppStartState.getInstance().isColdStart(); + + final TransactionOptions transactionOptions = new TransactionOptions(); + if (options.isEnableActivityLifecycleTracingAutoFinish()) { + transactionOptions.setIdleTimeout(options.getIdleTimeout()); + transactionOptions.setTrimEnd(true); + } + transactionOptions.setWaitForChildren(true); + transactionOptions.setTransactionFinishedCallback( + (finishingTransaction) -> { + @Nullable Activity unwrappedActivity = weakActivity.get(); + if (unwrappedActivity != null) { + activityFramesTracker.setMetrics( + unwrappedActivity, finishingTransaction.getEventId()); + } else { + if (options != null) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Unable to track activity frames as the Activity %s has been destroyed.", + activityName); + } } - } - }); - - // This will be the start timestamp of the transaction, as well as the ttid/ttfd spans - final @NotNull SentryDate ttidStartTime; + }); - if (!(firstActivityCreated || appStartTime == null || coldStart == null)) { - // The first activity ttid/ttfd spans should start at the app start time - ttidStartTime = appStartTime; - } else { - // The ttid/ttfd spans should start when the previous activity called its onPause method - ttidStartTime = lastPausedTime; - } - transactionOptions.setStartTimestamp(ttidStartTime); - - // we can only bind to the scope if there's no running transaction - ITransaction transaction = - hub.startTransaction( - new TransactionContext(activityName, TransactionNameSource.COMPONENT, UI_LOAD_OP), - transactionOptions); - - // in case appStartTime isn't available, we don't create a span for it. - if (!(firstActivityCreated || appStartTime == null || coldStart == null)) { - // start specific span for app start - appStartSpan = - transaction.startChild( - getAppStartOp(coldStart), - getAppStartDesc(coldStart), - appStartTime, - Instrumenter.SENTRY); - - // in case there's already an end time (e.g. due to deferred SDK init) - // we can finish the app-start span - finishAppStartSpan(); - } - final @NotNull ISpan ttidSpan = - transaction.startChild( - TTID_OP, getTtidDesc(activityName), ttidStartTime, Instrumenter.SENTRY); - ttidSpanMap.put(activity, ttidSpan); + // This will be the start timestamp of the transaction, as well as the ttid/ttfd spans + final @NotNull SentryDate ttidStartTime; - if (timeToFullDisplaySpanEnabled && fullyDisplayedReporter != null && options != null) { - final @NotNull ISpan ttfdSpan = + if (!(firstActivityCreated || appStartTime == null || coldStart == null)) { + // The first activity ttid/ttfd spans should start at the app start time + ttidStartTime = appStartTime; + } else { + // The ttid/ttfd spans should start when the previous activity called its onPause method + ttidStartTime = lastPausedTime; + } + transactionOptions.setStartTimestamp(ttidStartTime); + + // we can only bind to the scope if there's no running transaction + ITransaction transaction = + hub.startTransaction( + new TransactionContext(activityName, TransactionNameSource.COMPONENT, UI_LOAD_OP), + transactionOptions); + + // in case appStartTime isn't available, we don't create a span for it. + if (!(firstActivityCreated || appStartTime == null || coldStart == null)) { + // start specific span for app start + appStartSpan = + transaction.startChild( + getAppStartOp(coldStart), + getAppStartDesc(coldStart), + appStartTime, + Instrumenter.SENTRY); + + // in case there's already an end time (e.g. due to deferred SDK init) + // we can finish the app-start span + finishAppStartSpan(); + } + final @NotNull ISpan ttidSpan = transaction.startChild( - TTFD_OP, getTtfdDesc(activityName), ttidStartTime, Instrumenter.SENTRY); - try { - ttfdSpanMap.put(activity, ttfdSpan); - ttfdAutoCloseFuture = - options - .getExecutorService() - .schedule(() -> finishExceededTtfdSpan(ttfdSpan, ttidSpan), TTFD_TIMEOUT_MILLIS); - } catch (RejectedExecutionException e) { - options - .getLogger() - .log( - SentryLevel.ERROR, - "Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?", - e); + TTID_OP, getTtidDesc(activityName), ttidStartTime, Instrumenter.SENTRY); + ttidSpanMap.put(activity, ttidSpan); + + if (timeToFullDisplaySpanEnabled && fullyDisplayedReporter != null && options != null) { + final @NotNull ISpan ttfdSpan = + transaction.startChild( + TTFD_OP, getTtfdDesc(activityName), ttidStartTime, Instrumenter.SENTRY); + try { + ttfdSpanMap.put(activity, ttfdSpan); + ttfdAutoCloseFuture = + options + .getExecutorService() + .schedule( + () -> finishExceededTtfdSpan(ttfdSpan, ttidSpan), TTFD_TIMEOUT_MILLIS); + } catch (RejectedExecutionException e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?", + e); + } } - } - // lets bind to the scope so other integrations can pick it up - hub.configureScope( - scope -> { - applyScope(scope, transaction); - }); + // lets bind to the scope so other integrations can pick it up + hub.configureScope( + scope -> { + applyScope(scope, transaction); + }); - activitiesWithOngoingTransactions.put(activity, transaction); + activitiesWithOngoingTransactions.put(activity, transaction); + } } } @@ -306,7 +312,7 @@ void clearScope(final @NotNull Scope scope, final @NotNull ITransaction transact }); } - private boolean isRunningTransaction(final @NotNull Activity activity) { + private boolean isRunningTransactionOrTrace(final @NotNull Activity activity) { return activitiesWithOngoingTransactions.containsKey(activity); } @@ -367,42 +373,45 @@ public synchronized void onActivityCreated( @Override public synchronized void onActivityStarted(final @NotNull Activity activity) { - // The docs on the screen rendering performance tracing - // (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition), - // state that the tracing starts for every Activity class when the app calls .onActivityStarted. - // Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not - // working. Moving this to onActivityStarted fixes the problem. - activityFramesTracker.addActivity(activity); - + if (performanceEnabled) { + // The docs on the screen rendering performance tracing + // (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition), + // state that the tracing starts for every Activity class when the app calls + // .onActivityStarted. + // Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not + // working. Moving this to onActivityStarted fixes the problem. + activityFramesTracker.addActivity(activity); + } addBreadcrumb(activity, "started"); } @SuppressLint("NewApi") @Override public synchronized void onActivityResumed(final @NotNull Activity activity) { - - // app start span - @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime(); - @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); - // in case the SentryPerformanceProvider is disabled it does not set the app start times, - // and we need to set the end time manually here, - // the start time gets set manually in SentryAndroid.init() - if (appStartStartTime != null && appStartEndTime == null) { - AppStartState.getInstance().setAppStartEnd(); - } - finishAppStartSpan(); - - final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity); - final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity); - final View rootView = activity.findViewById(android.R.id.content); - if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN - && rootView != null) { - FirstDrawDoneListener.registerForNextDraw( - rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), buildInfoProvider); - } else { - // Posting a task to the main thread's handler will make it executed after it finished - // its current job. That is, right after the activity draws the layout. - mainHandler.post(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan)); + if (performanceEnabled) { + // app start span + @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime(); + @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); + // in case the SentryPerformanceProvider is disabled it does not set the app start times, + // and we need to set the end time manually here, + // the start time gets set manually in SentryAndroid.init() + if (appStartStartTime != null && appStartEndTime == null) { + AppStartState.getInstance().setAppStartEnd(); + } + finishAppStartSpan(); + + final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity); + final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity); + final View rootView = activity.findViewById(android.R.id.content); + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN + && rootView != null) { + FirstDrawDoneListener.registerForNextDraw( + rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), buildInfoProvider); + } else { + // Posting a task to the main thread's handler will make it executed after it finished + // its current job. That is, right after the activity draws the layout. + mainHandler.post(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan)); + } } addBreadcrumb(activity, "resumed"); } @@ -450,36 +459,37 @@ public synchronized void onActivitySaveInstanceState( @Override public synchronized void onActivityDestroyed(final @NotNull Activity activity) { - addBreadcrumb(activity, "destroyed"); + if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) { + addBreadcrumb(activity, "destroyed"); - // in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid - // memory leak - finishSpan(appStartSpan, SpanStatus.CANCELLED); + // in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid + // memory leak + finishSpan(appStartSpan, SpanStatus.CANCELLED); - // we finish the ttidSpan as cancelled in case it isn't completed yet - final ISpan ttidSpan = ttidSpanMap.get(activity); - final ISpan ttfdSpan = ttfdSpanMap.get(activity); - finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED); + // we finish the ttidSpan as cancelled in case it isn't completed yet + final ISpan ttidSpan = ttidSpanMap.get(activity); + final ISpan ttfdSpan = ttfdSpanMap.get(activity); + finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED); - // we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet - finishExceededTtfdSpan(ttfdSpan, ttidSpan); - cancelTtfdAutoClose(); + // we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet + finishExceededTtfdSpan(ttfdSpan, ttidSpan); + cancelTtfdAutoClose(); - // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, - // we make sure to finish it when the activity gets destroyed. - stopTracing(activity, true); + // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, + // we make sure to finish it when the activity gets destroyed. + stopTracing(activity, true); - // set it to null in case its been just finished as cancelled - appStartSpan = null; - ttidSpanMap.remove(activity); - ttfdSpanMap.remove(activity); + // set it to null in case its been just finished as cancelled + appStartSpan = null; + ttidSpanMap.remove(activity); + ttfdSpanMap.remove(activity); + } - // clear it up, so we don't start again for the same activity if the activity is in the activity + // clear it up, so we don't start again for the same activity if the activity is in the + // activity // stack still. // if the activity is opened again and not in memory, transactions will be created normally. - if (performanceEnabled) { - activitiesWithOngoingTransactions.remove(activity); - } + activitiesWithOngoingTransactions.remove(activity); } private void finishSpan(final @Nullable ISpan span) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java index 2bc8a52694..3dda6183cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java @@ -20,6 +20,7 @@ import io.sentry.android.core.SentryAndroidOptions; import io.sentry.internal.gestures.UiElement; import io.sentry.protocol.TransactionNameSource; +import io.sentry.util.TracingUtils; import java.lang.ref.WeakReference; import java.util.Collections; import java.util.Map; @@ -185,7 +186,13 @@ private void addBreadcrumb( } private void startTracing(final @NotNull UiElement target, final @NotNull String eventType) { + final UiElement uiElement = activeUiElement; if (!(options.isTracingEnabled() && options.isEnableUserInteractionTracing())) { + if (!(target.equals(uiElement) && eventType.equals(activeEventType))) { + TracingUtils.startNewTrace(hub); + activeUiElement = target; + activeEventType = eventType; + } return; } @@ -196,7 +203,6 @@ private void startTracing(final @NotNull UiElement target, final @NotNull String } final @Nullable String viewIdentifier = target.getIdentifier(); - final UiElement uiElement = activeUiElement; if (activeTransaction != null) { if (target.equals(uiElement) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index ce66f1dcc7..284117f855 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -16,6 +16,7 @@ import io.sentry.FullyDisplayedReporter import io.sentry.Hub import io.sentry.ISentryExecutorService import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryDate import io.sentry.SentryLevel @@ -32,10 +33,12 @@ import io.sentry.protocol.MeasurementValue import io.sentry.protocol.TransactionNameSource import io.sentry.test.getProperty import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check +import org.mockito.kotlin.clearInvocations import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -54,6 +57,7 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull +import kotlin.test.assertNotSame import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -141,13 +145,13 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb and tracing are disabled, it doesn't register callback`() { + fun `When activity lifecycle breadcrumb and tracing are disabled, it still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test @@ -162,7 +166,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it does not register callback`() { + fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false fixture.options.tracesSampleRate = 1.0 @@ -170,7 +174,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test @@ -196,7 +200,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, it doesn't register callback`() { + fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, its still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false fixture.options.tracesSampleRate = 1.0 @@ -205,7 +209,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test @@ -1384,6 +1388,60 @@ class ActivityLifecycleIntegrationTest { assertFalse(ttfdSpan2.isFinished) } + @Test + fun `starts new trace if performance is disabled`() { + val sut = fixture.getSut() + val activity = mock() + fixture.options.enableTracing = false + + val argumentCaptor: ArgumentCaptor = ArgumentCaptor.forClass(ScopeCallback::class.java) + val scope = Scope(fixture.options) + val propagationContextAtStart = scope.propagationContext + whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer { + argumentCaptor.value.run(scope) + } + + sut.register(fixture.hub, fixture.options) + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).configureScope(any()) + assertNotSame(propagationContextAtStart, scope.propagationContext) + } + + @Test + fun `does not start another new trace if one has already been started but does after activity was destroyed`() { + val sut = fixture.getSut() + val activity = mock() + fixture.options.enableTracing = false + + val argumentCaptor: ArgumentCaptor = ArgumentCaptor.forClass(ScopeCallback::class.java) + val scope = Scope(fixture.options) + val propagationContextAtStart = scope.propagationContext + whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer { + argumentCaptor.value.run(scope) + } + + sut.register(fixture.hub, fixture.options) + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).configureScope(any()) + val propagationContextAfterNewTrace = scope.propagationContext + assertNotSame(propagationContextAtStart, propagationContextAfterNewTrace) + + clearInvocations(fixture.hub) + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub, never()).configureScope(any()) + assertSame(propagationContextAfterNewTrace, scope.propagationContext) + + sut.onActivityDestroyed(activity) + + clearInvocations(fixture.hub) + sut.onActivityCreated(activity, fixture.bundle) + verify(fixture.hub).configureScope(any()) + assertNotSame(propagationContextAfterNewTrace, scope.propagationContext) + } + private fun runFirstDraw(view: View) { // Removes OnDrawListener in the next OnGlobalLayout after onDraw view.viewTreeObserver.dispatchOnDraw() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt index b61f3805f0..2fdbc9dc07 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt @@ -11,11 +11,16 @@ import android.widget.CheckBox import android.widget.RadioButton import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.PropagationContext +import io.sentry.Scope +import io.sentry.Scope.IWithPropagationContext +import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -36,6 +41,8 @@ class SentryGestureListenerClickTest { dsn = "https://key@sentry.io/proj" } val hub = mock() + val scope = mock() + val propagationContext = PropagationContext() lateinit var target: View lateinit var invalidTarget: View @@ -79,6 +86,8 @@ class SentryGestureListenerClickTest { whenever(context.resources).thenReturn(resources) whenever(this.target.context).thenReturn(context) whenever(activity.window).thenReturn(window) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(propagationContext); propagationContext; }.whenever(scope).withPropagationContext(any()) return SentryGestureListener( activity, hub, @@ -228,4 +237,20 @@ class SentryGestureListenerClickTest { anyOrNull() ) } + + @Test + fun `creates new trace on click`() { + class LocalView(context: Context) : View(context) + + val event = mock() + val sut = fixture.getSut(event, attachViewsToRoot = false) + fixture.window.mockDecorView(event = event, touchWithinBounds = false) { + whenever(it.childCount).thenReturn(1) + whenever(it.getChildAt(0)).thenReturn(fixture.target) + } + + sut.onSingleTapUp(event) + + verify(fixture.scope).propagationContext = any() + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt index e00d22c73e..dd78e75d6c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt @@ -12,14 +12,20 @@ import android.widget.ListAdapter import androidx.core.view.ScrollingView import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.PropagationContext +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -39,6 +45,8 @@ class SentryGestureListenerScrollTest { gestureTargetLocators = listOf(AndroidViewGestureTargetLocator(true)) } val hub = mock() + val scope = mock() + val propagationContext = PropagationContext() val firstEvent = mock() val eventsInBetween = listOf(mock(), mock(), mock()) @@ -69,6 +77,8 @@ class SentryGestureListenerScrollTest { endEvent.mockDirection(firstEvent, direction) } whenever(activity.window).thenReturn(window) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + doAnswer { (it.arguments[0] as Scope.IWithPropagationContext).accept(propagationContext); propagationContext }.whenever(scope).withPropagationContext(any()) return SentryGestureListener( activity, hub, @@ -145,6 +155,7 @@ class SentryGestureListenerScrollTest { }, anyOrNull() ) + verify(fixture.hub).configureScope(anyOrNull()) verify(fixture.hub).addBreadcrumb( check { assertEquals("ui.swipe", it.category) @@ -182,6 +193,50 @@ class SentryGestureListenerScrollTest { verify(fixture.hub, never()).addBreadcrumb(any()) } + @Test + fun `starts a new trace on scroll`() { + val sut = fixture.getSut(direction = "left") + + sut.onDown(fixture.firstEvent) + fixture.eventsInBetween.forEach { + sut.onScroll(fixture.firstEvent, it, 10.0f, 0f) + } + sut.onUp(fixture.endEvent) + + verify(fixture.scope).propagationContext = any() + } + + @Test + fun `does not start a new trace on repeated scroll but does for a different event`() { + val sut = fixture.getSut(direction = "left") + + sut.onDown(fixture.firstEvent) + fixture.eventsInBetween.forEach { + sut.onScroll(fixture.firstEvent, it, 10.0f, 0f) + } + sut.onUp(fixture.endEvent) + + verify(fixture.scope).propagationContext = any() + + clearInvocations(fixture.scope) + + sut.onDown(fixture.firstEvent) + fixture.eventsInBetween.forEach { + sut.onScroll(fixture.firstEvent, it, 10.0f, 0f) + } + sut.onUp(fixture.endEvent) + verify(fixture.scope, never()).propagationContext = any() + + clearInvocations(fixture.scope) + + sut.onDown(fixture.firstEvent) + fixture.eventsInBetween.forEach { sut.onScroll(fixture.firstEvent, it, 0f, 30.0f) } + sut.onFling(fixture.firstEvent, fixture.endEvent, 1.0f, 1.0f) + sut.onUp(fixture.endEvent) + + verify(fixture.scope).propagationContext = any() + } + internal class ScrollableView : View(mock()), ScrollingView { override fun computeVerticalScrollOffset(): Int = 0 override fun computeVerticalScrollExtent(): Int = 0 diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt index bd31d98fda..5f216a660a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt @@ -11,6 +11,7 @@ import android.widget.AbsListView import android.widget.ListAdapter import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext @@ -20,6 +21,7 @@ import io.sentry.protocol.TransactionNameSource import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -40,6 +42,7 @@ class SentryGestureListenerTracingTest { } val hub = mock() val event = mock() + val scope = mock() lateinit var target: View lateinit var transaction: SentryTracer @@ -79,6 +82,7 @@ class SentryGestureListenerTracingTest { whenever(hub.startTransaction(any(), any())) .thenReturn(this.transaction) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) return SentryGestureListener( activity, diff --git a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt index 9661ee1925..625b612f0b 100644 --- a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt +++ b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt @@ -19,6 +19,7 @@ import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint import io.sentry.protocol.TransactionNameSource +import io.sentry.util.TracingUtils import java.lang.ref.WeakReference /** @@ -96,6 +97,7 @@ class SentryNavigationListener @JvmOverloads constructor( arguments: Map ) { if (!isPerformanceEnabled) { + TracingUtils.startNewTrace(hub) return } diff --git a/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt b/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt index e1e2be4830..d7ddfed02f 100644 --- a/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt +++ b/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt @@ -18,6 +18,7 @@ import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.TransactionNameSource import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check @@ -29,6 +30,7 @@ import org.mockito.kotlin.whenever import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotSame import kotlin.test.assertNull @RunWith(AndroidJUnit4::class) @@ -43,6 +45,7 @@ class SentryNavigationListenerTest { val context = mock() val resources = mock() val scope = mock() + lateinit var options: SentryOptions lateinit var transaction: SentryTracer @@ -56,14 +59,13 @@ class SentryNavigationListenerTest { hasViewIdInRes: Boolean = true, transaction: SentryTracer? = null ): SentryNavigationListener { - whenever(hub.options).thenReturn( - SentryOptions().apply { - dsn = "http://key@localhost/proj" - setTracesSampleRate( - tracesSampleRate - ) - } - ) + options = SentryOptions().apply { + dsn = "http://key@localhost/proj" + setTracesSampleRate( + tracesSampleRate + ) + } + whenever(hub.options).thenReturn(options) this.transaction = transaction ?: SentryTracer( TransactionContext( @@ -347,4 +349,21 @@ class SentryNavigationListenerTest { captor.thirdValue.accept(fixture.transaction) verify(fixture.scope).clearTransaction() } + + @Test + fun `starts new trace if performance is disabled`() { + val sut = fixture.getSut(enableTracing = false) + + val argumentCaptor: ArgumentCaptor = ArgumentCaptor.forClass(ScopeCallback::class.java) + val scope = Scope(fixture.options) + val propagationContextAtStart = scope.propagationContext + whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer { + argumentCaptor.value.run(scope) + } + + sut.onDestinationChanged(fixture.navController, fixture.destination, null) + + verify(fixture.hub).configureScope(any()) + assertNotSame(propagationContextAtStart, scope.propagationContext) + } } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 7c19c1c3b0..06cc756ecf 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -20,6 +20,7 @@ import io.sentry.exception.SentryHttpClientException import io.sentry.protocol.Mechanism import io.sentry.util.HttpUtils import io.sentry.util.PropagationTargetsUtils +import io.sentry.util.TracingUtils import io.sentry.util.UrlUtils import okhttp3.Headers import okhttp3.Interceptor @@ -87,18 +88,20 @@ class SentryOkHttpInterceptor( var code: Int? = null try { val requestBuilder = request.newBuilder() - if (span != null && !span.isNoOp && - PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url.toString()) - ) { - span.toSentryTrace().let { - requestBuilder.addHeader(it.name, it.value) - } - span.toBaggageHeader(request.headers(BaggageHeader.BAGGAGE_HEADER))?.let { + TracingUtils.traceIfAllowed( + hub, + request.url.toString(), + request.headers(BaggageHeader.BAGGAGE_HEADER), + span + )?.let { tracingHeaders -> + requestBuilder.addHeader(tracingHeaders.sentryTraceHeader.name, tracingHeaders.sentryTraceHeader.value) + tracingHeaders.baggageHeader?.let { requestBuilder.removeHeader(BaggageHeader.BAGGAGE_HEADER) requestBuilder.addHeader(it.name, it.value) } } + request = requestBuilder.build() response = chain.proceed(request) code = response.code diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index c686d4b9a2..b96b1dcfae 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -7,6 +7,8 @@ import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.HttpStatusCodeRange import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -27,6 +29,7 @@ import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -47,6 +50,7 @@ class SentryOkHttpInterceptorTest { val server = MockWebServer() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions + lateinit var scope: Scope @SuppressWarnings("LongParameterList") fun getSut( @@ -76,7 +80,9 @@ class SentryOkHttpInterceptorTest { } isSendDefaultPii = sendDefaultPii } + scope = Scope(options) whenever(hub.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) @@ -180,10 +186,20 @@ class SentryOkHttpInterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `when there is no active span, adds sentry trace header to the request from scope`() { val sut = fixture.getSut(isSpanActive = false) sut.newCall(getRequest()).execute() val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when there is no active span and host if not allowed, does not add sentry trace header to the request`() { + val sut = fixture.getSut(isSpanActive = false) + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + sut.newCall(getRequest()).execute() + val recorderRequest = fixture.server.takeRequest() assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } 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 2a9b17e92e..eac9f8b3c6 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 @@ -8,7 +8,7 @@ import com.apollographql.apollo3.api.http.HttpResponse import com.apollographql.apollo3.exception.ApolloHttpException import com.apollographql.apollo3.network.http.HttpInterceptor import com.apollographql.apollo3.network.http.HttpInterceptorChain -import io.sentry.BaggageHeader.BAGGAGE_HEADER +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.HubAdapter @@ -29,6 +29,7 @@ import io.sentry.protocol.Request import io.sentry.protocol.Response import io.sentry.util.HttpUtils import io.sentry.util.PropagationTargetsUtils +import io.sentry.util.TracingUtils import io.sentry.util.UrlUtils import io.sentry.vendor.Base64 import okio.Buffer @@ -65,43 +66,13 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor( val operationType = decodeHeaderValue(request, SENTRY_APOLLO_3_OPERATION_TYPE) val operationId = getHeader(HEADER_APOLLO_OPERATION_ID, request.headers) - var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() var span: ISpan? = null if (activeSpan != null) { span = startChild(request, activeSpan, operationName, operationType, operationId) - - if (!span.isNoOp && PropagationTargetsUtils.contain( - hub.options.tracePropagationTargets, - request.url - ) - ) { - val sentryTraceHeader = span.toSentryTrace() - val baggageHeader = span.toBaggageHeader( - cleanedHeaders.filter { - it.name.equals( - BAGGAGE_HEADER, - true - ) - }.map { it.value } - ) - cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value)) - - baggageHeader?.let { newHeader -> - cleanedHeaders = - cleanedHeaders.filterNot { it.name.equals(BAGGAGE_HEADER, true) } - .toMutableList().apply { - add(HttpHeader(newHeader.name, newHeader.value)) - } - } - } - } - - val requestBuilder = request.newBuilder().apply { - headers(cleanedHeaders) } - val modifiedRequest = requestBuilder.build() + val modifiedRequest = maybeAddTracingHeaders(hub, request, span) var httpResponse: HttpResponse? = null var statusCode: Int? = null @@ -141,6 +112,25 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor( } } + private fun maybeAddTracingHeaders(hub: IHub, request: HttpRequest, span: ISpan?): HttpRequest { + var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() + + TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span)?.let { + cleanedHeaders.add(HttpHeader(it.sentryTraceHeader.name, it.sentryTraceHeader.value)) + it.baggageHeader?.let { baggageHeader -> + cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply { + add(HttpHeader(baggageHeader.name, baggageHeader.value)) + } + } + } + + val requestBuilder = request.newBuilder().apply { + headers(cleanedHeaders) + } + + return requestBuilder.build() + } + override fun getIntegrationName(): String { return super.getIntegrationName().replace("Http", "") } 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 64563c4578..d197eeae4a 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 @@ -11,6 +11,8 @@ import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS import io.sentry.SentryTraceHeader @@ -28,8 +30,10 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -43,14 +47,16 @@ class SentryApollo3InterceptorTest { class Fixture { val server = MockWebServer() - val hub = mock().apply { - whenever(options).thenReturn( - SentryOptions().apply { - dsn = "https://key@sentry.io/proj" - setTracePropagationTargets(listOf(DEFAULT_PROPAGATION_TARGETS)) - sdkVersion = SdkVersion("test", "1.2.3") - } - ) + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + setTracePropagationTargets(listOf(DEFAULT_PROPAGATION_TARGETS)) + sdkVersion = SdkVersion("test", "1.2.3") + } + val scope = Scope(options) + val hub = mock().also { + whenever(it.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } private var httpInterceptor = SentryApollo3HttpInterceptor(hub) @@ -173,7 +179,8 @@ class SentryApollo3InterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `does not add sentry trace header to the request if host is disallowed`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) executeQuery(isSpanActive = false) val recorderRequest = fixture.server.takeRequest() @@ -181,6 +188,15 @@ class SentryApollo3InterceptorTest { assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } + @Test + fun `when there is no active span, does not add sentry trace header to the request`() { + executeQuery(isSpanActive = false) + + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + @Test fun `when there is an active span, adds sentry trace headers to the request`() { executeQuery() 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 1f54fe32ca..ec62744f35 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -11,6 +11,7 @@ import com.apollographql.apollo.interceptor.ApolloInterceptor.FetchSourceType import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorRequest import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorResponse import com.apollographql.apollo.interceptor.ApolloInterceptorChain +import com.apollographql.apollo.request.RequestHeaders import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.Hint @@ -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.util.TracingUtils import java.util.concurrent.Executor class SentryApolloInterceptor( @@ -42,25 +44,14 @@ class SentryApolloInterceptor( override fun interceptAsync(request: InterceptorRequest, chain: ApolloInterceptorChain, dispatcher: Executor, callBack: CallBack) { val activeSpan = hub.span if (activeSpan == null) { - chain.proceedAsync(request, dispatcher, callBack) + val headers = addTracingHeaders(request, null) + val modifiedRequest = request.toBuilder().requestHeaders(headers).build() + chain.proceedAsync(modifiedRequest, dispatcher, callBack) } else { val span = startChild(request, activeSpan) - val requestWithHeader = if (span.isNoOp) { - request - } else { - val sentryTraceHeader = span.toSentryTrace() - - // we have no access to URI, no way to verify tracing origins - val requestHeaderBuilder = request.requestHeaders.toBuilder() - requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value) - span.toBaggageHeader(listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER))) - ?.let { - requestHeaderBuilder.addHeader(it.name, it.value) - } - val headers = requestHeaderBuilder.build() - request.toBuilder().requestHeaders(headers).build() - } + val headers = addTracingHeaders(request, span) + val requestWithHeader = request.toBuilder().requestHeaders(headers).build() span.setData("operationId", requestWithHeader.operation.operationId()) span.setData("variables", requestWithHeader.operation.variables().valueMap().toString()) @@ -111,6 +102,29 @@ class SentryApolloInterceptor( override fun dispose() {} + private fun addTracingHeaders(request: InterceptorRequest, span: ISpan?): RequestHeaders { + val requestHeaderBuilder = request.requestHeaders.toBuilder() + + if (hub.options.isTraceSampling) { + // we have no access to URI, no way to verify tracing origins + TracingUtils.trace( + hub, + listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)), + span + )?.let { tracingHeaders -> + requestHeaderBuilder.addHeader( + tracingHeaders.sentryTraceHeader.name, + tracingHeaders.sentryTraceHeader.value + ) + tracingHeaders.baggageHeader?.let { + requestHeaderBuilder.addHeader(it.name, it.value) + } + } + } + + return requestHeaderBuilder.build() + } + private fun startChild(request: InterceptorRequest, activeSpan: ISpan): ISpan { val operation = request.operation.name().name() val operationType = when (request.operation) { 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 107f849d47..f2bb4a7403 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -3,9 +3,12 @@ package io.sentry.apollo import com.apollographql.apollo.ApolloClient import com.apollographql.apollo.coroutines.await import com.apollographql.apollo.exception.ApolloException +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -21,8 +24,10 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -35,12 +40,15 @@ class SentryApolloInterceptorTest { class Fixture { val server = MockWebServer() - val hub = mock().apply { - whenever(options).thenReturn( - SentryOptions().apply { - dsn = "https://key@sentry.io/proj" - sdkVersion = SdkVersion("test", "1.2.3") - } + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + sdkVersion = SdkVersion("test", "1.2.3") + } + val scope = Scope(options) + val hub = mock().also { + whenever(it.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope( + any() ) } private var interceptor = SentryApolloInterceptor(hub) @@ -132,11 +140,12 @@ class SentryApolloInterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `when there is no active span, adds sentry trace header to the request from scope`() { executeQuery(isSpanActive = false) val recorderRequest = fixture.server.takeRequest() - assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -144,6 +153,7 @@ class SentryApolloInterceptorTest { executeQuery() val recorderRequest = fixture.server.takeRequest() assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test 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 8c0f2bfbf6..54fdd7601b 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -11,11 +11,10 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanDataConvention; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import java.util.ArrayList; @@ -47,8 +46,10 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O Response response = null; try { final ISpan activeSpan = hub.getSpan(); + if (activeSpan == null) { - return delegate.execute(request, options); + final @NotNull Request modifiedRequest = maybeAddTracingHeaders(request, null); + return delegate.execute(modifiedRequest, options); } ISpan span = activeSpan.startChild("http.client"); @@ -56,26 +57,10 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O span.setDescription(request.httpMethod().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - final RequestWrapper requestWrapper = new RequestWrapper(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - final @Nullable Collection requestBaggageHeader = - request.headers().get(BaggageHeader.BAGGAGE_HEADER); - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader( - requestBaggageHeader != null ? new ArrayList<>(requestBaggageHeader) : null); - requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - if (baggageHeader != null) { - requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); - requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); - } - } + final @NotNull Request modifiedRequest = maybeAddTracingHeaders(request, span); try { - response = delegate.execute(requestWrapper.build(), options); + response = delegate.execute(modifiedRequest, options); // handles both success and error responses span.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.status()); span.setStatus(SpanStatus.fromHttpStatusCode(response.status())); @@ -104,6 +89,34 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O } } + private @NotNull Request maybeAddTracingHeaders( + final @NotNull Request request, final @Nullable ISpan span) { + final @NotNull RequestWrapper requestWrapper = new RequestWrapper(request); + final @Nullable Collection requestBaggageHeaders = + request.headers().get(BaggageHeader.BAGGAGE_HEADER); + + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url(), + (requestBaggageHeaders != null ? new ArrayList<>(requestBaggageHeaders) : null), + span); + + if (tracingHeaders != null) { + requestWrapper.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); + requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); + } + } + + return requestWrapper.build(); + } + private void addBreadcrumb(final @NotNull Request request, final @Nullable Response response) { final Breadcrumb breadcrumb = Breadcrumb.http( 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 e2b9b2c57b..759486d400 100644 --- a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt +++ b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt @@ -8,6 +8,8 @@ import feign.RequestLine import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -19,6 +21,7 @@ import okhttp3.mockwebserver.MockWebServer import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -40,9 +43,11 @@ class SentryFeignClientTest { val sentryOptions = SentryOptions().apply { dsn = "http://key@localhost/proj" } + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) } @@ -114,8 +119,18 @@ class SentryFeignClientTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { - fixture.sentryOptions.isTraceSampling = true + fun `when there is no active span, adds sentry trace header to the request from scope`() { + fixture.sentryOptions.dsn = "https://key@sentry.io/proj" + val sut = fixture.getSut(isSpanActive = false) + sut.getOk() + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when there is no active span, does not add sentry trace header to the request if host is disallowed`() { + fixture.sentryOptions.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) fixture.sentryOptions.dsn = "https://key@sentry.io/proj" val sut = fixture.getSut(isSpanActive = false) sut.getOk() diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java index 16f8e99976..2b10b5c36e 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java @@ -18,6 +18,7 @@ import io.sentry.ISpan; import io.sentry.ITransaction; import io.sentry.Instrumenter; +import io.sentry.PropagationContext; import io.sentry.SentryDate; import io.sentry.SentryLevel; import io.sentry.SentryLongDate; @@ -105,22 +106,14 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri final @NotNull TransactionContext transactionContext = traceData.getSentryTraceHeader() == null ? new TransactionContext( - transactionName, - op, - new SentryId(traceData.getTraceId()), - spanId, - transactionNameSource, - null, - null, - null) - : TransactionContext.fromSentryTrace( - transactionName, - transactionNameSource, - op, - traceData.getSentryTraceHeader(), - traceData.getBaggage(), - spanId); + new SentryId(traceData.getTraceId()), spanId, null, null, null) + : TransactionContext.fromPropagationContext( + PropagationContext.fromHeaders( + traceData.getSentryTraceHeader(), traceData.getBaggage(), spanId)); ; + transactionContext.setName(transactionName); + transactionContext.setTransactionNameSource(transactionNameSource); + transactionContext.setOperation(op); transactionContext.setInstrumenter(Instrumenter.OTEL); TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java index 2a0def3722..5c8c96be3b 100644 --- a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java @@ -240,7 +240,6 @@ static class SentrySecurityConfiguration { } @Bean - @Conditional(SentryTracingCondition.class) @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IHub hub, final @NotNull TransactionNameProvider transactionNameProvider) { diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt index 0d7e363732..b686681b59 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt @@ -458,18 +458,10 @@ class SentryAutoConfigurationTest { } @Test - fun `when tracing is set, does not create tracing filter`() { + fun `creates tracing filter`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") - } - } - - @Test - fun `when tracing is disabled, does not create tracing filter`() { - contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") + assertThat(it).hasBean("sentryTracingFilter") } } diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt index 4afaa9983b..2c69175fa5 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.boot.jakarta import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -13,8 +15,10 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -38,9 +42,11 @@ class SentrySpanRestTemplateCustomizerTest { val transaction: SentryTracer internal val customizer = SentrySpanRestTemplateCustomizer(hub) val url = mockServer.url("/test/123").toString() + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) } @@ -164,6 +170,28 @@ class SentrySpanRestTemplateCustomizerTest { assertThat(fixture.transaction.spans).isEmpty() } + @Test + fun `when transaction is not active, adds tracing headers from scope`() { + val sut = fixture.getSut(isTransactionActive = false) + val headers = HttpHeaders() + headers.add("baggage", "thirdPartyBaggage=someValue") + headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + + val requestEntity = HttpEntity(headers) + + sut.exchange(fixture.url, HttpMethod.GET, requestEntity, String::class.java) + + val recorderRequest = fixture.mockServer.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + + val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER) + assertEquals(baggageHeaderValues.size, 1) + assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")) + assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key")) + assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) + } + @Test fun `avoids duplicate registration`() { val restTemplate = fixture.getSut(isTransactionActive = true) diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt index 2f298992d0..2aaea06efc 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt @@ -1,8 +1,10 @@ package io.sentry.spring.boot.jakarta +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -16,8 +18,10 @@ import okhttp3.mockwebserver.RecordedRequest import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -33,6 +37,7 @@ import kotlin.test.assertNull class SentrySpanWebClientCustomizerTest { class Fixture { lateinit var sentryOptions: SentryOptions + lateinit var scope: Scope val hub = mock() var mockServer = MockWebServer() lateinit var transaction: SentryTracer @@ -47,7 +52,9 @@ class SentrySpanWebClientCustomizerTest { } dsn = "http://key@localhost/proj" } + scope = Scope(sentryOptions) whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) val webClientBuilder = WebClient.builder() customizer.customize(webClientBuilder) @@ -124,6 +131,33 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active and server is not listed in tracing origins, does not add sentry trace header to the request`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = false) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active, adds sentry trace header to the request from scope`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = true) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -136,6 +170,7 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index c4183c002a..f7bc510875 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -240,7 +240,6 @@ static class SentrySecurityConfiguration { } @Bean - @Conditional(SentryTracingCondition.class) @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IHub hub, final @NotNull TransactionNameProvider transactionNameProvider) { diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index b9e858b8e2..c87bcf674f 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -458,18 +458,10 @@ class SentryAutoConfigurationTest { } @Test - fun `when tracing is set, does not create tracing filter`() { + fun `creates tracing filter`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") - } - } - - @Test - fun `when tracing is disabled, does not create tracing filter`() { - contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") + assertThat(it).hasBean("sentryTracingFilter") } } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt index cf60680dee..21c989fb7d 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.boot import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -13,8 +15,10 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -38,9 +42,13 @@ class SentrySpanRestTemplateCustomizerTest { val transaction: SentryTracer internal val customizer = SentrySpanRestTemplateCustomizer(hub) val url = mockServer.url("/test/123").toString() + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope( + any() + ) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) } @@ -164,6 +172,28 @@ class SentrySpanRestTemplateCustomizerTest { assertThat(fixture.transaction.spans).isEmpty() } + @Test + fun `when transaction is not active, adds tracing headers from scope`() { + val sut = fixture.getSut(isTransactionActive = false) + val headers = HttpHeaders() + headers.add("baggage", "thirdPartyBaggage=someValue") + headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + + val requestEntity = HttpEntity(headers) + + sut.exchange(fixture.url, HttpMethod.GET, requestEntity, String::class.java) + + val recorderRequest = fixture.mockServer.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + + val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER) + assertEquals(baggageHeaderValues.size, 1) + assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")) + assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key")) + assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) + } + @Test fun `avoids duplicate registration`() { val restTemplate = fixture.getSut(isTransactionActive = true) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt index e664d69a8a..ba26fef3e9 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt @@ -1,8 +1,10 @@ package io.sentry.spring.boot +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -16,8 +18,10 @@ import okhttp3.mockwebserver.RecordedRequest import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -33,6 +37,7 @@ import kotlin.test.assertNull class SentrySpanWebClientCustomizerTest { class Fixture { lateinit var sentryOptions: SentryOptions + lateinit var scope: Scope val hub = mock() var mockServer = MockWebServer() lateinit var transaction: SentryTracer @@ -47,7 +52,11 @@ class SentrySpanWebClientCustomizerTest { } dsn = "http://key@localhost/proj" } + scope = Scope(sentryOptions) whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope( + any() + ) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) val webClientBuilder = WebClient.builder() customizer.customize(webClientBuilder) @@ -124,6 +133,33 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active and server is not listed in tracing origins, does not add sentry trace header to the request`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = false) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active, adds sentry trace header to the request from scope`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = true) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -136,6 +172,7 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index f4387ce93a..32f76fbf7c 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -168,7 +168,7 @@ public abstract class io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter : protected fun doOnError (Lio/sentry/ITransaction;Ljava/lang/Throwable;)V protected fun maybeStartTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/ITransaction; protected fun shouldTraceRequest (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Z - protected fun startTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/ITransaction; + protected fun startTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; } public final class io/sentry/spring/jakarta/webflux/ReactorUtils { diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index c2a08da4ca..e862d620bb 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -10,11 +10,10 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanDataConvention; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; @@ -43,6 +42,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { try { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { + maybeAddTracingHeaders(request, null); return execution.execute(request, body); } @@ -53,18 +53,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.getURI())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - @Nullable - BaggageHeader baggage = - span.toBaggageHeader(request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER)); - if (baggage != null) { - request.getHeaders().set(baggage.getName(), baggage.getValue()); - } - } + maybeAddTracingHeaders(request, span); try { response = execution.execute(request, body); @@ -86,6 +75,29 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { } } + private void maybeAddTracingHeaders( + final @NotNull HttpRequest request, final @Nullable ISpan span) { + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + } + } + private void addBreadcrumb( final @NotNull HttpRequest request, final @NotNull byte[] body, diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java index 07b3a043d9..be677ab446 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java @@ -9,11 +9,10 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanDataConvention; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.web.reactive.function.client.ClientRequest; @@ -35,36 +34,17 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final @NotNull ClientRequest request, final @NotNull ExchangeFunction next) { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { - addBreadcrumb(request, null); - return next.exchange(request); + final @NotNull ClientRequest modifiedRequest = maybeAddTracingHeaders(request, null); + addBreadcrumb(modifiedRequest, null); + return next.exchange(modifiedRequest); } final ISpan span = activeSpan.startChild("http.client"); span.setDescription(request.method().name() + " " + request.url()); - final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); + final @NotNull ClientRequest modifiedRequest = maybeAddTracingHeaders(request, span); - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader(request.headers().get(BaggageHeader.BAGGAGE_HEADER)); - - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - } - - final ClientRequest clientRequestWithSentryTraceHeader = requestBuilder.build(); - - return next.exchange(clientRequestWithSentryTraceHeader) + return next.exchange(modifiedRequest) .flatMap( response -> { span.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.statusCode().value()); @@ -83,6 +63,35 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { }); } + private @NotNull ClientRequest maybeAddTracingHeaders( + final @NotNull ClientRequest request, final @Nullable ISpan span) { + final ClientRequest.Builder requestBuilder = ClientRequest.from(request); + + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + } + + return requestBuilder.build(); + } + private void addBreadcrumb( final @NotNull ClientRequest request, final @Nullable ClientResponse response) { final Breadcrumb breadcrumb = diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java index 8104a5383b..3bbcca3468 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java @@ -1,18 +1,15 @@ package io.sentry.spring.jakarta.tracing; import com.jakewharton.nopen.annotation.Open; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.CustomSamplingContext; import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import jakarta.servlet.FilterChain; @@ -29,7 +26,10 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; -/** Creates {@link ITransaction} around HTTP request executions. */ +/** + * Creates {@link ITransaction} around HTTP request executions if performance is enabled. Otherwise + * just reads tracing information from incoming request. + */ @Open public class SentryTracingFilter extends OncePerRequestFilter { /** Operation used by {@link SentryTransaction} created in {@link SentryTracingFilter}. */ @@ -74,44 +74,57 @@ protected void doFilterInternal( final @NotNull HttpServletResponse httpResponse, final @NotNull FilterChain filterChain) throws ServletException, IOException { - - if (hub.isEnabled() && shouldTraceRequest(httpRequest)) { - final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final List baggageHeader = + if (hub.isEnabled()) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - - // at this stage we are not able to get real transaction name - final ITransaction transaction = - startTransaction(httpRequest, sentryTraceHeader, baggageHeader); - try { + final @Nullable TransactionContext transactionContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); + if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { + doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); + } else { filterChain.doFilter(httpRequest, httpResponse); - } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); - throw e; - } finally { - // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); - // if transaction name is not resolved, the request has not been processed by a controller - // and we should not report it to Sentry - if (transactionName != null) { - transaction.setName(transactionName, transactionNameSource); - transaction.setOperation(TRANSACTION_OP); - // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and - // httpResponse.getStatus() returns 200. - if (transaction.getStatus() == null) { - transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); - } - transaction.finish(); - } } } else { filterChain.doFilter(httpRequest, httpResponse); } } + private void doFilterWithTransaction( + HttpServletRequest httpRequest, + HttpServletResponse httpResponse, + FilterChain filterChain, + final @Nullable TransactionContext transactionContext) + throws IOException, ServletException { + // at this stage we are not able to get real transaction name + final ITransaction transaction = startTransaction(httpRequest, transactionContext); + try { + filterChain.doFilter(httpRequest, httpResponse); + } catch (Throwable e) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; + } finally { + // after all filters run, templated path pattern is available in request attribute + final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); + final TransactionNameSource transactionNameSource = + transactionNameProvider.provideTransactionSource(); + // if transaction name is not resolved, the request has not been processed by a controller + // and we should not report it to Sentry + if (transactionName != null) { + transaction.setName(transactionName, transactionNameSource); + transaction.setOperation(TRANSACTION_OP); + // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and + // httpResponse.getStatus() returns 200. + if (transaction.getStatus() == null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); + } + transaction.finish(); + } + } + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return hub.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); @@ -119,37 +132,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable String sentryTraceHeader, - final @Nullable List baggageHeader) { + final @Nullable TransactionContext transactionContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - final Baggage baggage = Baggage.fromHeader(baggageHeader, hub.getOptions().getLogger()); - - if (sentryTraceHeader != null) { - try { - final TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - "http.server", - new SentryTraceHeader(sentryTraceHeader), - baggage, - null); - - final TransactionOptions transactionOptions = new TransactionOptions(); - transactionOptions.setCustomSamplingContext(customSamplingContext); - transactionOptions.setBindToScope(true); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation("http.server"); + + final TransactionOptions transactionOptions = new TransactionOptions(); + transactionOptions.setCustomSamplingContext(customSamplingContext); + transactionOptions.setBindToScope(true); + + return hub.startTransaction(transactionContext, transactionOptions); } final TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java index cebfb74f96..8b93ade6cf 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java @@ -3,7 +3,6 @@ import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.Breadcrumb; import io.sentry.CustomSamplingContext; @@ -12,12 +11,10 @@ import io.sentry.ITransaction; import io.sentry.NoOpHub; import io.sentry.Sentry; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.util.List; @@ -46,13 +43,20 @@ public AbstractSentryWebFilter(final @NotNull IHub hub) { protected @Nullable ITransaction maybeStartTransaction( final @NotNull IHub requestHub, final @NotNull ServerHttpRequest request) { - if (requestHub.isEnabled() - && requestHub.getOptions().isTracingEnabled() - && shouldTraceRequest(requestHub, request)) { - return startTransaction(requestHub, request); - } else { - return null; + if (requestHub.isEnabled()) { + final @NotNull HttpHeaders headers = request.getHeaders(); + final @Nullable String sentryTraceHeader = + headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @Nullable TransactionContext transactionContext = + requestHub.continueTrace(sentryTraceHeader, baggageHeaders); + + if (requestHub.getOptions().isTracingEnabled() && shouldTraceRequest(requestHub, request)) { + return startTransaction(requestHub, request, transactionContext); + } } + + return null; } protected void doFinally( @@ -119,11 +123,9 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact } protected @NotNull ITransaction startTransaction( - final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { - final @NotNull HttpHeaders headers = request.getHeaders(); - final @Nullable List sentryTraceHeaders = - headers.get(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @NotNull IHub hub, + final @NotNull ServerHttpRequest request, + final @Nullable TransactionContext transactionContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -132,25 +134,12 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (sentryTraceHeaders != null && sentryTraceHeaders.size() > 0) { - final @NotNull Baggage baggage = - Baggage.fromHeader(baggageHeaders, hub.getOptions().getLogger()); - try { - final @NotNull TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - TRANSACTION_OP, - new SentryTraceHeader(sentryTraceHeaders.get(0)), - baggage, - null); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation(TRANSACTION_OP); + + return hub.startTransaction(transactionContext, transactionOptions); } return hub.startTransaction( diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index 849cbbb12c..bdd93bb60e 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -1,6 +1,8 @@ package io.sentry.spring.jakarta.tracing import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanId @@ -9,6 +11,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import jakarta.servlet.FilterChain import jakarta.servlet.http.HttpServletRequest @@ -16,8 +19,10 @@ import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -40,13 +45,15 @@ class SentryTracingFilterTest { val transactionNameProvider = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" + enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -56,9 +63,13 @@ class SentryTracingFilterTest { request.addHeader("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + request.addHeader("baggage", baggageHeaders) + } response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) + whenever(hub.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryTracingFilter(hub, transactionNameProvider) } } @@ -205,7 +216,8 @@ class SentryTracingFilterTest { verify(fixture.chain).doFilter(fixture.request, fixture.response) verify(fixture.hub).isEnabled - verify(fixture.hub).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) + verify(fixture.hub, times(2)).options verifyNoMoreInteractions(fixture.hub) verify(fixture.transactionNameProvider, never()).provideTransactionName(any()) } @@ -249,4 +261,26 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt index 8cb30dcb50..87f9130c5d 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.jakarta.webflux import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryOptions @@ -13,6 +15,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import io.sentry.spring.jakarta.webflux.AbstractSentryWebFilter.SENTRY_HUB_KEY import org.assertj.core.api.Assertions.assertThat @@ -20,7 +23,9 @@ import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions @@ -50,17 +55,21 @@ class SentryWebFluxTracingFilterTest { dsn = "https://key@sentry.io/proj" enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, baggageHeaders: List? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { var requestBuilder = MockServerHttpRequest.method(method, "/product/{id}", 12) if (sentryTraceHeader != null) { requestBuilder = requestBuilder.header("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + requestBuilder = requestBuilder.header("baggage", *baggageHeaders.toTypedArray()) + } request = requestBuilder.build() exchange = MockServerWebExchange.builder(request).build() exchange.attributes.put(SENTRY_HUB_KEY, hub) @@ -69,6 +78,7 @@ class SentryWebFluxTracingFilterTest { whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) whenever(chain.filter(any())).thenReturn(Mono.create { s -> s.success() }) + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryWebFilter(hub) } } @@ -235,6 +245,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub, times(3)).isEnabled verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verify(fixture.hub).pushScope() verify(fixture.hub).addBreadcrumb(any(), any()) verify(fixture.hub).configureScope(any()) @@ -286,4 +297,28 @@ class SentryWebFluxTracingFilterTest { ) } } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + withMockHub { + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + } + } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 851ac13de8..e365398c6d 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -10,11 +10,10 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanDataConvention; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; @@ -43,6 +42,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { try { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { + maybeAddTracingHeaders(request, null); return execution.execute(request, body); } @@ -53,18 +53,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { urlDetails.applyToSpan(span); span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.getURI())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - @Nullable - BaggageHeader baggage = - span.toBaggageHeader(request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER)); - if (baggage != null) { - request.getHeaders().set(baggage.getName(), baggage.getValue()); - } - } + maybeAddTracingHeaders(request, span); try { response = execution.execute(request, body); @@ -86,6 +75,29 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { } } + private void maybeAddTracingHeaders( + final @NotNull HttpRequest request, final @Nullable ISpan span) { + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + } + } + private void addBreadcrumb( final @NotNull HttpRequest request, final @NotNull byte[] body, diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 9e5d51a8df..b3d462ab4f 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -9,11 +9,10 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanDataConvention; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -37,7 +36,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { addBreadcrumb(request, null); - return next.exchange(request); + return next.exchange(maybeAddHeaders(request, null)); } final ISpan span = activeSpan.startChild("http.client"); @@ -45,28 +44,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { span.setDescription(request.method().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader(request.headers().get(BaggageHeader.BAGGAGE_HEADER)); - - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - } - - final ClientRequest clientRequestWithSentryTraceHeader = requestBuilder.build(); + final ClientRequest clientRequestWithSentryTraceHeader = maybeAddHeaders(request, span); return next.exchange(clientRequestWithSentryTraceHeader) .flatMap( @@ -87,6 +65,35 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { }); } + private ClientRequest maybeAddHeaders( + final @NotNull ClientRequest request, final @Nullable ISpan span) { + final ClientRequest.Builder requestBuilder = ClientRequest.from(request); + + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + } + + return requestBuilder.build(); + } + private void addBreadcrumb( final @NotNull ClientRequest request, final @Nullable ClientResponse response) { final Breadcrumb breadcrumb = diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java index 1086950b94..a2afbf3578 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java @@ -1,18 +1,15 @@ package io.sentry.spring.tracing; import com.jakewharton.nopen.annotation.Open; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.CustomSamplingContext; import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.io.IOException; @@ -75,43 +72,58 @@ protected void doFilterInternal( final @NotNull FilterChain filterChain) throws ServletException, IOException { - if (hub.isEnabled() && shouldTraceRequest(httpRequest)) { - final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final List baggageHeader = + if (hub.isEnabled()) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + final @Nullable TransactionContext transactionContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); - // at this stage we are not able to get real transaction name - final ITransaction transaction = - startTransaction(httpRequest, sentryTraceHeader, baggageHeader); - try { + if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { + doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); + } else { filterChain.doFilter(httpRequest, httpResponse); - } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); - throw e; - } finally { - // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); - // if transaction name is not resolved, the request has not been processed by a controller - // and we should not report it to Sentry - if (transactionName != null) { - transaction.setName(transactionName, transactionNameSource); - transaction.setOperation(TRANSACTION_OP); - // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and - // httpResponse.getStatus() returns 200. - if (transaction.getStatus() == null) { - transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); - } - transaction.finish(); - } } } else { filterChain.doFilter(httpRequest, httpResponse); } } + private void doFilterWithTransaction( + HttpServletRequest httpRequest, + HttpServletResponse httpResponse, + FilterChain filterChain, + final @Nullable TransactionContext transactionContext) + throws IOException, ServletException { + // at this stage we are not able to get real transaction name + final ITransaction transaction = startTransaction(httpRequest, transactionContext); + try { + filterChain.doFilter(httpRequest, httpResponse); + } catch (Throwable e) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; + } finally { + // after all filters run, templated path pattern is available in request attribute + final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); + final TransactionNameSource transactionNameSource = + transactionNameProvider.provideTransactionSource(); + // if transaction name is not resolved, the request has not been processed by a controller + // and we should not report it to Sentry + if (transactionName != null) { + transaction.setName(transactionName, transactionNameSource); + transaction.setOperation(TRANSACTION_OP); + // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and + // httpResponse.getStatus() returns 200. + if (transaction.getStatus() == null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); + } + transaction.finish(); + } + } + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return hub.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); @@ -119,37 +131,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable String sentryTraceHeader, - final @Nullable List baggageHeader) { + final @Nullable TransactionContext transactionContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - final Baggage baggage = Baggage.fromHeader(baggageHeader, hub.getOptions().getLogger()); - - if (sentryTraceHeader != null) { - try { - final TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - "http.server", - new SentryTraceHeader(sentryTraceHeader), - baggage, - null); - - final TransactionOptions transactionOptions = new TransactionOptions(); - transactionOptions.setCustomSamplingContext(customSamplingContext); - transactionOptions.setBindToScope(true); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation("http.server"); + + final TransactionOptions transactionOptions = new TransactionOptions(); + transactionOptions.setCustomSamplingContext(customSamplingContext); + transactionOptions.setBindToScope(true); + + return hub.startTransaction(transactionContext, transactionOptions); } final TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java index 6e29739282..f5aa5482b6 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java @@ -3,7 +3,6 @@ import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.Breadcrumb; import io.sentry.CustomSamplingContext; @@ -12,12 +11,10 @@ import io.sentry.ITransaction; import io.sentry.NoOpHub; import io.sentry.Sentry; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.util.List; @@ -57,9 +54,16 @@ public Mono filter( final boolean isTracingEnabled = requestHub.getOptions().isTracingEnabled(); final @NotNull ServerHttpRequest request = serverWebExchange.getRequest(); + final @NotNull HttpHeaders headers = request.getHeaders(); + final @Nullable String sentryTraceHeader = + headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @Nullable TransactionContext transactionContext = + requestHub.continueTrace(sentryTraceHeader, baggageHeaders); + final @Nullable ITransaction transaction = isTracingEnabled && shouldTraceRequest(requestHub, request) - ? startTransaction(requestHub, request) + ? startTransaction(requestHub, request, transactionContext) : null; return webFilterChain @@ -105,11 +109,9 @@ private boolean shouldTraceRequest( } private @NotNull ITransaction startTransaction( - final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { - final @NotNull HttpHeaders headers = request.getHeaders(); - final @Nullable List sentryTraceHeaders = - headers.get(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @NotNull IHub hub, + final @NotNull ServerHttpRequest request, + final @Nullable TransactionContext transactionContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -118,25 +120,12 @@ private boolean shouldTraceRequest( transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (sentryTraceHeaders != null && sentryTraceHeaders.size() > 0) { - final @NotNull Baggage baggage = - Baggage.fromHeader(baggageHeaders, hub.getOptions().getLogger()); - try { - final @NotNull TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - TRANSACTION_OP, - new SentryTraceHeader(sentryTraceHeaders.get(0)), - baggage, - null); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation(TRANSACTION_OP); + + return hub.startTransaction(transactionContext, transactionOptions); } return hub.startTransaction( diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index cd72743d91..01ebfc53bd 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -1,6 +1,8 @@ package io.sentry.spring.tracing import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanId @@ -9,13 +11,16 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -40,13 +45,15 @@ class SentryTracingFilterTest { val transactionNameProvider = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" + enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -56,9 +63,13 @@ class SentryTracingFilterTest { request.addHeader("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + request.addHeader("baggage", baggageHeaders) + } response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) + whenever(hub.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryTracingFilter(hub, transactionNameProvider) } } @@ -205,7 +216,8 @@ class SentryTracingFilterTest { verify(fixture.chain).doFilter(fixture.request, fixture.response) verify(fixture.hub).isEnabled - verify(fixture.hub).options + verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verifyNoMoreInteractions(fixture.hub) verify(fixture.transactionNameProvider, never()).provideTransactionName(any()) } @@ -249,4 +261,26 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt index 388dbd9446..8bedbea1f4 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.webflux import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryOptions @@ -13,6 +15,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import io.sentry.spring.webflux.SentryWebFilter.SENTRY_HUB_KEY import org.assertj.core.api.Assertions.assertThat @@ -20,7 +23,9 @@ import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions @@ -50,17 +55,21 @@ class SentryWebFluxTracingFilterTest { dsn = "https://key@sentry.io/proj" enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, baggageHeaders: List? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { var requestBuilder = MockServerHttpRequest.method(method, "/product/{id}", 12) if (sentryTraceHeader != null) { requestBuilder = requestBuilder.header("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + requestBuilder = requestBuilder.header("baggage", *baggageHeaders.toTypedArray()) + } request = requestBuilder.build() exchange = MockServerWebExchange.builder(request).build() exchange.attributes.put(SENTRY_HUB_KEY, hub) @@ -69,6 +78,7 @@ class SentryWebFluxTracingFilterTest { whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) whenever(chain.filter(any())).thenReturn(Mono.create { s -> s.success() }) + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryWebFilter(hub) } } @@ -236,6 +246,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub).isEnabled verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verify(fixture.hub).pushScope() verify(fixture.hub).addBreadcrumb(any(), any()) verify(fixture.hub).configureScope(any()) @@ -287,4 +298,28 @@ class SentryWebFluxTracingFilterTest { ) } } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + withMockHub { + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + } + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 0e9d4c1a84..468e7e64de 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -30,6 +30,7 @@ public abstract interface class io/sentry/BackfillingEventProcessor : io/sentry/ } public final class io/sentry/Baggage { + public fun (Lio/sentry/Baggage;)V public fun (Lio/sentry/ILogger;)V public fun (Ljava/util/Map;Ljava/lang/String;ZLio/sentry/ILogger;)V public fun freeze ()V @@ -61,6 +62,7 @@ public final class io/sentry/Baggage { public fun setTransaction (Ljava/lang/String;)V public fun setUserId (Ljava/lang/String;)V public fun setUserSegment (Ljava/lang/String;)V + public fun setValuesFromScope (Lio/sentry/Scope;Lio/sentry/SentryOptions;)V public fun setValuesFromTransaction (Lio/sentry/ITransaction;Lio/sentry/protocol/User;Lio/sentry/SentryOptions;Lio/sentry/TracesSamplingDecision;)V public fun toHeaderString (Ljava/lang/String;)Ljava/lang/String; public fun toTraceContext ()Lio/sentry/TraceContext; @@ -348,11 +350,14 @@ public final class io/sentry/Hub : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -391,12 +396,15 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getInstance ()Lio/sentry/HubAdapter; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -461,11 +469,14 @@ public abstract interface class io/sentry/IHub { public abstract fun clone ()Lio/sentry/IHub; public abstract fun close ()V public abstract fun configureScope (Lio/sentry/ScopeCallback;)V + public abstract fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public abstract fun endSession ()V public abstract fun flush (J)V + public abstract fun getBaggage ()Lio/sentry/BaggageHeader; public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; public abstract fun getOptions ()Lio/sentry/SentryOptions; public abstract fun getSpan ()Lio/sentry/ISpan; + public abstract fun getTraceparent ()Lio/sentry/SentryTraceHeader; public abstract fun isCrashedLastRun ()Ljava/lang/Boolean; public abstract fun isEnabled ()Z public abstract fun popScope ()V @@ -819,12 +830,15 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getInstance ()Lio/sentry/NoOpHub; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -1112,6 +1126,26 @@ public final class io/sentry/ProfilingTransactionData$JsonKeys { public fun ()V } +public final class io/sentry/PropagationContext { + public fun ()V + public fun (Lio/sentry/PropagationContext;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Lio/sentry/Baggage;Ljava/lang/Boolean;)V + public static fun fromHeaders (Lio/sentry/ILogger;Ljava/lang/String;Ljava/lang/String;)Lio/sentry/PropagationContext; + public static fun fromHeaders (Lio/sentry/ILogger;Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; + public static fun fromHeaders (Lio/sentry/SentryTraceHeader;Lio/sentry/Baggage;Lio/sentry/SpanId;)Lio/sentry/PropagationContext; + public fun getBaggage ()Lio/sentry/Baggage; + public fun getParentSpanId ()Lio/sentry/SpanId; + public fun getSpanId ()Lio/sentry/SpanId; + public fun getTraceId ()Lio/sentry/protocol/SentryId; + public fun isSampled ()Ljava/lang/Boolean; + public fun setBaggage (Lio/sentry/Baggage;)V + public fun setParentSpanId (Lio/sentry/SpanId;)V + public fun setSampled (Ljava/lang/Boolean;)V + public fun setSpanId (Lio/sentry/SpanId;)V + public fun setTraceId (Lio/sentry/protocol/SentryId;)V + public fun traceContext ()Lio/sentry/TraceContext; +} + public final class io/sentry/RequestDetails { public fun (Ljava/lang/String;Ljava/util/Map;)V public fun getHeaders ()Ljava/util/Map; @@ -1136,6 +1170,7 @@ public final class io/sentry/Scope { public fun clearTransaction ()V public fun getContexts ()Lio/sentry/protocol/Contexts; public fun getLevel ()Lio/sentry/SentryLevel; + public fun getPropagationContext ()Lio/sentry/PropagationContext; public fun getRequest ()Lio/sentry/protocol/Request; public fun getSession ()Lio/sentry/Session; public fun getSpan ()Lio/sentry/ISpan; @@ -1156,14 +1191,20 @@ public final class io/sentry/Scope { public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V public fun setFingerprint (Ljava/util/List;)V public fun setLevel (Lio/sentry/SentryLevel;)V + public fun setPropagationContext (Lio/sentry/PropagationContext;)V public fun setRequest (Lio/sentry/protocol/Request;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Lio/sentry/ITransaction;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V + public fun withPropagationContext (Lio/sentry/Scope$IWithPropagationContext;)Lio/sentry/PropagationContext; public fun withTransaction (Lio/sentry/Scope$IWithTransaction;)V } +public abstract interface class io/sentry/Scope$IWithPropagationContext { + public abstract fun accept (Lio/sentry/PropagationContext;)V +} + public abstract interface class io/sentry/Scope$IWithTransaction { public abstract fun accept (Lio/sentry/ITransaction;)V } @@ -1224,11 +1265,14 @@ public final class io/sentry/Sentry { public static fun cloneMainHub ()Lio/sentry/IHub; public static fun close ()V public static fun configureScope (Lio/sentry/ScopeCallback;)V + public static fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public static fun endSession ()V public static fun flush (J)V + public static fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getCurrentHub ()Lio/sentry/IHub; public static fun getLastEventId ()Lio/sentry/protocol/SentryId; public static fun getSpan ()Lio/sentry/ISpan; + public static fun getTraceparent ()Lio/sentry/SentryTraceHeader; public static fun init ()V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;)V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;Z)V @@ -2232,13 +2276,12 @@ public final class io/sentry/TracesSamplingDecision { } public final class io/sentry/TransactionContext : io/sentry/SpanContext { + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Lio/sentry/TracesSamplingDecision;Lio/sentry/Baggage;)V public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;)V public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V public fun (Ljava/lang/String;Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V - public fun (Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/protocol/TransactionNameSource;Lio/sentry/SpanId;Lio/sentry/TracesSamplingDecision;Lio/sentry/Baggage;)V - public static fun fromSentryTrace (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext; - public static fun fromSentryTrace (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/SentryTraceHeader;Lio/sentry/Baggage;Lio/sentry/SpanId;)Lio/sentry/TransactionContext; + public static fun fromPropagationContext (Lio/sentry/PropagationContext;)Lio/sentry/TransactionContext; public static fun fromSentryTrace (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext; public fun getBaggage ()Lio/sentry/Baggage; public fun getInstrumenter ()Lio/sentry/Instrumenter; @@ -2247,8 +2290,10 @@ public final class io/sentry/TransactionContext : io/sentry/SpanContext { public fun getParentSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource; public fun setInstrumenter (Lio/sentry/Instrumenter;)V + public fun setName (Ljava/lang/String;)V public fun setParentSampled (Ljava/lang/Boolean;)V public fun setParentSampled (Ljava/lang/Boolean;Ljava/lang/Boolean;)V + public fun setTransactionNameSource (Lio/sentry/protocol/TransactionNameSource;)V } public abstract interface class io/sentry/TransactionFinishedCallback { @@ -4155,6 +4200,20 @@ public final class io/sentry/util/StringUtils { public static fun removeSurrounding (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; } +public final class io/sentry/util/TracingUtils { + public fun ()V + public static fun maybeUpdateBaggage (Lio/sentry/Scope;Lio/sentry/SentryOptions;)Lio/sentry/PropagationContext; + public static fun startNewTrace (Lio/sentry/IHub;)V + public static fun trace (Lio/sentry/IHub;Ljava/util/List;Lio/sentry/ISpan;)Lio/sentry/util/TracingUtils$TracingHeaders; + public static fun traceIfAllowed (Lio/sentry/IHub;Ljava/lang/String;Ljava/util/List;Lio/sentry/ISpan;)Lio/sentry/util/TracingUtils$TracingHeaders; +} + +public final class io/sentry/util/TracingUtils$TracingHeaders { + public fun (Lio/sentry/SentryTraceHeader;Lio/sentry/BaggageHeader;)V + public fun getBaggageHeader ()Lio/sentry/BaggageHeader; + public fun getSentryTraceHeader ()Lio/sentry/SentryTraceHeader; +} + public final class io/sentry/util/UrlUtils { public static final field SENSITIVE_DATA_SUBSTITUTE Ljava/lang/String; public fun ()V diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 1500d1c36f..e2cca117da 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -130,6 +130,11 @@ public Baggage(final @NotNull ILogger logger) { this(new HashMap<>(), null, true, logger); } + @ApiStatus.Internal + public Baggage(final @NotNull Baggage baggage) { + this(baggage.keyValues, baggage.thirdPartyHeader, baggage.mutable, baggage.logger); + } + @ApiStatus.Internal public Baggage( final @NotNull Map keyValues, @@ -352,6 +357,19 @@ public void setValuesFromTransaction( setSampleRate(sampleRateToString(sampleRate(samplingDecision))); } + @ApiStatus.Internal + public void setValuesFromScope(final @NotNull Scope scope, final @NotNull SentryOptions options) { + final @NotNull PropagationContext propagationContext = scope.getPropagationContext(); + final @Nullable User user = scope.getUser(); + setTraceId(propagationContext.getTraceId().toString()); + setPublicKey(new Dsn(options.getDsn()).getPublicKey()); + setRelease(options.getRelease()); + setEnvironment(options.getEnvironment()); + setUserSegment(user != null ? getSegment(user) : null); + setTransaction(null); + setSampleRate(null); + } + private static @Nullable String getSegment(final @NotNull User user) { if (user.getSegment() != null) { return user.getSegment(); diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 38a2250b9d..6b22488a02 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -11,6 +11,7 @@ import io.sentry.util.HintUtils; import io.sentry.util.Objects; import io.sentry.util.Pair; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.lang.ref.WeakReference; import java.util.Collections; @@ -741,21 +742,11 @@ public void flush(long timeoutMillis) { return transaction; } + @Deprecated + @SuppressWarnings("InlineMeSuggester") @Override public @Nullable SentryTraceHeader traceHeaders() { - SentryTraceHeader traceHeader = null; - if (!isEnabled()) { - options - .getLogger() - .log( - SentryLevel.WARNING, "Instance is disabled and this 'traceHeaders' call is a no-op."); - } else { - final ISpan span = stack.peek().getScope().getSpan(); - if (span != null && !span.isNoOp()) { - traceHeader = span.toSentryTrace(); - } - } - return traceHeader; + return getTraceparent(); } @Override @@ -818,4 +809,57 @@ private Scope buildLocalScope( } return scope; } + + @Override + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + @NotNull + PropagationContext propagationContext = + PropagationContext.fromHeaders(getOptions().getLogger(), sentryTrace, baggageHeaders); + configureScope( + (scope) -> { + scope.setPropagationContext(propagationContext); + }); + if (options.isTracingEnabled()) { + return TransactionContext.fromPropagationContext(propagationContext); + } else { + return null; + } + } + + @Override + public @Nullable SentryTraceHeader getTraceparent() { + if (!isEnabled()) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Instance is disabled and this 'getTraceparent' call is a no-op."); + } else { + final @Nullable TracingUtils.TracingHeaders headers = + TracingUtils.trace(this, null, getSpan()); + if (headers != null) { + return headers.getSentryTraceHeader(); + } + } + + return null; + } + + @Override + public @Nullable BaggageHeader getBaggage() { + if (!isEnabled()) { + options + .getLogger() + .log(SentryLevel.WARNING, "Instance is disabled and this 'getBaggage' call is a no-op."); + } else { + final @Nullable TracingUtils.TracingHeaders headers = + TracingUtils.trace(this, null, getSpan()); + if (headers != null) { + return headers.getBaggageHeader(); + } + } + + return null; + } } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index b777076d0f..e6ec220874 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -202,6 +202,7 @@ public void flush(long timeoutMillis) { return Sentry.startTransaction(transactionContext, transactionOptions); } + @Deprecated @Override public @Nullable SentryTraceHeader traceHeaders() { return Sentry.traceHeaders(); @@ -234,4 +235,20 @@ public void setSpanContext( public void reportFullyDisplayed() { Sentry.reportFullyDisplayed(); } + + @Override + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + return Sentry.continueTrace(sentryTrace, baggageHeaders); + } + + @Override + public @Nullable SentryTraceHeader getTraceparent() { + return Sentry.getTraceparent(); + } + + @Override + public @Nullable BaggageHeader getBaggage() { + return Sentry.getBaggage(); + } } diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index b99e99ab82..97583f66e8 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -524,10 +524,13 @@ ITransaction startTransaction( } /** - * Returns trace header of active transaction or {@code null} if no transaction is active. + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getBaggage()}. * - * @return trace header or null + * @deprecated please use {@link IHub#getTraceparent()} instead. + * @return sentry trace header or null */ + @Deprecated @Nullable SentryTraceHeader traceHeaders(); @@ -589,4 +592,34 @@ void setSpanContext( default void reportFullDisplayed() { reportFullyDisplayed(); } + + /** + * Continue a trace based on HTTP header values. If no "sentry-trace" header is provided a random + * trace ID and span ID is created. + * + * @param sentryTrace "sentry-trace" header + * @param baggageHeaders "baggage" headers + * @return a transaction context for starting a transaction or null if performance is disabled + */ + @Nullable + TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders); + + /** + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getBaggage()}. + * + * @return sentry trace header or null + */ + @Nullable + SentryTraceHeader getTraceparent(); + + /** + * Returns the "baggage" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getTraceparent()}. + * + * @return baggage header or null + */ + @Nullable + BaggageHeader getBaggage(); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 6735fe832f..aa5d846975 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -162,6 +162,8 @@ public void flush(long timeoutMillis) {} } @Override + @Deprecated + @SuppressWarnings("InlineMeSuggester") public @NotNull SentryTraceHeader traceHeaders() { return new SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true); } @@ -189,4 +191,20 @@ public void setSpanContext( @Override public void reportFullyDisplayed() {} + + @Override + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + return null; + } + + @Override + public @Nullable SentryTraceHeader getTraceparent() { + return null; + } + + @Override + public @Nullable BaggageHeader getBaggage() { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java new file mode 100644 index 0000000000..9a29e8c161 --- /dev/null +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -0,0 +1,142 @@ +package io.sentry; + +import io.sentry.exception.InvalidSentryTraceHeaderException; +import io.sentry.protocol.SentryId; +import java.util.Arrays; +import java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class PropagationContext { + + public static PropagationContext fromHeaders( + final @NotNull ILogger logger, + final @Nullable String sentryTraceHeader, + final @Nullable String baggageHeader) { + return fromHeaders(logger, sentryTraceHeader, Arrays.asList(baggageHeader)); + } + + public static @NotNull PropagationContext fromHeaders( + final @NotNull ILogger logger, + final @Nullable String sentryTraceHeaderString, + final @Nullable List baggageHeaderStrings) { + if (sentryTraceHeaderString == null) { + return new PropagationContext(); + } + + try { + final @NotNull SentryTraceHeader traceHeader = new SentryTraceHeader(sentryTraceHeaderString); + final @NotNull Baggage baggage = Baggage.fromHeader(baggageHeaderStrings, logger); + return fromHeaders(traceHeader, baggage, null); + } catch (InvalidSentryTraceHeaderException e) { + logger.log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); + return new PropagationContext(); + } + } + + public static @NotNull PropagationContext fromHeaders( + final @NotNull SentryTraceHeader sentryTraceHeader, + final @Nullable Baggage baggage, + final @Nullable SpanId spanId) { + final @NotNull SpanId spanIdToUse = spanId == null ? new SpanId() : spanId; + + return new PropagationContext( + sentryTraceHeader.getTraceId(), + spanIdToUse, + sentryTraceHeader.getSpanId(), + baggage, + sentryTraceHeader.isSampled()); + } + + private @NotNull SentryId traceId; + private @NotNull SpanId spanId; + private @Nullable SpanId parentSpanId; + + private @Nullable Boolean sampled; + + private @Nullable Baggage baggage; + + public PropagationContext() { + this(new SentryId(), new SpanId(), null, null, null); + } + + public PropagationContext(final @NotNull PropagationContext propagationContext) { + this( + propagationContext.getTraceId(), + propagationContext.getSpanId(), + propagationContext.getParentSpanId(), + cloneBaggage(propagationContext.getBaggage()), + propagationContext.isSampled()); + } + + private static @Nullable Baggage cloneBaggage(final @Nullable Baggage baggage) { + if (baggage != null) { + return new Baggage(baggage); + } + + return null; + } + + public PropagationContext( + final @NotNull SentryId traceId, + final @NotNull SpanId spanId, + final @Nullable SpanId parentSpanId, + final @Nullable Baggage baggage, + final @Nullable Boolean sampled) { + this.traceId = traceId; + this.spanId = spanId; + this.parentSpanId = parentSpanId; + this.baggage = baggage; + this.sampled = sampled; + } + + public @NotNull SentryId getTraceId() { + return traceId; + } + + public void setTraceId(final @NotNull SentryId traceId) { + this.traceId = traceId; + } + + public @NotNull SpanId getSpanId() { + return spanId; + } + + public void setSpanId(final @NotNull SpanId spanId) { + this.spanId = spanId; + } + + public @Nullable SpanId getParentSpanId() { + return parentSpanId; + } + + public void setParentSpanId(final @Nullable SpanId parentSpanId) { + this.parentSpanId = parentSpanId; + } + + public @Nullable Baggage getBaggage() { + return baggage; + } + + public void setBaggage(final @Nullable Baggage baggage) { + this.baggage = baggage; + } + + public @Nullable Boolean isSampled() { + return sampled; + } + + public void setSampled(final @Nullable Boolean sampled) { + this.sampled = sampled; + } + + public @Nullable TraceContext traceContext() { + if (baggage != null) { + return baggage.toTraceContext(); + } + + return null; + } +} diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 997f93c05d..9e65984913 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -65,12 +65,17 @@ public final class Scope { /** Transaction lock, Ops should be atomic */ private final @NotNull Object transactionLock = new Object(); + /** PropagationContext lock, Ops should be atomic */ + private final @NotNull Object propagationContextLock = new Object(); + /** Scope's contexts */ private @NotNull Contexts contexts = new Contexts(); /** Scope's attachments */ private @NotNull List attachments = new CopyOnWriteArrayList<>(); + private @NotNull PropagationContext propagationContext; + /** * Scope's ctor * @@ -79,6 +84,7 @@ public final class Scope { public Scope(final @NotNull SentryOptions options) { this.options = Objects.requireNonNull(options, "SentryOptions is required."); this.breadcrumbs = createBreadcrumbsList(this.options.getMaxBreadcrumbs()); + this.propagationContext = new PropagationContext(); } Scope(final @NotNull Scope scope) { @@ -134,6 +140,8 @@ public Scope(final @NotNull SentryOptions options) { this.contexts = new Contexts(scope.contexts); this.attachments = new CopyOnWriteArrayList<>(scope.attachments); + + this.propagationContext = new PropagationContext(scope.propagationContext); } /** @@ -799,6 +807,25 @@ public void withTransaction(final @NotNull IWithTransaction callback) { return session; } + @ApiStatus.Internal + public void setPropagationContext(final @NotNull PropagationContext propagationContext) { + this.propagationContext = propagationContext; + } + + @ApiStatus.Internal + public @NotNull PropagationContext getPropagationContext() { + return propagationContext; + } + + @ApiStatus.Internal + public @NotNull PropagationContext withPropagationContext( + final @NotNull IWithPropagationContext callback) { + synchronized (propagationContextLock) { + callback.accept(propagationContext); + return new PropagationContext(propagationContext); + } + } + /** the IWithTransaction callback */ @ApiStatus.Internal public interface IWithTransaction { @@ -810,4 +837,16 @@ public interface IWithTransaction { */ void accept(@Nullable ITransaction transaction); } + + /** the IWithPropagationContext callback */ + @ApiStatus.Internal + public interface IWithPropagationContext { + + /** + * The accept method of the callback + * + * @param propagationContext the current propagationContext + */ + void accept(@NotNull PropagationContext propagationContext); + } } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 0cd640c8c4..254c1fffd5 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -903,10 +903,14 @@ public static void endSession() { } /** - * Returns trace header of active transaction or {@code null} if no transaction is active. + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getBaggage()}. * - * @return trace header or null + * @deprecated please use {@link Sentry#getTraceparent()} instead. + * @return sentry trace header or null */ + @Deprecated + @SuppressWarnings("InlineMeSuggester") public static @Nullable SentryTraceHeader traceHeaders() { return getCurrentHub().traceHeaders(); } @@ -969,4 +973,38 @@ public interface OptionsConfiguration { */ void configure(@NotNull T options); } + + /** + * Continue a trace based on HTTP header values. If no "sentry-trace" header is provided a random + * trace ID and span ID is created. + * + * @param sentryTrace "sentry-trace" header + * @param baggageHeaders "baggage" headers + * @return a transaction context for starting a transaction or null if performance is disabled + */ + // return TransactionContext (if performance enabled) or null (if performance disabled) + public static @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + return getCurrentHub().continueTrace(sentryTrace, baggageHeaders); + } + + /** + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getBaggage()}. + * + * @return sentry trace header or null + */ + public static @Nullable SentryTraceHeader getTraceparent() { + return getCurrentHub().getTraceparent(); + } + + /** + * Returns the "baggage" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getTraceparent()}. + * + * @return baggage header or null + */ + public static @Nullable BaggageHeader getBaggage() { + return getCurrentHub().getBaggage(); + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 77f3e1462e..71769aa8b8 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -11,6 +11,7 @@ import io.sentry.transport.ITransport; import io.sentry.util.HintUtils; import io.sentry.util.Objects; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.io.IOException; import java.security.SecureRandom; @@ -171,10 +172,18 @@ private boolean shouldApplyScopeData( } try { - final TraceContext traceContext = - scope != null && scope.getTransaction() != null - ? scope.getTransaction().traceContext() - : null; + @Nullable TraceContext traceContext = null; + if (scope != null) { + final @Nullable ITransaction transaction = scope.getTransaction(); + if (transaction != null) { + traceContext = transaction.traceContext(); + } else { + final @NotNull PropagationContext propagationContext = + TracingUtils.maybeUpdateBaggage(scope, options); + traceContext = propagationContext.traceContext(); + } + } + final boolean shouldSendAttachments = event != null; List attachments = shouldSendAttachments ? getAttachments(hint) : null; final SentryEnvelope envelope = @@ -657,8 +666,14 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint } // Set trace data from active span to connect events with transactions final ISpan span = scope.getSpan(); - if (event.getContexts().getTrace() == null && span != null) { - event.getContexts().setTrace(span.getSpanContext()); + if (event.getContexts().getTrace() == null) { + if (span == null) { + event + .getContexts() + .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext())); + } else { + event.getContexts().setTrace(span.getSpanContext()); + } } event = processEvent(event, hint, scope.getEventProcessors()); diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index 4463f9ae4f..0885b9916a 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -3,13 +3,18 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; +import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public final class TransactionContext extends SpanContext { - private final @NotNull String name; - private final @NotNull TransactionNameSource transactionNameSource; + private static final @NotNull String DEFAULT_NAME = ""; + private static final @NotNull TransactionNameSource DEFAULT_NAME_SOURCE = + TransactionNameSource.CUSTOM; + private static final @NotNull String DEFAULT_OPERATION = "default"; + private @NotNull String name; + private @NotNull TransactionNameSource transactionNameSource; private @Nullable TracesSamplingDecision parentSamplingDecision; private @Nullable Baggage baggage; private @NotNull Instrumenter instrumenter = Instrumenter.SENTRY; @@ -20,69 +25,48 @@ public final class TransactionContext extends SpanContext { * @param name - the transaction name * @param operation - the operation * @param sentryTrace - the sentry-trace header + * @deprecated use {@link Sentry#continueTrace(String, List)} and setters for name and operation + * here instead. * @return the transaction contexts */ + @Deprecated public static @NotNull TransactionContext fromSentryTrace( final @NotNull String name, final @NotNull String operation, final @NotNull SentryTraceHeader sentryTrace) { - return fromSentryTrace(name, TransactionNameSource.CUSTOM, operation, sentryTrace, null, null); - } - - /** - * Creates {@link TransactionContext} from sentry-trace header. - * - * @param name - the transaction name - * @param transactionNameSource - source of the transaction name - * @param operation - the operation - * @param sentryTrace - the sentry-trace header - * @return the transaction contexts - */ - @ApiStatus.Internal - public static @NotNull TransactionContext fromSentryTrace( - final @NotNull String name, - final @NotNull TransactionNameSource transactionNameSource, - final @NotNull String operation, - final @NotNull SentryTraceHeader sentryTrace) { @Nullable Boolean parentSampled = sentryTrace.isSampled(); - return new TransactionContext( - name, - operation, - sentryTrace.getTraceId(), - new SpanId(), - transactionNameSource, - sentryTrace.getSpanId(), - parentSampled == null ? null : new TracesSamplingDecision(parentSampled), - null); + TracesSamplingDecision samplingDecision = + parentSampled == null ? null : new TracesSamplingDecision(parentSampled); + + TransactionContext transactionContext = + new TransactionContext( + sentryTrace.getTraceId(), + new SpanId(), + sentryTrace.getSpanId(), + samplingDecision, + null); + + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.CUSTOM); + transactionContext.setOperation(operation); + + return transactionContext; } - /** - * Creates {@link TransactionContext} from sentry-trace header. - * - * @param name - the transaction name - * @param transactionNameSource - source of the transaction name - * @param operation - the operation - * @param sentryTrace - the sentry-trace header - * @param baggage - the baggage header - * @return the transaction contexts - */ @ApiStatus.Internal - public static @NotNull TransactionContext fromSentryTrace( - final @NotNull String name, - final @NotNull TransactionNameSource transactionNameSource, - final @NotNull String operation, - final @NotNull SentryTraceHeader sentryTrace, - final @Nullable Baggage baggage, - final @Nullable SpanId spanId) { - @Nullable Boolean parentSampled = sentryTrace.isSampled(); + public static TransactionContext fromPropagationContext( + final @NotNull PropagationContext propagationContext) { + @Nullable Boolean parentSampled = propagationContext.isSampled(); TracesSamplingDecision samplingDecision = parentSampled == null ? null : new TracesSamplingDecision(parentSampled); + @Nullable Baggage baggage = propagationContext.getBaggage(); + if (baggage != null) { baggage.freeze(); Double sampleRate = baggage.getSampleRateDouble(); - Boolean sampled = parentSampled != null ? parentSampled.booleanValue() : true; + Boolean sampled = parentSampled != null ? parentSampled.booleanValue() : false; if (sampleRate != null) { samplingDecision = new TracesSamplingDecision(sampled, sampleRate); } else { @@ -90,15 +74,10 @@ public final class TransactionContext extends SpanContext { } } - final @NotNull SpanId spanIdToUse = spanId == null ? new SpanId() : spanId; - return new TransactionContext( - name, - operation, - sentryTrace.getTraceId(), - spanIdToUse, - transactionNameSource, - sentryTrace.getSpanId(), + propagationContext.getTraceId(), + propagationContext.getSpanId(), + propagationContext.getParentSpanId(), samplingDecision, baggage); } @@ -150,18 +129,15 @@ public TransactionContext( @ApiStatus.Internal public TransactionContext( - final @NotNull String name, - final @NotNull String operation, final @NotNull SentryId traceId, final @NotNull SpanId spanId, - final @NotNull TransactionNameSource transactionNameSource, final @Nullable SpanId parentSpanId, final @Nullable TracesSamplingDecision parentSamplingDecision, final @Nullable Baggage baggage) { - super(traceId, spanId, operation, parentSpanId, null); - this.name = Objects.requireNonNull(name, "name is required"); + super(traceId, spanId, DEFAULT_OPERATION, parentSpanId, null); + this.name = DEFAULT_NAME; this.parentSamplingDecision = parentSamplingDecision; - this.transactionNameSource = transactionNameSource; + this.transactionNameSource = DEFAULT_NAME_SOURCE; this.baggage = baggage; } @@ -216,4 +192,12 @@ public void setParentSampled( public void setInstrumenter(final @NotNull Instrumenter instrumenter) { this.instrumenter = instrumenter; } + + public void setName(final @NotNull String name) { + this.name = Objects.requireNonNull(name, "name is required"); + } + + public void setTransactionNameSource(final @NotNull TransactionNameSource transactionNameSource) { + this.transactionNameSource = transactionNameSource; + } } diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java new file mode 100644 index 0000000000..fbb8ccb775 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -0,0 +1,119 @@ +package io.sentry.util; + +import io.sentry.Baggage; +import io.sentry.BaggageHeader; +import io.sentry.IHub; +import io.sentry.ISpan; +import io.sentry.PropagationContext; +import io.sentry.Scope; +import io.sentry.SentryOptions; +import io.sentry.SentryTraceHeader; +import java.util.List; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class TracingUtils { + + public static void startNewTrace(final @NotNull IHub hub) { + hub.configureScope( + scope -> { + scope.withPropagationContext( + propagationContext -> { + scope.setPropagationContext(new PropagationContext()); + }); + }); + } + + public static @Nullable TracingHeaders traceIfAllowed( + final @NotNull IHub hub, + final @NotNull String requestUrl, + @Nullable List thirdPartyBaggageHeaders, + final @Nullable ISpan span) { + final @NotNull SentryOptions sentryOptions = hub.getOptions(); + if (sentryOptions.isTraceSampling() && shouldAttachTracingHeaders(requestUrl, sentryOptions)) { + return trace(hub, thirdPartyBaggageHeaders, span); + } + + return null; + } + + public static @Nullable TracingHeaders trace( + final @NotNull IHub hub, + @Nullable List thirdPartyBaggageHeaders, + final @Nullable ISpan span) { + final @NotNull SentryOptions sentryOptions = hub.getOptions(); + + if (span != null && !span.isNoOp()) { + return new TracingHeaders( + span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders)); + } else { + final @NotNull PropagationContextHolder returnValue = new PropagationContextHolder(); + hub.configureScope( + (scope) -> { + returnValue.propagationContext = maybeUpdateBaggage(scope, sentryOptions); + }); + + if (returnValue.propagationContext != null) { + final @NotNull PropagationContext propagationContext = returnValue.propagationContext; + final @Nullable Baggage baggage = propagationContext.getBaggage(); + @Nullable BaggageHeader baggageHeader = null; + if (baggage != null) { + baggageHeader = + BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); + } + + return new TracingHeaders( + new SentryTraceHeader( + propagationContext.getTraceId(), propagationContext.getSpanId(), null), + baggageHeader); + } + + return null; + } + } + + public static @NotNull PropagationContext maybeUpdateBaggage( + final @NotNull Scope scope, final @NotNull SentryOptions sentryOptions) { + return scope.withPropagationContext( + propagationContext -> { + @Nullable Baggage baggage = propagationContext.getBaggage(); + if (baggage == null) { + baggage = new Baggage(sentryOptions.getLogger()); + propagationContext.setBaggage(baggage); + } + if (baggage.isMutable()) { + baggage.setValuesFromScope(scope, sentryOptions); + baggage.freeze(); + } + }); + } + + private static boolean shouldAttachTracingHeaders( + final @NotNull String requestUrl, final @NotNull SentryOptions sentryOptions) { + return PropagationTargetsUtils.contain(sentryOptions.getTracePropagationTargets(), requestUrl); + } + + private static final class PropagationContextHolder { + private @Nullable PropagationContext propagationContext = null; + } + + public static final class TracingHeaders { + private final @NotNull SentryTraceHeader sentryTraceHeader; + private final @Nullable BaggageHeader baggageHeader; + + public TracingHeaders( + final @NotNull SentryTraceHeader sentryTraceHeader, + final @Nullable BaggageHeader baggageHeader) { + this.sentryTraceHeader = sentryTraceHeader; + this.baggageHeader = baggageHeader; + } + + public @NotNull SentryTraceHeader getSentryTraceHeader() { + return sentryTraceHeader; + } + + public @Nullable BaggageHeader getBaggageHeader() { + return baggageHeader; + } + } +} diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index baab6e4e57..96a44462d4 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -31,12 +31,14 @@ import java.io.File import java.nio.file.Files import java.util.Queue import java.util.UUID +import java.util.concurrent.atomic.AtomicReference import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -1580,10 +1582,15 @@ class HubTest { //region startTransaction tests @Test - fun `when traceHeaders and no transaction is active, traceHeaders are null`() { + fun `when traceHeaders and no transaction is active, traceHeaders are generated from scope`() { val hub = generateHub() - assertNull(hub.traceHeaders()) + var spanId: SpanId? = null + hub.configureScope { spanId = it.propagationContext.spanId } + + val traceHeader = hub.traceHeaders() + assertNotNull(traceHeader) + assertEquals(spanId, traceHeader.spanId) } @Test @@ -1720,6 +1727,79 @@ class HubTest { verify(hub).reportFullyDisplayed() } + @Test + fun `continueTrace creates propagation context from headers and returns transaction context if performance enabled`() { + val hub = generateHub() + val traceId = SentryId() + val parentSpanId = SpanId() + val transactionContext = hub.continueTrace("$traceId-$parentSpanId-1", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertEquals(traceId, scope.propagationContext.traceId) + assertEquals(parentSpanId, scope.propagationContext.parentSpanId) + } + + assertEquals(traceId, transactionContext!!.traceId) + assertEquals(parentSpanId, transactionContext!!.parentSpanId) + } + + @Test + fun `continueTrace creates new propagation context if header invalid and returns transaction context if performance enabled`() { + val hub = generateHub() + val traceId = SentryId() + var propagationContextHolder = AtomicReference() + + hub.configureScope { propagationContextHolder.set(it.propagationContext) } + val propagationContextAtStart = propagationContextHolder.get()!! + + val transactionContext = hub.continueTrace("invalid", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertNotEquals(propagationContextAtStart.traceId, scope.propagationContext.traceId) + assertNotEquals(propagationContextAtStart.parentSpanId, scope.propagationContext.parentSpanId) + assertNotEquals(propagationContextAtStart.spanId, scope.propagationContext.spanId) + + assertEquals(scope.propagationContext.traceId, transactionContext!!.traceId) + assertEquals(scope.propagationContext.parentSpanId, transactionContext!!.parentSpanId) + assertEquals(scope.propagationContext.spanId, transactionContext!!.spanId) + } + } + + @Test + fun `continueTrace creates propagation context from headers and returns null if performance disabled`() { + val hub = generateHub { it.enableTracing = false } + val traceId = SentryId() + val parentSpanId = SpanId() + val transactionContext = hub.continueTrace("$traceId-$parentSpanId-1", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertEquals(traceId, scope.propagationContext.traceId) + assertEquals(parentSpanId, scope.propagationContext.parentSpanId) + } + + assertNull(transactionContext) + } + + @Test + fun `continueTrace creates new propagation context if header invalid and returns null if performance disabled`() { + val hub = generateHub { it.enableTracing = false } + val traceId = SentryId() + var propagationContextHolder = AtomicReference() + + hub.configureScope { propagationContextHolder.set(it.propagationContext) } + val propagationContextAtStart = propagationContextHolder.get()!! + + val transactionContext = hub.continueTrace("invalid", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertNotEquals(propagationContextAtStart.traceId, scope.propagationContext.traceId) + assertNotEquals(propagationContextAtStart.parentSpanId, scope.propagationContext.parentSpanId) + assertNotEquals(propagationContextAtStart.spanId, scope.propagationContext.spanId) + } + + assertNull(transactionContext) + } + private val dsnTest = "https://key@sentry.io/proj" private fun generateHub(optionsConfiguration: Sentry.OptionsConfiguration? = null): IHub { diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index b17235f738..28e16e954d 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 io.sentry.Scope.IWithPropagationContext import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent @@ -1340,12 +1341,14 @@ class SentryClientTest { } @Test - fun `when scope does not have an active transaction, trace state is not set on the envelope`() { + fun `when scope does not have an active transaction, trace state is set on the envelope from scope`() { val sut = fixture.getSut() - sut.captureEvent(SentryEvent(), createScope()) + val scope = createScope() + sut.captureEvent(SentryEvent(), scope) verify(fixture.transport).send( check { - assertNull(it.header.traceContext) + assertNotNull(it.header.traceContext) + assertEquals(scope.propagationContext.traceId, it.header.traceContext?.traceId) }, anyOrNull() ) @@ -2095,6 +2098,9 @@ class SentryClientTest { whenever(scope.breadcrumbs).thenReturn(LinkedList()) whenever(scope.extras).thenReturn(emptyMap()) whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val transactionEnd = object : TransactionEnd {} val transactionEndHint = HintUtils.createWithTypeCheckHint(transactionEnd) @@ -2110,6 +2116,174 @@ class SentryClientTest { ) } + @Test + fun `attaches trace context from span if none present yet`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + whenever(scope.span).thenReturn(transaction) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(spanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(spanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `attaches trace context from scope if none present yet and no span on scope`() { + val sut = fixture.getSut() + + // scope + val scope = mock() + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `keeps existing trace context if already present`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + + val preExistingSpanContext = SpanContext("op.load") + + val sentryEvent = SentryEvent() + sentryEvent.contexts.trace = preExistingSpanContext + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(preExistingSpanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(preExistingSpanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(spanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(spanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `uses propagation context on scope for trace header if no transaction is on scope`() { + val sut = fixture.getSut() + + // scope + val scope = mock() + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertNotNull(it.header.traceContext) + assertEquals(scopePropagationContext.traceId, it.header.traceContext!!.traceId) + }, + any() + ) + } + + @Test + fun `uses trace context on transaction for trace header if a transaction is on scope`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + val transactionTraceContext = TraceContext(SentryId(), "pubkey") + whenever(transaction.traceContext()).thenReturn(transactionTraceContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) + + val preExistingSpanContext = SpanContext("op.load") + + val sentryEvent = SentryEvent() + sentryEvent.contexts.trace = preExistingSpanContext + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertNotNull(it.header.traceContext) + assertEquals(transactionTraceContext.traceId, it.header.traceContext!!.traceId) + }, + any() + ) + } + private fun givenScopeWithStartedSession(errored: Boolean = false, crashed: Boolean = false): Scope { val scope = createScope(fixture.sentryOptions) scope.startSession() diff --git a/sentry/src/test/java/io/sentry/TransactionContextTest.kt b/sentry/src/test/java/io/sentry/TransactionContextTest.kt index 4c40a65272..6f5a0ead33 100644 --- a/sentry/src/test/java/io/sentry/TransactionContextTest.kt +++ b/sentry/src/test/java/io/sentry/TransactionContextTest.kt @@ -1,7 +1,7 @@ package io.sentry import io.sentry.protocol.SentryId -import io.sentry.protocol.TransactionNameSource +import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -30,10 +30,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of false is set from trace header`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), false) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of false is set from trace header`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), false).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3" + ) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -41,10 +45,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of false is set from trace header if no sample rate is available`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), false) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of false is set from trace header if no sample rate is available`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), false).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction" + ) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -52,10 +60,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of true is set from trace header`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), true) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of true is set from trace header`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), true).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3" + ) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) @@ -63,10 +75,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of true is set from trace header if no sample rate is available`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), true) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of true is set from trace header if no sample rate is available`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), true).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction" + ) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) diff --git a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt new file mode 100644 index 0000000000..e38410641e --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt @@ -0,0 +1,197 @@ +package io.sentry.util + +import io.sentry.Baggage +import io.sentry.IHub +import io.sentry.NoOpSpan +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.Span +import io.sentry.SpanOptions +import io.sentry.TracesSamplingDecision +import io.sentry.TransactionContext +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class TracingUtilsTest { + + class Fixture { + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + } + val hub = mock() + val scope = Scope(options) + lateinit var span: Span + val preExistingBaggage = listOf("some-baggage-key=some-baggage-value") + + fun setup() { + whenever(hub.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + span = Span( + TransactionContext("name", "op", TracesSamplingDecision(true)), + SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), hub), + hub, + null, + SpanOptions() + ) + } + } + + val fixture = Fixture() + + @Test + fun `returns headers if allowed from scope without span`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertTrue(headers.baggageHeader!!.value.contains("sentry-trace_id=${fixture.scope.propagationContext.traceId}")) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `returns headers if allowed from scope if span is noop`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, NoOpSpan.getInstance()) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `returns headers if allowed from scope without span leaving frozen baggage alone`() { + fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET").also { it.freeze() } + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertTrue(headers.baggageHeader!!.value.contains("sentry-trace_id=2722d9f6ec019ade60c776169d9a8904")) + assertFalse(headers.baggageHeader!!.value.contains("sentry-trace_id=${fixture.scope.propagationContext.traceId}")) + } + + @Test + fun `returns headers if allowed from span`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.span.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.span.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + } + + @Test + fun `does not return headers if not trace sampling without span`() { + fixture.options.isTraceSampling = false + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNull(headers) + } + + @Test + fun `does not return headers if not trace sampling from span`() { + fixture.options.isTraceSampling = false + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNull(headers) + } + + @Test + fun `does not return headers if host is disallowed without span`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNull(headers) + } + + @Test + fun `does not return headers if host is disallowed from span`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNull(headers) + } + + @Test + fun `start new trace sets propagation context on scope`() { + fixture.setup() + + val propagationContextBefore = fixture.scope.propagationContext + + TracingUtils.startNewTrace(fixture.hub) + + assertNotEquals(propagationContextBefore.traceId, fixture.scope.propagationContext.traceId) + assertNotEquals(propagationContextBefore.spanId, fixture.scope.propagationContext.spanId) + } + + @Test + fun `creates new baggage if none present`() { + fixture.setup() + assertNull(fixture.scope.propagationContext.baggage) + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertNotNull(fixture.scope.propagationContext.baggage) + assertEquals(fixture.scope.propagationContext.traceId.toString(), fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `updates mutable baggage`() { + fixture.setup() + // not frozen because it doesn't contain sentry-* keys + fixture.scope.propagationContext.baggage = Baggage.fromHeader(fixture.preExistingBaggage) + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertEquals(fixture.scope.propagationContext.traceId.toString(), fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `does not change frozen baggage`() { + fixture.setup() + // frozen automatically because it contains sentry-* keys + fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertEquals("2722d9f6ec019ade60c776169d9a8904", fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } +}