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

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+* Feat: Add an overload for `startTransaction` that sets the created transaction to the Scope #1313
+* Ref: Separate user facing and protocol classes in the Performance feature (#1304)
+
 # 4.3.0
 
 * Fix: Fix setting in-app-includes from external properties (#1291)

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

@@ -15,8 +15,10 @@ import io.sentry.Hub
 import io.sentry.Scope
 import io.sentry.SentryLevel
 import io.sentry.SentryOptions
-import io.sentry.SentryTransaction
+import io.sentry.SentryTracer
 import io.sentry.SpanStatus
+import io.sentry.TransactionContext
+import io.sentry.protocol.SentryTransaction
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -31,7 +33,7 @@ class ActivityLifecycleIntegrationTest {
         val options = SentryAndroidOptions()
         val activity = mock<Activity>()
         val bundle = mock<Bundle>()
-        val transaction = SentryTransaction("name", "op", hub)
+        val transaction = SentryTracer(TransactionContext("name", "op"), hub)
 
         fun getSut(): ActivityLifecycleIntegration {
             whenever(hub.startTransaction(any<String>(), any())).thenReturn(transaction)
@@ -256,7 +258,7 @@ class ActivityLifecycleIntegrationTest {
 
         whenever(fixture.hub.configureScope(any())).thenAnswer {
             val scope = Scope(fixture.options)
-            val previousTransaction = SentryTransaction("name", "op", fixture.hub)
+            val previousTransaction = SentryTracer(TransactionContext("name", "op"), fixture.hub)
             scope.setTransaction(previousTransaction)
 
             sut.applyScope(scope, fixture.transaction)
@@ -278,7 +280,7 @@ class ActivityLifecycleIntegrationTest {
 
         verify(fixture.hub).captureTransaction(check<SentryTransaction> {
             assertEquals(SpanStatus.OK, it.status)
-        }, eq(null))
+        })
     }
 
     @Test
@@ -295,7 +297,7 @@ class ActivityLifecycleIntegrationTest {
 
         verify(fixture.hub).captureTransaction(check<SentryTransaction> {
             assertEquals(SpanStatus.UNKNOWN_ERROR, it.status)
-        }, eq(null))
+        })
     }
 
     @Test
@@ -330,7 +332,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityPreCreated(fixture.activity, fixture.bundle)
         sut.onActivityDestroyed(fixture.activity)
 
-        verify(fixture.hub).captureTransaction(any<SentryTransaction>(), eq(null))
+        verify(fixture.hub).captureTransaction(any<SentryTransaction>())
     }
 
     @Test
@@ -366,6 +368,6 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityPreCreated(activity, mock())
 
         sut.onActivityPreCreated(fixture.activity, fixture.bundle)
-        verify(fixture.hub).captureTransaction(any<SentryTransaction>(), eq(null))
+        verify(fixture.hub).captureTransaction(any<SentryTransaction>())
     }
 }

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

@@ -5,9 +5,9 @@ import com.nhaarman.mockitokotlin2.whenever
 import io.sentry.IHub
 import io.sentry.Scope
 import io.sentry.SentryOptions
-import io.sentry.SentryTransaction
-import io.sentry.SpanContext
+import io.sentry.SentryTracer
 import io.sentry.SpanStatus
+import io.sentry.TransactionContext
 import kotlin.test.Test
 import org.assertj.core.api.Assertions.assertThat
 import org.springframework.http.HttpMethod
