Skip to content

Commit b1d738b

Browse files
fix(android): Prevent duplicate standalone app start measurements
Require the app-start pending flag even when standalone app-start transactions bypass foreground checks. Preserve completed non-activity app-start timings so fallback resolution does not overwrite stopped spans. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent cbaebe2 commit b1d738b

5 files changed

Lines changed: 60 additions & 17 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,7 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr
775775
public fun setClassLoadedUptimeMs (J)V
776776
public fun setOnNoActivityStartedListener (Lio/sentry/android/core/performance/AppStartMetrics$OnNoActivityStartedListener;)V
777777
public fun shouldSendStartMeasurements ()Z
778+
public fun shouldSendStartMeasurements (Z)Z
778779
}
779780

780781
public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,13 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) {
8585
// the app start measurement is only sent once and only if the transaction has
8686
// the app.start span, which is automatically created by the SDK.
8787
if (hasAppStartSpan(transaction)) {
88-
// For non-activity starts, appLaunchedInForeground is false, so
89-
// shouldSendStartMeasurements() would return false. We still want to attach child spans.
88+
// For non-activity starts, appLaunchedInForeground is false, so standalone app start
89+
// transactions bypass only the foreground check, not the duplicate-send guard.
9090
final @Nullable SpanContext traceContext = transaction.getContexts().getTrace();
9191
final boolean isStandaloneAppStartTxn =
9292
traceContext != null && STANDALONE_APP_START_OP.equals(traceContext.getOperation());
9393

94-
if (appStartMetrics.shouldSendStartMeasurements() || isStandaloneAppStartTxn) {
94+
if (appStartMetrics.shouldSendStartMeasurements(isStandaloneAppStartTxn)) {
9595
final @NotNull TimeSpan appStartTimeSpan =
9696
isStandaloneAppStartTxn
9797
? appStartMetrics.getAppStartTimeSpanDirect()

sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,13 @@ public void onAppStartSpansSent() {
228228
activityLifecycles.clear();
229229
}
230230

231+
public boolean shouldSendStartMeasurements(final boolean ignoreForegroundCheck) {
232+
return shouldSendStartMeasurements
233+
&& (ignoreForegroundCheck || appLaunchedInForeground.getValue());
234+
}
235+
231236
public boolean shouldSendStartMeasurements() {
232-
return shouldSendStartMeasurements && appLaunchedInForeground.getValue();
237+
return shouldSendStartMeasurements(false);
233238
}
234239

235240
public long getClassLoadedUptimeMs() {
@@ -502,7 +507,9 @@ private void resolveNonActivityAppStartEndTime() {
502507

503508
private void stopNonActivityAppStartAt(final long stopUptimeMs) {
504509
if (appStartSpan.hasStarted()) {
505-
appStartSpan.setStoppedAt(stopUptimeMs);
510+
if (appStartSpan.hasNotStopped()) {
511+
appStartSpan.setStoppedAt(stopUptimeMs);
512+
}
506513
} else if (sdkInitTimeSpan.hasStarted() && sdkInitTimeSpan.hasNotStopped()) {
507514
sdkInitTimeSpan.setStoppedAt(stopUptimeMs);
508515
}

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,23 @@ class PerformanceAndroidEventProcessorTest {
206206
assertTrue(tr2.measurements.isEmpty())
207207
}
208208

209+
@Test
210+
fun `do not add standalone app start metric twice`() {
211+
val sut = fixture.getSut()
212+
213+
setStandaloneColdAppStartMetrics()
214+
215+
var tr1 = getTransaction(AppStartType.COLD)
216+
tr1 = sut.process(tr1, Hint())
217+
218+
var tr2 = getTransaction(AppStartType.COLD)
219+
tr2 = sut.process(tr2, Hint())
220+
221+
assertTrue(tr1.measurements.containsKey(MeasurementValue.KEY_APP_START_COLD))
222+
assertFalse(tr2.measurements.containsKey(MeasurementValue.KEY_APP_START_COLD))
223+
assertFalse(tr2.measurements.containsKey(MeasurementValue.KEY_APP_START_WARM))
224+
}
225+
209226
@Test
210227
fun `do not add app start metric if its not ready`() {
211228
val sut = fixture.getSut()
@@ -522,10 +539,10 @@ class PerformanceAndroidEventProcessorTest {
522539
val appStartSpan = createAppStartSpan(tr.contexts.trace!!.traceId)
523540
tr.spans.add(appStartSpan)
524541

525-
assertTrue(appStartMetrics.shouldSendStartMeasurements())
542+
assertTrue(appStartMetrics.shouldSendStartMeasurements(false))
526543
// then the app start metrics should be attached
527544
tr = sut.process(tr, Hint())
528-
assertFalse(appStartMetrics.shouldSendStartMeasurements())
545+
assertFalse(appStartMetrics.shouldSendStartMeasurements(false))
529546

530547
assertTrue(tr.spans.any { "application.load" == it.op })
531548

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

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class AppStartMetricsTest {
175175
// but no activity creation happened
176176
// then the app wasn't launched in foreground and nothing should be sent
177177
assertFalse(metrics.isAppLaunchedInForeground)
178-
assertFalse(metrics.shouldSendStartMeasurements())
178+
assertFalse(metrics.shouldSendStartMeasurements(false))
179179

180180
val now = TimeUnit.MINUTES.toMillis(2) + 1234567
181181
SystemClock.setCurrentTimeMillis(now)
@@ -185,7 +185,7 @@ class AppStartMetricsTest {
185185

186186
// then it should restart the timespan
187187
assertTrue(metrics.isAppLaunchedInForeground)
188-
assertTrue(metrics.shouldSendStartMeasurements())
188+
assertTrue(metrics.shouldSendStartMeasurements(false))
189189
assertTrue(metrics.appStartTimeSpan.hasStarted())
190190
assertEquals(now, metrics.appStartTimeSpan.startUptimeMs)
191191
assertFalse(metrics.applicationOnCreateTimeSpan.hasStarted())
@@ -288,6 +288,24 @@ class AppStartMetricsTest {
288288
assertEquals(100, metrics.appStartTimeSpan.durationMs)
289289
}
290290

291+
@Test
292+
fun `resolveNonActivityAppStartEndTime does not overwrite stopped appStartTimeSpan`() {
293+
val metrics = AppStartMetrics.getInstance()
294+
metrics.appStartTimeSpan.apply {
295+
setStartedAt(100)
296+
setStoppedAt(150)
297+
}
298+
metrics.applicationOnCreateTimeSpan.apply {
299+
setStartedAt(120)
300+
setStoppedAt(200)
301+
}
302+
303+
metrics.registerLifecycleCallbacks(mock<Application>())
304+
waitForMainLooperIdle()
305+
306+
assertEquals(50, metrics.appStartTimeSpan.durationMs)
307+
}
308+
291309
@Test
292310
fun `getAppStartTimeSpanDirect falls back to sdkInitTimeSpan when appStartSpan has not stopped`() {
293311
val metrics = AppStartMetrics.getInstance()
@@ -461,11 +479,11 @@ class AppStartMetricsTest {
461479
val appStartMetrics = AppStartMetrics.getInstance()
462480
appStartMetrics.addActivityLifecycleTimeSpans(mock())
463481
appStartMetrics.contentProviderOnCreateTimeSpans.add(mock())
464-
assertTrue(appStartMetrics.shouldSendStartMeasurements())
482+
assertTrue(appStartMetrics.shouldSendStartMeasurements(false))
465483
appStartMetrics.onAppStartSpansSent()
466484
assertTrue(appStartMetrics.activityLifecycleTimeSpans.isEmpty())
467485
assertTrue(appStartMetrics.contentProviderOnCreateTimeSpans.isEmpty())
468-
assertFalse(appStartMetrics.shouldSendStartMeasurements())
486+
assertFalse(appStartMetrics.shouldSendStartMeasurements(false))
469487
}
470488

471489
@Test
@@ -479,18 +497,18 @@ class AppStartMetricsTest {
479497

480498
// then the app start type should be cold and measurements should be sent
481499
assertEquals(AppStartMetrics.AppStartType.COLD, appStartMetrics.appStartType)
482-
assertTrue(appStartMetrics.shouldSendStartMeasurements())
500+
assertTrue(appStartMetrics.shouldSendStartMeasurements(false))
483501

484502
// when the activity gets destroyed
485503
appStartMetrics.onAppStartSpansSent()
486-
assertFalse(appStartMetrics.shouldSendStartMeasurements())
504+
assertFalse(appStartMetrics.shouldSendStartMeasurements(false))
487505

488506
appStartMetrics.onActivityDestroyed(activity0)
489507

490508
// then it should reset sending the measurements for the next warm activity
491509
appStartMetrics.onActivityCreated(mock<Activity>(), mock<Bundle>())
492510
assertEquals(AppStartMetrics.AppStartType.WARM, appStartMetrics.appStartType)
493-
assertTrue(appStartMetrics.shouldSendStartMeasurements())
511+
assertTrue(appStartMetrics.shouldSendStartMeasurements(false))
494512
}
495513

496514
@Test
@@ -882,7 +900,7 @@ class AppStartMetricsTest {
882900
whenever(firstActivity.isChangingConfigurations).thenReturn(false)
883901
metrics.onActivityCreated(firstActivity, null)
884902
assertEquals(AppStartMetrics.AppStartType.COLD, metrics.appStartType)
885-
assertTrue(metrics.shouldSendStartMeasurements())
903+
assertTrue(metrics.shouldSendStartMeasurements(false))
886904
metrics.onAppStartSpansSent()
887905
waitForMainLooperIdle()
888906

@@ -895,15 +913,15 @@ class AppStartMetricsTest {
895913
metrics.onActivityCreated(secondActivity, null)
896914
assertEquals(AppStartMetrics.AppStartType.WARM, metrics.appStartType)
897915
assertTrue(metrics.isAppLaunchedInForeground)
898-
assertTrue(metrics.shouldSendStartMeasurements())
916+
assertTrue(metrics.shouldSendStartMeasurements(false))
899917
metrics.onAppStartSpansSent()
900918

901919
// Third activity - should still be warm
902920
SystemClock.setCurrentTimeMillis(SystemClock.uptimeMillis() + 100)
903921
metrics.onActivityCreated(mock<Activity>(), null)
904922
assertEquals(AppStartMetrics.AppStartType.WARM, metrics.appStartType)
905923
assertTrue(metrics.isAppLaunchedInForeground)
906-
assertFalse(metrics.shouldSendStartMeasurements())
924+
assertFalse(metrics.shouldSendStartMeasurements(false))
907925
}
908926

909927
@Test

0 commit comments

Comments
 (0)