Fix: set status and associate events with transactions (#1426)

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Fix: set correct transaction status for unhandled exceptions in SentryTracingFilter (#1406)
 * Fix: handle network errors in SentrySpanClientHttpRequestInterceptor (#1407)
 * Fix: set scope on transaction (#1409)
+* Fix: set status and associate events with transactions (#1426)
 * Fix: Do not set free memory and is low memory fields when it's a NDK hard crash (#1399)
 * Fix: Apply user from the scope to transaction (#1424)
 * Fix: Pass maxBreadcrumbs config. to sentry-native (#1425)

sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTransactionAdvice.java

@@ -3,6 +3,7 @@
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.IHub;
 import io.sentry.ITransaction;
+import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import java.lang.reflect.Method;
 import org.aopalliance.intercept.MethodInterceptor;
@@ -59,7 +60,13 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
       hub.pushScope();
       final ITransaction transaction = hub.startTransaction(name, operation, true);
       try {
-        return invocation.proceed();
+        final Object result = invocation.proceed();
+        transaction.setStatus(SpanStatus.OK);
+        return result;
+      } catch (Exception e) {
+        transaction.setStatus(SpanStatus.INTERNAL_ERROR);
+        transaction.setThrowable(e);
+        throw e;
       } finally {
         transaction.finish();
         hub.popScope();

sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt

@@ -7,11 +7,17 @@ import com.nhaarman.mockitokotlin2.reset
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import com.nhaarman.mockitokotlin2.whenever
+import io.sentry.IHub
 import io.sentry.ITransportFactory
 import io.sentry.Sentry
+import io.sentry.SentryOptions
+import io.sentry.SpanStatus
+import io.sentry.spring.tracing.SentryTracingConfiguration
+import io.sentry.spring.tracing.SentryTransaction
 import io.sentry.test.checkEvent
+import io.sentry.test.checkTransaction
 import io.sentry.transport.ITransport
-import java.lang.RuntimeException
+import java.lang.Exception
 import java.time.Duration
 import org.assertj.core.api.Assertions.assertThat
 import org.awaitility.kotlin.await
@@ -25,6 +31,7 @@ import org.springframework.boot.test.web.client.TestRestTemplate
 import org.springframework.boot.web.server.LocalServerPort
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
+import org.springframework.context.annotation.Import
 import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpMethod
@@ -37,6 +44,7 @@ import org.springframework.security.core.userdetails.UserDetailsService
 import org.springframework.security.crypto.factory.PasswordEncoderFactories
 import org.springframework.security.crypto.password.PasswordEncoder
 import org.springframework.security.provisioning.InMemoryUserDetailsManager
+import org.springframework.stereotype.Service
 import org.springframework.test.context.junit4.SpringRunner
 import org.springframework.web.bind.annotation.ControllerAdvice
 import org.springframework.web.bind.annotation.ExceptionHandler
@@ -53,6 +61,12 @@ class SentrySpringIntegrationTest {
     @Autowired
     lateinit var transport: ITransport
 
+    @Autowired
+    lateinit var someService: SomeService
+
+    @Autowired
+    lateinit var hub: IHub
+
     @LocalServerPort
     lateinit var port: Integer
 
@@ -137,10 +151,51 @@ class SentrySpringIntegrationTest {
             verifyZeroInteractions(transport)
         }
     }
+
+    @Test
+    fun `calling a method annotated with @SentryTransaction creates transaction`() {
+        someService.aMethod()
+        await.untilAsserted {
+            verify(transport).send(checkTransaction {
+                assertThat(it.status).isEqualTo(SpanStatus.OK)
+            }, anyOrNull())
+        }
+    }
+
+    @Test
+    fun `calling a method annotated with @SentryTransaction throwing exception associates Sentry event with transaction`() {
+        try {
+            someService.aMethodThrowing()
+        } catch (e: Exception) {
+            hub.captureException(e)
+        }
+        await.untilAsserted {
+            verify(transport).send(checkEvent {
+                assertThat(it.contexts.trace).isNotNull
+                assertThat(it.contexts.trace!!.operation).isEqualTo("bean")
+            }, anyOrNull())
+        }
+    }
+
+    @Test
+    fun `calling a method annotated with @SentryTransaction, where an inner span is created within transaction, throwing exception associates Sentry event with inner span`() {
+        try {
+            someService.aMethodWithInnerSpanThrowing()
+        } catch (e: Exception) {
+            hub.captureException(e)
+        }
+        await.untilAsserted {
+            verify(transport).send(checkEvent {
+                assertThat(it.contexts.trace).isNotNull
+                assertThat(it.contexts.trace!!.operation).isEqualTo("child-op")
+            }, anyOrNull())
+        }
+    }
 }
 
 @SpringBootApplication
 @EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true)
+@Import(SentryTracingConfiguration::class)
 open class App {
 
     private val transport = mock<ITransport>()
@@ -157,6 +212,37 @@ open class App {
 
     @Bean
     open fun sentrySpringRequestListener() = SentrySpringRequestListener()
+
+    @Bean
+    open fun tracesSamplerCallback() = SentryOptions.TracesSamplerCallback {
+        1.0
+    }
+}
+
+@Service
+open class SomeService {
+
+    @SentryTransaction(operation = "bean")
+    open fun aMethod() { Thread.sleep(100) }
+
+    @SentryTransaction(operation = "bean")
+    open fun aMethodThrowing() {
+        throw RuntimeException("oops")
+    }
+
+    @SentryTransaction(operation = "bean")
+    open fun aMethodWithInnerSpanThrowing() {
+        val span = Sentry.getSpan()!!.startChild("child-op")
+        try {
+            throw RuntimeException("oops")
+        } catch (e: Exception) {
+            span.status = SpanStatus.INTERNAL_ERROR
+            span.throwable = e
+            throw e
+        } finally {
+            span.finish()
+        }
+    }
 }
 
 @RestController

sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTransactionAdviceTest.kt

@@ -12,10 +12,12 @@ import io.sentry.IHub
 import io.sentry.Scope
 import io.sentry.ScopeCallback
 import io.sentry.SentryOptions
+import io.sentry.SpanStatus
 import io.sentry.TransactionContext
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.assertThrows
 import org.junit.runner.RunWith
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.context.annotation.Bean
@@ -53,6 +55,15 @@ class SentryTransactionAdviceTest {
         verify(hub).captureTransaction(check {
             assertThat(it.transaction).isEqualTo("customName")
             assertThat(it.contexts.trace!!.operation).isEqualTo("bean")
+            assertThat(it.status).isEqualTo(SpanStatus.OK)
+        })
+    }
+
+    @Test
+    fun `when method annotated with @SentryTransaction throws exception, sets error status on transaction`() {
+        assertThrows<RuntimeException> { sampleService.methodThrowingException() }
+        verify(hub).captureTransaction(check {
+            assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
         })
     }
 
@@ -133,6 +144,9 @@ class SentryTransactionAdviceTest {
 
         @SentryTransaction(operation = "op")
         open fun methodWithoutTransactionNameSet() = Unit
+
+        @SentryTransaction(operation = "op")
+        open fun methodThrowingException(): Nothing = throw RuntimeException()
     }
 
     @SentryTransaction(operation = "op")

sentry-test-support/api/sentry-test-support.api

@@ -1,4 +1,5 @@
 public final class io/sentry/test/AssertionsKt {
 	public static final fun checkEvent (Lkotlin/jvm/functions/Function1;)Lio/sentry/SentryEnvelope;
+	public static final fun checkTransaction (Lkotlin/jvm/functions/Function1;)Lio/sentry/SentryEnvelope;
 }
 

sentry-test-support/src/main/kotlin/io/sentry/test/assertions.kt

@@ -1,20 +1,39 @@
 package io.sentry.test
-
 import com.nhaarman.mockitokotlin2.check
 import io.sentry.GsonSerializer
 import io.sentry.SentryEnvelope
 import io.sentry.SentryEvent
 import io.sentry.SentryOptions
+import io.sentry.protocol.SentryTransaction
+import java.lang.AssertionError
 
 /**
  * Verifies is [SentryEnvelope] contains first event matching a predicate.
  */
-inline fun checkEvent(noinline predicate: (SentryEvent) -> Unit): SentryEnvelope {
+fun checkEvent(predicate: (SentryEvent) -> Unit): SentryEnvelope {
+    val options = SentryOptions().apply {
+        setSerializer(GsonSerializer(SentryOptions()))
+    }
+    return check {
+        val event: SentryEvent? = it.items.first().getEvent(options.serializer)
+        if (event != null) {
+            predicate(event)
+        } else {
+            throw AssertionError("event is null")
+        }
+    }
+}
+
+fun checkTransaction(predicate: (SentryTransaction) -> Unit): SentryEnvelope {
     val options = SentryOptions().apply {
         setSerializer(GsonSerializer(SentryOptions()))
     }
     return check {
-        val event = it.items.first().getEvent(options.serializer)!!
-        predicate(event)
+        val transaction = it.items.first().getTransaction(options.serializer)
+        if (transaction != null) {
+            predicate(transaction)
+        } else {
+            throw AssertionError("transaction is null")
+        }
     }
 }

sentry/api/sentry.api

@@ -127,7 +127,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/ISpan;)V
+	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)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
@@ -165,7 +165,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/ISpan;)V
+	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)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
@@ -218,7 +218,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/ISpan;)V
+	public abstract fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)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
@@ -368,7 +368,7 @@ public final class io/sentry/NoOpHub : 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/ISpan;)V
+	public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)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
@@ -1871,6 +1871,12 @@ public final class io/sentry/util/Objects {
 	public static fun requireNonNull (Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;
 }
 
+public final class io/sentry/util/Pair {
+	public fun <init> (Ljava/lang/Object;Ljava/lang/Object;)V
+	public fun getFirst ()Ljava/lang/Object;
+	public fun getSecond ()Ljava/lang/Object;
+}
+
 public final class io/sentry/util/StringUtils {
 	public static fun capitalize (Ljava/lang/String;)Ljava/lang/String;
 	public static fun getStringAfterDot (Ljava/lang/String;)Ljava/lang/String;

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

@@ -7,6 +7,7 @@
 import io.sentry.protocol.SentryTransaction;
 import io.sentry.protocol.User;
 import io.sentry.util.Objects;
+import io.sentry.util.Pair;
 import java.io.Closeable;
 import java.util.Collections;
 import java.util.List;
@@ -22,7 +23,7 @@ public final class Hub implements IHub {
   private volatile boolean isEnabled;
   private final @NotNull Stack stack;
   private final @NotNull TracesSampler tracesSampler;
-  private final @NotNull Map<Throwable, ISpan> throwableToSpan =
+  private final @NotNull Map<Throwable, Pair<ISpan, String>> throwableToSpan =
       Collections.synchronizedMap(new WeakHashMap<>());
 
   public Hub(final @NotNull SentryOptions options) {
@@ -174,10 +175,13 @@ public SentryId captureEnvelope(
 
   private void assignTraceContext(final @NotNull SentryEvent event) {
     if (event.getThrowable() != null) {
-      final ISpan span = throwableToSpan.get(event.getThrowable());
-      if (span != null) {
+      final Pair<ISpan, String> pair = throwableToSpan.get(event.getThrowable());
+      if (pair != null) {
         if (event.getContexts().getTrace() == null) {
-          event.getContexts().setTrace(span.getSpanContext());
+          event.getContexts().setTrace(pair.getFirst().getSpanContext());
+        }
+        if (event.getTransaction() == null) {
+          event.setTransaction(pair.getSecond());
         }
       }
     }
@@ -632,18 +636,25 @@ public void flush(long timeoutMillis) {
 
   @Override
   @ApiStatus.Internal
-  public void setSpanContext(final @NotNull Throwable throwable, final @NotNull ISpan span) {
+  public void setSpanContext(
+      final @NotNull Throwable throwable,
+      final @NotNull ISpan span,
+      final @NotNull String transactionName) {
     Objects.requireNonNull(throwable, "throwable is required");
     Objects.requireNonNull(span, "span is required");
-    this.throwableToSpan.put(throwable, span);
+    Objects.requireNonNull(transactionName, "transactionName is required");
+    // the most inner span should be assigned to a throwable
+    if (!throwableToSpan.containsKey(throwable)) {
+      throwableToSpan.put(throwable, new Pair<>(span, transactionName));
+    }
   }
 
   @Nullable
   SpanContext getSpanContext(final @NotNull Throwable throwable) {
     Objects.requireNonNull(throwable, "throwable is required");
-    final ISpan span = this.throwableToSpan.get(throwable);
+    final Pair<ISpan, String> span = this.throwableToSpan.get(throwable);
     if (span != null) {
-      return span.getSpanContext();
+      return span.getFirst().getSpanContext();
     }
     return null;
   }

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

@@ -178,8 +178,11 @@ public IHub clone() {
   }
 
   @Override
-  public void setSpanContext(final @NotNull Throwable t, final @NotNull ISpan sc) {
-    Sentry.getCurrentHub().setSpanContext(t, sc);
+  public void setSpanContext(
+      final @NotNull Throwable throwable,
+      final @NotNull ISpan span,
+      final @NotNull String transactionName) {
+    Sentry.getCurrentHub().setSpanContext(throwable, span, transactionName);
   }
 
   @Override