Closed Bug 1373174 Opened 7 years ago Closed 7 years ago

(Leanplum) - Change user identification method and add the general documentation for Leanplum

Categories

(Firefox for Android Graveyard :: Metrics, enhancement)

Product:
Firefox for Android Graveyard
Component:
Metrics
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox56 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: cnevinchen, Assigned: cnevinchen)

References

Details

(Whiteboard: [LP_M1])

Attachments

(2 files, 2 obsolete files)

Bug 1373174 - Change Leanplum user identification method.
59 bytes, text/x-review-board-request
maliu
: review+
cnevinchen
: review+
Details
Bug 1373174 - Add general documentation for Leanplum.
59 bytes, text/x-review-board-request
cnevinchen
: review+
Details 
Leanplum uses Hashed wifi MAC address to identify users. We should use a Random UUID instead of it for better user prviasy protection. This UUID is not related to Fennec client id nor Advertising ID. So users are anonynoust to Leanplum even events and messages are known by Leanplum

I'll also write down the document for data reviewer to review the data collected by Leaplum. It should contain 
1) The data we send to leanplum in general and 
2) For those events we've landed in bug 1361664.

This bug is to list the data that send out by/to Leanplum.
Comment on attachment 8877938 [details]
Bug 1373174 - Add general documentation for Leanplum.

Hi Chenxia
There are some fields that I need to double confirm with Leanplum.
But the major part of the document is done 
Could you please guide me on what I've missed for data review?

Thank you!
Flags: needinfo?(liuche)
Comment on attachment 8877938 [details]
Bug 1373174 - Add general documentation for Leanplum.

https://reviewboard.mozilla.org/r/149350/#review155496

Thank you, Nevin! This is good documentation.

If there's an in-code constants file where these events are added, could you add a note that adding new probes/events should also be documented in this file? Just as a reminder to people who are adding code, so they know where to add documentation.

In general, for data review, we want documentation on the specifics of what is being collected (such as what units are being used, when the data is being collected) so that we can understand what is being collected without having to read the code; the other thing is that the data can't be personally identifiable. The list of events you've documented so far is good, and for now, the probes that are being collected are pretty basic and self-explanatory.

Thank you for writing up this documentation! The draft looks good, and unless you make big changes or add a lot more probes, I'll give it an r+.

::: mobile/android/docs/mma.rst:9
(Diff revision 2)
> + MMA Mobile Marketing Automation
> +======================================
> +
> +We want to engage with users more. MMA is the project for this purpose. When a user performs a certain
> +UI action, he/she will see a prompt and have a chance to  interact with it. For example, if a user uses
> +firefox 10 times a week, but firefox is not his default browser, we'll prompt the user the next time

nit: capitalize Firefox x 2

::: mobile/android/docs/mma.rst:13
(Diff revision 2)
> +UI action, he/she will see a prompt and have a chance to  interact with it. For example, if a user uses
> +firefox 10 times a week, but firefox is not his default browser, we'll prompt the user the next time
> +when he launchers our app, and guide him to set us as default browser.
> +
> +Mozilla is using a third party framework called "Leanplum" in order to achieve above goal for
> +Android 56 release. Leanplum is an American Company in San Francisco since 2012. We put their SDK in

nit: Leanplum is a San Francisco company, founded in 2012.

::: mobile/android/docs/mma.rst:17
(Diff revision 2)
> +Mozilla is using a third party framework called "Leanplum" in order to achieve above goal for
> +Android 56 release. Leanplum is an American Company in San Francisco since 2012. We put their SDK in
> +our codebase and sync upstream when there's a major update. Please find it in ``mobile/android/thirdparty/com/leanplum``.
> +The SDK is documented at https://www.leanplum.com/docs/android/
> +
> +There are three major component is Leanplum SDK.

nit: "in" not is, components plural

::: mobile/android/docs/mma.rst:19
(Diff revision 2)
> +our codebase and sync upstream when there's a major update. Please find it in ``mobile/android/thirdparty/com/leanplum``.
> +The SDK is documented at https://www.leanplum.com/docs/android/
> +
> +There are three major component is Leanplum SDK.
> +1. Events : Triggers when users perform certain actions. An event will normally trigger a prompt message.
> +2. Deep Links : Actions that users can perform to interact with the prompt message.

Could this be more specific? Example: "Presented in prompts and allow users to trigger certain in-app actions from prompt messages (such as opening the Settings menu)." (or whatever it actually does, but specifically)

::: mobile/android/docs/mma.rst:20
(Diff revision 2)
> +The SDK is documented at https://www.leanplum.com/docs/android/
> +
> +There are three major component is Leanplum SDK.
> +1. Events : Triggers when users perform certain actions. An event will normally trigger a prompt message.
> +2. Deep Links : Actions that users can perform to interact with the prompt message.
> +3. Messages :  Campaigns that we want to engage with users. Messages are combination with an Event and a Deep Link.

