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: context defualts on native platforms other than iOS/Android #2439

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Nov 21, 2024

📜 Description

I've noticed windows doesn't have device context filled in. It's because originally we expected native integrations to sync from native iOS/Android to dart via LoadContexts(). This is not available with sentry-native which doesn't have such APIs.

To fix this, I've changed the logic to always set contexts in IoEnricherEventProcessor and made it so that it runs after the native _LoadContextsIntegrationEventProcessor. This way, the former will fill in the blanks all the time.

💡 Motivation and Context

💚 How did you test it?

📝 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

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.87%. Comparing base (25fc225) to head (ca374dd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2439      +/-   ##
==========================================
- Coverage   86.94%   86.87%   -0.08%     
==========================================
  Files         257      257              
  Lines        9238     9239       +1     
==========================================
- Hits         8032     8026       -6     
- Misses       1206     1213       +7     

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


🚨 Try these New Features:

Copy link
Contributor

github-actions bot commented Nov 21, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 471.09 ms 508.68 ms 37.59 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
62dde43 339.21 ms 423.06 ms 83.85 ms
5d2b46d 307.74 ms 349.85 ms 42.11 ms
4efee31 308.92 ms 368.68 ms 59.76 ms
4b943a1 348.17 ms 437.15 ms 88.98 ms
33d0587 308.79 ms 370.86 ms 62.07 ms
b98109e 296.46 ms 362.68 ms 66.22 ms
3de8b9b 348.55 ms 445.84 ms 97.29 ms
d53c6fa 282.83 ms 344.00 ms 61.17 ms
464b4d0 320.71 ms 380.02 ms 59.31 ms
2e8b1e1 367.00 ms 450.12 ms 83.12 ms

App size

Revision Plain With Sentry Diff
62dde43 5.94 MiB 6.96 MiB 1.02 MiB
5d2b46d 6.16 MiB 7.13 MiB 1000.82 KiB
4efee31 5.94 MiB 6.92 MiB 1003.76 KiB
4b943a1 6.34 MiB 7.28 MiB 968.41 KiB
33d0587 6.16 MiB 7.14 MiB 1007.47 KiB
b98109e 6.06 MiB 7.03 MiB 993.53 KiB
3de8b9b 6.27 MiB 7.20 MiB 957.75 KiB
d53c6fa 6.16 MiB 7.14 MiB 1011.18 KiB
464b4d0 6.06 MiB 7.03 MiB 990.27 KiB
2e8b1e1 6.35 MiB 7.40 MiB 1.05 MiB

Previous results on branch: fix/native-contexts-propagation

Startup times

Revision Plain With Sentry Diff
0eae47f 436.68 ms 472.85 ms 36.17 ms
c3fa61f 457.23 ms 520.26 ms 63.03 ms
0344dd8 475.67 ms 511.56 ms 35.89 ms

App size

Revision Plain With Sentry Diff
0eae47f 6.49 MiB 7.56 MiB 1.07 MiB
c3fa61f 6.49 MiB 7.56 MiB 1.07 MiB
0344dd8 6.49 MiB 7.56 MiB 1.07 MiB

Copy link
Contributor

github-actions bot commented Nov 21, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.43 ms 1256.87 ms 25.45 ms
Size 8.38 MiB 9.78 MiB 1.40 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d7758e8 1271.69 ms 1288.08 ms 16.39 ms
379d7a8 1267.65 ms 1288.39 ms 20.74 ms
0118295 1211.31 ms 1227.02 ms 15.71 ms
8cb6557 1265.14 ms 1266.08 ms 0.94 ms
bf4aed7 1274.63 ms 1286.48 ms 11.85 ms
0210372 1255.49 ms 1280.98 ms 25.49 ms
9811573 1259.78 ms 1278.33 ms 18.55 ms
6a5a65d 1237.22 ms 1250.29 ms 13.07 ms
1edf30e 1254.43 ms 1272.82 ms 18.39 ms
3a43905 1254.31 ms 1266.35 ms 12.04 ms

App size

Revision Plain With Sentry Diff
d7758e8 8.15 MiB 9.12 MiB 989.76 KiB
379d7a8 8.16 MiB 9.16 MiB 1.00 MiB
0118295 8.32 MiB 9.38 MiB 1.05 MiB
8cb6557 8.10 MiB 9.18 MiB 1.08 MiB
bf4aed7 8.10 MiB 9.17 MiB 1.08 MiB
0210372 8.38 MiB 9.73 MiB 1.35 MiB
9811573 8.16 MiB 9.17 MiB 1.01 MiB
6a5a65d 8.34 MiB 9.65 MiB 1.31 MiB
1edf30e 8.16 MiB 9.17 MiB 1.01 MiB
3a43905 8.10 MiB 9.18 MiB 1.08 MiB

Previous results on branch: fix/native-contexts-propagation

Startup times

Revision Plain With Sentry Diff
0eae47f 1246.22 ms 1267.33 ms 21.11 ms
c3fa61f 1242.08 ms 1267.04 ms 24.96 ms
0344dd8 1252.76 ms 1267.66 ms 14.90 ms

App size

Revision Plain With Sentry Diff
0eae47f 8.38 MiB 9.77 MiB 1.40 MiB
c3fa61f 8.38 MiB 9.77 MiB 1.40 MiB
0344dd8 8.38 MiB 9.77 MiB 1.40 MiB

@vaind vaind marked this pull request as ready for review November 22, 2024 09:01
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.

1 participant