From 342830515d80462565a338b299b827e583534a22 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Apr 2023 13:09:10 +0200 Subject: [PATCH 1/8] Load bundle IDs from AndroidManifest and debug-meta.properties and send with event --- .../core/AndroidOptionsInitializer.java | 53 +++++++++++++++- .../android/core/ManifestMetadataReader.java | 9 +++ .../core/AndroidOptionsInitializerTest.kt | 13 ++++ .../core/ManifestMetadataReaderTest.kt | 60 +++++++++++++++++++ sentry/api/sentry.api | 5 ++ .../main/java/io/sentry/ExternalOptions.java | 12 ++++ .../java/io/sentry/MainEventProcessor.java | 31 +++++++--- .../main/java/io/sentry/SentryOptions.java | 31 ++++++++++ .../java/io/sentry/protocol/DebugImage.java | 1 + .../java/io/sentry/ExternalOptionsTest.kt | 31 ++++++++++ .../test/java/io/sentry/SentryOptionsTest.kt | 2 + 11 files changed, 237 insertions(+), 11 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 1172ce21af..c6b2f7445c 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 @@ -279,11 +279,60 @@ private static void readDefaultOptionValues( } } - if (options.getProguardUuid() == null) { - options.setProguardUuid(getProguardUUID(context, options.getLogger())); + // TODO when to parse, when not? + // TODO parse bundle IDs as well; extract proguard parsing to parser; also parse bundleIds; + // return data class containing both properties + // TODO on SAGP side, add bundleIds to sentry-debug-meta.properties file + final @Nullable Properties debugMetaProperties = + loadDebugMetaProperties(context, options.getLogger()); + + if (debugMetaProperties != null) { + if (options.getProguardUuid() == null) { + final @Nullable String proguardUuid = + debugMetaProperties.getProperty("io.sentry.ProguardUuids"); + options.getLogger().log(SentryLevel.DEBUG, "Proguard UUID found: %s", proguardUuid); + options.setProguardUuid(proguardUuid); + } + + if (options.getBundleIds().isEmpty()) { + final @Nullable String bundleIdStrings = + debugMetaProperties.getProperty("io.sentry.bundle-ids"); + options.getLogger().log(SentryLevel.DEBUG, "Bundle IDs found: %s", bundleIdStrings); + if (bundleIdStrings != null) { + // TODO really nullable? + final @Nullable String[] bundleIds = bundleIdStrings.split(",", -1); + if (bundleIds != null) { + for (final String bundleId : bundleIds) { + options.addBundleId(bundleId); + } + } + } + } } } + private static @Nullable Properties loadDebugMetaProperties( + final @NotNull Context context, final @NotNull ILogger logger) { + final AssetManager assets = context.getAssets(); + // one may have thousands of asset files and looking up this list might slow down the SDK init. + // quite a bit, for this reason, we try to open the file directly and take care of errors + // like FileNotFoundException + try (final InputStream is = + new BufferedInputStream(assets.open("sentry-debug-meta.properties"))) { + final Properties properties = new Properties(); + properties.load(is); + return properties; + } catch (FileNotFoundException e) { + logger.log(SentryLevel.INFO, "sentry-debug-meta.properties file was not found."); + } catch (IOException e) { + logger.log(SentryLevel.ERROR, "Error getting Proguard UUIDs.", e); + } catch (RuntimeException e) { + logger.log(SentryLevel.ERROR, "sentry-debug-meta.properties file is malformed.", e); + } + + return null; + } + private static @Nullable String getProguardUUID( final @NotNull Context context, final @NotNull ILogger logger) { final AssetManager assets = context.getAssets(); 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 2d04798722..9df6e9d7b9 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 @@ -77,6 +77,8 @@ final class ManifestMetadataReader { static final String ATTACH_THREADS = "io.sentry.attach-threads"; static final String PROGUARD_UUID = "io.sentry.proguard-uuid"; + + static final String SOURCE_BUNDLE_IDS = "io.sentry.bundle-ids"; static final String IDLE_TIMEOUT = "io.sentry.traces.idle-timeout"; static final String ATTACH_SCREENSHOT = "io.sentry.attach-screenshot"; @@ -341,6 +343,13 @@ static void applyMetadata( SentryIntegrationPackageStorage.getInstance().addIntegration(integration); } } + + List sourceBundleIds = readList(metadata, logger, SOURCE_BUNDLE_IDS); + if (sourceBundleIds != null) { + for (String bundleId : sourceBundleIds) { + options.addBundleId(bundleId); + } + } } options diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 114646985a..98ec551e98 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -266,6 +266,19 @@ class AndroidOptionsInitializerTest { assertEquals("proguard-uuid", fixture.sentryOptions.proguardUuid) } + @Test + fun `init should set bundle IDs id on start`() { + fixture.initSut( + Bundle().apply { + putString(ManifestMetadataReader.SOURCE_BUNDLE_IDS, "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") + }, + hasAppContext = false + ) + + assertTrue(fixture.sentryOptions.bundleIds.size == 2) + assertTrue(fixture.sentryOptions.bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"))) + } + @Test fun `init should set Android transport gate`() { fixture.initSut() 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 efd1a47f89..d4d9e44119 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 @@ -1243,4 +1243,64 @@ class ManifestMetadataReaderTest { assertNotNull(resultingSet) assert(resultingSet.containsAll(listOf("Database Instrumentation", "OkHttp Instrumentation"))) } + + @Test + fun `applyMetadata reads single bundle ID`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "12ea7a02-46ac-44c0-a5bb-6d1fd9586411") + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + val bundleIds = fixture.options.bundleIds + assertNotNull(bundleIds) + assert(bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411"))) + } + + @Test + fun `applyMetadata reads multiple bundle IDs`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + val bundleIds = fixture.options.bundleIds + assertNotNull(bundleIds) + assert(bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"))) + } + + @Test + fun `applyMetadata reads empty bundle IDs`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "") + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + val bundleIds = fixture.options.bundleIds + assertNotNull(bundleIds) + assert(bundleIds.isEmpty()) + } + + @Test + fun `applyMetadata reads missing bundle IDs`() { + // Arrange + val bundle = bundleOf() + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + val bundleIds = fixture.options.bundleIds + assertNotNull(bundleIds) + assert(bundleIds.isEmpty()) + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 4eed2fc739..736b105f03 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -228,6 +228,7 @@ public abstract interface class io/sentry/EventProcessor { public final class io/sentry/ExternalOptions { public fun ()V + public fun addBundleId (Ljava/lang/String;)V public fun addContextTag (Ljava/lang/String;)V public fun addIgnoredExceptionForType (Ljava/lang/Class;)V public fun addInAppExclude (Ljava/lang/String;)V @@ -235,6 +236,7 @@ public final class io/sentry/ExternalOptions { public fun addTracePropagationTarget (Ljava/lang/String;)V public fun addTracingOrigin (Ljava/lang/String;)V public static fun from (Lio/sentry/config/PropertiesProvider;Lio/sentry/ILogger;)Lio/sentry/ExternalOptions; + public fun getBundleIds ()Ljava/util/List; public fun getContextTags ()Ljava/util/List; public fun getDebug ()Ljava/lang/Boolean; public fun getDist ()Ljava/lang/String; @@ -1539,6 +1541,7 @@ public final class io/sentry/SentryNanotimeDateProvider : io/sentry/SentryDatePr public class io/sentry/SentryOptions { public fun ()V + public fun addBundleId (Ljava/lang/String;)V public fun addCollector (Lio/sentry/ICollector;)V public fun addContextTag (Ljava/lang/String;)V public fun addEventProcessor (Lio/sentry/EventProcessor;)V @@ -1551,6 +1554,7 @@ public class io/sentry/SentryOptions { public fun getBeforeBreadcrumb ()Lio/sentry/SentryOptions$BeforeBreadcrumbCallback; public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback; public fun getBeforeSendTransaction ()Lio/sentry/SentryOptions$BeforeSendTransactionCallback; + public fun getBundleIds ()Ljava/util/List; public fun getCacheDirPath ()Ljava/lang/String; public fun getClientReportRecorder ()Lio/sentry/clientreport/IClientReportRecorder; public fun getCollectors ()Ljava/util/List; @@ -2686,6 +2690,7 @@ public final class io/sentry/protocol/Contexts$Deserializer : io/sentry/JsonDese } public final class io/sentry/protocol/DebugImage : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public static final field JVM Ljava/lang/String; public static final field PROGUARD Ljava/lang/String; public fun ()V public fun getArch ()Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index 7c1fafe97e..4ccef8c690 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -41,6 +41,7 @@ public final class ExternalOptions { new CopyOnWriteArraySet<>(); private @Nullable Boolean printUncaughtStackTrace; private @Nullable Boolean sendClientReports; + private @NotNull List bundleIds = new CopyOnWriteArrayList<>(); @SuppressWarnings("unchecked") public static @NotNull ExternalOptions from( @@ -109,6 +110,9 @@ public final class ExternalOptions { options.addContextTag(contextTag); } options.setProguardUuid(propertiesProvider.getProperty("proguard-uuid")); + for (final String bundleId : propertiesProvider.getList("bundle-ids")) { + options.addBundleId(bundleId); + } options.setIdleTimeout(propertiesProvider.getLongProperty("idle-timeout")); for (final String ignoredExceptionType : @@ -335,4 +339,12 @@ public void setIdleTimeout(final @Nullable Long idleTimeout) { public void setSendClientReports(final @Nullable Boolean sendClientReports) { this.sendClientReports = sendClientReports; } + + public @NotNull List getBundleIds() { + return bundleIds; + } + + public void addBundleId(final @NotNull String bundleId) { + bundleIds.add(bundleId); + } } diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index ad8fc83612..8526492373 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -15,6 +15,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -66,23 +67,35 @@ public MainEventProcessor(final @NotNull SentryOptions options) { } private void setDebugMeta(final @NotNull SentryBaseEvent event) { + final @NotNull List debugImages = new CopyOnWriteArrayList<>(); + if (options.getProguardUuid() != null) { + final DebugImage proguardMappingImage = new DebugImage(); + proguardMappingImage.setType(DebugImage.PROGUARD); + proguardMappingImage.setUuid(options.getProguardUuid()); + debugImages.add(proguardMappingImage); + } + + for (final @NotNull String bundleId : options.getBundleIds()) { + final DebugImage sourceBundleImage = new DebugImage(); + sourceBundleImage.setType(DebugImage.JVM); + sourceBundleImage.setDebugId(bundleId); + debugImages.add(sourceBundleImage); + } + + if (!debugImages.isEmpty()) { DebugMeta debugMeta = event.getDebugMeta(); if (debugMeta == null) { debugMeta = new DebugMeta(); } if (debugMeta.getImages() == null) { - debugMeta.setImages(new ArrayList<>()); - } - List images = debugMeta.getImages(); - if (images != null) { - final DebugImage debugImage = new DebugImage(); - debugImage.setType(DebugImage.PROGUARD); - debugImage.setUuid(options.getProguardUuid()); - images.add(debugImage); - event.setDebugMeta(debugMeta); + debugMeta.setImages(debugImages); + } else { + debugMeta.getImages().addAll(debugImages); } + + event.setDebugMeta(debugMeta); } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index e19064f1da..bc28fb4494 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -66,6 +66,9 @@ public class SentryOptions { */ private final @NotNull List integrations = new CopyOnWriteArrayList<>(); + /** List of bundle IDs representing source bundles. */ + private final @NotNull Set bundleIds = new CopyOnWriteArraySet<>(); + /** * The DSN tells the SDK where to send the events to. If this value is not provided, the SDK will * just not send any events. @@ -1776,6 +1779,31 @@ public void setProguardUuid(final @Nullable String proguardUuid) { this.proguardUuid = proguardUuid; } + /** + * Adds a bundle ID (also known as debugId) representing a source bundle that contains sources for + * this application. These sources will be used to source code for frames of an exceptions stack + * trace. + * + * @param bundleId Bundle ID generated by sentry-cli or the sentry-android-gradle-plugin + */ + public void addBundleId(final @Nullable String bundleId) { + if (bundleId != null) { + final @NotNull String trimmedBundleId = bundleId.trim(); + if (!trimmedBundleId.isEmpty()) { + this.bundleIds.add(trimmedBundleId); + } + } + } + + /** + * Returns all configured bundle IDs referencing source code bundles. + * + * @return list of bundle IDs + */ + public @NotNull Set getBundleIds() { + return bundleIds; + } + /** * Returns Context tags names applied to Sentry events as Sentry tags. * @@ -2229,6 +2257,9 @@ public void merge(final @NotNull ExternalOptions options) { if (options.getIdleTimeout() != null) { setIdleTimeout(options.getIdleTimeout()); } + for (String bundleId : options.getBundleIds()) { + addBundleId(bundleId); + } } private @NotNull SdkVersion createSdkVersion() { diff --git a/sentry/src/main/java/io/sentry/protocol/DebugImage.java b/sentry/src/main/java/io/sentry/protocol/DebugImage.java index 128eabd53f..20d81143db 100644 --- a/sentry/src/main/java/io/sentry/protocol/DebugImage.java +++ b/sentry/src/main/java/io/sentry/protocol/DebugImage.java @@ -50,6 +50,7 @@ */ public final class DebugImage implements JsonUnknown, JsonSerializable { public static final String PROGUARD = "proguard"; + public static final String JVM = "jvm"; /** * The unique UUID of the image. diff --git a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt index f2adefc972..57be223218 100644 --- a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt @@ -216,6 +216,37 @@ class ExternalOptionsTest { } } + @Test + fun `creates options with single bundle ID using external properties`() { + withPropertiesFile("bundle-ids=12ea7a02-46ac-44c0-a5bb-6d1fd9586411") { options -> + assertTrue(options.bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411"))) + } + } + + @Test + fun `creates options with multiple bundle IDs using external properties`() { + withPropertiesFile("bundle-ids=12ea7a02-46ac-44c0-a5bb-6d1fd9586411,faa3ab42-b1bd-4659-af8e-1682324aa744") { options -> + assertTrue(options.bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"))) + } + } + + @Test + fun `creates options with empty bundle IDs using external properties`() { + withPropertiesFile("bundle-ids=") { options -> + assertTrue(options.bundleIds.size == 1) + // trimming is tested in SentryOptionsTest so even though there's an empty string here + // it will be filtered when being merged with SentryOptions + assertTrue(options.bundleIds.containsAll(listOf(""))) + } + } + + @Test + fun `creates options with missing bundle IDs using external properties`() { + withPropertiesFile("") { options -> + assertTrue(options.bundleIds.isEmpty()) + } + } + private fun withPropertiesFile(textLines: List = emptyList(), logger: ILogger = mock(), fn: (ExternalOptions) -> Unit) { // create a sentry.properties file in temporary folder val temporaryFolder = TemporaryFolder() diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 204635f87c..427973edc8 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -357,6 +357,7 @@ class SentryOptionsTest { externalOptions.addContextTag("requestId") externalOptions.proguardUuid = "1234" externalOptions.idleTimeout = 1500L + externalOptions.bundleIds.addAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411 ", " faa3ab42-b1bd-4659-af8e-1682324aa744")) val options = SentryOptions() options.merge(externalOptions) @@ -380,6 +381,7 @@ class SentryOptionsTest { assertEquals(listOf("userId", "requestId"), options.contextTags) assertEquals("1234", options.proguardUuid) assertEquals(1500L, options.idleTimeout) + assertEquals(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"), options.bundleIds) } @Test From 128c41f988839be909053ada15d46500dab5df36 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Apr 2023 13:39:06 +0200 Subject: [PATCH 2/8] use set; generate api; cleanup --- sentry/api/sentry.api | 4 ++-- sentry/src/main/java/io/sentry/ExternalOptions.java | 4 ++-- sentry/src/test/java/io/sentry/SentryOptionsTest.kt | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 736b105f03..490926dda6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -236,7 +236,7 @@ public final class io/sentry/ExternalOptions { public fun addTracePropagationTarget (Ljava/lang/String;)V public fun addTracingOrigin (Ljava/lang/String;)V public static fun from (Lio/sentry/config/PropertiesProvider;Lio/sentry/ILogger;)Lio/sentry/ExternalOptions; - public fun getBundleIds ()Ljava/util/List; + public fun getBundleIds ()Ljava/util/Set; public fun getContextTags ()Ljava/util/List; public fun getDebug ()Ljava/lang/Boolean; public fun getDist ()Ljava/lang/String; @@ -1554,7 +1554,7 @@ public class io/sentry/SentryOptions { public fun getBeforeBreadcrumb ()Lio/sentry/SentryOptions$BeforeBreadcrumbCallback; public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback; public fun getBeforeSendTransaction ()Lio/sentry/SentryOptions$BeforeSendTransactionCallback; - public fun getBundleIds ()Ljava/util/List; + public fun getBundleIds ()Ljava/util/Set; public fun getCacheDirPath ()Ljava/lang/String; public fun getClientReportRecorder ()Lio/sentry/clientreport/IClientReportRecorder; public fun getCollectors ()Ljava/util/List; diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index 4ccef8c690..4ef3f31d89 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -41,7 +41,7 @@ public final class ExternalOptions { new CopyOnWriteArraySet<>(); private @Nullable Boolean printUncaughtStackTrace; private @Nullable Boolean sendClientReports; - private @NotNull List bundleIds = new CopyOnWriteArrayList<>(); + private @NotNull Set bundleIds = new CopyOnWriteArraySet<>(); @SuppressWarnings("unchecked") public static @NotNull ExternalOptions from( @@ -340,7 +340,7 @@ public void setSendClientReports(final @Nullable Boolean sendClientReports) { this.sendClientReports = sendClientReports; } - public @NotNull List getBundleIds() { + public @NotNull Set getBundleIds() { return bundleIds; } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 427973edc8..fff91004a5 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -381,7 +381,7 @@ class SentryOptionsTest { assertEquals(listOf("userId", "requestId"), options.contextTags) assertEquals("1234", options.proguardUuid) assertEquals(1500L, options.idleTimeout) - assertEquals(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"), options.bundleIds) + assertEquals(setOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"), options.bundleIds) } @Test From 4d34601859e927b5a5438e6cb323d8d0dceb3a1f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 19 Apr 2023 15:36:14 +0200 Subject: [PATCH 3/8] Remove AndroidManifest parsing --- .../core/AndroidOptionsInitializer.java | 3 - .../android/core/ManifestMetadataReader.java | 9 --- .../core/AndroidOptionsInitializerTest.kt | 2 +- .../core/ManifestMetadataReaderTest.kt | 60 ------------------- 4 files changed, 1 insertion(+), 73 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 c6b2f7445c..204bbf081d 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 @@ -280,9 +280,6 @@ private static void readDefaultOptionValues( } // TODO when to parse, when not? - // TODO parse bundle IDs as well; extract proguard parsing to parser; also parse bundleIds; - // return data class containing both properties - // TODO on SAGP side, add bundleIds to sentry-debug-meta.properties file final @Nullable Properties debugMetaProperties = loadDebugMetaProperties(context, options.getLogger()); 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 9df6e9d7b9..2d04798722 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 @@ -77,8 +77,6 @@ final class ManifestMetadataReader { static final String ATTACH_THREADS = "io.sentry.attach-threads"; static final String PROGUARD_UUID = "io.sentry.proguard-uuid"; - - static final String SOURCE_BUNDLE_IDS = "io.sentry.bundle-ids"; static final String IDLE_TIMEOUT = "io.sentry.traces.idle-timeout"; static final String ATTACH_SCREENSHOT = "io.sentry.attach-screenshot"; @@ -343,13 +341,6 @@ static void applyMetadata( SentryIntegrationPackageStorage.getInstance().addIntegration(integration); } } - - List sourceBundleIds = readList(metadata, logger, SOURCE_BUNDLE_IDS); - if (sourceBundleIds != null) { - for (String bundleId : sourceBundleIds) { - options.addBundleId(bundleId); - } - } } options diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 98ec551e98..69646633f2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -270,7 +270,7 @@ class AndroidOptionsInitializerTest { fun `init should set bundle IDs id on start`() { fixture.initSut( Bundle().apply { - putString(ManifestMetadataReader.SOURCE_BUNDLE_IDS, "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") + putString("io.sentry.bundle-ids", "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") }, hasAppContext = false ) 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 d4d9e44119..efd1a47f89 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 @@ -1243,64 +1243,4 @@ class ManifestMetadataReaderTest { assertNotNull(resultingSet) assert(resultingSet.containsAll(listOf("Database Instrumentation", "OkHttp Instrumentation"))) } - - @Test - fun `applyMetadata reads single bundle ID`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "12ea7a02-46ac-44c0-a5bb-6d1fd9586411") - val context = fixture.getContext(metaData = bundle) - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - val bundleIds = fixture.options.bundleIds - assertNotNull(bundleIds) - assert(bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411"))) - } - - @Test - fun `applyMetadata reads multiple bundle IDs`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") - val context = fixture.getContext(metaData = bundle) - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - val bundleIds = fixture.options.bundleIds - assertNotNull(bundleIds) - assert(bundleIds.containsAll(listOf("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", "faa3ab42-b1bd-4659-af8e-1682324aa744"))) - } - - @Test - fun `applyMetadata reads empty bundle IDs`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.SOURCE_BUNDLE_IDS to "") - val context = fixture.getContext(metaData = bundle) - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - val bundleIds = fixture.options.bundleIds - assertNotNull(bundleIds) - assert(bundleIds.isEmpty()) - } - - @Test - fun `applyMetadata reads missing bundle IDs`() { - // Arrange - val bundle = bundleOf() - val context = fixture.getContext(metaData = bundle) - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - val bundleIds = fixture.options.bundleIds - assertNotNull(bundleIds) - assert(bundleIds.isEmpty()) - } } From ac90db0bd8a0f6d500fbc24edbc9f9b620376479 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 11 May 2023 12:51:59 +0200 Subject: [PATCH 4/8] Add test; cleanup --- .../core/AndroidOptionsInitializer.java | 10 +++------ .../core/AndroidOptionsInitializerTest.kt | 22 ++++++++++++++----- .../sentry/android/core/ContextUtilsTest.kt | 13 +++++++---- 3 files changed, 28 insertions(+), 17 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 7a8c8bdc2b..d239106224 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 @@ -287,7 +287,6 @@ private static void readDefaultOptionValues( } } - // TODO when to parse, when not? final @Nullable Properties debugMetaProperties = loadDebugMetaProperties(context, options.getLogger()); @@ -304,12 +303,9 @@ private static void readDefaultOptionValues( debugMetaProperties.getProperty("io.sentry.bundle-ids"); options.getLogger().log(SentryLevel.DEBUG, "Bundle IDs found: %s", bundleIdStrings); if (bundleIdStrings != null) { - // TODO really nullable? - final @Nullable String[] bundleIds = bundleIdStrings.split(",", -1); - if (bundleIds != null) { - for (final String bundleId : bundleIds) { - options.addBundleId(bundleId); - } + final @NotNull String[] bundleIds = bundleIdStrings.split(",", -1); + for (final String bundleId : bundleIds) { + options.addBundleId(bundleId); } } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 2faa804fb5..de779842fd 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -1,6 +1,7 @@ package io.sentry.android.core import android.content.Context +import android.content.res.AssetManager import android.os.Bundle import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -46,12 +47,14 @@ class AndroidOptionsInitializerTest { hasAppContext: Boolean = true, useRealContext: Boolean = false, configureOptions: SentryAndroidOptions.() -> Unit = {}, - configureContext: Context.() -> Unit = {} + configureContext: Context.() -> Unit = {}, + assets: AssetManager? = null ) { mockContext = if (metadata != null) { ContextUtilsTest.mockMetaData( mockContext = ContextUtilsTest.createMockContext(hasAppContext), - metaData = metadata + metaData = metadata, + assets = assets ) } else { ContextUtilsTest.createMockContext(hasAppContext) @@ -279,11 +282,18 @@ class AndroidOptionsInitializerTest { @Test fun `init should set bundle IDs id on start`() { + val assets = mock() + + whenever(assets.open("sentry-debug-meta.properties")).thenReturn( + """ + io.sentry.bundle-ids=12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744 + """.trimIndent().byteInputStream() + ) + fixture.initSut( - Bundle().apply { - putString("io.sentry.bundle-ids", "12ea7a02-46ac-44c0-a5bb-6d1fd9586411, faa3ab42-b1bd-4659-af8e-1682324aa744") - }, - hasAppContext = false + Bundle(), + hasAppContext = false, + assets = assets ) assertTrue(fixture.sentryOptions.bundleIds.size == 2) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt index b0a5e60685..c05d71e2b4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt @@ -12,17 +12,22 @@ import org.mockito.kotlin.whenever import java.io.FileNotFoundException object ContextUtilsTest { - fun mockMetaData(mockContext: Context = createMockContext(hasAppContext = false), metaData: Bundle): Context { + fun mockMetaData(mockContext: Context = createMockContext(hasAppContext = false), metaData: Bundle, assets: AssetManager? = null): Context { val mockPackageManager = mock() val mockApplicationInfo = mock() - val assets = mock() whenever(mockContext.packageName).thenReturn("io.sentry.sample.test") whenever(mockContext.packageManager).thenReturn(mockPackageManager) whenever(mockPackageManager.getApplicationInfo(mockContext.packageName, PackageManager.GET_META_DATA)) .thenReturn(mockApplicationInfo) - whenever(assets.open(any())).thenThrow(FileNotFoundException()) - whenever(mockContext.assets).thenReturn(assets) + + if (assets == null) { + val mockAssets = mock() + whenever(mockAssets.open(any())).thenThrow(FileNotFoundException()) + whenever(mockContext.assets).thenReturn(mockAssets) + } else { + whenever(mockContext.assets).thenReturn(assets) + } mockApplicationInfo.metaData = metaData return mockContext From 749beb1e0b5f2269b1ec657915fd8c550835593a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 11 May 2023 14:33:33 +0200 Subject: [PATCH 5/8] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4054f490ef..3d3a8affb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ - If you would like us to provide support for the old approach working alongside the new one on Android 11 and above (e.g. for raising events for slow code on main thread), consider upvoting [this issue](https://github.com/getsentry/sentry-java/issues/2693). - The old watchdog implementation will continue working for older API versions (Android < 11) - Open up `TransactionOptions`, `ITransaction` and `IHub` methods allowing consumers modify start/end timestamp of transactions and spans ([#2701](https://github.com/getsentry/sentry-java/pull/2701)) +- Send source bundle IDs to Sentry to enable source context ([#2663](https://github.com/getsentry/sentry-java/pull/2663)) + - For more information on how to enable source context, please refer to [#633](https://github.com/getsentry/sentry-java/issues/633) ### Fixes From e98afc301aeb504562ce956e9d383039bc86213a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 11 May 2023 15:35:41 +0200 Subject: [PATCH 6/8] More tests --- .../core/AndroidOptionsInitializerTest.kt | 20 +++++++ .../java/io/sentry/MainEventProcessor.java | 3 +- .../java/io/sentry/MainEventProcessorTest.kt | 57 +++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index de779842fd..18bd55bba0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -280,6 +280,26 @@ class AndroidOptionsInitializerTest { assertEquals("proguard-uuid", fixture.sentryOptions.proguardUuid) } + @Test + fun `init should set proguard uuid from properties id on start`() { + val assets = mock() + + whenever(assets.open("sentry-debug-meta.properties")).thenReturn( + """ + io.sentry.ProguardUuids=12ea7a02-46ac-44c0-a5bb-6d1fd9586411 + """.trimIndent().byteInputStream() + ) + + fixture.initSut( + Bundle(), + hasAppContext = false, + assets = assets + ) + + assertNotNull(fixture.sentryOptions.proguardUuid) + assertEquals("12ea7a02-46ac-44c0-a5bb-6d1fd9586411", fixture.sentryOptions.proguardUuid) + } + @Test fun `init should set bundle IDs id on start`() { val assets = mock() diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index b435bf4970..d046855958 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -15,7 +15,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -66,7 +65,7 @@ public MainEventProcessor(final @NotNull SentryOptions options) { } private void setDebugMeta(final @NotNull SentryBaseEvent event) { - final @NotNull List debugImages = new CopyOnWriteArrayList<>(); + final @NotNull List debugImages = new ArrayList<>(); if (options.getProguardUuid() != null) { final DebugImage proguardMappingImage = new DebugImage(); diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index cc2774e323..4e6b4f3d99 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -49,6 +49,7 @@ class MainEventProcessorTest { resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10, proguardUuid: String? = null, + bundleIds: List? = null, modules: Map? = null ): MainEventProcessor { sentryOptions.isAttachThreads = attachThreads @@ -63,6 +64,7 @@ class MainEventProcessorTest { if (proguardUuid != null) { sentryOptions.proguardUuid = proguardUuid } + bundleIds?.let { it.forEach { sentryOptions.addBundleId(it) } } tags.forEach { sentryOptions.setTag(it.key, it.value) } whenever(getLocalhost.canonicalHostName).thenAnswer { if (resolveHostDelay != null) { @@ -488,6 +490,23 @@ class MainEventProcessorTest { } } + @Test + fun `when event does not have debug meta and bundle ids are set, attaches debug information`() { + val sut = fixture.getSut(bundleIds = listOf("id1", "id2")) + + var event = SentryEvent() + event = sut.process(event, Hint()) + + assertNotNull(event.debugMeta) { + assertNotNull(it.images) { images -> + assertEquals("id1", images[0].debugId) + assertEquals("jvm", images[0].type) + assertEquals("id2", images[1].debugId) + assertEquals("jvm", images[1].type) + } + } + } + @Test fun `when event has debug meta and proguard uuids are set, attaches debug information`() { val sut = fixture.getSut(proguardUuid = "id1") @@ -504,6 +523,44 @@ class MainEventProcessorTest { } } + @Test + fun `when event has debug meta and bundle ids are set, attaches debug information`() { + val sut = fixture.getSut(bundleIds = listOf("id1", "id2")) + + var event = SentryEvent() + event.debugMeta = DebugMeta() + event = sut.process(event, Hint()) + + assertNotNull(event.debugMeta) { + assertNotNull(it.images) { images -> + assertEquals("id1", images[0].debugId) + assertEquals("jvm", images[0].type) + assertEquals("id2", images[1].debugId) + assertEquals("jvm", images[1].type) + } + } + } + + @Test + fun `when event has debug meta as well as images and bundle ids are set, attaches debug information`() { + val sut = fixture.getSut(bundleIds = listOf("id1", "id2")) + + var event = SentryEvent() + event.debugMeta = DebugMeta().also { + it.images = listOf() + } + event = sut.process(event, Hint()) + + assertNotNull(event.debugMeta) { + assertNotNull(it.images) { images -> + assertEquals("id1", images[0].debugId) + assertEquals("jvm", images[0].type) + assertEquals("id2", images[1].debugId) + assertEquals("jvm", images[1].type) + } + } + } + @Test fun `when processor is closed, closes hostname cache`() { val sut = fixture.getSut(serverName = null) From 885b8afd75ec71c9c3fd225647242761d94b1905 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 15 May 2023 12:29:48 +0200 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Roman Zavarnitsyn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d3a8affb6..7325160718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ - The old watchdog implementation will continue working for older API versions (Android < 11) - Open up `TransactionOptions`, `ITransaction` and `IHub` methods allowing consumers modify start/end timestamp of transactions and spans ([#2701](https://github.com/getsentry/sentry-java/pull/2701)) - Send source bundle IDs to Sentry to enable source context ([#2663](https://github.com/getsentry/sentry-java/pull/2663)) - - For more information on how to enable source context, please refer to [#633](https://github.com/getsentry/sentry-java/issues/633) + - For more information on how to enable source context, please refer to [#633](https://github.com/getsentry/sentry-java/issues/633#issuecomment-1465599120) ### Fixes From 84a52065f9b853a149c8a1e077c8e14930a4458a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 15 May 2023 12:40:54 +0200 Subject: [PATCH 8/8] Remove unused method --- .../core/AndroidOptionsInitializer.java | 25 ------------------- 1 file changed, 25 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 d239106224..67101e3655 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 @@ -334,31 +334,6 @@ private static void readDefaultOptionValues( return null; } - private static @Nullable String getProguardUUID( - final @NotNull Context context, final @NotNull ILogger logger) { - final AssetManager assets = context.getAssets(); - // one may have thousands of asset files and looking up this list might slow down the SDK init. - // quite a bit, for this reason, we try to open the file directly and take care of errors - // like FileNotFoundException - try (final InputStream is = - new BufferedInputStream(assets.open("sentry-debug-meta.properties"))) { - final Properties properties = new Properties(); - properties.load(is); - - final String uuid = properties.getProperty("io.sentry.ProguardUuids"); - logger.log(SentryLevel.DEBUG, "Proguard UUID found: %s", uuid); - return uuid; - } catch (FileNotFoundException e) { - logger.log(SentryLevel.INFO, "sentry-debug-meta.properties file was not found."); - } catch (IOException e) { - logger.log(SentryLevel.ERROR, "Error getting Proguard UUIDs.", e); - } catch (RuntimeException e) { - logger.log(SentryLevel.ERROR, "sentry-debug-meta.properties file is malformed.", e); - } - - return null; - } - /** * Returns the sentry release version (eg io.sentry.sample@1.0.0+10000) - * packageName@versionName+buildVersion