From cd407e20f9f9646545209f0de061186a0db151c0 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Jun 2022 12:13:13 +0200 Subject: [PATCH 1/6] Allow opting out of device info requiring IPC --- .../api/sentry-android-core.api | 2 + .../core/AndroidOptionsInitializer.java | 3 +- .../core/DefaultAndroidEventProcessor.java | 79 +++++++++++-------- .../android/core/ManifestMetadataReader.java | 4 + .../android/core/SentryAndroidOptions.java | 14 ++++ .../core/DefaultAndroidEventProcessorTest.kt | 40 ++++++++-- .../core/ManifestMetadataReaderTest.kt | 25 ++++++ 7 files changed, 127 insertions(+), 40 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index f326074de5..d4c35ec1d2 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -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 @@ -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 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index de292ed614..a7f0a4f463 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -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())); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 3575aea785..fdf5ec03c5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -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 @@ -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 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 f88d0999ff..9dfc142016 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 @@ -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() {} @@ -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) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 5373c26d70..1d4c2cd937 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -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()); @@ -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; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 095c2e8488..4202c97d46 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -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 @@ -48,7 +47,7 @@ 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) @@ -56,7 +55,7 @@ class DefaultAndroidEventProcessorTest { private class Fixture { val buildInfo = mock() - val options = SentryOptions().apply { + val options = SentryAndroidOptions().apply { setDebug(true) setLogger(mock()) sdkVersion = SdkVersion("test", "1.2.3") @@ -64,7 +63,7 @@ class DefaultAndroidEventProcessorTest { val sentryTracer = SentryTracer(TransactionContext("", ""), mock()) fun getSut(context: Context): DefaultAndroidEventProcessor { - return DefaultAndroidEventProcessor(context, options.logger, buildInfo) + return DefaultAndroidEventProcessor(context, options.logger, buildInfo, options) } } @@ -86,7 +85,15 @@ class DefaultAndroidEventProcessorTest { fun `when null context is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(null, mock(), null) + val params = arrayOf(null, mock(), null, mock()) + assertFailsWith { ctor.newInstance(params) } + } + + @Test + fun `when null logger is provided, invalid argument is thrown`() { + val ctor = className.getCtor(ctorTypes) + + val params = arrayOf(mock(), null, null, mock()) assertFailsWith { ctor.newInstance(params) } } @@ -94,7 +101,7 @@ class DefaultAndroidEventProcessorTest { fun `when null options is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(mock(), null, null) + val params = arrayOf(mock(), mock(), mock(), null) assertFailsWith { ctor.newInstance(params) } } @@ -102,7 +109,7 @@ class DefaultAndroidEventProcessorTest { fun `when null buildInfo is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(null, null, mock()) + val params = arrayOf(null, null, mock(), mock()) assertFailsWith { ctor.newInstance(params) } } @@ -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) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 81f3efb1cd..a8aab0d3c9 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -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) + } } From b2c91af54124910c7d7b2e43d1414bf305044695 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Jun 2022 12:31:15 +0200 Subject: [PATCH 2/6] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69e1558f76..cdf1fb732d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Replace `tracestate` header with `baggage` header ([#2078](https://github.com/getsentry/sentry-java/pull/2078)) +- Allow opting out of device info collection that requires Inter-Process Communication (IPC) ([#2100](https://github.com/getsentry/sentry-java/pull/2100)) ## 6.1.0 From 1810d535000e78e552089097c1eb951d6b07538f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Jun 2022 13:29:22 +0200 Subject: [PATCH 3/6] Update sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- .../java/io/sentry/android/core/ManifestMetadataReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9dfc142016..4d101c8faa 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 @@ -71,7 +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"; + static final String COLLECT_IPC_DEVICE_INFO = "io.sentry.ipc-device-context"; /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} From 6f8dae8109e795860284c37380857e5add3adfef Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Jun 2022 13:49:40 +0200 Subject: [PATCH 4/6] Remove logger --- .../core/AndroidOptionsInitializer.java | 2 +- .../core/DefaultAndroidEventProcessor.java | 85 ++++++++++--------- .../core/DefaultAndroidEventProcessorTest.kt | 15 ++-- 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index a7f0a4f463..4be4e8870f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -147,7 +147,7 @@ static void init( readDefaultOptionValues(options, context); options.addEventProcessor( - new DefaultAndroidEventProcessor(context, logger, buildInfoProvider, options)); + new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index fdf5ec03c5..25a6424f54 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -23,7 +23,6 @@ import io.sentry.DateUtils; import io.sentry.EventProcessor; import io.sentry.Hint; -import io.sentry.ILogger; import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryLevel; @@ -71,29 +70,23 @@ final class DefaultAndroidEventProcessor implements EventProcessor { 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, final @NotNull SentryAndroidOptions options) { this( context, - logger, buildInfoProvider, - new RootChecker(context, buildInfoProvider, logger), + new RootChecker(context, buildInfoProvider, options.getLogger()), options); } DefaultAndroidEventProcessor( final @NotNull Context context, - final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider, 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."); @@ -158,10 +151,12 @@ private boolean shouldApplyScopeData( if (HintUtils.shouldApplyScopeData(hint)) { return true; } else { - logger.log( - SentryLevel.DEBUG, - "Event was cached so not applying data relevant to the current app execution/version: %s", - event.getEventId()); + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Event was cached so not applying data relevant to the current app execution/version: %s", + event.getEventId()); return false; } } @@ -229,7 +224,7 @@ private void setThreads(final @NotNull SentryEvent event) { private void setPackageInfo(final @NotNull SentryBaseEvent event, final @NotNull App app) { final PackageInfo packageInfo = - ContextUtils.getPackageInfo(context, PackageManager.GET_PERMISSIONS, logger); + ContextUtils.getPackageInfo(context, PackageManager.GET_PERMISSIONS, options.getLogger()); if (packageInfo != null) { String versionCode = ContextUtils.getVersionCode(packageInfo); @@ -307,7 +302,7 @@ private void setArchitectures(final @NotNull Device device) { device.setSimulator((Boolean) emulator); } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting emulator.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting emulator.", e); } DisplayMetrics displayMetrics = getDisplayMetrics(); @@ -348,7 +343,7 @@ private void setDeviceIO(final @NotNull Device device, final boolean applyScopeD if (options.isCollectIpcDeviceInfo()) { Boolean connected; - switch (ConnectivityChecker.getConnectionStatus(context, logger)) { + switch (ConnectivityChecker.getConnectionStatus(context, options.getLogger())) { case NOT_CONNECTED: connected = false; break; @@ -393,7 +388,7 @@ private void setDeviceIO(final @NotNull Device device, final boolean applyScopeD if (device.getConnectionType() == null) { // wifi, ethernet or cellular, null if none device.setConnectionType( - ConnectivityChecker.getConnectionType(context, logger, buildInfoProvider)); + ConnectivityChecker.getConnectionType(context, options.getLogger(), buildInfoProvider)); } } @@ -424,7 +419,7 @@ private TimeZone getTimeZone() { // currentTimeMillis returns UTC already return DateUtils.getDateTime(System.currentTimeMillis() - SystemClock.elapsedRealtime()); } catch (IllegalArgumentException e) { - logger.log(SentryLevel.ERROR, e, "Error getting the device's boot time."); + options.getLogger().log(SentryLevel.ERROR, e, "Error getting the device's boot time."); } return null; } @@ -442,10 +437,10 @@ private TimeZone getTimeZone() { actManager.getMemoryInfo(memInfo); return memInfo; } - logger.log(SentryLevel.INFO, "Error getting MemoryInfo."); + options.getLogger().log(SentryLevel.INFO, "Error getting MemoryInfo."); return null; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting MemoryInfo.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting MemoryInfo.", e); return null; } } @@ -464,7 +459,7 @@ private TimeZone getTimeZone() { try { return Build.MODEL.split(" ", -1)[0]; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting device family.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting device family.", e); return null; } } @@ -487,7 +482,7 @@ private TimeZone getTimeZone() { return ((float) level / (float) scale) * percentMultiplier; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting device battery level.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting device battery level.", e); return null; } } @@ -503,7 +498,7 @@ private TimeZone getTimeZone() { return plugged == BatteryManager.BATTERY_PLUGGED_AC || plugged == BatteryManager.BATTERY_PLUGGED_USB; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting device charging state.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting device charging state.", e); return null; } } @@ -515,7 +510,7 @@ private TimeZone getTimeZone() { return ((float) temperature) / 10; // celsius } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting battery temperature.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting battery temperature.", e); } return null; } @@ -532,13 +527,15 @@ private TimeZone getTimeZone() { deviceOrientation = DeviceOrientations.getOrientation(context.getResources().getConfiguration().orientation); if (deviceOrientation == null) { - logger.log( - SentryLevel.INFO, - "No device orientation available (ORIENTATION_SQUARE|ORIENTATION_UNDEFINED)"); + options + .getLogger() + .log( + SentryLevel.INFO, + "No device orientation available (ORIENTATION_SQUARE|ORIENTATION_UNDEFINED)"); return null; } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting device orientation.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting device orientation.", e); } return deviceOrientation; } @@ -554,7 +551,7 @@ private TimeZone getTimeZone() { long totalBlocks = getBlockCountLong(stat); return totalBlocks * blockSize; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting total internal storage amount.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting total internal storage amount.", e); return null; } } @@ -609,7 +606,9 @@ private int getAvailableBlocksDep(final @NotNull StatFs stat) { long availableBlocks = getAvailableBlocksLong(stat); return availableBlocks * blockSize; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting unused internal storage amount.", e); + options + .getLogger() + .log(SentryLevel.ERROR, "Error getting unused internal storage amount.", e); return null; } } @@ -620,10 +619,10 @@ private int getAvailableBlocksDep(final @NotNull StatFs stat) { if (path != null) { // && path.canRead()) { canRead() will read return false return new StatFs(path.getPath()); } - logger.log(SentryLevel.INFO, "Not possible to read external files directory"); + options.getLogger().log(SentryLevel.INFO, "Not possible to read external files directory"); return null; } - logger.log(SentryLevel.INFO, "External storage is not mounted or emulated."); + options.getLogger().log(SentryLevel.INFO, "External storage is not mounted or emulated."); return null; } @@ -664,7 +663,7 @@ private int getAvailableBlocksDep(final @NotNull StatFs stat) { return file; } } else { - logger.log(SentryLevel.INFO, "Not possible to read getExternalFilesDirs"); + options.getLogger().log(SentryLevel.INFO, "Not possible to read getExternalFilesDirs"); } return null; } @@ -681,7 +680,7 @@ private int getAvailableBlocksDep(final @NotNull StatFs stat) { long totalBlocks = getBlockCountLong(stat); return totalBlocks * blockSize; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting total external storage amount.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting total external storage amount.", e); return null; } } @@ -705,7 +704,9 @@ private boolean isExternalStorageMounted() { long availableBlocks = getAvailableBlocksLong(stat); return availableBlocks * blockSize; } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting unused external storage amount.", e); + options + .getLogger() + .log(SentryLevel.ERROR, "Error getting unused external storage amount.", e); return null; } } @@ -719,7 +720,7 @@ private boolean isExternalStorageMounted() { try { return context.getResources().getDisplayMetrics(); } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting DisplayMetrics.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting DisplayMetrics.", e); return null; } } @@ -741,7 +742,7 @@ private boolean isExternalStorageMounted() { os.setRooted((Boolean) rooted); } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting OperatingSystem.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting OperatingSystem.", e); } return os; @@ -796,7 +797,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf try (BufferedReader br = new BufferedReader(new FileReader(file))) { return br.readLine(); } catch (IOException e) { - logger.log(SentryLevel.ERROR, errorMsg, e); + options.getLogger().log(SentryLevel.ERROR, errorMsg, e); } return defaultVersion; @@ -820,7 +821,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf return context.getString(stringId); } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting application name.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting application name.", e); } return null; @@ -842,7 +843,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf try { return Installation.id(context); } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting installationId.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting installationId.", e); } return null; } @@ -851,7 +852,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf private @Nullable Map getSideLoadedInfo() { String packageName = null; try { - final PackageInfo packageInfo = ContextUtils.getPackageInfo(context, logger); + final PackageInfo packageInfo = ContextUtils.getPackageInfo(context, options.getLogger()); final PackageManager packageManager = context.getPackageManager(); if (packageInfo != null && packageManager != null) { @@ -876,7 +877,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf } } catch (IllegalArgumentException e) { // it'll never be thrown as we are querying its own App's package. - logger.log(SentryLevel.DEBUG, "%s package isn't installed.", packageName); + options.getLogger().log(SentryLevel.DEBUG, "%s package isn't installed.", packageName); } return null; @@ -894,7 +895,7 @@ private void setSideLoadedInfo(final @NotNull SentryBaseEvent event) { } } } catch (Throwable e) { - logger.log(SentryLevel.ERROR, "Error getting side loaded info.", e); + options.getLogger().log(SentryLevel.ERROR, "Error getting side loaded info.", e); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 4202c97d46..4de18edb3c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -13,7 +13,6 @@ import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.DiagnosticLogger import io.sentry.Hint -import io.sentry.ILogger import io.sentry.SentryEvent import io.sentry.SentryLevel import io.sentry.SentryTracer @@ -47,7 +46,7 @@ class DefaultAndroidEventProcessorTest { private val className = "io.sentry.android.core.DefaultAndroidEventProcessor" private val ctorTypes = - arrayOf(Context::class.java, ILogger::class.java, BuildInfoProvider::class.java, SentryAndroidOptions::class.java) + arrayOf(Context::class.java, BuildInfoProvider::class.java, SentryAndroidOptions::class.java) init { Locale.setDefault(Locale.US) @@ -63,7 +62,7 @@ class DefaultAndroidEventProcessorTest { val sentryTracer = SentryTracer(TransactionContext("", ""), mock()) fun getSut(context: Context): DefaultAndroidEventProcessor { - return DefaultAndroidEventProcessor(context, options.logger, buildInfo, options) + return DefaultAndroidEventProcessor(context, buildInfo, options) } } @@ -85,7 +84,7 @@ class DefaultAndroidEventProcessorTest { fun `when null context is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(null, mock(), null, mock()) + val params = arrayOf(null, null, mock()) assertFailsWith { ctor.newInstance(params) } } @@ -93,7 +92,7 @@ class DefaultAndroidEventProcessorTest { fun `when null logger is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(mock(), null, null, mock()) + val params = arrayOf(mock(), null, mock()) assertFailsWith { ctor.newInstance(params) } } @@ -101,7 +100,7 @@ class DefaultAndroidEventProcessorTest { fun `when null options is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(mock(), mock(), mock(), null) + val params = arrayOf(mock(), mock(), null) assertFailsWith { ctor.newInstance(params) } } @@ -109,7 +108,7 @@ class DefaultAndroidEventProcessorTest { fun `when null buildInfo is provided, invalid argument is thrown`() { val ctor = className.getCtor(ctorTypes) - val params = arrayOf(null, null, mock(), mock()) + val params = arrayOf(null, mock(), mock()) assertFailsWith { ctor.newInstance(params) } } @@ -313,7 +312,7 @@ class DefaultAndroidEventProcessorTest { @Test fun `Processor won't throw exception when theres a hint`() { val processor = - DefaultAndroidEventProcessor(context, fixture.options.logger, fixture.buildInfo, mock()) + DefaultAndroidEventProcessor(context, fixture.buildInfo, mock(), fixture.options) val hints = HintUtils.createWithTypeCheckHint(CachedEvent()) processor.process(SentryEvent(), hints) From 7adea7391751f37a44c07787caab3ce4044f9a0c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 15 Jun 2022 17:11:39 +0200 Subject: [PATCH 5/6] Rename option and manifest entry --- sentry-android-core/api/sentry-android-core.api | 4 ++-- .../android/core/DefaultAndroidEventProcessor.java | 6 +----- .../io/sentry/android/core/ManifestMetadataReader.java | 10 +++++++--- .../io/sentry/android/core/SentryAndroidOptions.java | 10 +++++----- .../android/core/DefaultAndroidEventProcessorTest.kt | 2 +- .../sentry/android/core/ManifestMetadataReaderTest.kt | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index d4c35ec1d2..be61d862e7 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -129,7 +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 isCollectAdditionalContext ()Z public fun isEnableActivityLifecycleBreadcrumbs ()Z public fun isEnableActivityLifecycleTracingAutoFinish ()Z public fun isEnableAppComponentBreadcrumbs ()Z @@ -142,7 +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 setCollectAdditionalContext (Z)V public fun setDebugImagesLoader (Lio/sentry/android/core/IDebugImagesLoader;)V public fun setEnableActivityLifecycleBreadcrumbs (Z)V public fun setEnableActivityLifecycleTracingAutoFinish (Z)V diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 25a6424f54..bdea6817dc 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -332,16 +332,14 @@ private void setArchitectures(final @NotNull Device device) { } private void setDeviceIO(final @NotNull Device device, final boolean applyScopeData) { - if (options.isCollectIpcDeviceInfo()) { + if (options.isCollectAdditionalContext()) { 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, options.getLogger())) { case NOT_CONNECTED: @@ -354,9 +352,7 @@ private void setDeviceIO(final @NotNull Device device, final boolean applyScopeD connected = null; } device.setOnline(connected); - } - if (options.isCollectIpcDeviceInfo()) { final ActivityManager.MemoryInfo memInfo = getMemInfo(); if (memInfo != null) { // in bytes 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 4d101c8faa..8bea377728 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 @@ -71,7 +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.ipc-device-context"; + static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context"; /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -211,8 +211,12 @@ static void applyMetadata( options.setSendClientReports( readBool(metadata, logger, CLIENT_REPORTS_ENABLE, options.isSendClientReports())); - options.setCollectIpcDeviceInfo( - readBool(metadata, logger, COLLECT_IPC_DEVICE_INFO, options.isCollectIpcDeviceInfo())); + options.setCollectAdditionalContext( + readBool( + metadata, + logger, + COLLECT_ADDITIONAL_CONTEXT, + options.isCollectAdditionalContext())); if (options.getTracesSampleRate() == null) { final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 1d4c2cd937..e2c2e8b7ec 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -100,7 +100,7 @@ public final class SentryAndroidOptions extends SentryOptions { * Enables or disables collecting of device information which requires Inter-Process Communication * (IPC) */ - private boolean collectIpcDeviceInfo = true; + private boolean collectAdditionalContext = true; public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); @@ -301,11 +301,11 @@ public void setEnableUserInteractionTracing(boolean enableUserInteractionTracing this.enableUserInteractionTracing = enableUserInteractionTracing; } - public boolean isCollectIpcDeviceInfo() { - return collectIpcDeviceInfo; + public boolean isCollectAdditionalContext() { + return collectAdditionalContext; } - public void setCollectIpcDeviceInfo(boolean collectIpcDeviceInfo) { - this.collectIpcDeviceInfo = collectIpcDeviceInfo; + public void setCollectAdditionalContext(boolean collectAdditionalContext) { + this.collectAdditionalContext = collectAdditionalContext; } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 4de18edb3c..2a22b0f675 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -464,7 +464,7 @@ class DefaultAndroidEventProcessorTest { @Test fun `Does not collect device info that requires IPC if disabled`() { - fixture.options.isCollectIpcDeviceInfo = false + fixture.options.isCollectAdditionalContext = false val sut = fixture.getSut(context) assertNotNull(sut.process(SentryEvent(), Hint())) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index a8aab0d3c9..d02f3cfa4e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -923,14 +923,14 @@ class ManifestMetadataReaderTest { @Test fun `applyMetadata reads collect ipc device info to options`() { // Arrange - val bundle = bundleOf(ManifestMetadataReader.COLLECT_IPC_DEVICE_INFO to false) + val bundle = bundleOf(ManifestMetadataReader.COLLECT_ADDITIONAL_CONTEXT to false) val context = fixture.getContext(metaData = bundle) // Act ManifestMetadataReader.applyMetadata(context, fixture.options) // Assert - assertFalse(fixture.options.isCollectIpcDeviceInfo) + assertFalse(fixture.options.isCollectAdditionalContext) } @Test @@ -942,6 +942,6 @@ class ManifestMetadataReaderTest { ManifestMetadataReader.applyMetadata(context, fixture.options) // Assert - assertTrue(fixture.options.isCollectIpcDeviceInfo) + assertTrue(fixture.options.isCollectAdditionalContext) } } From 7a2838b7ee9a05df3f60f86eba75507342083a8a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 17 Jun 2022 15:21:40 +0200 Subject: [PATCH 6/6] Allow opting out of getDeviceIo completely --- .../core/DefaultAndroidEventProcessor.java | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index bdea6817dc..e3b72058a9 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -291,7 +291,9 @@ private void setArchitectures(final @NotNull Device device) { // setting such values require IO hence we don't run for transactions if (errorEvent) { - setDeviceIO(device, applyScopeData); + if (options.isCollectAdditionalContext()) { + setDeviceIO(device, applyScopeData); + } } device.setOrientation(getOrientation()); @@ -332,42 +334,41 @@ private void setArchitectures(final @NotNull Device device) { } private void setDeviceIO(final @NotNull Device device, final boolean applyScopeData) { - if (options.isCollectAdditionalContext()) { - 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, options.getLogger())) { - 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); - } - // there are runtime.totalMemory() and runtime.freeMemory(), but I kept the same for - // compatibility + 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, options.getLogger())) { + 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); } + // 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 - // check the use of https://developer.android.com/reference/java/io/File.html#getFreeSpace%28%29 + // check the use of + // https://developer.android.com/reference/java/io/File.html#getFreeSpace%28%29 final File internalStorageFile = context.getExternalFilesDir(null); if (internalStorageFile != null) { StatFs internalStorageStat = new StatFs(internalStorageFile.getPath());