Skip to content

Commit

Permalink
Merge 46d0849 into c92eb47
Browse files Browse the repository at this point in the history
  • Loading branch information
lbloder committed Sep 29, 2022
2 parents c92eb47 + 46d0849 commit 0ed7ccd
Show file tree
Hide file tree
Showing 26 changed files with 408 additions and 104 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Make user segment a top level property ([#2257](https://github.com/getsentry/sentry-java/pull/2257))
- Replace user `other` with `data` ([#2258](https://github.com/getsentry/sentry-java/pull/2258))
- `isTraceSampling` is now on by default. `tracingOrigins` has been replaced by `tracePropagationTargets` ([#2255](https://github.com/getsentry/sentry-java/pull/2255))

## 6.5.0-beta.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.sentry.protocol.SdkVersion;
import io.sentry.util.Objects;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -64,7 +65,10 @@ final class ManifestMetadataReader {

@ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling";

static final String TRACING_ORIGINS = "io.sentry.traces.tracing-origins";
// TODO: remove in favor of TRACE_PROPAGATION_TARGETS
@Deprecated static final String TRACING_ORIGINS = "io.sentry.traces.tracing-origins";

static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets";

static final String ATTACH_THREADS = "io.sentry.attach-threads";
static final String PROGUARD_UUID = "io.sentry.proguard-uuid";
Expand Down Expand Up @@ -264,11 +268,22 @@ static void applyMetadata(
options.setIdleTimeout(idleTimeout);
}

final List<String> tracingOrigins = readList(metadata, logger, TRACING_ORIGINS);
if (tracingOrigins != null) {
for (final String tracingOrigin : tracingOrigins) {
options.addTracingOrigin(tracingOrigin);
}
@Nullable
List<String> tracePropagationTargets =
readList(metadata, logger, TRACE_PROPAGATION_TARGETS);

// TODO remove once TRACING_ORIGINS have been removed
if (!metadata.containsKey(TRACE_PROPAGATION_TARGETS)
&& (tracePropagationTargets == null || tracePropagationTargets.isEmpty())) {
tracePropagationTargets = readList(metadata, logger, TRACING_ORIGINS);
}

if ((metadata.containsKey(TRACE_PROPAGATION_TARGETS)
|| metadata.containsKey(TRACING_ORIGINS))
&& tracePropagationTargets == null) {
options.setTracePropagationTargets(Collections.emptyList());
} else if (tracePropagationTargets != null) {
options.setTracePropagationTargets(tracePropagationTargets);
}

options.setProguardUuid(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -698,14 +698,14 @@ class ManifestMetadataReaderTest {
@Test
fun `applyMetadata reads traceSampling to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.TRACE_SAMPLING to true)
val bundle = bundleOf(ManifestMetadataReader.TRACE_SAMPLING to false)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.isTraceSampling)
assertFalse(fixture.options.isTraceSampling)
}

@Test
Expand All @@ -717,7 +717,7 @@ class ManifestMetadataReaderTest {
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.isTraceSampling)
assertTrue(fixture.options.isTraceSampling)
}

@Test
Expand Down Expand Up @@ -787,28 +787,103 @@ class ManifestMetadataReaderTest {
}

@Test
fun `applyMetadata reads tracingOrigins to options`() {
fun `applyMetadata reads tracePropagationTargets to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.TRACING_ORIGINS to """localhost,^(http|https)://api\..*$""")
val bundle = bundleOf(ManifestMetadataReader.TRACE_PROPAGATION_TARGETS to """localhost,^(http|https)://api\..*$""")
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertEquals(listOf("localhost", """^(http|https)://api\..*$"""), fixture.options.tracingOrigins)
assertEquals(listOf("localhost", """^(http|https)://api\..*$"""), fixture.options.tracePropagationTargets)
}

@Test
fun `applyMetadata ignores tracingOrigins if tracePropagationTargets is present`() {
// Arrange
val bundle = bundleOf(
ManifestMetadataReader.TRACE_PROPAGATION_TARGETS to """localhost,^(http|https)://api\..*$""",
ManifestMetadataReader.TRACING_ORIGINS to """otherhost"""
)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options)

// Assert
assertEquals(listOf("localhost", """^(http|https)://api\..*$"""), fixture.options.tracePropagationTargets)
}

@Test
fun `applyMetadata ignores tracingOrigins if tracePropagationTargets is present even if null`() {
// Arrange
val bundle = bundleOf(
ManifestMetadataReader.TRACE_PROPAGATION_TARGETS to null,
ManifestMetadataReader.TRACING_ORIGINS to """otherhost"""
)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options)

// Assert
assertTrue(fixture.options.tracePropagationTargets.isEmpty())
}

@Test
fun `applyMetadata ignores tracingOrigins if tracePropagationTargets is present even if empty string`() {
// Arrange
val bundle = bundleOf(
ManifestMetadataReader.TRACE_PROPAGATION_TARGETS to "",
ManifestMetadataReader.TRACING_ORIGINS to """otherhost"""
)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options)

// Assert
assertTrue(fixture.options.tracePropagationTargets.isEmpty())
}

@Test
fun `applyMetadata uses tracingOrigins if tracePropagationTargets is not present`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.TRACING_ORIGINS to """otherhost""")
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options)

// Assert
assertEquals(listOf("otherhost"), fixture.options.tracePropagationTargets)
}

@Test
fun `applyMetadata reads null tracePropagationTargets and sets empty list`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.TRACE_PROPAGATION_TARGETS to null)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options)

