mozilla / debug-ping-view

Glean Debug Ping Viewer
Mozilla Public License 2.0
4 stars 2 forks source link

Record clicks via glean's builtin element click recording feature #154

Closed abhi-agg closed 6 months ago

abhi-agg commented 6 months ago

This PR adds capability to record element clicks via Glean's builtin element click recording api (in https://github.com/mozilla/glean.js/pull/1848).

Scenarios tested:

  1. GleanMetrics.recordElementClick() api and
  2. automatic element click event recording (setting data-glean-label attribute for Back to top button)

Steps to test:

  1. Running npm start on local machine
  2. Setting window.Glean.setLogPings(true) in console
  3. Clicking on various buttons and checking in the console logs that glean.element_click ping is sent for each of the button click

EDIT: We are more interested in testing the automatic click event feature. Changed PR accordingly. See https://github.com/mozilla/debug-ping-view/pull/154#issuecomment-1970892702 for details.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1884644

abhi-agg commented 6 months ago

More generally, why are we choosing to use the GleanMetrics.recordElementClick in some places and then in others we are using data-glean-label?

Is there a reason we can't use data-glean-label in all the places?

@rosahbruno I just wanted to use both options in the code i.e. the automatic event capturing as well as calling manual apis. Apart from this, no other reason.

If you think we should switch to using just one option, I am fine with that as well. I have no strong opinion on this one 👍🏾

rosahbruno commented 6 months ago

@abhi-agg Personally, I think we should try and avoid using the manual events if possible. They are the most similar to existing click events, so we know they are going to work like normal. I am more interested in seeing the behavior around the automatic events and making sure we don't run into any issues there.

abhi-agg commented 6 months ago

@rosahbruno I had made the switch to automatically capture clicks (i.e. via data-glean-label attribute) at all places. However, during local testing I found out that clicks on some of the elements are not being recorded this way.

I investigated the issue and figured out that <div> tag is somehow creating problems in these cases (please see NavBar.js, Breadcrumbs.js and ReadMore.js for details). However, resolving this issue will take a bit more time. Therefore, in favor of not blocking the Dashboards work, I have done following:

  1. Used automatic click capturing at all possible places where it works
  2. Manually called click recording API where automatic click capturing fails and resolve this in a follow up task (filed https://bugzilla.mozilla.org/show_bug.cgi?id=1882747)

Please let me know if that sounds good with you.

abhi-agg commented 6 months ago

@rosahbruno I found out the issue and I believe I know how to fix it. I will update the details in the ticket https://bugzilla.mozilla.org/show_bug.cgi?id=1882747 today asap.