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

Taps not always associated with the correct fragment #1387

Open
GetsEclectic opened this issue Apr 14, 2023 · 8 comments
Open

Taps not always associated with the correct fragment #1387

GetsEclectic opened this issue Apr 14, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@GetsEclectic
Copy link

Thanks for taking the time for reporting an issue!

Describe what happened
Include any error message or stack trace if available.

I noticed that in the RUM UI, taps are often not associated with the fragment they belong to. I don't see the fragment information being sent for a tap in the sdk, it seems like it might be associated with the most recent 'View Load'? I played around with the code a bit and was able to more reliably determine the correct fragment in handleTapUp like this:

target.findFragment().javaClass.name

Steps to reproduce the issue:

Tap things in the app and then look at RUM

Describe what you expected:

These MaterialCardViews are actually both in InvestHomeFragment:

Tap stash-androidstash-android Tap on Retirement Portfolio on screen com.stash.bottomnav.BottomNavContainerFragment
Tap stash-androidstash-android Tap on Personal Portfolio on screen com.stash.features.invest.portfolio.ui.fragment.InvestHomeFragment
-- -- --

Additional context

  • Android OS version: 14
  • Device Model: pixel 4 xl
  • Datadog SDK version: 1.17.1
  • Versions of any other relevant dependencies (OkHttp, …):
  • Proguard configuration:
  • Gradle Plugins:
@GetsEclectic GetsEclectic added the bug Something isn't working label Apr 14, 2023
@0xnm
Copy link
Contributor

0xnm commented Apr 17, 2023

Hello @GetsEclectic! Thanks for reporting the issue. Indeed actions are always associated with the current active RUM view, and we are making our best effort to find the element which was tapped.

As far as I understand from the table you've provided both Personal Portfolio and Retirement Portfolio elements are in the com.stash.features.invest.portfolio.ui.fragment.InvestHomeFragment actually (meaning Retirement Portfolio doesn't actually belong to com.stash.bottomnav.BottomNavContainerFragment)? So that the name of the element tapped is correct, but the view ownership is wrong. Do I understand it the right way?

@GetsEclectic
Copy link
Author

Yes, that's correct.

@0xnm
Copy link
Contributor

0xnm commented Apr 18, 2023

In RUM all the actions are living (=linked) in the scope of the view which was started previously, it is not possible to re-link the action to another view (which may be not even reported to RUM).

In this particular case where multiple fragments can co-exist on the screen it is hard to say which one is the "the most real" one with automated view tracking strategies, so some manual tuning is needed: you can try to provide the custom predicate with the view tracking strategy to filter out fragments which shouldn't be reported as RUM view.

In your particular case I guess com.stash.bottomnav.BottomNavContainerFragment can be filtered out.

Let me know if it works for you.

@GetsEclectic
Copy link
Author

I can't just filter out BottomNavContainerFragment unfortunately because it does have 4-5 buttons that can be tapped. It looks like calling findFragment() on the target does a better job of attributing it to the correct fragment.

@0xnm
Copy link
Contributor

0xnm commented Apr 18, 2023

Unfortunately we don't have a way to specify view name in the RUM action tracking API, because it may break the core concepts of RUM: for example the view submitted to startAction call may not be reported to RUM yet, etc. That it why all the RUM actions are reported in the scope of the view which was started earlier (and of course if there are multiple fragments starting at the same time, it may be the case that the last one started is not expected one).

@GetsEclectic
Copy link
Author

Yeah sounds like it wouldn't be a small change, it might be valuable to be able to supply a 'suggested' fragment which could be overridden? I think it's pretty common that screens contain multiple fragments?

@0xnm
Copy link
Contributor

0xnm commented Apr 24, 2023

Looking from the high perspective it is indeed not a simple problem to tackle, simply because of the fragment lifecycle: they can be started in the different order and at the different time. We could provide some predicate supplying the list of fragments detected and asking to provide the active one, but then we have to wait for all fragments to become active, which may skew some performance metrics and bring other problems or inconsistencies.

I'll add this problem to the backlog, will see what we can do about it.

@GetsEclectic
Copy link
Author

sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants