From 3a8904f142674aff484acaa816f63d84be94e76d Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 30 Jun 2022 14:51:46 +0200 Subject: [PATCH 1/3] Skip sending userId in DSC if send default pii is off --- .../src/main/java/io/sentry/TraceContext.java | 11 +++- .../test/java/io/sentry/SentryTracerTest.kt | 55 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index 83c2fda212..28a235797b 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -61,12 +61,21 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { new Dsn(sentryOptions.getDsn()).getPublicKey(), sentryOptions.getRelease(), sentryOptions.getEnvironment(), - user != null ? user.getId() : null, + getUserId(sentryOptions, user), user != null ? getSegment(user) : null, transaction.getName(), sampleRateToString(sampleRate(samplingDecision))); } + private static @Nullable String getUserId( + final @NotNull SentryOptions options, final @Nullable User user) { + if (options.isSendDefaultPii() && user != null) { + return user.getId(); + } + + return null; + } + private static @Nullable String getSegment(final @NotNull User user) { final Map others = user.getOthers(); if (others != null) { diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 527a2c9af0..99efac6892 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -481,6 +481,7 @@ class SentryTracerTest { fun `returns trace state`() { val transaction = fixture.getSut({ it.isTraceSampling = true + it.isSendDefaultPii = true }) fixture.hub.setUser( User().apply { @@ -500,6 +501,29 @@ class SentryTracerTest { } } + @Test + fun `returns trace state without userId if not send pii`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + }) + fixture.hub.setUser( + User().apply { + id = "user-id" + others = mapOf("segment" to "pro") + } + ) + val trace = transaction.traceContext() + assertNotNull(trace) { + assertEquals(transaction.spanContext.traceId, it.traceId) + assertEquals("key", it.publicKey) + assertEquals("environment", it.environment) + assertEquals("release@3.0.0", it.release) + assertEquals(transaction.name, it.transaction) + assertNull(it.userId) + assertEquals("pro", it.userSegment) + } + } + @Test fun `trace state does not change once computed`() { val transaction = fixture.getSut({ @@ -525,6 +549,7 @@ class SentryTracerTest { it.isTraceSampling = true it.environment = "production" it.release = "1.0.99-rc.7" + it.isSendDefaultPii = true }) fixture.hub.setUser( @@ -549,6 +574,36 @@ class SentryTracerTest { } } + @Test + fun `returns baggage header without userId if not sendp pii`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + it.environment = "production" + it.release = "1.0.99-rc.7" + }) + + fixture.hub.setUser( + User().apply { + id = "userId12345" + others = mapOf("segment" to "pro") + } + ) + + val header = transaction.toBaggageHeader() + assertNotNull(header) { + assertEquals("baggage", it.name) + assertNotNull(it.value) + println(it.value) + assertTrue(it.value.contains("sentry-trace_id=[^,]+".toRegex())) + assertTrue(it.value.contains("sentry-public_key=key,")) + assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) + assertTrue(it.value.contains("sentry-environment=production,")) + assertTrue(it.value.contains("sentry-transaction=name,")) + assertFalse(it.value.contains("sentry-user_id=userId12345,")) + assertTrue(it.value.contains("sentry-user_segment=pro$".toRegex())) + } + } + @Test fun `sets ITransaction data as extra in SentryTransaction`() { val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true)) From 5f8bfb6eb2cd6a2c5095866df5ae4e56296c4a34 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 30 Jun 2022 15:04:08 +0200 Subject: [PATCH 2/3] Add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99864a4ab8..855e5c3cc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Only send userid in Dynamic Sampling Context if sendDefaultPii is true ([#2147](https://github.com/getsentry/sentry-java/pull/2147)) + ### Features - New package `sentry-android-navigation` for AndroidX Navigation support ([#2136](https://github.com/getsentry/sentry-java/pull/2136)) From d7bf614bdb4e5cd83a41d57ac0b968573bbb9b7b Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 30 Jun 2022 17:03:16 +0200 Subject: [PATCH 3/3] Add test case --- .../test/java/io/sentry/SentryTracerTest.kt | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 99efac6892..c5bc1a37a6 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -575,7 +575,7 @@ class SentryTracerTest { } @Test - fun `returns baggage header without userId if not sendp pii`() { + fun `returns baggage header without userId if not send pii`() { val transaction = fixture.getSut({ it.isTraceSampling = true it.environment = "production" @@ -599,11 +599,37 @@ class SentryTracerTest { assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) assertTrue(it.value.contains("sentry-environment=production,")) assertTrue(it.value.contains("sentry-transaction=name,")) - assertFalse(it.value.contains("sentry-user_id=userId12345,")) + assertFalse(it.value.contains("sentry-user_id")) assertTrue(it.value.contains("sentry-user_segment=pro$".toRegex())) } } + @Test + fun `returns baggage header without userId if send pii and null user`() { + val transaction = fixture.getSut({ + it.isTraceSampling = true + it.environment = "production" + it.release = "1.0.99-rc.7" + it.isSendDefaultPii = true + }) + + fixture.hub.setUser(null) + + val header = transaction.toBaggageHeader() + assertNotNull(header) { + assertEquals("baggage", it.name) + assertNotNull(it.value) + println(it.value) + assertTrue(it.value.contains("sentry-trace_id=[^,]+".toRegex())) + assertTrue(it.value.contains("sentry-public_key=key,")) + assertTrue(it.value.contains("sentry-release=1.0.99-rc.7,")) + assertTrue(it.value.contains("sentry-environment=production,")) + assertTrue(it.value.contains("sentry-transaction=name")) + assertFalse(it.value.contains("sentry-user_id")) + assertFalse(it.value.contains("sentry-user_segment")) + } + } + @Test fun `sets ITransaction data as extra in SentryTransaction`() { val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true))