Skip to content

Commit

Permalink
Allow disabling tracing through SentryOptions. (#1337)
Browse files Browse the repository at this point in the history
* Allow disabling tracing through SentryOptions.

- When both tracesSampleRate = null and tracesSampler = null tracing is disabled.
- Deprecate `SentryProperties#enableTracing` and trigger auto-configuration on custom traces sampler bean or `traces-sample-rate` property.
  • Loading branch information
maciejwalkowiak committed Mar 24, 2021
1 parent e1a7226 commit bc99848
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* Fix: Allow 0.0 to be set on tracesSampleRate (#1328)
* Fix: set "java" platform to transactions #1332
* Feat: Add OkHttp client application interceptor (#1330)
* Fix: Allow disabling tracing through SentryOptions (#1337)

# 4.3.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio
}

private boolean isPerformanceEnabled(final @NotNull SentryAndroidOptions options) {
return ((options.getTracesSampleRate() != null || options.getTracesSampler() != null)
&& options.isEnableAutoActivityLifecycleTracing());
return options.isTracingEnabled() && options.isEnableAutoActivityLifecycleTracing();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ sentry.max-breadcrumbs=150
sentry.logging.minimum-event-level=info
sentry.logging.minimum-breadcrumb-level=debug
# Performance configuration
sentry.enable-tracing=true
sentry.traces-sample-rate=1.0
sentry.debug=true
in-app-includes="io.sentry.samples"
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.condition.AnyNestedCondition;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
Expand All @@ -35,6 +37,7 @@
import org.springframework.boot.info.GitProperties;
import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.core.Ordered;
Expand Down Expand Up @@ -103,6 +106,9 @@ static class HubConfiguration {

options.setSentryClientName(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME);
options.setSdkVersion(createSdkVersion(options));
if (options.getTracesSampleRate() == null) {
options.setTracesSampleRate(0.0);
}
Sentry.init(options);
return HubAdapter.getInstance();
}
Expand Down Expand Up @@ -133,7 +139,7 @@ static class SentryWebMvcConfiguration {
}

@Bean
@ConditionalOnProperty(name = "sentry.enable-tracing", havingValue = "true")
@Conditional(SentryTracingCondition.class)
@ConditionalOnMissingBean(name = "sentryTracingFilter")
public FilterRegistrationBean<SentryTracingFilter> sentryTracingFilter(
final @NotNull IHub hub, final @NotNull SentryRequestResolver sentryRequestResolver) {
Expand All @@ -145,7 +151,7 @@ public FilterRegistrationBean<SentryTracingFilter> sentryTracingFilter(
}

@Configuration(proxyBeanMethods = false)
@ConditionalOnProperty(name = "sentry.enable-tracing", havingValue = "true")
@Conditional(SentryTracingCondition.class)
@ConditionalOnClass(ProceedingJoinPoint.class)
@Import(SentryAdviceConfiguration.class)
@Open
Expand Down Expand Up @@ -200,4 +206,23 @@ static class ApacheHttpClientTransportFactoryAutoconfiguration {
return sdkVersion;
}
}

static final class SentryTracingCondition extends AnyNestedCondition {

public SentryTracingCondition() {
super(ConfigurationPhase.REGISTER_BEAN);
}

@ConditionalOnProperty(name = "sentry.enable-tracing", havingValue = "true")
@SuppressWarnings("UnusedNestedClass")
private static class SentryTracingEnabled {}

@ConditionalOnProperty(name = "sentry.traces-sample-rate")
@SuppressWarnings("UnusedNestedClass")
private static class SentryTracesSampleRateCondition {}

@ConditionalOnBean(SentryOptions.TracesSamplerCallback.class)
@SuppressWarnings("UnusedNestedClass")
private static class SentryTracesSamplerBeanCondition {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ public class SentryProperties extends SentryOptions {
/** Weather to use Git commit id as a release. */
private boolean useGitCommitIdAsRelease = true;

/** Turns tracing on or off. Default is disabled. */
private boolean enableTracing;
/**
* Turns tracing on or off. Default is disabled.
*
* @deprecated use {{@link SentryProperties#setTracesSampleRate(Double)} } or create {@link
* io.sentry.SentryOptions.TracesSamplerCallback} bean instead
*/
@Deprecated private boolean enableTracing;

/** Report all or only uncaught web exceptions. */
private int exceptionResolverOrder = 1;
Expand All @@ -32,10 +37,12 @@ public void setUseGitCommitIdAsRelease(boolean useGitCommitIdAsRelease) {
this.useGitCommitIdAsRelease = useGitCommitIdAsRelease;
}

@Deprecated
public boolean isEnableTracing() {
return enableTracing;
}

@Deprecated
public void setEnableTracing(boolean enableTracing) {
this.enableTracing = enableTracing;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguratio
import org.springframework.boot.context.annotation.UserConfigurations
import org.springframework.boot.info.GitProperties
import org.springframework.boot.test.context.FilteredClassLoader
import org.springframework.boot.test.context.assertj.ApplicationContextAssert
import org.springframework.boot.test.context.runner.WebApplicationContextRunner
import org.springframework.boot.web.servlet.FilterRegistrationBean
import org.springframework.context.ApplicationContext
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.Ordered
Expand Down Expand Up @@ -170,6 +172,31 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `when traces sample rate is set to null and tracing is enabled, traces sample rate should be set to 0`() {
contextRunner.withPropertyValues(
"sentry.dsn=http://key@localhost/proj",
"sentry.enable-tracing=true"
).run {
val options = it.getBean(SentryProperties::class.java)
assertThat(options.isEnableTracing).isTrue()
assertThat(options.tracesSampleRate).isNotNull().isEqualTo(0.0)
}
}

@Test
fun `when traces sample rate is set to a value and tracing is enabled, traces sample rate should not be overwritten`() {
contextRunner.withPropertyValues(
"sentry.dsn=http://key@localhost/proj",
"sentry.enable-tracing=true",
"sentry.traces-sample-rate=0.3"
).run {
val options = it.getBean(SentryProperties::class.java)
assertThat(options.isEnableTracing).isTrue()
assertThat(options.tracesSampleRate).isNotNull().isEqualTo(0.3)
}
}

@Test
fun `sets sentryClientName property on SentryOptions`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
Expand Down Expand Up @@ -318,6 +345,39 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `when traces sample rate is set, creates tracing filter`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=0.2")
.run {
assertThat(it).hasBean("sentryTracingFilter")
}
}

@Test
fun `when enable tracing is set to false and traces sample rate is set, creates tracing filter`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=false", "sentry.traces-sample-rate=0.2")
.run {
assertThat(it).hasBean("sentryTracingFilter")
}
}

@Test
fun `when traces sample rate is set to 0, creates tracing filter`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=0.0")
.run {
assertThat(it).hasBean("sentryTracingFilter")
}
}

@Test
fun `when custom traces sampler callback is registered, creates tracing filter`() {
contextRunner.withUserConfiguration(CustomTracesSamplerCallbackConfiguration::class.java)
.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it).hasBean("sentryTracingFilter")
}
}

@Test
fun `when tracing is set, does not create tracing filter`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
Expand Down Expand Up @@ -354,19 +414,32 @@ class SentryAutoConfigurationTest {
fun `when tracing is enabled creates AOP beans to support @SentryTransaction`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=true")
.run {
assertThat(it).hasBean("sentryTransactionPointcut")
assertThat(it).hasBean("sentryTransactionAdvice")
assertThat(it).hasBean("sentryTransactionAdvisor")
assertThat(it).hasSentryTransactionBeans()
}
}

@Test
fun `when traces sample rate is set to 0, creates AOP beans to support @SentryTransaction`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=0.0")
.run {
assertThat(it).hasSentryTransactionBeans()
}
}

@Test
fun `when custom traces sampler callback is registered, creates AOP beans to support @SentryTransaction`() {
contextRunner.withUserConfiguration(CustomTracesSamplerCallbackConfiguration::class.java)
.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it).hasSentryTransactionBeans()
}
}