@@ -23,7 +23,7 @@ class SentrySpanRestTemplateCustomizerTest {
         val hub = mock<IHub>()
         val restTemplate = RestTemplate()
         var mockServer = MockRestServiceServer.createServer(restTemplate)
-        val transaction = SentryTransaction("aTransaction", SpanContext("op", true), hub)
+        val transaction = SentryTracer(TransactionContext("aTransaction", "op", true), hub)
         internal val customizer = SentrySpanRestTemplateCustomizer(hub)
 
         fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK): RestTemplate {

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

@@ -1,5 +1,6 @@
 package io.sentry.spring.tracing;
 
+import io.sentry.protocol.SentryTransaction;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -8,8 +9,8 @@
 
 /**
  * Makes annotated method execution or a method execution within a class annotated with {@link
- * SentrySpan} executed within running {@link io.sentry.SentryTransaction} to get wrapped into
- * {@link io.sentry.Span}.
+ * SentrySpan} executed within running {@link SentryTransaction} to get wrapped into {@link
+ * io.sentry.Span}.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD, ElementType.TYPE})

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

@@ -73,6 +73,11 @@ protected void doFilterInternal(
     if (hub.isEnabled()) {
       final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER);
 
+      hub.configureScope(
+          scope -> {
+            scope.setRequest(requestResolver.resolveSentryRequest(httpRequest));
+          });
+
       // at this stage we are not able to get real transaction name
       final ITransaction transaction = startTransaction(httpRequest, sentryTraceHeader);
       try {
@@ -86,7 +91,6 @@ protected void doFilterInternal(
         if (transactionName != null) {
           transaction.setName(transactionName);
           transaction.setOperation(TRANSACTION_OP);
-          transaction.setRequest(requestResolver.resolveSentryRequest(httpRequest));
           transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus()));
           transaction.finish();
         }
@@ -109,9 +113,7 @@ private ITransaction startTransaction(
         final TransactionContext contexts =
             TransactionContext.fromSentryTrace(
                 name, "http.server", new SentryTraceHeader(sentryTraceHeader));
-        final ITransaction transaction = hub.startTransaction(contexts, customSamplingContext);
-        hub.configureScope(scope -> scope.setTransaction(transaction));
-        return transaction;
+        return hub.startTransaction(contexts, customSamplingContext);
       } catch (InvalidSentryTraceHeaderException e) {
         hub.getOptions()
             .getLogger()

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

@@ -8,7 +8,7 @@
 
 /**
  * Makes annotated method execution or a method execution within an annotated class to get wrapped
- * into {@link io.sentry.SentryTransaction}.
+ * into {@link io.sentry.protocol.SentryTransaction}.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD, ElementType.TYPE})

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

@@ -2,11 +2,9 @@
 
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.IHub;
-import io.sentry.ISpan;
 import io.sentry.ITransaction;
 import io.sentry.util.Objects;
 import java.lang.reflect.Method;
-import java.util.concurrent.atomic.AtomicBoolean;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.jetbrains.annotations.ApiStatus;
@@ -58,8 +56,6 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
         operation = "bean";
       }
       final ITransaction transaction = hub.startTransaction(name, operation);
-      hub.configureScope(scope -> scope.setTransaction(transaction));
-
       try {
         return invocation.proceed();
       } finally {
@@ -78,16 +74,6 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
   }
 
   private boolean isTransactionActive() {
-    AtomicBoolean isTransactionActiveRef = new AtomicBoolean(false);
-
-    hub.configureScope(
-        scope -> {
-          ISpan span = scope.getSpan();
-
-          if (span != null) {
-            isTransactionActiveRef.set(true);
-          }
-        });
-    return isTransactionActiveRef.get();
+    return hub.getSpan() != null;
   }
 }

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

@@ -1,17 +1,14 @@
 package io.sentry.spring.tracing
 
-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.SentryOptions
-import io.sentry.SentryTransaction
-import io.sentry.SpanContext
+import io.sentry.SentryTracer
 import io.sentry.SpanStatus
 import io.sentry.TransactionContext
 import java.lang.RuntimeException
-import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNull
@@ -40,15 +37,10 @@ class SentrySpanAdviceTest {
     @Autowired
     lateinit var hub: IHub
 
-    @BeforeTest
-    fun setup() {
-        whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
-    }
-
     @Test
     fun `when class is annotated with @SentrySpan, every method call attaches span to existing transaction`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)
@@ -62,7 +54,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when class is annotated with @SentrySpan with operation set, every method call attaches span to existing transaction`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)
@@ -76,7 +68,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when method is annotated with @SentrySpan with properties set, attaches span to existing transaction`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)
@@ -90,7 +82,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when method is annotated with @SentrySpan without properties set, attaches span to existing transaction and sets Span description as className dot methodName`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)
@@ -104,7 +96,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when method is annotated with @SentrySpan and returns, attached span has status OK`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)
@@ -115,7 +107,7 @@ class SentrySpanAdviceTest {
     @Test
     fun `when method is annotated with @SentrySpan and throws exception, attached span has throwable set and INTERNAL_ERROR status`() {
         val scope = Scope(SentryOptions())
-        val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
+        val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
         scope.setTransaction(tx)
 
         whenever(hub.span).thenReturn(tx)

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

@@ -5,14 +5,16 @@ import com.nhaarman.mockitokotlin2.check
 import com.nhaarman.mockitokotlin2.eq
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.spy
+import com.nhaarman.mockitokotlin2.times
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions
 import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import com.nhaarman.mockitokotlin2.whenever
+import io.sentry.CustomSamplingContext
 import io.sentry.IHub
+import io.sentry.Scope
 import io.sentry.SentryOptions
-import io.sentry.SentryTransaction
-import io.sentry.SpanContext
+import io.sentry.SentryTracer
 import io.sentry.SpanId
 import io.sentry.SpanStatus
 import io.sentry.TransactionContext
@@ -47,24 +49,40 @@ class SentryTracingFilterTest {
             request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}")
             if (sentryTraceHeader != null) {
                 request.addHeader("sentry-trace", sentryTraceHeader)
-                whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTransaction((it.arguments[0] as TransactionContext).name, it.arguments[0] as SpanContext, hub) }
+                whenever(hub.startTransaction(any(), any<CustomSamplingContext>())).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) }
             }
             response.status = 200
-            whenever(hub.startTransaction(any<String>(), any(), any())).thenAnswer { SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
+            whenever(hub.startTransaction(any(), any(), any<CustomSamplingContext>())).thenAnswer { SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
             whenever(hub.isEnabled).thenReturn(isEnabled)
             return SentryTracingFilter(hub, sentryRequestResolver, transactionNameProvider)
         }
     }
 
     private val fixture = Fixture()
 
+    @Test
+    fun `sets the request on the scope`() {
+        val filter = fixture.getSut()
+
+        filter.doFilter(fixture.request, fixture.response, fixture.chain)
+
+        verify(fixture.chain).doFilter(fixture.request, fixture.response)
+        verify(fixture.hub, times(1)).configureScope(check {
+            val scope = mock<Scope>()
+            it.run(scope)
+            verify(scope).setRequest(check { request ->
+                assertThat(request.url).isEqualTo("http://localhost/product/12")
+            })
+        })
+    }
+
     @Test
     fun `creates transaction around the request`() {
         val filter = fixture.getSut()
 
         filter.doFilter(fixture.request, fixture.response, fixture.chain)
 
-        verify(fixture.hub).startTransaction(eq("POST /product/12"), eq("http.server"), check {
+        verify(fixture.hub).startTransaction(eq("POST /product/12"), eq("http.server"), check<CustomSamplingContext> {
             assertNotNull(it["request"])
             assertTrue(it["request"] is HttpServletRequest)
         })
@@ -73,8 +91,7 @@ class SentryTracingFilterTest {
             assertThat(it.transaction).isEqualTo("POST /product/{id}")
             assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
             assertThat(it.contexts.trace!!.operation).isEqualTo("http.server")
-            assertThat(it.request).isNotNull()
-        }, eq(null))
+        })
     }
 
     @Test
@@ -85,7 +102,7 @@ class SentryTracingFilterTest {
 
         verify(fixture.hub).captureTransaction(check {
             assertThat(it.contexts.trace!!.parentSpanId).isNull()
-        }, eq(null))
+        })
     }
 
     @Test
@@ -97,7 +114,7 @@ class SentryTracingFilterTest {
 
         verify(fixture.hub).captureTransaction(check {
             assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId)
-        }, eq(null))
+        })
     }
 
     @Test