Skip to content

Commit 45f6c0b

Browse files
buenaflorclaude
andcommitted
refactor(extend-app-start): Return ExtendedAppStart from the listener 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>
1 parent 3d13c67 commit 45f6c0b

4 files changed

Lines changed: 58 additions & 32 deletions

File tree

sentry-android-core/api/sentry-android-core.api

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,18 @@ public final class io/sentry/android/core/AppStartExtension : io/sentry/IAppStar
192192
public fun getExtendedAppStartSpan ()Lio/sentry/ISpan;
193193
public fun getExtendedEndTime ()Lio/sentry/SentryDate;
194194
public fun isActive ()Z
195-
public fun onExtended (Lio/sentry/ITransaction;Lio/sentry/ISpan;)V
196195
public fun reset ()V
197196
public fun setExtendAppStartListener (Lio/sentry/android/core/AppStartExtension$ExtendAppStartListener;)V
198197
}
199198

200199
public abstract interface class io/sentry/android/core/AppStartExtension$ExtendAppStartListener {
201-
public abstract fun onExtendAppStartRequested ()V
200+
public abstract fun onExtendAppStartRequested ()Lio/sentry/android/core/AppStartExtension$ExtendedAppStart;
201+
}
202+
203+
public final class io/sentry/android/core/AppStartExtension$ExtendedAppStart {
204+
public final field span Lio/sentry/ISpan;
205+
public final field transaction Lio/sentry/ITransaction;
206+
public fun <init> (Lio/sentry/ITransaction;Lio/sentry/ISpan;)V
202207
}
203208

