Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Count PDF Downloads #24075

Closed
mcarare opened this issue Mar 3, 2022 · 17 comments
Closed

Count PDF Downloads #24075

mcarare opened this issue Mar 3, 2022 · 17 comments

Comments

@mcarare
Copy link
Contributor

mcarare commented Mar 3, 2022

https://mozilla-hub.atlassian.net/browse/FNXV2-19514

┆Issue is synchronized with this Jira Task

@mcarare mcarare self-assigned this Mar 3, 2022
@github-actions github-actions bot added the needs:triage Issue needs triage label Mar 3, 2022
mcarare added a commit to mcarare/fenix that referenced this issue Mar 3, 2022
mcarare added a commit to mcarare/fenix that referenced this issue Mar 3, 2022
mcarare added a commit to mcarare/fenix that referenced this issue Mar 3, 2022
mergify bot pushed a commit that referenced this issue Mar 4, 2022
mergify bot pushed a commit that referenced this issue Mar 4, 2022
@mcarare mcarare added eng:qa:needed QA Needed and removed needs:triage Issue needs triage labels Mar 4, 2022
@AndiAJ AndiAJ removed the eng:qa:needed QA Needed label Mar 10, 2022
@AndiAJ
Copy link
Collaborator

AndiAJ commented Mar 10, 2022

Hi, I've just checked this matter on both Nightly 100.0a1 and Firefox 99.0.0 Beta 1 using a OnePlus A3 (Android 6.0.1) and neither pdf_download_count nor downloaded_pdf_open_count get generated in the metrics ping.

@mcarare please review and share your thoughts ☺️

@mcarare
Copy link
Contributor Author

mcarare commented Mar 10, 2022

@mcarare mcarare added the eng:qa:needed QA Needed label Mar 10, 2022
pedroldk pushed a commit to pedroldk/fenix that referenced this issue Mar 22, 2022
pedroldk pushed a commit to pedroldk/fenix that referenced this issue Mar 22, 2022
@AndiAJ
Copy link
Collaborator

AndiAJ commented Mar 24, 2022

Hi @mcarare , I've re-checked with fellow colleague @SoftVision-LorandJanos this matter on the latest Nightly build 100.0a1 using a Google Pixel 3a (Android 11) and still neither pdf_download_count nor downloaded_pdf_open_count get generated in the metrics ping.

@Dexterp37 or @badboy could you please share your thoughts ? ☺️
Thanks!

@Dexterp37
Copy link
Contributor

Hi @mcarare , I've re-checked with fellow colleague @SoftVision-LorandJanos this matter on the latest Nightly build 100.0a1 using a Google Pixel 3a (Android 11) and still neither pdf_download_count nor downloaded_pdf_open_count get generated in the metrics ping.

I can confirm that data is not showing up in the tables. Looks like it's not being recorded by Fenix.

@Dexterp37 or @badboy could you please share your thoughts ? ☺️ Thanks!

@mcarare are we sure the code for recording the data is being called?
@AndiAJ what is the STR you're using?

If data is not showing up in the ping, but other metrics are, there's a very high likelihood that the Glean APIs are not being called at all :) Let me know if that's the case, if not, we can dive into this together.

@mcarare
Copy link
Contributor Author

mcarare commented Mar 24, 2022

@Dexterp37 If the metric is not recorded by Fenix where is the data from looker coming from?

https://mozilla.cloud.looker.com/explore/fenix/metrics?qid=mtURu0BHGg1c6SuA6EtvVe&toggle=fil,vis
https://mozilla.cloud.looker.com/explore/fenix/metrics?qid=chYApJQcKaU6CwmckivNr8&toggle=fil,vis

Do null values also show up there?

@Dexterp37
Copy link
Contributor

@mcarare whoops, you are indeed correct, sorry for the confusion. The previous query is now showing data (I guess I was too hasty :-) ).

So, given that data is flowing in, this seems more like a validation issue. @AndiAJ , what is the STR you are using to test?

@AndiAJ
Copy link
Collaborator

AndiAJ commented Mar 28, 2022

Hi @Dexterp37 👋🏽

  1. Perform a search "Download sample pdf" (while Google is set as default search engine)
  2. Open one of the featured results
  3. Download pdf file
  4. Open the pdf after download
  5. Generate the metrics ping (Force stop Fenix, Set date to D+1, Set time to 03:59 AM, Re-open fenix using the cmd, at 04:00 AM the ping is generated)

Tried with various websites and neither pdf_download_count nor downloaded_pdf_open_count are generated in the metrics ping

@Dexterp37
Copy link
Contributor

Hey @AndiAJ, I would recommend starting Fenix with the command to tag pings for the debug vew or log them in order to make sure that the metric doesn't go in a metrics ping that's generated before your step 5.

@AndiAJ
Copy link
Collaborator

AndiAJ commented Mar 29, 2022

Hey @Dexterp37, indeed I'm always starting Fenix (freshly installed or having a fresh profile) using the cmd

@Dexterp37
Copy link
Contributor

@AndiAJ unfortunately I'm not sure how to help more with this on this ticket. Would you like to meet over zoom for about 30 minutes to figure it out? Otherwise, happy to take it async . If you could do a screencap that would be good!

@mcarare
Copy link
Contributor Author

mcarare commented Mar 29, 2022

I tested using terminal commands and I managed to record pdf opening and downloading:

{"ping_info":{"seq":9,"start_time": "2022-03-29T16:18+03:00", "end_time": "2022-03-29T16:28+03:00"},"client_info":{"telemetry_sdk_build": "44.0.0", "android_sdk_version": "29", "device_model": "Nokia 7 plus", "app_channel": "debug", "app_display_version": "1.0.2213", "locale": "en-US", "app_build": "1", "os_version": "10", "os": "Android", "architecture": "arm64-v8a", "device_manufacturer": "HMD Global", "first_run_date": "2022-03-01+02:00", "build_date": "2022-03-24T10:40:01+00:00", "client_id": "feeedf5c-86f0-4161-a7dc-f125d57b158d"},"counter":{"glean.upload.pending_pings":2,"places_manager.write_query_count":21,"places_manager.read_query_count":1,"glean.validation.foreground_count":9,"metrics.tabs_open_count":3,"metrics.top_sites_count":3,"downloads.pdf_download_count":2,"power.total_cpu_time_ms":36019,"downloads.downloaded_pdf_open_count":1}

https://debug-ping-preview.firebaseapp.com/pings/mcarare
Mar 29, 2022, 16:28:16

Note: I used sendPing command

@mcarare
Copy link
Contributor Author

mcarare commented Mar 29, 2022

@Dexterp37 Is the fact that the lifetime of the counter is set to lifetime: application making the testing more difficult?
On a side note, we use lifetime: application with the goal of avoiding double-counting some user interactions. Not sure where this info originally came from but is this lifetime option appropriate in this current use case?
Thank you!

@Dexterp37
Copy link
Contributor

@Dexterp37 Is the fact that the lifetime of the counter is set to lifetime: application making the testing more difficult?

I think that's orthogonal to the testing. As documented, this should actually make it easier to test: metrics with that lifetime, once set, stick there for the whole lifetime of the application (until the next restart), therefore the name :)

On a side note, we use lifetime: application with the goal of avoiding double-counting some user interactions. Not sure where this info originally came from but is this lifetime option appropriate in this current use case? Thank you!

I'm not sure this would help with preventing double counting: could you please add some more information? A metric lifetime exclusively relates to when a metric gets cleared from the Glean internal storage (this could happen after a ping is submitted, or after the application is restarted, or never, like for example for the client id). See our docs.

Happy to chat more about this, @mcarare , to shed some light on it!

@mcarare
Copy link
Contributor Author

mcarare commented Mar 30, 2022

@Dexterp37 I do not have any context for the double-counting comment, I just remember seeing it once in our code base.
I have seen the documentation, it is very helpful!

@Dexterp37
Copy link
Contributor

@Dexterp37 I do not have any context for the double-counting comment, I just remember seeing it once in our code base. I have seen the documentation, it is very helpful!

Glad to hear :-) We're on the same timezone, so feel free to reach out in case you want to talk more about any detail related to the Glean usage!

@mcarare
Copy link
Contributor Author

mcarare commented Apr 5, 2022

@AndiAJ can you retest this, maybe using sendPing command? Thank you!

@AndiAJ
Copy link
Collaborator

AndiAJ commented Apr 5, 2022

Hi, verified as fixed on the latest Nightly 100.0a1 build using a Google Pixel 3a (Android 11)

✅ Download and open 1 PDF file - Force generate the metrics ping 050025f9-366b-4a91-a9c1-b055b8d2da31

"downloads.downloaded_pdf_open_count": 1,
"downloads.pdf_download_count": 1,

✅ Download and open 2 more PDF files - Force generate the metrics ping 7f77bb0e-c32f-4955-a249-919560a2cf9a

 "downloads.pdf_download_count": 3,
"downloads.downloaded_pdf_open_count": 3,

⏳ Force stop Fenix and set the date to D+1
✅ Download 2 pdf files and open only one of them - Force generate the metrics ping 63461183-41af-4a86-8502-74ce8405ed79

"downloads.downloaded_pdf_open_count": 1,
"downloads.pdf_download_count": 2

Glean dashboard
Logcat

Thanks @Dexterp37 and @mcarare for helping me out! ☺️

@AndiAJ AndiAJ closed this as completed Apr 5, 2022
@AndiAJ AndiAJ added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Apr 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants