getsentry / team-sdks

A meta repository for tracking work across all SDK teams.
1 stars 0 forks source link

Explicity set `user.ip_address: {{auto}}` #118

Open cleptric opened 4 weeks ago

cleptric commented 4 weeks ago

Description

For client side SDKs, we need to explicitly set the user's IP address to {{auto}}, if the property was not set by the user.

By default, SDKs should not set {{auto}} and only do so if sendDefaultPii was set to true by the user.

Docs: https://develop.sentry.dev/sdk/expected-features/data-handling/

See https://github.com/getsentry/relay/issues/4431

Stakeholder(s)

@jjbayer

Team(s)

Mobile, Web Frontend, GDX

SDKs

## SDKs
- [x] Flutter SDK - Does already set `{{auto}}` https://github.com/getsentry/sentry-dart/blob/d1488a1bce9359342af5062f910a087dc2250e6e/dart/lib/src/sentry_client.dart#L35-L38
- [x] Dart SDK - Depends on Flutter SDK
- [x] Apple/Cocoa SDK https://github.com/getsentry/sentry-cocoa/blob/72e34fae44b817d8c12490bbc5c1ce7540f86762/Sources/Sentry/SentryClient.m#L781-L786
- [ ] https://github.com/getsentry/sentry-java/issues/4068
- [x] Kotlin Multiplatform SDK - depends on Apple/Cocoa and Java/Android SDK
- [x] Electron SDK - https://github.com/getsentry/sentry-electron/pull/1057
- [ ] JavaScript SDK - https://github.com/getsentry/sentry-javascript/pull/15008 & https://github.com/getsentry/sentry-javascript/issues/5347
- [x] .NET SDK - https://github.com/getsentry/sentry-dotnet/pull/3893
- [x] Unity SDK - Inherits it from the .NET SDK, see above.
- [x] PowerShell SDK - Inherits it from the .NET SDK, see above.
- [ ] Unreal SDK
- [ ] Capacitor SDK https://github.com/getsentry/sentry-capacitor/issues/819
- [ ] Cordova SDK https://github.com/getsentry/sentry-cordova/issues/369
- [ ] React Native SDK https://github.com/getsentry/sentry-react-native/issues/4462
bruno-garcia commented 4 weeks ago

@bitsandfoxes I'm pretty sure Unity does the right thing (the dotnet sdk that is, which would be true then for Unity, Xamarin and PowerShell. But could you please double check?

kahest commented 4 weeks ago

sentry-cocoa already sets {{auto}} in https://github.com/getsentry/sentry-cocoa/blob/72e34fae44b817d8c12490bbc5c1ce7540f86762/Sources/Sentry/SentryClient.m#L781-L786, marking as done

jjbayer commented 4 weeks ago

for spans, relay populates the client.address attribute (even if no explicit auto is set): https://github.com/getsentry/relay/blob/2e5e53191f4cf960b49bed86ab6564552084a0b1/relay-server/src/services/processor/span/processing.rs#L582-L590

for sessions there's the ip_address attribute: https://github.com/getsentry/relay/blob/2e5e53191f4cf960b49bed86ab6564552084a0b1/relay-event-schema/src/protocol/session.rs#L484-L496

lucas-zimerman commented 3 weeks ago

Issue created for Capacitor https://github.com/getsentry/sentry-capacitor/issues/819 and Cordova https://github.com/getsentry/sentry-cordova/issues/369

jjbayer commented 3 weeks ago

Current Relay behavior:

Data type auto-derive when IP missing auto-derive when IP is {{auto}}
Errors + Transactions only for platform:[js,cocoa] Yes
Sessions No Yes
Standalone (INP) spans Yes Yes
Replays only for platform:[js,cocoa] Yes

We can change the special casing for js and cocoa to a specific verion. Since INP spans also come from javascript, we can apply the same fallback behavior there, so the behavior is aligned across data types.

krystofwoldrich commented 3 weeks ago

@jjbayer What is covered by js, any sdk starting with sentry.javascript.*? Just checking what the behaviour will be for hybrid SDKs (RN, Flutter, ...).

jjbayer commented 2 weeks ago

@krystofwoldrich relay currently special cases its behavior if the event.platform is literally "javascript". I'm not sure what value the hybrid SDKs set for event.platform.

timfish commented 2 weeks ago

The Electron SDK uses javascript or native for event.platform depending on the source of the error/crash.

bruno-garcia commented 1 week ago

@tustanivsky could you take a look how it works on the Unreal SDK?

@limbonaut could you take a look how it works on the Godot SDK, in case we already added this to it?

krystofwoldrich commented 1 week ago

@jjbayer RN Uses platform "javascript" for JS events.

krystofwoldrich commented 1 week ago

Does sentry-native used in sentry-android also set the {{auto}} value?

JoshuaMoelans commented 1 week ago

@krystofwoldrich No, since we don't have sendDefaultPii in Native we expect the calling SDKs to set this.