Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow removing integrations in SentryAndroid.init #2826

Merged
merged 5 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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))

## 6.25.0
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(
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
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