nit: "a combination of an Event and a Deep Link"

::: mobile/android/docs/mma.rst:26
(Diff revision 2)
> +
> +
> +Data collection
> +~~~~~~~~~~~~~~~
> +
> +When is data collected and sent to the Leanplum backend?

Looking at the descriptions below, this is more "Who will have Leanplum enabled?"

::: mobile/android/docs/mma.rst:29
(Diff revision 2)
> +~~~~~~~~~~~~~~~
> +
> +When is data collected and sent to the Leanplum backend?
> +======================================================
> +
> +* We use Switchboard to filter users to have Leanplum enabled. Currently if the user is in USA

nit: "Currently, for users in the USA..."

::: mobile/android/docs/mma.rst:30
(Diff revision 2)
> +
> +When is data collected and sent to the Leanplum backend?
> +======================================================
> +
> +* We use Switchboard to filter users to have Leanplum enabled. Currently if the user is in USA
> +  and speak English, 10% of that users will have Leanplum enabled.

nit: who speak English
Alternatively, could be "whose locale is set to English"

"10% of those users"

::: mobile/android/docs/mma.rst:31
(Diff revision 2)
> +When is data collected and sent to the Leanplum backend?
> +======================================================
> +
> +* We use Switchboard to filter users to have Leanplum enabled. Currently if the user is in USA
> +  and speak English, 10% of that users will have Leanplum enabled.
> +* If the user have "Health Report" setting enabled.

nit: "has"

::: mobile/android/docs/mma.rst:32
(Diff revision 2)
> +======================================================
> +
> +* We use Switchboard to filter users to have Leanplum enabled. Currently if the user is in USA
> +  and speak English, 10% of that users will have Leanplum enabled.
> +* If the user have "Health Report" setting enabled.
> +* If above two is true, when the app start, and switchboard configure arrived, Fennec will send the

nit: "above two are true"
"when the app starts and receives the Switchboard configuration,"

::: mobile/android/docs/mma.rst:53
(Diff revision 2)
> +======================================================
> +
> +The Leanplum SDK collects and sends two messages to the Leanplum backend.  The messages have the
> +following parameters::
> +
> +  "deviceId" -> "b13b3c239d01aa7c"      // set by Fennec, a random UUID

This is a pretty comprehensize list!

