We need to know whether or not people are using Firefox in restricted profiles. John Jensen suggested adding an extra flag to the FHR environment to capture this data, and mentioned that his team could help with the analysis on the server side. Richard, can you advise on how we implement this on the client?

Actually, it sounds like FHR is being killed sooner than I thought... maybe we'll just need to wait until we have opt-out telemetry pings to send this. Alternately, in the interim, I wonder if we could somehow leverage Adjust to track this?

"sooner than I thought" Is it being killed before we would land this in 45? What do you mean by opt-out telemetry pings?

(In reply to Barbara Bermes [:barbara] from comment #2) > "sooner than I thought" > > Is it being killed before we would land this in 45? The pipeline is being shut down Dec 31, so yes, killed before we would land this. > What do you mean by opt-out telemetry pings? I mean unified telemetry, which is the system that will replace FHR (bug 1220177).

BrowserHealthRecorder shows environment gathering, which will probably still be relevant for UT. I can advise more closer to the time. Relevant, though: if a profile starts restricted and never changes, we can simplify. If it can change outside our knowledge, then the lifecycle of BrowserLocaleManager will be a good inspiration.

(In reply to :Margaret Leibovic from comment #3) > (In reply to Barbara Bermes [:barbara] from comment #2) > > "sooner than I thought" > > > > Is it being killed before we would land this in 45? > > The pipeline is being shut down Dec 31, so yes, killed before we would land > this. > > > What do you mean by opt-out telemetry pings? > > I mean unified telemetry, which is the system that will replace FHR (bug > 1220177). Ok, so that means, we should just wait and not put any metrics in there for now. Moving this to unified telemetry, do we see any changes in the concept that we have to run by legal or is it the same approach?

(In reply to Barbara Bermes [:barbara] from comment #5) > (In reply to :Margaret Leibovic from comment #3) > > (In reply to Barbara Bermes [:barbara] from comment #2) > > > "sooner than I thought" > > > > > > Is it being killed before we would land this in 45? > > > > The pipeline is being shut down Dec 31, so yes, killed before we would land > > this. > > > > > What do you mean by opt-out telemetry pings? > > > > I mean unified telemetry, which is the system that will replace FHR (bug > > 1220177). > > Ok, so that means, we should just wait and not put any metrics in there for > now. > > Moving this to unified telemetry, do we see any changes in the concept that > we have to run by legal or is it the same approach? Same approach, different implementation. It may be possible to get something landed for 45 here, but it would need to be a simplified version of what we hope to achieve in the future. gfritzsche, given the current UT system on Android, do you think it would be possible to send an opt-out ping that includes a boolean field to indicate whether or not Firefox is running in a restricted profile? I know that eventually (hopefully soon) we want to be uploading data from Java, but maybe in the very immediate future, we could find a way to leverage the Gecko uploader to send this data in an opt-out telemetry ping.

It depends on what exactly you want to do and how quickly we need this. Two possible approaches: * sending the proper opt-out pings like on desktop will probably need a bit of investigation and work (probably: data choices policy, checking the code paths, turning on some UT features) * a faster possibility is sending a small separate ping type now, lets say including client-id, no environment data, minimal payload. We'd still have to check how this works with the data choices policy though. The API is TelemetryController.submitExternalPing() - we would have to give it a test-run to see whether it works as expected on Android. For testing we can point Fennec to submit to a local server (pref toolkit.telemetry.server): https://github.com/vdjeric/gzipServer Then test submitting pings: TelemetryController.submitExternalPing("your-custom-ping-type", { restrictedProfile: <true/false> }, { addClientId: true, addEnvironment: false, }); ... have we made a call yet on how data collection review works for Fennec? Is it the same as Fx Desktop? https://wiki.mozilla.org/Firefox/Data_Collection

(In reply to Georg Fritzsche [:gfritzsche] from comment #7) > ... have we made a call yet on how data collection review works for Fennec? > Is it the same as Fx Desktop? > https://wiki.mozilla.org/Firefox/Data_Collection

Yes it is all-Firefox.

(In reply to Georg Fritzsche [:gfritzsche] [away Dec 19 - Jan 3] from comment #7) > * a faster possibility is sending a small separate ping type now, lets say > including client-id, no environment data, minimal payload. > We'd still have to check how this works with the data choices policy > though. > The API is TelemetryController.submitExternalPing() - we would have to > give it a test-run to see whether it works as expected on Android. > > For testing we can point Fennec to submit to a local server (pref > toolkit.telemetry.server): > https://github.com/vdjeric/gzipServer > > Then test submitting pings: > TelemetryController.submitExternalPing("your-custom-ping-type", > { > restrictedProfile: <true/false> > }, > { > addClientId: true, > addEnvironment: false, > }); I think we should go ahead and implement this approach. We had a meeting with legal a few weeks ago, and they approved us sending an opt-out boolean to indicate whether the browser was launched in a restricted profile. But we will also flag a data steward for review before landing the patch, following the Firefox data collection guidelines.

After some tinkering I was able to receive the following ping on a locally running server: > { > "clientId": "6b1e8c4a-1698-47b2-bd66-1727ffae5c99", > "payload": { > "restrictedProfile": false > }, > "application": { > "vendor": "Mozilla", > "name": "Fennec", > "buildId": "20151216034107", > "version": "46.0a1", > "architecture": "arm", > "platformVersion": "46.0a1", > "xpcomAbi": "arm-eabi-gcc3", > "channel": "default" > }, > "version": 4, > "creationDate": "2015-12-16T15:19:58.809Z", > "type": "fennec-startup-ping", > "id": "3b1c0a0b-fe15-47d1-a925-29aafa90d922" > } For testing I've send this ping from startup() in browser.js. This telemetry topic is still new to me: Would we want to send something like this on every startup? > > TelemetryController.submitExternalPing("your-custom-ping-type", How do we define custom ping types? Can it just be anything? Is there any guideline I should follow? In the example above I just used "fennec-startup-ping" for testing. Using TelemetryController.jsm caused a bunch of JavaScript errors - all related to logging (in configureLogging() and get _log()). I patched the methods locally to use the Android logger or just do nothing. We might need some actual patches to make this work.

(In reply to Sebastian Kaspari (:sebastian) from comment #11) > After some tinkering I was able to receive the following ping on a locally > running server: > > > { > > "clientId": "6b1e8c4a-1698-47b2-bd66-1727ffae5c99", > > "payload": { > > "restrictedProfile": false > > }, > > "application": { > > "vendor": "Mozilla", > > "name": "Fennec", > > "buildId": "20151216034107", > > "version": "46.0a1", > > "architecture": "arm", > > "platformVersion": "46.0a1", > > "xpcomAbi": "arm-eabi-gcc3", > > "channel": "default" > > }, > > "version": 4, > > "creationDate": "2015-12-16T15:19:58.809Z", > > "type": "fennec-startup-ping", > > "id": "3b1c0a0b-fe15-47d1-a925-29aafa90d922" > > } > > For testing I've send this ping from startup() in browser.js. This telemetry > topic is still new to me: Would we want to send something like this on every > startup? A ping sent on every startup starts to sound like our plan for a mobile-friendly "core" data ping: https://docs.google.com/document/d/1Ap5Z48Rh4t1r5lKmDAVCHNWpLS4nJIG4abZmw0nQiq4/edit#heading=h.j26q8bbe65p0 If we modify this custom ping slightly, maybe we can just start using that as our "core" data ping. > > > TelemetryController.submitExternalPing("your-custom-ping-type", > > How do we define custom ping types? Can it just be anything? Is there any > guideline I should follow? In the example above I just used > "fennec-startup-ping" for testing. NI to gfritzsche to answer this. > Using TelemetryController.jsm caused a bunch of JavaScript errors - all > related to logging (in configureLogging() and get _log()). I patched the > methods locally to use the Android logger or just do nothing. We might need > some actual patches to make this work. Hm... we've been planning to implement our own uploader in Java. If actual patches turn out to be a lot of work, we may want to go straight for a Java implementation.

(In reply to :Margaret Leibovic from comment #12) > A ping sent on every startup starts to sound like our plan for a > mobile-friendly "core" data ping: > https://docs.google.com/document/d/ > 1Ap5Z48Rh4t1r5lKmDAVCHNWpLS4nJIG4abZmw0nQiq4/edit#heading=h.j26q8bbe65p0 > > If we modify this custom ping slightly, maybe we can just start using that > as our "core" data ping. Modifying the Telemetry JS modules to do that would involve a bit of work (everything except the "payload" content is common ping data added to all pings currently). If we want to do core-data pings we should start a new clean code path somewhere (Java or modifying the JS modules for now, whatever works best for the mobile team). > > > > TelemetryController.submitExternalPing("your-custom-ping-type", > > > > How do we define custom ping types? Can it just be anything? Is there any > > guideline I should follow? In the example above I just used > > "fennec-startup-ping" for testing. > > NI to gfritzsche to answer this. This can be pretty much anything, it should be descriptive though and only contain simple characters. The only other thing needed is to file a bug so that the ping will not be sorted into the "unknown pings"/"other" bucket on the server-side. The bug would go into "Cloud Services :: Metrics: Pipeline", for further questions you can check with mreid. > > > Using TelemetryController.jsm caused a bunch of JavaScript errors - all > > related to logging (in configureLogging() and get _log()). I patched the > > methods locally to use the Android logger or just do nothing. We might need > > some actual patches to make this work. > > Hm... we've been planning to implement our own uploader in Java. If actual > patches turn out to be a lot of work, we may want to go straight for a Java > implementation. What is the problem there? Does Log.jsm not work properly on Android?

Revisiting this bug now that bug 1205835 landed... is there a way for us to use the new core ping to track whether or not Firefox was launched in a restricted profile? Or maybe we should hack the experiments field and include this as an experiment?

If it is considered important enough to be in the "core" ping, it would be much cleaner to put this in an optional separate field.

(In reply to Georg Fritzsche [:gfritzsche] from comment #15) > If it is considered important enough to be in the "core" ping, it would be > much cleaner to put this in an optional separate field. Okay, that sounds like a better idea. My main concern is that the majority of browser use doesn't happen in a restricted profile, so I don't want to be sending additional data in those cases. But if we can add this as an optional field, that sounds like it satisfies our use case. Similarly, if we ever drop support for restricted profiles (based on this valuable data we're getting here), I don't want us to need to legacy support some unnecessary field. Maybe I'm scope creeping, but this would also be useful for tracking guest profile usage, since we are currently skeptical of our telemetry data around that feature as well.

we could consider a "launch" field which has states like: normal, url, guest, restricted, shortcut

(In reply to Mark Finkle (:mfinkle) from comment #17) > we could consider a "launch" field which has states like: normal, url, > guest, restricted, shortcut That sounds useful. Is this an important core metric that should be part of the "core" ping? Are all cases except "normal" relatively rare? If so, we can optimize by keeping it optional (only set for non-"normal" launches).

(In reply to Georg Fritzsche [:gfritzsche] from comment #18) > (In reply to Mark Finkle (:mfinkle) from comment #17) > > we could consider a "launch" field which has states like: normal, url, > > guest, restricted, shortcut > > That sounds useful. Is this an important core metric that should be part of > the "core" ping? > Are all cases except "normal" relatively rare? If so, we can optimize by > keeping it optional (only set for non-"normal" launches). "url" is very frequent (that's would be what happens when you open a URL from another app). Given that engagement is one of our top priorities, and that we're trying to increase the number of ways people enter the app, I could see this as an important field to include. We have some goals around increasing homescreen shortcut usage, so this could be a good data point for our KPIs. If it doesn't hurt us to exclude the field for the default case (launching from the Firefox icon), I think it makes sense to keep it optional. I'd just want to make sure we can handle the analysis on the pipeline side.

(In reply to Mark Finkle (:mfinkle) from comment #17) > we could consider a "launch" field which has states like: normal, url, > guest, restricted, shortcut Don't they overlap? "url" could be a restricted, guest or normal profile. Will we still be able to accurately measure restricted profiles usage if some of the usage is included in "url"?

(In reply to Sebastian Kaspari (:sebastian) from comment #20) > (In reply to Mark Finkle (:mfinkle) from comment #17) > > we could consider a "launch" field which has states like: normal, url, > > guest, restricted, shortcut > > Don't they overlap? "url" could be a restricted, guest or normal profile. > Will we still be able to accurately measure restricted profiles usage if > some of the usage is included in "url"? That's a really excellent point, and evidence that this has certainly scope creeped. Restricted profiles could also create homescreen shortcuts. So I think for the sake of this bug we should just create an optional "restricted" field, which will be sent with a very small % of our core pings. Then we can discuss whether it makes sense to include launch data as well. We already have opt-in telemetry probes in place for launching the browser, so we do have some information about that to help guide us.

One issue I forsee is that afaict, we don't currently have a way to uniquely identify installs. We're currently uploading the client ID from Gecko and afaik the client ID is specific to a given profile (as opposed to an install). Also afaik, Restricted & Guest profiles use a different profile on disk and thus a separate client ID. Assuming I understand both the Gecko client ID & the restricted/guest profiles, we'd both have to add a client ID specific to the install and a new field to identify alternative profiles, or change client ID to be used per install, rather than per profile.

Margaret, is my understand for restricted/guest profiles correct in comment 22?

(In reply to Michael Comella (:mcomella) from comment #22) > One issue I forsee is that afaict, we don't currently have a way to uniquely > identify installs. > > We're currently uploading the client ID from Gecko and afaik the client ID > is specific to a given profile (as opposed to an install). Also afaik, > Restricted & Guest profiles use a different profile on disk and thus a > separate client ID. Correct, restricted/guest profiles both use a separate gecko profile. Restricted profiles go one step further and act as a complete separate install on the Android system (we don't have a shared /data/data directory we can read from with the admin profile app install). > Assuming I understand both the Gecko client ID & the restricted/guest > profiles, we'd both have to add a client ID specific to the install and a > new field to identify alternative profiles, or change client ID to be used > per install, rather than per profile. I don't think we should try to match these guest/restricted profiles with their main profile. It's fine to treat these as separate clients/installs in telemetry. The main question we're trying to answer is "how many people use this feature?". You do raise a point that if that's a lot of people, that might skew our overall MAU/DAU numbers, but let's cross that bridge when we get to it. We could always filter these out in that analysis. Or alternately, maybe we *should* count these as separate users, since technically it is actually another person using the browser.

Margaret asked on IRC if we were blocked on this and afaict we're not. To add this value, I assume we'd need to grab it from the Activity context so it'd need to be passed in with the Intent (see BrowserApp.uploadTelemetry). Then the value would need to be passed around TelemetryUploadService until TelemetryUploadService.uploadCorePing, where it'd need to get passed into TelemetryPingGenerator.createCorePing. Then it'd get passed into TelemetryPingGenerator.createCorePingPayload, where it can be added to the ping itself. A corresponding JSON key would need to be added to the CorePing class. fwiw, to avoid having too many arguments and better handling optional arguments, I wanted to move TelemetryPingGenerator.createCorePing over to a Builder (either for creating the argument to createCorePing or for just creating the core ping itself), but we don't have to do this here.

My proposed strategy to change the format is we add the "restricted" field if the profile is restricted, otherwise we omit the field and it's assumed to be false. This doesn't deal with guest mode in Firefox – do we want to send information about that too, Margaret?

This patch seems simple enough to work but I can't generate a client ID on this new profile (see bug 1244295) which prevents us from uploading the telemetry. Data choices are disabled in the restricted profile so I presume we don't upload telemetry via gecko and thus I assume this client ID will never be created without bug 1244295. Still TODO: * Update core ping docs * If we're sending core pings from this profile, we should probably give it an opt-out data choice. * Probably fix bug 1244295 first Review commit: https://reviewboard.mozilla.org/r/34435/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34435/

(In reply to Michael Comella (:mcomella) from comment #27) > * If we're sending core pings from this profile, we should probably give it > an > opt-out data choice. Any thoughts, Margaret? Should we be sending data from a restricted profile at all?

(In reply to Michael Comella (:mcomella) from comment #26) > My proposed strategy to change the format is we add the "restricted" field > if the profile is restricted, otherwise we omit the field and it's assumed > to be false. This doesn't deal with guest mode in Firefox – do we want to > send information about that too, Margaret? I don't care as much about guest mode, but this is another case where users are very unlikely to opt in to the opt-in telemetry. Can we send this data in the same field? As much as I want to add a quick fix to get restricted profile data, I don't want us to go crazy adding all sort of new custom fields. (In reply to Michael Comella (:mcomella) from comment #28) > (In reply to Michael Comella (:mcomella) from comment #27) > > * If we're sending core pings from this profile, we should probably give it > > an > > opt-out data choice. > > Any thoughts, Margaret? Should we be sending data from a restricted profile > at all? The admin UI controls the opt-out preference, so this should already be taken care of.

(In reply to :Margaret Leibovic from comment #24) > You do raise a point that if that's a lot of people, that might skew our > overall MAU/DAU numbers, but let's cross that bridge when we get to it. We > could always filter these out in that analysis. Or alternately, maybe we > *should* count these as separate users, since technically it is actually > another person using the browser. We should definitely treat these people as unique active users.

(In reply to Michael Comella (:mcomella) from comment #27) > Created attachment 8718160 [details] > MozReview Request: Bug 1226322 - (WIP) Add 'restricted' field to core > telemetry ping. r=sebastian This type of patch could be setting a bad precedent. We spent time working on the fields to include in the "core" ping and now we are willy-nilly adding a new one. It's not added in a scalable manner. Adding a boolean field like this for a one-off situation is not a good plan. What about the next simple, one-off? Lack of the field is not | false | either. It's more like "undefined" because we don't know if the ping format in this version made an explicit choice to _not_ include the field or just didn't know about the data. In order to better handle that, we'd bump the version (v) to 2. But that has potential impact on the server side and analysis scripts.

(In reply to Mark Finkle (:mfinkle) from comment #31) > This type of patch could be setting a bad precedent. We spent time working > on the fields to include in the "core" ping and now we are willy-nilly > adding a new one. It's not added in a scalable manner. Adding a boolean > field like this for a one-off situation is not a good plan. What about the > next simple, one-off? > > Lack of the field is not | false | either. It's more like "undefined" > because we don't know if the ping format in this version made an explicit > choice to _not_ include the field or just didn't know about the data. In > order to better handle that, we'd bump the version (v) to 2. But that has > potential impact on the server side and analysis scripts. Maybe we could change this to a field saying whether the current profile is "normal", "guest" or "restricted"? This is okay too and not hard. We just should not mix them with launch modes like we discussed above.

(In reply to Mark Finkle (:mfinkle) from comment #31) > Lack of the field is not | false | either. It's more like "undefined" > because we don't know if the ping format in this version made an explicit > choice to _not_ include the field or just didn't know about the data. In > order to better handle that, we'd bump the version (v) to 2. But that has > potential impact on the server side and analysis scripts. I think once we have the basic fields in the "core" ping that we need for launch (we should?), we should definitely bump versions with each new addition. We will need this for validation and future integrated schema checks.

Not actively working on this. I imagine we'll reconsider this when we create a Java-based main ping or move the existing Gecko pings to the Java uploader (bug 1251639).

Bulk Move: https://bugzilla.mozilla.org/show_bug.cgi?id=1399539

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195 Needinfo :susheel if you think this bug should be re-triaged.

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.