@Test
fun `when tracing is disabled, does not create AOP beans to support @SentryTransaction`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=false")
.run {
assertThat(it).doesNotHaveBean("sentryTransactionPointcut")
assertThat(it).doesNotHaveBean("sentryTransactionAdvice")
assertThat(it).doesNotHaveBean("sentryTransactionAdvisor")
assertThat(it).doesNotHaveSentryTransactionBeans()
}
}

Expand All @@ -375,9 +448,7 @@ class SentryAutoConfigurationTest {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=true")
.withClassLoader(FilteredClassLoader(ProceedingJoinPoint::class.java))
.run {
assertThat(it).doesNotHaveBean("sentryTransactionPointcut")
assertThat(it).doesNotHaveBean("sentryTransactionAdvice")
assertThat(it).doesNotHaveBean("sentryTransactionAdvisor")
assertThat(it).doesNotHaveSentryTransactionBeans()
}
}

Expand All @@ -396,19 +467,32 @@ class SentryAutoConfigurationTest {
fun `when tracing is enabled creates AOP beans to support @SentrySpan`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=true")
.run {
assertThat(it).hasBean("sentrySpanPointcut")
assertThat(it).hasBean("sentrySpanAdvice")
assertThat(it).hasBean("sentrySpanAdvisor")
assertThat(it).hasSentrySpanBeans()
}
}

