Send user.ip_address = {{auto}} when sendDefaultPii is true

[maciejwalkowiak]

📜 Description

Send user.ip_address = {{ auto }} when sendDefaultPii is true

💡 Motivation and Context

Fixes #1015

💚 How did you test it?

Unit tests.

📝 Checklist

• I reviewed submitted code
• I added tests to verify changes
• All tests passing
• No breaking changes

[codecov-io]

Codecov Report

Merging #1144 (f7d952c) into main (32d4471) will increase coverage by 0.07%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main    #1144      +/-   ##
============================================
+ Coverage     75.16%   75.23%   +0.07%     
- Complexity     1620     1627       +7     
============================================
  Files           167      167              
  Lines          5624     5637      +13     
  Branches        549      555       +6     
============================================
+ Hits           4227     4241      +14     
+ Misses         1146     1144       -2     
- Partials        251      252       +1     

Impacted Files	Coverage Δ	Complexity Δ
...io/sentry/spring/boot/SentryAutoConfiguration.java	93.61% <ø> (ø)	1.00 <0.00> (ø)
.../io/sentry/spring/SentryInitBeanPostProcessor.java	92.30% <ø> (ø)	5.00 <0.00> (ø)
...ry/src/main/java/io/sentry/MainEventProcessor.java	78.12% <71.42%> (-0.83%)	22.00 <0.00> (+2.00)	⬇️
...entry/spring/SentryUserProviderEventProcessor.java	96.15% <100.00%> (+1.15%)	10.00 <1.00> (+3.00)
sentry/src/main/java/io/sentry/SentryOptions.java	85.50% <0.00%> (+1.11%)	112.00% <0.00%> (+2.00%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32d4471...f7d952c. Read the comment docs.

[bruno-garcia]

You probably want to undo this in the spring integration, since {{auto}} there will give the IP of the server.

Like check for default and set null instead.

[maciejwalkowiak]

In such case wouldn't it be better if we make it an opt in which is pre-configured in Android? Undoing it in Spring looks for me like a workaround.

[bruno-garcia]

@maciejwalkowiak That means desktop apps would have to opt-in too.
I think it's fair to say: It's enabled by default but you can opt-out. And we know better in some integrations to opt out already.

We need to document this behavior. Similar to how the default of another option is documented here:
getsentry/sentry-docs#2815

[marandaneto]

if we check for {{auto}} and set it to null if isSendDefaultPii is disabled would be a breaking change btw which is fine as 4.0 is a major bump, but yeah we'd need to document.

Also, it could be written {{ auto }} with spaces, so we'd need to trim spaces off if we want to compare that properly as well, otherwise, it'd not be a match and IPs will be set by the server.

[marandaneto]

this is breaking the Android user, see:

https://github.com/getsentry/sentry-java/blob/main/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java#L151-L154

now the user will never be null (if isSendDefaultPii enabled), we need to fix the if condition in the DefaultAndroidEventProcessor class

[maciejwalkowiak]

Undoing in Spring integration ✅
Updating Android code ✅

[marandaneto]

this has been merged but there's a bug here.

{{ auto }} is valid
{{auto}} is also valid

previously {{auto}} was not valid and its been fixed on the backend, so people were setting {{ auto }} manually, those people, it won't work as the comparison is not trimming off the spaces.

[maciejwalkowiak]

Got it. Will be fixed in next PR.