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

total_uri_count should be added to the metrics ping #4456

Closed
boek opened this issue Aug 1, 2019 · 11 comments
Closed

total_uri_count should be added to the metrics ping #4456

boek opened this issue Aug 1, 2019 · 11 comments
Assignees
@sblatz
Labels
E3 Estimation Point: average, 2 - 3 days Feature:Telemetry
Milestone
v3.1

Comments

@boek
Copy link
Contributor

boek commented Aug 1, 2019 •
edited by data-sync-user
Loading

┆Issue is synchronized with this Jira Task
@boek boek added 🐞 bug Crashes, Something isn't working, .. Feature:Telemetry labels Aug 1, 2019
@vesta0 vesta0 added must and removed 🐞 bug Crashes, Something isn't working, .. must labels Oct 3, 2019
@boek boek added E1 Estimation Point: trivial, less than half day and removed needs:group-triage labels Oct 8, 2019
@sblatz sblatz self-assigned this Oct 10, 2019
sblatz added a commit to sblatz/fenix that referenced this issue Oct 11, 2019
@sblatz
For mozilla-mobile#4456: Adds total_uri_count to metrics core ping
14cb713
sblatz added a commit to sblatz/fenix that referenced this issue Oct 14, 2019
@sblatz
For mozilla-mobile#4456: Adds total_uri_count to metrics core ping
283cb59
sblatz added a commit to sblatz/fenix that referenced this issue Oct 14, 2019
@sblatz
For mozilla-mobile#4456: Adds total_uri_count to metrics core ping
e8ddf9c
sblatz added a commit to sblatz/fenix that referenced this issue Oct 28, 2019
@sblatz
For mozilla-mobile#4456: Adds total_uri_count to metrics core ping
ab062d2
liuche pushed a commit that referenced this issue Oct 30, 2019
@sblatz @liuche
For #4456: Adds total_uri_count to metrics core ping (#6003)
8549b80
@sblatz sblatz added the eng:qa:needed QA Needed label Oct 30, 2019
@AndiAJ AndiAJ removed the eng:qa:needed QA Needed label Oct 31, 2019
@AndiAJ
Copy link
Collaborator

AndiAJ commented Oct 31, 2019

Hi, I've just checked on the latest Nightly Build #13040608 from 31/10 using a Google Pixel 3a XL (Android 9) and noticed the following:

• Performed 3 searches
The baseline ping 294b7f7a-c381-483f-b7b8-9cdcc0a3a1c0 shows "events.total_uri_count": 4
The metrics ping shows bb9612e8-a4c9-4e85-9cee-caebc871798a shows "metrics.total_uri_count": "0"
Logcat
Glean dashboard

• Performed 6 searches
The baseline 6b6cf012-661e-4fd3-b505-452b8565a0cc ping shows "events.total_uri_count": 7
The metrics ping shows 5f94bcc1-9239-4ed9-bb99-57c3750d1330 shows "metrics.total_uri_count": "0"
Logcat
Glean Dashboard

@sblatz please help me out with the following:
Why is the total_uri_count = to the numer of searches + 1? in both cases
Why the "metrics.total_uri_count" isn't displayed in the logs from Android Studio?

To generate the metrics ping I've force stopped Fenix, set the date to T+1 (01/11) and set the time to 03:59AM. At around 4AM the metrics ping is generated and sent.
Based on the "client_id" & the "first_run_date" in both cases the metrics is properly generated.

Please review the above. 😊
I'll remove the QA needed label until further clarifications.

@sblatz
Copy link
Contributor

sblatz commented Nov 6, 2019

Why is the total_uri_count = to the numer of searches + 1? in both cases

I think @baron-severin may be able to answer this better?

Why the "metrics.total_uri_count" isn't displayed in the logs from Android Studio?

Investigating this one.

@sblatz sblatz added E3 Estimation Point: average, 2 - 3 days and removed E1 Estimation Point: trivial, less than half day labels Nov 6, 2019
@severinrudie
Copy link
Contributor

Why is the total_uri_count = to the numer of searches + 1? in both cases

I think @baron-severin may be able to answer this better?

I don't actually have any context on this one. I noticed a related bug, but don't know anything else about this ping.

@sblatz
Copy link
Contributor

sblatz commented Nov 6, 2019

@boek maybe? I think he was the one who added this :)

