Skip to content

Commit

Permalink
Disable Sentry with SentryOptions (#2840)
Browse files Browse the repository at this point in the history
* add enabled flag to SentryOptions

* add enabled flag to external options, add tests

* add changelog

* Update CHANGELOG.md

Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>

* only check for dsn null after checking whether sentry is enabled

* format

* add test for disabling sentry by option flag with dsn = null

* add test to verify that exception is thrown when Senty is initialized with dsn = null and enabled = true

* fix changelog after merge

* add enabled flag to AndroidManifest.xml

* Format code

---------

Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
  • Loading branch information
3 people committed Aug 4, 2023
1 parent 7cdf121 commit 9c8b5aa
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- Add autoconfigure modules for Spring Boot called `sentry-spring-boot` and `sentry-spring-boot-jakarta` ([#2880](https://github.com/getsentry/sentry-java/pull/2880))
- The autoconfigure modules `sentry-spring-boot` and `sentry-spring-boot-jakarta` have a `compileOnly` dependency on `spring-boot-starter` which is needed for our auto installation in [sentry-android-gradle-plugin](https://github.com/getsentry/sentry-android-gradle-plugin)
- The starter modules `sentry-spring-boot-starter` and `sentry-spring-boot-starter-jakarta` now bring `spring-boot-starter` as a dependency
- You can now disable Sentry by setting the `enabled` option to `false` ([#2840](https://github.com/getsentry/sentry-java/pull/2840))

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ final class ManifestMetadataReader {

static final String ENABLE_ROOT_CHECK = "io.sentry.enable-root-check";

static final String ENABLE_SENTRY = "io.sentry.enabled";

/** ManifestMetadataReader ctor */
private ManifestMetadataReader() {}

Expand Down Expand Up @@ -157,13 +159,21 @@ static void applyMetadata(
options.getAnrTimeoutIntervalMillis()));

final String dsn = readString(metadata, logger, DSN, options.getDsn());
if (dsn == null) {
final boolean enabled = readBool(metadata, logger, ENABLE_SENTRY, options.isEnabled());

if (!enabled || (dsn != null && dsn.isEmpty())) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Sentry enabled flag set to false or DSN is empty: disabling sentry-android");
} else if (dsn == null) {
options
.getLogger()
.log(SentryLevel.FATAL, "DSN is required. Use empty string to disable SDK.");
} else if (dsn.isEmpty()) {
options.getLogger().log(SentryLevel.DEBUG, "DSN is empty, disabling sentry-android");
}

options.setEnabled(enabled);
options.setDsn(dsn);

options.setEnableNdk(readBool(metadata, logger, NDK_ENABLE, options.isEnableNdk()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1268,4 +1268,29 @@ class ManifestMetadataReaderTest {
// Assert
assertTrue(fixture.options.isEnableRootCheck)
}

@Test
fun `applyMetadata reads enabled flag to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.ENABLE_SENTRY to false)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertFalse(fixture.options.isEnabled)
}

@Test
fun `applyMetadata reads enabled flag to options and keeps default if not found`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertTrue(fixture.options.isEnabled)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,5 +148,8 @@
<!-- how to enable the send default pii-->
<meta-data android:name="io.sentry.send-default-pii" android:value="true" />

<!-- how to disable sentry -->
<!-- <meta-data android:name="io.sentry.enabled" android:value="false" /> -->

</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ class SentryAutoConfigurationTest {
"sentry.tags.tag1=tag1-value",
"sentry.tags.tag2=tag2-value",
"sentry.ignored-exceptions-for-type=java.lang.RuntimeException,java.lang.IllegalStateException,io.sentry.Sentry",
"sentry.trace-propagation-targets=localhost,^(http|https)://api\\..*\$"
"sentry.trace-propagation-targets=localhost,^(http|https)://api\\..*\$",
"sentry.enabled=false"
).run {
val options = it.getBean(SentryProperties::class.java)
assertThat(options.readTimeoutMillis).isEqualTo(10)
Expand Down Expand Up @@ -184,6 +185,7 @@ class SentryAutoConfigurationTest {
assertThat(options.tags).containsEntry("tag1", "tag1-value").containsEntry("tag2", "tag2-value")
assertThat(options.ignoredExceptionsForType).containsOnly(RuntimeException::class.java, IllegalStateException::class.java)
assertThat(options.tracePropagationTargets).containsOnly("localhost", "^(http|https)://api\\..*\$")
assertThat(options.isEnabled).isEqualTo(false)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ class SentryAutoConfigurationTest {
"sentry.tags.tag1=tag1-value",
"sentry.tags.tag2=tag2-value",
"sentry.ignored-exceptions-for-type=java.lang.RuntimeException,java.lang.IllegalStateException,io.sentry.Sentry",
"sentry.trace-propagation-targets=localhost,^(http|https)://api\\..*\$"
"sentry.trace-propagation-targets=localhost,^(http|https)://api\\..*\$",
"sentry.enabled=false"
).run {
val options = it.getBean(SentryProperties::class.java)
assertThat(options.readTimeoutMillis).isEqualTo(10)
Expand Down Expand Up @@ -184,6 +185,7 @@ class SentryAutoConfigurationTest {
assertThat(options.tags).containsEntry("tag1", "tag1-value").containsEntry("tag2", "tag2-value")
assertThat(options.ignoredExceptionsForType).containsOnly(RuntimeException::class.java, IllegalStateException::class.java)
assertThat(options.tracePropagationTargets).containsOnly("localhost", "^(http|https)://api\\..*\$")
assertThat(options.isEnabled).isEqualTo(false)
}
}

Expand Down
4 changes: 4 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,14 @@ public final class io/sentry/ExternalOptions {
public fun getTracePropagationTargets ()Ljava/util/List;
public fun getTracesSampleRate ()Ljava/lang/Double;
public fun getTracingOrigins ()Ljava/util/List;
public fun isEnabled ()Ljava/lang/Boolean;
public fun setDebug (Ljava/lang/Boolean;)V
public fun setDist (Ljava/lang/String;)V
public fun setDsn (Ljava/lang/String;)V
public fun setEnableDeduplication (Ljava/lang/Boolean;)V
public fun setEnableTracing (Ljava/lang/Boolean;)V
public fun setEnableUncaughtExceptionHandler (Ljava/lang/Boolean;)V
public fun setEnabled (Ljava/lang/Boolean;)V
public fun setEnvironment (Ljava/lang/String;)V
public fun setIdleTimeout (Ljava/lang/Long;)V
public fun setMaxRequestBodySize (Lio/sentry/SentryOptions$RequestSize;)V
Expand Down Expand Up @@ -1829,6 +1831,7 @@ public class io/sentry/SentryOptions {
public fun isEnableUncaughtExceptionHandler ()Z
public fun isEnableUserInteractionBreadcrumbs ()Z
public fun isEnableUserInteractionTracing ()Z
public fun isEnabled ()Z
public fun isPrintUncaughtStackTrace ()Z
public fun isProfilingEnabled ()Z
public fun isSendClientReports ()Z
Expand Down Expand Up @@ -1863,6 +1866,7 @@ public class io/sentry/SentryOptions {
public fun setEnableUncaughtExceptionHandler (Z)V
public fun setEnableUserInteractionBreadcrumbs (Z)V
public fun setEnableUserInteractionTracing (Z)V
public fun setEnabled (Z)V
public fun setEnvelopeDiskCache (Lio/sentry/cache/IEnvelopeCache;)V
public fun setEnvelopeReader (Lio/sentry/IEnvelopeReader;)V
public fun setEnvironment (Ljava/lang/String;)V
Expand Down
11 changes: 11 additions & 0 deletions sentry/src/main/java/io/sentry/ExternalOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public final class ExternalOptions {
private @Nullable Boolean printUncaughtStackTrace;
private @Nullable Boolean sendClientReports;
private @NotNull Set<String> bundleIds = new CopyOnWriteArraySet<>();
private @Nullable Boolean enabled;

@SuppressWarnings("unchecked")
public static @NotNull ExternalOptions from(
Expand Down Expand Up @@ -115,6 +116,8 @@ public final class ExternalOptions {
}
options.setIdleTimeout(propertiesProvider.getLongProperty("idle-timeout"));

options.setEnabled(propertiesProvider.getBooleanProperty("enabled"));

for (final String ignoredExceptionType :
propertiesProvider.getList("ignored-exceptions-for-type")) {
try {
Expand Down Expand Up @@ -347,4 +350,12 @@ public void setSendClientReports(final @Nullable Boolean sendClientReports) {
public void addBundleId(final @NotNull String bundleId) {
bundleIds.add(bundleId);
}

public @Nullable Boolean isEnabled() {
return enabled;
}

public void setEnabled(final @Nullable Boolean enabled) {
this.enabled = enabled;
}
}
8 changes: 5 additions & 3 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,13 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
}

final String dsn = options.getDsn();
if (dsn == null) {
throw new IllegalArgumentException("DSN is required. Use empty string to disable SDK.");
} else if (dsn.isEmpty()) {

if (!options.isEnabled() || (dsn != null && dsn.isEmpty())) {
close();
return false;
} else if (dsn == null) {
throw new IllegalArgumentException(
"DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK.");
}

@SuppressWarnings("unused")
Expand Down
25 changes: 25 additions & 0 deletions sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ public class SentryOptions {
private final @NotNull FullyDisplayedReporter fullyDisplayedReporter =
FullyDisplayedReporter.getInstance();

/** Whether Sentry should be enabled */
private boolean enabled = true;

/**
* Adds an event processor
*
Expand Down Expand Up @@ -2096,6 +2099,24 @@ public void setTraceOptionsRequests(boolean traceOptionsRequests) {
this.traceOptionsRequests = traceOptionsRequests;
}

/**
* Whether Sentry is enabled.
*
* @return true if Sentry should be enabled
*/
public boolean isEnabled() {
return enabled;
}

/**
* Whether Sentry should be enabled.
*
* @param enabled true if Sentry should be enabled
*/
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}

/** Returns the current {@link SentryDateProvider} that is used to retrieve the current date. */
@ApiStatus.Internal
public @NotNull SentryDateProvider getDateProvider() {
Expand Down Expand Up @@ -2333,6 +2354,10 @@ public void merge(final @NotNull ExternalOptions options) {
for (String bundleId : options.getBundleIds()) {
addBundleId(bundleId);
}

if (options.isEnabled() != null) {
setEnabled(options.isEnabled());
}
}

private @NotNull SdkVersion createSdkVersion() {
Expand Down
7 changes: 7 additions & 0 deletions sentry/src/test/java/io/sentry/ExternalOptionsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,13 @@ class ExternalOptionsTest {
}
}

@Test
fun `creates options with enabled set to true`() {
withPropertiesFile("enabled=true") { options ->
assertTrue(options.isEnabled() == true)
}
}

private fun withPropertiesFile(textLines: List<String> = emptyList(), logger: ILogger = mock(), fn: (ExternalOptions) -> Unit) {
// create a sentry.properties file in temporary folder
val temporaryFolder = TemporaryFolder()
Expand Down
7 changes: 7 additions & 0 deletions sentry/src/test/java/io/sentry/SentryOptionsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ class SentryOptionsTest {
externalOptions.proguardUuid = "1234"
externalOptions.idleTimeout = 1500L
externalOptions.bundleIds.addAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411 ", " faa3ab42-b1bd-4659-af8e-1682324aa744"))
externalOptions.isEnabled = false
val options = SentryOptions()

options.merge(externalOptions)
Expand All @@ -392,6 +393,7 @@ class SentryOptionsTest {
assertEquals("1234", options.proguardUuid)
assertEquals(1500L, options.idleTimeout)
assertEquals(setOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"), options.bundleIds)
assertFalse(options.isEnabled)
}

@Test
Expand Down Expand Up @@ -483,4 +485,9 @@ class SentryOptionsTest {
fun `when options are initialized, FullyDrawnReporter is set`() {
assertEquals(FullyDisplayedReporter.getInstance(), SentryOptions().fullyDisplayedReporter)
}

@Test
fun `when options are initialized, enabled is set to true by default`() {
assertTrue(SentryOptions().isEnabled)
}
}
44 changes: 44 additions & 0 deletions sentry/src/test/java/io/sentry/SentryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.sentry.test.ImmediateExecutorService
import io.sentry.util.thread.IMainThreadChecker
import io.sentry.util.thread.MainThreadChecker
import org.awaitility.kotlin.await
import org.junit.Assert.assertThrows
import org.junit.Rule
import org.junit.rules.TemporaryFolder
import org.mockito.kotlin.any
Expand Down Expand Up @@ -167,6 +168,49 @@ class SentryTest {
}
}

@Test
fun `initializes Sentry with enabled=false, thus disabling Sentry even if dsn is set`() {
Sentry.init {
it.isEnabled = false
it.dsn = "http://key@localhost/proj"
}

Sentry.setTag("none", "shouldNotExist")

var value: String? = null
Sentry.getCurrentHub().configureScope {
value = it.tags[value]
}
assertTrue(Sentry.getCurrentHub() is NoOpHub)
assertNull(value)
}

@Test
fun `initializes Sentry with enabled=false, thus disabling Sentry even if dsn is null`() {
Sentry.init {
it.isEnabled = false
}

Sentry.setTag("none", "shouldNotExist")

var value: String? = null
Sentry.getCurrentHub().configureScope {
value = it.tags[value]
}
assertTrue(Sentry.getCurrentHub() is NoOpHub)
assertNull(value)
}

@Test
fun `initializes Sentry with dsn = null, throwing IllegalArgumentException`() {
val exception =
assertThrows(java.lang.IllegalArgumentException::class.java) { Sentry.init() }
assertEquals(
"DSN is required. Use empty string or set enabled to false in SentryOptions to disable SDK.",
exception.message
)
}

@Test
fun `captureUserFeedback gets forwarded to client`() {
Sentry.init { it.dsn = dsn }
Expand Down

0 comments on commit 9c8b5aa

Please sign in to comment.