Closed
Bug 668842
Opened 13 years ago
Closed 13 years ago
Add locale to telemetry metadata
Categories
(Toolkit :: Telemetry, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
robert.strong.bugs
:
review+
zbraniecki
:
review+
|
Details | Diff | Splinter Review |
it would be great to have the locale code included in the ping so that we can check how normalized is the performance/memory across regions.
Assignee | ||
Comment 1•13 years ago
|
||
patch.
taras: can you help me with who's there to review it from the update side?
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment 2•13 years ago
|
||
Comment on attachment 543464 [details] [diff] [review]
patch v1
Rob should do the update review
Attachment #543464 -
Flags: review?(robert.bugzilla)
Comment 3•13 years ago
|
||
Comment on attachment 543464 [details] [diff] [review]
patch v1
Are you positive that you want the original installation's locale which is needed by app update and not the locale the user is using?
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#246
If you do, please bump the uuid.
Assignee | ||
Comment 4•13 years ago
|
||
Comment on attachment 543464 [details] [diff] [review]
patch v1
getting sr from pike will help. He said not to use general.useragent.locale .
Attachment #543464 -
Flags: superreview?(l10n)
Comment 5•13 years ago
|
||
Also, if this value is needed I'd prefer if it were added to application.ini and made part of XULAppInfo but I won't make that hold up this bug if you do need installation locale vs. the locale the user is using. In case you aren't sure which of the two you want I highly suspect you want the user's locale.
Comment 6•13 years ago
|
||
Another example
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#358
The question is whether you want the locale the user is using or the installation's locale which isn't really something pike can answer. App update originally used the same methods as the blocklist service and add-ons manager which caused problems when updating an installation where the user had changed their locale with a locale pack since the update is locale specific.
If you want the data across regions you shouldn't be using the locale used by app update and if there is an issue with using general.useragent.locale then the blocklist service and the add-ons manager both have that same issue. I highly suspect it is what you actually want.
Assignee | ||
Comment 7•13 years ago
|
||
I want the locale the user is using.
Pike: are you ok with going for general.useragent.locale pref here?
Comment 8•13 years ago
|
||
Comment on attachment 543464 [details] [diff] [review]
patch v1
Since you want the locale the user is using I'm going to minus this since this won't give you that.
Attachment #543464 -
Flags: review?(robert.bugzilla) → review-
Comment 9•13 years ago
|
||
Going back to the initial comment, the mapping between region and locale is so week, it doesn't really matter whether it's used or installed locale.
What are we really trying to measure/correlate?
PS: general.useragent.locale is not the locale the user is using. selected_locale for the global package from the xul registry is the most fail-safe way there.
Assignee | ||
Comment 10•13 years ago
|
||
> Going back to the initial comment, the mapping between region and locale is so week, it doesn't really matter whether it's used or installed locale.
I'd question that. First of all, in most cases there is a significant correlation, but then, even when there is less I'd say that the locale may be even more important than geographic location as a division line between different "ways" of using Internet. Especially for major locales (users that use zh-CN browser in Canada have probably quite similar habits to those that usezh-CN browser in China but very different from the ones that use en-US in Canada)
So, should I go for selected_locale for the global package? Do you have a pointer to the piece of code where we already use this way?
Comment 11•13 years ago
|
||
... or get the intl.accept_languages pref, just to add more options. Though that one may need more of a privacy review than the selected or installed locale.
I really don't know enough about what telemtry is sending and is going to send to make a call about what we're interested in.
PS: chrome reg code sample is at http://mxr.mozilla.org/mozilla-central/source/toolkit/components/urlformatter/nsURLFormatter.js#141
Assignee | ||
Comment 12•13 years ago
|
||
patch v2.
Ok, moved to chrome reg for global.
Attachment #543464 -
Attachment is obsolete: true
Attachment #543602 -
Flags: superreview?(l10n)
Attachment #543602 -
Flags: review?(robert.bugzilla)
Attachment #543464 -
Flags: superreview?(l10n)
Assignee | ||
Comment 13•13 years ago
|
||
@Axel: telemetry is collecting data about the browsing characteristic. Sample thing we're going to evaluate is:
- memory use
- number of websites open
- CPU usage
- cycle collector caused browser freezes
- startup time
- shutdown time
We'll collect data from statistically significant number of users and will be able to execute basic data mining on it - averages, standard deviation etc.
I'd like to have locale in the samples to profile the data against locales in case we'll be able to identify significant anomalies in certain locales and such insight may help us understanding how users of a given locale experience using Firefox (how fast/slow it is, how many extensions do they use etc.)
Comment 14•13 years ago
|
||
Comment on attachment 543602 [details] [diff] [review]
patch, v2
>diff --git a/toolkit/components/telemetry/TelemetryPing.js b/toolkit/components/telemetry/TelemetryPing.js
>--- a/toolkit/components/telemetry/TelemetryPing.js
>+++ b/toolkit/components/telemetry/TelemetryPing.js
>...
>@@ -132,16 +138,17 @@ function getMetadata(reason) {
> let ret = {
> reason: reason,
> OS: ai.OS,
> appID: ai.ID,
> appVersion: ai.version,
> appName: ai.name,
> appBuildID: ai.appBuildID,
> platformBuildID: ai.platformBuildID,
>+ locale: getLocale(),
> };
we no longer log the trailing comma so this is ok.
Attachment #543602 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 15•13 years ago
|
||
Comment on attachment 543602 [details] [diff] [review]
patch, v2
additional r=tglek on IRC
Attachment #543602 -
Flags: superreview?(l10n) → review+
Assignee | ||
Comment 16•13 years ago
|
||
landed on central: http://hg.mozilla.org/mozilla-central/rev/0ac4818e5b4b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•