Skip to content

fix(android): [Data Collection 19] Preserve installation ID #5937

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
adinauer wants to merge 1 commit into
base: fix/data-collection-spring-binding
Choose a base branch
from
Draft

fix(android): [Data Collection 19] Preserve installation ID #5937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions ...-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
}

// userId should be set even if event is Cached as the userId is static and won't change anyway.
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (user.getId() == null) {
user.setId(getDeviceId());
}
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
Expand Down Expand Up @@ -635,8 +635,7 @@ private void setDevice(final @NotNull SentryBaseEvent event) {
device.setScreenDpi(displayMetrics.densityDpi);
}

if (device.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (device.getId() == null) {
device.setId(getDeviceId());
}

Expand Down
6 changes: 2 additions & 4 deletions sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
}

// userId should be set even if event is Cached as the userId is static and won't change anyway.
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (user.getId() == null) {
user.setId(Installation.id(context));
}
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
Expand Down Expand Up @@ -374,9 +374,7 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
*/
public @NotNull User getDefaultUser(final @NotNull Context context) {
final @NotNull User user = new User();
if (options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
user.setId(Installation.id(context));
}
user.setId(Installation.id(context));
return user;
}

Expand Down
3 changes: 1 addition & 2 deletions sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ public Device collectDeviceInformation(
device.setBootTime(getBootTime());
device.setTimezone(getTimeZone());

if (device.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (device.getId() == null) {
device.setId(getDeviceId());
}

Expand Down
3 changes: 1 addition & 2 deletions sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ public static Map<String, Object> serializeScope(
user = new User();
scope.setUser(user);
}
if (user.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (user.getId() == null) {
try {
user.setId(Installation.id(context));
} catch (RuntimeException e) {
Expand Down
3 changes: 1 addition & 2 deletions sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ public static void init(
"Error in the 'OptionsConfiguration.configure' callback.",
t);
}
if (options.getDistinctId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
if (options.getDistinctId() == null) {
try {
options.setDistinctId(Installation.id(context));
} catch (RuntimeException e) {
Expand Down
9 changes: 3 additions & 6 deletions sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,7 @@ class AndroidOptionsInitializerTest {
)

sentryOptions.configureOptions()
if (
sentryOptions.distinctId == null &&
sentryOptions.dataCollectionResolver.isUserInfoWithLegacyAlways
) {
if (sentryOptions.distinctId == null) {
sentryOptions.distinctId = Installation.id(if (useRealContext) context else mockContext)
}
AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
Expand Down Expand Up @@ -356,10 +353,10 @@ class AndroidOptionsInitializerTest {
}

@Test
fun `init should not set generated distinct id when user info is disabled`() {
fun `init should set generated distinct id when user info is disabled`() {
fixture.initSut(configureOptions = { dataCollection.setUserInfo(false) })

assertNull(fixture.sentryOptions.distinctId)
assertNotNull(fixture.sentryOptions.distinctId)
}

@Test
Expand Down
8 changes: 4 additions & 4 deletions ...ndroid-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,13 @@ class ApplicationExitInfoEventProcessorTest {
}

@Test
fun `when user info is disabled, does not set device id`() {
fun `when user info is disabled, sets device id`() {
fixture.options.dataCollection.setUserInfo(false)
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())

val processed = processEvent(hint)

assertNull(processed.contexts.device!!.id)
assertNotNull(processed.contexts.device!!.id)
}

@Test
Expand Down Expand Up @@ -477,7 +477,7 @@ class ApplicationExitInfoEventProcessorTest {
}

@Test
fun `when user info is disabled, does not set installation id for missing user id`() {
fun `when user info is disabled, sets installation id for missing user id`() {
fixture.options.dataCollection.setUserInfo(false)
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val original = SentryEvent()
Expand All @@ -486,7 +486,7 @@ class ApplicationExitInfoEventProcessorTest {

val processed = processor.process(original, hint)

assertNull(processed!!.user!!.id)
assertEquals(Installation.deviceId, processed!!.user!!.id)
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,15 @@ class DefaultAndroidEventProcessorTest {
}

@Test
fun `when user info is disabled, does not set automatic user data`() {
fun `when user info is disabled, sets installation id but not automatic ip`() {
fixture.options.dataCollection.setUserInfo(false)
val sut = fixture.getSut(context, isSendDefaultPii = true)
val event = SentryEvent().apply { user = User() }

sut.process(event, Hint())

assertNotNull(event.user) {
assertNull(it.id)
assertNotNull(it.id)
assertNull(it.ipAddress)
}
}
Expand Down
6 changes: 3 additions & 3 deletions sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class DeviceInfoUtilTest {
assertNotNull(enabledDevice.id)
assertNotNull(enabledDevice.storageSize)
assertNotNull(enabled.operatingSystem.isRooted)
assertNull(disabledDevice.id)
assertNotNull(disabledDevice.id)
assertNull(disabledDevice.storageSize)
assertNull(disabled.operatingSystem.isRooted)
}
Expand All @@ -94,12 +94,12 @@ class DeviceInfoUtilTest {
}

@Test
fun `does not set device id when user info is disabled`() {
fun `sets device id when user info is disabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
val deviceInfo =
DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)

assertNull(deviceInfo.id)
assertNotNull(deviceInfo.id)
}

@Test
Expand Down
5 changes: 2 additions & 3 deletions sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import java.util.concurrent.atomic.AtomicReference
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
Expand Down Expand Up @@ -327,14 +326,14 @@ class InternalSentrySdkTest {
}

@Test
fun `serializeScope does not provide fallback user id when user info is disabled`() {
fun `serializeScope provides fallback user id when user info is disabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
val scope = Scope(options)
scope.user = null

val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)

assertFalse((serializedScope["user"] as Map<*, *>).containsKey("id"))
assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
}

@Test
Expand Down
1 change: 0 additions & 1 deletion sentry/api/sentry.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,6 @@ public final class io/sentry/DataCollectionResolver {
public fun isOutgoingResponseBody ()Z
public fun isOutgoingResponseBodyWithLegacyBodyGate ()Z
public fun isUserInfo ()Z
public fun isUserInfoWithLegacyAlways ()Z
}

public final class io/sentry/DateUtils {
Expand Down
4 changes: 0 additions & 4 deletions sentry/src/main/java/io/sentry/DataCollectionResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ public boolean isUserInfo() {
return explicitOrSendDefaultPii(options.getDataCollection().getUserInfo(), true);
}

public boolean isUserInfoWithLegacyAlways() {
return explicitOrDefault(options.getDataCollection().getUserInfo(), true, true);
}

public boolean isDatabaseQueryData() {
return explicitOrSendDefaultPii(options.getDataCollection().getDatabaseQueryData(), true);
}
Expand Down
11 changes: 0 additions & 11 deletions sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,6 @@ class DataCollectionResolverTest {
assertThat(options.dataCollectionResolver.isUserInfo).isTrue()
}

@Test
fun `user info legacy always variant preserves collection when namespace is absent`() {
val options = SentryOptions().apply { isSendDefaultPii = false }

assertThat(options.dataCollectionResolver.isUserInfoWithLegacyAlways).isTrue()

options.dataCollection.setUserInfo(false)

assertThat(options.dataCollectionResolver.isUserInfoWithLegacyAlways).isFalse()
}

@Test
fun `omitted booleans use data collection defaults once namespace is explicit`() {
val options = SentryOptions().apply { isSendDefaultPii = false }
Expand Down
Loading