Closed Bug 1503671 Opened 6 years ago Closed 6 years ago

Added Probe Dictionary link in about:telemetry to assist new users in understanding histograms.

Categories

(Toolkit :: Telemetry, defect)

Product:
Toolkit
Component:
Telemetry
Version:
63 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla65
Tracking Flags:
Tracking Status
firefox65 --- verified
firefox66 --- verified

People

(Reporter: e412byoy7, Assigned: e412byoy7)

Details

Attachments

(2 files, 3 obsolete files)

Screenshot of what about:telemetry looks like with the patch applied
49.62 KB, image/png
Details
0001-bug-1503671-Add-to-about-telemetry-a-link-to-the-Pro.patch
2.59 KB, patch
chutten
: review+
RyanVM
: approval-mozilla-beta-
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

See pull request: https://github.com/mozilla/gecko-dev/pull/139


Actual results:

no link is there currently to direct to the probe dictionary.


Expected results:

Should've been easier to find the probe dictionary.
Component: Untriaged → Telemetry
Product: Firefox → Toolkit
Summary: Added Probe Dictionary link in about:telemetry to assist new users in understanding histograms → Added Probe Dictionary link in about:telemetry to assist new users in understanding histograms. r=chutten
Assignee: nobody → danielboontje
Status: NEW → ASSIGNED
Summary: Added Probe Dictionary link in about:telemetry to assist new users in understanding histograms. r=chutten → Added Probe Dictionary link in about:telemetry to assist new users in understanding histograms.
Comment on attachment 9021629 [details] [diff] [review]
edited the description line to be shorter, more easily understandable.

changed 3 files, created "aboutTelemetry.telemetryProbeDictionary", added description

Review of attachment 9021629 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch! We have a few things to do before it will be ready to land.

The commit message needs to be something like "bug 1503671 - Add Probe Dictionary link to about:telemetry r=chutten"

::: toolkit/locales/en-US/chrome/global/aboutTelemetry.dtd
@@ +23,4 @@
>  <!ENTITY aboutTelemetry.firefoxDataDoc "The <a>Firefox Data Documentation</a> contains guides about how to work with our data tools.">
>  <!ENTITY aboutTelemetry.telemetryClientDoc "The <a>Firefox Telemetry client documentation</a> includes definitions for concepts, API documentation and data references.">
>  <!ENTITY aboutTelemetry.telemetryDashboard "The <a>Telemetry dashboards</a> allow you to visualize the data Mozilla receives via Telemetry.">
> +<!ENTITY aboutTelemetry.telemetryProbeDictionary "The <a>Probe Dictionary</a> helps you to understand what each of the histogram items means, clicking on an item in the Probe Dictionary shows you a description.">

We need to keep these short. I recommend "The <a>Probe Dictionary</a> provides details and descriptions for the probes collected by Telemetry."
Attachment #9021629 - Flags: review?(chutten) → review-
Comment on attachment 9021629 [details] [diff] [review]
edited the description line to be shorter, more easily understandable.

changed 3 files, created "aboutTelemetry.telemetryProbeDictionary", added description

Review of attachment 9021629 [details] [diff] [review]:
-----------------------------------------------------------------

To make it easier for new about:telemetry users to understand the items in the "histograms" menu, I've created "aboutTelemetry.telemetryProbeDictionary" and added the following description:
"The Probe Dictionary helps you to understand what each of the histogram items does, clicking on an item in the Probe Dictionary shows you a description."
Attachment #9021629 - Flags: review- → review?(chutten)
Comment on attachment 9021629 [details] [diff] [review]
edited the description line to be shorter, more easily understandable.

changed 3 files, created "aboutTelemetry.telemetryProbeDictionary", added description

Review of attachment 9021629 [details] [diff] [review]:
-----------------------------------------------------------------

Resetting the flag.
Attachment #9021629 - Flags: review?(chutten) → review-
To make it easier for new about:telemetry users to understand the items in the "histograms" menu, I've created "aboutTelemetry.telemetryProbeDictionary" and added a description sentence (thanks to chutten for his suggestion of a better description)
Attachment #9021629 - Attachment is obsolete: true
Attachment #9021646 - Flags: review?(chutten)
Comment on attachment 9021646 [details] [diff] [review]
edited the description line to be shorter, more easily understandable.  [changed 3 files, created "aboutTelemetry.telemetryProbeDictionary", added description]

Review of attachment 9021646 [details] [diff] [review]:
-----------------------------------------------------------------

The commit message is "Update aboutTelemetry.xhtml

added aboutTelemetry.telemetryProbeDictionary"

which isn't in the needed format. Also, the patch may not apply as the change to aboutTelemetry.dtd is repeated twice in the patch file.
Attachment #9021646 - Flags: review?(chutten) → review-
This is the same patch as before, but condensed somewhat and with a reworded commit message.

Daniel, can you look this over and make sure everything looks good to you?
Flags: needinfo?(danielboontje)
Attachment #9021870 - Flags: review+
Attachment #9021651 - Attachment is obsolete: true
Attachment #9021651 - Flags: review?(chutten)
Yes looking good!
Flags: needinfo?(danielboontje) → needinfo?(chutten)
No need to needinfo if there's no question :)
Oh sorry. Now we wait for this to be included into Nightly or what happens next?
Now I'll mark this bug for the Sheriffs' attention to be included the next time they merge. Once there's a comment here saying it has landed in mozilla-central, then you'll know your code will be in the following Nightly.

Good job!

Now, if you're looking for more ways to help I recommend going through the documentation here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

It'll instruct you on how to get a code checkout and build running. It'll probably also show you how to use Phabricator for reviews, which gives us a way to get code into the tree without bothering the sheriffs each time. It's a bit of a process, especially if you're used to lighter-weight issue,code,review,merge cycles at smaller projects, but think of it as a free education in how large software projects work :)

Thank you again for your contribution!
Flags: needinfo?(chutten)
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bbfbefd208f
Add to about:telemetry a link to the Probe Dictionary r=chutten
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9bbfbefd208f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Is there a way to see when this will land in Nightly? :)
Flags: needinfo?(dvarga)
ah I think it is the progress on this page? https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=2dd516cee24f58f9b833d9db676c5752b2ba2cd8 When this is at 100%, will we receive the Nightly update? :)
I think in ~12h in the next merge.
Flags: needinfo?(dvarga)
Thanks so much, also for the push! :)
If you look at https://hg.mozilla.org/mozilla-central/rev/9bbfbefd208f it tells you the first build with your change is Nightly build id 20181102100039 which is currently the latest release (as you can tell by clicking on its link which will direct you to 
https://hg.mozilla.org/mozilla-central/firefoxreleases#a6493a0b53a3nightlylinux3220181102100039)

Congratulations, code you wrote is shipping to Firefox users!

According to a quick query I did on the backend, it's in the hands of at least 450 users already (and the data I query is always a little out of date, so the real number is actually higher). By Monday it should reach saturation across the Nightly population. On December 10 it will ship to Beta. Then on January 28 it will ship to Release. (https://wiki.mozilla.org/Release_Management/Calendar)
OH YES, I just got the update, now it's IN! :D Thanks sooo much for your help and patience with my first pull request ("push"?) ! :) And thanks for congratulating me! :D I'm just sooo glad that "my code" didn't break the Nightly, as I just found out that there's a "try" thing on treeherder where one can test it privately first :/ :)
Can't wait for it to ship to Beta! :D I wonder, why will it take till 12.10.18 if according to the google calendar we have 2 Beta build releases every week? (this link https://goo.gl/uhWPdm )

Btw, is the query public, can I for myself find the approximate number of current revision a6493a0b53a3 users somewhere? :)
Hope you find the time to answer my questions from this comment :P
OOPS, I just noticed! We seem to have forgotten to add translation! I use Nightly in german (I'm german) and my "push" (?) of this code shows in english in my Nightly instead of german :P (I expected the sentence "The Probe Dictionary provides details and descriptions for the probes collected by Telemetry." to show up in german just like the other sentences in about:telemetry)
Flags: needinfo?(chutten)
Our localization (l10n) contributors will be helping out with translations. Since you speak German and you're enthusiastic, maybe you'll be interested in helping them out: https://l10n.mozilla.org/

Beta only picks up things when we ask for them to be included. If you'd like you can flag your patch approval-mozilla-beta? (in the Details) and fill out the form. Then someone (possibly the Release Manager) will come along and determine if it's a good fit to include in Beta. This patch is small, self-contained, and verified to be working Nightly so it's a decent candidate. It -does- have a translation requirement, though, so that may complicate things. Doesn't hurt to ask, though.

The number of users running different versions isn't something public. It's something ad hoc I calculated using internal tools. The Firefox Public Data Report (https://data.firefox.com/) does have monthly and weekly user population figures if you'd like to get a sense of scale, though.
Flags: needinfo?(chutten)
Comment on attachment 9021870 [details] [diff] [review]
0001-bug-1503671-Add-to-about-telemetry-a-link-to-the-Pro.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: 

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): not risky as only a text/description addition in about:telemetry page.

String changes made/needed:
Attachment #9021870 - Flags: approval-mozilla-beta?
Done! :)

about l10n, thanks, I just checked, it appears they only translate for FF Beta, not Nightly... so it'll have to wait :P (the english sentence would be in here https://pontoon.mozilla.org/de/firefox/toolkit/chrome/global/aboutTelemetry.dtd/?string=81255 )

Yes that's what I thought aswell, as it's self-contained the risk of something going wrong is very minor.
Not sure why "String changes made/needed" was left blank in comment 25 when this patch is clearly adding a new string. NI Flod for approval since Beta is supposed to be string frozen.
Flags: needinfo?(francesco.lodolo)
Sorry but I don't see a good reason to uplift this change, it should ride the trains with 65.


P.S. note that the request is coming from a contributor at their first patch
Flags: needinfo?(francesco.lodolo)
Comment on attachment 9021870 [details] [diff] [review]
0001-bug-1503671-Add-to-about-telemetry-a-link-to-the-Pro.patch

Per comment 28, this can ship in 65.
Attachment #9021870 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
@Daniel

To give you more background: Beta is string frozen, and only particularly urgent patches are allowed to be uplifted if they contain string changes (that includes adding new strings).

And if you're interested in contributing to localization, see https://pontoon.mozilla.org/de/ ;-)
Alright, no big deal, can't wait for it to get into Beta mid december then :) And as I wrote in Comment 26 : "it appears they only translate for FF Beta, not Nightly... https://pontoon.mozilla.org/de/firefox/toolkit/chrome/global/aboutTelemetry.dtd/?string=81255" (Pontoon)

Ryan I'm sorry, as there was only 1 line available in "string changes made" I didn't know whether I should post several lines of code into that line.
Any idea which ESR version will most likely contain this addition? :)
According to https://wiki.mozilla.org/Release_Management/Calendar the next ESR will be 68.
Flags: qe-verify+
Verified the Probe dictionary text and link addition in about:telemetry + the redirect to https://telemetry.mozilla.org/probe-dictionary/ for:
-65.0b8 20190103150357
-66.0a1 20190104093221
 on Windows 10/ Ubuntu 16.04 / OSX 10.14.2.
Status: RESOLVED → VERIFIED
status-firefox65: fixed → verified
status-firefox66: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: