Skip to content
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

fix: load contexts not setting the user #2089

Merged
merged 9 commits into from
Jun 17, 2024
Merged

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Jun 6, 2024

📜 Description

By default the user's ip address is now {{auto}} so the user object is never null, however when loading the native context we check if user == null then we retrieve it. and by default we get the ID that way. but since the user is never null now by default, we don't retrieve it anymore.

https://github.com/getsentry/sentry-dart/blob/main/flutter/lib/src/integrations/load_contexts_integration.dart#L113

Solution: move setting the default ipAddress to after the event processors have run

💡 Motivation and Context

Fixes #2062

💚 How did you test it?

Added another test + manual testing

📝 Checklist

  • I reviewed submitted code
  • 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
  • All tests passing
  • No breaking changes

🔮 Next steps

@buenaflor buenaflor requested a review from denrase June 6, 2024 18:33
@buenaflor buenaflor marked this pull request as draft June 6, 2024 18:33
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.25%. Comparing base (0b22d2d) to head (6ebd295).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2089   +/-   ##
=======================================
  Coverage   95.25%   95.25%           
=======================================
  Files          54       54           
  Lines        1791     1791           
=======================================
  Hits         1706     1706           
  Misses         85       85           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jun 6, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 375.50 ms 453.22 ms 77.72 ms
Size 6.35 MiB 7.34 MiB 1008.36 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8f88a49 364.98 ms 413.78 ms 48.80 ms
daa1b33 366.98 ms 451.59 ms 84.61 ms
04db237 330.16 ms 428.38 ms 98.22 ms
afa6e2a 349.73 ms 428.48 ms 78.75 ms
e2d89fc 323.84 ms 376.23 ms 52.39 ms
86d4841 286.35 ms 372.43 ms 86.08 ms
8609bd8 359.76 ms 437.40 ms 77.64 ms
72dfc83 298.62 ms 340.14 ms 41.52 ms
870f5eb 329.45 ms 369.29 ms 39.84 ms
f79eecf 341.35 ms 388.98 ms 47.63 ms

App size

Revision Plain With Sentry Diff
8f88a49 6.34 MiB 7.30 MiB 979.60 KiB
daa1b33 6.27 MiB 7.20 MiB 956.36 KiB
04db237 5.94 MiB 6.95 MiB 1.01 MiB
afa6e2a 6.27 MiB 7.20 MiB 955.69 KiB
e2d89fc 6.06 MiB 7.03 MiB 989.37 KiB
86d4841 6.15 MiB 7.13 MiB 1000.49 KiB
8609bd8 6.27 MiB 7.20 MiB 959.07 KiB
72dfc83 5.94 MiB 6.92 MiB 1001.71 KiB
870f5eb 5.94 MiB 6.92 MiB 1005.77 KiB
f79eecf 6.15 MiB 7.13 MiB 1000.07 KiB

Previous results on branch: fix/affected-user-metric

Startup times

Revision Plain With Sentry Diff
4182952 371.37 ms 441.41 ms 70.04 ms
e42f069 370.96 ms 450.21 ms 79.26 ms
39b67f2 444.76 ms 522.12 ms 77.37 ms
77e659f 426.14 ms 491.68 ms 65.54 ms
d14f206 410.54 ms 487.51 ms 76.97 ms

App size

Revision Plain With Sentry Diff
4182952 6.35 MiB 7.34 MiB 1008.09 KiB
e42f069 6.35 MiB 7.34 MiB 1008.08 KiB
39b67f2 6.35 MiB 7.33 MiB 1006.04 KiB
77e659f 6.35 MiB 7.34 MiB 1008.08 KiB
d14f206 6.35 MiB 7.34 MiB 1008.08 KiB

Copy link
Contributor

github-actions bot commented Jun 6, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1216.92 ms 1243.81 ms 26.89 ms
Size 8.33 MiB 9.55 MiB 1.21 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
3f23617 1261.93 ms 1286.10 ms 24.17 ms
ca7f531 1242.51 ms 1282.61 ms 40.10 ms
d783424 1265.71 ms 1277.00 ms 11.29 ms
754cdbe 1261.67 ms 1280.88 ms 19.20 ms
a7acb24 1296.71 ms 1317.69 ms 20.98 ms
f2db4ec 1244.14 ms 1259.79 ms 15.65 ms
25e9b59 1289.76 ms 1295.27 ms 5.51 ms
683fd34 1239.83 ms 1259.08 ms 19.25 ms
b8562d0 1249.92 ms 1267.56 ms 17.64 ms
3e9fb0e 1262.49 ms 1280.65 ms 18.16 ms

App size

Revision Plain With Sentry Diff
3f23617 8.16 MiB 9.17 MiB 1.01 MiB
ca7f531 8.32 MiB 9.38 MiB 1.06 MiB
d783424 8.29 MiB 9.38 MiB 1.09 MiB
754cdbe 8.29 MiB 9.37 MiB 1.08 MiB
a7acb24 8.16 MiB 9.17 MiB 1.01 MiB
f2db4ec 8.10 MiB 9.16 MiB 1.07 MiB
25e9b59 8.16 MiB 9.15 MiB 1021.15 KiB
683fd34 8.28 MiB 9.34 MiB 1.06 MiB
b8562d0 8.33 MiB 9.54 MiB 1.22 MiB
3e9fb0e 8.15 MiB 9.12 MiB 989.77 KiB

Previous results on branch: fix/affected-user-metric

Startup times

Revision Plain With Sentry Diff
e42f069 1249.78 ms 1266.40 ms 16.62 ms
39b67f2 1238.86 ms 1258.29 ms 19.43 ms
d14f206 1226.80 ms 1247.76 ms 20.96 ms
4182952 1256.10 ms 1277.30 ms 21.20 ms

App size

Revision Plain With Sentry Diff
e42f069 8.33 MiB 9.55 MiB 1.21 MiB
39b67f2 8.33 MiB 9.54 MiB 1.22 MiB
d14f206 8.33 MiB 9.55 MiB 1.21 MiB
4182952 8.33 MiB 9.55 MiB 1.21 MiB

@buenaflor buenaflor marked this pull request as ready for review June 10, 2024 08:52
@buenaflor buenaflor marked this pull request as draft June 10, 2024 09:58
@buenaflor buenaflor marked this pull request as ready for review June 10, 2024 11:07
Copy link
Collaborator

@denrase denrase left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@buenaflor buenaflor merged commit 3cac5bf into main Jun 17, 2024
120 of 122 checks passed
@buenaflor buenaflor deleted the fix/affected-user-metric branch June 17, 2024 11:31
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.

Affected user metrics are off
2 participants