Skip to content

Commit

Permalink
Only send {{auto}} ip-adress if sendDefaultPii is enabled (8.x.x) (#4072
Browse files Browse the repository at this point in the history
)

* Only provide {{auto}} ip-address if sendDefaultPii is enabled

* Update Changelog

* Fix PR number

* Fix test
  • Loading branch information
markushi authored Jan 20, 2025
1 parent 0cf50f9 commit 23235b9
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 9 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Behavioural Changes

- The user ip-address is now only set to `"{{auto}}"` if sendDefaultPii is enabled ([#4072](https://github.com/getsentry/sentry-java/pull/4072))
- This change gives you control over IP address collection directly on the client

### Fixes

- Do not set the exception group marker when there is a suppressed exception ([#4056](https://github.com/getsentry/sentry-java/pull/4056))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
if (user.getId() == null) {
user.setId(getDeviceId());
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
if (user.getId() == null) {
user.setId(Installation.id(context));
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,20 @@ class AnrV2EventProcessorTest {
lateinit var context: Context
val options = SentryAndroidOptions().apply {
setLogger(NoOpLogger.getInstance())
isSendDefaultPii = true
}

fun getSut(
dir: TemporaryFolder,
currentSdk: Int = Build.VERSION_CODES.LOLLIPOP,
populateScopeCache: Boolean = false,
populateOptionsCache: Boolean = false,
replayErrorSampleRate: Double? = null
replayErrorSampleRate: Double? = null,
isSendDefaultPii: Boolean = true
): AnrV2EventProcessor {
options.cacheDirPath = dir.newFolder().absolutePath
options.environment = "release"
options.isSendDefaultPii = isSendDefaultPii

whenever(buildInfo.sdkInfoVersion).thenReturn(currentSdk)
whenever(buildInfo.isEmulator).thenReturn(true)

Expand Down Expand Up @@ -278,6 +280,7 @@ class AnrV2EventProcessorTest {
// user
assertEquals("bot", processed.user!!.username)
assertEquals("bot@me.com", processed.user!!.id)
assertEquals("{{auto}}", processed.user!!.ipAddress)
// trace
assertEquals("ui.load", processed.contexts.trace!!.operation)
// tags
Expand All @@ -304,6 +307,13 @@ class AnrV2EventProcessorTest {
assertEquals("Google Chrome", processed.contexts.browser!!.name)
}

@Test
fun `when backfillable event is enrichable, does not backfill user ip`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val processed = processEvent(hint, isSendDefaultPii = false, populateScopeCache = true)
assertNull(processed.user!!.ipAddress)
}

@Test
fun `when backfillable event is enrichable, backfills serialized options data`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
Expand Down Expand Up @@ -617,14 +627,16 @@ class AnrV2EventProcessorTest {
hint: Hint,
populateScopeCache: Boolean = false,
populateOptionsCache: Boolean = false,
isSendDefaultPii: Boolean = true,
configureEvent: SentryEvent.() -> Unit = {}
): SentryEvent {
val original = SentryEvent().apply(configureEvent)

val processor = fixture.getSut(
tmpDir,
populateScopeCache = populateScopeCache,
populateOptionsCache = populateOptionsCache
populateOptionsCache = populateOptionsCache,
isSendDefaultPii = isSendDefaultPii
)
return processor.process(original, hint)!!
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ class DefaultAndroidEventProcessorTest {

lateinit var sentryTracer: SentryTracer

fun getSut(context: Context): DefaultAndroidEventProcessor {
fun getSut(
context: Context,
isSendDefaultPii: Boolean = false
): DefaultAndroidEventProcessor {
options.isSendDefaultPii = isSendDefaultPii
whenever(scopes.options).thenReturn(options)
sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
return DefaultAndroidEventProcessor(context, buildInfo, options)
Expand Down Expand Up @@ -284,8 +288,20 @@ class DefaultAndroidEventProcessorTest {
}

@Test
fun `when event user data does not have ip address set, sets {{auto}} as the ip address`() {
val sut = fixture.getSut(context)
fun `when event user data does not have ip address set, sets no ip address if sendDefaultPii is false`() {
val sut = fixture.getSut(context, isSendDefaultPii = false)
val event = SentryEvent().apply {
user = User()
}
sut.process(event, Hint())
assertNotNull(event.user) {
assertNull(it.ipAddress)
}
}

@Test
fun `when event user data does not have ip address set, sets {{auto}} if sendDefaultPii is true`() {
val sut = fixture.getSut(context, isSendDefaultPii = true)
val event = SentryEvent().apply {
user = User()
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/MainEventProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
user = new User();
event.setUser(user);
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
10 changes: 10 additions & 0 deletions sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,16 @@ class MainEventProcessorTest {
}
}

@Test
fun `when event does not have ip address set, do not enrich ip address if sendDefaultPii is false`() {
val sut = fixture.getSut(sendDefaultPii = false)
val event = SentryEvent()
sut.process(event, Hint())
assertNotNull(event.user) {
assertNull(it.ipAddress)
}
}

@Test
fun `when event has ip address set, keeps original ip address`() {
val sut = fixture.getSut(sendDefaultPii = true)
Expand Down

0 comments on commit 23235b9

Please sign in to comment.