@Test
fun `when traces sample rate is set to 0, creates AOP beans to support @SentrySpan`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.traces-sample-rate=0.0")
.run {
assertThat(it).hasSentrySpanBeans()
}
}

@Test
fun `when custom traces sampler callback is registered, creates AOP beans to support @SentrySpan`() {
contextRunner.withUserConfiguration(CustomTracesSamplerCallbackConfiguration::class.java)
.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it).hasSentrySpanBeans()
}
}

@Test
fun `when tracing is disabled, does not create AOP beans to support @Span`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=false")
.run {
assertThat(it).doesNotHaveBean("sentrySpanPointcut")
assertThat(it).doesNotHaveBean("sentrySpanAdvice")
assertThat(it).doesNotHaveBean("sentrySpanAdvisor")
assertThat(it).doesNotHaveSentrySpanBeans()
}
}

Expand All @@ -417,9 +501,7 @@ class SentryAutoConfigurationTest {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj", "sentry.enable-tracing=true")
.withClassLoader(FilteredClassLoader(ProceedingJoinPoint::class.java))
.run {
assertThat(it).doesNotHaveBean("sentrySpanPointcut")
assertThat(it).doesNotHaveBean("sentrySpanAdvice")
assertThat(it).doesNotHaveBean("sentrySpanAdvisor")
assertThat(it).doesNotHaveSentrySpanBeans()
}
}

Expand Down Expand Up @@ -640,4 +722,32 @@ class SentryAutoConfigurationTest {
return user
}
}

private fun <C : ApplicationContext> ApplicationContextAssert<C>.hasSentryTransactionBeans(): ApplicationContextAssert<C> {
this.hasBean("sentryTransactionPointcut")
this.hasBean("sentryTransactionAdvice")
this.hasBean("sentryTransactionAdvisor")
return this
}

private fun <C : ApplicationContext> ApplicationContextAssert<C>.doesNotHaveSentryTransactionBeans(): ApplicationContextAssert<C> {
this.doesNotHaveBean("sentryTransactionPointcut")
this.doesNotHaveBean("sentryTransactionAdvice")
this.doesNotHaveBean("sentryTransactionAdvisor")
return this
}

private fun <C : ApplicationContext> ApplicationContextAssert<C>.hasSentrySpanBeans(): ApplicationContextAssert<C> {
this.hasBean("sentrySpanPointcut")
this.hasBean("sentrySpanAdvice")
this.hasBean("sentrySpanAdvisor")
return this
}

private fun <C : ApplicationContext> ApplicationContextAssert<C>.doesNotHaveSentrySpanBeans(): ApplicationContextAssert<C> {
this.doesNotHaveBean("sentrySpanPointcut")
this.doesNotHaveBean("sentrySpanAdvice")
this.doesNotHaveBean("sentrySpanAdvisor")
return this
}
}
1 change: 1 addition & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ public class io/sentry/SentryOptions {
public fun isEnableSessionTracking ()Z
public fun isEnableUncaughtExceptionHandler ()Z
public fun isSendDefaultPii ()Z
public fun isTracingEnabled ()Z
public fun setAttachServerName (Z)V
public fun setAttachStacktrace (Z)V
public fun setAttachThreads (Z)V
Expand Down
6 changes: 6 additions & 0 deletions sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,12 @@ public void flush(long timeoutMillis) {
SentryLevel.WARNING,
"Instance is disabled and this 'startTransaction' returns a no-op.");
transaction = NoOpTransaction.getInstance();
} else if (!options.isTracingEnabled()) {
options
.getLogger()
.log(
SentryLevel.INFO, "Tracing is disabled and this 'startTransaction' returns a no-op.");
transaction = NoOpTransaction.getInstance();
} else {
final SamplingContext samplingContext =
new SamplingContext(transactionContext, customSamplingContext);
Expand Down
10 changes: 10 additions & 0 deletions sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,16 @@ public void setEnableDeduplication(final @Nullable Boolean enableDeduplication)
this.enableDeduplication = enableDeduplication;
}

/**
* Returns if tracing should be enabled. If tracing is disabled, starting transactions returns
* {@link NoOpTransaction}.
*
* @return if tracing is enabled.
*/
public boolean isTracingEnabled() {
return getTracesSampleRate() != null || getTracesSampler() != null;
}

/** The BeforeSend callback */
public interface BeforeSendCallback {

Expand Down

0 comments on commit bc99848

Please sign in to comment.