-
-
Notifications
You must be signed in to change notification settings - Fork 464
feat: minimal tombstone integration #4933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ba0c7db
9933e47
10b1e94
f12b658
4b6e1fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| import android.app.Application; | ||
| import android.content.Context; | ||
| import android.content.pm.PackageInfo; | ||
| import android.os.Build; | ||
| import io.sentry.CompositePerformanceCollector; | ||
| import io.sentry.DeduplicateMultithreadedEventProcessor; | ||
| import io.sentry.DefaultCompositePerformanceCollector; | ||
|
|
@@ -372,6 +373,10 @@ static void installDefaultIntegrations( | |
| final Class<?> sentryNdkClass = loadClass.loadClass(SENTRY_NDK_CLASS_NAME, options.getLogger()); | ||
| options.addIntegration(new NdkIntegration(sentryNdkClass)); | ||
|
|
||
| if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.R) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not entirely sure about this one:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As without tombstones the reported events won't be any useful I suggest to check against There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Wrong Android API level check for tombstone supportThe integration checks for |
||
| options.addIntegration(new TombstoneIntegration(context)); | ||
| } | ||
|
|
||
| // this integration uses android.os.FileObserver, we can't move to sentry | ||
| // before creating a pure java impl. | ||
| options.addIntegration(EnvelopeFileObserverIntegration.getOutboxFileObserver()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion; | ||
|
|
||
| import android.app.ActivityManager; | ||
| import android.app.ApplicationExitInfo; | ||
| import android.content.Context; | ||
| import android.os.Build; | ||
| import androidx.annotation.RequiresApi; | ||
| import io.sentry.DateUtils; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.Integration; | ||
| import io.sentry.SentryEvent; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.android.core.internal.tombstone.TombstoneParser; | ||
| import io.sentry.cache.EnvelopeCache; | ||
| import io.sentry.cache.IEnvelopeCache; | ||
| import io.sentry.transport.CurrentDateProvider; | ||
| import io.sentry.transport.ICurrentDateProvider; | ||
| import io.sentry.util.Objects; | ||
| import java.io.Closeable; | ||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| @ApiStatus.Internal | ||
| public class TombstoneIntegration implements Integration, Closeable { | ||
| static final long NINETY_DAYS_THRESHOLD = TimeUnit.DAYS.toMillis(91); | ||
|
|
||
| private final @NotNull Context context; | ||
| private final @NotNull ICurrentDateProvider dateProvider; | ||
| private @Nullable SentryAndroidOptions options; | ||
|
|
||
| public TombstoneIntegration(final @NotNull Context context) { | ||
| // using CurrentDateProvider instead of AndroidCurrentDateProvider as AppExitInfo uses | ||
| // System.currentTimeMillis | ||
| this(context, CurrentDateProvider.getInstance()); | ||
| } | ||
|
|
||
| TombstoneIntegration( | ||
| final @NotNull Context context, final @NotNull ICurrentDateProvider dateProvider) { | ||
| this.context = ContextUtils.getApplicationContext(context); | ||
| this.dateProvider = dateProvider; | ||
| } | ||
|
|
||
| @Override | ||
| public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) { | ||
| this.options = | ||
| Objects.requireNonNull( | ||
| (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, | ||
| "SentryAndroidOptions is required"); | ||
|
|
||
| this.options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "TombstoneIntegration enabled: %s", | ||
| this.options.isTombstoneEnabled()); | ||
|
|
||
| if (this.options.isTombstoneEnabled()) { | ||
| if (this.options.getCacheDirPath() == null) { | ||
| this.options | ||
| .getLogger() | ||
| .log(SentryLevel.INFO, "Cache dir is not set, unable to process Tombstones"); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| options | ||
| .getExecutorService() | ||
| .submit(new TombstoneProcessor(context, scopes, this.options, dateProvider)); | ||
| } catch (Throwable e) { | ||
| options.getLogger().log(SentryLevel.DEBUG, "Failed to start TombstoneProcessor.", e); | ||
| } | ||
| options.getLogger().log(SentryLevel.DEBUG, "TombstoneIntegration installed."); | ||
| addIntegrationToSdkVersion("Tombstone"); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| if (options != null) { | ||
| options.getLogger().log(SentryLevel.DEBUG, "TombstoneIntegration removed."); | ||
| } | ||
| } | ||
|
|
||
| @ApiStatus.Internal | ||
| public static class TombstoneProcessor implements Runnable { | ||
|
|
||
| @NotNull private final Context context; | ||
| @NotNull private final IScopes scopes; | ||
| @NotNull private final SentryAndroidOptions options; | ||
| private final long threshold; | ||
|
|
||
| public TombstoneProcessor( | ||
| @NotNull Context context, | ||
| @NotNull IScopes scopes, | ||
| @NotNull SentryAndroidOptions options, | ||
| @NotNull ICurrentDateProvider dateProvider) { | ||
| this.context = context; | ||
| this.scopes = scopes; | ||
| this.options = options; | ||
|
|
||
| this.threshold = dateProvider.getCurrentTimeMillis() - NINETY_DAYS_THRESHOLD; | ||
| } | ||
|
|
||
| @Override | ||
| @RequiresApi(api = Build.VERSION_CODES.R) | ||
| public void run() { | ||
| final ActivityManager activityManager = | ||
| (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); | ||
|
|
||
| final List<ApplicationExitInfo> applicationExitInfoList; | ||
| applicationExitInfoList = activityManager.getHistoricalProcessExitReasons(null, 0, 0); | ||
supervacuus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (applicationExitInfoList.isEmpty()) { | ||
| options.getLogger().log(SentryLevel.DEBUG, "No records in historical exit reasons."); | ||
| return; | ||
| } | ||
|
|
||
| final IEnvelopeCache cache = options.getEnvelopeDiskCache(); | ||
| if (cache instanceof EnvelopeCache) { | ||
| if (options.isEnableAutoSessionTracking() | ||
| && !((EnvelopeCache) cache).waitPreviousSessionFlush()) { | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "Timed out waiting to flush previous session to its own file."); | ||
|
|
||
| // if we timed out waiting here, we can already flush the latch, because the timeout is | ||
| // big | ||
| // enough to wait for it only once and we don't have to wait again in | ||
| // PreviousSessionFinalizer | ||
| ((EnvelopeCache) cache).flushPreviousSession(); | ||
| } | ||
| } | ||
|
|
||
| // making a deep copy as we're modifying the list | ||
| final List<ApplicationExitInfo> exitInfos = new ArrayList<>(applicationExitInfoList); | ||
|
|
||
| // search for the latest Tombstone to report it separately as we're gonna enrich it. The | ||
| // latest | ||
| // Tombstone will be first in the list, as it's filled last-to-first in order of appearance | ||
| ApplicationExitInfo latestTombstone = null; | ||
| for (ApplicationExitInfo applicationExitInfo : exitInfos) { | ||
| if (applicationExitInfo.getReason() == ApplicationExitInfo.REASON_CRASH_NATIVE) { | ||
| latestTombstone = applicationExitInfo; | ||
| // remove it, so it's not reported twice | ||
| // TODO: if we fail after this, we effectively lost the ApplicationExitInfo (maybe only | ||
| // remove after we reported it) | ||
| exitInfos.remove(applicationExitInfo); | ||
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentionally marked as
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah good point, for ANRs we write a timestamp to disk (impl) to ensure we don't double report on the next app launch. @romtsn what do you think about this? I guess what's to discuss here:
|
||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (latestTombstone == null) { | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "No Tombstones have been found in the historical exit reasons list."); | ||
| return; | ||
| } | ||
|
|
||
| if (latestTombstone.getTimestamp() < threshold) { | ||
| options | ||
| .getLogger() | ||
| .log(SentryLevel.DEBUG, "Latest Tombstones happened too long ago, returning early."); | ||
| return; | ||
| } | ||
|
|
||
| reportAsSentryEvent(latestTombstone); | ||
| } | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.R) | ||
| private void reportAsSentryEvent(ApplicationExitInfo exitInfo) { | ||
| SentryEvent event; | ||
| try { | ||
| TombstoneParser parser = new TombstoneParser(exitInfo.getTraceInputStream()); | ||
This comment was marked as outdated.
Sorry, something went wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, but also left it open, since I don't know yet how the general propagation of errors is handled in integration
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually use a wide try/catch and silently swallow those errors in combination with logging the error using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missing null check for trace input stream
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See: #4933 (comment) |
||
| event = parser.parse(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: InputStream from getTraceInputStream is never closedThe |
||
| event.setTimestamp(DateUtils.getDateTime(exitInfo.getTimestamp())); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Uncaught exception crashes background thread silentlyWhen |
||
| } | ||
|
|
||
| scopes.captureEvent(event); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romtsn Not sure if you followed the conversations, but as you can see here, protobuf requires a runtime dependency. It will have an impact of around 10kb. IMHO fine for now, we should still check how stable this library is to avoid and consumer version mismatch issues.