Fork isolation scope for root spans in OtelSentrySpanProcessor instead of OtelSentryPropagator (#4434)

* Fork isolation scope in SpanProcessor

* improve creator strings

* changelog

* remove irrelevant tests

Author: adinauer

CHANGELOG.md

@@ -21,6 +21,13 @@
     - `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))

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()