mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.49k stars 1.28k forks source link

[Telemetry] Add uri_count to ping #1301

Closed bbinto closed 4 years ago

bbinto commented 5 years ago

Similar to Focus: https://github.com/mozilla-mobile/focus-android/blob/master/docs/Telemetry.md#uri-count

What questions will you answer with this data?

Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements?

What probes (suggested, if applicable)

Dependencies (Added by PM and Eng)

Acceptance Criteria (Added by PM)

┆Issue is synchronized with this Jira Task

boek commented 5 years ago

This can be done pretty easily with the counter metric type

bsurd commented 5 years ago

The total_uri_count is not displayed in the events when sending telemetry. Looked in both baseline and events JSON files and the data wasn't available anywhere.

Started the app with glean via cmd and I've tried a scenario where I visited multiple different pages in different tabs, one where I visited multiple pages in the same tab and one where I navigated from one link to another in the same tab.

Please let me know if I've missed anything.

colintheshots commented 5 years ago

total_uri_count should be in the metrics ping. I noticed a new regression from last week that might cause some URI counts to not get counted and I'm fixing it.

    val totalUriCount: CounterMetricType by lazy {
        CounterMetricType(
            name = "total_uri_count",
            category = "events",
            sendInPings = listOf("metrics"),
            lifetime = Lifetime.Ping,
            disabled = false
        )
    }
colintheshots commented 5 years ago

It should now be moved to the baseline ping.

vesta0 commented 5 years ago

@colintheshots I re-added the QA needed label based on your comment.

AndiAJ commented 5 years ago

Hi all, the uri_count total_uri_count is now sent with the baseline ping, it works.

Please find attached and review the following:

•1 search performed: Logcat: UriCount3.txt Glean Dashboard:https://debug-ping-preview.firebaseapp.com/pings/2311bde3-47fc-4272-9a8b-b1cce4e7eaf5/aj-uritest-04

•4 searches performed: Logcat: UriCount1.txt Glean Dashboard: Glean Dashboard:https://debug-ping-preview.firebaseapp.com/pings/2311bde3-47fc-4272-9a8b-b1cce4e7eaf5/aj-uritest-00

•8 searches performed: Logcat: UriCount2.txt Glean Dashboard: (second baseline) https://debug-ping-preview.firebaseapp.com/pings/2311bde3-47fc-4272-9a8b-b1cce4e7eaf5/aj-uritest-03

Before closing this ticket, in order to have a better understanding regarding thesearch_count VS uri_count I would like to ask, why the uri_count is always higher than the search_count ? what is the difference between the two? Please review the above attached logs and confirm if from your point of view the uri_count works properly.

Until then, based on this comment I'll remove the QA needed label.

sblatz commented 5 years ago

@colintheshots or @boek are you able to explain this behavior?

boek commented 5 years ago

Looks like there is a bug that is causing UriOpened to get called an extra time in a session. I filed a follow up bug #3676 and we can track/fix it there.

boek commented 5 years ago

@WhoIsAndi is that good enough to close this issue?

AndiAJ commented 5 years ago

@boek sure thing ! I'll go ahead and close this issue.