One thing I would suggest is separating this list out further into subsections: things we are collecting to use specifically for campaigns (such as userAttributes), basic system data (most of these things), and then finally the things thate are sent but are defaults/not used (though I wonder if it's possible to remove these entirely, or if the SDK requires it).

Specificall, having userAttributes in a smaller subsection will be helpful because it will be clear where we should add new probes that are being collected, or remove probes for campaigns that are finished.

::: mobile/android/docs/mma.rst:84
(Diff revision 2)
> +
> +User Identifier:
> +Since Device ID is a random UUID, Leanplum can't map the device to any know Client ID in Fennec nor Advertising ID.
> +
> +Events:
> +Most of the Leanplum events can mapped to a single combination of Telemetry event (Event+Method+Extra).

nit: "can be mapped"

::: mobile/android/docs/mma.rst:89
(Diff revision 2)
> +Most of the Leanplum events can mapped to a single combination of Telemetry event (Event+Method+Extra).
> +Some events are not collected in Mozilla Telemetry. This will be addressed separately in each campaign review.
> +List of current Events are :
> +
> +Deep Links:
> +Deep links are actions that can point Fennet to execute certain page or feature. Such as `show bookmark list` or

nit: "Fennec" typo
"to open certain pages or load features such as 'show..."

::: mobile/android/docs/mma.rst:93
(Diff revision 2)
> +Deep Links:
> +Deep links are actions that can point Fennet to execute certain page or feature. Such as `show bookmark list` or
> +`open a SUMO page`. This data is not colloected by Leanplum (To be confirmed)
> +
> +Messages :
> +Message is what Leanplum prompt to to user. The interaction of that prompt will be kept and sent ot Leanplum backend.

nit: "Messages are in-app prompts to the user from Leanplum."

"ot" typo ("to")

::: mobile/android/docs/mma.rst:109
(Diff revision 2)
> +To test this locally, add lines like:
> +
> +export MOZ_ANDROID_MMA=1
> +ac_add_options --with-leaplum-sdk-keyfile=/path/to/leaplum-sdk-developer.token
> +
> +MOZ_ANDROID_MMA depends on MOZ_NATIVE_DEVICES and MOZ_ANDROID_GCM,

nit: period not comma

::: mobile/android/docs/mma.rst:110
(Diff revision 2)
> +
> +export MOZ_ANDROID_MMA=1
> +ac_add_options --with-leaplum-sdk-keyfile=/path/to/leaplum-sdk-developer.token
> +
> +MOZ_ANDROID_MMA depends on MOZ_NATIVE_DEVICES and MOZ_ANDROID_GCM,
> +since Leanplum requires Google Play Services library that those flags

nit: unnecessary "that"

::: mobile/android/docs/mma.rst:111
(Diff revision 2)
> +export MOZ_ANDROID_MMA=1
> +ac_add_options --with-leaplum-sdk-keyfile=/path/to/leaplum-sdk-developer.token
> +
> +MOZ_ANDROID_MMA depends on MOZ_NATIVE_DEVICES and MOZ_ANDROID_GCM,
> +since Leanplum requires Google Play Services library that those flags
> +are a proxy for and enable, respectiviely.

nit: missing a word in front of "and"

::: mobile/android/docs/mma.rst:115
(Diff revision 2)
> +since Leanplum requires Google Play Services library that those flags
> +are a proxy for and enable, respectiviely.
> +
> +We want to enable MOZ_ANDROID_MMA in Nightly, but only for
> +MOZILLA_OFFICIAL builds.  Since MOZILLA_OFFICIAL is still defined in
> +old-configure.in, we can't interrogate it in

nit: "integrate"

::: mobile/android/docs/mma.rst:123
(Diff revision 2)
> +
> +Technical notes on the Leanplum SDK integration
> +=============================================
> +
> +Just like Adjust, MmaDelegate uses mmaInterface to inject the MmaLeanplumImp and MmaStubImp.
> +Constants used by Leanplum is in MmaConstants. Services in AndroidManifest is in

nit: "are" instead of "is"
Attachment #8877938 - Flags: review+
Comment on attachment 8877937 [details]
Bug 1373174 Change Change Leanplum user identification method.

https://reviewboard.mozilla.org/r/149348/#review155720

LGTM
Attachment #8877937 - Flags: review?(max) → review+
Comment on attachment 8877937 [details]
Bug 1373174 Change Change Leanplum user identification method.

https://reviewboard.mozilla.org/r/149348/#review155736

::: commit-message-27cad:1
(Diff revision 1)
> +Bug 1373174 Change Change Leanplum user identification method. r?maliu

nit: Would you fix the double "Change" please?
To be more clear. This document is for the general Leanplum SDK documentation and for stuff that already landed in Bug 1361664. Any new things added will need to go through data review again (e.g Bug 1374889)
Summary: [Leanplum] - Change user identification method and documentation. → (Leanplum) - Change user identification method and add the general documentation for Leanplum
Blocks: 1374889
Attachment #8877937 - Attachment is obsolete: true
Attachment #8877937 - Attachment is obsolete: false
Attachment #8877937 - Attachment is obsolete: true
Comment on attachment 8880893 [details]
Bug 1373174 - Change Leanplum user identification method.

https://reviewboard.mozilla.org/r/152256/#review157302

I accidentally set the old patch https://bugzilla.mozilla.org/attachment.cgi?id=8877937 obsolete. So r+ again.
Attachment #8880893 - Flags: review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e82fba8c9817
Change Change Leanplum user identification method. r=nechen
https://hg.mozilla.org/integration/autoland/rev/11a5bb57423d
Add general documentation for Leanplum. r=liuche
Keywords: checkin-needed
backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=109875237&repo=autoland
Flags: needinfo?(cnevinchen)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4a28736cce0
Backed out changeset 11a5bb57423d 
https://hg.mozilla.org/integration/autoland/rev/5406ad144c09
Backed out changeset e82fba8c9817 for android bustage
Attachment #8877938 - Attachment is obsolete: true
Flags: needinfo?(liuche)
Comment on attachment 8881569 [details]
Bug 1373174 - Add general documentation for Leanplum.

https://reviewboard.mozilla.org/r/152734/#review157984

carry over the r+ cause I lost previous commit
Attachment #8881569 - Flags: review+
Comment on attachment 8880893 [details]
Bug 1373174 - Change Leanplum user identification method.

https://reviewboard.mozilla.org/r/152256/#review158162

carry over the r+ cause I lost previous commit
Attachment #8880893 - Flags: review+
Comment on attachment 8881569 [details]
Bug 1373174 - Add general documentation for Leanplum.

https://reviewboard.mozilla.org/r/152734/#review158164

carry over the r+ cause I lost previous commit
Attachment #8881569 - Flags: review+
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2db417899461
Change Leanplum user identification method. r=nechen
https://hg.mozilla.org/integration/autoland/rev/5c0e99ef7f00
Add general documentation for Leanplum. r=nechen
Thanks for the notice.
Flags: needinfo?(cnevinchen)
https://hg.mozilla.org/mozilla-central/rev/2db417899461
https://hg.mozilla.org/mozilla-central/rev/5c0e99ef7f00
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Attachment #8881569 - Flags: review?(liuche)
Comment on attachment 8880893 [details]
Bug 1373174 - Change Leanplum user identification method.

https://reviewboard.mozilla.org/r/152256/#review160662
Attachment #8880893 - Flags: review?(max) → review+
Whiteboard: [LP_M2]
Whiteboard: [LP_M2] → [LP_M1]
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: