Skip to content

fix(sdk): Set {{auto}} if user.ip_address is undefined and sendDefaultPii: true #4466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 16, 2025

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Jan 20, 2025

📢 Type of change

  • Bugfix
  • Enhancement

Tasks

  • To align the behavior we need to add {{auto}} to RN v5 or upgrade to JS v9
  • And stop the Replay auto-fill for Cocoa and RN/JS.

❓ What to do for RN v6?

Map current behavior for RN JS, Cocoa, Android, and Android Native.

Plat Relay adds {{auto}} SDK adds {{auto}}
RN JS YES NO
Cocoa YES YES
Android NO YES
Android Native NO UNKNOWN

Related JS v9 changes

💡 Motivation and Context

💚 How did you test it?

unit tests

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

@lucas-zimerman
Copy link
Collaborator

This PR depends on JS v9 before releasing? if so will we bump it only on a major version of Sentry React Native?

krystofwoldrich added a commit to getsentry/sentry-javascript that referenced this pull request Feb 3, 2025
- Related to #15008

Export helpers to avoid duplications in sentry/react-native.

- RN PR getsentry/sentry-react-native#4466
@krystofwoldrich
Copy link
Member Author

This PR depends on JS v9 before releasing? if so will we bump it only on a major version of Sentry React Native?

Yes, this depends on JS v9, which also means a major version of RN.

@lucas-zimerman
Copy link
Collaborator

@krystofwoldrich we should point this PR towards v7, where #4568 will be merged, since this depends on JavaScript V9

@krystofwoldrich krystofwoldrich force-pushed the kw-handle-user-ip-address branch from f6555c1 to 7669fb0 Compare April 15, 2025 13:45
@krystofwoldrich krystofwoldrich changed the base branch from main to v7 April 15, 2025 13:51
@krystofwoldrich krystofwoldrich force-pushed the kw-handle-user-ip-address branch from 5d55818 to 5b1fd7e Compare April 15, 2025 14:49
@krystofwoldrich krystofwoldrich changed the base branch from v7 to kw-bump-latest-js April 15, 2025 14:50
@krystofwoldrich krystofwoldrich changed the title fix(sdk): Set {{auto}} if user.ip_address is undefined fix(sdk): Set {{auto}} if user.ip_address is undefined and sendDefaultPii: true Apr 15, 2025
@krystofwoldrich
Copy link
Member Author

This is ready for a review.

Copy link
Contributor

github-actions bot commented Apr 15, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 364.35 ms 368.44 ms 4.08 ms
Size 7.15 MiB 8.29 MiB 1.14 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e+dirty 399.70 ms 407.34 ms 7.64 ms

App size

Revision Plain With Sentry Diff
9da5c4e+dirty 7.15 MiB 8.42 MiB 1.27 MiB

Copy link
Contributor

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 424.23 ms 442.98 ms 18.75 ms
Size 17.75 MiB 20.15 MiB 2.41 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e 478.08 ms 467.46 ms -10.63 ms

App size

Revision Plain With Sentry Diff
9da5c4e 17.75 MiB 20.16 MiB 2.41 MiB

Copy link
Contributor

github-actions bot commented Apr 15, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.38 ms 1229.82 ms 0.44 ms
Size 2.63 MiB 3.77 MiB 1.14 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e+dirty 1231.84 ms 1235.49 ms 3.65 ms

App size

Revision Plain With Sentry Diff
9da5c4e+dirty 2.63 MiB 3.76 MiB 1.13 MiB

Copy link
Contributor

github-actions bot commented Apr 15, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.54 ms 1235.26 ms -3.29 ms
Size 3.19 MiB 4.34 MiB 1.15 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e+dirty 1215.41 ms 1226.38 ms 10.97 ms

App size

Revision Plain With Sentry Diff
9da5c4e+dirty 3.19 MiB 4.33 MiB 1.14 MiB

Base automatically changed from kw-bump-latest-js to v7 April 16, 2025 10:50
Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@krystofwoldrich krystofwoldrich merged commit 29876df into v7 Apr 16, 2025
72 of 74 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-handle-user-ip-address branch April 16, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explicity set user.ip_address: {{auto}}
4 participants