Fix incoming defer sampling decision sentry-trace header

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Fix incoming defer sampling decision `sentry-trace` header ([#3942](https://github.com/getsentry/sentry-java/pull/3942))
+  - A `sentry-trace` header that only contains trace ID and span ID but no sampled flag (`-1`, `-0` suffix) means the receiving system can make its own sampling decision
+  - When generating `sentry-trace` header from `PropagationContext` we now copy the `sampled` flag.
+  - In `TransactionContext.fromPropagationContext` when there is no parent sampling decision, keep the decision `null` so a new sampling decision is made instead of defaulting to `false`
+
 ## 8.0.0-rc.1
 
 ### Features

sentry-samples/sentry-samples-spring-boot-opentelemetry/src/test/kotlin/io/sentry/systemtest/PersonSystemTest.kt

@@ -44,4 +44,48 @@ class PersonSystemTest {
                 testHelper.doesTransactionContainSpanWithOp(transaction, "spanCreatedThroughSentryApi")
         }
     }
+
+    @Test
+    fun `create person creates transaction if no sampled flag in sentry-trace header`() {
+        val restClient = testHelper.restClient
+        val person = Person("firstA", "lastB")
+        val returnedPerson = restClient.createPerson(
+            person,
+            mapOf(
+                "sentry-trace" to "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee",
+                "baggage" to "sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=f9118105af4a2d42b4124532cd1065ff,sentry-transaction=HTTP%20GET"
+            )
+        )
+        assertEquals(HttpStatus.OK, restClient.lastKnownStatusCode)
+
+        assertEquals(person.firstName, returnedPerson!!.firstName)
+        assertEquals(person.lastName, returnedPerson!!.lastName)
+
+        testHelper.ensureTransactionReceived { transaction ->
+            testHelper.doesTransactionContainSpanWithOp(transaction, "spanCreatedThroughOtelApi") &&
+                testHelper.doesTransactionContainSpanWithOp(transaction, "spanCreatedThroughSentryApi")
+        }
+    }
+
+    @Test
+    fun `create person creates transaction if sampled true in sentry-trace header`() {
+        val restClient = testHelper.restClient
+        val person = Person("firstA", "lastB")
+        val returnedPerson = restClient.createPerson(
+            person,
+            mapOf(
+                "sentry-trace" to "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1",
+                "baggage" to "sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=f9118105af4a2d42b4124532cd1065ff,sentry-transaction=HTTP%20GET"
+            )
+        )
+        assertEquals(HttpStatus.OK, restClient.lastKnownStatusCode)
+
+        assertEquals(person.firstName, returnedPerson!!.firstName)
+        assertEquals(person.lastName, returnedPerson!!.lastName)
+
+        testHelper.ensureTransactionReceived { transaction ->
+            testHelper.doesTransactionContainSpanWithOp(transaction, "spanCreatedThroughOtelApi") &&
+                testHelper.doesTransactionContainSpanWithOp(transaction, "spanCreatedThroughSentryApi")
+        }
+    }
 }

sentry-samples/sentry-samples-spring-boot-opentelemetry/src/test/kotlin/io/sentry/systemtest/util/RestTestClient.kt

@@ -22,9 +22,9 @@ class RestTestClient(private val backendBaseUrl: String) : LoggingInsecureRestCl
         }
     }
 
