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

Remove code that set tracesSampleRate to 0.0 for Spring Boot if not set #2800

Merged
merged 26 commits into from
Jul 3, 2023

Conversation

adinauer
Copy link
Member

@adinauer adinauer commented Jun 26, 2023

📜 Description

Remove some workaround code that was there to not require users to set a flag (that is no longer there).

💡 Motivation and Context

This caused sentryOptions.isTracingEnabled() to return true (as it should in this case) for a fresh setup without any performance options set causing Tracing without Performance code (#2788) not to be used.

💚 How did you test it?

Manually

📝 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.
  • 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.

🔮 Next steps

adinauer and others added 22 commits June 14, 2023 22:12
- continueTrace now returns a TransactionContext making it easier to
  create a transaction from incoming headers
- TransactionContext now has defaults for name, source and op and
  setters for them
- deprecate traceHeaders() and replace with getTraceparent()
- rename baggageHeader() to getBaggage()
- getBaggage() no longer takes pre existing headers
- add trace context to envelope payload from scope if none present and
  no span on scope
- improve JavaDoc
- create new PropagationContext when cloning scope
@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

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

Generated by 🚫 dangerJS against 09c8022

@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 289.06 ms 330.31 ms 41.24 ms
Size 1.72 MiB 2.29 MiB 574.43 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c03a05e 390.40 ms 419.35 ms 28.94 ms
dc67004 273.86 ms 346.37 ms 72.51 ms
9246ed4 275.63 ms 321.31 ms 45.69 ms
496bdfd 301.22 ms 343.96 ms 42.73 ms
8820c5c 330.60 ms 416.86 ms 86.26 ms
0310da5 381.20 ms 404.50 ms 23.30 ms
496bdfd 272.86 ms 407.33 ms 134.48 ms
9246ed4 281.79 ms 352.08 ms 70.29 ms

App size

Revision Plain With Sentry Diff
c03a05e 1.72 MiB 2.29 MiB 574.43 KiB
dc67004 1.72 MiB 2.28 MiB 573.45 KiB
9246ed4 1.72 MiB 2.28 MiB 572.22 KiB
496bdfd 1.72 MiB 2.28 MiB 571.82 KiB
8820c5c 1.72 MiB 2.28 MiB 571.82 KiB
0310da5 1.72 MiB 2.28 MiB 573.45 KiB
496bdfd 1.72 MiB 2.28 MiB 571.82 KiB
9246ed4 1.72 MiB 2.28 MiB 572.22 KiB

Previous results on branch: feat/remove-spring-traces-sample-rate-workaround

Startup times

Revision Plain With Sentry Diff
05075c5 299.04 ms 346.10 ms 47.06 ms
9b77d44 281.06 ms 370.00 ms 88.94 ms

App size

Revision Plain With Sentry Diff
05075c5 1.72 MiB 2.29 MiB 574.44 KiB
9b77d44 1.72 MiB 2.29 MiB 574.44 KiB

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (581d779) 81.25% compared to head (09c8022) 81.26%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2800   +/-   ##
=========================================
  Coverage     81.25%   81.26%           
  Complexity     4560     4560           
=========================================
  Files           350      350           
  Lines         16870    16866    -4     
  Branches       2274     2272    -2     
=========================================
- Hits          13708    13706    -2     
  Misses         2219     2219           
+ Partials        943      941    -2     
Impacted Files Coverage Δ
...y/spring/boot/jakarta/SentryAutoConfiguration.java 98.55% <ø> (+1.36%) ⬆️
...io/sentry/spring/boot/SentryAutoConfiguration.java 98.55% <ø> (+1.36%) ⬆️

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

Copy link
Member

@stefanosiano stefanosiano left a comment

Choose a reason for hiding this comment

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

Looks fine, but could it be considered a breaking/behavioural change?
If so, we should at least point it out better in the changelog

Base automatically changed from feat/tracing-without-performance to main June 30, 2023 08:35
@adinauer
Copy link
Member Author

adinauer commented Jul 3, 2023

Looks fine, but could it be considered a breaking/behavioural change?
If so, we should at least point it out better in the changelog

I added some more clarification to the changelog.

@adinauer adinauer merged commit c0ad3b3 into main Jul 3, 2023
@adinauer adinauer deleted the feat/remove-spring-traces-sample-rate-workaround branch July 3, 2023 11: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.

3 participants