Merge remote-tracking branch 'origin/main' into gh-981

CHANGELOG.md

@@ -1,5 +1,7 @@
 # vNext
 
+* 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)
 
 # 4.0.0-alpha.1

sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java

@@ -10,6 +10,7 @@
 import java.util.Locale;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /** Class responsible for reading values from manifest and setting them to the options */
 final class ManifestMetadataReader {
@@ -44,6 +45,8 @@ final class ManifestMetadataReader {
   static final String UNCAUGHT_EXCEPTION_HANDLER_ENABLE =
       "io.sentry.uncaught-exception-handler.enable";
 
+  static final String ATTACH_THREADS = "io.sentry.attach-threads";
+
   /** ManifestMetadataReader ctor */
   private ManifestMetadataReader() {}
 
@@ -185,6 +188,11 @@ static void applyMetadata(
                 UNCAUGHT_EXCEPTION_HANDLER_ENABLE, options.isEnableUncaughtExceptionHandler());
         options.getLogger().log(SentryLevel.DEBUG, "enableUncaughtExceptionHandler read: %s", ndk);
         options.setEnableUncaughtExceptionHandler(enableUncaughtExceptionHandler);
+
+        final boolean attachThreads =
+            metadata.getBoolean(ATTACH_THREADS, options.isAttachThreads());
+        options.getLogger().log(SentryLevel.DEBUG, "attachThreads read: %s", attachThreads);
+        options.setAttachThreads(attachThreads);
       }
       options
           .getLogger()
@@ -228,7 +236,7 @@ static boolean isAutoInit(final @NotNull Context context, final @NotNull ILogger
    * @return the Bundle attached to the PackageManager
    * @throws PackageManager.NameNotFoundException if the package name is non-existent
    */
-  private static Bundle getMetadata(final @NotNull Context context)
+  private static @Nullable Bundle getMetadata(final @NotNull Context context)
       throws PackageManager.NameNotFoundException {
     final ApplicationInfo app =
         context

sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt

@@ -381,4 +381,33 @@ class ManifestMetadataReaderTest {
         // Assert
         assertTrue(options.isEnableUncaughtExceptionHandler)
     }
+
+    @Test
+    fun `applyMetadata reads attachThreads to options`() {
+        // Arrange
+        val options = SentryAndroidOptions()
+        val bundle = Bundle()
+        val mockContext = ContextUtilsTest.mockMetaData(metaData = bundle)
+        bundle.putBoolean(ManifestMetadataReader.ATTACH_THREADS, true)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(mockContext, options)
+
+        // Assert
+        assertTrue(options.isAttachThreads)
+    }
+
+    @Test
+    fun `applyMetadata reads attachThreads and keep default value if not found`() {
+        // Arrange
+        val options = SentryAndroidOptions()
+        val bundle = Bundle()
+        val mockContext = ContextUtilsTest.mockMetaData(metaData = bundle)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(mockContext, options)
+
+        // Assert
+        assertFalse(options.isAttachThreads)
+    }
 }

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentrySpanClientHttpRequestInterceptor.java

@@ -11,7 +11,6 @@
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.springframework.core.NamedThreadLocal;
@@ -37,7 +36,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
       @NotNull byte[] body,
       @NotNull ClientHttpRequestExecution execution)
       throws IOException {
-    final ISpan activeSpan = resolveActiveSpan();
+    final ISpan activeSpan = hub.getSpan();
     if (activeSpan == null) {
       return execution.execute(request, body);
     }
@@ -61,22 +60,6 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) {
     }
   }
 
-  // TODO: this method ideally gets extracted or moves to Hub itself
-  private @Nullable ISpan resolveActiveSpan() {
-    final AtomicReference<ISpan> spanRef = new AtomicReference<>();
-
-    hub.configureScope(
-        scope -> {
-          final ISpan span = scope.getSpan();
-
-          if (span != null) {
-            spanRef.set(span);
-          }
-        });
-
-    return spanRef.get();
-  }
-
   UriTemplateHandler createUriTemplateHandler(final @NotNull UriTemplateHandler delegate) {
     return new UriTemplateHandler() {
 

sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt

@@ -1,11 +1,9 @@
 package io.sentry.spring.boot
 
-import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.whenever
 import io.sentry.IHub
 import io.sentry.Scope
-import io.sentry.ScopeCallback
 import io.sentry.SentryOptions
 import io.sentry.SentryTransaction
 import io.sentry.SpanContext
@@ -35,9 +33,7 @@ class SentrySpanRestTemplateCustomizerTest {
             if (isTransactionActive) {
                 val scope = Scope(SentryOptions())
                 scope.setTransaction(transaction)
-                whenever(hub.configureScope(any())).thenAnswer {
-                    (it.arguments[0] as ScopeCallback).run(scope)
-                }
+                whenever(hub.span).thenReturn(transaction)
 
                 mockServer.expect(MockRestRequestMatchers.requestTo("/test/123"))
                     .andExpect(MockRestRequestMatchers.method(HttpMethod.GET))

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

@@ -8,7 +8,6 @@
 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;
@@ -27,8 +26,6 @@
 @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) {
@@ -49,15 +46,14 @@ 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);
+
     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);
       }
-      // connects the event with transaction
-      event.setTransaction(transactionNameProvider.provideTransactionName(request));
     }
     hub.captureEvent(event);
 

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

