Only send userid in Dynamic Sampling Context if sendDefaultPii is true (

#2147) * Skip sending userId in DSC if send default pii is off * Add changelog * Add test case
getsentry · Jul 1, 2022 · 30e4ed6 · 30e4ed6
1 parent 685c725
commit 30e4ed6
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 1 deletion.
diff --git 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))

diff --git 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<String, String> 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
@@ -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,62 @@ class SentryTracerTest {
         }
     }
 
+    @Test
+    fun `returns baggage header without userId if not send 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"))
+            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))