Closed Bug 860571 Opened 7 years ago Closed 7 years ago

Pings for Firefox OS

Categories

(Core Graveyard :: DOM: Apps, defect)

defect
Not set

Tracking

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 verified, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)

VERIFIED FIXED
mozilla23
blocking-b2g tef+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- verified
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- verified

People

(Reporter: aphadke, Assigned: fabrice)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Attached patch wip patch (obsolete) — Splinter Review
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.
Assignee: nobody → fabrice
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.
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
Depends on: 862499
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,
Flags: needinfo?(fabrice)
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?
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.
(In reply to Jonas Sicking (:sicking) from comment #5)
> Has this been reviewed by the privacy team?

Alina?
Flags: needinfo?(ahua)
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.
Group: mozilla-corporation-confidential
Flags: needinfo?(ahua) → needinfo?(rfant)
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?
Depends on: 863789
> 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.
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?
Flags: needinfo?(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.
Flags: needinfo?(clouserw)
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.
(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.
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?
Flags: needinfo?(jonas)
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.
Flags: needinfo?(jonas)
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?
s/furtive/future (hard to comment on bugzilla via iPhone)
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.
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.
FYI, if you want to follow along in Firefox OS missing from Google Analytics (intended behavior), cc yourself on bug 862868.
(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.
My understanding from the beginning is that the marketplace team wants that not to adapt the marketplace content, but only to track device metrics.
Flags: needinfo?(fabrice)
(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?
I think the Accept-Language already contains the user's locale.
fabrice - can we push code review + push the patch?
Flags: needinfo?(rfant)
Attachment #736594 - Attachment is obsolete: true
Attachment #744026 - Flags: review?(ferjmoreno)
any update on this? is this live?
No, it's waiting for Fernando's review.
blocking-b2g: --- → tef?
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)
Attachment #744026 - Flags: review?(ferjmoreno) → review+
https://hg.mozilla.org/mozilla-central/rev/bc9a464f5c81
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
fabrice/ryan - does this mean we can start getting the data from marketplace.mozilla.org?
Flags: needinfo?(fabrice)
(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 ;) )
Flags: needinfo?(fabrice)
Component: Project Review → DOM: Apps
Product: mozilla.org → Core
Version: other → Trunk
The patch here is changing code in DOM:Apps, so I've moved over to that component.
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.
Attachment #744026 - Flags: approval-mozilla-b2g18?
blocking-b2g: tef? → tef+
Attachment #744026 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
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?
Flags: needinfo?(jsmith)
Flags: needinfo?(aphadke)
Keywords: verifyme
Yup, sure. I'll add myself as a QA contact to keep this on my radar.
Flags: needinfo?(jsmith)
QA Contact: jsmith
(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
Flags: needinfo?(aphadke)
Duplicate of this bug: 860808
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.
thx jason - just sent u the data 5/14 (UTC) via email.
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.
Depends on: 872830
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.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.