sblatz added a commit to sblatz/fenix that referenced this issue Nov 6, 2019
@sblatz
For mozilla-mobile#4456: Updates total_uri_count upload
15d946f
@severinrudie severinrudie mentioned this issue Nov 6, 2019
Closed
30 tasks
liuche pushed a commit that referenced this issue Nov 7, 2019
@sblatz @liuche
For #4456: Updates total_uri_count upload (#6482)
284f3cf
@sblatz sblatz added the eng:qa:needed QA Needed label Nov 7, 2019
@sblatz
Copy link
Contributor

sblatz commented Nov 7, 2019

I have addressed "metrics.total_uri_count": being 0 and marking this ready for qa needed again.

I'm not sure why the total_uri_count is one plus the search number, though. We can address that in another ticket?

@sblatz sblatz mentioned this issue Nov 7, 2019
Closed
@severinrudie severinrudie added this to the v3.0 milestone Nov 7, 2019
@liuche liuche modified the milestones: v3.0, v3.1 Nov 8, 2019
@AndiAJ AndiAJ removed the eng:qa:needed QA Needed label Nov 12, 2019
@AndiAJ
Copy link
Collaborator

AndiAJ commented Nov 12, 2019

Hi, I've just re-tested on Nightly Build #13160607 from 12/11 using a Google Pixel 3a XL (Android 9) phone and learned the following:

Performed 3 searches and generated the metrics ping:
"metrics.total_uri_count": "4"

Performed 6 searches and generated the metrics ping:
"metrics.total_uri_count": "7"

Logcat
Glean dashboard

@sblatz I think we should first try to solve the mystery behind #6499 from the baseline pings and afterwards re-test this matter.
Please share your thoughts on this. ☺️

@Dexterp37 - While testing this matter I've noticed that "metrics.total_uri_count": is displayed only when checking the metrics pings in the Glean dashboard and not in the logcat from Android Studio.
Do you know why is this happening? ☺️

I'll remove the QA needed label until further clarifications.

@Dexterp37
Copy link
Contributor

Dexterp37 commented Nov 12, 2019 •
edited
Loading

@Dexterp37 - While testing this matter I've noticed that "metrics.total_uri_count": is displayed only when checking the metrics pings in the Glean dashboard and not in the logcat from Android Studio.
Do you know why is this happening? ☺️

@AndiAJ what is the id of the specific ping that shows up on the dashboard but not in the logs? I randomly picked 52c2e975-6998-4ac2-bc94-46fcf189d0c0 from the debug view and it shows up in the logs.

Update After talking to @AndiAJ , we found that Android logcat is limiting the length of log messages and thus cropping local ping contents in the logs. I filed this bug. QA can still be performed using the glean ping view.

@sblatz
Copy link
Contributor

sblatz commented Nov 12, 2019

@AndiAJ can you tell me what steps you're performing to do these searches? I've tested locally and I seem to be getting the right number of total_uri_count (3 searches = 3 for total_uri_count).

@sblatz sblatz mentioned this issue Nov 13, 2019
Closed
@liuche
Copy link
Contributor

liuche commented Nov 21, 2019

Just a bump here @AndiAJ ^

@sblatz
Copy link
Contributor

sblatz commented Nov 22, 2019 •
edited
Loading

@liuche We're going to need to fix this bug mentioned by Glean team: #6577. We should pull that into a sprint if we want to get this ticket closed.

@boek
Copy link
Contributor Author

boek commented Nov 26, 2019

Going to track in #6577

@boek boek closed this as completed Nov 26, 2019
@Mugurell Mugurell mentioned this issue Feb 11, 2020
Merged
4 tasks
boek pushed a commit that referenced this issue Feb 13, 2020
@Mugurell @boek
For #6577 - Remove the total_uri_count Glean StringMetricType
ad1144c 
This was added as a duplicate to an already existing `total_uri_count`
CounterMetricType in #4456.
boek pushed a commit that referenced this issue Feb 13, 2020
@Mugurell @boek
For #6577 - Add total_uri_count to the Glean metrics ping
b231afb 
Re-apply the change for #4456.
Keep `total_uri_count` as a CounterMetricType and let Glean manage it's
persistence and reset time (resets with each metrics ping sent).
@tdsmith tdsmith mentioned this issue Feb 20, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@sblatz sblatz

Labels
E3 Estimation Point: average, 2 - 3 days Feature:Telemetry
Projects
None yet
Milestone
v3.1
Development

No branches or pull requests
7 participants
@Dexterp37 @liuche @boek @sblatz @severinrudie @vesta0 @AndiAJ