Skip to content

Commit 7d8a394

Browse files
romtsnsentry-junior[bot]getsentry-botclaude
authored
fix(android-fragment): support detach/attach navigation in fragment tracing (#5660)
* fix(android-fragment): support detach/attach navigation in fragment tracing For detach/attach tab navigation (manual tab switching, ViewPager v1 with FragmentPagerAdapter, custom navigation frameworks), onFragmentCreated is skipped for off-screen fragments that are re-attached. Previously this left ui.load spans open until the 30s activity transaction deadline, producing inflated performance data. Fix by calling startTracing in onFragmentViewCreated as well as onFragmentCreated. startTracing is idempotent (no-op if a span is already running), so the normal onFragmentCreated -> onFragmentViewCreated path is unaffected. Add matching stopTracing calls in onFragmentResumed (covers the detach/attach path where onFragmentStarted may be skipped) and onFragmentViewDestroyed (failsafe for fragments destroyed before reaching STARTED or RESUMED). stopTracing is also idempotent, so the normal path is unaffected. Co-Authored-By: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> * Format code * Add changelog entry and detach/attach sample Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove duplicate test methods in fragment lifecycle test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Update screen name on scope for detach/attach fragment re-attachment onFragmentCreated is skipped during detach/attach navigation, so the screen name was never updated for re-attached fragments. Mirror the screen tracking into onFragmentViewCreated to cover that path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Format code * Address PR feedback: internalize guards into startTracing and fix sample layout Move isAdded check, screen tracking, and tracing logic into startTracing() to deduplicate guards from onFragmentCreated and onFragmentViewCreated. Fix DetachAttachTabsActivity sample rendering on API 35+ by using NoActionBar theme and fitsSystemWindows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: Decouple screen tracking from performance tracing in fragments Screen name updates on scope should work independently of whether performance tracing is enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 32ca3d0 commit 7d8a394

8 files changed

Lines changed: 257 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
### Fixes
1010

11+
- Fix fragment tracing not working with detach/attach navigation ([#5660](https://github.com/getsentry/sentry-java/pull/5660))
1112
- Don't start a redundant UI interaction transaction when a transaction is already bound to the Scope ([#5491](https://github.com/getsentry/sentry-java/issues/5491))
1213
- Previously, `SentryGestureListener` always started a UI transaction and only afterwards skipped binding it to the Scope when a manually-bound transaction already existed, leaving the new transaction to be dropped as an idle transaction without children.
1314
- Fix potential NPE within `Scope.endSession()` ([#5657](https://github.com/getsentry/sentry-java/pull/5657))

‎sentry-android-fragment/src/main/java/io/sentry/android/fragment/SentryFragmentLifecycleCallbacks.kt‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,7 @@ public class SentryFragmentLifecycleCallbacks(
7676
) {
7777
addBreadcrumb(fragment, FragmentLifecycleState.CREATED)
7878

79-
// we only start the tracing for the fragment if the fragment has been added to its activity
80-
// and not only to the backstack
81-
if (fragment.isAdded) {
82-
if (scopes.options.isEnableScreenTracking) {
83-
scopes.configureScope { it.screen = getFragmentName(fragment) }
84-
}
85-
startTracing(fragment)
86-
}
79+
startTracing(fragment)
8780
}
8881

8982
override fun onFragmentViewCreated(
@@ -93,17 +86,30 @@ public class SentryFragmentLifecycleCallbacks(
9386
savedInstanceState: Bundle?,
9487
) {
9588
addBreadcrumb(fragment, FragmentLifecycleState.VIEW_CREATED)
89+
90+
// For detach/attach navigation (e.g. manual tab switching, ViewPager v1 with
91+
// FragmentPagerAdapter, custom navigation frameworks), onFragmentCreated is never called for
92+
// off-screen fragments that are re-attached. Starting here enables a narrower
93+
// "view created -> resumed" span for those paths. startTracing is idempotent, so for the
94+
// normal onFragmentCreated -> onFragmentViewCreated path this is a no-op.
95+
startTracing(fragment)
9696
}
9797

9898
override fun onFragmentStarted(fragmentManager: FragmentManager, fragment: Fragment) {
9999
addBreadcrumb(fragment, FragmentLifecycleState.STARTED)
100100

101-
// ViewPager2 locks background fragments to STARTED state
101+
// ViewPager2 locks background fragments to STARTED state, so we stop here to avoid
102+
// spans hanging for off-screen fragments that never reach RESUMED.
102103
stopTracing(fragment)
103104
}
104105

105106
override fun onFragmentResumed(fragmentManager: FragmentManager, fragment: Fragment) {
106107
addBreadcrumb(fragment, FragmentLifecycleState.RESUMED)
108+
109+
// For detach/attach navigation, onFragmentStarted may not fire before onFragmentResumed.
110+
// If a span is still running here, stop it now. stopTracing is idempotent, so this is a
111+
// no-op for the normal path where onFragmentStarted already stopped the span.
112+
stopTracing(fragment)
107113
}
108114

109115
override fun onFragmentPaused(fragmentManager: FragmentManager, fragment: Fragment) {
@@ -116,6 +122,10 @@ public class SentryFragmentLifecycleCallbacks(
116122

117123
override fun onFragmentViewDestroyed(fragmentManager: FragmentManager, fragment: Fragment) {
118124
addBreadcrumb(fragment, FragmentLifecycleState.VIEW_DESTROYED)
125+
126+
// Failsafe: cancel any span that didn't finish via the normal started/resumed path
127+
// (e.g. fragment view destroyed before reaching STARTED or RESUMED).
128+
stopTracing(fragment)
119129
}
120130

121131
override fun onFragmentDestroyed(fragmentManager: FragmentManager, fragment: Fragment) {
@@ -153,14 +163,23 @@ public class SentryFragmentLifecycleCallbacks(
153163
fragmentsWithOngoingTransactions.containsKey(fragment)
154164

155165
private fun startTracing(fragment: Fragment) {
166+
if (!fragment.isAdded) {
167+
return
168+
}
169+
170+
val fragmentName = getFragmentName(fragment)
171+
172+
if (scopes.options.isEnableScreenTracking) {
173+
scopes.configureScope { it.screen = fragmentName }
174+
}
175+
156176
if (!isPerformanceEnabled || isRunningSpan(fragment)) {
157177
return
158178
}
159179

160180
var transaction: ISpan? = null
161181
scopes.configureScope { transaction = it.transaction }
162182

163-
val fragmentName = getFragmentName(fragment)
164183
val span = transaction?.startChild(FRAGMENT_LOAD_OP, fragmentName)
165184

166185
span?.let {

‎sentry-android-fragment/src/test/java/io/sentry/android/fragment/SentryFragmentLifecycleCallbacksTest.kt‎

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,15 @@ class SentryFragmentLifecycleCallbacksTest {
4343
enableAutoFragmentLifecycleTracing: Boolean = false,
4444
tracesSampleRate: Double? = 1.0,
4545
isAdded: Boolean = true,
46+
enableScreenTracking: Boolean = false,
4647
): SentryFragmentLifecycleCallbacks {
4748
whenever(scopes.options)
48-
.thenReturn(SentryOptions().apply { setTracesSampleRate(tracesSampleRate) })
49+
.thenReturn(
50+
SentryOptions().apply {
51+
setTracesSampleRate(tracesSampleRate)
52+
isEnableScreenTracking = enableScreenTracking
53+
}
54+
)
4955
whenever(span.spanContext)
5056
.thenReturn(SpanContext(SentryId.EMPTY_ID, SpanId.EMPTY_ID, "op", null, null))
5157
whenever(transaction.startChild(any<String>(), any<String>())).thenReturn(span)
@@ -251,6 +257,115 @@ class SentryFragmentLifecycleCallbacksTest {
251257
verify(fixture.span).finish(check { assertEquals(SpanStatus.OK, it) })
252258
}
253259

260+
@Test
261+
fun `When fragment view is created via detach-attach, it should start tracing if enabled`() {
262+
// Simulates detach/attach navigation: onFragmentCreated is NOT called, only
263+
// onFragmentViewCreated
264+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)
265+
266+
sut.onFragmentViewCreated(
267+
fixture.fragmentManager,
268+
fixture.fragment,
269+
view = mock(),
270+
savedInstanceState = null,
271+
)
272+
273+
verify(fixture.transaction)
274+
.startChild(
275+
check<String> { assertEquals(SentryFragmentLifecycleCallbacks.FRAGMENT_LOAD_OP, it) },
276+
check<String> { assertEquals("androidx.fragment.app.Fragment", it) },
277+
)
278+
}
279+
280+
@Test
281+
fun `When fragment view is created via detach-attach, it should update screen name`() {
282+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true, enableScreenTracking = true)
283+
284+
sut.onFragmentViewCreated(
285+
fixture.fragmentManager,
286+
fixture.fragment,
287+
view = mock(),
288+
savedInstanceState = null,
289+
)
290+
291+
verify(fixture.scope).screen = "androidx.fragment.app.Fragment"
292+
}
293+
294+
@Test
295+
fun `When performance is disabled, it should still update screen name`() {
296+
val sut =
297+
fixture.getSut(enableAutoFragmentLifecycleTracing = false, enableScreenTracking = true)
298+
299+
sut.onFragmentViewCreated(
300+
fixture.fragmentManager,
301+
fixture.fragment,
302+
view = mock(),
303+
savedInstanceState = null,
304+
)
305+
306+
verify(fixture.scope).screen = "androidx.fragment.app.Fragment"
307+
verify(fixture.transaction, never()).startChild(any<String>(), any<String>())
308+
}
309+
310+
@Test
311+
fun `When fragment view is created after onFragmentCreated, it should not start a second span`() {
312+
// Normal path: onFragmentCreated already started the span; onFragmentViewCreated is a no-op
313+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)
314+
315+
sut.onFragmentCreated(fixture.fragmentManager, fixture.fragment, savedInstanceState = null)
316+
sut.onFragmentViewCreated(
317+
fixture.fragmentManager,
318+
fixture.fragment,
319+
view = mock(),
320+
savedInstanceState = null,
321+
)
322+
323+
verify(fixture.transaction).startChild(any<String>(), any<String>())
324+
}
325+
326+
@Test
327+
fun `When fragment is resumed, it should stop tracing if span is still running`() {
328+
// Simulates detach/attach path where onFragmentStarted may be skipped
329+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)
330+
331+
sut.onFragmentViewCreated(
332+
fixture.fragmentManager,
333+
fixture.fragment,
334+
view = mock(),
335+
savedInstanceState = null,
336+
)
337+
sut.onFragmentResumed(fixture.fragmentManager, fixture.fragment)
338+
339+
verify(fixture.span).finish(check { assertEquals(SpanStatus.OK, it) })
340+
}
341+
342+
@Test
343+
fun `When fragment is resumed after started, it should not double-finish the span`() {
344+
// Normal path: onFragmentStarted already stopped the span; onFragmentResumed is a no-op
345+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)
346+
347+
sut.onFragmentCreated(fixture.fragmentManager, fixture.fragment, savedInstanceState = null)
348+
sut.onFragmentStarted(fixture.fragmentManager, fixture.fragment)
349+
sut.onFragmentResumed(fixture.fragmentManager, fixture.fragment)
350+
351+
verify(fixture.span).finish(any())
352+
}
353+
354+
@Test
355+
fun `When fragment view is destroyed before started, it should stop tracing as failsafe`() {
356+
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)
357+
358+
sut.onFragmentViewCreated(
359+
fixture.fragmentManager,
360+
fixture.fragment,
361+
view = mock(),
362+
savedInstanceState = null,
363+
)
364+
sut.onFragmentViewDestroyed(fixture.fragmentManager, fixture.fragment)
365+
366+
verify(fixture.span).finish(check { assertEquals(SpanStatus.OK, it) })
367+
}
368+
254369
private fun verifyBreadcrumbAdded(expectedState: String) {
255370
verify(fixture.scopes)
256371
.addBreadcrumb(

‎sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@
6565
android:name=".ThirdActivityFragment"
6666
android:exported="false" />
6767

68+
<activity
69+
android:name=".DetachAttachTabsActivity"
70+
android:exported="false"
71+
android:theme="@style/AppTheme.Main" />
72+
6873
<activity
6974
android:name=".GesturesActivity"
7075
android:exported="false" />

‎sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DetachAttachTabsActivity.kt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package io.sentry.samples.android
2+
3+
import android.os.Bundle
4+
import android.view.View
5+
import android.widget.TextView
6+
import androidx.appcompat.app.AppCompatActivity
7+
import androidx.fragment.app.Fragment
8+
import androidx.fragment.app.commit
9+
10+
class DetachAttachTabsActivity : AppCompatActivity(R.layout.activity_detach_attach_tabs) {
11+
12+
private val tags = arrayOf("tab_a", "tab_b")
13+
14+
override fun onCreate(savedInstanceState: Bundle?) {
15+
super.onCreate(savedInstanceState)
16+
17+
findViewById<View>(R.id.btn_tab_a).setOnClickListener { showTab(0) }
18+
findViewById<View>(R.id.btn_tab_b).setOnClickListener { showTab(1) }
19+
20+
if (savedInstanceState == null) {
21+
val tabB = TabFragmentB()
22+
supportFragmentManager.commit {
23+
add(R.id.tab_container, TabFragmentA(), tags[0])
24+
add(R.id.tab_container, tabB, tags[1])
25+
detach(tabB)
26+
}
27+
}
28+
}
29+
30+
private fun showTab(index: Int) {
31+
supportFragmentManager.commit {
32+
for (i in tags.indices) {
33+
val frag = supportFragmentManager.findFragmentByTag(tags[i]) ?: continue
34+
if (i == index) attach(frag) else detach(frag)
35+
}
36+
}
37+
}
38+
}
39+
40+
class TabFragmentA : Fragment(R.layout.fragment_tab) {
41+
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
42+
view.findViewById<TextView>(R.id.tab_label).text = "Tab A"
43+
}
44+
}
45+
46+
class TabFragmentB : Fragment(R.layout.fragment_tab) {
47+
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
48+
view.findViewById<TextView>(R.id.tab_label).text = "Tab B"
49+
}
50+
}

‎sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,18 @@ fun IntegrationsScreen() {
794794
}
795795
}
796796
}
797+
item {
798+
SentryTraced("open_detach_attach_tabs") {
799+
OutlinedButton(
800+
onClick = {
801+
activity.startActivity(Intent(activity, DetachAttachTabsActivity::class.java))
802+
},
803+
modifier = Modifier,
804+
) {
805+
Text("Open Detach/Attach Tabs", maxLines = 2, overflow = TextOverflow.Ellipsis)
806+
}
807+
}
808+
}
797809
item {
798810
SentryTraced("open_permissions_activity") {
799811
OutlinedButton(

‎sentry-samples/sentry-samples-android/src/main/res/layout/activity_detach_attach_tabs.xml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
3+
android:orientation="vertical"
4+
android:layout_width="match_parent"
5+
android:layout_height="match_parent"
6+
android:fitsSystemWindows="true">
7+
8+
<LinearLayout
9+
android:orientation="horizontal"
10+
android:layout_width="match_parent"
11+
android:layout_height="wrap_content"
12+
android:padding="8dp">
13+
14+
<Button
15+
android:id="@+id/btn_tab_a"
16+
android:layout_width="wrap_content"
17+
android:layout_height="wrap_content"
18+
android:text="Tab A" />
19+
20+
<Button
21+
android:id="@+id/btn_tab_b"
22+
android:layout_width="wrap_content"
23+
android:layout_height="wrap_content"
24+
android:text="Tab B" />
25+
</LinearLayout>
26+
27+
<FrameLayout
28+
android:id="@+id/tab_container"
29+
android:layout_width="match_parent"
30+
android:layout_height="0dp"
31+
android:layout_weight="1" />
32+
</LinearLayout>

‎sentry-samples/sentry-samples-android/src/main/res/layout/fragment_tab.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
3+
android:layout_width="match_parent"
4+
android:layout_height="match_parent">
5+
6+
<TextView
7+
android:id="@+id/tab_label"
8+
android:layout_width="wrap_content"
9+
android:layout_height="wrap_content"
10+
android:layout_gravity="center"
11+
android:textSize="24sp" />
12+
</FrameLayout>

0 commit comments

Comments
 (0)