-    fun createPerson(person: Person): Person? {
+    fun createPerson(person: Person, extraHeaders: Map<String, String>? = null): Person? {
         return try {
-            val response = restTemplate().exchange("$backendBaseUrl/person/", HttpMethod.POST, entityWithAuth(person), Person::class.java, person)
+            val response = restTemplate().exchange("$backendBaseUrl/person/", HttpMethod.POST, entityWithAuth(person, extraHeaders), Person::class.java, person)
             lastKnownStatusCode = response.statusCode
             response.body
         } catch (e: HttpStatusCodeException) {
@@ -55,9 +55,12 @@ class RestTestClient(private val backendBaseUrl: String) : LoggingInsecureRestCl
         }
     }
 
-    private fun entityWithAuth(request: Any? = null): HttpEntity<Any?> {
-        val headers = HttpHeaders().also {
-            it.setBasicAuth("user", "password")
+    private fun entityWithAuth(request: Any? = null, extraHeaders: Map<String, String>? = null): HttpEntity<Any?> {
+        val headers = HttpHeaders().also { httpHeaders ->
+            httpHeaders.setBasicAuth("user", "password")
+            extraHeaders?.forEach { key, value ->
+                httpHeaders.set(key, value)
+            }
         }
 
         return HttpEntity<Any?>(request, headers)

sentry-spring-boot-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestClientCustomizerTest.kt

@@ -28,6 +28,7 @@ import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpStatus
 import org.springframework.http.client.HttpComponentsClientHttpRequestFactory
 import org.springframework.web.client.RestClient
+import org.springframework.web.client.toEntity
 import java.time.Duration
 import java.util.concurrent.TimeUnit
 import kotlin.test.Test
@@ -195,10 +196,11 @@ class SentrySpanRestClientCustomizerTest {
                 .get()
                 .uri(fixture.url)
                 .retrieve()
+                .toEntity(String::class.java)
         } catch (t: Throwable) {
             println(t)
         }
-//        fixture.mockServer.takeRequest(mockServerRequestTimeoutMillis, TimeUnit.MILLISECONDS)!!
+        fixture.mockServer.takeRequest(mockServerRequestTimeoutMillis, TimeUnit.MILLISECONDS)!!
         assertThat(fixture.transaction.spans).hasSize(1)
         val span = fixture.transaction.spans.first()
         assertThat(span.operation).isEqualTo("http.client")

sentry/src/main/java/io/sentry/TransactionContext.java

@@ -30,11 +30,12 @@ public static TransactionContext fromPropagationContext(
       baggage.freeze();
 
       Double sampleRate = baggage.getSampleRateDouble();
-      Boolean sampled = parentSampled != null ? parentSampled.booleanValue() : false;
-      if (sampleRate != null) {
-        samplingDecision = new TracesSamplingDecision(sampled, sampleRate);
-      } else {
-        samplingDecision = new TracesSamplingDecision(sampled);
+      if (parentSampled != null) {
+        if (sampleRate != null) {
+          samplingDecision = new TracesSamplingDecision(parentSampled.booleanValue(), sampleRate);
+        } else {
+          samplingDecision = new TracesSamplingDecision(parentSampled.booleanValue());
+        }
       }
     }
 

sentry/src/main/java/io/sentry/util/TracingUtils.java

@@ -65,7 +65,9 @@ public static void startNewTrace(final @NotNull IScopes scopes) {
 
         return new TracingHeaders(
             new SentryTraceHeader(
-                propagationContext.getTraceId(), propagationContext.getSpanId(), null),
+                propagationContext.getTraceId(),
+                propagationContext.getSpanId(),
+                propagationContext.isSampled()),
             baggageHeader);
       }
 

sentry/src/test/java/io/sentry/ScopesTest.kt

@@ -1968,6 +1968,23 @@ class ScopesTest {
         assertEquals(parentSpanId, transactionContext!!.parentSpanId)
     }
 
+    @Test
+    fun `continueTrace creates propagation context from headers and returns transaction context if performance enabled no sampled value`() {
+        val scopes = generateScopes()
+        val traceId = SentryId()
+        val parentSpanId = SpanId()
+        val transactionContext = scopes.continueTrace("$traceId-$parentSpanId", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET"))
+
+        scopes.configureScope { scope ->
+            assertEquals(traceId, scope.propagationContext.traceId)
+            assertEquals(parentSpanId, scope.propagationContext.parentSpanId)
+        }
+
+        assertEquals(traceId, transactionContext!!.traceId)
+        assertEquals(parentSpanId, transactionContext!!.parentSpanId)
+        assertEquals(null, transactionContext!!.parentSamplingDecision)
+    }
+
     @Test
     fun `continueTrace creates new propagation context if header invalid and returns transaction context if performance enabled`() {
         val scopes = generateScopes()

sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt

@@ -58,6 +58,7 @@ class TracingUtilsTest {
         assertNotNull(headers.baggageHeader)
         assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId)
         assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId)
+        assertEquals(fixture.scope.propagationContext.isSampled, headers.sentryTraceHeader.isSampled)
         assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value"))
         assertTrue(headers.baggageHeader!!.value.contains("sentry-trace_id=${fixture.scope.propagationContext.traceId}"))
         assertFalse(fixture.scope.propagationContext.baggage!!.isMutable)
@@ -73,6 +74,55 @@ class TracingUtilsTest {
         assertNotNull(headers.baggageHeader)
         assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId)
         assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId)
+        assertEquals(fixture.scope.propagationContext.isSampled, headers.sentryTraceHeader.isSampled)
+        assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value"))
+        assertFalse(fixture.scope.propagationContext.baggage!!.isMutable)
+    }
+
+    @Test
+    fun `returns headers if allowed from scope if span is noop sampled=null`() {
+        fixture.setup()
+        fixture.scope.propagationContext.isSampled = null
+
+        val headers = TracingUtils.traceIfAllowed(fixture.scopes, "https://sentry.io/hello", fixture.preExistingBaggage, NoOpSpan.getInstance())
+
+        assertNotNull(headers)
+        assertNotNull(headers.baggageHeader)
+        assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId)
+        assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId)
+        assertEquals(fixture.scope.propagationContext.isSampled, headers.sentryTraceHeader.isSampled)
+        assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value"))
+        assertFalse(fixture.scope.propagationContext.baggage!!.isMutable)
+    }
+
+    @Test
+    fun `returns headers if allowed from scope if span is noop sampled=true`() {
+        fixture.setup()
+        fixture.scope.propagationContext.isSampled = true
+
+        val headers = TracingUtils.traceIfAllowed(fixture.scopes, "https://sentry.io/hello", fixture.preExistingBaggage, NoOpSpan.getInstance())
+
+        assertNotNull(headers)
+        assertNotNull(headers.baggageHeader)
+        assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId)
+        assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId)
+        assertEquals(fixture.scope.propagationContext.isSampled, headers.sentryTraceHeader.isSampled)
+        assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value"))
+        assertFalse(fixture.scope.propagationContext.baggage!!.isMutable)
+    }
+
+    @Test
+    fun `returns headers if allowed from scope if span is noop sampled=false`() {
+        fixture.setup()
+        fixture.scope.propagationContext.isSampled = false
+
+        val headers = TracingUtils.traceIfAllowed(fixture.scopes, "https://sentry.io/hello", fixture.preExistingBaggage, NoOpSpan.getInstance())
+
+        assertNotNull(headers)
+        assertNotNull(headers.baggageHeader)
+        assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId)
+        assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId)
+        assertEquals(fixture.scope.propagationContext.isSampled, headers.sentryTraceHeader.isSampled)
         assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value"))
         assertFalse(fixture.scope.propagationContext.baggage!!.isMutable)
     }