diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ee6a1d584c..ce06843ff98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348)) +- Fix ensure all options are processed before integrations are loaded ([#2377](https://github.com/getsentry/sentry-java/pull/2377)) ### Features diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 4bdcfecd270..2767a3738d6 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -224,6 +224,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V + public fun getDirectory ()Ljava/io/File; public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)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 8f3543825be..821f1471195 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 @@ -16,6 +16,7 @@ import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; +import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.util.Objects; import java.io.BufferedInputStream; import java.io.File; @@ -25,6 +26,7 @@ import java.util.Properties; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * Android Options initializer, it reads configurations from AndroidManifest and sets to the @@ -42,62 +44,11 @@ private AndroidOptionsInitializer() {} * @param options the SentryAndroidOptions * @param context the Application context */ - static void init(final @NotNull SentryAndroidOptions options, final @NotNull Context context) { - Objects.requireNonNull(context, "The application context is required."); - Objects.requireNonNull(options, "The options object is required."); - - init(options, context, new AndroidLogger(), false, false); - } - - /** - * Init method of the Android Options initializer - * - * @param options the SentryAndroidOptions - * @param context the Application context - * @param logger the ILogger interface - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath - */ - static void init( - final @NotNull SentryAndroidOptions options, - @NotNull Context context, - final @NotNull ILogger logger, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { - init( - options, - context, - logger, - new BuildInfoProvider(logger), - isFragmentAvailable, - isTimberAvailable); - } - - /** - * Init method of the Android Options initializer - * - * @param options the SentryAndroidOptions - * @param context the Application context - * @param logger the ILogger interface - * @param buildInfoProvider the BuildInfoProvider interface - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath - */ - static void init( - final @NotNull SentryAndroidOptions options, - @NotNull Context context, - final @NotNull ILogger logger, - final @NotNull BuildInfoProvider buildInfoProvider, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { - init( - options, - context, - logger, - buildInfoProvider, - new LoadClass(), - isFragmentAvailable, - isTimberAvailable); + @TestOnly + static void loadDefaultAndMetadataOptions( + final @NotNull SentryAndroidOptions options, final @NotNull Context context) { + final ILogger logger = new AndroidLogger(); + loadDefaultAndMetadataOptions(options, context, logger, new BuildInfoProvider(logger)); } /** @@ -107,18 +58,12 @@ static void init( * @param context the Application context * @param logger the ILogger interface * @param buildInfoProvider the BuildInfoProvider interface - * @param loadClass the LoadClass wrapper - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath */ - static void init( + static void loadDefaultAndMetadataOptions( final @NotNull SentryAndroidOptions options, @NotNull Context context, final @NotNull ILogger logger, - final @NotNull BuildInfoProvider buildInfoProvider, - final @NotNull LoadClass loadClass, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { + final @NotNull BuildInfoProvider buildInfoProvider) { Objects.requireNonNull(context, "The context is required."); // it returns null if ContextImpl, so let's check for nullability @@ -134,7 +79,34 @@ static void init( ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider); initializeCacheDirs(context, options); - options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); + + readDefaultOptionValues(options, context, buildInfoProvider); + } + + @TestOnly + static void initializeIntegrationsAndProcessors( + final @NotNull SentryAndroidOptions options, final @NotNull Context context) { + initializeIntegrationsAndProcessors( + options, + context, + new BuildInfoProvider(new AndroidLogger()), + new LoadClass(), + false, + false); + } + + static void initializeIntegrationsAndProcessors( + final @NotNull SentryAndroidOptions options, + final @NotNull Context context, + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull LoadClass loadClass, + final boolean isFragmentAvailable, + final boolean isTimberAvailable) { + + if (options.getCacheDirPath() != null + && options.getEnvelopeDiskCache() instanceof NoOpEnvelopeCache) { + options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); + } final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options); @@ -148,8 +120,6 @@ static void init( isFragmentAvailable, isTimberAvailable); - readDefaultOptionValues(options, context, buildInfoProvider); - options.addEventProcessor( new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index 9bcc107d21a..e9e8a75c1fb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -9,10 +9,8 @@ import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; -import io.sentry.transport.NoOpEnvelopeCache; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Date; @@ -102,11 +100,23 @@ public static synchronized void init( (isTimberUpstreamAvailable && classLoader.isClassAvailable(SENTRY_TIMBER_INTEGRATION_CLASS_NAME, options)); - AndroidOptionsInitializer.init( - options, context, logger, isFragmentAvailable, isTimberAvailable); + final BuildInfoProvider buildInfoProvider = new BuildInfoProvider(logger); + final LoadClass loadClass = new LoadClass(); + + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + options, context, logger, buildInfoProvider); + configuration.configure(options); + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + options, + context, + buildInfoProvider, + loadClass, + isFragmentAvailable, + isTimberAvailable); + deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); - resetEnvelopeCacheIfNeeded(options); }, true); } catch (IllegalAccessException e) { @@ -132,7 +142,8 @@ public static synchronized void init( /** * Deduplicate potentially duplicated Fragment and Timber integrations, which can be added - * automatically by our SDK as well as by the user. The user's ones win over ours. + * automatically by our SDK as well as by the user. The user's ones (provided first in the + * options.integrations list) win over ours. * * @param options SentryOptions to retrieve integrations from */ @@ -158,31 +169,17 @@ private static void deduplicateIntegrations( } if (fragmentIntegrations.size() > 1) { - for (int i = 0; i < fragmentIntegrations.size() - 1; i++) { + for (int i = 1; i < fragmentIntegrations.size(); i++) { final Integration integration = fragmentIntegrations.get(i); options.getIntegrations().remove(integration); } } if (timberIntegrations.size() > 1) { - for (int i = 0; i < timberIntegrations.size() - 1; i++) { + for (int i = 1; i < timberIntegrations.size(); i++) { final Integration integration = timberIntegrations.get(i); options.getIntegrations().remove(integration); } } } - - /** - * Resets envelope cache if {@link SentryOptions#getCacheDirPath()} was set to null by the user - * and the IEnvelopCache implementation remained ours (AndroidEnvelopeCache), which relies on - * cacheDirPath set. - * - * @param options SentryOptions to retrieve cacheDirPath from - */ - private static void resetEnvelopeCacheIfNeeded(final @NotNull SentryAndroidOptions options) { - if (options.getCacheDirPath() == null - && options.getEnvelopeDiskCache() instanceof AndroidEnvelopeCache) { - options.setEnvelopeDiskCache(NoOpEnvelopeCache.getInstance()); - } - } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index e08ef8b1281..1104adadd47 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -17,6 +17,7 @@ import java.io.File; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal public final class AndroidEnvelopeCache extends EnvelopeCache { @@ -58,6 +59,11 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { } } + @TestOnly + public @NotNull File getDirectory() { + return directory; + } + private void writeStartupCrashMarkerFile() { // we use outbox path always, as it's the one that will also contain markers if hybrid sdks // decide to write it, which will trigger the blocking init 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 e760dadee80..f3bc66b7f16 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 @@ -52,8 +52,12 @@ class AndroidOptionsInitializerTest { whenever(mockContext.applicationContext.cacheDir).thenReturn(file) } mockContext.configureContext() + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + sentryOptions, + if (useRealContext) context else mockContext + ) sentryOptions.configureOptions() - AndroidOptionsInitializer.init( + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( sentryOptions, if (useRealContext) context else mockContext ) @@ -71,12 +75,19 @@ class AndroidOptionsInitializerTest { putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") } ) - sentryOptions.setDebug(true) - AndroidOptionsInitializer.init( + sentryOptions.isDebug = true + val buildInfo = createBuildInfo(minApi) + + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( sentryOptions, - mockContext, + context, logger, - createBuildInfo(minApi), + buildInfo + ) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + sentryOptions, + context, + buildInfo, createClassMock(classToLoad), isFragmentAvailable, isTimberAvailable @@ -381,9 +392,13 @@ class AndroidOptionsInitializerTest { @Test fun `When Activity Frames Tracking is enabled, the Activity Frames Tracker should be available`() { - fixture.initSut(hasAppContext = true, configureOptions = { - isEnableFramesTracking = true - }) + fixture.initSut( + hasAppContext = true, + useRealContext = true, + configureOptions = { + isEnableFramesTracking = true + } + ) val activityLifeCycleIntegration = fixture.sentryOptions.integrations .first { it is ActivityLifecycleIntegration } @@ -395,7 +410,7 @@ class AndroidOptionsInitializerTest { @Test fun `When Frames Tracking is disabled, the Activity Frames Tracker should not be available`() { - fixture.initSut(hasAppContext = true, configureOptions = { + fixture.initSut(hasAppContext = true, useRealContext = true, configureOptions = { isEnableFramesTracking = false }) @@ -410,9 +425,13 @@ class AndroidOptionsInitializerTest { @Test fun `When Frames Tracking is initially disabled, but enabled via configureOptions it should be available`() { fixture.sentryOptions.isEnableFramesTracking = false - fixture.initSut(hasAppContext = true, configureOptions = { - isEnableFramesTracking = true - }) + fixture.initSut( + hasAppContext = true, + useRealContext = true, + configureOptions = { + isEnableFramesTracking = true + } + ) val activityLifeCycleIntegration = fixture.sentryOptions.integrations .first { it is ActivityLifecycleIntegration } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 57dcb5448cd..f3a32f30e6b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -88,7 +88,21 @@ class AndroidTransactionProfilerTest { @BeforeTest fun `set up`() { context = ApplicationProvider.getApplicationContext() - AndroidOptionsInitializer.init(fixture.options, context, fixture.mockLogger, false, false) + val buildInfoProvider = BuildInfoProvider(fixture.mockLogger) + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + fixture.options, + context, + fixture.mockLogger, + buildInfoProvider + ) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + fixture.options, + context, + buildInfoProvider, + LoadClass(), + false, + false + ) // Profiler doesn't start if the folder doesn't exists. // Usually it's generated when calling Sentry.init, but for tests we can create it manually. File(fixture.options.profilingTracesDirPath!!).mkdirs() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index a6d4f6cddfe..ebbfd13b1f3 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -10,16 +10,21 @@ import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.FATAL +import io.sentry.SentryOptions +import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration import io.sentry.cache.IEnvelopeCache import io.sentry.transport.NoOpEnvelopeCache +import io.sentry.util.StringUtils import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import java.nio.file.Files +import kotlin.io.path.absolutePathString import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -163,6 +168,24 @@ class SentryAndroidTest { assertTrue { refOptions!!.envelopeDiskCache is CustomEnvelopCache } } + @Test + fun `When initializing Sentry manually and changing both cache dir and dsn, the corresponding options should reflect that change`() { + var options: SentryOptions? = null + + val mockContext = ContextUtilsTest.createMockContext(true) + val cacheDirPath = Files.createTempDirectory("new_cache").absolutePathString() + SentryAndroid.init(mockContext) { + it.dsn = "https://key@sentry.io/123" + it.cacheDirPath = cacheDirPath + options = it + } + + val dsnHash = StringUtils.calculateStringHash(options!!.dsn, options!!.logger) + val expectedCacheDir = "$cacheDirPath/$dsnHash" + assertEquals(expectedCacheDir, options!!.cacheDirPath) + assertEquals(expectedCacheDir, (options!!.envelopeDiskCache as AndroidEnvelopeCache).directory.absolutePath) + } + private class CustomEnvelopCache : IEnvelopeCache { override fun iterator(): MutableIterator = TODO() override fun store(envelope: SentryEnvelope, hint: Hint) = Unit diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt index a54f39e5684..990d83b142f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt @@ -132,7 +132,8 @@ class SentryInitProviderTest { val mockContext = ContextUtilsTest.mockMetaData(metaData = metaData) metaData.putBoolean(ManifestMetadataReader.NDK_ENABLE, false) - AndroidOptionsInitializer.init(sentryOptions, mockContext) + AndroidOptionsInitializer.loadDefaultAndMetadataOptions(sentryOptions, mockContext) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors(sentryOptions, mockContext) assertFalse(sentryOptions.isEnableNdk) }