-
-
Notifications
You must be signed in to change notification settings - Fork 472
feat(extend-app-start): [2/4] Extract AppStartExtension component #5606
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: feat/app-start-extension-core
Are you sure you want to change the base?
Changes from all commits
7dfe617
4d0c97d
b5ac7f5
81efded
3cc743e
1dafb9b
8548b4d
01b1dc4
05a9df2
5830303
ec4da6c
d2c16a2
b6dd76e
ff8c056
e26ee97
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 |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import io.sentry.IAppStartExtender; | ||
| import io.sentry.ISentryLifecycleToken; | ||
| import io.sentry.ISpan; | ||
| import io.sentry.ITransaction; | ||
| import io.sentry.NoOpSpan; | ||
| import io.sentry.Sentry; | ||
| import io.sentry.SentryDate; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SpanStatus; | ||
| import io.sentry.android.core.performance.AppStartMetrics; | ||
| import io.sentry.util.AutoClosableReentrantLock; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| @ApiStatus.Internal | ||
| public final class AppStartExtension implements IAppStartExtender { | ||
|
|
||
| public static final class ExtendedAppStart { | ||
| public final @NotNull ITransaction transaction; | ||
| public final @NotNull ISpan span; | ||
|
|
||
| public ExtendedAppStart(final @NotNull ITransaction transaction, final @NotNull ISpan span) { | ||
| this.transaction = transaction; | ||
| this.span = span; | ||
| } | ||
| } | ||
|
|
||
| public interface ExtendAppStartListener { | ||
| @Nullable | ||
| ExtendedAppStart onExtendAppStartRequested(); | ||
| } | ||
|
|
||
| private final @NotNull AppStartMetrics metrics; | ||
| private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); | ||
|
|
||
| private @Nullable ExtendAppStartListener extendAppStartListener; | ||
| private @Nullable ISpan extendedSpan; | ||
| private @Nullable ITransaction extendedTransaction; | ||
|
|
||
| public AppStartExtension(final @NotNull AppStartMetrics metrics) { | ||
| this.metrics = metrics; | ||
| } | ||
|
|
||
| public void setExtendAppStartListener(final @Nullable ExtendAppStartListener listener) { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| this.extendAppStartListener = listener; | ||
| } | ||
| } | ||
|
Copilot marked this conversation as resolved.
|
||
|
|
||
| @Override | ||
| public void extendAppStart() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| if (extendedSpan != null) { | ||
| Sentry.getCurrentScopes() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log(SentryLevel.WARNING, "App start is already being extended."); | ||
| return; | ||
| } | ||
| if (!metrics.isAppStartWindowOpen()) { | ||
| Sentry.getCurrentScopes() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "Cannot extend app start: the app start window has already passed."); | ||
| return; | ||
| } | ||
| final @Nullable ExtendAppStartListener listener = extendAppStartListener; | ||
| if (listener != null) { | ||
| final @Nullable ExtendedAppStart extended = listener.onExtendAppStartRequested(); | ||
| if (extended != null) { | ||
| this.extendedTransaction = extended.transaction; | ||
| this.extendedSpan = extended.span; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void finishExtendedAppStart() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| final @Nullable ISpan span = extendedSpan; | ||
| if (span != null && !span.isFinished()) { | ||
| span.finish(SpanStatus.OK); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public @NotNull ISpan getExtendedAppStartSpan() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| final @Nullable ISpan span = extendedSpan; | ||
| if (span != null && !span.isFinished()) { | ||
|
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: The Suggested FixTo fix the reentrancy vulnerability, change the condition at line 97 from Prompt for AI Agent |
||
| return span; | ||
| } | ||
| return NoOpSpan.getInstance(); | ||
| } | ||
| } | ||
|
|
||
| public boolean isActive() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| return extendedTransaction != null && !extendedTransaction.isFinished(); | ||
| } | ||
| } | ||
|
|
||
| public void finishTransaction(final @NotNull SentryDate endTimestamp) { | ||
|
Contributor
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 will be used in the next PR 3/4 |
||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| final @Nullable ITransaction transaction = extendedTransaction; | ||
| if (transaction != null && !transaction.isFinished()) { | ||
| // End at the extended span's finish if it ran past endTimestamp, so the txn covers it. | ||
| final @Nullable ISpan span = extendedSpan; | ||
| final @Nullable SentryDate spanEnd = span == null ? null : span.getFinishDate(); | ||
| final @NotNull SentryDate end = | ||
| spanEnd != null && spanEnd.isAfter(endTimestamp) ? spanEnd : endTimestamp; | ||
| transaction.finish(SpanStatus.OK, end); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public @Nullable SentryDate getExtendedEndTime() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| final @Nullable ISpan span = extendedSpan; | ||
| if (span == null) { | ||
| return null; | ||
| } | ||
| // A deadline timeout would report an artificially inflated duration; suppress the vital | ||
| // instead. | ||
| if (span.getStatus() == SpanStatus.DEADLINE_EXCEEDED) { | ||
| return null; | ||
| } | ||
| // Read the finish date, not isFinished(): finishing the extended span completes the | ||
| // waitForChildren transaction and runs the event processor re-entrantly before the span's | ||
| // finished flag is set, but the finish timestamp is already in place. Null until finished. | ||
| return span.getFinishDate(); | ||
| } | ||
| } | ||
|
|
||
| public void clear() { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| extendedSpan = null; | ||
| extendedTransaction = null; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import io.sentry.NoOpLogger; | ||
| import io.sentry.SentryDate; | ||
| import io.sentry.TracesSamplingDecision; | ||
| import io.sentry.android.core.AppStartExtension; | ||
| import io.sentry.android.core.BuildInfoProvider; | ||
| import io.sentry.android.core.ContextUtils; | ||
| import io.sentry.android.core.CurrentActivityHolder; | ||
|
|
@@ -98,6 +99,7 @@ public enum AppStartType { | |
| private @Nullable String appStartBaggageHeader; | ||
| private @Nullable SentryDate appStartEndTime; | ||
| private @Nullable ApplicationStartInfo cachedStartInfo; | ||
| private final @NotNull AppStartExtension appStartExtension = new AppStartExtension(this); | ||
|
|
||
| public static @NotNull AppStartMetrics getInstance() { | ||
| if (instance == null) { | ||
|
|
@@ -281,6 +283,7 @@ public void onAppStartSpansSent() { | |
| shouldSendStartMeasurements = false; | ||
| contentProviderOnCreates.clear(); | ||
| activityLifecycles.clear(); | ||
| appStartExtension.clear(); | ||
| } | ||
|
|
||
| public boolean shouldSendStartMeasurements(final boolean ignoreForegroundCheck) { | ||
|
|
@@ -336,6 +339,21 @@ public long getClassLoadedUptimeMs() { | |
| return new TimeSpan(); | ||
| } | ||
|
|
||
| public @NotNull AppStartExtension getAppStartExtension() { | ||
| return appStartExtension; | ||
| } | ||
|
|
||
| /** | ||
| * Whether the app start window is still open, i.e. an app start can be extended: measurements | ||
| * haven't been sent yet, no activity has been created, and the first frame hasn't been drawn. The | ||
| * foreground check is ignored so headless app starts (broadcast/service) can also be extended. | ||
| */ | ||
| public boolean isAppStartWindowOpen() { | ||
| return shouldSendStartMeasurements(true) | ||
| && activeActivitiesCounter.get() == 0 | ||
| && !firstDrawDone.get(); | ||
| } | ||
|
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. Extend window reopens after destroyMedium Severity
Reviewed by Cursor Bugbot for commit e26ee97. Configure here. |
||
|
|
||
| @TestOnly | ||
| void setFirstIdle(final long firstIdle) { | ||
| this.firstIdle = firstIdle; | ||
|
|
@@ -377,6 +395,7 @@ public void clear() { | |
| appStartBaggageHeader = null; | ||
| appStartEndTime = null; | ||
| cachedStartInfo = null; | ||
| appStartExtension.clear(); | ||
| } | ||
|
|
||
| public @Nullable ITransactionProfiler getAppStartProfiler() { | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.