@@ -3,6 +3,7 @@
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.EventProcessor;
 import io.sentry.SentryEvent;
+import io.sentry.spring.tracing.TransactionNameProvider;
 import io.sentry.util.Objects;
 import javax.servlet.http.HttpServletRequest;
 import org.jetbrains.annotations.NotNull;
@@ -13,6 +14,8 @@
 public class SentryRequestHttpServletRequestProcessor implements EventProcessor {
   private final @NotNull HttpServletRequest request;
   private final @NotNull SentryRequestResolver sentryRequestResolver;
+  private final @NotNull TransactionNameProvider transactionNameProvider =
+      new TransactionNameProvider();
 
   public SentryRequestHttpServletRequestProcessor(
       final @NotNull HttpServletRequest request,
@@ -26,6 +29,9 @@ public SentryRequestHttpServletRequestProcessor(
   public @NotNull SentryEvent process(
       final @NotNull SentryEvent event, final @Nullable Object hint) {
     event.setRequest(sentryRequestResolver.resolveSentryRequest(request));
+    if (event.getTransaction() == null) {
+      event.setTransaction(transactionNameProvider.provideTransactionName(request));
+    }
     return event;
   }
 }

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

@@ -6,7 +6,6 @@
 import io.sentry.SpanStatus;
 import io.sentry.util.Objects;
 import java.lang.reflect.Method;
-import java.util.concurrent.atomic.AtomicReference;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.jetbrains.annotations.NotNull;
@@ -29,7 +28,7 @@ public SentrySpanAdvice(final @NotNull IHub hub) {
 
   @Override
   public Object invoke(final @NotNull MethodInvocation invocation) throws Throwable {
-    final ISpan activeSpan = resolveActiveSpan();
+    final ISpan activeSpan = hub.getSpan();
 
     if (activeSpan == null) {
       // there is no active transaction, we do not start new span
@@ -66,19 +65,4 @@ private String resolveSpanDescription(
         ? targetClass.getSimpleName() + "." + method.getName()
         : sentrySpan.value();
   }
-
-  private @Nullable ISpan resolveActiveSpan() {
-    final AtomicReference<ISpan> spanRef = new AtomicReference<>();
-
-    hub.configureScope(
-        scope -> {
-          final ISpan span = scope.getSpan();
-
-          if (span != null) {
-            spanRef.set(span);
-          }
-        });
-
-    return spanRef.get();
-  }
 }

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

@@ -6,11 +6,13 @@ import java.net.URI
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 import org.springframework.http.MediaType
 import org.springframework.mock.web.MockServletContext
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders
+import org.springframework.web.servlet.HandlerMapping
 
 class SentryRequestHttpServletRequestProcessorTest {
 
@@ -108,4 +110,35 @@ class SentryRequestHttpServletRequestProcessorTest {
         assertFalse(event.request.headers.containsKey("Cookie"))
         assertTrue(event.request.headers.containsKey("some-header"))
     }
+
+    @Test
+    fun `when event does not have transaction name, sets the transaction name from the current request`() {
+        val request = MockMvcRequestBuilders
+            .get(URI.create("http://example.com?param1=xyz"))
+            .requestAttr(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/some-path")
+            .buildRequest(MockServletContext())
+        val eventProcessor = SentryRequestHttpServletRequestProcessor(request, SentryRequestResolver(SentryOptions()))
+        val event = SentryEvent()
+
+        eventProcessor.process(event, null)
+
+        assertNotNull(event.transaction)
+        assertEquals("GET /some-path", event.transaction)
+    }
+
+    @Test
+    fun `when event has transaction name set, does not overwrite transaction name with value from the current request`() {
+        val request = MockMvcRequestBuilders
+            .get(URI.create("http://example.com?param1=xyz"))
+            .requestAttr(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/some-path")
+            .buildRequest(MockServletContext())
+        val eventProcessor = SentryRequestHttpServletRequestProcessor(request, SentryRequestResolver(SentryOptions()))
+        val event = SentryEvent()
+        event.transaction = "some-transaction"
+
+        eventProcessor.process(event, null)
+
+        assertNotNull(event.transaction)
+        assertEquals("some-transaction", event.transaction)
+    }
 }

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

@@ -110,6 +110,19 @@ class SentrySpringIntegrationTest {
         }
     }
 
+    @Test
+    fun `attaches transaction name to events`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+
+        restTemplate.getForEntity("http://localhost:$port/throws", String::class.java)
+
+        await.untilAsserted {
+            verify(transport).send(checkEvent { event ->
+                assertThat(event.transaction).isEqualTo("GET /throws")
+            })
+        }
+    }
+
     @Test
     fun `does not send events for handled exceptions`() {
         val restTemplate = TestRestTemplate().withBasicAuth("user", "password")

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

@@ -5,7 +5,6 @@ import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.whenever
 import io.sentry.IHub
 import io.sentry.Scope
-import io.sentry.ScopeCallback
 import io.sentry.SentryOptions
 import io.sentry.SentryTransaction
 import io.sentry.SpanContext
@@ -50,9 +49,7 @@ class SentrySpanAdviceTest {
         val tx = SentryTransaction("aTransaction", SpanContext(), hub)
         scope.setTransaction(tx)
 
-        whenever(hub.configureScope(any())).thenAnswer {
-            (it.arguments[0] as ScopeCallback).run(scope)
-        }
+        whenever(hub.span).thenReturn(tx)
         val result = sampleService.methodWithSpanDescriptionSet()
         assertEquals(1, result)
         assertEquals(1, tx.spans.size)
@@ -66,9 +63,7 @@ class SentrySpanAdviceTest {
         val tx = SentryTransaction("aTransaction", SpanContext(), hub)
         scope.setTransaction(tx)
 
-        whenever(hub.configureScope(any())).thenAnswer {
-            (it.arguments[0] as ScopeCallback).run(scope)
-        }
+        whenever(hub.span).thenReturn(tx)
         val result = sampleService.methodWithoutSpanDescriptionSet()
         assertEquals(2, result)
         assertEquals(1, tx.spans.size)
@@ -82,9 +77,7 @@ class SentrySpanAdviceTest {
         val tx = SentryTransaction("aTransaction", SpanContext(), hub)
         scope.setTransaction(tx)
 
-        whenever(hub.configureScope(any())).thenAnswer {
-            (it.arguments[0] as ScopeCallback).run(scope)
-        }
+        whenever(hub.span).thenReturn(tx)
         sampleService.methodWithSpanDescriptionSet()
         assertEquals(SpanStatus.OK, tx.spans.first().status)
     }
@@ -95,9 +88,7 @@ class SentrySpanAdviceTest {
         val tx = SentryTransaction("aTransaction", SpanContext(), hub)
         scope.setTransaction(tx)
 
-        whenever(hub.configureScope(any())).thenAnswer {
-            (it.arguments[0] as ScopeCallback).run(scope)
-        }
+        whenever(hub.span).thenReturn(tx)
         var throwable: Throwable? = null
         try {
             sampleService.methodThrowingException()
@@ -111,9 +102,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when method is annotated with @SentrySpan and there is no active transaction, span is not created and method is executed`() {
         val scope = Scope(SentryOptions())
-        whenever(hub.configureScope(any())).thenAnswer {
-            (it.arguments[0] as ScopeCallback).run(scope)
-        }
+        whenever(hub.span).thenReturn(null)
         val result = sampleService.methodWithSpanDescriptionSet()
         assertEquals(1, result)
     }

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

@@ -735,4 +735,22 @@ public void flush(long timeoutMillis) {
     }
     return traceHeader;
   }
+
+  @Override
+  public @Nullable ISpan getSpan() {
+    ISpan span = null;
+    if (!isEnabled()) {
+      options
+          .getLogger()
+          .log(SentryLevel.WARNING, "Instance is disabled and this 'getSpan' call is a no-op.");
+    } else {
+      final StackItem item = stack.peek();
+      if (item != null) {
+        span = item.scope.getSpan();
+      } else {
+        options.getLogger().log(SentryLevel.FATAL, "Stack peek was null when getSpan");
+      }
+    }
+    return span;
+  }
 }