860571 - Pings for Firefox OS

Status: VERIFIED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Anurag Phadke[:[e-mail]]]

Initial Questions:

Project/Feature Name: Pings for Firefox OS
Tracking  ID:
Description:
We need the following "MUST HAVE's" to provide a seamless and customized experience to users using Firefox OS (B2G)

1. Operator Code (accessible via Gecko)
2. Device Type / model info (accessible via Gecko)
3. Major OS Version (part of User-Agent string)
Additional Information:
Etherpad @ https://etherpad.mozilla.org/YX73peq4WX

Urgency: a week
Key Initiative: Firefox Platform
Release Date: 2013-07-01
Project Status: development
Mozilla Data: No
New or Change: Existing
Mozilla Project: FirefoxOS
Mozilla Related: Firefox OS nightly update ping
Separate Party: No

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: wip patch

This patch add headers to the marketplace manifest ping. It's currently sending different headers, one per field. If it's easier for the backend to only have one and parse the value, I can change what we send.

For now we send:
X-MOZ-B2G-DEVICE
X-MOZ-B2G-MCC
X-MOZ-B2G-MNC
X-MOZ-B2G-SHORTNAME (short operator name)
X-MOZ-B2G-LONGNAME (long operator name)

I'm not sure we want or even need the last 2, but let me know.

Comment 2

[Wil Clouser [:clouserw]]

On our massive sites today we currently parse webserver logs offline to gather statistics.  Headers won't be a part of those logs by default, but they are certainly cleaner than what we normally do (stuffing this data into a query string).  I'm CCing ops to make sure they have the ability to log these headers.  If log parsing is the way we're going with this, ops + metrics will need to decide on a format.

Comment 3

[Anurag Phadke[:[e-mail]]]

The log format is going to be similar to blocklist ping / AMO. We (metrics) and IT have a predefined log format that works for both of us. I will file a bug tomorrow to ensure we are getting the correct log format

Comment 4

[Anurag Phadke[:[e-mail]]]

any update on the code review? Once pushed, metrics needs to confirm that we are getting the correct data and the intended metrics can be deduced from the same.

thx,

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I have a few questions here:

What are the use cases that are requiring us to add this?

Has this been reviewed by the privacy team?

In the past, the requirement around ADU tracking has always been that we can't add any features specifically for tracking, we should only rely on features that already exist for other reasons. Sounds like we are now diverging from that policy, is that intentional?

Why is this being discussed in a mo-co confidential bug?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Also, please note that these pings contain user-identifiable information. The cookie that is sent with this ping is the normal store cookies, so it lets us identify the user if they have ever logged in to the marketplace.

Comment 7

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Jonas Sicking (:sicking) from comment #5)
> Has this been reviewed by the privacy team?

Alina?

Comment 8

[Anurag Phadke[:[e-mail]]]

https://etherpad.mozilla.org/YX73peq4WX is the etherpad that was used to discuss the reasons + steps to include the above metrics in FirefoxOS.

Re: comment #5 - 
use case: Rick Fant wants these as must-have's for providing an optimum experience to FirefoxOS users. Rick - can you please share your reasons?

review by privacy team: Yes, Alina (from privacy) and Jishnu (from legal) have been part of the conversation, we have had couple of meetings discussing the same and both, Alina and Jishnu, have reviewed and approved the above pings.

tracking users/diverging from our policy: We are adding features only to provide experience and not for the purpose of tracking. Rick can elaborate on the same.

The bug is open to public. Not sure how it got marked as confidential.

Re: comment #6
The information that is being requested from the pings is part of Apache header and not part of the cookie. AFAIK, based on the header information, there isn't any way to identify whether they have ever logged in marketplace.

Comment 9

[Jishnu Menon :jishnu]

From Jonas' comments I'm not sure if I fully understood this feature or if all the relevant facts were mentioned to me when we discussed it - have we opened a legal bug for input? Could you please do that and include Jonas in it?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> Re: comment #6
> The information that is being requested from the pings is part of Apache
> header and not part of the cookie. AFAIK, based on the header information,
> there isn't any way to identify whether they have ever logged in marketplace.

The same request contains both the cookie and the newly added headers. This means that the server knows which user is making the request, and which device/country/operator the user uses.

Comment 11

[Jishnu Menon :jishnu]

A couple questions

1. Are the cookies going from server to client or client to server.
2. If client -> server is this necessary, if not let's turn the cookie off.
3. If server -> client, why are we setting cookies for pings?

Comment 12

[Wil Clouser [:clouserw]]

A cookie is given to you when you log into a website.  When you send any subsequent requests to that website you send the cookie along as well so the website can identify you.  That is your session, otherwise the website wouldn't know you were logged in.

The marketplace is no different.  When you log in we start a session and give you a cookie.  Not returning the cookie when you come back will (effectively) anonymize you but it also breaks fundamental standards of the web.  It would require a special case in FxOS and depending on how that is done it may break in the future when we change the domain to kickassmarketplace.com or change the name of the cookie.

The standard way to do what you want would be for the update check to be on another domain which doesn't have the cookie.

Comment 13

[Sid Stamm [:geekboy or :sstamm]]

I think what Jishnu is asking is whether we are setting a cookie on the ping response or if FxOS just happens to be sending a cookie with the request.  If it's just a tag-along cookie for the request, he is asking if we can fix the ping to not send it.

(In reply to Wil Clouser [:clouserw] from comment #12)
> It would require a special case in FxOS and depending on how that is
> done it may break in the future when we change the domain to
> kickassmarketplace.com or change the name of the cookie.

If we don't need to send a cookie with the ping request, gecko makes it trivial to send anonymous requests.  See http://mxr.mozilla.org/mozilla-central/source/services/sync/modules/resource.js#27 for an example.

Comment 14

[Sid Stamm [:geekboy or :sstamm]]

Actually this is a better example:
http://mxr.mozilla.org/mozilla-central/source/dom/system/NetworkGeolocationProvider.js#251

Comment 15

[Wil Clouser [:clouserw]]

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #13)
> I think what Jishnu is asking is whether we are setting a cookie on the ping
> response or if FxOS just happens to be sending a cookie with the request. 
> If it's just a tag-along cookie for the request, he is asking if we can fix
> the ping to not send it.

We aren't starting a session off the ping request so you won't get a cookie set if that's the only request you ever do.

> (In reply to Wil Clouser [:clouserw] from comment #12)
> > It would require a special case in FxOS and depending on how that is
> > done it may break in the future when we change the domain to
> > kickassmarketplace.com or change the name of the cookie.
> 
> If we don't need to send a cookie with the ping request, gecko makes it
> trivial to send anonymous requests.  See
> http://mxr.mozilla.org/mozilla-central/source/services/sync/modules/resource.
> js#27 for an example.

We don't need the cookie to be set to respond to the update request with the correct info.

Comment 16

[Jishnu Menon :jishnu]

To summarize impact: unnecessary cookie being sent that does nothing additional to add to the privacy implications of the features because the user would have had to browser the marketplace from the device already to have the cookie be set at all.

Best path here is to patch FFOS to stop cookie from being sent.

Does this present a resolution path?

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

If we somehow remove the cookie that does indeed remove the ability to identify the user when we are sending the information requested in comment 0.

However that still doesn't answer the question of why we are collecting this data. In the past tracking, such as ADU tracking, has had as a requirement that we can't send information solely for the purpose of collecting metrics. Instead we have collected whatever data we needed off of existing requests that were built for other reasons. I.e. we do ADU tracking based off of update requests that are done not in order to enable ADU tracking, but in order to check if the user needs an updated version of firefox.

So the relevant question here is: *Why* do we need to collect this data. I.e. what service are we providing the user that is forcing us to send this privacy sensitive data.

Comment 18

[Anurag Phadke[:[e-mail]]]

From my meeting notes with Rick Fant:
From user persepective: 
The marketplace caters to a variety of devices, screen sizes and locales. In order to provide a compelling experience to the end user, one that's optimum for the device and country s/he is in, we need to know the above requested info. 

From Mozilla perspective:
Knowing the raw counts helps Mozilla prioritize bug fixes, resources and enhancements for furtive versions of marketplace and FFOS, by ensuring that we are allocating enough resources for devices of critical mass. 

From privacy perspective:
There isn't any PII that's being sent out here. The above information independently or combined with other attributes cannot identify any single user. At worst, it will let us know that thousands of users are using ZTE113 model. 

Rick - anything else that I might be missing?

Comment 19

[Anurag Phadke[:[e-mail]]]

s/furtive/future (hard to comment on bugzilla via iPhone)

Comment 20

[Ben Adida [:benadida]]

Two thoughts:

- I agree that removing any unnecessary information from ping is a must, thanks for pointing this out Jonas.

- I can't comment on past policy regarding ADU tracking, but I think we have a different situation here. Whereas, in ADU times, our product was client software, here our product is a service: the Marketplace. To provide a good user service, basic data about how the service is used and the needs of its users (locale, screen size, etc.) is critical. How we do this - whether it's over existing channels or adding a new channel - seems irrelevant. What's relevant is *what* we collect and what *value* it provides to users.

My tentative conclusion: there is clear value to the user to building a Marketplace that responds to their needs. We should reduce the data we send as much as possible as long as we fulfill that value. Whether this is a separate ping or not seems irrelevant to me.

Comment 21

[Chris More [:cmore]]

So, we are considering to put this information in a separate ping, but not in the user-agent string per bug 777710 and blog post http://lawrencemandel.com/2012/07/27/decision-made-firefox-os-user-agent-string/ where the OS/hardware is removed from the user agent?

We use Google Analytics across Mozilla websites including Marketplace and it does not collect PII. Since the OS/platform/hardware is blank in the user-agent string, Google Analytics defaults to desktop-traffic and "not set" for the OS/platform. If we were looking at aggregated traffic to marketplace in Google Analytics, the Firefox OS traffic would skew the numbers toward desktop instead of mobile because of the blank OS string.

Comment 22

[Chris More [:cmore]]

FYI, if you want to follow along in Firefox OS missing from Google Analytics (intended behavior), cc yourself on bug 862868.

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to aphadke from comment #18)
> From my meeting notes with Rick Fant:
> From user persepective: 
> The marketplace caters to a variety of devices, screen sizes and locales.

The user's locale is already available through JS API. And note that this is not one of the pieces of information requested in comment 0.

We should not make any decisions based on device type. We've been consistently arguing that websites should not base decisions based on device type, but rather based on device capabilities which can already be measured using JS. This is the traditional "UA-detection vs. capability detection" debate where we have consistently argued that UA-detection is bad for interoperability and bad for the web.

Screen size is again already accessible from JS.

Also note that removing the cookie from the request will make it harder for us to track ADU since it means that we can't detect duplicate pings from the same user. Something that the metrics team has recently requested.

Comment 24

[[:fabrice] Fabrice Desré]

My understanding from the beginning is that the marketplace team wants that not to adapt the marketplace content, but only to track device metrics.

Comment 25

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Fabrice Desré [:fabrice] from comment #24)
> My understanding from the beginning is that the marketplace team wants that
> not to adapt the marketplace content, but only to track device metrics.

And if we follow that a step further, to then prioritize bug fixes that benefit the largest populations of users first.  Is this the intent?  Or are we just interested in tracking usage trends?

Comment 26

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I think the Accept-Language already contains the user's locale.

Comment 27

[Anurag Phadke[:[e-mail]]]

fabrice - can we push code review + push the patch?

Comment 28

[[:fabrice] Fabrice Desré]

Attachment: patch sending only device model

Comment 29

[Anurag Phadke[:[e-mail]]]

any update on this? is this live?

Comment 30

[[:fabrice] Fabrice Desré]

No, it's waiting for Fernando's review.

Comment 31

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 744026 [details] [diff] [review]
patch sending only device model

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

::: dom/apps/src/Webapps.jsm
@@ +21,5 @@
>  Cu.import("resource://gre/modules/SystemMessagePermissionsChecker.jsm");
>  Cu.import("resource://gre/modules/AppDownloadManager.jsm");
>  
> +#ifdef MOZ_WIDGET_GONK
> +XPCOMUtils.defineLazyGetter(this, "libcutils", function () {

nit: function()

@@ +1583,5 @@
>        }
>      }
>  
>      // Try to download a new manifest.
> +    function doRequest(oldManifest, aHeaders) {

nit: (aOldManifest, aHeaders) or (oldManifest, headers)

Comment 32

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/projects/birch/rev/bc9a464f5c81

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bc9a464f5c81

Comment 34

[Anurag Phadke[:[e-mail]]]

fabrice/ryan - does this mean we can start getting the data from marketplace.mozilla.org?

Comment 35

[[:fabrice] Fabrice Desré]

(In reply to aphadke from comment #34)
> fabrice/ryan - does this mean we can start getting the data from
> marketplace.mozilla.org?

It's not uplifted to b2g18 yet. You may have some pings already from people on m-c builds (like me ;) )

Comment 36

[Jason Smith [:jsmith]]

The patch here is changing code in DOM:Apps, so I've moved over to that component.

Comment 37

[[:fabrice] Fabrice Desré]

Comment on attachment 744026 [details] [diff] [review]
patch sending only device model

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

We need that patch for metrics to track device usage and use that data to better prioritize device-specific bug fixes.

The patch itself is just adding an http header to the marketplace manifest pings, and is very low risk.

Comment 38

[Alex Keybl [:akeybl]]

It's very important that this is verified. Jason, could you take a look at this with aphadke once this lands on v1.0.1?

Comment 39

[Jason Smith [:jsmith]]

Yup, sure. I'll add myself as a QA contact to keep this on my radar.

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/683582be87df
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/7d7a5f0ad3e2

Comment 41

[Anurag Phadke[:[e-mail]]]

(In reply to Alex Keybl [:akeybl] from comment #38)
> It's very important that this is verified. Jason, could you take a look at
> this with aphadke once this lands on v1.0.1?

Alex - Jason pinged me yday, the patch lands today (5/9) and we should start getting the data in DWH on 5/10. Jason and I will work together on this and update the bug.

-anurag

Comment 43

[Jason Smith [:jsmith]]

Just met with Anurag to go over the state of this ping. I'm going to do the following for immediate verification:

Analyze a dump of the resulting pings that we have over the past few days and look for presence of:

- Presence of the device type with a ping
- Presence of the user agent with a ping
- Presence of the IP with a ping
- Presence of the marketplace URL with a ping
- Presence of the standard HTTP information with a ping
- Each possible device type that is actively used should be present across the pings
- Multiple distinct IPs are present across different locations we know we have users in currently (e.g. CA, Spain)

Note that we can't test for reliability of the pings here, we can only test for presence of the data.

Comment 44

[Anurag Phadke[:[e-mail]]]

thx jason - just sent u the data 5/14 (UTC) via email.

Comment 45

[Jason Smith [:jsmith]]

These tests passed cleanly:

* Device type was present in each ping
* User agent was present in each ping
* IP was present in each ping
* Marketplace URL present in each ping
* HTTP misc information included in each ping
* Distinct user agents was seen across the pings
* Distinct IPs in different locations was seen across the pings

The test in question right now is:

* Each possible device type that is actively used should be present across pings

I'm following up with Fabrice and Anurag over email to understand why.

Comment 46

[Jason Smith [:jsmith]]

Marking verified. I finished testing here and finished discussing this offline. We've got one followup in bug 872830. I'm also following up with m1 one other oddity offline.