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

isTraceSampling is now on by default. tracingOrigins has been replaced by tracePropagationTargets #2255

Merged
merged 31 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0205111
initial implementation for server-side baggage support and third part…
lbloder Aug 29, 2022
b2db551
Format code
getsentry-bot Aug 29, 2022
8d2ef82
add changelog, dump api
lbloder Aug 30, 2022
2fbb9fe
Merge branch 'feat/server_side_baggage' of github.com:getsentry/sentr…
lbloder Aug 30, 2022
a06ad71
fix tests
lbloder Sep 2, 2022
a383022
add baggage merge tests
lbloder Sep 6, 2022
b34bc54
Merge branch 'main' into feat/server_side_baggage
lbloder Sep 6, 2022
f329bf7
add baggage merge test for Spring RestTemplate
lbloder Sep 6, 2022
2b0e79e
Update sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/S…
lbloder Sep 11, 2022
a46de63
CR - create override for fromSentryTrace, add more complex pre-existi…
lbloder Sep 11, 2022
6d03e10
Merge branch 'main' into feat/server_side_baggage
lbloder Sep 15, 2022
f42fbb8
re-introduce max-member limit
lbloder Sep 15, 2022
b925630
deprecate traceSampling flag, deprecate tracingOrigins in favor of ne…
lbloder Sep 16, 2022
a79c650
Merge branch 'main' into feat/server_side_baggage
adinauer Sep 19, 2022
fc52172
Merge branch 'feat/server_side_baggage' into feat/tracePropagationTar…
lbloder Sep 22, 2022
b0b0a9f
remove equal and hashcode from TraceContext, adapt tests
lbloder Sep 22, 2022
9580dc1
Merge branch 'feat/server_side_baggage' into feat/tracePropagationTar…
lbloder Sep 22, 2022
47523bc
Merge branch 'main' into feat/tracePropagationTargets
lbloder Sep 23, 2022
27b59a4
[WIP] default value for tracePropagationTargets
lbloder Sep 27, 2022
c726c5d
Format code
getsentry-bot Sep 27, 2022
070949a
Allow disabling via tracePropagationTargets; fix tests; add tests
adinauer Sep 28, 2022
0d6ccc6
Merge branch 'main' into feat/tracePropagationTargets
adinauer Sep 28, 2022
3d018bc
Add more tests; fix spring parsing for tracing-origins
adinauer Sep 28, 2022
3c4da43
Update sentry/src/main/java/io/sentry/Dsn.java
adinauer Sep 28, 2022
7c1ce3f
Format code
getsentry-bot Sep 28, 2022
e9bde17
Add changelog; fix build
adinauer Sep 28, 2022
5955807
Rename TracingOrigins; remove addTracePropagationTarget from SentryOp…
adinauer Sep 28, 2022
a3c9a36
CR
adinauer Sep 28, 2022
0420714
Fix JavaDoc
adinauer Sep 29, 2022
46d0849
Merge branch 'main' into feat/tracePropagationTargets
adinauer Sep 29, 2022
da0ccc3
Fix test after merging in main
adinauer Sep 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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`() {
adinauer marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Up @@ -58,6 +58,7 @@ class SentryNavigationListenerTest {
): SentryNavigationListener {
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "http://key@localhost/proj"
setTracesSampleRate(
tracesSampleRate
)
Expand Down
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
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
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
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
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
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
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