Skip to content

feat(extend-app-start): [2/4] Extract AppStartExtension component#5606

Open
buenaflor wants to merge 23 commits into
feat/app-start-extension-corefrom
feat/app-start-extension-android
Open

feat(extend-app-start): [2/4] Extract AppStartExtension component#5606
buenaflor wants to merge 23 commits into
feat/app-start-extension-corefrom
feat/app-start-extension-android

Conversation

@buenaflor

@buenaflor buenaflor commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

PR Stack (Extend App Start)


📜 Description

Pulls the Android extender out of the already-large AppStartMetrics into a focused, testable AppStartExtension component.

Class Change
AppStartExtension (new) implements IAppStartExtender. Owns the lock-guarded extend lifecycle and holds the eager App Start ITransaction + its child ISpan.
AppStartMetrics Holds the component and exposes the extend gate canExtendAppStart().
AndroidOptionsInitializer Registers the component as the options' app start extender.

Notes

  • AppStartExtension has no scopes, so it can't create the transaction/span itself — the integration builds them and returns an ExtendedAppStart from the listener.
  • getExtendedEndTime() is deadline-aware: null on DEADLINE_EXCEEDED, so the vital is suppressed rather than inflated.
  • canExtendAppStart() gate = measurements not sent + no activity + first frame not drawn. The foreground check is ignored so headless starts can extend as well.
  • Inert on its own: no listener is registered until [3/4], so extendAppStart() is a no-op and getExtendedAppStartSpan() returns NoOpSpan.

💡 Motivation and Context

Part of the app start extension API stack (#5553). A dedicated component keeps the new concern out of AppStartMetrics and is easy to isolate and test.

💚 How did you test it?

Unit tests in AppStartExtensionTest, AppStartMetricsTest, and AndroidOptionsInitializerTest cover the extend gate, listener firing, inert behavior, idempotent finish, deadline suppression, and wiring. apiCheck, spotless, and the :sentry-android-core unit suite pass.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[3/4] registers the listener, eagerly creates the App Start transaction + child, and extends/suppresses the app start vital in PerformanceAndroidEventProcessor.

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

#skip-changelog

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against b11b0fa

@sentry

sentry Bot commented Jun 23, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.44.1 (1) release

⚙️ sentry-android Build Distribution Settings

@buenaflor buenaflor force-pushed the feat/app-start-extension-core branch from 064d3f1 to 7a7d63f Compare June 23, 2026 12:13
@buenaflor buenaflor force-pushed the feat/app-start-extension-android branch from fd450bb to 54be119 Compare June 23, 2026 12:13
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 305.32 ms 370.59 ms 65.27 ms
Size 0 B 0 B 0 B

Baseline results on branch: feat/app-start-extension-core

Startup times

Revision Plain With Sentry Diff
3d04aae 324.42 ms 359.30 ms 34.88 ms
69d43cc 380.70 ms 424.90 ms 44.20 ms
2821a4d 315.46 ms 366.56 ms 51.10 ms

App size

Revision Plain With Sentry Diff
3d04aae 0 B 0 B 0 B
69d43cc 0 B 0 B 0 B
2821a4d 0 B 0 B 0 B

Previous results on branch: feat/app-start-extension-android

Startup times

Revision Plain With Sentry Diff
ba0608b 319.63 ms 373.04 ms 53.41 ms
682e090 347.33 ms 427.28 ms 79.95 ms
db5be2f 311.84 ms 365.94 ms 54.10 ms
388ffc5 315.18 ms 362.43 ms 47.25 ms
ca0c0cfc14d958d067e640e132d8de863dfe8eb1 318.45 ms 370.06 ms 51.61 ms
b76203f 336.02 ms 403.42 ms 67.40 ms
201a6fd 326.00 ms 371.92 ms 45.92 ms

App size

Revision Plain With Sentry Diff
ba0608b 0 B 0 B 0 B
682e090 0 B 0 B 0 B
db5be2f 0 B 0 B 0 B
388ffc5 0 B 0 B 0 B
ca0c0cfc14d958d067e640e132d8de863dfe8eb1 0 B 0 B 0 B
b76203f 0 B 0 B 0 B
201a6fd 0 B 0 B 0 B

@buenaflor buenaflor force-pushed the feat/app-start-extension-android branch from 54be119 to d2b96ad Compare June 24, 2026 10:47
@buenaflor buenaflor changed the title feat(extend-app-start): [2/4] Add deferred extended span and extension state feat(extend-app-start): [2/4] Extract AppStartExtension component Jun 25, 2026
@buenaflor buenaflor force-pushed the feat/app-start-extension-android branch 2 times, most recently from 3bc1643 to 45f6c0b Compare June 25, 2026 11:37
buenaflor and others added 8 commits June 25, 2026 14:27
…ndroid extender

Replaces the AppStartMetrics IAppStartExtender implementation and the deferred
ExtendedAppStartSpan with a focused, lock-guarded AppStartExtension that owns the
eager App Start transaction and extended span. AppStartMetrics now only holds the
component and exposes isAppStartWindowOpen(). Inert until 3/4 registers the listener.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ead of static scope lookup

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… instead of a callback

The listener now returns the created transaction+span (or null to decline)
rather than calling back into onExtended() while extendAppStart() holds the
lock. This removes the re-entrant lock acquisition and the A->B->A round trip,
collapsing two public methods into one linear flow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o clear

Drop comments that restate code or annotate omissions (region markers, getter
javadoc, the reset call-site comment, the ExtendedAppStart/isActive docs). Rename
AppStartExtension.reset() to clear() to match its owner AppStartMetrics.clear().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ting it

The extension logs only two warnings, both on rare guard paths in extendAppStart().
Inline Sentry.getCurrentScopes().getOptions().getLogger() at those sites instead of
holding a logger field set at init, dropping the field, setLogger(), and the
AndroidOptionsInitializer wiring. extendAppStart() runs post-init, so the lookup
always yields the configured logger.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tartExtension

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mments

Drop finishTransaction's javadoc (trivial body; it described caller context and
waitForChildren behavior configured elsewhere) and reduce getExtendedEndTime's
javadoc to a single inline note on the only non-obvious branch (deadline suppression).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o finishExtendedAppStart

