From 1b07765244dc48f1e43f8d4c272b2fa9d1d2621c Mon Sep 17 00:00:00 2001 From: Andy Zolyak Date: Tue, 7 May 2024 12:03:34 -0400 Subject: [PATCH 1/3] FEAT: Add version of withSentryObservableEffect that has better interop with fragment navigation --- .../compose/SentryNavigationIntegration.kt | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt index 4af368f0652..022278290d2 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt @@ -48,30 +48,23 @@ internal class SentryLifecycleObserver( * A [DisposableEffect] that captures a [Breadcrumb] and starts an [ITransaction] and sends * them to Sentry for every navigation event when being attached to the respective [NavHostController]. * - * @param enableNavigationBreadcrumbs Whether the integration should capture breadcrumbs for - * navigation events. - * @param enableNavigationTracing Whether the integration should start a new [ITransaction] - * with [SentryOptions.idleTimeout] for navigation events. + * @param navListener An instance of a [SentryNavigationListener] that is shared with other sentry integrations, like + * the fragment navigation integration. */ @Composable @NonRestartableComposable -public fun NavHostController.withSentryObservableEffect( - enableNavigationBreadcrumbs: Boolean = true, - enableNavigationTracing: Boolean = true +fun NavHostController.withSentryObservableEffect( + navListener: SentryNavigationListener ): NavHostController { - val enableBreadcrumbsSnapshot by rememberUpdatedState(enableNavigationBreadcrumbs) - val enableTracingSnapshot by rememberUpdatedState(enableNavigationTracing) + + val navListenerSnapshot by rememberUpdatedState(navListener) // As described in https://developer.android.com/codelabs/jetpack-compose-advanced-state-side-effects#6 val lifecycle = LocalLifecycleOwner.current.lifecycle DisposableEffect(lifecycle, this) { val observer = SentryLifecycleObserver( this@withSentryObservableEffect, - navListener = SentryNavigationListener( - enableNavigationBreadcrumbs = enableBreadcrumbsSnapshot, - enableNavigationTracing = enableTracingSnapshot, - traceOriginAppendix = TRACE_ORIGIN_APPENDIX - ) + navListener = navListenerSnapshot ) lifecycle.addObserver(observer) @@ -84,6 +77,32 @@ public fun NavHostController.withSentryObservableEffect( return this } +/** + * A [DisposableEffect] that captures a [Breadcrumb] and starts an [ITransaction] and sends + * them to Sentry for every navigation event when being attached to the respective [NavHostController]. + * This version of withSentryObservableEffect should be used if you are working purely with Compose. + * + * @param enableNavigationBreadcrumbs Whether the integration should capture breadcrumbs for + * navigation events. + * @param enableNavigationTracing Whether the integration should start a new [ITransaction] + * with [SentryOptions.idleTimeout] for navigation events. + */ +@Composable +@NonRestartableComposable +fun NavHostController.withSentryObservableEffect( + enableNavigationBreadcrumbs: Boolean = true, + enableNavigationTracing: Boolean = true +): NavHostController { + val enableBreadcrumbsSnapshot by rememberUpdatedState(enableNavigationBreadcrumbs) + val enableTracingSnapshot by rememberUpdatedState(enableNavigationTracing) + + return withSentryObservableEffect(navListener = SentryNavigationListener( + enableNavigationBreadcrumbs = enableBreadcrumbsSnapshot, + enableNavigationTracing = enableTracingSnapshot, + traceOriginAppendix = TRACE_ORIGIN_APPENDIX + )) +} + /** * A [DisposableEffect] that captures a [Breadcrumb] and starts an [ITransaction] and sends * them to Sentry for every navigation event when being attached to the respective [NavHostController]. From b92a2b4a297360dd56e67dc8593239668d1f133e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 7 Feb 2025 00:46:26 +0100 Subject: [PATCH 2/3] Changelog --- CHANGELOG.md | 4 +++- sentry-compose/api/android/sentry-compose.api | 1 + .../compose/SentryNavigationIntegration.kt | 21 ++++++++++--------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e51fb436fdd..3311aa1f26b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ ### Features - Create onCreate and onStart spans for all Activities ([#4025](https://github.com/getsentry/sentry-java/pull/4025)) -- Add split apks info to the `App` context ([#3193](https://github.com/getsentry/sentry-java/pull/3193))) +- Add split apks info to the `App` context ([#3193](https://github.com/getsentry/sentry-java/pull/3193)) +- Expose new `withSentryObservableEffect` method overload that accepts `SentryNavigationListener` as a parameter ([#3411](https://github.com/getsentry/sentry-java/pull/3411)) + - This allows sharing the same `SentryNavigationListener` instance across fragments and composables to preserve the trace ### Fixes diff --git a/sentry-compose/api/android/sentry-compose.api b/sentry-compose/api/android/sentry-compose.api index f8da7ecfb06..728e248dd31 100644 --- a/sentry-compose/api/android/sentry-compose.api +++ b/sentry-compose/api/android/sentry-compose.api @@ -18,6 +18,7 @@ public final class io/sentry/compose/SentryModifier { } public final class io/sentry/compose/SentryNavigationIntegrationKt { + public static final fun withSentryObservableEffect (Landroidx/navigation/NavHostController;Lio/sentry/android/navigation/SentryNavigationListener;Landroidx/compose/runtime/Composer;I)Landroidx/navigation/NavHostController; public static final fun withSentryObservableEffect (Landroidx/navigation/NavHostController;ZZLandroidx/compose/runtime/Composer;II)Landroidx/navigation/NavHostController; } diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt index 022278290d2..0246a5b9918 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryNavigationIntegration.kt @@ -49,14 +49,13 @@ internal class SentryLifecycleObserver( * them to Sentry for every navigation event when being attached to the respective [NavHostController]. * * @param navListener An instance of a [SentryNavigationListener] that is shared with other sentry integrations, like - * the fragment navigation integration. + * the fragment navigation integration. */ @Composable @NonRestartableComposable -fun NavHostController.withSentryObservableEffect( +public fun NavHostController.withSentryObservableEffect( navListener: SentryNavigationListener ): NavHostController { - val navListenerSnapshot by rememberUpdatedState(navListener) // As described in https://developer.android.com/codelabs/jetpack-compose-advanced-state-side-effects#6 @@ -80,7 +79,7 @@ fun NavHostController.withSentryObservableEffect( /** * A [DisposableEffect] that captures a [Breadcrumb] and starts an [ITransaction] and sends * them to Sentry for every navigation event when being attached to the respective [NavHostController]. - * This version of withSentryObservableEffect should be used if you are working purely with Compose. + * This version of withSentryObservableEffect should be used if you are working purely with Compose. * * @param enableNavigationBreadcrumbs Whether the integration should capture breadcrumbs for * navigation events. @@ -89,18 +88,20 @@ fun NavHostController.withSentryObservableEffect( */ @Composable @NonRestartableComposable -fun NavHostController.withSentryObservableEffect( +public fun NavHostController.withSentryObservableEffect( enableNavigationBreadcrumbs: Boolean = true, enableNavigationTracing: Boolean = true ): NavHostController { val enableBreadcrumbsSnapshot by rememberUpdatedState(enableNavigationBreadcrumbs) val enableTracingSnapshot by rememberUpdatedState(enableNavigationTracing) - return withSentryObservableEffect(navListener = SentryNavigationListener( - enableNavigationBreadcrumbs = enableBreadcrumbsSnapshot, - enableNavigationTracing = enableTracingSnapshot, - traceOriginAppendix = TRACE_ORIGIN_APPENDIX - )) + return withSentryObservableEffect( + navListener = SentryNavigationListener( + enableNavigationBreadcrumbs = enableBreadcrumbsSnapshot, + enableNavigationTracing = enableTracingSnapshot, + traceOriginAppendix = TRACE_ORIGIN_APPENDIX + ) + ) } /** From 96f14fdd1a974a62d3ecce844edb2b1bf9e5497d Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 7 Feb 2025 00:51:29 +0100 Subject: [PATCH 3/3] pr id --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3311aa1f26b..326043e723a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - Create onCreate and onStart spans for all Activities ([#4025](https://github.com/getsentry/sentry-java/pull/4025)) - Add split apks info to the `App` context ([#3193](https://github.com/getsentry/sentry-java/pull/3193)) -- Expose new `withSentryObservableEffect` method overload that accepts `SentryNavigationListener` as a parameter ([#3411](https://github.com/getsentry/sentry-java/pull/3411)) +- Expose new `withSentryObservableEffect` method overload that accepts `SentryNavigationListener` as a parameter ([#4143](https://github.com/getsentry/sentry-java/pull/4143)) - This allows sharing the same `SentryNavigationListener` instance across fragments and composables to preserve the trace ### Fixes