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

Implementation for server-side baggage support and third party header handling #2226

Merged
merged 15 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
- Avoid sending empty profiles ([#2232](https://github.com/getsentry/sentry-java/pull/2232))
- Fix file descriptor leak in FileIO instrumentation ([#2248](https://github.com/getsentry/sentry-java/pull/2248))

### Features

- Server-Side Dynamic Sampling Context support ([#2226](https://github.com/getsentry/sentry-java/pull/2226))

## 6.4.1

### Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ class ActivityLifecycleIntegrationTest {
val context = TransactionContext("name", "op")
val activityFramesTracker = mock<ActivityFramesTracker>()
val transactionFinishedCallback = mock<TransactionFinishedCallback>()
val transaction = SentryTracer(context, hub, true, transactionFinishedCallback)
lateinit var transaction: SentryTracer
val buildInfo = mock<BuildInfoProvider>()

fun getSut(apiVersion: Int = 29, importance: Int = RunningAppProcessInfo.IMPORTANCE_FOREGROUND): ActivityLifecycleIntegration {
whenever(hub.options).thenReturn(options)
transaction = SentryTracer(context, hub, true, transactionFinishedCallback)
whenever(hub.startTransaction(any(), any<TransactionOptions>())).thenReturn(transaction)
whenever(buildInfo.sdkInfoVersion).thenReturn(apiVersion)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.nhaarman.mockitokotlin2.never
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.IHub
import io.sentry.ILogger
import io.sentry.SentryLevel
import io.sentry.SentryTracer
Expand Down Expand Up @@ -44,11 +45,18 @@ class AndroidTransactionProfilerTest {
isDebug = true
setLogger(mockLogger)
}
val transaction1 = SentryTracer(TransactionContext("", ""), mock())
val transaction2 = SentryTracer(TransactionContext("", ""), mock())

fun getSut(context: Context, buildInfoProvider: BuildInfoProvider = buildInfo): AndroidTransactionProfiler =
AndroidTransactionProfiler(context, options, buildInfoProvider)
val hub: IHub = mock()

lateinit var transaction1: SentryTracer
lateinit var transaction2: SentryTracer

fun getSut(context: Context, buildInfoProvider: BuildInfoProvider = buildInfo): AndroidTransactionProfiler {
whenever(hub.options).thenReturn(options)
transaction1 = SentryTracer(TransactionContext("", ""), hub)
transaction2 = SentryTracer(TransactionContext("", ""), hub)
return AndroidTransactionProfiler(context, options, buildInfoProvider)
}
}

@BeforeTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.DiagnosticLogger
import io.sentry.Hint
import io.sentry.IHub
import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SentryTracer
Expand Down Expand Up @@ -59,9 +60,14 @@ class DefaultAndroidEventProcessorTest {
setLogger(mock())
sdkVersion = SdkVersion("test", "1.2.3")
}
val sentryTracer = SentryTracer(TransactionContext("", ""), mock())

val hub: IHub = mock<IHub>()

lateinit var sentryTracer: SentryTracer

fun getSut(context: Context): DefaultAndroidEventProcessor {
whenever(hub.options).thenReturn(options)
sentryTracer = SentryTracer(TransactionContext("", ""), hub)
return DefaultAndroidEventProcessor(context, buildInfo, options)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@ class PerformanceAndroidEventProcessorTest {

val hub = mock<IHub>()
val context = TransactionContext("name", "op", TracesSamplingDecision(true))
val tracer = SentryTracer(context, hub)
lateinit var tracer: SentryTracer
val activityFramesTracker = mock<ActivityFramesTracker>()

fun getSut(tracesSampleRate: Double? = 1.0): PerformanceAndroidEventProcessor {
options.tracesSampleRate = tracesSampleRate
whenever(hub.options).thenReturn(options)
tracer = SentryTracer(context, hub)
return PerformanceAndroidEventProcessor(options, activityFramesTracker)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,13 @@ class SentryGestureListenerTracingTest {
hasViewIdInRes: Boolean = true,
tracesSampleRate: Double? = 1.0,
isEnableUserInteractionTracing: Boolean = true,
transaction: SentryTracer = SentryTracer(TransactionContext("name", "op"), hub)
transaction: SentryTracer? = null
): SentryGestureListener {
options.tracesSampleRate = tracesSampleRate
options.isEnableUserInteractionTracing = isEnableUserInteractionTracing
whenever(hub.options).thenReturn(options)

this.transaction = transaction
this.transaction = transaction ?: SentryTracer(TransactionContext("name", "op"), hub)

target = mockView<T>(event = event, clickable = true)
window.mockDecorView<ViewGroup>(event = event) {
Expand All @@ -74,8 +75,8 @@ class SentryGestureListenerTracingTest {
whenever(activity.window).thenReturn(window)

whenever(hub.startTransaction(any(), any<TransactionOptions>()))
.thenReturn(transaction)
whenever(hub.options).thenReturn(options)
.thenReturn(this.transaction)

return SentryGestureListener(
activity,
hub,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,27 @@ class SentryNavigationListenerTest {
enableTracing: Boolean = true,
tracesSampleRate: Double? = 1.0,
hasViewIdInRes: Boolean = true,
transaction: SentryTracer = SentryTracer(
TransactionContext(
"/$toRoute",
SentryNavigationListener.NAVIGATION_OP
),
hub
)
transaction: SentryTracer? = null
): SentryNavigationListener {
this.transaction = transaction

whenever(hub.startTransaction(any<TransactionContext>(), any<TransactionOptions>()))
.thenReturn(transaction)
whenever(hub.options).thenReturn(
SentryOptions().apply {
setTracesSampleRate(
tracesSampleRate
)
}
)

this.transaction = transaction ?: SentryTracer(
TransactionContext(
"/$toRoute",
SentryNavigationListener.NAVIGATION_OP
),
hub
)

whenever(hub.startTransaction(any<TransactionContext>(), any<TransactionOptions>()))
.thenReturn(this.transaction)

whenever(hub.configureScope(any())).thenAnswer {
(it.arguments[0] as ScopeCallback).run(scope)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.sentry.android.okhttp

import io.sentry.BaggageHeader
import io.sentry.Breadcrumb
import io.sentry.Hint
import io.sentry.HubAdapter
Expand Down Expand Up @@ -40,7 +41,9 @@ class SentryOkHttpInterceptor(
span.toSentryTrace().let {
requestBuilder.addHeader(it.name, it.value)
}
span.toBaggageHeader()?.let {

span.toBaggageHeader(request.headers(BaggageHeader.BAGGAGE_HEADER))?.let {
requestBuilder.removeHeader(BaggageHeader.BAGGAGE_HEADER)
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
requestBuilder.addHeader(it.name, it.value)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class SentryOkHttpInterceptorTest {
val hub = mock<IHub>()
var interceptor = SentryOkHttpInterceptor(hub)
val server = MockWebServer()
val sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)
lateinit var sentryTracer: SentryTracer

@SuppressWarnings("LongParameterList")
fun getSut(
Expand All @@ -61,6 +61,8 @@ class SentryOkHttpInterceptorTest {
}
)

sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)

if (isSpanActive) {
whenever(hub.span).thenReturn(sentryTracer)
}
Expand All @@ -81,6 +83,7 @@ class SentryOkHttpInterceptorTest {
private val fixture = Fixture()

private val getRequest = { Request.Builder().get().url(fixture.server.url("/hello")).build() }
private val getRequestWithBaggagHeader = { Request.Builder().addHeader("baggage", "thirdPartyBaggage=someValue").addHeader("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue").get().url(fixture.server.url("/hello")).build() }
private val postRequest = {
Request.Builder().post(
"request-body"
Expand Down Expand Up @@ -118,6 +121,22 @@ class SentryOkHttpInterceptorTest {
assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
}

@Test
fun `when there is an active span, existing baggage headers are merged with sentry baggage into single header`() {
val sut = fixture.getSut()
sut.newCall(getRequestWithBaggagHeader()).execute()
val recorderRequest = fixture.server.takeRequest()
assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])

val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER)
assertEquals(baggageHeaderValues.size, 1)
assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue"))
assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key"))
assertTrue(baggageHeaderValues[0].contains("sentry-transaction=name"))
assertTrue(baggageHeaderValues[0].contains("sentry-trace_id"))
}

@Test
fun `does not overwrite response body`() {
val sut = fixture.getSut()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.apollographql.apollo3.exception.ApolloHttpException
import com.apollographql.apollo3.exception.ApolloNetworkException
import com.apollographql.apollo3.network.http.HttpInterceptor
import com.apollographql.apollo3.network.http.HttpInterceptorChain
import io.sentry.BaggageHeader
import io.sentry.Breadcrumb
import io.sentry.Hint
import io.sentry.HubAdapter
Expand All @@ -30,22 +31,24 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH
} else {
val span = startChild(request, activeSpan)

val cleanedHeaders = removeSentryInternalHeaders(request.headers)

val requestBuilder = request.newBuilder().apply {
headers(cleanedHeaders)
}
var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList()

if (TracingOrigins.contain(hub.options.tracingOrigins, request.url)) {
val sentryTraceHeader = span.toSentryTrace()
val baggageHeader = span.toBaggageHeader()
requestBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value)
val baggageHeader = span.toBaggageHeader(request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value })
cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value))

baggageHeader?.let {
requestBuilder.addHeader(it.name, it.value)
baggageHeader?.let { newHeader ->
cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply {
add(HttpHeader(newHeader.name, newHeader.value))
}
}
}

val requestBuilder = request.newBuilder().apply {
headers(cleanedHeaders)
}

val modifiedRequest = requestBuilder.build()
var httpResponse: HttpResponse? = null
var statusCode: Int? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class SentryApollo3InterceptorTest {
}
}""",
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
addThirdPartyBaggageHeader: Boolean = false,
beforeSpan: BeforeSpanCallback? = null,
): ApolloClient {
whenever(hub.options).thenReturn(
Expand All @@ -80,6 +81,11 @@ class SentryApollo3InterceptorTest {
.serverUrl(server.url("/").toString())
.addHttpInterceptor(httpInterceptor)

if (addThirdPartyBaggageHeader) {
builder.addHttpHeader("baggage", "thirdPartyBaggage=someValue")
adinauer marked this conversation as resolved.
Show resolved Hide resolved
.addHttpHeader("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")
}

return builder.build()
}
}
Expand Down Expand Up @@ -148,6 +154,21 @@ class SentryApollo3InterceptorTest {
assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
}

@Test
fun `when there is an active span, existing baggage headers are merged with sentry baggage into single header`() {
executeQuery(sut = fixture.getSut(addThirdPartyBaggageHeader = true))
val recorderRequest = fixture.server.takeRequest()
assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])

val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER)
assertEquals(baggageHeaderValues.size, 1)
assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue"))
assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key"))
assertTrue(baggageHeaderValues[0].contains("sentry-transaction=op"))
assertTrue(baggageHeaderValues[0].contains("sentry-trace_id"))
}

@Test
fun `customizer modifies span`() {
executeQuery(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.apollographql.apollo.interceptor.ApolloInterceptor.FetchSourceType
import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorRequest
import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorResponse
import com.apollographql.apollo.interceptor.ApolloInterceptorChain
import io.sentry.BaggageHeader
import io.sentry.Breadcrumb
import io.sentry.Hint
import io.sentry.HubAdapter
Expand Down Expand Up @@ -41,7 +42,7 @@ class SentryApolloInterceptor(
// we have no access to URI, no way to verify tracing origins
val requestHeaderBuilder = request.requestHeaders.toBuilder()
requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value)
span.toBaggageHeader()?.let {
span.toBaggageHeader(listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)))?.let {
requestHeaderBuilder.addHeader(it.name, it.value)
}
val headers = requestHeaderBuilder.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ class SentryInstrumentationTest {

class Fixture {
val hub = mock<IHub>()
val activeSpan = SentryTracer(TransactionContext("name", "op"), hub)
lateinit var activeSpan: SentryTracer

fun getSut(isTransactionActive: Boolean = true, dataFetcherThrows: Boolean = false, beforeSpan: SentryInstrumentation.BeforeSpanCallback? = null): GraphQL {
whenever(hub.options).thenReturn(SentryOptions())
activeSpan = SentryTracer(TransactionContext("name", "op"), hub)
val schema = """
type Query {
shows: [Show]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ class SentryJdbcEventListenerTest {

class Fixture {
private val hub = mock<IHub>()
val tx = SentryTracer(TransactionContext("name", "op"), hub)
lateinit var tx: SentryTracer
val actualDataSource = JDBCDataSource()

fun getSut(withRunningTransaction: Boolean = true, existingRow: Int? = null): DataSource {
whenever(hub.options).thenReturn(SentryOptions())
tx = SentryTracer(TransactionContext("name", "op"), hub)
if (withRunningTransaction) {
whenever(hub.span).thenReturn(tx)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.sentry.TracingOrigins;
import io.sentry.util.Objects;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -56,9 +57,14 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O

if (TracingOrigins.contain(hub.getOptions().getTracingOrigins(), url)) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
final @Nullable BaggageHeader baggageHeader = span.toBaggageHeader();
final @Nullable Collection<String> requestBaggageHeader =
request.headers().get(BaggageHeader.BAGGAGE_HEADER);
final @Nullable BaggageHeader baggageHeader =
span.toBaggageHeader(
requestBaggageHeader != null ? new ArrayList<>(requestBaggageHeader) : null);
requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue());
if (baggageHeader != null) {
requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER);
requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue());
}
}
Expand Down Expand Up @@ -128,6 +134,10 @@ public void header(final @NotNull String name, final @NotNull String value) {
}
}

public void removeHeader(final @NotNull String name) {
headers.remove(name);
}

@NotNull
Request build() {
return Request.create(
Expand Down
Loading