From 557923d4ce3613a332ed3d28db50369e60bf30b9 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 5 Oct 2022 17:38:21 +0200 Subject: [PATCH 01/13] Block init to flush Startup Crashes --- .../sentry/samples/android/MyApplication.java | 1 + ...achedEnvelopeFireAndForgetIntegration.java | 28 +++++++- .../SendFireAndForgetEnvelopeSender.java | 4 ++ .../sentry/SendFireAndForgetOutboxSender.java | 4 ++ .../main/java/io/sentry/SentryOptions.java | 67 +++++++++++++++++++ .../java/io/sentry/cache/EnvelopeCache.java | 37 ++++++++++ 6 files changed, 140 insertions(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index a4a1c5397a..4284850abe 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -24,6 +24,7 @@ public void onCreate() { // }); // */ // }); + //throw new RuntimeException("Startup Crash!"); } private void strictMode() { diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 8cd1fa350e..8cb09479a0 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -1,7 +1,11 @@ package io.sentry; +import io.sentry.cache.EnvelopeCache; import io.sentry.util.Objects; import java.io.File; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -23,6 +27,9 @@ public interface SendFireAndForgetFactory { @Nullable SendFireAndForget create(@NotNull IHub hub, @NotNull SentryOptions options); + @Nullable + String getDirPath(); + default boolean hasValidPath(final @Nullable String dirPath, final @NotNull ILogger logger) { if (dirPath == null) { logger.log(SentryLevel.INFO, "No cached dir path is defined in options."); @@ -71,7 +78,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions } try { - options + Future future = options .getExecutorService() .submit( () -> { @@ -84,6 +91,25 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions } }); + final String dirPath = factory.getDirPath(); + if (dirPath != null && EnvelopeCache.hasStartupCrashMarker(dirPath, options)) { + options + .getLogger() + .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); + try { + future.get(options.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + // TODO: handle more exceptions + options + .getLogger() + .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + } + } else { + options + .getLogger() + .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); + } + options .getLogger() .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java index ecf4cb7913..0724ed4e65 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java @@ -37,4 +37,8 @@ public SendFireAndForgetEnvelopeSender( return processDir(envelopeSender, dirPath, options.getLogger()); } + + @Override public @Nullable String getDirPath() { + return sendFireAndForgetDirPath.getDirPath(); + } } diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java index 0666b37cda..61053a68c7 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java @@ -41,4 +41,8 @@ public SendFireAndForgetOutboxSender( return processDir(outboxSender, dirPath, options.getLogger()); } + + @Override public @Nullable String getDirPath() { + return sendFireAndForgetDirPath.getDirPath(); + } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index e7db4c41fa..932f467358 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -354,6 +354,31 @@ public class SentryOptions { /** ClientReportRecorder to track count of lost events / transactions / ... * */ @NotNull IClientReportRecorder clientReportRecorder = new ClientReportRecorder(this); + /** + * Controls how many seconds to wait for sending events in case there were Startup Crashes in the + * previous run. Sentry SDKs normally send events from a background queue, but in the case of + * Startup Crashes, it blocks the execution of the {@link Sentry#init()} function for the amount + * of startupCrashFlushTimeoutMillis to make sure the events make it to Sentry. + * + * When the timeout is reached, the execution will continue on background. + * + * Default is 5000 = 5s. + */ + private long startupCrashFlushTimeoutMillis = 5000; // 5s + + /** + * Controls the threshold after the application startup time, within which a crash should happen + * to be considered a Startup Crash. + * + * Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by + * {@link SentryOptions#startupCrashFlushTimeoutMillis}. + * + * Default is 2000 = 2s. + */ + private long startupCrashDurationThresholdMillis = 2000; // 2s + + private long sdkInitTime = 0; + /** * Adds an event processor * @@ -1742,6 +1767,47 @@ public void setSendClientReports(boolean sendClientReports) { return clientReportRecorder; } + /** + * Returns the Startup Crash flush timeout in Millis + * + * @return the timeout in Millis + */ + public long getStartupCrashFlushTimeoutMillis() { + return startupCrashFlushTimeoutMillis; + } + + /** + * Sets the Startup Crash flush timeout in Millis + * + * @param startupCrashFlushTimeoutMillis the timeout in Millis + */ + public void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { + this.startupCrashFlushTimeoutMillis = startupCrashFlushTimeoutMillis; + } + + /** + * Returns the Startup Crash duration threshold in Millis + * + * @return the threshold in Millis + */ + public long getStartupCrashDurationThresholdMillis() { + return startupCrashDurationThresholdMillis; + } + + /** + * Sets the Startup Crash duration threshold in Millis + * + * @param startupCrashDurationThresholdMillis the threshold in Millis + */ + public void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { + this.startupCrashDurationThresholdMillis = startupCrashDurationThresholdMillis; + } + + @ApiStatus.Internal + public long getSdkInitTime() { + return sdkInitTime; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1821,6 +1887,7 @@ public SentryOptions() { */ private SentryOptions(final boolean empty) { if (!empty) { + sdkInitTime = System.currentTimeMillis(); // SentryExecutorService should be initialized before any // SendCachedEventFireAndForgetIntegration executorService = new SentryExecutorService(); diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index e700e07d02..108869a12d 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -58,6 +58,8 @@ public final class EnvelopeCache extends CacheStrategy implements IEnvelopeCache public static final String CRASH_MARKER_FILE = "last_crash"; public static final String NATIVE_CRASH_MARKER_FILE = ".sentry-native/" + CRASH_MARKER_FILE; + public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; + private final @NotNull Map fileNameMap = new WeakHashMap<>(); public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) { @@ -202,7 +204,42 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi // write file to the disk when its about to crash so crashedLastRun can be marked on restart if (HintUtils.hasType(hint, DiskFlushNotification.class)) { writeCrashMarkerFile(); + + long timeSinceSdkInit = System.currentTimeMillis() - options.getSdkInitTime(); + if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { + options + .getLogger() + .log(DEBUG, + "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", + timeSinceSdkInit); + writeStartupCrashMarkerFile(); + } + } + } + + private void writeStartupCrashMarkerFile() { + final File crashMarkerFile = new File(options.getCacheDirPath(), STARTUP_CRASH_MARKER_FILE); + try { + crashMarkerFile.createNewFile(); + } catch (Throwable e) { + options.getLogger().log(ERROR, "Error writing the startup crash marker file to the disk", e); + } + } + + public static boolean hasStartupCrashMarker(final @NotNull String dirPath, final @NotNull SentryOptions options) { + final File crashMarkerFile = new File(dirPath, STARTUP_CRASH_MARKER_FILE); + final boolean exists = crashMarkerFile.exists(); + if (exists) { + if (!crashMarkerFile.delete()) { + options + .getLogger() + .log( + ERROR, + "Failed to delete the startup crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); + } } + return exists; } private void writeCrashMarkerFile() { From 7301d2ac5cfeca33e436ddfb5604679c77f511b2 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 11 Oct 2022 16:14:21 +0200 Subject: [PATCH 02/13] Make startup crashes only implemented on Android --- .../core/AndroidOptionsInitializer.java | 7 +- .../io/sentry/android/core/AppStartState.java | 5 ++ .../android/core/ManifestMetadataReader.java | 18 +++++ .../core/SendCachedEnvelopeIntegration.java | 80 +++++++++++++++++++ .../android/core/SentryAndroidOptions.java | 59 ++++++++++++++ .../core/cache/AndroidEnvelopeCache.java | 80 +++++++++++++++++++ ...achedEnvelopeFireAndForgetIntegration.java | 24 +----- sentry/src/main/java/io/sentry/Sentry.java | 10 ++- .../main/java/io/sentry/SentryOptions.java | 67 ---------------- .../java/io/sentry/cache/EnvelopeCache.java | 44 +--------- 10 files changed, 260 insertions(+), 134 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java 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 6eb31de464..9a313653de 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 @@ -8,10 +8,10 @@ import android.content.res.AssetManager; import android.os.Build; import io.sentry.ILogger; -import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; import io.sentry.SentryLevel; +import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; import io.sentry.util.Objects; @@ -132,6 +132,7 @@ static void init( ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider); initializeCacheDirs(context, options); + options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options.getLogger()); @@ -165,7 +166,7 @@ private static void installDefaultIntegrations( final boolean isTimberAvailable) { options.addIntegration( - new SendCachedEnvelopeFireAndForgetIntegration( + new SendCachedEnvelopeIntegration( new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()))); // Integrations are registered in the same order. NDK before adding Watch outbox, @@ -184,7 +185,7 @@ private static void installDefaultIntegrations( // this should be executed after NdkIntegration because sentry-native move files on init. // and we'd like to send them right away options.addIntegration( - new SendCachedEnvelopeFireAndForgetIntegration( + new SendCachedEnvelopeIntegration( new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()))); options.addIntegration(new AnrIntegration(context)); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 7ad96f9ea0..bce93e143f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -84,6 +84,11 @@ public Date getAppStartTime() { return appStartTime; } + @Nullable + public Long getAppStartMillis() { + return appStartMillis; + } + synchronized void setAppStartTime(final long appStartMillis, final @NotNull Date appStartTime) { // method is synchronized because the SDK may by init. on a background thread. if (this.appStartTime != null && this.appStartMillis != null) { 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 b22f4f3bf1..923d2b9b06 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 @@ -78,6 +78,10 @@ final class ManifestMetadataReader { static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports"; static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context"; + static final String STARTUP_CRASH_FLUSH_TIMEOUT = "io.sentry.startup-crash.flush-timeout"; + + static final String STARTUP_CRASH_DURATION_THRESHOLD = "io.sentry.startup-crash.duration-threshold"; + /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -226,6 +230,20 @@ static void applyMetadata( COLLECT_ADDITIONAL_CONTEXT, options.isCollectAdditionalContext())); + options.setStartupCrashDurationThresholdMillis( + readLong( + metadata, + logger, + STARTUP_CRASH_DURATION_THRESHOLD, + options.getStartupCrashDurationThresholdMillis())); + + options.setStartupCrashFlushTimeoutMillis( + readLong( + metadata, + logger, + STARTUP_CRASH_FLUSH_TIMEOUT, + options.getStartupCrashFlushTimeoutMillis())); + if (options.getTracesSampleRate() == null) { final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); if (tracesSampleRate != -1) { 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 new file mode 100644 index 0000000000..f01e7a3b9b --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -0,0 +1,80 @@ +package io.sentry.android.core; + +import io.sentry.IHub; +import io.sentry.Integration; +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import io.sentry.android.core.cache.AndroidEnvelopeCache; +import io.sentry.util.Objects; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.jetbrains.annotations.NotNull; + +final class SendCachedEnvelopeIntegration implements Integration { + + private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory; + + public SendCachedEnvelopeIntegration( + final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory) { + this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); + } + + @Override public void register(@NotNull IHub hub, @NotNull SentryOptions options) { + Objects.requireNonNull(hub, "Hub is required"); + final SentryAndroidOptions androidOptions = + Objects.requireNonNull( + (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, + "SentryAndroidOptions is required"); + + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget + sender = factory.create(hub, androidOptions); + + if (sender == null) { + androidOptions.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); + return; + } + + try { + Future future = androidOptions + .getExecutorService() + .submit( + () -> { + try { + sender.send(); + } catch (Throwable e) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); + } + }); + + final String dirPath = factory.getDirPath(); + if (dirPath != null && AndroidEnvelopeCache.hasStartupCrashMarker(dirPath, androidOptions)) { + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); + try { + future.get(androidOptions.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + } + } else { + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); + } + + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); + } catch (Throwable e) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "Failed to call the executor. Cached events will not be sent", e); + } + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 32eb2e01ed..854d742f3b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -107,6 +107,29 @@ public final class SentryAndroidOptions extends SentryOptions { */ private boolean collectAdditionalContext = true; + /** + * Controls how many seconds to wait for sending events in case there were Startup Crashes in the + * previous run. Sentry SDKs normally send events from a background queue, but in the case of + * Startup Crashes, it blocks the execution of the {@link Sentry#init()} function for the amount + * of startupCrashFlushTimeoutMillis to make sure the events make it to Sentry. + * + * When the timeout is reached, the execution will continue on background. + * + * Default is 5000 = 5s. + */ + private long startupCrashFlushTimeoutMillis = 5000; // 5s + + /** + * Controls the threshold after the application startup time, within which a crash should happen + * to be considered a Startup Crash. + * + * Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by + * {@link SentryAndroidOptions#startupCrashFlushTimeoutMillis}. + * + * Default is 2000 = 2s. + */ + private long startupCrashDurationThresholdMillis = 2000; // 2s + public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); @@ -332,4 +355,40 @@ public boolean isCollectAdditionalContext() { public void setCollectAdditionalContext(boolean collectAdditionalContext) { this.collectAdditionalContext = collectAdditionalContext; } + + /** + * Returns the Startup Crash flush timeout in Millis + * + * @return the timeout in Millis + */ + public long getStartupCrashFlushTimeoutMillis() { + return startupCrashFlushTimeoutMillis; + } + + /** + * Sets the Startup Crash flush timeout in Millis + * + * @param startupCrashFlushTimeoutMillis the timeout in Millis + */ + public void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { + this.startupCrashFlushTimeoutMillis = startupCrashFlushTimeoutMillis; + } + + /** + * Returns the Startup Crash duration threshold in Millis + * + * @return the threshold in Millis + */ + public long getStartupCrashDurationThresholdMillis() { + return startupCrashDurationThresholdMillis; + } + + /** + * Sets the Startup Crash duration threshold in Millis + * + * @param startupCrashDurationThresholdMillis the threshold in Millis + */ + public void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { + this.startupCrashDurationThresholdMillis = startupCrashDurationThresholdMillis; + } } 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 new file mode 100644 index 0000000000..6dc94f1f78 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -0,0 +1,80 @@ +package io.sentry.android.core.cache; + +import io.sentry.Hint; +import io.sentry.SentryEnvelope; +import io.sentry.SentryOptions; +import io.sentry.android.core.AppStartState; +import io.sentry.android.core.SentryAndroidOptions; +import io.sentry.cache.EnvelopeCache; +import io.sentry.hints.DiskFlushNotification; +import io.sentry.util.HintUtils; +import io.sentry.util.Objects; +import java.io.File; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +import static io.sentry.SentryLevel.DEBUG; +import static io.sentry.SentryLevel.ERROR; + +@ApiStatus.Internal +public final class AndroidEnvelopeCache extends EnvelopeCache { + + public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; + + private final @NotNull SentryAndroidOptions options; + + public AndroidEnvelopeCache( + final @NotNull SentryAndroidOptions options + ) { + super(options, Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), options.getMaxCacheItems()); + this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required."); + } + + @Override public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { + super.store(envelope, hint); + + final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); + if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { + long timeSinceSdkInit = System.currentTimeMillis() - appStartTime; + if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { + options + .getLogger() + .log(DEBUG, + "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", + timeSinceSdkInit); + writeStartupCrashMarkerFile(); + } + } + } + + private void writeStartupCrashMarkerFile() { + final File crashMarkerFile = new File(options.getCacheDirPath(), STARTUP_CRASH_MARKER_FILE); + try { + crashMarkerFile.createNewFile(); + } catch (Throwable e) { + options.getLogger().log(ERROR, "Error writing the startup crash marker file to the disk", e); + } + } + + public static boolean hasStartupCrashMarker(final @NotNull String dirPath, + final @NotNull SentryOptions options) { + final File crashMarkerFile = new File(dirPath, STARTUP_CRASH_MARKER_FILE); + try { + final boolean exists = crashMarkerFile.exists(); + if (exists) { + if (!crashMarkerFile.delete()) { + options + .getLogger() + .log( + ERROR, + "Failed to delete the startup crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); + } + } + return exists; + } catch (Throwable e) { + options.getLogger().log(ERROR, "Error reading/deleting the startup crash marker file on the disk", e); + } + return false; + } +} diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 8cb09479a0..7a204ed0fa 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -64,7 +64,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions Objects.requireNonNull(hub, "Hub is required"); Objects.requireNonNull(options, "SentryOptions is required"); - final String cachedDir = options.getCacheDirPath(); + final String cachedDir = factory.getDirPath(); if (!factory.hasValidPath(cachedDir, options.getLogger())) { options.getLogger().log(SentryLevel.ERROR, "No cache dir path is defined in options."); return; @@ -78,7 +78,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions } try { - Future future = options + options .getExecutorService() .submit( () -> { @@ -90,26 +90,6 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); } }); - - final String dirPath = factory.getDirPath(); - if (dirPath != null && EnvelopeCache.hasStartupCrashMarker(dirPath, options)) { - options - .getLogger() - .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); - try { - future.get(options.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { - // TODO: handle more exceptions - options - .getLogger() - .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); - } - } else { - options - .getLogger() - .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); - } - options .getLogger() .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index cdb108a057..6fe6afe435 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -1,9 +1,11 @@ package io.sentry; import io.sentry.cache.EnvelopeCache; +import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.util.FileUtils; import java.io.File; import java.lang.reflect.InvocationTargetException; @@ -230,7 +232,13 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) if (cacheDirPath != null) { final File cacheDir = new File(cacheDirPath); cacheDir.mkdirs(); - options.setEnvelopeDiskCache(EnvelopeCache.create(options)); + final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); + // only overwrite the cache impl if it's not set by Android + if (envelopeCache instanceof NoOpEnvelopeCache) { + options.setEnvelopeDiskCache(EnvelopeCache.create(options)); + } + } else { + options.setEnvelopeDiskCache(NoOpEnvelopeCache.getInstance()); } final String profilingTracesDirPath = options.getProfilingTracesDirPath(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 932f467358..e7db4c41fa 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -354,31 +354,6 @@ public class SentryOptions { /** ClientReportRecorder to track count of lost events / transactions / ... * */ @NotNull IClientReportRecorder clientReportRecorder = new ClientReportRecorder(this); - /** - * Controls how many seconds to wait for sending events in case there were Startup Crashes in the - * previous run. Sentry SDKs normally send events from a background queue, but in the case of - * Startup Crashes, it blocks the execution of the {@link Sentry#init()} function for the amount - * of startupCrashFlushTimeoutMillis to make sure the events make it to Sentry. - * - * When the timeout is reached, the execution will continue on background. - * - * Default is 5000 = 5s. - */ - private long startupCrashFlushTimeoutMillis = 5000; // 5s - - /** - * Controls the threshold after the application startup time, within which a crash should happen - * to be considered a Startup Crash. - * - * Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by - * {@link SentryOptions#startupCrashFlushTimeoutMillis}. - * - * Default is 2000 = 2s. - */ - private long startupCrashDurationThresholdMillis = 2000; // 2s - - private long sdkInitTime = 0; - /** * Adds an event processor * @@ -1767,47 +1742,6 @@ public void setSendClientReports(boolean sendClientReports) { return clientReportRecorder; } - /** - * Returns the Startup Crash flush timeout in Millis - * - * @return the timeout in Millis - */ - public long getStartupCrashFlushTimeoutMillis() { - return startupCrashFlushTimeoutMillis; - } - - /** - * Sets the Startup Crash flush timeout in Millis - * - * @param startupCrashFlushTimeoutMillis the timeout in Millis - */ - public void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { - this.startupCrashFlushTimeoutMillis = startupCrashFlushTimeoutMillis; - } - - /** - * Returns the Startup Crash duration threshold in Millis - * - * @return the threshold in Millis - */ - public long getStartupCrashDurationThresholdMillis() { - return startupCrashDurationThresholdMillis; - } - - /** - * Sets the Startup Crash duration threshold in Millis - * - * @param startupCrashDurationThresholdMillis the threshold in Millis - */ - public void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { - this.startupCrashDurationThresholdMillis = startupCrashDurationThresholdMillis; - } - - @ApiStatus.Internal - public long getSdkInitTime() { - return sdkInitTime; - } - /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1887,7 +1821,6 @@ public SentryOptions() { */ private SentryOptions(final boolean empty) { if (!empty) { - sdkInitTime = System.currentTimeMillis(); // SentryExecutorService should be initialized before any // SendCachedEventFireAndForgetIntegration executorService = new SentryExecutorService(); diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 108869a12d..7d823ee931 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -48,7 +48,7 @@ import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { +public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { /** File suffix added to all serialized envelopes files. */ public static final String SUFFIX_ENVELOPE_FILE = ".envelope"; @@ -57,23 +57,20 @@ public final class EnvelopeCache extends CacheStrategy implements IEnvelopeCache static final String SUFFIX_CURRENT_SESSION_FILE = ".json"; public static final String CRASH_MARKER_FILE = "last_crash"; public static final String NATIVE_CRASH_MARKER_FILE = ".sentry-native/" + CRASH_MARKER_FILE; - - public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; - private final @NotNull Map fileNameMap = new WeakHashMap<>(); public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) { final String cacheDirPath = options.getCacheDirPath(); final int maxCacheItems = options.getMaxCacheItems(); if (cacheDirPath == null) { - options.getLogger().log(WARNING, "maxCacheItems is null, returning NoOpEnvelopeCache"); + options.getLogger().log(WARNING, "cacheDirPath is null, returning NoOpEnvelopeCache"); return NoOpEnvelopeCache.getInstance(); } else { return new EnvelopeCache(options, cacheDirPath, maxCacheItems); } } - private EnvelopeCache( + public EnvelopeCache( final @NotNull SentryOptions options, final @NotNull String cacheDirPath, final int maxCacheItems) { @@ -204,42 +201,7 @@ public void store(final @NotNull SentryEnvelope envelope, final @NotNull Hint hi // write file to the disk when its about to crash so crashedLastRun can be marked on restart if (HintUtils.hasType(hint, DiskFlushNotification.class)) { writeCrashMarkerFile(); - - long timeSinceSdkInit = System.currentTimeMillis() - options.getSdkInitTime(); - if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { - options - .getLogger() - .log(DEBUG, - "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", - timeSinceSdkInit); - writeStartupCrashMarkerFile(); - } - } - } - - private void writeStartupCrashMarkerFile() { - final File crashMarkerFile = new File(options.getCacheDirPath(), STARTUP_CRASH_MARKER_FILE); - try { - crashMarkerFile.createNewFile(); - } catch (Throwable e) { - options.getLogger().log(ERROR, "Error writing the startup crash marker file to the disk", e); - } - } - - public static boolean hasStartupCrashMarker(final @NotNull String dirPath, final @NotNull SentryOptions options) { - final File crashMarkerFile = new File(dirPath, STARTUP_CRASH_MARKER_FILE); - final boolean exists = crashMarkerFile.exists(); - if (exists) { - if (!crashMarkerFile.delete()) { - options - .getLogger() - .log( - ERROR, - "Failed to delete the startup crash marker file. %s.", - crashMarkerFile.getAbsolutePath()); - } } - return exists; } private void writeCrashMarkerFile() { From 5ef3325b70e9388da5137e2a9ca5999c135e46f8 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 11 Oct 2022 14:18:48 +0000 Subject: [PATCH 03/13] Format code --- .../android/core/ManifestMetadataReader.java | 23 +++---- .../core/SendCachedEnvelopeIntegration.java | 63 ++++++++++--------- .../android/core/SentryAndroidOptions.java | 10 +-- .../core/cache/AndroidEnvelopeCache.java | 45 +++++++------ .../sentry/samples/android/MyApplication.java | 2 +- ...achedEnvelopeFireAndForgetIntegration.java | 4 -- .../SendFireAndForgetEnvelopeSender.java | 3 +- .../sentry/SendFireAndForgetOutboxSender.java | 3 +- 8 files changed, 80 insertions(+), 73 deletions(-) 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 923d2b9b06..59223a63e5 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 @@ -80,7 +80,8 @@ final class ManifestMetadataReader { static final String STARTUP_CRASH_FLUSH_TIMEOUT = "io.sentry.startup-crash.flush-timeout"; - static final String STARTUP_CRASH_DURATION_THRESHOLD = "io.sentry.startup-crash.duration-threshold"; + static final String STARTUP_CRASH_DURATION_THRESHOLD = + "io.sentry.startup-crash.duration-threshold"; /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -231,18 +232,18 @@ static void applyMetadata( options.isCollectAdditionalContext())); options.setStartupCrashDurationThresholdMillis( - readLong( - metadata, - logger, - STARTUP_CRASH_DURATION_THRESHOLD, - options.getStartupCrashDurationThresholdMillis())); + readLong( + metadata, + logger, + STARTUP_CRASH_DURATION_THRESHOLD, + options.getStartupCrashDurationThresholdMillis())); options.setStartupCrashFlushTimeoutMillis( - readLong( - metadata, - logger, - STARTUP_CRASH_FLUSH_TIMEOUT, - options.getStartupCrashFlushTimeoutMillis())); + readLong( + metadata, + logger, + STARTUP_CRASH_FLUSH_TIMEOUT, + options.getStartupCrashFlushTimeoutMillis())); if (options.getTracesSampleRate() == null) { final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); 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 f01e7a3b9b..855eccbe1a 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 @@ -14,22 +14,24 @@ final class SendCachedEnvelopeIntegration implements Integration { - private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory; + private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory + factory; public SendCachedEnvelopeIntegration( - final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory) { + final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory) { this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); } - @Override public void register(@NotNull IHub hub, @NotNull SentryOptions options) { + @Override + public void register(@NotNull IHub hub, @NotNull SentryOptions options) { Objects.requireNonNull(hub, "Hub is required"); final SentryAndroidOptions androidOptions = - Objects.requireNonNull( - (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, - "SentryAndroidOptions is required"); + Objects.requireNonNull( + (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, + "SentryAndroidOptions is required"); - final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget - sender = factory.create(hub, androidOptions); + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = + factory.create(hub, androidOptions); if (sender == null) { androidOptions.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); @@ -37,44 +39,45 @@ public SendCachedEnvelopeIntegration( } try { - Future future = androidOptions - .getExecutorService() - .submit( - () -> { - try { - sender.send(); - } catch (Throwable e) { - androidOptions - .getLogger() - .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); - } - }); + Future future = + androidOptions + .getExecutorService() + .submit( + () -> { + try { + sender.send(); + } catch (Throwable e) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); + } + }); final String dirPath = factory.getDirPath(); if (dirPath != null && AndroidEnvelopeCache.hasStartupCrashMarker(dirPath, androidOptions)) { androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); + .getLogger() + .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); try { future.get(androidOptions.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); } catch (TimeoutException e) { androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + .getLogger() + .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); } } else { androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); + .getLogger() + .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); } androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); + .getLogger() + .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); } catch (Throwable e) { androidOptions - .getLogger() - .log(SentryLevel.ERROR, "Failed to call the executor. Cached events will not be sent", e); + .getLogger() + .log(SentryLevel.ERROR, "Failed to call the executor. Cached events will not be sent", e); } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 854d742f3b..5cfad191cb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -113,9 +113,9 @@ public final class SentryAndroidOptions extends SentryOptions { * Startup Crashes, it blocks the execution of the {@link Sentry#init()} function for the amount * of startupCrashFlushTimeoutMillis to make sure the events make it to Sentry. * - * When the timeout is reached, the execution will continue on background. + *

When the timeout is reached, the execution will continue on background. * - * Default is 5000 = 5s. + *

Default is 5000 = 5s. */ private long startupCrashFlushTimeoutMillis = 5000; // 5s @@ -123,10 +123,10 @@ public final class SentryAndroidOptions extends SentryOptions { * Controls the threshold after the application startup time, within which a crash should happen * to be considered a Startup Crash. * - * Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by - * {@link SentryAndroidOptions#startupCrashFlushTimeoutMillis}. + *

Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by {@link + * SentryAndroidOptions#startupCrashFlushTimeoutMillis}. * - * Default is 2000 = 2s. + *

Default is 2000 = 2s. */ private long startupCrashDurationThresholdMillis = 2000; // 2s 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 6dc94f1f78..394187f949 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 @@ -1,5 +1,8 @@ package io.sentry.android.core.cache; +import static io.sentry.SentryLevel.DEBUG; +import static io.sentry.SentryLevel.ERROR; + import io.sentry.Hint; import io.sentry.SentryEnvelope; import io.sentry.SentryOptions; @@ -13,9 +16,6 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -import static io.sentry.SentryLevel.DEBUG; -import static io.sentry.SentryLevel.ERROR; - @ApiStatus.Internal public final class AndroidEnvelopeCache extends EnvelopeCache { @@ -23,14 +23,16 @@ public final class AndroidEnvelopeCache extends EnvelopeCache { private final @NotNull SentryAndroidOptions options; - public AndroidEnvelopeCache( - final @NotNull SentryAndroidOptions options - ) { - super(options, Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), options.getMaxCacheItems()); + public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) { + super( + options, + Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), + options.getMaxCacheItems()); this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required."); } - @Override public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { + @Override + public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { super.store(envelope, hint); final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); @@ -38,10 +40,11 @@ public AndroidEnvelopeCache( long timeSinceSdkInit = System.currentTimeMillis() - appStartTime; if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { options - .getLogger() - .log(DEBUG, - "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", - timeSinceSdkInit); + .getLogger() + .log( + DEBUG, + "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", + timeSinceSdkInit); writeStartupCrashMarkerFile(); } } @@ -56,24 +59,26 @@ private void writeStartupCrashMarkerFile() { } } - public static boolean hasStartupCrashMarker(final @NotNull String dirPath, - final @NotNull SentryOptions options) { + public static boolean hasStartupCrashMarker( + final @NotNull String dirPath, final @NotNull SentryOptions options) { final File crashMarkerFile = new File(dirPath, STARTUP_CRASH_MARKER_FILE); try { final boolean exists = crashMarkerFile.exists(); if (exists) { if (!crashMarkerFile.delete()) { options - .getLogger() - .log( - ERROR, - "Failed to delete the startup crash marker file. %s.", - crashMarkerFile.getAbsolutePath()); + .getLogger() + .log( + ERROR, + "Failed to delete the startup crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); } } return exists; } catch (Throwable e) { - options.getLogger().log(ERROR, "Error reading/deleting the startup crash marker file on the disk", e); + options + .getLogger() + .log(ERROR, "Error reading/deleting the startup crash marker file on the disk", e); } return false; } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index 4284850abe..921b0cc2b4 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -24,7 +24,7 @@ public void onCreate() { // }); // */ // }); - //throw new RuntimeException("Startup Crash!"); + // throw new RuntimeException("Startup Crash!"); } private void strictMode() { diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 7a204ed0fa..7dd5b1cd04 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -1,11 +1,7 @@ package io.sentry; -import io.sentry.cache.EnvelopeCache; import io.sentry.util.Objects; import java.io.File; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java index 0724ed4e65..dae5493955 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java @@ -38,7 +38,8 @@ public SendFireAndForgetEnvelopeSender( return processDir(envelopeSender, dirPath, options.getLogger()); } - @Override public @Nullable String getDirPath() { + @Override + public @Nullable String getDirPath() { return sendFireAndForgetDirPath.getDirPath(); } } diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java index 61053a68c7..ced3865d4e 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java @@ -42,7 +42,8 @@ public SendFireAndForgetOutboxSender( return processDir(outboxSender, dirPath, options.getLogger()); } - @Override public @Nullable String getDirPath() { + @Override + public @Nullable String getDirPath() { return sendFireAndForgetDirPath.getDirPath(); } } From cc8f21a71d2fc891548f0b127943371479b41eb7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 11 Oct 2022 18:10:38 +0200 Subject: [PATCH 04/13] Extract startup crash marker file read into a single operation --- .../core/AndroidOptionsInitializer.java | 8 ++++-- .../core/SendCachedEnvelopeIntegration.java | 17 +++++++---- .../core/cache/AndroidEnvelopeCache.java | 28 +++++++++++++------ .../sentry/samples/android/MyApplication.java | 1 - .../src/main/java/io/sentry/OutboxSender.java | 4 ++- ...achedEnvelopeFireAndForgetIntegration.java | 5 +--- .../SendFireAndForgetEnvelopeSender.java | 5 ---- .../sentry/SendFireAndForgetOutboxSender.java | 5 ---- .../java/io/sentry/cache/EnvelopeCache.java | 5 ++++ 9 files changed, 46 insertions(+), 32 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 9a313653de..030e9d7942 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 @@ -165,9 +165,13 @@ private static void installDefaultIntegrations( final boolean isFragmentAvailable, final boolean isTimberAvailable) { + // read the startup crash marker here to avoid doing double-IO for the SendCachedEnvelope + // integrations below + final boolean hasStartupCrashMarker = AndroidEnvelopeCache.hasStartupCrashMarker(options); + options.addIntegration( new SendCachedEnvelopeIntegration( - new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()))); + new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()), hasStartupCrashMarker)); // 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. @@ -186,7 +190,7 @@ private static void installDefaultIntegrations( // and we'd like to send them right away options.addIntegration( new SendCachedEnvelopeIntegration( - new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()))); + new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()), hasStartupCrashMarker)); options.addIntegration(new AnrIntegration(context)); options.addIntegration(new AppLifecycleIntegration()); 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 855eccbe1a..8bfee2103f 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,7 +5,6 @@ import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.util.Objects; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -16,10 +15,13 @@ final class SendCachedEnvelopeIntegration implements Integration { private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory; + private final boolean hasStartupCrashMarker; public SendCachedEnvelopeIntegration( - final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory) { + final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory, + final boolean hasStartupCrashMarker) { this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); + this.hasStartupCrashMarker = hasStartupCrashMarker; } @Override @@ -30,6 +32,12 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, "SentryAndroidOptions is required"); + final String cachedDir = options.getCacheDirPath(); + if (!factory.hasValidPath(cachedDir, options.getLogger())) { + options.getLogger().log(SentryLevel.ERROR, "No cache dir path is defined in options."); + return; + } + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = factory.create(hub, androidOptions); @@ -53,8 +61,7 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { } }); - final String dirPath = factory.getDirPath(); - if (dirPath != null && AndroidEnvelopeCache.hasStartupCrashMarker(dirPath, androidOptions)) { + if (hasStartupCrashMarker) { androidOptions .getLogger() .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); @@ -73,7 +80,7 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { androidOptions .getLogger() - .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); + .log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); } catch (Throwable e) { androidOptions .getLogger() 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 394187f949..c7d15d23d0 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 @@ -3,6 +3,7 @@ import static io.sentry.SentryLevel.DEBUG; import static io.sentry.SentryLevel.ERROR; +import android.os.SystemClock; import io.sentry.Hint; import io.sentry.SentryEnvelope; import io.sentry.SentryOptions; @@ -19,25 +20,22 @@ @ApiStatus.Internal public final class AndroidEnvelopeCache extends EnvelopeCache { - public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; - - private final @NotNull SentryAndroidOptions options; - public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) { super( options, Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), options.getMaxCacheItems()); - this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required."); } @Override public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { super.store(envelope, hint); + final SentryAndroidOptions options = (SentryAndroidOptions) this.options; + final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { - long timeSinceSdkInit = System.currentTimeMillis() - appStartTime; + long timeSinceSdkInit = SystemClock.uptimeMillis() - appStartTime; if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { options .getLogger() @@ -51,7 +49,14 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { } private void writeStartupCrashMarkerFile() { - final File crashMarkerFile = new File(options.getCacheDirPath(), STARTUP_CRASH_MARKER_FILE); + // 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 + final String outboxPath = options.getOutboxPath(); + if (outboxPath == null) { + options.getLogger().log(DEBUG, "Outbox path is null, the startup crash marker file will not be written"); + return; + } + final File crashMarkerFile = new File(options.getOutboxPath(), STARTUP_CRASH_MARKER_FILE); try { crashMarkerFile.createNewFile(); } catch (Throwable e) { @@ -60,8 +65,13 @@ private void writeStartupCrashMarkerFile() { } public static boolean hasStartupCrashMarker( - final @NotNull String dirPath, final @NotNull SentryOptions options) { - final File crashMarkerFile = new File(dirPath, STARTUP_CRASH_MARKER_FILE); + final @NotNull SentryOptions options) { final String outboxPath = options.getOutboxPath(); + if (outboxPath == null) { + options.getLogger().log(DEBUG, "Outbox path is null, the startup crash marker file does not exist"); + return false; + } + + final File crashMarkerFile = new File(options.getOutboxPath(), STARTUP_CRASH_MARKER_FILE); try { final boolean exists = crashMarkerFile.exists(); if (exists) { diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index 921b0cc2b4..a4a1c5397a 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -24,7 +24,6 @@ public void onCreate() { // }); // */ // }); - // throw new RuntimeException("Startup Crash!"); } private void strictMode() { diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 902e76bfb0..033cc0c538 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -3,6 +3,7 @@ import static io.sentry.SentryLevel.ERROR; import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE; +import io.sentry.cache.EnvelopeCache; import io.sentry.hints.Flushable; import io.sentry.hints.Resettable; import io.sentry.hints.Retryable; @@ -96,7 +97,8 @@ protected void processFile(final @NotNull File file, @NotNull Hint hint) { @Override protected boolean isRelevantFileName(final @Nullable String fileName) { // ignore current.envelope - return fileName != null && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE); + return fileName != null && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE) && !fileName.startsWith( + EnvelopeCache.STARTUP_CRASH_MARKER_FILE); // TODO: Use an extension to filter out relevant files } diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 7dd5b1cd04..1f38373e7d 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -23,9 +23,6 @@ public interface SendFireAndForgetFactory { @Nullable SendFireAndForget create(@NotNull IHub hub, @NotNull SentryOptions options); - @Nullable - String getDirPath(); - default boolean hasValidPath(final @Nullable String dirPath, final @NotNull ILogger logger) { if (dirPath == null) { logger.log(SentryLevel.INFO, "No cached dir path is defined in options."); @@ -60,7 +57,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions Objects.requireNonNull(hub, "Hub is required"); Objects.requireNonNull(options, "SentryOptions is required"); - final String cachedDir = factory.getDirPath(); + final String cachedDir = options.getCacheDirPath(); if (!factory.hasValidPath(cachedDir, options.getLogger())) { options.getLogger().log(SentryLevel.ERROR, "No cache dir path is defined in options."); return; diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java index dae5493955..ecf4cb7913 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetEnvelopeSender.java @@ -37,9 +37,4 @@ public SendFireAndForgetEnvelopeSender( return processDir(envelopeSender, dirPath, options.getLogger()); } - - @Override - public @Nullable String getDirPath() { - return sendFireAndForgetDirPath.getDirPath(); - } } diff --git a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java index ced3865d4e..0666b37cda 100644 --- a/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java +++ b/sentry/src/main/java/io/sentry/SendFireAndForgetOutboxSender.java @@ -41,9 +41,4 @@ public SendFireAndForgetOutboxSender( return processDir(outboxSender, dirPath, options.getLogger()); } - - @Override - public @Nullable String getDirPath() { - return sendFireAndForgetDirPath.getDirPath(); - } } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 7d823ee931..973a978fcc 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -6,6 +6,7 @@ import static io.sentry.SentryLevel.WARNING; import static java.lang.String.format; +import com.jakewharton.nopen.annotation.Open; import io.sentry.DateUtils; import io.sentry.Hint; import io.sentry.SentryCrashLastRunState; @@ -47,6 +48,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +@Open @ApiStatus.Internal public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { @@ -57,6 +59,9 @@ public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { static final String SUFFIX_CURRENT_SESSION_FILE = ".json"; public static final String CRASH_MARKER_FILE = "last_crash"; public static final String NATIVE_CRASH_MARKER_FILE = ".sentry-native/" + CRASH_MARKER_FILE; + + public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; + private final @NotNull Map fileNameMap = new WeakHashMap<>(); public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) { From 5abf2b00da82f195681a54cccdee3ad466f81009 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 11 Oct 2022 18:13:03 +0200 Subject: [PATCH 05/13] Remove line --- .../io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 1f38373e7d..8cd1fa350e 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -83,6 +83,7 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); } }); + options .getLogger() .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); From 705a1d654b24e41366a33f76cac41a229e1c0759 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 11 Oct 2022 16:14:11 +0000 Subject: [PATCH 06/13] Format code --- .../android/core/AndroidOptionsInitializer.java | 6 ++++-- .../android/core/SendCachedEnvelopeIntegration.java | 6 ++---- .../android/core/cache/AndroidEnvelopeCache.java | 12 ++++++++---- sentry/src/main/java/io/sentry/OutboxSender.java | 5 +++-- 4 files changed, 17 insertions(+), 12 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 030e9d7942..8a6c244a4b 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 @@ -171,7 +171,8 @@ private static void installDefaultIntegrations( options.addIntegration( new SendCachedEnvelopeIntegration( - new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()), hasStartupCrashMarker)); + new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()), + hasStartupCrashMarker)); // 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. @@ -190,7 +191,8 @@ private static void installDefaultIntegrations( // and we'd like to send them right away options.addIntegration( new SendCachedEnvelopeIntegration( - new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()), hasStartupCrashMarker)); + new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()), + hasStartupCrashMarker)); options.addIntegration(new AnrIntegration(context)); options.addIntegration(new AppLifecycleIntegration()); 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 8bfee2103f..3543af380a 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 @@ -19,7 +19,7 @@ final class SendCachedEnvelopeIntegration implements Integration { public SendCachedEnvelopeIntegration( final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory, - final boolean hasStartupCrashMarker) { + final boolean hasStartupCrashMarker) { this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); this.hasStartupCrashMarker = hasStartupCrashMarker; } @@ -78,9 +78,7 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); } - androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); + androidOptions.getLogger().log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); } catch (Throwable e) { androidOptions .getLogger() 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 c7d15d23d0..26119010e5 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 @@ -53,7 +53,9 @@ private void writeStartupCrashMarkerFile() { // decide to write it, which will trigger the blocking init final String outboxPath = options.getOutboxPath(); if (outboxPath == null) { - options.getLogger().log(DEBUG, "Outbox path is null, the startup crash marker file will not be written"); + options + .getLogger() + .log(DEBUG, "Outbox path is null, the startup crash marker file will not be written"); return; } final File crashMarkerFile = new File(options.getOutboxPath(), STARTUP_CRASH_MARKER_FILE); @@ -64,10 +66,12 @@ private void writeStartupCrashMarkerFile() { } } - public static boolean hasStartupCrashMarker( - final @NotNull SentryOptions options) { final String outboxPath = options.getOutboxPath(); + public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options) { + final String outboxPath = options.getOutboxPath(); if (outboxPath == null) { - options.getLogger().log(DEBUG, "Outbox path is null, the startup crash marker file does not exist"); + options + .getLogger() + .log(DEBUG, "Outbox path is null, the startup crash marker file does not exist"); return false; } diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 033cc0c538..db7efb5812 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -97,8 +97,9 @@ protected void processFile(final @NotNull File file, @NotNull Hint hint) { @Override protected boolean isRelevantFileName(final @Nullable String fileName) { // ignore current.envelope - return fileName != null && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE) && !fileName.startsWith( - EnvelopeCache.STARTUP_CRASH_MARKER_FILE); + return fileName != null + && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE) + && !fileName.startsWith(EnvelopeCache.STARTUP_CRASH_MARKER_FILE); // TODO: Use an extension to filter out relevant files } From 73361849ae2c07c44567a502169c1b6206b866ca Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 13 Oct 2022 13:34:33 +0200 Subject: [PATCH 07/13] Add tests --- .../io/sentry/android/core/AppStartState.java | 12 ++ .../core/SendCachedEnvelopeIntegration.java | 4 - .../io/sentry/android/core/SentryAndroid.java | 16 +++ .../android/core/SentryAndroidOptions.java | 7 +- .../core/cache/AndroidEnvelopeCache.java | 18 ++- .../util/AndroidCurrentDateProvider.java | 22 ++++ .../core/AndroidOptionsInitializerTest.kt | 12 +- .../core/SendCachedEnvelopeIntegrationTest.kt | 124 ++++++++++++++++++ .../sentry/android/core/SentryAndroidTest.kt | 37 ++++++ .../core/cache/AndroidEnvelopeCacheTest.kt | 116 ++++++++++++++++ .../sentry/samples/android/MyApplication.java | 1 + .../src/main/java/io/sentry/OutboxSender.java | 1 + sentry/src/main/java/io/sentry/Sentry.java | 4 +- .../main/java/io/sentry/SentryOptions.java | 5 +- .../test/java/io/sentry/OutboxSenderTest.kt | 5 + sentry/src/test/java/io/sentry/SentryTest.kt | 35 +++++ 16 files changed, 403 insertions(+), 16 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index bce93e143f..263f6e10d3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -97,4 +97,16 @@ synchronized void setAppStartTime(final long appStartMillis, final @NotNull Date this.appStartTime = appStartTime; this.appStartMillis = appStartMillis; } + + @TestOnly + public synchronized void setAppStartMillis(final long appStartMillis) { + this.appStartMillis = appStartMillis; + } + + @TestOnly + public synchronized void reset() { + appStartTime = null; + appStartMillis = null; + appStartEndMillis = null; + } } 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 3543af380a..61ec47618d 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 @@ -72,10 +72,6 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { .getLogger() .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); } - } else { - androidOptions - .getLogger() - .log(SentryLevel.DEBUG, "No Startup Crash marker exists, flushing asynchronously."); } androidOptions.getLogger().log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); 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 e3a83964f4..069a3a05b4 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,8 +9,10 @@ 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; @@ -104,6 +106,7 @@ public static synchronized void init( options, context, logger, isFragmentAvailable, isTimberAvailable); configuration.configure(options); deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); + resetEnvelopeCacheIfNeeded(options); }, true); } catch (IllegalAccessException e) { @@ -168,4 +171,17 @@ private static void deduplicateIntegrations( } } } + + /** + * 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/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 5cfad191cb..802730278f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -8,6 +8,7 @@ import io.sentry.protocol.SdkVersion; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; /** Sentry SDK options for Android */ public final class SentryAndroidOptions extends SentryOptions { @@ -370,7 +371,8 @@ public long getStartupCrashFlushTimeoutMillis() { * * @param startupCrashFlushTimeoutMillis the timeout in Millis */ - public void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { + @TestOnly + void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { this.startupCrashFlushTimeoutMillis = startupCrashFlushTimeoutMillis; } @@ -388,7 +390,8 @@ public long getStartupCrashDurationThresholdMillis() { * * @param startupCrashDurationThresholdMillis the threshold in Millis */ - public void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { + @TestOnly + void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { this.startupCrashDurationThresholdMillis = startupCrashDurationThresholdMillis; } } 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 26119010e5..fd13e7c034 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 @@ -9,8 +9,10 @@ import io.sentry.SentryOptions; import io.sentry.android.core.AppStartState; import io.sentry.android.core.SentryAndroidOptions; +import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; import io.sentry.cache.EnvelopeCache; import io.sentry.hints.DiskFlushNotification; +import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.HintUtils; import io.sentry.util.Objects; import java.io.File; @@ -20,11 +22,19 @@ @ApiStatus.Internal public final class AndroidEnvelopeCache extends EnvelopeCache { + private final @NotNull ICurrentDateProvider currentDateProvider; + public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) { + this(options, AndroidCurrentDateProvider.getInstance()); + } + + AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options, + final @NotNull ICurrentDateProvider currentDateProvider) { super( - options, - Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), - options.getMaxCacheItems()); + options, + Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), + options.getMaxCacheItems()); + this.currentDateProvider = currentDateProvider; } @Override @@ -35,7 +45,7 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { - long timeSinceSdkInit = SystemClock.uptimeMillis() - appStartTime; + long timeSinceSdkInit = currentDateProvider.getCurrentTimeMillis() - appStartTime; if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { options .getLogger() diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java new file mode 100644 index 0000000000..466c19ec82 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java @@ -0,0 +1,22 @@ +package io.sentry.android.core.internal.util; + +import android.os.SystemClock; +import io.sentry.transport.CurrentDateProvider; +import io.sentry.transport.ICurrentDateProvider; +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class AndroidCurrentDateProvider implements ICurrentDateProvider { + + private static final ICurrentDateProvider instance = new AndroidCurrentDateProvider(); + + public static ICurrentDateProvider getInstance() { + return instance; + } + + private AndroidCurrentDateProvider() {} + + @Override public long getCurrentTimeMillis() { + return SystemClock.uptimeMillis(); + } +} 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 3f1ff1a586..768898273c 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 @@ -11,6 +11,7 @@ import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SendCachedEnvelopeFireAndForgetIntegration import io.sentry.SentryOptions +import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration import org.junit.runner.RunWith @@ -288,12 +289,12 @@ class AndroidOptionsInitializerTest { } @Test - fun `SendCachedEnvelopeFireAndForgetIntegration added to integration list`() { + fun `SendCachedEnvelopeIntegration added to integration list`() { fixture.initSut() val actual = fixture.sentryOptions.integrations - .firstOrNull { it is SendCachedEnvelopeFireAndForgetIntegration } + .firstOrNull { it is SendCachedEnvelopeIntegration } assertNotNull(actual) } @@ -364,4 +365,11 @@ class AndroidOptionsInitializerTest { fixture.sentryOptions.integrations.firstOrNull { it is SentryTimberIntegration } assertNull(actual) } + + @Test + fun `AndroidEnvelopeCache is set to options`() { + fixture.initSut() + + assertTrue { fixture.sentryOptions.envelopeDiskCache is AndroidEnvelopeCache } + } } 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 new file mode 100644 index 0000000000..b6dd09cd28 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -0,0 +1,124 @@ +package io.sentry.android.core + +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.eq +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.ISentryExecutorService +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory +import io.sentry.SentryLevel.DEBUG +import io.sentry.SentryLevel.ERROR +import io.sentry.SentryOptions +import org.awaitility.kotlin.await +import java.util.concurrent.ExecutionException +import java.util.concurrent.TimeUnit.MILLISECONDS +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.test.Test + +class SendCachedEnvelopeIntegrationTest { + private class Fixture { + val hub: IHub = mock() + val options = SentryAndroidOptions() + val logger = mock() + val factory = mock() + val flag = AtomicBoolean(true) + val sender = mock() + + fun getSut( + cacheDirPath: String? = "abc", + hasStartupCrashMarker: Boolean = false, + hasSender: Boolean = true, + delaySend: Long = 0L, + taskFails: Boolean = false + ): SendCachedEnvelopeIntegration { + options.cacheDirPath = cacheDirPath + options.setLogger(logger) + options.isDebug = true + + whenever(sender.send()).then { + Thread.sleep(delaySend) + if (taskFails) { + throw ExecutionException(RuntimeException("Something went wrong")) + } + flag.set(false) + } + whenever(factory.hasValidPath(any(), any())).thenCallRealMethod() + whenever(factory.create(any(), any())).thenReturn( + if (hasSender) { + sender + } else { + null + } + ) + + return SendCachedEnvelopeIntegration(factory, hasStartupCrashMarker) + } + } + + private val fixture = Fixture() + + @Test + fun `when cacheDirPath is not set, does nothing`() { + val sut = fixture.getSut(cacheDirPath = null) + + sut.register(fixture.hub, fixture.options) + + verify(fixture.factory, never()).create(any(), any()) + } + + @Test + fun `when factory returns null, does nothing`() { + val sut = fixture.getSut(hasSender = false) + + sut.register(fixture.hub, fixture.options) + + verify(fixture.factory).create(any(), any()) + verify(fixture.sender, never()).send() + } + + @Test + fun `when has factory and cacheDirPath set, submits task into queue`() { + val sut = fixture.getSut() + + sut.register(fixture.hub, fixture.options) + + await.untilFalse(fixture.flag) + verify(fixture.sender).send() + } + + @Test + fun `when has startup crash marker, awaits the task on the calling thread`() { + val sut = fixture.getSut(hasStartupCrashMarker = true) + + sut.register(fixture.hub, fixture.options) + + // we do not need to await here, because it's executed synchronously + verify(fixture.sender).send() + } + + @Test + fun `when synchronous send times out, continues the task on a background thread`() { + val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 50) + fixture.options.startupCrashFlushTimeoutMillis = 10 + + sut.register(fixture.hub, fixture.options) + + // first wait until synchronous send times out and check that the logger was hit in the catch block + await.atLeast(11, MILLISECONDS) + verify(fixture.logger).log( + eq(DEBUG), + eq("Synchronous send timed out, continuing in the background.") + ) + + // then wait until the async send finishes in background + await.untilFalse(fixture.flag) + verify(fixture.sender).send() + } +} 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 3da78a530c..68178671cd 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 @@ -8,13 +8,17 @@ import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify +import io.sentry.Hint import io.sentry.ILogger import io.sentry.Sentry +import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.FATAL import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration +import io.sentry.cache.IEnvelopeCache +import io.sentry.transport.NoOpEnvelopeCache import org.junit.runner.RunWith import kotlin.test.BeforeTest import kotlin.test.Test @@ -131,4 +135,37 @@ class SentryAndroidTest { // but we just verify here that the single integration is preserved assertEquals(refOptions!!.integrations.filterIsInstance().size, 1) } + + @Test + fun `AndroidEnvelopeCache is reset if the user disabled caching via cacheDirPath`() { + var refOptions: SentryAndroidOptions? = null + + fixture.initSut { + it.cacheDirPath = null + + refOptions = it + } + + assertTrue { refOptions!!.envelopeDiskCache is NoOpEnvelopeCache } + } + + @Test + fun `envelopeCache remains unchanged if the user set their own IEnvelopCache impl`() { + var refOptions: SentryAndroidOptions? = null + + fixture.initSut { + it.cacheDirPath = null + it.setEnvelopeDiskCache(CustomEnvelopCache()) + + refOptions = it + } + + assertTrue { refOptions!!.envelopeDiskCache is CustomEnvelopCache } + } + + private class CustomEnvelopCache : IEnvelopeCache { + override fun iterator(): MutableIterator = TODO() + override fun store(envelope: SentryEnvelope, hint: Hint) = Unit + override fun discard(envelope: SentryEnvelope) = Unit + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt new file mode 100644 index 0000000000..27817c277e --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -0,0 +1,116 @@ +package io.sentry.android.core.cache + +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.ILogger +import io.sentry.SentryEnvelope +import io.sentry.SentryOptions +import io.sentry.android.core.AppStartState +import io.sentry.android.core.SentryAndroidOptions +import io.sentry.cache.EnvelopeCache +import io.sentry.hints.DiskFlushNotification +import io.sentry.transport.ICurrentDateProvider +import io.sentry.util.HintUtils +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class AndroidEnvelopeCacheTest { + private class Fixture { + private val dir: Path = Files.createTempDirectory("sentry-cache") + val envelope = mock { + whenever(it.header).thenReturn(mock()) + } + val options = SentryAndroidOptions() + val dateProvider = mock() + lateinit var markerFile: File + + fun getSut( + appStartMillis: Long? = null, + currentTimeMillis: Long? = null + ): AndroidEnvelopeCache { + options.cacheDirPath = dir.toAbsolutePath().toFile().absolutePath + val outboxDir = File(options.outboxPath!!) + outboxDir.mkdirs() + + markerFile = File(outboxDir, EnvelopeCache.STARTUP_CRASH_MARKER_FILE) + + options.setLogger(mock()) + + if (appStartMillis != null) { + AppStartState.getInstance().setAppStartMillis(appStartMillis) + } + if (currentTimeMillis != null) { + whenever(dateProvider.currentTimeMillis).thenReturn(currentTimeMillis) + } + + return AndroidEnvelopeCache(options, dateProvider) + } + } + + private val fixture = Fixture() + + @BeforeTest + fun `set up`() { + AppStartState.getInstance().reset() + } + + @Test + fun `when no flush hint exists, does not write startup crash file`() { + val cache = fixture.getSut() + + cache.store(fixture.envelope) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when startup time is null, does not write startup crash file`() { + val cache = fixture.getSut() + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when time since sdk init is more than duration threshold, does not write startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 5000L) + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when outbox dir is not set, does not write startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 2000L) + + fixture.options.cacheDirPath = null + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when time since sdk init is less than duration threshold, writes startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 2000L) + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertTrue(fixture.markerFile.exists()) + } + + internal class DiskFlushHint : DiskFlushNotification { + override fun markFlushed() {} + } +} diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index a4a1c5397a..577f8a40f7 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -24,6 +24,7 @@ public void onCreate() { // }); // */ // }); + throw new RuntimeException("Startup Crash!"); } private void strictMode() { diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index db7efb5812..d470d8dad6 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -2,6 +2,7 @@ import static io.sentry.SentryLevel.ERROR; import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE; +import static io.sentry.cache.EnvelopeCache.STARTUP_CRASH_MARKER_FILE; import io.sentry.cache.EnvelopeCache; import io.sentry.hints.Flushable; diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 6fe6afe435..223c540a77 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -233,12 +233,10 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) final File cacheDir = new File(cacheDirPath); cacheDir.mkdirs(); final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); - // only overwrite the cache impl if it's not set by Android + // only overwrite the cache impl if it's not already set if (envelopeCache instanceof NoOpEnvelopeCache) { options.setEnvelopeDiskCache(EnvelopeCache.create(options)); } - } else { - options.setEnvelopeDiskCache(NoOpEnvelopeCache.getInstance()); } final String profilingTracesDirPath = options.getProfilingTracesDirPath(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index e7db4c41fa..50090385be 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -28,6 +28,7 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** Sentry SDK options */ @Open @@ -1085,7 +1086,9 @@ public ISentryExecutorService getExecutorService() { * * @param executorService the SentryExecutorService */ - void setExecutorService(final @NotNull ISentryExecutorService executorService) { + @ApiStatus.Internal + @TestOnly + public void setExecutorService(final @NotNull ISentryExecutorService executorService) { if (executorService != null) { this.executorService = executorService; } diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 4b23fdea90..dc91a76c0b 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -315,6 +315,11 @@ class OutboxSenderTest { assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.PREFIX_CURRENT_SESSION_FILE)) } + @Test + fun `when file name is startup crash marker, should be ignored`() { + assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.STARTUP_CRASH_MARKER_FILE)) + } + @Test fun `when file name is relevant, should return true`() { assertTrue(fixture.getSut().isRelevantFileName("123.envelope")) diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 460131b0f3..11154cb9a6 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -4,6 +4,8 @@ import com.nhaarman.mockitokotlin2.argThat import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify +import io.sentry.cache.EnvelopeCache +import io.sentry.cache.IEnvelopeCache import io.sentry.protocol.SentryId import org.junit.rules.TemporaryFolder import java.io.File @@ -233,6 +235,39 @@ class SentryTest { assertFalse(hub is NoOpHub) } + @Test + fun `overrides envelope cache if it's not set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + it.cacheDirPath = getTempPath() + sentryOptions = it + } + + assertTrue { sentryOptions!!.envelopeDiskCache is EnvelopeCache } + } + + @Test + fun `does not override envelope cache if it's already set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + it.cacheDirPath = getTempPath() + it.setEnvelopeDiskCache(CustomEnvelopCache()) + sentryOptions = it + } + + assertTrue { sentryOptions!!.envelopeDiskCache is CustomEnvelopCache } + } + + private class CustomEnvelopCache : IEnvelopeCache { + override fun iterator(): MutableIterator = TODO() + override fun store(envelope: SentryEnvelope, hint: Hint) = Unit + override fun discard(envelope: SentryEnvelope) = Unit + } + private fun getTempPath(): String { val tempFile = Files.createTempDirectory("cache").toFile() tempFile.delete() From 26615c10938695ed91dc977986d3468e3a06b91c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 13 Oct 2022 13:34:50 +0200 Subject: [PATCH 08/13] spotless --- .../java/io/sentry/android/core/SentryAndroid.java | 3 ++- .../android/core/cache/AndroidEnvelopeCache.java | 12 ++++++------ .../internal/util/AndroidCurrentDateProvider.java | 4 ++-- .../android/core/AndroidOptionsInitializerTest.kt | 1 - .../core/SendCachedEnvelopeIntegrationTest.kt | 5 ----- .../android/core/cache/AndroidEnvelopeCacheTest.kt | 2 -- sentry/src/main/java/io/sentry/OutboxSender.java | 1 - 7 files changed, 10 insertions(+), 18 deletions(-) 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 069a3a05b4..9bcc107d21 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 @@ -180,7 +180,8 @@ private static void deduplicateIntegrations( * @param options SentryOptions to retrieve cacheDirPath from */ private static void resetEnvelopeCacheIfNeeded(final @NotNull SentryAndroidOptions options) { - if (options.getCacheDirPath() == null && options.getEnvelopeDiskCache() instanceof AndroidEnvelopeCache) { + 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 fd13e7c034..e08ef8b128 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 @@ -3,7 +3,6 @@ import static io.sentry.SentryLevel.DEBUG; import static io.sentry.SentryLevel.ERROR; -import android.os.SystemClock; import io.sentry.Hint; import io.sentry.SentryEnvelope; import io.sentry.SentryOptions; @@ -28,12 +27,13 @@ public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) { this(options, AndroidCurrentDateProvider.getInstance()); } - AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options, - final @NotNull ICurrentDateProvider currentDateProvider) { + AndroidEnvelopeCache( + final @NotNull SentryAndroidOptions options, + final @NotNull ICurrentDateProvider currentDateProvider) { super( - options, - Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), - options.getMaxCacheItems()); + options, + Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), + options.getMaxCacheItems()); this.currentDateProvider = currentDateProvider; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java index 466c19ec82..cdf66c319a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java @@ -1,7 +1,6 @@ package io.sentry.android.core.internal.util; import android.os.SystemClock; -import io.sentry.transport.CurrentDateProvider; import io.sentry.transport.ICurrentDateProvider; import org.jetbrains.annotations.ApiStatus; @@ -16,7 +15,8 @@ public static ICurrentDateProvider getInstance() { private AndroidCurrentDateProvider() {} - @Override public long getCurrentTimeMillis() { + @Override + public long getCurrentTimeMillis() { return SystemClock.uptimeMillis(); } } 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 768898273c..2f26c4317a 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 @@ -9,7 +9,6 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import io.sentry.ILogger import io.sentry.MainEventProcessor -import io.sentry.SendCachedEnvelopeFireAndForgetIntegration import io.sentry.SentryOptions import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration 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 b6dd09cd28..d0dedbf823 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,17 +5,12 @@ import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions import com.nhaarman.mockitokotlin2.whenever import io.sentry.IHub import io.sentry.ILogger -import io.sentry.ISentryExecutorService -import io.sentry.SendCachedEnvelopeFireAndForgetIntegration import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory import io.sentry.SentryLevel.DEBUG -import io.sentry.SentryLevel.ERROR -import io.sentry.SentryOptions import org.awaitility.kotlin.await import java.util.concurrent.ExecutionException import java.util.concurrent.TimeUnit.MILLISECONDS diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 27817c277e..6741cbc8c4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -2,9 +2,7 @@ package io.sentry.android.core.cache import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever -import io.sentry.ILogger import io.sentry.SentryEnvelope -import io.sentry.SentryOptions import io.sentry.android.core.AppStartState import io.sentry.android.core.SentryAndroidOptions import io.sentry.cache.EnvelopeCache diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index d470d8dad6..db7efb5812 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -2,7 +2,6 @@ import static io.sentry.SentryLevel.ERROR; import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE; -import static io.sentry.cache.EnvelopeCache.STARTUP_CRASH_MARKER_FILE; import io.sentry.cache.EnvelopeCache; import io.sentry.hints.Flushable; From bb817ac98a041d3674fe3c95877844c6c4a54e3c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 13 Oct 2022 13:36:08 +0200 Subject: [PATCH 09/13] api dump --- sentry-android-core/api/sentry-android-core.api | 11 +++++++++++ sentry/api/sentry.api | 5 ++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 0fac802055..1d906d1eeb 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -44,9 +44,12 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In public final class io/sentry/android/core/AppStartState { public fun getAppStartInterval ()Ljava/lang/Long; + public fun getAppStartMillis ()Ljava/lang/Long; public fun getAppStartTime ()Ljava/util/Date; public static fun getInstance ()Lio/sentry/android/core/AppStartState; public fun isColdStart ()Ljava/lang/Boolean; + public fun reset ()V + public fun setAppStartMillis (J)V } public final class io/sentry/android/core/BuildConfig { @@ -127,6 +130,8 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader; public fun getProfilingTracesHz ()I public fun getProfilingTracesIntervalMillis ()I + public fun getStartupCrashDurationThresholdMillis ()J + public fun getStartupCrashFlushTimeoutMillis ()J public fun isAnrEnabled ()Z public fun isAnrReportInDebug ()Z public fun isAttachScreenshot ()Z @@ -215,3 +220,9 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } +public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { + public fun (Lio/sentry/android/core/SentryAndroidOptions;)V + public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z + public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V +} + diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e3bac05f63..843aec7bb3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1376,6 +1376,7 @@ public class io/sentry/SentryOptions { public fun setEnvelopeDiskCache (Lio/sentry/cache/IEnvelopeCache;)V public fun setEnvelopeReader (Lio/sentry/IEnvelopeReader;)V public fun setEnvironment (Ljava/lang/String;)V + public fun setExecutorService (Lio/sentry/ISentryExecutorService;)V public fun setFlushTimeoutMillis (J)V public fun setHostnameVerifier (Ljavax/net/ssl/HostnameVerifier;)V public fun setIdleTimeout (Ljava/lang/Long;)V @@ -1884,12 +1885,14 @@ public final class io/sentry/UserFeedback$JsonKeys { public fun ()V } -public final class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { +public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { public static final field CRASH_MARKER_FILE Ljava/lang/String; public static final field NATIVE_CRASH_MARKER_FILE Ljava/lang/String; public static final field PREFIX_CURRENT_SESSION_FILE Ljava/lang/String; + public static final field STARTUP_CRASH_MARKER_FILE Ljava/lang/String; public static final field SUFFIX_ENVELOPE_FILE Ljava/lang/String; protected static final field UTF_8 Ljava/nio/charset/Charset; + public fun (Lio/sentry/SentryOptions;Ljava/lang/String;I)V public static fun create (Lio/sentry/SentryOptions;)Lio/sentry/cache/IEnvelopeCache; public fun discard (Lio/sentry/SentryEnvelope;)V public fun iterator ()Ljava/util/Iterator; From f79b06e976f7dc3ab8909de2dda924769b0ecb4c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 13 Oct 2022 13:39:16 +0200 Subject: [PATCH 10/13] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67a1a2cca8..dc939a895e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293)) +### Features + +- Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277)) + ## 6.5.0 ### Fixes From 340d087948219cf391c5329136388671e9ab7aa8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 13 Oct 2022 14:46:26 +0200 Subject: [PATCH 11/13] Add kill switch for Startup Crashes feature --- .../api/sentry-android-core.api | 3 +- .../android/core/ManifestMetadataReader.java | 21 +++---------- .../android/core/SentryAndroidOptions.java | 31 ++++++++++++------- .../core/cache/AndroidEnvelopeCache.java | 5 +++ .../core/ManifestMetadataReaderTest.kt | 25 +++++++++++++++ .../core/cache/AndroidEnvelopeCacheTest.kt | 20 +++++++++++- .../sentry/samples/android/MyApplication.java | 1 - 7 files changed, 75 insertions(+), 31 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 6361a02ccc..a794edf843 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -132,7 +132,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getProfilingTracesHz ()I public fun getProfilingTracesIntervalMillis ()I public fun getStartupCrashDurationThresholdMillis ()J - public fun getStartupCrashFlushTimeoutMillis ()J public fun isAnrEnabled ()Z public fun isAnrReportInDebug ()Z public fun isAttachScreenshot ()Z @@ -142,6 +141,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun isEnableAppComponentBreadcrumbs ()Z public fun isEnableAppLifecycleBreadcrumbs ()Z public fun isEnableAutoActivityLifecycleTracing ()Z + public fun isEnableStartupCrashDetection ()Z public fun isEnableSystemEventBreadcrumbs ()Z public fun isEnableUserInteractionBreadcrumbs ()Z public fun isEnableUserInteractionTracing ()Z @@ -156,6 +156,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableAppComponentBreadcrumbs (Z)V public fun setEnableAppLifecycleBreadcrumbs (Z)V public fun setEnableAutoActivityLifecycleTracing (Z)V + public fun setEnableStartupCrashDetection (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V public fun setEnableUserInteractionBreadcrumbs (Z)V public fun setEnableUserInteractionTracing (Z)V 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 59223a63e5..97ede8036b 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 @@ -78,10 +78,7 @@ final class ManifestMetadataReader { static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports"; static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context"; - static final String STARTUP_CRASH_FLUSH_TIMEOUT = "io.sentry.startup-crash.flush-timeout"; - - static final String STARTUP_CRASH_DURATION_THRESHOLD = - "io.sentry.startup-crash.duration-threshold"; + static final String STARTUP_CRASH_ENABLE = "io.sentry.startup-crashes.enable"; /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -231,19 +228,9 @@ static void applyMetadata( COLLECT_ADDITIONAL_CONTEXT, options.isCollectAdditionalContext())); - options.setStartupCrashDurationThresholdMillis( - readLong( - metadata, - logger, - STARTUP_CRASH_DURATION_THRESHOLD, - options.getStartupCrashDurationThresholdMillis())); - - options.setStartupCrashFlushTimeoutMillis( - readLong( - metadata, - logger, - STARTUP_CRASH_FLUSH_TIMEOUT, - options.getStartupCrashFlushTimeoutMillis())); + options.setEnableStartupCrashDetection( + readBool( + metadata, logger, STARTUP_CRASH_ENABLE, options.isEnableStartupCrashDetection())); if (options.getTracesSampleRate() == null) { final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 802730278f..3e1c604ed3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -124,12 +124,21 @@ public final class SentryAndroidOptions extends SentryOptions { * Controls the threshold after the application startup time, within which a crash should happen * to be considered a Startup Crash. * - *

Startup Crashes are sent on @{@link Sentry#init()} in a blocking way, controlled by {@link + *

Startup Crashes are sent on {@link Sentry#init()} in a blocking way, controlled by {@link * SentryAndroidOptions#startupCrashFlushTimeoutMillis}. * *

Default is 2000 = 2s. */ - private long startupCrashDurationThresholdMillis = 2000; // 2s + private final long startupCrashDurationThresholdMillis = 2000; // 2s + + /** + * Controls whether the SDK should detect Startup Crashes. + * + *

Startup Crashes are sent on {@link Sentry#init()} in a blocking way. + * + *

Default is true + */ + private boolean enableStartupCrashDetection = true; public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); @@ -362,7 +371,8 @@ public void setCollectAdditionalContext(boolean collectAdditionalContext) { * * @return the timeout in Millis */ - public long getStartupCrashFlushTimeoutMillis() { + @ApiStatus.Internal + long getStartupCrashFlushTimeoutMillis() { return startupCrashFlushTimeoutMillis; } @@ -381,17 +391,16 @@ void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { * * @return the threshold in Millis */ + @ApiStatus.Internal public long getStartupCrashDurationThresholdMillis() { return startupCrashDurationThresholdMillis; } - /** - * Sets the Startup Crash duration threshold in Millis - * - * @param startupCrashDurationThresholdMillis the threshold in Millis - */ - @TestOnly - void setStartupCrashDurationThresholdMillis(long startupCrashDurationThresholdMillis) { - this.startupCrashDurationThresholdMillis = startupCrashDurationThresholdMillis; + public boolean isEnableStartupCrashDetection() { + return enableStartupCrashDetection; + } + + public void setEnableStartupCrashDetection(boolean enableStartupCrashDetection) { + this.enableStartupCrashDetection = enableStartupCrashDetection; } } 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 e08ef8b128..9d4de084d4 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 @@ -43,6 +43,11 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { final SentryAndroidOptions options = (SentryAndroidOptions) this.options; + if (!options.isEnableStartupCrashDetection()) { + options.getLogger().log(DEBUG, "Startup Crash detection is disabled."); + return; + } + final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { long timeSinceSdkInit = currentDateProvider.getCurrentTimeMillis() - appStartTime; 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 3e4f0b32f4..c06e52cc00 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 @@ -1061,4 +1061,29 @@ class ManifestMetadataReaderTest { // Assert assertTrue(fixture.options.isCollectAdditionalContext) } + + @Test + fun `applyMetadata reads enableStartupCrashDetection to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.STARTUP_CRASH_ENABLE to false) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertFalse(fixture.options.isEnableStartupCrashDetection) + } + + @Test + fun `applyMetadata reads enableStartupCrashDetection and keep default value if not found`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertTrue(fixture.options.isEnableStartupCrashDetection) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 6741cbc8c4..6afe28acae 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -1,8 +1,12 @@ package io.sentry.android.core.cache +import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever +import io.sentry.ILogger import io.sentry.SentryEnvelope +import io.sentry.SentryLevel.DEBUG import io.sentry.android.core.AppStartState import io.sentry.android.core.SentryAndroidOptions import io.sentry.cache.EnvelopeCache @@ -25,9 +29,11 @@ class AndroidEnvelopeCacheTest { } val options = SentryAndroidOptions() val dateProvider = mock() + val logger = mock() lateinit var markerFile: File fun getSut( + enableStartupCrashDetection: Boolean = true, appStartMillis: Long? = null, currentTimeMillis: Long? = null ): AndroidEnvelopeCache { @@ -37,7 +43,9 @@ class AndroidEnvelopeCacheTest { markerFile = File(outboxDir, EnvelopeCache.STARTUP_CRASH_MARKER_FILE) - options.setLogger(mock()) + options.setLogger(logger) + options.isDebug = true + options.isEnableStartupCrashDetection = enableStartupCrashDetection if (appStartMillis != null) { AppStartState.getInstance().setAppStartMillis(appStartMillis) @@ -108,6 +116,16 @@ class AndroidEnvelopeCacheTest { assertTrue(fixture.markerFile.exists()) } + @Test + fun `when startup crashes detection is disabled, does not write startup crash file`() { + val cache = fixture.getSut(enableStartupCrashDetection = false) + + cache.store(fixture.envelope) + + verify(fixture.logger).log(eq(DEBUG), eq("Startup Crash detection is disabled.")) + assertFalse(fixture.markerFile.exists()) + } + internal class DiskFlushHint : DiskFlushNotification { override fun markFlushed() {} } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index 577f8a40f7..a4a1c5397a 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -24,7 +24,6 @@ public void onCreate() { // }); // */ // }); - throw new RuntimeException("Startup Crash!"); } private void strictMode() { From 80068db597d31d446444ee82b7520c0a36e4ea96 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 17 Oct 2022 21:26:00 +0200 Subject: [PATCH 12/13] Remove enableStartupDetection option --- .../api/sentry-android-core.api | 2 -- .../android/core/ManifestMetadataReader.java | 6 ----- .../android/core/SentryAndroidOptions.java | 17 ------------- .../core/cache/AndroidEnvelopeCache.java | 5 ---- .../core/ManifestMetadataReaderTest.kt | 25 ------------------- .../core/cache/AndroidEnvelopeCacheTest.kt | 20 --------------- 6 files changed, 75 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index a794edf843..11e5f679ce 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -141,7 +141,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun isEnableAppComponentBreadcrumbs ()Z public fun isEnableAppLifecycleBreadcrumbs ()Z public fun isEnableAutoActivityLifecycleTracing ()Z - public fun isEnableStartupCrashDetection ()Z public fun isEnableSystemEventBreadcrumbs ()Z public fun isEnableUserInteractionBreadcrumbs ()Z public fun isEnableUserInteractionTracing ()Z @@ -156,7 +155,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableAppComponentBreadcrumbs (Z)V public fun setEnableAppLifecycleBreadcrumbs (Z)V public fun setEnableAutoActivityLifecycleTracing (Z)V - public fun setEnableStartupCrashDetection (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V public fun setEnableUserInteractionBreadcrumbs (Z)V public fun setEnableUserInteractionTracing (Z)V 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 97ede8036b..b22f4f3bf1 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 @@ -78,8 +78,6 @@ final class ManifestMetadataReader { static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports"; static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context"; - static final String STARTUP_CRASH_ENABLE = "io.sentry.startup-crashes.enable"; - /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -228,10 +226,6 @@ static void applyMetadata( COLLECT_ADDITIONAL_CONTEXT, options.isCollectAdditionalContext())); - options.setEnableStartupCrashDetection( - readBool( - metadata, logger, STARTUP_CRASH_ENABLE, options.isEnableStartupCrashDetection())); - if (options.getTracesSampleRate() == null) { final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); if (tracesSampleRate != -1) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 3e1c604ed3..e456228537 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -131,15 +131,6 @@ public final class SentryAndroidOptions extends SentryOptions { */ private final long startupCrashDurationThresholdMillis = 2000; // 2s - /** - * Controls whether the SDK should detect Startup Crashes. - * - *

Startup Crashes are sent on {@link Sentry#init()} in a blocking way. - * - *

Default is true - */ - private boolean enableStartupCrashDetection = true; - public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); @@ -395,12 +386,4 @@ void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { public long getStartupCrashDurationThresholdMillis() { return startupCrashDurationThresholdMillis; } - - public boolean isEnableStartupCrashDetection() { - return enableStartupCrashDetection; - } - - public void setEnableStartupCrashDetection(boolean enableStartupCrashDetection) { - this.enableStartupCrashDetection = enableStartupCrashDetection; - } } 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 9d4de084d4..e08ef8b128 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 @@ -43,11 +43,6 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { final SentryAndroidOptions options = (SentryAndroidOptions) this.options; - if (!options.isEnableStartupCrashDetection()) { - options.getLogger().log(DEBUG, "Startup Crash detection is disabled."); - return; - } - final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { long timeSinceSdkInit = currentDateProvider.getCurrentTimeMillis() - appStartTime; 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 c06e52cc00..3e4f0b32f4 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 @@ -1061,29 +1061,4 @@ class ManifestMetadataReaderTest { // Assert assertTrue(fixture.options.isCollectAdditionalContext) } - - @Test - fun `applyMetadata reads enableStartupCrashDetection to options`() { - // Arrange - val bundle = bundleOf(ManifestMetadataReader.STARTUP_CRASH_ENABLE to false) - val context = fixture.getContext(metaData = bundle) - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - assertFalse(fixture.options.isEnableStartupCrashDetection) - } - - @Test - fun `applyMetadata reads enableStartupCrashDetection and keep default value if not found`() { - // Arrange - val context = fixture.getContext() - - // Act - ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) - - // Assert - assertTrue(fixture.options.isEnableStartupCrashDetection) - } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 6afe28acae..1528730312 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -1,12 +1,8 @@ package io.sentry.android.core.cache -import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever -import io.sentry.ILogger import io.sentry.SentryEnvelope -import io.sentry.SentryLevel.DEBUG import io.sentry.android.core.AppStartState import io.sentry.android.core.SentryAndroidOptions import io.sentry.cache.EnvelopeCache @@ -29,11 +25,9 @@ class AndroidEnvelopeCacheTest { } val options = SentryAndroidOptions() val dateProvider = mock() - val logger = mock() lateinit var markerFile: File fun getSut( - enableStartupCrashDetection: Boolean = true, appStartMillis: Long? = null, currentTimeMillis: Long? = null ): AndroidEnvelopeCache { @@ -43,10 +37,6 @@ class AndroidEnvelopeCacheTest { markerFile = File(outboxDir, EnvelopeCache.STARTUP_CRASH_MARKER_FILE) - options.setLogger(logger) - options.isDebug = true - options.isEnableStartupCrashDetection = enableStartupCrashDetection - if (appStartMillis != null) { AppStartState.getInstance().setAppStartMillis(appStartMillis) } @@ -116,16 +106,6 @@ class AndroidEnvelopeCacheTest { assertTrue(fixture.markerFile.exists()) } - @Test - fun `when startup crashes detection is disabled, does not write startup crash file`() { - val cache = fixture.getSut(enableStartupCrashDetection = false) - - cache.store(fixture.envelope) - - verify(fixture.logger).log(eq(DEBUG), eq("Startup Crash detection is disabled.")) - assertFalse(fixture.markerFile.exists()) - } - internal class DiskFlushHint : DiskFlushNotification { override fun markFlushed() {} } From be08f25b3e5fc52f7d7c132ee71fdc05d02ccfa6 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 17 Oct 2022 21:38:33 +0200 Subject: [PATCH 13/13] Fix changelog --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 763b0c1138..896e619709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,6 @@ - Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246)) - Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290)) - -### Features - - Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277)) ## 6.5.0