diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e495e829e..86694e240c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Allow removing integrations in SentryAndroid.init ([#2826](https://github.com/getsentry/sentry-java/pull/2826)) - Fix concurrent access to frameMetrics listener ([#2823](https://github.com/getsentry/sentry-java/pull/2823)) ## 6.25.0 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 8c1f78b86c..f3cf00f96c 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 @@ -26,6 +26,7 @@ import io.sentry.internal.gestures.GestureTargetLocator; import io.sentry.internal.viewhierarchy.ViewHierarchyExporter; import io.sentry.transport.NoOpEnvelopeCache; +import io.sentry.util.LazyEvaluator; import io.sentry.util.Objects; import java.io.File; import java.util.ArrayList; @@ -100,14 +101,16 @@ static void loadDefaultAndMetadataOptions( @TestOnly static void initializeIntegrationsAndProcessors( - final @NotNull SentryAndroidOptions options, final @NotNull Context context) { + final @NotNull SentryAndroidOptions options, + final @NotNull Context context, + final @NotNull LoadClass loadClass, + final @NotNull ActivityFramesTracker activityFramesTracker) { initializeIntegrationsAndProcessors( options, context, new BuildInfoProvider(new AndroidLogger()), - new LoadClass(), - false, - false); + loadClass, + activityFramesTracker); } static void initializeIntegrationsAndProcessors( @@ -115,26 +118,13 @@ static void initializeIntegrationsAndProcessors( final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider, final @NotNull LoadClass loadClass, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { + final @NotNull ActivityFramesTracker activityFramesTracker) { if (options.getCacheDirPath() != null && options.getEnvelopeDiskCache() instanceof NoOpEnvelopeCache) { options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); } - final ActivityFramesTracker activityFramesTracker = - new ActivityFramesTracker(loadClass, options); - - installDefaultIntegrations( - context, - options, - buildInfoProvider, - loadClass, - activityFramesTracker, - isFragmentAvailable, - isTimberAvailable); - options.addEventProcessor( new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); @@ -192,7 +182,7 @@ static void initializeIntegrationsAndProcessors( } } - private static void installDefaultIntegrations( + static void installDefaultIntegrations( final @NotNull Context context, final @NotNull SentryAndroidOptions options, final @NotNull BuildInfoProvider buildInfoProvider, @@ -201,14 +191,18 @@ private static void installDefaultIntegrations( final boolean isFragmentAvailable, final boolean isTimberAvailable) { + // Integration MUST NOT cache option values in ctor, as they will be configured later by the + // user + // read the startup crash marker here to avoid doing double-IO for the SendCachedEnvelope // integrations below - final boolean hasStartupCrashMarker = AndroidEnvelopeCache.hasStartupCrashMarker(options); + LazyEvaluator startupCrashMarkerEvaluator = + new LazyEvaluator<>(() -> AndroidEnvelopeCache.hasStartupCrashMarker(options)); options.addIntegration( new SendCachedEnvelopeIntegration( new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()), - hasStartupCrashMarker)); + startupCrashMarkerEvaluator)); // Integrations are registered in the same order. NDK before adding Watch outbox, // because sentry-native move files around and we don't want to watch that. @@ -228,7 +222,7 @@ private static void installDefaultIntegrations( options.addIntegration( new SendCachedEnvelopeIntegration( new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()), - hasStartupCrashMarker)); + startupCrashMarkerEvaluator)); // AppLifecycleIntegration has to be installed before AnrIntegration, because AnrIntegration // relies on AppState set by it diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index bda8335822..e0d08325b4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -5,6 +5,7 @@ import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.util.LazyEvaluator; import io.sentry.util.Objects; import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; @@ -16,13 +17,13 @@ final class SendCachedEnvelopeIntegration implements Integration { private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory; - private final boolean hasStartupCrashMarker; + private final @NotNull LazyEvaluator startupCrashMarkerEvaluator; public SendCachedEnvelopeIntegration( final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory, - final boolean hasStartupCrashMarker) { + final @NotNull LazyEvaluator startupCrashMarkerEvaluator) { this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); - this.hasStartupCrashMarker = hasStartupCrashMarker; + this.startupCrashMarkerEvaluator = startupCrashMarkerEvaluator; } @Override @@ -62,7 +63,7 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { } }); - if (hasStartupCrashMarker) { + if (startupCrashMarkerEvaluator.getValue()) { androidOptions .getLogger() .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); 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 eb33039a71..044e727a5c 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 @@ -104,20 +104,29 @@ public static synchronized void init( final BuildInfoProvider buildInfoProvider = new BuildInfoProvider(logger); final LoadClass loadClass = new LoadClass(); + final ActivityFramesTracker activityFramesTracker = + new ActivityFramesTracker(loadClass, options); AndroidOptionsInitializer.loadDefaultAndMetadataOptions( options, context, logger, buildInfoProvider); - configuration.configure(options); - - AndroidOptionsInitializer.initializeIntegrationsAndProcessors( - options, + // We install the default integrations before the option configuration, so that the user + // can remove any of them. Integrations will not evaluate the options immediately, but + // will use them later, after being configured. + AndroidOptionsInitializer.installDefaultIntegrations( context, + options, buildInfoProvider, loadClass, + activityFramesTracker, isFragmentAvailable, isTimberAvailable); + configuration.configure(options); + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + options, context, buildInfoProvider, loadClass, activityFramesTracker); + deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); }, true); @@ -151,7 +160,7 @@ 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 (provided first in the + * automatically by our SDK as well as by the user. The user's ones (provided last in the * options.integrations list) win over ours. * * @param options SentryOptions to retrieve integrations from @@ -178,14 +187,14 @@ private static void deduplicateIntegrations( } if (fragmentIntegrations.size() > 1) { - for (int i = 1; i < fragmentIntegrations.size(); i++) { + for (int i = 0; i < fragmentIntegrations.size() - 1; i++) { final Integration integration = fragmentIntegrations.get(i); options.getIntegrations().remove(integration); } } if (timberIntegrations.size() > 1) { - for (int i = 1; i < timberIntegrations.size(); i++) { + for (int i = 0; i < timberIntegrations.size() - 1; i++) { final Integration integration = timberIntegrations.get(i); options.getIntegrations().remove(integration); } 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 da870ab32a..7f48dcd7ef 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 @@ -22,6 +22,9 @@ 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.spy +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.annotation.Config import java.io.File @@ -68,10 +71,26 @@ class AndroidOptionsInitializerTest { sentryOptions, if (useRealContext) context else mockContext ) + + val loadClass = LoadClass() + val activityFramesTracker = ActivityFramesTracker(loadClass, sentryOptions) + + AndroidOptionsInitializer.installDefaultIntegrations( + if (useRealContext) context else mockContext, + sentryOptions, + BuildInfoProvider(AndroidLogger()), + loadClass, + activityFramesTracker, + false, + false + ) + sentryOptions.configureOptions() AndroidOptionsInitializer.initializeIntegrationsAndProcessors( sentryOptions, - if (useRealContext) context else mockContext + if (useRealContext) context else mockContext, + loadClass, + activityFramesTracker ) } @@ -89,6 +108,8 @@ class AndroidOptionsInitializerTest { ) sentryOptions.isDebug = true val buildInfo = createBuildInfo(minApi) + val loadClass = createClassMock(classesToLoad) + val activityFramesTracker = ActivityFramesTracker(loadClass, sentryOptions) AndroidOptionsInitializer.loadDefaultAndMetadataOptions( sentryOptions, @@ -96,14 +117,24 @@ class AndroidOptionsInitializerTest { logger, buildInfo ) - AndroidOptionsInitializer.initializeIntegrationsAndProcessors( - sentryOptions, + + AndroidOptionsInitializer.installDefaultIntegrations( context, + sentryOptions, buildInfo, - createClassMock(classesToLoad), + loadClass, + activityFramesTracker, isFragmentAvailable, isTimberAvailable ) + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + sentryOptions, + context, + buildInfo, + loadClass, + activityFramesTracker + ) } private fun createBuildInfo(minApi: Int = 16): BuildInfoProvider { @@ -571,6 +602,22 @@ class AndroidOptionsInitializerTest { assertFalse(fixture.sentryOptions.scopeObservers.any { it is PersistingScopeObserver }) } + @Test + fun `installDefaultIntegrations does not evaluate cacheDir or outboxPath when called`() { + val mockOptions = spy(fixture.sentryOptions) + AndroidOptionsInitializer.installDefaultIntegrations( + fixture.context, + mockOptions, + mock(), + mock(), + mock(), + false, + false + ) + verify(mockOptions, never()).outboxPath + verify(mockOptions, never()).cacheDirPath + } + @Config(sdk = [30]) @Test fun `AnrV2Integration added to integrations list for API 30 and above`() { 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 6486315807..6710a418de 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 @@ -109,20 +109,32 @@ class AndroidTransactionProfilerTest { fun `set up`() { context = ApplicationProvider.getApplicationContext() val buildInfoProvider = BuildInfoProvider(fixture.mockLogger) + val loadClass = LoadClass() + val activityFramesTracker = ActivityFramesTracker(loadClass, fixture.options) AndroidOptionsInitializer.loadDefaultAndMetadataOptions( fixture.options, context, fixture.mockLogger, buildInfoProvider ) - AndroidOptionsInitializer.initializeIntegrationsAndProcessors( - fixture.options, + + AndroidOptionsInitializer.installDefaultIntegrations( context, + fixture.options, buildInfoProvider, - LoadClass(), + loadClass, + activityFramesTracker, false, false ) + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + fixture.options, + context, + buildInfoProvider, + loadClass, + activityFramesTracker + ) // 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/SendCachedEnvelopeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt index 0bf3b271fa..36094f78eb 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -5,6 +5,7 @@ import io.sentry.ILogger import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory import io.sentry.SentryLevel.DEBUG +import io.sentry.util.LazyEvaluator import org.awaitility.kotlin.await import org.mockito.kotlin.any import org.mockito.kotlin.eq @@ -53,7 +54,7 @@ class SendCachedEnvelopeIntegrationTest { } ) - return SendCachedEnvelopeIntegration(factory, hasStartupCrashMarker) + return SendCachedEnvelopeIntegration(factory, LazyEvaluator { hasStartupCrashMarker }) } } 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 876bb3bd78..b441c7789a 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 @@ -1,6 +1,7 @@ package io.sentry.android.core import android.app.ActivityManager +import android.app.Application import android.app.ApplicationExitInfo import android.content.Context import android.os.Bundle @@ -17,6 +18,8 @@ import io.sentry.SentryLevel.FATAL import io.sentry.SentryOptions import io.sentry.SentryOptions.BeforeSendCallback import io.sentry.Session +import io.sentry.ShutdownHookIntegration +import io.sentry.UncaughtExceptionHandlerIntegration import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration @@ -388,6 +391,36 @@ class SentryAndroidTest { ) } + @Test + fun `init can remove all integrations`() { + lateinit var optionsRef: SentryOptions + fixture.initSut(context = mock()) { options -> + optionsRef = options + options.dsn = "https://key@sentry.io/123" + assertEquals(18, options.integrations.size) + options.integrations.removeAll { + it is UncaughtExceptionHandlerIntegration || + it is ShutdownHookIntegration || + it is SendCachedEnvelopeIntegration || + it is NdkIntegration || + it is EnvelopeFileObserverIntegration || + it is AppLifecycleIntegration || + it is AnrIntegration || + it is ActivityLifecycleIntegration || + it is CurrentActivityIntegration || + it is UserInteractionIntegration || + it is FragmentLifecycleIntegration || + it is SentryTimberIntegration || + it is AppComponentsBreadcrumbsIntegration || + it is SystemEventsBreadcrumbsIntegration || + it is NetworkBreadcrumbsIntegration || + it is TempSensorBreadcrumbsIntegration || + it is PhoneStateBreadcrumbsIntegration + } + } + assertEquals(0, optionsRef.integrations.size) + } + private fun prefillScopeCache(cacheDir: String) { val scopeDir = File(cacheDir, SCOPE_CACHE).also { it.mkdirs() } File(scopeDir, BREADCRUMBS_FILENAME).writeText( 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 990d83b142..65086b8aa0 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 @@ -133,7 +133,20 @@ class SentryInitProviderTest { metaData.putBoolean(ManifestMetadataReader.NDK_ENABLE, false) AndroidOptionsInitializer.loadDefaultAndMetadataOptions(sentryOptions, mockContext) - AndroidOptionsInitializer.initializeIntegrationsAndProcessors(sentryOptions, mockContext) + + val loadClass = LoadClass() + val activityFramesTracker = ActivityFramesTracker(loadClass, sentryOptions) + AndroidOptionsInitializer.installDefaultIntegrations( + mockContext, + sentryOptions, + BuildInfoProvider(AndroidLogger()), + loadClass, + activityFramesTracker, + false, + false + ) + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors(sentryOptions, mockContext, loadClass, activityFramesTracker) assertFalse(sentryOptions.isEnableNdk) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 3ec27fa85b..f636d6775b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4151,6 +4151,15 @@ public final class io/sentry/util/JsonSerializationUtils { public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map; } +public final class io/sentry/util/LazyEvaluator { + public fun (Lio/sentry/util/LazyEvaluator$Evaluator;)V + public fun getValue ()Ljava/lang/Object; +} + +public abstract interface class io/sentry/util/LazyEvaluator$Evaluator { + public abstract fun evaluate ()Ljava/lang/Object; +} + public final class io/sentry/util/LogUtils { public fun ()V public static fun logNotInstanceOf (Ljava/lang/Class;Ljava/lang/Object;Lio/sentry/ILogger;)V diff --git a/sentry/src/main/java/io/sentry/util/LazyEvaluator.java b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java new file mode 100644 index 0000000000..5db376e710 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/LazyEvaluator.java @@ -0,0 +1,42 @@ +package io.sentry.util; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Class that evaluates a function lazily. It means the evaluator function is called only when + * getValue is called, and it's cached. + */ +@ApiStatus.Internal +public final class LazyEvaluator { + private @Nullable T value = null; + private final @NotNull Evaluator evaluator; + + /** + * Class that evaluates a function lazily. It means the evaluator function is called only when + * getValue is called, and it's cached. + * + * @param evaluator The function to evaluate. + */ + public LazyEvaluator(final @NotNull Evaluator evaluator) { + this.evaluator = evaluator; + } + + /** + * Executes the evaluator function and caches its result, so that it's called only once. + * + * @return The result of the evaluator function. + */ + public synchronized @NotNull T getValue() { + if (value == null) { + value = evaluator.evaluate(); + } + return value; + } + + public interface Evaluator { + @NotNull + T evaluate(); + } +} diff --git a/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt b/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt new file mode 100644 index 0000000000..8f0e3bc0a7 --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/LazyEvaluatorTest.kt @@ -0,0 +1,41 @@ +package io.sentry.util + +import kotlin.test.Test +import kotlin.test.assertEquals + +class LazyEvaluatorTest { + + class Fixture { + var count = 0 + + fun getSut(): LazyEvaluator { + count = 0 + return LazyEvaluator { ++count } + } + } + + private val fixture = Fixture() + + @Test + fun `does not evaluate on instantiation`() { + fixture.getSut() + assertEquals(0, fixture.count) + } + + @Test + fun `evaluator is called on getValue`() { + val evaluator = fixture.getSut() + assertEquals(0, fixture.count) + assertEquals(1, evaluator.value) + assertEquals(1, fixture.count) + } + + @Test + fun `evaluates only once`() { + val evaluator = fixture.getSut() + assertEquals(0, fixture.count) + assertEquals(1, evaluator.value) + assertEquals(1, evaluator.value) + assertEquals(1, fixture.count) + } +}