Merge branch 'main' into feat/scopes-chain-debug-util

Author: adinauer

CHANGELOG.md

@@ -21,6 +21,17 @@
     - `SentryAttribute.stringAttribute()` takes a `String` value
   - We opted for handling parameters via `SentryLogParameters` to avoid creating tons of overloads that are ambiguous.
 
+### Fixes
+
+- Isolation scope is now forked in `OtelSentrySpanProcessor` instead of `OtelSentryPropagator` ([#4434](https://github.com/getsentry/sentry-java/pull/4434))
+  - Since propagator may never be invoked we moved the location where isolation scope is forked.
+  - Not invoking `OtelSentryPropagator.extract` or having a `sentry-trace` header that failed to parse would cause isolation scope not to be forked.
+  - This in turn caused data to bleed between scopes, e.g. from one request into another
+
+### Dependencies
+
+- Bump Spring Boot to `3.5.0` ([#4111](https://github.com/getsentry/sentry-java/pull/4111))
+
 ## 8.12.0
 
 ### Features

buildSrc/src/main/java/Config.kt

@@ -6,7 +6,7 @@ object Config {
     val kotlinStdLib = "stdlib-jdk8"
 
     val springBootVersion = "2.7.18"
-    val springBoot3Version = "3.4.2"
+    val springBoot3Version = "3.5.0"
     val kotlinCompatibleLanguageVersion = "1.6"
 
     val androidComposeCompilerVersion = "1.5.14"

gradle/libs.versions.toml

@@ -9,7 +9,7 @@ kotlin = "1.9.24"
 # see https://developer.android.com/jetpack/androidx/releases/compose-kotlin
 okhttp = "4.9.2"
 springTwo = "2.7.18"
-springThree = "3.4.2"
+springThree = "3.5.0"
 targetSdk = "34"
 compileSdk = "34"
 minSdk = "21"

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java

@@ -1,7 +1,5 @@
 package io.sentry.opentelemetry;
 
-import static io.sentry.opentelemetry.SentryOtelKeys.SENTRY_SCOPES_KEY;
-
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
@@ -15,7 +13,6 @@
 import io.sentry.BaggageHeader;
 import io.sentry.IScopes;
 import io.sentry.ScopesAdapter;
-import io.sentry.Sentry;
 import io.sentry.SentryLevel;
 import io.sentry.SentryOptions;
 import io.sentry.SentryTraceHeader;
@@ -105,16 +102,10 @@ public <C> void inject(final Context context, final C carrier, final TextMapSett
   @Override
   public <C> Context extract(
       final Context context, final C carrier, final TextMapGetter<C> getter) {
-    final @Nullable IScopes scopesFromParentContext = context.get(SENTRY_SCOPES_KEY);
-    final @NotNull IScopes scopesToUse =
-        scopesFromParentContext != null
-            ? scopesFromParentContext.forkedScopes("propagator")
-            : Sentry.forkedRootScopes("propagator");
-
     final @Nullable String sentryTraceString =
         getter.get(carrier, SentryTraceHeader.SENTRY_TRACE_HEADER);
     if (sentryTraceString == null) {
-      return context.with(SENTRY_SCOPES_KEY, scopesToUse);
+      return context;
     }
 
     try {
@@ -136,7 +127,6 @@ public <C> Context extract(
       final @NotNull Context modifiedContext =
           context
               .with(wrappedSpan)
-              .with(SENTRY_SCOPES_KEY, scopesToUse)
               .with(SentryOtelKeys.SENTRY_TRACE_KEY, sentryTraceHeader)
               .with(SentryOtelKeys.SENTRY_BAGGAGE_KEY, baggage);
 

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java

@@ -10,6 +10,7 @@
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.data.EventData;
 import io.opentelemetry.sdk.trace.data.ExceptionEventData;
+import io.opentelemetry.sdk.trace.data.SpanData;
 import io.sentry.Baggage;
 import io.sentry.DateUtils;
 import io.sentry.IScopes;
@@ -48,11 +49,7 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri
       return;
     }
 
-    final @Nullable IScopes scopesFromContext = parentContext.get(SENTRY_SCOPES_KEY);
-    final @NotNull IScopes scopes =
-        scopesFromContext != null
-            ? scopesFromContext.forkedCurrentScope("spanprocessor")
-            : Sentry.forkedRootScopes("spanprocessor");
+    final @NotNull IScopes scopes = forkScopes(parentContext, otelSpan.toSpanData());
 
     final @Nullable IOtelSpanWrapper sentryParentSpan =
         spanStorage.getSentrySpan(otelSpan.getParentSpanContext());
@@ -111,6 +108,22 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri
     spanStorage.storeSentrySpan(spanContext, sentrySpan);
   }
 
+  private IScopes forkScopes(final @NotNull Context context, final @NotNull SpanData span) {
+    final @Nullable IScopes scopesFromContext = context.get(SENTRY_SCOPES_KEY);
+    if (scopesFromContext == null) {
+      return Sentry.forkedRootScopes("spanprocessor.new");
+    }
+    if (isRootSpan(span)) {
+      return scopesFromContext.forkedScopes("spanprocessor.rootspan");
+    }
+
+    return scopesFromContext.forkedCurrentScope("spanprocessor.nonrootspan");
+  }
+
+  private boolean isRootSpan(SpanData otelSpan) {
+    return !otelSpan.getParentSpanContext().isValid() || otelSpan.getParentSpanContext().isRemote();
+  }
+
   private @Nullable Boolean isSampled(
       final @NotNull ReadWriteSpan otelSpan,
       final @Nullable TracesSamplingDecision samplingDecision) {

sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OtelSentryPropagatorTest.kt

@@ -47,62 +47,6 @@ class OtelSentryPropagatorTest {
         assertEquals(listOf("sentry-trace", "baggage"), propagator.fields())
     }
 
-    @Test
-    fun `forks root scopes if none in context without headers`() {
-        val propagator = OtelSentryPropagator()
-        val carrier: Map<String, String> = mapOf()
-
-        val newContext = propagator.extract(Context.root(), carrier, MapGetter())
-
-        val scopes = newContext.get(SENTRY_SCOPES_KEY)
-        assertNotNull(scopes)
-        assertSame(Sentry.forkedRootScopes("test").parentScopes, scopes.parentScopes)
-    }
-
-    @Test
-    fun `forks scopes from context if present without headers`() {
-        val propagator = OtelSentryPropagator()
-        val carrier: Map<String, String> = mapOf()
-        val scopeInContext = Sentry.forkedRootScopes("test")
-
-        val newContext = propagator.extract(Context.root().with(SENTRY_SCOPES_KEY, scopeInContext), carrier, MapGetter())
-
-        val scopes = newContext.get(SENTRY_SCOPES_KEY)
-        assertNotNull(scopes)
-        assertSame(scopeInContext, scopes.parentScopes)
-    }
-
-    @Test
-    fun `forks root scopes if none in context with headers`() {
-        val propagator = OtelSentryPropagator()
-        val carrier: Map<String, String> = mapOf(
-            "sentry-trace" to "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1",
-            "baggage" to "sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d"
-        )
-
-        val newContext = propagator.extract(Context.root(), carrier, MapGetter())
-
-        val scopes = newContext.get(SENTRY_SCOPES_KEY)
-        assertNotNull(scopes)
-        assertSame(Sentry.forkedRootScopes("test").parentScopes, scopes.parentScopes)
-    }
-
-    @Test
-    fun `forks scopes from context if present with headers`() {
-        val propagator = OtelSentryPropagator()
-        val carrier: Map<String, String> = mapOf(
-            "sentry-trace" to "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1",
-            "baggage" to "sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d"
-        )
-        val scopeInContext = Sentry.forkedRootScopes("test")
-
-        val newContext = propagator.extract(Context.root().with(SENTRY_SCOPES_KEY, scopeInContext), carrier, MapGetter())
-
-        val scopes = newContext.get(SENTRY_SCOPES_KEY)
-        assertNotNull(scopes)
-        assertSame(scopeInContext, scopes.parentScopes)
-    }
-
     @Test
     fun `invalid sentry trace header returns context without modification`() {
         val propagator = OtelSentryPropagator()

sentry-samples/sentry-samples-spring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/graphql/TaskCreatorController.java

@@ -20,6 +20,7 @@ public TaskCreatorController(final BatchLoaderRegistry batchLoaderRegistry) {
     // using mapped BatchLoader to not have to deal with correct ordering of items
     batchLoaderRegistry
         .forTypePair(String.class, ProjectController.Creator.class)
+        .withOptions((builder) -> builder.setBatchingEnabled(true))
         .registerMappedBatchLoader(
             (Set<String> keys, BatchLoaderEnvironment env) -> {
               return Mono.fromCallable(

sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/graphql/SentryBatchLoaderRegistry.java

@@ -76,7 +76,7 @@ public BatchLoaderRegistry.RegistrationSpec<K, V> withName(String name) {
 
     @Override
     public BatchLoaderRegistry.RegistrationSpec<K, V> withOptions(
-        Consumer<DataLoaderOptions> optionsConsumer) {
+        Consumer<DataLoaderOptions.Builder> optionsConsumer) {
       return delegate.withOptions(optionsConsumer);
     }
 

sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/graphql/SentrySpringSubscriptionHandlerTest.kt

@@ -1,14 +1,14 @@
-package io.sentry.spring.graphql
+package io.sentry.spring.jakarta.graphql
 
 import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
 import graphql.language.Document
 import graphql.language.OperationDefinition
 import graphql.schema.DataFetchingEnvironment
 import io.sentry.IScopes
 import io.sentry.graphql.ExceptionReporter
-import io.sentry.spring.jakarta.graphql.SentrySpringSubscriptionHandler
 import org.junit.jupiter.api.assertThrows
 import org.mockito.kotlin.anyOrNull
+import org.mockito.kotlin.check
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.same
 import org.mockito.kotlin.verify
@@ -40,10 +40,10 @@ class SentrySpringSubscriptionHandlerTest {
 
         verify(exceptionReporter).captureThrowable(
             same(exception),
-            org.mockito.kotlin.check {
+            check {
                 assertEquals(true, it.isSubscription)
                 assertSame(scopes, it.scopes)
-                assertEquals("query testQuery\n", it.query)
+                assertEquals("query testQuery \n", it.query)
             },
             anyOrNull()
         )
@@ -69,10 +69,10 @@ class SentrySpringSubscriptionHandlerTest {
 
         verify(exceptionReporter).captureThrowable(
             same(exception),
-            org.mockito.kotlin.check {
+            check {
                 assertEquals(true, it.isSubscription)
                 assertSame(scopes, it.scopes)
-                assertEquals("query testQuery\n", it.query)
+                assertEquals("query testQuery \n", it.query)
             },
             anyOrNull()
         )