Skip to content

Commit

Permalink
Allow removing integrations in SentryAndroid.init (#2826)
Browse files Browse the repository at this point in the history
* configure options now happens after adding integrations in SentryAndroid.init
* added LazyEvaluator to evaluate a function lazily
* AndroidOptionsInitializer.installDefaultIntegrations now evaluate cache dir lazily
  • Loading branch information
stefanosiano authored Jul 11, 2023
1 parent bb6705c commit 2186444
Show file tree
Hide file tree
Showing 12 changed files with 245 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

### Dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,41 +101,30 @@ 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(
final @NotNull SentryAndroidOptions options,
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));
Expand Down Expand Up @@ -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,
Expand All @@ -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<Boolean> 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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -16,13 +17,13 @@ final class SendCachedEnvelopeIntegration implements Integration {

private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory
factory;
private final boolean hasStartupCrashMarker;
private final @NotNull LazyEvaluator<Boolean> startupCrashMarkerEvaluator;

public SendCachedEnvelopeIntegration(
final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory,
final boolean hasStartupCrashMarker) {
final @NotNull LazyEvaluator<Boolean> startupCrashMarkerEvaluator) {
this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required");
this.hasStartupCrashMarker = hasStartupCrashMarker;
this.startupCrashMarkerEvaluator = startupCrashMarkerEvaluator;
}

@Override
Expand Down Expand Up @@ -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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
}

Expand All @@ -89,21 +108,33 @@ class AndroidOptionsInitializerTest {
)
sentryOptions.isDebug = true
val buildInfo = createBuildInfo(minApi)
val loadClass = createClassMock(classesToLoad)
val activityFramesTracker = ActivityFramesTracker(loadClass, sentryOptions)

AndroidOptionsInitializer.loadDefaultAndMetadataOptions(
sentryOptions,
context,
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 {
Expand Down Expand Up @@ -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`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,7 +54,7 @@ class SendCachedEnvelopeIntegrationTest {
}
)

return SendCachedEnvelopeIntegration(factory, hasStartupCrashMarker)
return SendCachedEnvelopeIntegration(factory, LazyEvaluator { hasStartupCrashMarker })
}
}

Expand Down
Loading

0 comments on commit 2186444

Please sign in to comment.