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

Add enableTracing option #1395

Merged
merged 29 commits into from
Apr 24, 2023
Merged

Add enableTracing option #1395

merged 29 commits into from
Apr 24, 2023

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Apr 18, 2023

📜 Description

Add enableTracing to option

💡 Motivation and Context

Closes #1267

💚 How did you test it?

Unit tests

📝 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

@github-actions
Copy link
Contributor

github-actions bot commented Apr 18, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against c967cc5

@github-actions
Copy link
Contributor

github-actions bot commented Apr 18, 2023

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 368.45 ms 427.24 ms 58.79 ms
Size 6.06 MiB 7.03 MiB 993.54 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
11fb408 320.10 ms 380.24 ms 60.14 ms
fdac48a 329.50 ms 396.46 ms 66.96 ms
a7acb24 301.00 ms 357.38 ms 56.38 ms
eecbbca 324.37 ms 352.49 ms 28.12 ms
24f71aa 358.49 ms 455.90 ms 97.41 ms
2f8f173 323.31 ms 373.29 ms 49.97 ms
6957bfd 325.88 ms 380.30 ms 54.43 ms
6325c3b 339.33 ms 409.86 ms 70.53 ms
d7758e8 300.12 ms 349.88 ms 49.76 ms
40680d3 323.55 ms 390.29 ms 66.73 ms

App size

Revision Plain With Sentry Diff
11fb408 6.06 MiB 7.10 MiB 1.04 MiB
fdac48a 6.06 MiB 7.09 MiB 1.03 MiB
a7acb24 5.94 MiB 6.95 MiB 1.01 MiB
eecbbca 5.94 MiB 6.89 MiB 975.78 KiB
24f71aa 6.06 MiB 7.03 MiB 990.30 KiB
2f8f173 5.94 MiB 6.95 MiB 1.01 MiB
6957bfd 5.94 MiB 6.95 MiB 1.01 MiB
6325c3b 5.94 MiB 6.96 MiB 1.02 MiB
d7758e8 5.94 MiB 6.95 MiB 1.01 MiB
40680d3 6.06 MiB 7.03 MiB 989.25 KiB

Previous results on branch: feat/enable-tracing-option

Startup times

Revision Plain With Sentry Diff
0a972f1 330.45 ms 374.69 ms 44.25 ms

App size

Revision Plain With Sentry Diff
0a972f1 6.06 MiB 7.03 MiB 993.54 KiB

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.73 🎉

Comparison is base (0613b95) 89.97% compared to head (c967cc5) 90.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1395      +/-   ##
==========================================
+ Coverage   89.97%   90.71%   +0.73%     
==========================================
  Files         120       59      -61     
  Lines        3742     2025    -1717     
==========================================
- Hits         3367     1837    -1530     
+ Misses        375      188     -187     

see 179 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@denrase denrase changed the title Add enableTracing option Add enableTracing options Apr 18, 2023
@denrase denrase changed the title Add enableTracing options Add enableTracing option Apr 18, 2023
@denrase denrase marked this pull request as ready for review April 18, 2023 09:11
@denrase denrase requested a review from marandaneto April 18, 2023 11:11
@github-actions
Copy link
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1252.96 ms 1274.49 ms 21.53 ms
Size 8.10 MiB 9.17 MiB 1.08 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
453e1bc 1294.78 ms 1308.10 ms 13.33 ms
633cf2e 1257.96 ms 1275.73 ms 17.77 ms
322aa66 1251.68 ms 1275.52 ms 23.84 ms
aa950e9 1275.17 ms 1295.33 ms 20.16 ms
a49594a 1284.83 ms 1313.29 ms 28.45 ms
3e5ee37 1248.25 ms 1265.38 ms 17.13 ms
0be962b 1264.10 ms 1281.16 ms 17.06 ms
613760b 1263.10 ms 1277.27 ms 14.16 ms
134c9f8 1284.48 ms 1306.18 ms 21.70 ms
9928a74 1249.98 ms 1269.08 ms 19.10 ms

App size

Revision Plain With Sentry Diff
453e1bc 8.16 MiB 9.16 MiB 1.00 MiB
633cf2e 8.15 MiB 9.12 MiB 986.26 KiB
322aa66 8.15 MiB 9.12 MiB 992.53 KiB
aa950e9 8.16 MiB 9.17 MiB 1.01 MiB
a49594a 8.16 MiB 9.16 MiB 1.00 MiB
3e5ee37 8.15 MiB 9.12 MiB 986.23 KiB
0be962b 8.10 MiB 9.16 MiB 1.07 MiB
613760b 8.15 MiB 9.13 MiB 1000.46 KiB
134c9f8 8.16 MiB 9.16 MiB 1.01 MiB
9928a74 8.16 MiB 9.17 MiB 1.01 MiB

@denrase
Copy link
Collaborator Author

denrase commented Apr 18, 2023

@marandaneto Sry, pushed to the wrong branch. Will revert

@marandaneto
Copy link
Contributor

We probably need to update the docs, need to check which files have to be changed, consider looking at JS/iOS.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

Thanks @denrase

@marandaneto marandaneto enabled auto-merge (squash) April 20, 2023 12:07
@marandaneto
Copy link
Contributor

@denrase an unit test is failing still.

@marandaneto
Copy link
Contributor

@denrase an unit test is failing still.

@denrase

when no tracesSampleRate is set, do not sample with enableTracing null (failed)

@denrase denrase requested a review from marandaneto April 24, 2023 14:17
@marandaneto marandaneto merged commit 82250c4 into main Apr 24, 2023
@marandaneto marandaneto deleted the feat/enable-tracing-option branch April 24, 2023 15:07
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.

Add enableTracing option
2 participants