204209
public final class io/sentry/android/core/AppState : java/io/Closeable {

sentry-android-core/src/main/java/io/sentry/android/core/AppStartExtension.java

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,36 @@
2121
* keeps the new "extend app start" concern out of that already-large class.
2222
*
2323
* <p>Both the eager standalone App Start {@link ITransaction} and its extended child {@link ISpan}
24-
* are created by the integration (which has access to scopes) and handed back here via {@link
25-
* #onExtended(ITransaction, ISpan)}. This component owns them from then on: it never stores them in
26-
* the integration's shared transaction field, so the per-activity cleanup can never cancel an
27-
* eagerly-created extension.
24+
* are created by the integration (which has access to scopes) and returned to this component from
25+
* {@link ExtendAppStartListener#onExtendAppStartRequested()}. This component owns them from then
26+
* on: it never stores them in the integration's shared transaction field, so the per-activity
27+
* cleanup can never cancel an eagerly-created extension.
2828
*/
2929
@ApiStatus.Internal
3030
public final class AppStartExtension implements IAppStartExtender {
3131

32+
/**
33+
* The standalone App Start transaction and its extended child span, created by the integration.
34+
*/
35+
public static final class ExtendedAppStart {
36+
public final @NotNull ITransaction transaction;
37+
public final @NotNull ISpan span;
38+
39+
public ExtendedAppStart(final @NotNull ITransaction transaction, final @NotNull ISpan span) {
40+
this.transaction = transaction;
41+
this.span = span;
42+
}
43+
}
44+
3245
/**
3346
* Notifies the integration that an extension was requested. The integration creates the
34-
* standalone App Start transaction + extended child span (it has scopes) and hands them back via
35-
* {@link #onExtended(ITransaction, ISpan)}. When no listener is registered (e.g. standalone
36-
* tracing is disabled), {@link #extendAppStart()} is inert and the whole API stays a no-op.
47+
* standalone App Start transaction + extended child span (it has scopes) and returns them, or
48+
* returns {@code null} to decline (e.g. standalone tracing is disabled). When no listener is
49+
* registered, {@link #extendAppStart()} is inert and the whole API stays a no-op.
3750
*/
3851
public interface ExtendAppStartListener {
39-
void onExtendAppStartRequested();
52+
@Nullable
53+
ExtendedAppStart onExtendAppStartRequested();
4054
}
4155

4256
private final @NotNull AppStartMetrics metrics;
@@ -81,24 +95,15 @@ public void extendAppStart() {
8195
}
8296
final @Nullable ExtendAppStartListener listener = extendAppStartListener;
8397
if (listener != null) {
84-
listener.onExtendAppStartRequested();
98+
final @Nullable ExtendedAppStart extended = listener.onExtendAppStartRequested();
99+
if (extended != null) {
100+
this.extendedTransaction = extended.transaction;
101+
this.extendedSpan = extended.span;
102+
}
85103
}
86104
}
87105
}
88106

89-
/**
90-
* Hands the eagerly-created standalone App Start transaction and its extended child span over to
91-
* this component, which owns them from now on. Called synchronously by the integration while
92-
* handling {@link ExtendAppStartListener#onExtendAppStartRequested()}.
93-
*/
94-
public void onExtended(
95-
final @NotNull ITransaction transaction, final @NotNull ISpan extendedSpan) {
96-
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
97-
this.extendedTransaction = transaction;
98-
this.extendedSpan = extendedSpan;
99-
}
100-
}
101-
102107
@Override
103108
public void finishAppStart() {
104109
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {

sentry-android-core/src/test/java/io/sentry/android/core/AppStartExtensionTest.kt

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,18 @@ class AppStartExtensionTest {
3939
txn: ITransaction = mock(),
4040
span: ISpan = mock(),
4141
): Pair<ITransaction, ISpan> {
42-
setExtendAppStartListener { onExtended(txn, span) }
42+
setExtendAppStartListener { AppStartExtension.ExtendedAppStart(txn, span) }
4343
return txn to span
4444
}
4545

4646
@Test
4747
fun `extendAppStart fires the listener when the window is open`() {
4848
val ext = extension(windowOpen = true)
4949
val calls = AtomicInteger()
50-
ext.setExtendAppStartListener { calls.incrementAndGet() }
50+
ext.setExtendAppStartListener {
51+
calls.incrementAndGet()
52+
null
53+
}
5154
ext.extendAppStart()
5255
assertEquals(1, calls.get())
5356
}
@@ -56,7 +59,10 @@ class AppStartExtensionTest {
5659
fun `extendAppStart does not fire the listener when the window is closed`() {
5760
val ext = extension(windowOpen = false)
5861
val calls = AtomicInteger()
59-
ext.setExtendAppStartListener { calls.incrementAndGet() }
62+
ext.setExtendAppStartListener {
63+
calls.incrementAndGet()
64+
null
65+
}
6066
ext.extendAppStart()
6167
assertEquals(0, calls.get())
6268
}
@@ -77,7 +83,7 @@ class AppStartExtensionTest {
7783
val span = mock<ISpan>()
7884
ext.setExtendAppStartListener {
7985
calls.incrementAndGet()
80-
ext.onExtended(txn, span)
86+
AppStartExtension.ExtendedAppStart(txn, span)
8187
}
8288
ext.extendAppStart()
8389
ext.extendAppStart()

sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.sentry.DateUtils
1313
import io.sentry.IContinuousProfiler
1414
import io.sentry.ITransactionProfiler
1515
import io.sentry.SentryNanotimeDate
16+
import io.sentry.android.core.AppStartExtension
1617
import io.sentry.android.core.ContextUtils
1718
import io.sentry.android.core.CurrentActivityHolder
1819
import io.sentry.android.core.SentryAndroidOptions
@@ -1067,22 +1068,31 @@ class AppStartMetricsTest {
10671068
assertSame(metrics.appStartExtension, metrics.appStartExtension)
10681069
}
10691070

1071+
/** Drives the singleton's eager extension into the active state via the listener path. */
1072+
private fun activateExtension(metrics: AppStartMetrics) {
1073+
metrics.appStartExtension.setExtendAppStartListener {
1074+
AppStartExtension.ExtendedAppStart(mock(), mock())
1075+
}
1076+
metrics.appStartExtension.extendAppStart()
1077+
assertTrue(metrics.appStartExtension.isActive)
1078+
}
1079+
10701080
@Test
10711081
fun `clear resets the extension state`() {
10721082
val metrics = AppStartMetrics.getInstance()
1073-
metrics.appStartExtension.onExtended(mock(), mock())
1074-
assertTrue(metrics.appStartExtension.isActive)
1083+
activateExtension(metrics)
10751084
metrics.clear()
10761085
assertFalse(metrics.appStartExtension.isActive)
1086+
metrics.appStartExtension.setExtendAppStartListener(null)
10771087
}
10781088

10791089
@Test
10801090
fun `onAppStartSpansSent resets the extension state`() {
10811091
val metrics = AppStartMetrics.getInstance()
1082-
metrics.appStartExtension.onExtended(mock(), mock())
1083-
assertTrue(metrics.appStartExtension.isActive)
1092+
activateExtension(metrics)
10841093
metrics.onAppStartSpansSent()
10851094
assertFalse(metrics.appStartExtension.isActive)
1095+
metrics.appStartExtension.setExtendAppStartListener(null)
10861096
}
10871097

10881098
// endregion

0 commit comments

Comments
 (0)