// Assert
assertTrue(fixture.options.tracePropagationTargets.isEmpty())
}

@Test
fun `applyMetadata reads tracingOrigins to options and keeps default`() {
fun `applyMetadata reads tracePropagationTargets to options and keeps default`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.tracingOrigins.isEmpty())
assertTrue(fixture.options.tracePropagationTargets.size == 1)
assertTrue(fixture.options.tracePropagationTargets.first() == ".*")
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class SentryNavigationListenerTest {
): SentryNavigationListener {
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "http://key@localhost/proj"
setTracesSampleRate(
tracesSampleRate
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import io.sentry.HubAdapter
import io.sentry.IHub
import io.sentry.ISpan
import io.sentry.SpanStatus
import io.sentry.TracingOrigins
import io.sentry.TracePropagationTargets
import io.sentry.TypeCheckHint.OKHTTP_REQUEST
import io.sentry.TypeCheckHint.OKHTTP_RESPONSE
import okhttp3.Interceptor
Expand Down Expand Up @@ -37,7 +37,9 @@ class SentryOkHttpInterceptor(
var code: Int? = null
try {
val requestBuilder = request.newBuilder()
if (span != null && TracingOrigins.contain(hub.options.tracingOrigins, request.url.toString())) {
if (span != null &&
TracePropagationTargets.contain(hub.options.tracePropagationTargets, request.url.toString())
) {
span.toSentryTrace().let {
requestBuilder.addHeader(it.name, it.value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class SentryOkHttpInterceptorTest {
var interceptor = SentryOkHttpInterceptor(hub)
val server = MockWebServer()
lateinit var sentryTracer: SentryTracer
lateinit var options: SentryOptions

@SuppressWarnings("LongParameterList")
fun getSut(
Expand All @@ -47,19 +48,19 @@ class SentryOkHttpInterceptorTest {
responseBody: String = "success",
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
beforeSpan: SentryOkHttpInterceptor.BeforeSpanCallback? = null,
includeMockServerInTracingOrigins: Boolean = true
includeMockServerInTracePropagationTargets: Boolean = true,
keepDefaultTracePropagationTargets: Boolean = false,
): OkHttpClient {
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "https://key@sentry.io/proj"
isTraceSampling = true
if (includeMockServerInTracingOrigins) {
tracingOrigins.add(server.hostName)
} else {
tracingOrigins.add("other-api")
}
options = SentryOptions().apply {
dsn = "https://key@sentry.io/proj"
isTraceSampling = true
if (includeMockServerInTracePropagationTargets) {
setTracePropagationTargets(listOf(server.hostName))
} else if (!keepDefaultTracePropagationTargets) {
setTracePropagationTargets(listOf("other-api"))
}
)
}
whenever(hub.options).thenReturn(options)

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

Expand Down Expand Up @@ -103,9 +104,29 @@ class SentryOkHttpInterceptorTest {
assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
}

@Test
fun `when there is an active span and tracing origins contains default regex, adds sentry trace headers to the request`() {
val sut = fixture.getSut(keepDefaultTracePropagationTargets = true)

sut.newCall(getRequest()).execute()
val recorderRequest = fixture.server.takeRequest()
assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER])
}

@Test
fun `when there is an active span and server is not listed in tracing origins, does not add sentry trace headers to the request`() {
val sut = fixture.getSut(includeMockServerInTracingOrigins = false)
val sut = fixture.getSut(includeMockServerInTracePropagationTargets = false)
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[BaggageHeader.BAGGAGE_HEADER])
}

@Test
fun `when there is an active span and server tracing origins is empty, does not add sentry trace headers to the request`() {
val sut = fixture.getSut()
fixture.options.setTracePropagationTargets(emptyList())
sut.newCall(Request.Builder().get().url(fixture.server.url("/hello")).build()).execute()
val recorderRequest = fixture.server.takeRequest()
assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import io.sentry.IHub
import io.sentry.ISpan
import io.sentry.SentryLevel
import io.sentry.SpanStatus
import io.sentry.TracingOrigins
import io.sentry.TracePropagationTargets
import io.sentry.TypeCheckHint

class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null) :
Expand All @@ -33,7 +33,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH

var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList()

if (TracingOrigins.contain(hub.options.tracingOrigins, request.url)) {
if (TracePropagationTargets.contain(hub.options.tracePropagationTargets, request.url)) {
val sentryTraceHeader = span.toSentryTrace()
val baggageHeader = span.toBaggageHeader(request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value })
cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ class SentryApollo3InterceptorWithVariablesTest {
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
beforeSpan: BeforeSpanCallback? = null,
): ApolloClient {
whenever(hub.options).thenReturn(SentryOptions())
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "http://key@localhost/proj"
}
)

server.enqueue(
MockResponse()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ class SentryApolloInterceptorTest {
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
beforeSpan: SentryApolloInterceptor.BeforeSpanCallback? = null
): ApolloClient {
whenever(hub.options).thenReturn(SentryOptions())
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "http://key@localhost/proj"
}
)

server.enqueue(
MockResponse()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import io.sentry.ISpan;
import io.sentry.SentryTraceHeader;
import io.sentry.SpanStatus;
import io.sentry.TracingOrigins;
import io.sentry.TracePropagationTargets;
import io.sentry.util.Objects;
import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -55,7 +55,7 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O

final RequestWrapper requestWrapper = new RequestWrapper(request);

if (TracingOrigins.contain(hub.getOptions().getTracingOrigins(), url)) {
if (TracePropagationTargets.contain(hub.getOptions().getTracePropagationTargets(), url)) {
final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
final @Nullable Collection<String> requestBaggageHeader =
request.headers().get(BaggageHeader.BAGGAGE_HEADER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ class SentryFeignClientTest {
val hub = mock<IHub>()
val server = MockWebServer()
val sentryTracer: SentryTracer
val sentryOptions = SentryOptions()
val sentryOptions = SentryOptions().apply {
dsn = "http://key@localhost/proj"
}

init {
whenever(hub.options).thenReturn(sentryOptions)
Expand Down Expand Up @@ -123,7 +125,7 @@ class SentryFeignClientTest {

@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.setTracePropagationTargets(listOf("http://some-other-url.sentry.io"))
fixture.sentryOptions.isTraceSampling = true
fixture.sentryOptions.dsn = "https://key@sentry.io/proj"
val sut = fixture.getSut()
Expand Down

0 comments on commit 0ed7ccd

Please sign in to comment.