Closed Bug 668842 Opened 13 years ago Closed 13 years ago

Add locale to telemetry metadata

Categories

(Toolkit :: Telemetry, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch patch v1 (obsolete) — Splinter Review
patch.

taras: can you help me with who's there to review it from the update side?
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment on attachment 543464 [details] [diff] [review]
patch v1

Rob should do the update review
Attachment #543464 - Flags: review?(robert.bugzilla)
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.
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)
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.
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.
I want the locale the user is using.

Pike: are you ok with going for general.useragent.locale pref here?
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-
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.
> 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?
... 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
Attached patch patch, v2Splinter Review
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)
@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 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+
Comment on attachment 543602 [details] [diff] [review]
patch, v2

additional r=tglek on IRC
Attachment #543602 - Flags: superreview?(l10n) → review+
landed on central: http://hg.mozilla.org/mozilla-central/rev/0ac4818e5b4b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Blocks: 733152
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: