Skip to content

Commit

Permalink
Allow opting out of device info requiring IPC
Browse files Browse the repository at this point in the history
  • Loading branch information
adinauer committed Jun 15, 2022
1 parent a88509c commit cd407e2
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 40 deletions.
2 changes: 2 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun isAnrEnabled ()Z
public fun isAnrReportInDebug ()Z
public fun isAttachScreenshot ()Z
public fun isCollectIpcDeviceInfo ()Z
public fun isEnableActivityLifecycleBreadcrumbs ()Z
public fun isEnableActivityLifecycleTracingAutoFinish ()Z
public fun isEnableAppComponentBreadcrumbs ()Z
Expand All @@ -141,6 +142,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr
public fun setAnrReportInDebug (Z)V
public fun setAnrTimeoutIntervalMillis (J)V
public fun setAttachScreenshot (Z)V
public fun setCollectIpcDeviceInfo (Z)V
public fun setDebugImagesLoader (Lio/sentry/android/core/IDebugImagesLoader;)V
public fun setEnableActivityLifecycleBreadcrumbs (Z)V
public fun setEnableActivityLifecycleTracingAutoFinish (Z)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ static void init(

readDefaultOptionValues(options, context);

options.addEventProcessor(new DefaultAndroidEventProcessor(context, logger, buildInfoProvider));
options.addEventProcessor(
new DefaultAndroidEventProcessor(context, logger, buildInfoProvider, options));
options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker));

