Skip to content

Add enableTracing option#1395

Merged
marandaneto merged 29 commits into
mainfrom
feat/enable-tracing-option
Apr 24, 2023
Merged

Add enableTracing option#1395
marandaneto merged 29 commits into
mainfrom
feat/enable-tracing-option

Conversation

@denrase

@denrase denrase commented Apr 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📜 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

github-actions Bot commented Apr 18, 2023 •
edited
Loading

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

Generated by 🚫 dangerJS against c967cc5

@github-actions

github-actions Bot commented Apr 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Apr 18, 2023 •
edited
Loading

Copy link
Copy Markdown

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
marandaneto
marandaneto reviewed Apr 18, 2023
View reviewed changes
Comment thread dart/lib/src/sentry_traces_sampler.dart Outdated
@denrase
denrase requested a review from marandaneto April 18, 2023 11:11
@github-actions

github-actions Bot commented Apr 18, 2023

Copy link
Copy Markdown
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

denrase commented Apr 18, 2023

Copy link
Copy Markdown
Collaborator Author

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

marandaneto
marandaneto reviewed Apr 18, 2023
View reviewed changes
Comment thread dart/lib/src/sentry_traces_sampler.dart Outdated
@marandaneto

marandaneto commented Apr 18, 2023

Copy link
Copy Markdown
Contributor

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

marandaneto
marandaneto approved these changes Apr 20, 2023
View reviewed changes

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Contributor

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

marandaneto commented Apr 20, 2023

Copy link
Copy Markdown
Contributor

@denrase an unit test is failing still.

@marandaneto

marandaneto commented Apr 24, 2023

Copy link
Copy Markdown
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
@NOBLES5E NOBLES5E mentioned this pull request Apr 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brustolin brustolin Awaiting requested review from brustolin
@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich
1 more reviewer
@marandaneto marandaneto marandaneto approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add enableTracing option

2 participants

@denrase @marandaneto