Allow disabling tracing through SentryOptions. #1337
Conversation
When both tracesSampleRate = null and tracesSampler = null tracing is disabled.
| if (options.getTracesSampleRate() == null) { | ||
| options.setTracesSampleRate(0.0); | ||
| } |
There was a problem hiding this comment.
why do we need that here? isn't users choice to set it up?
There was a problem hiding this comment.
Spring has already a property "enable-tracing" which triggers auto configuration. If we don't do this change, users would have to set both enable-tracing=true and traces-sample-rate=0.0 to enable integration.
There was a problem hiding this comment.
but that's what is described here right https://docs.sentry.io/platforms/java/guides/spring-boot/performance/#enable-tracing
if it's a matter of not doing multiple things, I'd say that removing enable-tracing would be the way to go instead.
this is actually the only integration that requires more setup than traces-sample-rate or the traces callback
There was a problem hiding this comment.
We can do it but it will be breaking. Probably better to do it in another PR and leave this one as it is?
There was a problem hiding this comment.
As decided on Friday, I'll deprecate enable-tracing and make traces-sample-rate turn on integration too.
There was a problem hiding this comment.
@maciejwalkowiak do we need docs for this? getsentry/sentry-docs#3289
There was a problem hiding this comment.
Sure, I'll prepare a PR
There was a problem hiding this comment.
after deprecating enable-tracing, we should remove this right now, right?
There was a problem hiding this comment.
I don't think so. This keeps the old behavior - users only need to set enable-tracing=true to get tracing working. If we remove it, it is a breaking change.
…tion on custom traces sampler bean or `traces-sample-rate` property.
|
@marandaneto please have another look |
Codecov Report
@@ Coverage Diff @@
## main #1337 +/- ##
============================================
+ Coverage 75.64% 75.67% +0.02%
- Complexity 1845 1848 +3
============================================
Files 185 185
Lines 6320 6330 +10
Branches 627 629 +2
============================================
+ Hits 4781 4790 +9
Misses 1254 1254
- Partials 285 286 +1
Continue to review full report at Codecov.
|
|
@maciejwalkowiak a conflict to be fixed and GG |
📜 Description
Allow disabling tracing through SentryOptions. When both tracesSampleRate = null and tracesSampler = null tracing is disabled.
Since this behavior is in place,
sentry.enable-tracingproperty in Spring Boot integration is not needed anymore and it gets deprecated.💡 Motivation and Context
According to specs, users can disable tracing by setting both tracesSampleRate to null and tracesSampler to null.
💚 How did you test it?
Unit tests.
📝 Checklist
Breaking change: for users who have set
nullon bothtracesSampleRateandtracesSampler- transactions with explicit sampling decision and transactions with inherited sampling decision will not be send to Sentry anymore. To bring back the old behaviour users would have to settracesSampleRateto0.0.