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 #2530

Merged
merged 13 commits into from
Feb 15, 2023
Merged

Add enableTracing option #2530

merged 13 commits into from
Feb 15, 2023

Conversation

adinauer
Copy link
Member

📜 Description

Adds a new enableTracing option. If set to true, performance is enabled, even if no tracesSampleRate or tracesSampler have been configured. For now we set a default tracesSampleRate of 1.0 if none has been configured explicitly. If enableTracing is set to false performance is disabled, regardless of tracesSampleRate and tracesSampler options. The default for enableTracing is null meaning existing behaviour remains unchanged (setting either tracesSampleRate or tracesSampler enables performance)

💡 Motivation and Context

Closes #2522

💚 How did you test it?

Unit tests, manually using Android and Spring Boot (Jakarta)

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed. - Will be done for all SDKs at once.
  • [-] Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • [?] No breaking change for hybrid SDKs or communicated to hybrid SDKs.
    • Shouldn't break anything, existing behaviour should still work

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

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

Generated by 🚫 dangerJS against 5149d11

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 290.10 ms 346.25 ms 56.15 ms
Size 1.73 MiB 2.34 MiB 624.39 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4b32504 315.69 ms 373.96 ms 58.27 ms
754021c 358.70 ms 361.98 ms 3.28 ms
c1399c1 345.06 ms 385.49 ms 40.43 ms
5fa24ec 326.29 ms 384.53 ms 58.24 ms
fe30606 310.82 ms 335.36 ms 24.55 ms
f6a135d 263.96 ms 383.59 ms 119.63 ms
14c083a 350.82 ms 388.86 ms 38.04 ms
4b32504 357.14 ms 404.04 ms 46.90 ms
fe30606 327.46 ms 351.74 ms 24.28 ms

App size

Revision Plain With Sentry Diff
4b32504 1.73 MiB 2.34 MiB 623.74 KiB
754021c 1.73 MiB 2.33 MiB 623.06 KiB
c1399c1 1.73 MiB 2.33 MiB 620.61 KiB
5fa24ec 1.73 MiB 2.33 MiB 620.61 KiB
fe30606 1.73 MiB 2.34 MiB 623.74 KiB
f6a135d 1.73 MiB 2.33 MiB 623.10 KiB
14c083a 1.73 MiB 2.33 MiB 620.61 KiB
4b32504 1.73 MiB 2.34 MiB 623.74 KiB
fe30606 1.73 MiB 2.34 MiB 623.74 KiB

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

Startup times

Revision Plain With Sentry Diff
d6b4286 340.94 ms 385.46 ms 44.52 ms
9a2b68e 387.20 ms 444.36 ms 57.16 ms
72b2528 302.10 ms 378.77 ms 76.67 ms
f7a4308 303.88 ms 325.63 ms 21.75 ms

App size

Revision Plain With Sentry Diff
d6b4286 1.73 MiB 2.34 MiB 623.95 KiB
9a2b68e 1.73 MiB 2.34 MiB 623.95 KiB
72b2528 1.73 MiB 2.34 MiB 624.39 KiB
f7a4308 1.73 MiB 2.34 MiB 623.96 KiB

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Base: 80.20% // Head: 80.21% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (5149d11) compared to base (de52f97).
Patch coverage: 90.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2530      +/-   ##
============================================
+ Coverage     80.20%   80.21%   +0.01%     
- Complexity     3960     3970      +10     
============================================
  Files           324      324              
  Lines         14937    14952      +15     
  Branches       1968     1972       +4     
============================================
+ Hits          11980    11994      +14     
  Misses         2183     2183              
- Partials        774      775       +1     
Impacted Files Coverage Δ
...y/spring/boot/jakarta/SentryAutoConfiguration.java 97.01% <0.00%> (-1.50%) ⬇️
...io/sentry/spring/boot/SentryAutoConfiguration.java 97.01% <0.00%> (-1.50%) ⬇️
...entry/src/main/java/io/sentry/ExternalOptions.java 97.90% <100.00%> (+0.06%) ⬆️
sentry/src/main/java/io/sentry/SentryOptions.java 80.30% <100.00%> (+0.52%) ⬆️
sentry/src/main/java/io/sentry/TracesSampler.java 100.00% <100.00%> (ø)

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adinauer
Copy link
Member Author

CHANGELOG.md Show resolved Hide resolved
Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

a couple of small things, but LGTM otherwise, thanks 🚀

@adinauer adinauer merged commit 319f191 into main Feb 15, 2023
@adinauer adinauer deleted the feat/add-enable-tracing branch February 15, 2023 15:56
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