Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace tracestate header with baggage #2078

Merged
merged 27 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1116054
Replace tracestate header with baggage
adinauer Jun 3, 2022
f235aa7
Update changelog with PR number
adinauer Jun 3, 2022
382b56e
Merge branch '6.x.x' into feat/replace-trace-state-with-baggage
bruno-garcia Jun 6, 2022
ed86e2d
Re-add experimental flag
adinauer Jun 7, 2022
66136e9
Add more tests to document current behaviour
adinauer Jun 7, 2022
bc421ba
Merge branch 'feat/replace-trace-state-with-baggage' of github.com:ge…
adinauer Jun 7, 2022
49561ec
Mark places where header could be added
adinauer Jun 8, 2022
4321a09
Remove unused code
adinauer Jun 8, 2022
8a95be3
Add tests to document which characters are encoded for baggage key an…
adinauer Jun 8, 2022
6e5314e
Mark places where header could be added
adinauer Jun 8, 2022
1247b38
Change limit from per value to total baggage string length of 8192; a…
adinauer Jun 9, 2022
c83ff56
Add baggage header to apollo, feign and web requests
adinauer Jun 9, 2022
318079d
Merge branch 'main' into feat/replace-trace-state-with-baggage
adinauer Jun 9, 2022
d9c1195
Add final; use constants in tests
adinauer Jun 9, 2022
390754d
Add log message for exceeding list member limit
adinauer Jun 9, 2022
5cc731d
Update sentry/src/main/java/io/sentry/SentryOptions.java
adinauer Jun 14, 2022
943d730
Update sentry/src/main/java/io/sentry/SentryOptions.java
adinauer Jun 14, 2022
ad095b3
Update sentry/src/main/java/io/sentry/Baggage.java
adinauer Jun 14, 2022
04cff15
Update sentry/src/main/java/io/sentry/Baggage.java
adinauer Jun 14, 2022
82a6874
Update sentry/src/main/java/io/sentry/Baggage.java
adinauer Jun 14, 2022
7929a6e
Update sentry/src/main/java/io/sentry/SentryOptions.java
adinauer Jun 14, 2022
1b6d330
Update sentry/src/main/java/io/sentry/SentryOptions.java
adinauer Jun 14, 2022
15bbb0e
Merge branch 'feat/replace-trace-state-with-baggage' of github.com:ge…
adinauer Jun 14, 2022
37aab6d
Merge branch 'main' into feat/replace-trace-state-with-baggage
adinauer Jun 14, 2022
44b0024
More Code Review changes
adinauer Jun 14, 2022
d0acca4
Format code
getsentry-bot Jun 14, 2022
bcbeef9
Remove files that were unintentionally added
adinauer Jun 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Features

- Implement local scope by adding overloads to the capture methods that accept a ScopeCallback ([#2084](https://github.com/getsentry/sentry-java/pull/2084))
- Replace `tracestate` header with `baggage` header ([#2078](https://github.com/getsentry/sentry-java/pull/2078))

## 6.0.0

Expand Down
1 change: 1 addition & 0 deletions buildSrc/src/main/java/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
adinauer marked this conversation as resolved.
Show resolved Hide resolved
import io.sentry.TransactionContext
import io.sentry.TransactionFinishedCallback
import java.util.Date
Expand Down Expand Up @@ -372,7 +372,7 @@ class ActivityLifecycleIntegrationTest {
check {
assertEquals(SpanStatus.OK, it.status)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -395,7 +395,7 @@ class ActivityLifecycleIntegrationTest {
check {
assertEquals(SpanStatus.UNKNOWN_ERROR, it.status)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
adinauer marked this conversation as resolved.
Show resolved Hide resolved
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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -85,7 +85,7 @@ class SentryApolloInterceptorTest {
assertTransactionDetails(it)
assertEquals(SpanStatus.OK, it.spans.first().status)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -100,7 +100,7 @@ class SentryApolloInterceptorTest {
assertTransactionDetails(it)
assertEquals(SpanStatus.PERMISSION_DENIED, it.spans.first().status)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -115,7 +115,7 @@ class SentryApolloInterceptorTest {
assertTransactionDetails(it)
assertEquals(SpanStatus.INTERNAL_ERROR, it.spans.first().status)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand Down Expand Up @@ -151,7 +151,7 @@ class SentryApolloInterceptorTest {
val httpClientSpan = it.spans.first()
assertEquals("overwritten description", httpClientSpan.description)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -167,7 +167,7 @@ class SentryApolloInterceptorTest {
check {
assertEquals(1, it.spans.size)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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;
Expand Down Expand Up @@ -50,8 +51,13 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O
span.setDescription(request.httpMethod().name() + " " + request.url());

final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
final @Nullable BaggageHeader baggageHeader = span.toBaggageHeader();
final RequestWrapper requestWrapper = new RequestWrapper(request);
requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
adinauer marked this conversation as resolved.
Show resolved Hide resolved
if (baggageHeader != null) {
requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue());
}

try {
response = delegate.execute(requestWrapper.build(), options);
// handles both success and error responses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import io.sentry.SentryOptions
import io.sentry.SentryTracer
import io.sentry.SpanId
import io.sentry.SpanStatus
import io.sentry.TraceState
import io.sentry.TraceContext
import io.sentry.TransactionContext
import io.sentry.protocol.SentryId
import org.assertj.core.api.Assertions.assertThat
Expand Down Expand Up @@ -84,7 +84,7 @@ class SentryTracingFilterTest {
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
assertThat(it.contexts.trace!!.operation).isEqualTo("http.server")
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -100,7 +100,7 @@ class SentryTracingFilterTest {
check {
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -116,7 +116,7 @@ class SentryTracingFilterTest {
check {
assertThat(it.contexts.trace!!.status).isNull()
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -132,7 +132,7 @@ class SentryTracingFilterTest {
check {
assertThat(it.contexts.trace!!.parentSpanId).isNull()
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -149,7 +149,7 @@ class SentryTracingFilterTest {
check {
assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand Down Expand Up @@ -182,7 +182,7 @@ class SentryTracingFilterTest {
check {
assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import io.sentry.IHub
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 org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.assertThrows
Expand Down Expand Up @@ -64,7 +64,7 @@ class SentryTransactionAdviceTest {
assertThat(it.contexts.trace!!.operation).isEqualTo("bean")
assertThat(it.status).isEqualTo(SpanStatus.OK)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -77,7 +77,7 @@ class SentryTransactionAdviceTest {
check {
assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -91,7 +91,7 @@ class SentryTransactionAdviceTest {
assertThat(it.transaction).isEqualTo("SampleService.methodWithoutTransactionNameSet")
assertThat(it.contexts.trace!!.operation).isEqualTo("op")
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -103,7 +103,7 @@ class SentryTransactionAdviceTest {

sampleService.methodWithTransactionNameSet()

verify(hub, times(0)).captureTransaction(any(), any<TraceState>())
verify(hub, times(0)).captureTransaction(any(), any<TraceContext>())
}

@Test
Expand All @@ -114,7 +114,7 @@ class SentryTransactionAdviceTest {
assertThat(it.transaction).isEqualTo("ClassAnnotatedSampleService.hello")
assertThat(it.contexts.trace!!.operation).isEqualTo("op")
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand All @@ -128,7 +128,7 @@ class SentryTransactionAdviceTest {
assertThat(it.transaction).isEqualTo("ClassAnnotatedWithOperationSampleService.hello")
assertThat(it.contexts.trace!!.operation).isEqualTo("my-op")
},
anyOrNull<TraceState>(),
anyOrNull<TraceContext>(),
anyOrNull(),
anyOrNull()
)
Expand Down
Loading