options.setTransportGate(new AndroidTransportGate(context, options.getLogger()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,26 +69,35 @@ final class DefaultAndroidEventProcessor implements EventProcessor {

private final @NotNull BuildInfoProvider buildInfoProvider;
private final @NotNull RootChecker rootChecker;
private final @NotNull SentryAndroidOptions options;

private final @NotNull ILogger logger;

public DefaultAndroidEventProcessor(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider) {
this(context, logger, buildInfoProvider, new RootChecker(context, buildInfoProvider, logger));
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull SentryAndroidOptions options) {
this(
context,
logger,
buildInfoProvider,
new RootChecker(context, buildInfoProvider, logger),
options);
}

DefaultAndroidEventProcessor(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull RootChecker rootChecker) {
final @NotNull RootChecker rootChecker,
final @NotNull SentryAndroidOptions options) {
this.context = Objects.requireNonNull(context, "The application context is required.");
this.logger = Objects.requireNonNull(logger, "The Logger is required.");
this.buildInfoProvider =
Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required.");
this.rootChecker = Objects.requireNonNull(rootChecker, "The RootChecker is required.");
this.options = Objects.requireNonNull(options, "The options object is required.");

ExecutorService executorService = Executors.newSingleThreadExecutor();
// dont ref. to method reference, theres a bug on it
Expand Down Expand Up @@ -328,36 +337,42 @@ private void setArchitectures(final @NotNull Device device) {
}

private void setDeviceIO(final @NotNull Device device, final boolean applyScopeData) {
final Intent batteryIntent = getBatteryIntent();
if (batteryIntent != null) {
device.setBatteryLevel(getBatteryLevel(batteryIntent));
device.setCharging(isCharging(batteryIntent));
device.setBatteryTemperature(getBatteryTemperature(batteryIntent));
}

Boolean connected;
switch (ConnectivityChecker.getConnectionStatus(context, logger)) {
case NOT_CONNECTED:
connected = false;
break;
case CONNECTED:
connected = true;
break;
default:
connected = null;
}
device.setOnline(connected);

final ActivityManager.MemoryInfo memInfo = getMemInfo();
if (memInfo != null) {
// in bytes
device.setMemorySize(getMemorySize(memInfo));
if (applyScopeData) {
device.setFreeMemory(memInfo.availMem);
device.setLowMemory(memInfo.lowMemory);
if (options.isCollectIpcDeviceInfo()) {
final Intent batteryIntent = getBatteryIntent();
if (batteryIntent != null) {
device.setBatteryLevel(getBatteryLevel(batteryIntent));
device.setCharging(isCharging(batteryIntent));
device.setBatteryTemperature(getBatteryTemperature(batteryIntent));
}
}

if (options.isCollectIpcDeviceInfo()) {
Boolean connected;
switch (ConnectivityChecker.getConnectionStatus(context, logger)) {
case NOT_CONNECTED:
connected = false;
break;
case CONNECTED:
connected = true;
break;
default:
connected = null;
}
device.setOnline(connected);
}

if (options.isCollectIpcDeviceInfo()) {
final ActivityManager.MemoryInfo memInfo = getMemInfo();
if (memInfo != null) {
// in bytes
device.setMemorySize(getMemorySize(memInfo));
if (applyScopeData) {
device.setFreeMemory(memInfo.availMem);
device.setLowMemory(memInfo.lowMemory);
}
// there are runtime.totalMemory() and runtime.freeMemory(), but I kept the same for
// compatibility
}
// there are runtime.totalMemory() and runtime.freeMemory(), but I kept the same for
// compatibility
}

// this way of getting the size of storage might be problematic for storages bigger than 2GB
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ final class ManifestMetadataReader {

static final String ATTACH_SCREENSHOT = "io.sentry.attach-screenshot";
static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports";
static final String COLLECT_IPC_DEVICE_INFO = "io.sentry.collect-ipc-device-info";

/** ManifestMetadataReader ctor */
private ManifestMetadataReader() {}
Expand Down Expand Up @@ -210,6 +211,9 @@ static void applyMetadata(
options.setSendClientReports(
readBool(metadata, logger, CLIENT_REPORTS_ENABLE, options.isSendClientReports()));

options.setCollectIpcDeviceInfo(
readBool(metadata, logger, COLLECT_IPC_DEVICE_INFO, options.isCollectIpcDeviceInfo()));

if (options.getTracesSampleRate() == null) {
final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE);
if (tracesSampleRate != -1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ public final class SentryAndroidOptions extends SentryOptions {
/** Enables or disables the attach screenshot feature when an error happened. */
private boolean attachScreenshot;

/**
* Enables or disables collecting of device information which requires Inter-Process Communication
* (IPC)
*/
private boolean collectIpcDeviceInfo = true;

public SentryAndroidOptions() {
setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME);
setSdkVersion(createSdkVersion());
Expand Down Expand Up @@ -294,4 +300,12 @@ public boolean isEnableUserInteractionTracing() {
public void setEnableUserInteractionTracing(boolean enableUserInteractionTracing) {
this.enableUserInteractionTracing = enableUserInteractionTracing;
}

public boolean isCollectIpcDeviceInfo() {
return collectIpcDeviceInfo;
}

public void setCollectIpcDeviceInfo(boolean collectIpcDeviceInfo) {
this.collectIpcDeviceInfo = collectIpcDeviceInfo;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import io.sentry.Hint
import io.sentry.ILogger
import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SentryOptions
import io.sentry.SentryTracer
import io.sentry.TransactionContext
import io.sentry.android.core.DefaultAndroidEventProcessor.EMULATOR
Expand Down Expand Up @@ -48,23 +47,23 @@ class DefaultAndroidEventProcessorTest {

private val className = "io.sentry.android.core.DefaultAndroidEventProcessor"
private val ctorTypes =
arrayOf(Context::class.java, ILogger::class.java, BuildInfoProvider::class.java)
arrayOf(Context::class.java, ILogger::class.java, BuildInfoProvider::class.java, SentryAndroidOptions::class.java)

init {
Locale.setDefault(Locale.US)
}

private class Fixture {
val buildInfo = mock<BuildInfoProvider>()
val options = SentryOptions().apply {
val options = SentryAndroidOptions().apply {
setDebug(true)
setLogger(mock())
sdkVersion = SdkVersion("test", "1.2.3")
}
val sentryTracer = SentryTracer(TransactionContext("", ""), mock())

fun getSut(context: Context): DefaultAndroidEventProcessor {
return DefaultAndroidEventProcessor(context, options.logger, buildInfo)
return DefaultAndroidEventProcessor(context, options.logger, buildInfo, options)
}
}

Expand All @@ -86,23 +85,31 @@ class DefaultAndroidEventProcessorTest {
fun `when null context is provided, invalid argument is thrown`() {
val ctor = className.getCtor(ctorTypes)

val params = arrayOf(null, mock<SentryOptions>(), null)
val params = arrayOf(null, mock<ILogger>(), null, mock<SentryAndroidOptions>())
assertFailsWith<IllegalArgumentException> { ctor.newInstance(params) }
}

@Test
fun `when null logger is provided, invalid argument is thrown`() {
val ctor = className.getCtor(ctorTypes)

val params = arrayOf(mock<Context>(), null, null, mock<SentryAndroidOptions>())
assertFailsWith<IllegalArgumentException> { ctor.newInstance(params) }
}

@Test
fun `when null options is provided, invalid argument is thrown`() {
val ctor = className.getCtor(ctorTypes)

val params = arrayOf(mock<Context>(), null, null)
val params = arrayOf(mock<Context>(), mock<ILogger>(), mock<BuildInfoProvider>(), null)
assertFailsWith<IllegalArgumentException> { ctor.newInstance(params) }
}

@Test
fun `when null buildInfo is provided, invalid argument is thrown`() {
val ctor = className.getCtor(ctorTypes)

val params = arrayOf(null, null, mock<BuildInfoProvider>())
val params = arrayOf(null, null, mock<BuildInfoProvider>(), mock<SentryAndroidOptions>())
assertFailsWith<IllegalArgumentException> { ctor.newInstance(params) }
}

Expand Down Expand Up @@ -456,6 +463,25 @@ class DefaultAndroidEventProcessorTest {
}
}

@Test
fun `Does not collect device info that requires IPC if disabled`() {
fixture.options.isCollectIpcDeviceInfo = false
val sut = fixture.getSut(context)

assertNotNull(sut.process(SentryEvent(), Hint())) {
val device = it.contexts.device!!
assertNull(device.freeMemory)
assertNull(device.isLowMemory)

// commented values are not mocked by robolectric
// assertNotNull(device.batteryLevel)
// assertNotNull(device.isCharging)
// assertNotNull(device.batteryTemperature)
// assertNotNull(device.isOnline)
// assertNotNull(device.connectionType)
}
}

@Test
fun `Event sets language and locale`() {
val sut = fixture.getSut(context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,4 +919,29 @@ class ManifestMetadataReaderTest {
// Assert
assertEquals(3000L, fixture.options.idleTimeout)
}

@Test
fun `applyMetadata reads collect ipc device info to options`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.COLLECT_IPC_DEVICE_INFO to false)
val context = fixture.getContext(metaData = bundle)

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

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

@Test
fun `applyMetadata reads collect ipc device info and keep default value if not found`() {
// Arrange
val context = fixture.getContext()

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

// Assert
assertTrue(fixture.options.isCollectIpcDeviceInfo)
}
}

0 comments on commit cd407e2

Please sign in to comment.