Replace tracestate header with baggage (#2078)

* Replace tracestate header with baggage * Update changelog with PR number * Re-add experimental flag * Add more tests to document current behaviour * Mark places where header could be added * Remove unused code * Add tests to document which characters are encoded for baggage key and value * Mark places where header could be added * Change limit from per value to total baggage string length of 8192; also limit list members to 64 for baggage * Add baggage header to apollo, feign and web requests * Add final; use constants in tests * Add log message for exceeding list member limit * Update sentry/src/main/java/io/sentry/SentryOptions.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/SentryOptions.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/Baggage.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/Baggage.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/Baggage.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/SentryOptions.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * Update sentry/src/main/java/io/sentry/SentryOptions.java Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * More Code Review changes * Format code * Remove files that were unintentionally added Co-authored-by: Bruno Garcia <bruno@brunogarcia.com> Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
getsentry · Jun 15, 2022 · a88509c · a88509c
1 parent 580aa75
commit a88509c
Show file tree

Hide file tree

Showing 44 changed files with 1,250 additions and 359 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Replace `tracestate` header with `baggage` header ([#2078](https://github.com/getsentry/sentry-java/pull/2078))
+
 ## 6.1.0
 
 ### Features

diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt
@@ -134,6 +134,7 @@ object Config {
         val mockWebserver3 = "com.squareup.okhttp3:mockwebserver:3.14.9"
         val jsonUnit = "net.javacrumbs.json-unit:json-unit:2.32.0"
         val hsqldb = "org.hsqldb:hsqldb:2.6.1"
+        val javaFaker = "com.github.javafaker:javafaker:1.0.2"
     }
 
     object QualityPlugins {

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt
@@ -20,7 +20,7 @@ import io.sentry.SentryLevel
 import io.sentry.SentryOptions
 import io.sentry.SentryTracer
 import io.sentry.SpanStatus
-import io.sentry.TraceState
+import io.sentry.TraceContext
 import io.sentry.TransactionContext
 import io.sentry.TransactionFinishedCallback
 import java.util.Date
@@ -372,7 +372,7 @@ class ActivityLifecycleIntegrationTest {
             check {
                 assertEquals(SpanStatus.OK, it.status)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -395,7 +395,7 @@ class ActivityLifecycleIntegrationTest {
             check {
                 assertEquals(SpanStatus.UNKNOWN_ERROR, it.status)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -412,7 +412,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityPostResumed(activity)
 
-        verify(fixture.hub, never()).captureTransaction(any(), anyOrNull<TraceState>(), anyOrNull(), anyOrNull())
+        verify(fixture.hub, never()).captureTransaction(any(), anyOrNull<TraceContext>(), anyOrNull(), anyOrNull())
     }
 
     @Test
@@ -423,7 +423,7 @@ class ActivityLifecycleIntegrationTest {
         val activity = mock<Activity>()
         sut.onActivityPostResumed(activity)
 
-        verify(fixture.hub, never()).captureTransaction(any(), anyOrNull<TraceState>(), anyOrNull(), anyOrNull())
+        verify(fixture.hub, never()).captureTransaction(any(), anyOrNull<TraceContext>(), anyOrNull(), anyOrNull())
     }
 
     @Test
@@ -436,7 +436,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityCreated(activity, fixture.bundle)
         sut.onActivityDestroyed(activity)
 
-        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceState>(), anyOrNull(), anyOrNull())
+        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceContext>(), anyOrNull(), anyOrNull())
     }
 
     @Test
@@ -505,7 +505,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityCreated(mock(), mock())
 
         sut.onActivityCreated(mock(), fixture.bundle)
-        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceState>(), anyOrNull(), anyOrNull())
+        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceContext>(), anyOrNull(), anyOrNull())
     }
 
     @Test
@@ -518,7 +518,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityCreated(activity, mock())
         sut.onActivityResumed(activity)
 
-        verify(fixture.hub, never()).captureTransaction(any(), any<TraceState>(), anyOrNull())
+        verify(fixture.hub, never()).captureTransaction(any(), any<TraceContext>(), anyOrNull())
     }
 
     @Test
@@ -545,7 +545,7 @@ class ActivityLifecycleIntegrationTest {
         sut.onActivityCreated(activity, mock())
         sut.onActivityResumed(activity)
 
-        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceState>(), anyOrNull(), anyOrNull())
+        verify(fixture.hub).captureTransaction(any(), anyOrNull<TraceContext>(), anyOrNull(), anyOrNull())
     }
 
     @Test

diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt
@@ -40,7 +40,7 @@ class SentryOkHttpInterceptor(
                 span.toSentryTrace().let {
                     requestBuilder.addHeader(it.name, it.value)
                 }
-                span.toTraceStateHeader()?.let {
+                span.toBaggageHeader()?.let {
                     requestBuilder.addHeader(it.name, it.value)
                 }
             }

diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt
@@ -7,13 +7,13 @@ import com.nhaarman.mockitokotlin2.check
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
+import io.sentry.BaggageHeader
 import io.sentry.Breadcrumb
 import io.sentry.IHub
 import io.sentry.SentryOptions
 import io.sentry.SentryTraceHeader
 import io.sentry.SentryTracer
 import io.sentry.SpanStatus
-import io.sentry.TraceStateHeader
 import io.sentry.TransactionContext
 import okhttp3.Interceptor
 import okhttp3.MediaType.Companion.toMediaType
@@ -97,7 +97,7 @@ class SentryOkHttpInterceptorTest {
         sut.newCall(getRequest()).execute()
         val recorderRequest = fixture.server.takeRequest()
         assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
-        assertNotNull(recorderRequest.headers[TraceStateHeader.TRACE_STATE_HEADER])
+        assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
     }
 
     @Test
@@ -106,7 +106,7 @@ class SentryOkHttpInterceptorTest {
         sut.newCall(Request.Builder().get().url(fixture.server.url("/hello")).build()).execute()
         val recorderRequest = fixture.server.takeRequest()
         assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
-        assertNull(recorderRequest.headers[TraceStateHeader.TRACE_STATE_HEADER])
+        assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
     }
 
     @Test
@@ -115,7 +115,7 @@ class SentryOkHttpInterceptorTest {
         sut.newCall(getRequest()).execute()
         val recorderRequest = fixture.server.takeRequest()
         assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
-        assertNull(recorderRequest.headers[TraceStateHeader.TRACE_STATE_HEADER])
+        assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
     }
 
     @Test

diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt
@@ -39,7 +39,12 @@ class SentryApolloInterceptor(
             val sentryTraceHeader = span.toSentryTrace()
 
             // we have no access to URI, no way to verify tracing origins
-            val headers = request.requestHeaders.toBuilder().addHeader(sentryTraceHeader.name, sentryTraceHeader.value).build()
+            val requestHeaderBuilder = request.requestHeaders.toBuilder()
+            requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value)
+            span.toBaggageHeader()?.let {
+                requestHeaderBuilder.addHeader(it.name, it.value)
+            }
+            val headers = requestHeaderBuilder.build()
             val requestWithHeader = request.toBuilder().requestHeaders(headers).build()
             span.setData("operationId", requestWithHeader.operation.operationId())
             span.setData("variables", requestWithHeader.operation.variables().valueMap().toString())

diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt
@@ -15,7 +15,7 @@ import io.sentry.SentryOptions
 import io.sentry.SentryTraceHeader
 import io.sentry.SentryTracer
 import io.sentry.SpanStatus
-import io.sentry.TraceState
+import io.sentry.TraceContext
 import io.sentry.TransactionContext
 import io.sentry.protocol.SentryTransaction
 import kotlinx.coroutines.launch
@@ -85,7 +85,7 @@ class SentryApolloInterceptorTest {
                 assertTransactionDetails(it)
                 assertEquals(SpanStatus.OK, it.spans.first().status)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -100,7 +100,7 @@ class SentryApolloInterceptorTest {
                 assertTransactionDetails(it)
                 assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -115,7 +115,7 @@ class SentryApolloInterceptorTest {
                 assertTransactionDetails(it)
                 assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -151,7 +151,7 @@ class SentryApolloInterceptorTest {
                 val httpClientSpan = it.spans.first()
                 assertEquals("overwritten description", httpClientSpan.description)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )
@@ -167,7 +167,7 @@ class SentryApolloInterceptorTest {
             check {
                 assertEquals(1, it.spans.size)
             },
-            anyOrNull<TraceState>(),
+            anyOrNull<TraceContext>(),
             anyOrNull(),
             anyOrNull()
         )

diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java
@@ -6,12 +6,14 @@
 import feign.Client;
 import feign.Request;
 import feign.Response;
+import io.sentry.BaggageHeader;
 import io.sentry.Breadcrumb;
 import io.sentry.Hint;
 import io.sentry.IHub;
 import io.sentry.ISpan;
 import io.sentry.SentryTraceHeader;
 import io.sentry.SpanStatus;
+import io.sentry.TracingOrigins;
 import io.sentry.util.Objects;
 import java.io.IOException;
 import java.util.Collection;
@@ -47,11 +49,20 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O
       }
 
       ISpan span = activeSpan.startChild("http.client");
-      span.setDescription(request.httpMethod().name() + " " + request.url());
+      String url = request.url();
+      span.setDescription(request.httpMethod().name() + " " + url);
 
-      final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
       final RequestWrapper requestWrapper = new RequestWrapper(request);
-      requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
+
+      if (TracingOrigins.contain(hub.getOptions().getTracingOrigins(), url)) {
+        final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
+        final @Nullable BaggageHeader baggageHeader = span.toBaggageHeader();
+        requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
+        if (baggageHeader != null) {
+          requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue());
+        }
+      }
+
       try {
         response = delegate.execute(requestWrapper.build(), options);
         // handles both success and error responses

diff --git a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt
@@ -10,6 +10,7 @@ import feign.Client
 import feign.Feign
 import feign.FeignException
 import feign.RequestLine
+import io.sentry.BaggageHeader
 import io.sentry.Breadcrumb
 import io.sentry.IHub
 import io.sentry.SentryOptions
@@ -19,6 +20,7 @@ import io.sentry.SpanStatus
 import io.sentry.TransactionContext
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
+import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -33,9 +35,10 @@ class SentryFeignClientTest {
         val hub = mock<IHub>()
         val server = MockWebServer()
         val sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)
+        val sentryOptions = SentryOptions()
 
         init {
-            whenever(hub.options).thenReturn(SentryOptions())
+            whenever(hub.options).thenReturn(sentryOptions)
         }
 
         fun getSut(
@@ -67,22 +70,45 @@ class SentryFeignClientTest {
         }
     }
 
-    private val fixture = Fixture()
+    private lateinit var fixture: Fixture
+
+    @BeforeTest
+    fun setup() {
+        fixture = Fixture()
+    }
 
     @Test
     fun `when there is an active span, adds sentry trace header to the request`() {
+        fixture.sentryOptions.isTraceSampling = true
+        fixture.sentryOptions.dsn = "https://key@sentry.io/proj"
         val sut = fixture.getSut()
         sut.getOk()
         val recorderRequest = fixture.server.takeRequest()
         assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
+        assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
     }
 
     @Test
     fun `when there is no active span, does not add sentry trace header to the request`() {
+        fixture.sentryOptions.isTraceSampling = true
+        fixture.sentryOptions.dsn = "https://key@sentry.io/proj"
         val sut = fixture.getSut(isSpanActive = false)
         sut.getOk()
         val recorderRequest = fixture.server.takeRequest()
         assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
+        assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
+    }
+
+    @Test
+    fun `when request url not in tracing origins, does not add sentry trace header to the request`() {
+        fixture.sentryOptions.addTracingOrigin("http://some-other-url.sentry.io")
+        fixture.sentryOptions.isTraceSampling = true
+        fixture.sentryOptions.dsn = "https://key@sentry.io/proj"
+        val sut = fixture.getSut()
+        sut.getOk()
+        val recorderRequest = fixture.server.takeRequest()
+        assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
+        assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
     }
 
     @Test

diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java
@@ -4,6 +4,7 @@
 import static io.sentry.TypeCheckHint.SPRING_EXCHANGE_FILTER_RESPONSE;
 
 import com.jakewharton.nopen.annotation.Open;
+import io.sentry.BaggageHeader;
 import io.sentry.Breadcrumb;
 import io.sentry.Hint;
 import io.sentry.IHub;
@@ -41,11 +42,15 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) {
     span.setDescription(request.method().name() + " " + request.url());
 
     final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
+    final @Nullable BaggageHeader baggageHeader = span.toBaggageHeader();
 
     final ClientRequest.Builder requestBuilder = ClientRequest.from(request);
 
     if (TracingOrigins.contain(hub.getOptions().getTracingOrigins(), request.url())) {
       requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
+      if (baggageHeader != null) {
+        requestBuilder.header(baggageHeader.getName(), baggageHeader.getValue());
+      }
     }
 
     final ClientRequest clientRequestWithSentryTraceHeader = requestBuilder.build();