Implements the renamed IAppStartExtender.finishExtendedAppStart().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@buenaflor buenaflor force-pushed the feat/app-start-extension-android branch from 2d5188e to 01b1dc4 Compare June 25, 2026 12:32
buenaflor and others added 3 commits June 25, 2026 15:10
…on extendAppStart

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd trim comments

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
}
}

public void finishTransaction(final @NotNull SentryDate endTimestamp) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be used in the next PR 3/4

@buenaflor buenaflor marked this pull request as ready for review June 25, 2026 13:16
Copilot AI review requested due to automatic review settings June 25, 2026 13:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the “Extend App Start” stack by extracting the Android app-start extension behavior out of AppStartMetrics into a dedicated, testable AppStartExtension component, and wiring it into Android options so the core IAppStartExtender bridge can route to Android.

Changes:

  • Introduces AppStartExtension (@ApiStatus.Internal) implementing IAppStartExtender, encapsulating the extension lifecycle and extended txn/span references.
  • Updates AppStartMetrics to own the component, expose an isAppStartWindowOpen() gate, and clear the extension state when spans are sent / metrics reset.
  • Registers the extender in AndroidOptionsInitializer and adds/extends unit coverage for the new behavior and gates.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sentry-android-core/src/main/java/io/sentry/android/core/AppStartExtension.java New internal extender component with lock-guarded lifecycle and span/txn accessors.
sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java Holds the extension component, adds isAppStartWindowOpen(), clears extension state on reset/send.
sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java Wires the Android extender implementation into SentryAndroidOptions.
sentry-android-core/src/test/java/io/sentry/android/core/AppStartExtensionTest.kt New unit tests for extension activation/inert behavior, finish semantics, and end-time suppression.
sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt Adds coverage for the new “window open” gate and extension state reset paths.
sentry-android-core/api/sentry-android-core.api API dump updates for newly added internal class and new AppStartMetrics methods.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…nished flag

getExtendedEndTime() gated on span.isFinished(), but finishing the extended
span completes the waitForChildren transaction and runs the event processor
re-entrantly within finishExtendedAppStart(), before the span's finished flag
is set. The processor then saw an unfinished span and dropped the app start
measurement whenever the extension finished after the first frame. Read
getFinishDate() (set before the finish callback) instead, which also keeps the
extended end controllable in tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@buenaflor buenaflor force-pushed the feat/app-start-extension-android branch from a622ea2 to d2c16a2 Compare June 25, 2026 14:51
buenaflor and others added 3 commits June 25, 2026 17:22
…pan end

When the extended span finished after the timestamp passed to
finishTransaction() but before that call ran (e.g. a synchronous extension in a
headless start, where finishTransaction runs later at main-thread idle),
waitForChildren had nothing left to wait for and the transaction kept the
earlier passed timestamp. The extended span then ended after the transaction,
and the app start vital exceeded the transaction duration. Finish at
max(endTimestamp, extended span finish date) so the span is contained and the
duration matches the vital.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The setter wrote the field without synchronization while extendAppStart()
reads it under the lock, leaving no happens-before edge. Acquire the same
lock in the setter, consistent with the rest of the class's mutable state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e26ee97. Configure here.

Comment thread sentry-android-core/src/main/java/io/sentry/android/core/AppStartExtension.java Outdated
buenaflor and others added 4 commits June 29, 2026 09:17
Finishing the extended span runs the event processor re-entrantly (via
the waitForChildren transaction) before the span's isFinished() flag is
set, while the finish timestamp is already in place. Reading isFinished()
could therefore hand out a span that is already finishing. Switch to
getFinishDate() == null, matching getExtendedEndTime().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…endedAppStartSpan

Replace the repeated reentrancy explanation with a cross-reference to
getExtendedEndTime(), which holds the canonical version.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…length

Single-line form fits the 100-char limit so spotless leaves it unwrapped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mment

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
buenaflor and others added 2 commits June 29, 2026 11:09
…ppStart

The predicate has a single consumer — the extend gate in
AppStartExtension — so name it for that intent. Also drop the
self-evident max-end comment in finishTransaction. Regenerated apiDump.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed test

After getExtendedAppStartSpan switched to getFinishDate(), the
"after the span finished" case exercises the same branch as the
reentrancy test (finishDate set -> NoOp); the isFinished stub was inert.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
buenaflor and others added 2 commits June 29, 2026 11:37
Move these component accessors into the component-extraction PR (they were
introduced later in the wiring PR). Keeps the full AppStartExtension surface
and its unit tests together here; the wiring PR only consumes them. Inert on
its own. Regenerated apiDump.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
It is written without a lock in onAppStartSpansSent() and read across threads in canExtendAppStart() (via extendAppStart()), with no happens-before edge. volatile guarantees visibility, matching the AtomicInteger/AtomicBoolean siblings in the same check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants