Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide default fragment breadcrumb implementation #1374

Closed
vlad-kasprov opened this issue Apr 5, 2021 · 7 comments · Fixed by #1522
Closed

Provide default fragment breadcrumb implementation #1374

vlad-kasprov opened this issue Apr 5, 2021 · 7 comments · Fixed by #1522
Labels
enhancement New feature or request Platform: Android

Comments

@vlad-kasprov
Copy link

vlad-kasprov commented Apr 5, 2021

Hi. We use a lot of fragments in our Android application, but we don't get any breadcrumbs for them. Is it possible to add default implementation for collecting fragment breadcrumbs?

P.S. From what I know, we'll have to manually set it in each activity using FragmentManager#registeeFragmenrLifecycleCallbacks.

@marandaneto marandaneto added Platform: Android enhancement New feature or request labels Apr 6, 2021
@marandaneto
Copy link
Contributor

@VladyslavKasprov thanks for raising this issue, yes, we're aware of that, the main reason is, there's no global hook that we can access and add our lifecycle callback automatically, you'd need to do it manually for each FragmentManager.

2nd reason is that we'd need to depend on androidx.fragment, so ideally we'd have a new package so we don't overload our android-core dependency as not everyone uses fragments.

@vlad-kasprov
Copy link
Author

Separate dependency and manual hooking would be fine) Thank you) Is there a sample implementation that we can use in the meantime?

@marandaneto
Copy link
Contributor

@VladyslavKasprov yes, take a look at the Activity lifecycle for inspiration https://github.com/getsentry/sentry-java/blob/main/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java

if you wish to contribute with a PR, happy to guide :) ideally creating a new Gradle module etc

@wzieba
Copy link
Contributor

wzieba commented Jun 8, 2021

👋 @marandaneto I'm working currently on a FragmentLifecycleIntegration POC but I have a problem with how it should be registered.

My concern is that SentryAndroid#init initializes Sentry SDK with passed options, including all default Integration objects. It's okay as default Integrations rely on Application lifecycle so Sentry SDK can be initialized when an app is starting.

The challenge I see here is how to add Integration during the app runtime. I'd like to add FragmentLifecycleIntegration which requires FragmentManager later than SDK initialization because when app is starting, I don't have yet access to FragmentManager.

My question is: is there any way to add Integration after Sentry SDK is initialized? If not, can you see a different way to make FragmentLifecycleIntegration work?

@marandaneto
Copy link
Contributor

marandaneto commented Jun 8, 2021

@wzieba hey, thanks for your feedback.

Integrations and EventProcessor can't be added at runtime if not using the passed options.

A way to see this is using the ActivityLifecycleIntegration and calling activity.getFragmentManager() in the lifecycle methods, but this method is deprecated so likely not fine using it as most of the people already migrated to androidx.fragment.

Maybe installing a new ActivityFragmentLifecycleIntegration that relies on androidx.fragment.app and then calling activity.getSupportFragmentManager(), to add and remove the FragmentLifecycleIntegration callback.

this would likely require a new package as the SDK does not depend on the fragments package, like sentry-android-fragment module.

you also probably need to set up your SDK in manual mode https://docs.sentry.io/platforms/android/configuration/manual-init. as you need to add your new ActivityFragmentLifecycleIntegration integration that also hooks up FragmentLifecycleIntegration, this is just an idea though, didn't test it yet.

You'd also need to think about fragment.getChildFragmentManager() recursively for every fragment I guess in case you want to execute actions for every child manager.

Every Activity has a different instance of FragmentManager, so you probably need to registerFragmentLifecycleCallbacks and unregisterFragmentLifecycleCallbacks per Activity's lifecycle.

does that help? let me know if its not clear :) thanks for working on it btw.

@marandaneto
Copy link
Contributor

marandaneto commented Jun 9, 2021

the other option is just to offer the SentryFragmentLifecycleCallbacks and people hook up in the registerFragmentLifecycleCallbacks themselves, not ideal, yes, but it works.
The OkHttp interceptor is like this https://github.com/getsentry/sentry-java/blob/main/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt people have to add themselves.

A more risky one is adding bytecode manipulation via Gradle plugin :D

@vlad-kasprov
Copy link
Author

the other option is just to offer the SentryFragmentLifecycleCallbacks as people hook up in the registerFragmentLifecycleCallbacks themselves, not ideal, yes, but it works.
The OkHttp interceptor is like this https://github.com/getsentry/sentry-java/blob/main/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt people have to add themselves.

A more risky one is adding bytecode manipulation via Gradle plugin :D

Having an option to manually hook up the listener would be good enough. Its fine since most applications these days have very few activities. And even if you have many it's not a big deal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Platform: Android
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants