GEOLOCATION_ACCURACY histogram scale difficult to read

RESOLVED FIXED in mozilla38

Status

()

Core
Geolocation
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: garvan, Assigned: garvan)

Tracking

unspecified
mozilla38
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
This telemetry histogram is linear, but should be exponential, and has too many buckets, 90% of them are empty.
(Assignee)

Comment 1

4 years ago
Created attachment 8546185 [details] [diff] [review]
Scale change in Histograms.json
Attachment #8546185 - Flags: review?(hschlichting)

Comment 2

4 years ago
Comment on attachment 8546185 [details] [diff] [review]
Scale change in Histograms.json

>   "GEOLOCATION_ACCURACY": {

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1116404#c17 - we have to rename the telemetry probe when changing any of its parameters, especially its type.

I'm bad at names, maybe GEOLOCATION_ACCURACY_EXPONENTIAL or _NEW :)

>     "expires_in_version": "default",
>-    "kind": "linear",
>+    "kind": "exponential",
>     "high": "18000",

Can we increase the high value to 100km? We currently get mean values of 80km+ for this probe and I'd like to have some more insight into the long tail here. Or rather I'd like to be able to tell the difference between GeoIP city level (10-50km) and GeoIP country level (hundreds of kilometers).

>-    "n_buckets": 200,
>+    "n_buckets": 50,
>     "description": "Location accuracy"
Attachment #8546185 - Flags: review?(hschlichting) → review-
(Assignee)

Comment 3

4 years ago
Created attachment 8547660 [details] [diff] [review]
Scale change in Histograms.json

Thanks Hanno, addressed review comments. Changed scale to 100km and new name for probe.
Attachment #8546185 - Attachment is obsolete: true
Attachment #8547660 - Flags: review?(hschlichting)

Comment 4

4 years ago
Comment on attachment 8547660 [details] [diff] [review]
Scale change in Histograms.json

Looks good, except wouldn't we have to change the name in http://mxr.mozilla.org/mozilla-central/source/dom/geolocation/nsGeolocation.cpp#1261 as well?
(Assignee)

Comment 5

4 years ago
The review process clearly works :). Yes, I'll do that.
(Assignee)

Comment 6

4 years ago
Created attachment 8547808 [details] [diff] [review]
Scale change in Histograms.json and name change in nsGeoloc.cpp

Updated constant in cpp as per review.

Try link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae4465578c27
Attachment #8547660 - Attachment is obsolete: true
Attachment #8547660 - Flags: review?(hschlichting)
Attachment #8547808 - Flags: review?(hschlichting)

Comment 7

4 years ago
Comment on attachment 8547808 [details] [diff] [review]
Scale change in Histograms.json and name change in nsGeoloc.cpp

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

Looks good now, third time's the charm :)
Attachment #8547808 - Flags: review?(hschlichting) → review+
(Assignee)

Comment 8

4 years ago
Repeating the try link from comment 6 for sheriff: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae4465578c27
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/ad469f83659d
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ad469f83659d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.