Performance: Refactor resolving SpanContext for Throwable

CHANGELOG.md

@@ -3,6 +3,7 @@
 * Enhancement: Set transaction name on events and transactions sent using Spring integration (#1067) 
 * Fix: Set current thread only if there are no exceptions
 * Enhancement: Set global tags on SentryOptions and load them from external configuration (#1066)
+* Ref: Refactor resolving SpanContext for Throwable (#1068)
 * Enhancement: Add API validator and remove deprecated methods
 * Enhancement: Add more convenient method to start a child span (#1073)
 * Enhancement: Autoconfigure traces callback in Spring Boot integration (#1074)

sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java

@@ -4,12 +4,11 @@
 import io.sentry.IHub;
 import io.sentry.SentryEvent;
 import io.sentry.SentryLevel;
-import io.sentry.SentryTransaction;
 import io.sentry.SpanContext;
 import io.sentry.exception.ExceptionMechanismException;
 import io.sentry.protocol.Mechanism;
+import io.sentry.spring.tracing.TransactionNameProvider;
 import io.sentry.util.Objects;
-import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.jetbrains.annotations.NotNull;
@@ -26,6 +25,8 @@
 @Open
 public class SentryExceptionResolver implements HandlerExceptionResolver, Ordered {
   private final @NotNull IHub hub;
+  private final @NotNull TransactionNameProvider transactionNameProvider =
+      new TransactionNameProvider();
   private final int order;
 
   public SentryExceptionResolver(final @NotNull IHub hub, final int order) {
@@ -46,14 +47,12 @@ public SentryExceptionResolver(final @NotNull IHub hub, final int order) {
         new ExceptionMechanismException(mechanism, ex, Thread.currentThread());
     final SentryEvent event = new SentryEvent(throwable);
     event.setLevel(SentryLevel.FATAL);
+    event.setTransaction(transactionNameProvider.provideTransactionName(request));
 
-    final SentryTransaction sentryTransaction = resolveActiveTransaction();
-    if (sentryTransaction != null) {
-      final SpanContext spanContext = sentryTransaction.getSpanContext(ex);
-      if (spanContext != null) {
-        // connects the event with a span
-        event.getContexts().setTrace(spanContext);
-      }
+    final SpanContext spanContext = hub.getSpanContext(ex);
+    if (spanContext != null) {
+      // connects the event with a span
+      event.getContexts().setTrace(spanContext);
     }
     hub.captureEvent(event);
 
@@ -65,19 +64,4 @@ public SentryExceptionResolver(final @NotNull IHub hub, final int order) {
   public int getOrder() {
     return order;
   }
-
-  private @Nullable SentryTransaction resolveActiveTransaction() {
-    final AtomicReference<SentryTransaction> spanRef = new AtomicReference<>();
-
-    hub.configureScope(
-        scope -> {
-          final SentryTransaction transaction = scope.getTransaction();
-
-          if (transaction != null) {
-            spanRef.set(transaction);
-          }
-        });
-
-    return spanRef.get();
-  }
 }

sentry/api/sentry.api

@@ -100,6 +100,7 @@ public final class io/sentry/Hub : io/sentry/IHub {
 	public fun flush (J)V
 	public fun getLastEventId ()Lio/sentry/protocol/SentryId;
 	public fun getSpan ()Lio/sentry/ISpan;
+	public fun getSpanContext (Ljava/lang/Throwable;)Lio/sentry/SpanContext;
 	public fun isEnabled ()Z
 	public fun popScope ()V
 	public fun pushScope ()V
@@ -108,6 +109,7 @@ public final class io/sentry/Hub : io/sentry/IHub {
 	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 setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setTransaction (Ljava/lang/String;)V
 	public fun setUser (Lio/sentry/protocol/User;)V
@@ -137,6 +139,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
 	public static fun getInstance ()Lio/sentry/HubAdapter;
 	public fun getLastEventId ()Lio/sentry/protocol/SentryId;
 	public fun getSpan ()Lio/sentry/ISpan;
+	public fun getSpanContext (Ljava/lang/Throwable;)Lio/sentry/SpanContext;
 	public fun isEnabled ()Z
 	public fun popScope ()V
 	public fun pushScope ()V
@@ -145,6 +148,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
 	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 setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
 	public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public fun setTransaction (Ljava/lang/String;)V
 	public fun setUser (Lio/sentry/protocol/User;)V
@@ -189,6 +193,7 @@ public abstract interface class io/sentry/IHub {
 	public abstract fun flush (J)V
 	public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId;
 	public abstract fun getSpan ()Lio/sentry/ISpan;
+	public abstract fun getSpanContext (Ljava/lang/Throwable;)Lio/sentry/SpanContext;
 	public abstract fun isEnabled ()Z
 	public abstract fun popScope ()V
 	public abstract fun pushScope ()V
@@ -197,6 +202,7 @@ public abstract interface class io/sentry/IHub {
 	public abstract fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public abstract fun setFingerprint (Ljava/util/List;)V
 	public abstract fun setLevel (Lio/sentry/SentryLevel;)V
+	public abstract fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/SpanContext;)V
 	public abstract fun setTag (Ljava/lang/String;Ljava/lang/String;)V
 	public abstract fun setTransaction (Ljava/lang/String;)V
 	public abstract fun setUser (Lio/sentry/protocol/User;)V
@@ -725,7 +731,6 @@ public final class io/sentry/SentryTransaction : io/sentry/SentryBaseEvent, io/s
 	public fun finish ()V
 	public fun getDescription ()Ljava/lang/String;
 	public fun getSpanContext ()Lio/sentry/SpanContext;
-	public fun getSpanContext (Ljava/lang/Throwable;)Lio/sentry/SpanContext;
 	public fun getSpans ()Ljava/util/List;
 	public fun getTransaction ()Ljava/lang/String;
 	public fun setDescription (Ljava/lang/String;)V

sentry/src/main/java/io/sentry/Hub.java

@@ -8,6 +8,7 @@
 import java.io.Closeable;
 import java.util.Deque;
 import java.util.List;
+import java.util.WeakHashMap;
 import java.util.concurrent.LinkedBlockingDeque;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -30,6 +31,8 @@ private static final class StackItem {
   private volatile boolean isEnabled;
   private final @NotNull Deque<StackItem> stack = new LinkedBlockingDeque<>();
   private final @NotNull TracingSampler tracingSampler;
+  private final @NotNull WeakHashMap<Throwable, SpanContext> throwableToSpanContext =
+      new WeakHashMap<>();
 
   public Hub(final @NotNull SentryOptions options) {
     this(options, createRootStackItem(options));
@@ -753,4 +756,18 @@ public void flush(long timeoutMillis) {
     }
     return span;
   }
+
+  @Override
+  public void setSpanContext(
+      final @NotNull Throwable throwable, final @NotNull SpanContext spanContext) {
+    Objects.requireNonNull(throwable, "throwable is required");
+    Objects.requireNonNull(spanContext, "spanContext is required");
+    this.throwableToSpanContext.put(throwable, spanContext);
+  }
+
+  @Override
+  public @Nullable SpanContext getSpanContext(final @NotNull Throwable throwable) {
+    Objects.requireNonNull(throwable, "throwable is required");
+    return this.throwableToSpanContext.get(throwable);
+  }
 }

sentry/src/main/java/io/sentry/HubAdapter.java

@@ -4,6 +4,7 @@
 import io.sentry.protocol.User;
 import java.util.List;
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 public final class HubAdapter implements IHub {
@@ -179,6 +180,16 @@ public SentryTransaction startTransaction(
     return Sentry.traceHeaders();
   }
 
+  @Override
+  public void setSpanContext(final @NotNull Throwable t, final @NotNull SpanContext sc) {
+    Sentry.getCurrentHub().setSpanContext(t, sc);
+  }
+
+  @Override
+  public @Nullable SpanContext getSpanContext(final @NotNull Throwable ex) {
+    return Sentry.getCurrentHub().getSpanContext(ex);
+  }
+
   @Override
   public @Nullable ISpan getSpan() {
     return Sentry.getCurrentHub().getSpan();

sentry/src/main/java/io/sentry/IHub.java

@@ -333,6 +333,27 @@ default SentryTransaction startTransaction(final @NotNull String name) {
   @Nullable
   SentryTraceHeader traceHeaders();
 
+  /**
+   * Associates {@link SpanContext} with the {@link Throwable}. Used to determine in which trace the
+   * exception has been thrown in framework integrations.
+   *
+   * @param throwable the throwable
+   * @param spanContext the span context
+   */
+  @ApiStatus.Internal
+  void setSpanContext(@NotNull Throwable throwable, @NotNull SpanContext spanContext);
+
+  /**
+   * Gets the span context for the span that was active while the throwable given by parameter was
+   * thrown.
+   *
+   * @param throwable - the throwable
+   * @return span context or {@code null} if no corresponding span context found.
+   */
+  @ApiStatus.Internal
+  @Nullable
+  SpanContext getSpanContext(Throwable throwable);
+
   /**
    * Gets the current active transaction or span.
    *

sentry/src/main/java/io/sentry/NoOpHub.java

@@ -133,6 +133,15 @@ public SentryTransaction startTransaction(
     return new SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true);
   }
 
+  @Override
+  public void setSpanContext(
+      final @NotNull Throwable throwable, final @NotNull SpanContext spanContext) {}
+
+  @Override
+  public @Nullable SpanContext getSpanContext(final @NotNull Throwable throwable) {
+    return null;
+  }
+
   @Override
   public @Nullable ISpan getSpan() {
     return null;

sentry/src/main/java/io/sentry/SentryTransaction.java

@@ -106,7 +106,7 @@ public Span startChild() {
    */
   Span startChild(final @NotNull SpanId parentSpanId) {
     Objects.requireNonNull(parentSpanId, "parentSpanId is required");
-    final Span span = new Span(getTraceId(), parentSpanId, this);
+    final Span span = new Span(getTraceId(), parentSpanId, this, this.hub);
     this.spans.add(span);
     return span;
   }
@@ -153,6 +153,9 @@ Boolean isSampled() {
   @Override
   public void finish() {
     this.timestamp = DateUtils.getCurrentDateTime();
+    if (this.throwable != null) {
+      hub.setSpanContext(this.throwable, this.getSpanContext());
+    }
     this.hub.captureTransaction(this, null);
   }
 
@@ -235,27 +238,4 @@ Span getLatestActiveSpan() {
     }
     return null;
   }
-
-  /**
-   * Gets the span context for the span that was active while the throwable given by parameter was
-   * thrown.
-   *
-   * @param throwable - the throwable
-   * @return span context or {@code null} if no corresponding span context found.
-   */
-  public SpanContext getSpanContext(final @NotNull Throwable throwable) {
-    SpanContext context = null;
-    if (this.throwable == throwable) {
-      context = this.getSpanContext();
-    } else {
-      final List<Span> spans = new ArrayList<>(this.spans);
-      for (int i = spans.size() - 1; i >= 0; i--) {
-        if (throwable == spans.get(i).getThrowable()) {
-          context = spans.get(i);
-          break;
-        }
-      }
-    }
-    return context;
-  }
 }

sentry/src/main/java/io/sentry/Span.java

@@ -22,13 +22,17 @@ public final class Span extends SpanContext implements ISpan {
   /** A throwable thrown during the execution of the span. */
   private transient @Nullable Throwable throwable;
 
+  private final transient @NotNull IHub hub;
+
   Span(
       final @NotNull SentryId traceId,
       final @NotNull SpanId parentSpanId,
-      final @NotNull SentryTransaction transaction) {
+      final @NotNull SentryTransaction transaction,
+      final @NotNull IHub hub) {
     super(traceId, new SpanId(), parentSpanId, transaction.isSampled());
     this.transaction = Objects.requireNonNull(transaction, "transaction is required");
     this.startTimestamp = DateUtils.getCurrentDateTime();
+    this.hub = Objects.requireNonNull(hub, "hub is required");
   }
 
   public @NotNull Date getStartTimestamp() {
@@ -56,7 +60,10 @@ public SentryTraceHeader toSentryTrace() {
 
   @Override
   public void finish() {
-    this.timestamp = DateUtils.getCurrentDateTime();
+    timestamp = DateUtils.getCurrentDateTime();
+    if (throwable != null) {
+      hub.setSpanContext(throwable, this);
+    }
   }
 
   @Override

sentry/src/test/java/io/sentry/HubTest.kt

@@ -1059,6 +1059,24 @@ class HubTest {
     }
     // endregion
 
+    //region setSpanContext
+    @Test
+    fun `associates span context with throwable`() {
+        val hub = generateHub()
+        val transaction = hub.startTransaction("aTransaction")
+        val span = transaction.startChild()
+        val exception = RuntimeException()
+        hub.setSpanContext(exception, span)
+        assertEquals(span, hub.getSpanContext(exception))
+    }
+
+    @Test
+    fun `returns null when no span context associated with throwable`() {
+        val hub = generateHub()
+        assertNull(hub.getSpanContext(RuntimeException()))
+    }
+    // endregion
+
     private fun generateHub(): IHub {
         val options = SentryOptions().apply {
             dsn = "https://key@sentry.io/proj"

sentry/src/test/java/io/sentry/NoOpHubTest.kt

@@ -1,10 +1,12 @@
 package io.sentry
 
+import com.nhaarman.mockitokotlin2.mock
 import io.sentry.protocol.SentryId
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
+import kotlin.test.assertNull
 import kotlin.test.assertSame
 
 class NoOpHubTest {
@@ -76,4 +78,17 @@ class NoOpHubTest {
     fun `traceHeaders is not null`() {
         assertNotNull(sut.traceHeaders())
     }
+
+    @Test
+    fun `getSpan returns null`() {
+        assertNull(sut.span)
+    }
+
+    @Test
+    fun `getSpanContext returns null`() {
+        assertNull(sut.getSpanContext(RuntimeException()))
+    }
+
+    @Test
+    fun `setSpanContext doesnt throw`() = sut.setSpanContext(RuntimeException(), mock())
 }