From 3fc8b390f761024ca76016d8462511d853a91d78 Mon Sep 17 00:00:00 2001 From: Stefano Date: Mon, 31 Jul 2023 10:53:36 +0200 Subject: [PATCH 1/4] propagate okhttp status to parent spans (#2872) * status of OkHttp calls now gets propagated to parent span * when OkHttp call finishes, status of unfinished spans is not overridden and is set to INTERNAL_ERROR --- CHANGELOG.md | 6 +++ .../android/okhttp/SentryOkHttpEvent.kt | 32 +++++++----- .../okhttp/SentryOkHttpEventListener.kt | 5 +- .../okhttp/SentryOkHttpEventListenerTest.kt | 52 +++++++++++++++++-- .../android/okhttp/SentryOkHttpEventTest.kt | 49 +++++++++++++++++ 5 files changed, 127 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b88bf7718d..dd801cf04a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +- propagate okhttp status to parent spans ([#2872](https://github.com/getsentry/sentry-java/pull/2872)) + +### Fixes + ## 6.27.0 ### Features diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index 5727eb7a83..e96829e12e 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -63,7 +63,6 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req breadcrumb.setData("status_code", response.code) callRootSpan?.setData(PROTOCOL_KEY, response.protocol.name) callRootSpan?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code) - callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code) } fun setProtocol(protocolName: String?) { @@ -98,24 +97,20 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req /** Starts a span, if the callRootSpan is not null. */ fun startSpan(event: String) { // Find the parent of the span being created. E.g. secureConnect is child of connect - val parentSpan = when (event) { - // PROXY_SELECT, DNS, CONNECT and CONNECTION are not children of one another - SECURE_CONNECT_EVENT -> eventSpans[CONNECT_EVENT] - REQUEST_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] - REQUEST_BODY_EVENT -> eventSpans[CONNECTION_EVENT] - RESPONSE_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] - RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT] - else -> callRootSpan - } ?: callRootSpan + val parentSpan = findParentSpan(event) val span = parentSpan?.startChild("http.client.$event") ?: return span.spanContext.origin = TRACE_ORIGIN eventSpans[event] = span } - /** Finishes a previously started span, and runs [beforeFinish] on it and on the call root span. */ + /** Finishes a previously started span, and runs [beforeFinish] on it, on its parent and on the call root span. */ fun finishSpan(event: String, beforeFinish: ((span: ISpan) -> Unit)? = null) { val span = eventSpans[event] ?: return + val parentSpan = findParentSpan(event) beforeFinish?.invoke(span) + if (parentSpan != null && parentSpan != callRootSpan) { + beforeFinish?.invoke(parentSpan) + } callRootSpan?.let { beforeFinish?.invoke(it) } span.finish() } @@ -125,7 +120,10 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req callRootSpan ?: return // We forcefully finish all spans, even if they should already have been finished through finishSpan() - eventSpans.values.filter { !it.isFinished }.forEach { it.finish(SpanStatus.DEADLINE_EXCEEDED) } + eventSpans.values.filter { !it.isFinished }.forEach { + // If a status was set on the span, we use that, otherwise we set its status as error. + it.finish(it.status ?: SpanStatus.INTERNAL_ERROR) + } beforeFinish?.invoke(callRootSpan) callRootSpan.finish() @@ -137,4 +135,14 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req hub.addBreadcrumb(breadcrumb, hint) return } + + private fun findParentSpan(event: String): ISpan? = when (event) { + // PROXY_SELECT, DNS, CONNECT and CONNECTION are not children of one another + SECURE_CONNECT_EVENT -> eventSpans[CONNECT_EVENT] + REQUEST_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] + REQUEST_BODY_EVENT -> eventSpans[CONNECTION_EVENT] + RESPONSE_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] + RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT] + else -> callRootSpan + } ?: callRootSpan } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index accc715c70..c2418bd9ab 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -313,7 +313,10 @@ class SentryOkHttpEventListener( okHttpEvent.setResponse(response) okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) { it.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code) - it.status = SpanStatus.fromHttpStatusCode(response.code) + // Let's not override the status of a span that was set + if (it.status == null) { + it.status = SpanStatus.fromHttpStatusCode(response.code) + } } } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index 6348027f65..ede442eaa7 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -19,6 +19,7 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.spy @@ -249,7 +250,7 @@ class SentryOkHttpEventListenerTest { @Test fun `propagate all calls to the event listener passed in the ctor`() { - val sut = fixture.getSut(eventListener = fixture.mockEventListener, httpStatusCode = 500) + val sut = fixture.getSut(eventListener = fixture.mockEventListener) val listener = fixture.sentryOkHttpEventListener val request = postRequest(body = "requestBody") val call = sut.newCall(request) @@ -260,7 +261,7 @@ class SentryOkHttpEventListenerTest { @Test fun `propagate all calls to the event listener factory passed in the ctor`() { - val sut = fixture.getSut(eventListenerFactory = fixture.mockEventListenerFactory, httpStatusCode = 500) + val sut = fixture.getSut(eventListenerFactory = fixture.mockEventListenerFactory) val listener = fixture.sentryOkHttpEventListener val request = postRequest(body = "requestBody") val call = sut.newCall(request) @@ -272,7 +273,7 @@ class SentryOkHttpEventListenerTest { @Test fun `propagate all calls to the SentryOkHttpEventListener passed in the ctor`() { val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener)) - val sut = fixture.getSut(eventListener = originalListener, httpStatusCode = 500) + val sut = fixture.getSut(eventListener = originalListener) val listener = fixture.sentryOkHttpEventListener val request = postRequest(body = "requestBody") val call = sut.newCall(request) @@ -284,7 +285,7 @@ class SentryOkHttpEventListenerTest { @Test fun `propagate all calls to the SentryOkHttpEventListener factory passed in the ctor`() { val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener)) - val sut = fixture.getSut(eventListenerFactory = { originalListener }, httpStatusCode = 500) + val sut = fixture.getSut(eventListenerFactory = { originalListener }) val listener = fixture.sentryOkHttpEventListener val request = postRequest(body = "requestBody") val call = sut.newCall(request) @@ -305,6 +306,49 @@ class SentryOkHttpEventListenerTest { assertEquals(9, fixture.sentryTracer.children.size) } + @Test + fun `status propagates to parent span and call root span`() { + val sut = fixture.getSut(httpStatusCode = 500) + val request = getRequest() + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + val responseHeaderSpan = + fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.response_headers" } + val connectionSpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.connection" } + response.close() + assertNotNull(callSpan) + assertNotNull(responseHeaderSpan) + assertNotNull(connectionSpan) + assertEquals(SpanStatus.fromHttpStatusCode(500), callSpan.status) + assertEquals(SpanStatus.fromHttpStatusCode(500), responseHeaderSpan.status) + assertEquals(SpanStatus.fromHttpStatusCode(500), connectionSpan.status) + } + + @Test + fun `call root span status is not overridden if not null`() { + val mockListener = mock() + lateinit var call: Call + whenever(mockListener.connectStart(any(), anyOrNull(), anyOrNull())).then { + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + assertNotNull(callSpan) + assertNull(callSpan.status) + callSpan.status = SpanStatus.UNKNOWN + it + } + val sut = fixture.getSut(eventListener = mockListener) + val request = getRequest() + call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + assertNotNull(callSpan) + response.close() + assertEquals(SpanStatus.UNKNOWN, callSpan.status) + } + private fun verifyDelegation( listener: SentryOkHttpEventListener, originalListener: EventListener, diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index 54fe8cef65..d4d84d829d 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -8,6 +8,7 @@ import io.sentry.SentryTracer import io.sentry.Span import io.sentry.SpanDataConvention import io.sentry.SpanOptions +import io.sentry.SpanStatus import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import io.sentry.TypeCheckHint @@ -242,6 +243,28 @@ class SentryOkHttpEventTest { ) } + @Test + fun `when finishEvent, all running spans are finished with internal_error status`() { + val sut = fixture.getSut() + sut.startSpan("span") + val spans = sut.getEventSpans() + assertNull(spans["span"]!!.status) + sut.finishEvent() + assertEquals(SpanStatus.INTERNAL_ERROR, spans["span"]!!.status) + } + + @Test + fun `when finishEvent, does not override running spans status if set`() { + val sut = fixture.getSut() + sut.startSpan("span") + val spans = sut.getEventSpans() + assertNull(spans["span"]!!.status) + spans["span"]!!.status = SpanStatus.OK + assertEquals(SpanStatus.OK, spans["span"]!!.status) + sut.finishEvent() + assertEquals(SpanStatus.OK, spans["span"]!!.status) + } + @Test fun `setResponse set protocol and code in the breadcrumb and root span, and response in the hint`() { val sut = fixture.getSut() @@ -443,6 +466,32 @@ class SentryOkHttpEventTest { assertEquals(rootSpan.spanId, responseBodySpan?.parentSpanId) } + @Test + fun `finishSpan beforeFinish is called on span, parent and call root span`() { + val sut = fixture.getSut() + sut.startSpan(CONNECTION_EVENT) + sut.startSpan(REQUEST_HEADERS_EVENT) + sut.startSpan("random event") + sut.finishSpan(REQUEST_HEADERS_EVENT) { it.status = SpanStatus.INTERNAL_ERROR } + sut.finishSpan("random event") { it.status = SpanStatus.DEADLINE_EXCEEDED } + sut.finishSpan(CONNECTION_EVENT) + val spans = sut.getEventSpans() + val connectionSpan = spans[CONNECTION_EVENT] as Span? + val requestHeadersSpan = spans[REQUEST_HEADERS_EVENT] as Span? + val randomEventSpan = spans["random event"] as Span? + assertNotNull(connectionSpan) + assertNotNull(requestHeadersSpan) + assertNotNull(randomEventSpan) + // requestHeadersSpan was finished with INTERNAL_ERROR + assertEquals(SpanStatus.INTERNAL_ERROR, requestHeadersSpan.status) + // randomEventSpan was finished with DEADLINE_EXCEEDED + assertEquals(SpanStatus.DEADLINE_EXCEEDED, randomEventSpan.status) + // requestHeadersSpan was finished with INTERNAL_ERROR, and it propagates to its parent + assertEquals(SpanStatus.INTERNAL_ERROR, connectionSpan.status) + // random event was finished last with DEADLINE_EXCEEDED, and it propagates to root call + assertEquals(SpanStatus.DEADLINE_EXCEEDED, sut.callRootSpan!!.status) + } + /** Retrieve all the spans started in the event using reflection. */ private fun SentryOkHttpEvent.getEventSpans() = getProperty>("eventSpans") } From 5f2f4df0af6e7f285c902ee64b98c91fab7c3a67 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 31 Jul 2023 10:54:37 +0200 Subject: [PATCH 2/4] Bump gradle/gradle-build-action from 2.6.1 to 2.7.0 (#2874) --- .github/workflows/agp-matrix.yml | 2 +- .github/workflows/enforce-license-compliance.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/agp-matrix.yml b/.github/workflows/agp-matrix.yml index 407074dcdb..25e603de75 100644 --- a/.github/workflows/agp-matrix.yml +++ b/.github/workflows/agp-matrix.yml @@ -35,7 +35,7 @@ jobs: uses: actions/checkout@v3 - name: Setup Gradle - uses: gradle/gradle-build-action@915a66c096a03101667f9df2e56c9efef558b165 # pin@v2 + uses: gradle/gradle-build-action@a4cf152f482c7ca97ef56ead29bf08bcd953284c # pin@v2 - name: Setup Java Version uses: actions/setup-java@v3 diff --git a/.github/workflows/enforce-license-compliance.yml b/.github/workflows/enforce-license-compliance.yml index adf8b8b528..041348415f 100644 --- a/.github/workflows/enforce-license-compliance.yml +++ b/.github/workflows/enforce-license-compliance.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Setup Gradle - uses: gradle/gradle-build-action@915a66c096a03101667f9df2e56c9efef558b165 # pin@v2 + uses: gradle/gradle-build-action@a4cf152f482c7ca97ef56ead29bf08bcd953284c # pin@v2 - name: Set up Java uses: actions/setup-java@v3 From 4bf202b61a67469bdca1e9c69577abbdacd97472 Mon Sep 17 00:00:00 2001 From: Martin Prebio Date: Mon, 31 Jul 2023 17:56:19 +0200 Subject: [PATCH 3/4] Fix spelling typo (#2876) --- .../java/io/sentry/spring/boot/jakarta/SentryProperties.java | 2 +- .../src/main/java/io/sentry/spring/boot/SentryProperties.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java index 520cc03f1d..6b263a3303 100644 --- a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java +++ b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java @@ -14,7 +14,7 @@ @Open public class SentryProperties extends SentryOptions { - /** Weather to use Git commit id as a release. */ + /** Whether to use Git commit id as a release. */ private boolean useGitCommitIdAsRelease = true; /** Report all or only uncaught web exceptions. */ diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java index 6cd491f324..6172966a74 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -14,7 +14,7 @@ @Open public class SentryProperties extends SentryOptions { - /** Weather to use Git commit id as a release. */ + /** Whether to use Git commit id as a release. */ private boolean useGitCommitIdAsRelease = true; /** Report all or only uncaught web exceptions. */ From 4c237f8e846a3c88de8e49ccf44a59b4756d93da Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 2 Aug 2023 16:43:50 +0200 Subject: [PATCH 4/4] Add `sampled` to Dynamic Sampling Context (#2869) --- CHANGELOG.md | 6 +++- sentry/api/sentry.api | 6 ++++ sentry/src/main/java/io/sentry/Baggage.java | 28 +++++++++++++++++-- .../src/main/java/io/sentry/TraceContext.java | 22 +++++++++++++-- .../main/java/io/sentry/util/StringUtils.java | 8 ++++++ sentry/src/test/java/io/sentry/BaggageTest.kt | 3 +- .../test/java/io/sentry/JsonSerializerTest.kt | 8 +++--- .../sentry/TraceContextSerializationTest.kt | 3 +- .../json/sentry_envelope_header.json | 3 +- .../src/test/resources/json/trace_state.json | 3 +- 10 files changed, 76 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd801cf04a..5d8fb477ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,14 @@ ## Unreleased -- propagate okhttp status to parent spans ([#2872](https://github.com/getsentry/sentry-java/pull/2872)) +### Features + +- Add `sampled` to Dynamic Sampling Context ([#2869](https://github.com/getsentry/sentry-java/pull/2869)) ### Fixes +- Propagate OkHttp status to parent spans ([#2872](https://github.com/getsentry/sentry-java/pull/2872)) + ## 6.27.0 ### Features diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 10c1b80af3..a7f02708e3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -47,6 +47,7 @@ public final class io/sentry/Baggage { public fun getRelease ()Ljava/lang/String; public fun getSampleRate ()Ljava/lang/String; public fun getSampleRateDouble ()Ljava/lang/Double; + public fun getSampled ()Ljava/lang/String; public fun getThirdPartyHeader ()Ljava/lang/String; public fun getTraceId ()Ljava/lang/String; public fun getTransaction ()Ljava/lang/String; @@ -59,6 +60,7 @@ public final class io/sentry/Baggage { public fun setPublicKey (Ljava/lang/String;)V public fun setRelease (Ljava/lang/String;)V public fun setSampleRate (Ljava/lang/String;)V + public fun setSampled (Ljava/lang/String;)V public fun setTraceId (Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUserId (Ljava/lang/String;)V @@ -74,6 +76,7 @@ public final class io/sentry/Baggage$DSCKeys { public static final field ENVIRONMENT Ljava/lang/String; public static final field PUBLIC_KEY Ljava/lang/String; public static final field RELEASE Ljava/lang/String; + public static final field SAMPLED Ljava/lang/String; public static final field SAMPLE_RATE Ljava/lang/String; public static final field TRACE_ID Ljava/lang/String; public static final field TRANSACTION Ljava/lang/String; @@ -2293,6 +2296,7 @@ public final class io/sentry/TraceContext : io/sentry/JsonSerializable, io/sentr public fun getPublicKey ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; public fun getSampleRate ()Ljava/lang/String; + public fun getSampled ()Ljava/lang/String; public fun getTraceId ()Lio/sentry/protocol/SentryId; public fun getTransaction ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; @@ -2312,6 +2316,7 @@ public final class io/sentry/TraceContext$JsonKeys { public static final field ENVIRONMENT Ljava/lang/String; public static final field PUBLIC_KEY Ljava/lang/String; public static final field RELEASE Ljava/lang/String; + public static final field SAMPLED Ljava/lang/String; public static final field SAMPLE_RATE Ljava/lang/String; public static final field TRACE_ID Ljava/lang/String; public static final field TRANSACTION Ljava/lang/String; @@ -4305,6 +4310,7 @@ public final class io/sentry/util/StringUtils { public static fun join (Ljava/lang/CharSequence;Ljava/lang/Iterable;)Ljava/lang/String; public static fun normalizeUUID (Ljava/lang/String;)Ljava/lang/String; public static fun removeSurrounding (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; + public static fun toString (Ljava/lang/Object;)Ljava/lang/String; } public final class io/sentry/util/TracingUtils { diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 8329c7c051..945183fa83 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -140,6 +140,7 @@ public static Baggage fromEvent( baggage.setTransaction(event.getTransaction()); // we don't persist sample rate baggage.setSampleRate(null); + baggage.setSampled(null); baggage.freeze(); return baggage; } @@ -329,11 +330,21 @@ public void setTransaction(final @Nullable String transaction) { return get(DSCKeys.SAMPLE_RATE); } + @ApiStatus.Internal + public @Nullable String getSampled() { + return get(DSCKeys.SAMPLED); + } + @ApiStatus.Internal public void setSampleRate(final @Nullable String sampleRate) { set(DSCKeys.SAMPLE_RATE, sampleRate); } + @ApiStatus.Internal + public void setSampled(final @Nullable String sampled) { + set(DSCKeys.SAMPLED, sampled); + } + @ApiStatus.Internal public void set(final @NotNull String key, final @Nullable String value) { if (mutable) { @@ -374,6 +385,7 @@ public void setValuesFromTransaction( ? transaction.getName() : null); setSampleRate(sampleRateToString(sampleRate(samplingDecision))); + setSampled(StringUtils.toString(sampled(samplingDecision))); } @ApiStatus.Internal @@ -387,6 +399,7 @@ public void setValuesFromScope(final @NotNull Scope scope, final @NotNull Sentry setUserSegment(user != null ? getSegment(user) : null); setTransaction(null); setSampleRate(null); + setSampled(null); } private static @Nullable String getSegment(final @NotNull User user) { @@ -420,6 +433,14 @@ public void setValuesFromScope(final @NotNull Scope scope, final @NotNull Sentry return df.format(sampleRateAsDouble); } + private static @Nullable Boolean sampled(@Nullable TracesSamplingDecision samplingDecision) { + if (samplingDecision == null) { + return null; + } + + return samplingDecision.getSampled(); + } + private static boolean isHighQualityTransactionName( @Nullable TransactionNameSource transactionNameSource) { return transactionNameSource != null @@ -458,7 +479,8 @@ public TraceContext toTraceContext() { getUserId(), getUserSegment(), getTransaction(), - getSampleRate()); + getSampleRate(), + getSampled()); traceContext.setUnknown(getUnknown()); return traceContext; } else { @@ -476,6 +498,7 @@ public static final class DSCKeys { public static final String USER_SEGMENT = "sentry-user_segment"; public static final String TRANSACTION = "sentry-transaction"; public static final String SAMPLE_RATE = "sentry-sample_rate"; + public static final String SAMPLED = "sentry-sampled"; public static final List ALL = Arrays.asList( @@ -486,6 +509,7 @@ public static final class DSCKeys { ENVIRONMENT, USER_SEGMENT, TRANSACTION, - SAMPLE_RATE); + SAMPLE_RATE, + SAMPLED); } } diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index fa3bcfb0c2..ef2944a9e9 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -20,12 +20,13 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { private final @Nullable String userSegment; private final @Nullable String transaction; private final @Nullable String sampleRate; + private final @Nullable String sampled; @SuppressWarnings("unused") private @Nullable Map unknown; TraceContext(@NotNull SentryId traceId, @NotNull String publicKey) { - this(traceId, publicKey, null, null, null, null, null, null); + this(traceId, publicKey, null, null, null, null, null, null, null); } TraceContext( @@ -36,7 +37,8 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { @Nullable String userId, @Nullable String userSegment, @Nullable String transaction, - @Nullable String sampleRate) { + @Nullable String sampleRate, + @Nullable String sampled) { this.traceId = traceId; this.publicKey = publicKey; this.release = release; @@ -45,6 +47,7 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { this.userSegment = userSegment; this.transaction = transaction; this.sampleRate = sampleRate; + this.sampled = sampled; } @SuppressWarnings("UnusedMethod") @@ -89,6 +92,10 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { return sampleRate; } + public @Nullable String getSampled() { + return sampled; + } + /** * @deprecated only here to support parsing legacy JSON with non flattened user */ @@ -190,6 +197,7 @@ public static final class JsonKeys { public static final String USER_SEGMENT = "user_segment"; public static final String TRANSACTION = "transaction"; public static final String SAMPLE_RATE = "sample_rate"; + public static final String SAMPLED = "sampled"; } @Override @@ -216,6 +224,9 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger if (sampleRate != null) { writer.name(TraceContext.JsonKeys.SAMPLE_RATE).value(sampleRate); } + if (sampled != null) { + writer.name(TraceContext.JsonKeys.SAMPLED).value(sampled); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -241,6 +252,7 @@ public static final class Deserializer implements JsonDeserializer String userSegment = null; String transaction = null; String sampleRate = null; + String sampled = null; Map unknown = null; while (reader.peek() == JsonToken.NAME) { @@ -273,6 +285,9 @@ public static final class Deserializer implements JsonDeserializer case TraceContext.JsonKeys.SAMPLE_RATE: sampleRate = reader.nextStringOrNull(); break; + case TraceContext.JsonKeys.SAMPLED: + sampled = reader.nextStringOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); @@ -304,7 +319,8 @@ public static final class Deserializer implements JsonDeserializer userId, userSegment, transaction, - sampleRate); + sampleRate, + sampled); traceContext.setUnknown(unknown); reader.endObject(); return traceContext; diff --git a/sentry/src/main/java/io/sentry/util/StringUtils.java b/sentry/src/main/java/io/sentry/util/StringUtils.java index 0eef8e9cb3..2158576619 100644 --- a/sentry/src/main/java/io/sentry/util/StringUtils.java +++ b/sentry/src/main/java/io/sentry/util/StringUtils.java @@ -181,4 +181,12 @@ public static String join( return stringBuilder.toString(); } + + public static @Nullable String toString(final @Nullable Object object) { + if (object == null) { + return null; + } + + return object.toString(); + } } diff --git a/sentry/src/test/java/io/sentry/BaggageTest.kt b/sentry/src/test/java/io/sentry/BaggageTest.kt index da8bd2fcc8..eb1cfa0383 100644 --- a/sentry/src/test/java/io/sentry/BaggageTest.kt +++ b/sentry/src/test/java/io/sentry/BaggageTest.kt @@ -319,8 +319,9 @@ class BaggageTest { baggage.setUserId(userId) baggage.setUserSegment("segmentA") baggage.setSampleRate((1.0 / 3.0).toString()) + baggage.setSampled("true") - assertEquals("sentry-environment=production,sentry-public_key=$publicKey,sentry-release=1.0-rc.1,sentry-sample_rate=0.3333333333333333,sentry-trace_id=$traceId,sentry-transaction=TX,sentry-user_id=$userId,sentry-user_segment=segmentA", baggage.toHeaderString(null)) + assertEquals("sentry-environment=production,sentry-public_key=$publicKey,sentry-release=1.0-rc.1,sentry-sample_rate=0.3333333333333333,sentry-sampled=true,sentry-trace_id=$traceId,sentry-transaction=TX,sentry-user_id=$userId,sentry-user_segment=segmentA", baggage.toHeaderString(null)) } @Test diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 1c9243d2ff..3d07b3e37c 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -437,16 +437,16 @@ class JsonSerializerTest { @Test fun `serializes trace context`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "segment", "transaction", "0.5")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction","sample_rate":"0.5"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", "userId", "segment", "transaction", "0.5", "true")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","user_id":"userId","user_segment":"segment","transaction":"transaction","sample_rate":"0.5","sampled":"true"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } @Test fun `serializes trace context with user having null id and segment`() { - val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, null, "transaction", "0.6")) - val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6"}}""" + val traceContext = SentryEnvelopeHeader(null, null, TraceContext(SentryId("3367f5196c494acaae85bbbd535379ac"), "key", "release", "environment", null, null, "transaction", "0.6", "false")) + val expected = """{"trace":{"trace_id":"3367f5196c494acaae85bbbd535379ac","public_key":"key","release":"release","environment":"environment","transaction":"transaction","sample_rate":"0.6","sampled":"false"}}""" val json = serializeToString(traceContext) assertEquals(expected, json) } diff --git a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt index b928f791c1..e79e5ebf8c 100644 --- a/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/TraceContextSerializationTest.kt @@ -23,7 +23,8 @@ class TraceContextSerializationTest { "c052c566-6619-45f5-a61f-172802afa39a", "f7d8662b-5551-4ef8-b6a8-090f0561a530", "0252ec25-cd0a-4230-bd2f-936a4585637e", - "0.00000021" + "0.00000021", + "true" ) } private val fixture = Fixture() diff --git a/sentry/src/test/resources/json/sentry_envelope_header.json b/sentry/src/test/resources/json/sentry_envelope_header.json index ab5169ae06..14c144f820 100644 --- a/sentry/src/test/resources/json/sentry_envelope_header.json +++ b/sentry/src/test/resources/json/sentry_envelope_header.json @@ -26,7 +26,8 @@ "user_id": "c052c566-6619-45f5-a61f-172802afa39a", "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", - "sample_rate": "0.00000021" + "sample_rate": "0.00000021", + "sampled": "true" }, "sent_at": "2020-02-07T14:16:00.000Z" } diff --git a/sentry/src/test/resources/json/trace_state.json b/sentry/src/test/resources/json/trace_state.json index ff4a4f86d9..17a95fdc33 100644 --- a/sentry/src/test/resources/json/trace_state.json +++ b/sentry/src/test/resources/json/trace_state.json @@ -6,5 +6,6 @@ "user_id": "c052c566-6619-45f5-a61f-172802afa39a", "user_segment": "f7d8662b-5551-4ef8-b6a8-090f0561a530", "transaction": "0252ec25-cd0a-4230-bd2f-936a4585637e", - "sample_rate": "0.00000021" + "sample_rate": "0.00000021", + "sampled": "true" }