diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index ec87fd8cd9..b5ef861a1a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -834,7 +834,7 @@ static void applyMetadata( final @NotNull Object metadata, final @NotNull ILogger logger, final @NotNull DataCollection currentDataCollection) { - final @NotNull DataCollection dataCollection = new DataCollection(false); + final @NotNull DataCollection dataCollection = new DataCollection(); if (containsKey(metadata, DATA_COLLECTION_USER_INFO)) { dataCollection.setUserInfo(readBool(metadata, logger, DATA_COLLECTION_USER_INFO, false)); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 9ec01a153c..53452c4c3b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -387,7 +387,7 @@ public final class io/sentry/DataCategory : java/lang/Enum { public final class io/sentry/DataCollection { public fun ()V - public fun (Z)V + public fun forceDataCollection ()V public fun getCookies ()Lio/sentry/KeyValueCollectionBehavior; public fun getDatabaseQueryData ()Ljava/lang/Boolean; public fun getFilePaths ()Ljava/lang/Boolean; diff --git a/sentry/src/main/java/io/sentry/DataCollection.java b/sentry/src/main/java/io/sentry/DataCollection.java index fa8de21b85..e0a45e6aa5 100644 --- a/sentry/src/main/java/io/sentry/DataCollection.java +++ b/sentry/src/main/java/io/sentry/DataCollection.java @@ -21,13 +21,11 @@ public final class DataCollection { private final @NotNull HttpHeaders httpHeaders = new HttpHeaders(); private final @NotNull Graphql graphql = new Graphql(); - public DataCollection() { - this(true); - } + public DataCollection() {} - @ApiStatus.Internal - public DataCollection(final boolean forceDataCollection) { - this.forceDataCollection = forceDataCollection; + /** Opts into the documented Data Collection defaults when no individual option is configured. */ + public void forceDataCollection() { + forceDataCollection = true; } public @Nullable Boolean getUserInfo() { diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index c44e9ca16f..272cf1c13a 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -250,7 +250,7 @@ public final class ExternalOptions { private static @Nullable DataCollection parseDataCollection( final @NotNull PropertiesProvider propertiesProvider) { - final DataCollection dataCollection = new DataCollection(false); + final DataCollection dataCollection = new DataCollection(); final Boolean userInfo = propertiesProvider.getBooleanProperty("data-collection.user-info"); if (userInfo != null) { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 0d7ada08ec..f37e00db0f 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -352,7 +352,7 @@ public class SentryOptions implements RateLimiterConfig { /** whether to send personal identifiable information along with events */ private boolean sendDefaultPii = false; - private @NotNull DataCollection dataCollection = new DataCollection(false); + private @NotNull DataCollection dataCollection = new DataCollection(); private final @NotNull DataCollectionResolver dataCollectionResolver = new DataCollectionResolver(this); @@ -1776,7 +1776,9 @@ public void setSendDefaultPii(boolean sendDefaultPii) { /** * Replaces the configuration for data that the SDK collects automatically. * - *

Passing an empty {@link DataCollection} opts into the documented data-collection defaults. + *

This discards any Data Collection options already configured on this instance. To opt into + * the documented defaults while preserving them, call {@link + * DataCollection#forceDataCollection()} on the object returned by {@link #getDataCollection()}. */ public void setDataCollection(final @NotNull DataCollection dataCollection) { if (dataCollection != null) { diff --git a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt index 3fb44b15a9..375cc96cfb 100644 --- a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt +++ b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt @@ -371,7 +371,7 @@ class DataCollectionResolverTest { @Test fun `explicit empty data collection enables every HTTP body direction`() { - val options = SentryOptions().apply { dataCollection = DataCollection() } + val options = SentryOptions().apply { dataCollection.forceDataCollection() } assertThat(options.dataCollectionResolver.isIncomingRequestBody).isTrue() assertThat(options.dataCollectionResolver.isOutgoingRequestBody).isTrue() diff --git a/sentry/src/test/java/io/sentry/DataCollectionTest.kt b/sentry/src/test/java/io/sentry/DataCollectionTest.kt index d39b253088..ae4bc3a056 100644 --- a/sentry/src/test/java/io/sentry/DataCollectionTest.kt +++ b/sentry/src/test/java/io/sentry/DataCollectionTest.kt @@ -6,7 +6,7 @@ import kotlin.test.assertFailsWith class DataCollectionTest { @Test - fun `public constructor forces Data Collection for empty configuration`() { + fun `public constructor does not force Data Collection for empty configuration`() { val dataCollection = DataCollection() assertThat(dataCollection.userInfo).isNull() @@ -19,19 +19,21 @@ class DataCollectionTest { assertThat(dataCollection.httpHeaders.response).isNull() assertThat(dataCollection.graphql.document).isNull() assertThat(dataCollection.graphql.variables).isNull() - assertThat(dataCollection.isExplicitlyConfigured()).isTrue() + assertThat(dataCollection.isExplicitlyConfigured()).isFalse() } @Test - fun `SDK-owned configuration does not force Data Collection`() { - val dataCollection = DataCollection(false) + fun `force Data Collection makes empty configuration explicit`() { + val dataCollection = DataCollection() - assertThat(dataCollection.isExplicitlyConfigured()).isFalse() + dataCollection.forceDataCollection() + + assertThat(dataCollection.isExplicitlyConfigured()).isTrue() } @Test - fun `nested override makes SDK-owned configuration explicit`() { - val dataCollection = DataCollection(false) + fun `nested override makes configuration explicit`() { + val dataCollection = DataCollection() dataCollection.graphql.setVariables(false) @@ -40,7 +42,7 @@ class DataCollectionTest { @Test fun `explicit false is distinct from unset`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.setUserInfo(false) @@ -50,7 +52,7 @@ class DataCollectionTest { @Test fun `nullable Boolean options are mutable Kotlin properties`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.userInfo = false dataCollection.databaseQueryData = false @@ -81,7 +83,7 @@ class DataCollectionTest { @Test fun `empty HTTP body set is distinct from unset`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.setHttpBodies(emptySet()) @@ -105,7 +107,7 @@ class DataCollectionTest { @Test fun `database query data false is distinct from unset`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.setDatabaseQueryData(false) @@ -115,7 +117,7 @@ class DataCollectionTest { @Test fun `file paths false is distinct from unset`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.setFilePaths(false) @@ -125,7 +127,7 @@ class DataCollectionTest { @Test fun `nested HTTP header override marks configuration explicit`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() val behavior = KeyValueCollectionBehavior.denyList("authorization") dataCollection.httpHeaders.setRequest(behavior) @@ -135,7 +137,7 @@ class DataCollectionTest { @Test fun `nested GraphQL false marks configuration explicit`() { - val dataCollection = DataCollection(false) + val dataCollection = DataCollection() dataCollection.graphql.setVariables(false) diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index ddddec8eb7..6b47d36e53 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -48,14 +48,33 @@ class SentryOptionsTest { } @Test - fun `setting an empty data collection marks it explicitly configured`() { + fun `setting an empty data collection preserves legacy mode`() { val options = SentryOptions() options.dataCollection = DataCollection() + assertThat(options.dataCollection.isExplicitlyConfigured()).isFalse() + } + + @Test + fun `forcing empty data collection marks it explicitly configured`() { + val options = SentryOptions() + + options.dataCollection.forceDataCollection() + assertThat(options.dataCollection.isExplicitlyConfigured()).isTrue() } + @Test + fun `forcing data collection preserves existing options`() { + val options = SentryOptions() + options.dataCollection.setUserInfo(false) + + options.dataCollection.forceDataCollection() + + assertThat(options.dataCollection.userInfo).isFalse() + } + @Test fun `setting data collection replaces the default instance`() { val options = SentryOptions()