Closed Bug 1423046 Opened 6 years ago Closed 6 years ago

Drop an event when User opens the Firefox app (from Leanplum contextual hints)

Categories

(Firefox for Android Graveyard :: Metrics, enhancement, P1)

Other
iOS
enhancement

Tracking

(firefox60 verified, firefox61 verified)

VERIFIED FIXED
Firefox 61
Tracking Status
firefox60 --- verified
firefox61 --- verified

People

(Reporter: jcollings, Assigned: petru)

References

Details

(Whiteboard: [Leanplum] [61])

Attachments

(1 file, 2 obsolete files)

Looking to create an event when a user opens the app. This will help us figure which message creates higher conversions.

We originally did not include this as an event because we were using LP's version but now we need to drop this event ourselves to be able to create targeting segments.
Hi Jean, this is referring to opening Firefox App from Push notifications right? Thanks
Flags: needinfo?(jcollings)
This is referring to a user opening an app in general. This is so we can do targeting (low engaged vs mid engaged vs high engaged - in which we specify "recent occurrences of a user opening the app in the last 7 days is between x and y days". 

We need the event of a user opening an app in order to do the above targeting.
Flags: needinfo?(jcollings)
Hi Joe, Wesly
Please help prioritize this.
Flags: needinfo?(wehuang)
Flags: needinfo?(jcheng)
Priority: -- → P3
Joe, would you help confirm if this is our priority for Fx60? (the coming Nightly cycle) Thanks.
Flags: needinfo?(wehuang)
[triage] Bulk edit: product thinks leanplum is high priority: P1 rank 1.
Rank: 1
Priority: P3 → P1
Whiteboard: [Leanplum] [61]
Assignee: nobody → petru.lingurar
Jean, I see that we already inform LP about the app launch with 2 events, depending if Fennec is default or not
- E_Launch_Browser
- E_Launch_But_Not_Default_Browser

Do you need another added, unaffected about the default browser status or are these what we want?
Flags: needinfo?(jcheng) → needinfo?(jcollings)
(In reply to Petru-Mugurel Lingurar[:petru] from comment #6)
> Jean, I see that we already inform LP about the app launch with 2 events,
> depending if Fennec is default or not
> - E_Launch_Browser
> - E_Launch_But_Not_Default_Browser
> 
> Do you need another added, unaffected about the default browser status or
> are these what we want?

Petru if you can confirm E_Launch_Browser is the same as opening the Firefox app (regardless of first run), then we can close this!
Flags: needinfo?(jcollings)
(In reply to Jean Collings from comment #7)
> Petru if you can confirm E_Launch_Browser is the same as opening the Firefox
> app (regardless of first run), then we can close this!

> - E_Launch_Browser(sent if Fennec is default browser)
and
> - E_Launch_But_Not_Default_Browser (sent if Fennec is NOT default browser)
are logged to LeanPlum everytime the app starts, if it was previously closed.

If the app is resumed from background this events will not be logged.

Is this what we needed?
Flags: needinfo?(jcollings)
Is there a way to track if app is resumed from background?

If not, then this will be just fine.
Flags: needinfo?(jcollings)
(In reply to Jean Collings from comment #9)
> Is there a way to track if app is resumed from background?
> 
> If not, then this will be just fine.

Just pushed a patch to make sure LP will be informed with "E_Launch_Browser" basically everytime the app starts (new instance/resumed from background)

This will indeed help to see clearly when was the last time the app was "started".
Attachment #8964821 - Flags: review?(michael.l.comella) → review?(sdaswani)
Comment on attachment 8964821 [details]
Bug 1423046 - Drop an event when User opens the Firefox app (from Leanplum contextual hints);

https://reviewboard.mozilla.org/r/233552/#review239268

Simple fix.
Attachment #8964821 - Flags: review?(sdaswani) → review+
Keywords: checkin-needed
Autoland can't push this because the patch doesn't meet MozReview's review requirements.
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html#landing-commits
Flags: needinfo?(michael.l.comella)
Ryan am I not allowed to review essentially since I'm not L3?
Flags: needinfo?(ryanvm)
If you want to use MozReview & Autoland, that's correct.
Flags: needinfo?(ryanvm)
Attachment #8964821 - Flags: review?(michael.l.comella)
I'm unclear what Jean needs for this bug.

(In reply to Jean Collings from comment #0)
> Looking to create an event when a user opens the app. This will help us
> figure which message creates higher conversions.

Petru mentions we have the current events:
- E_Launch_browser when fennec is the default browser and opened from an OOM
- E_Launch_But_Not_Default_Browser when fennec is not the default browser and opened from an OOM

i.e. these are not sent when fennec is resumed from the background.

(In reply to Jean Collings from comment #9)
> Is there a way to track if app is resumed from background?
> 
> If not, then this will be just fine.

It sounds like Jean is happy with the current probes (though I don't know how having "a way to track if app is resumed from background" changes whether or not the existing probes are fine so I wonder if there's a typo here).

However, whether or not the system OOM's the app is largely transparent to the user so I would think we would want our probes to track the app being resumed from the background.

> Just pushed a patch to make sure LP will be informed with "E_Launch_Browser"
> basically everytime the app starts (new instance/resumed from background)

Petru's patch does two things:
- It adds a probe that I'm not sure if Jean wants
- It changes the behavior of an existing probe which might be used for other purposes

I'm concerned about both of these: Jean, does my explanation make sense? What are you trying to achieve? We could take this to IRC/Slack/Vidyo to make this easier.
Flags: needinfo?(michael.l.comella) → needinfo?(jcollings)
I think Jean wants a backgrounded app to report that it's been resumed. Right Jean? But I do wonder if Petru's change should maintain the 'Launch' events and add a new one e.g., like 'Resumed_From_Background'?

Also Jean note that 'Last Active' is sent by Leanplum whether the app is launched from a cold start or resumed from the background (you can see that with my release device id dc463990-4ffe-4848-8d98-093eaa92c2ea)
I'm fine keeping them as separate events but eventually I'll need to build target segments so I can send specific messages based on activity. For example "high active" users which in this case would be defined in Leanplum as:

Recent occurrences of E_Launch_Browser OR E_Launch_But_Not_Default OR E_Resumed_From_Background in the last 7 days is more than X sessions

A low active user would be defined as:

Recent occurrences of E_Launch_Browser OR E_Launch_But_Not_Default OR E_Resumed_From_Background in the last 7 days is less than X sessions

Susheel - Last Active is helpful for some use cases but doesn't help me with the targeting.
Flags: needinfo?(jcollings)
Attachment #8964821 - Attachment is obsolete: true
Attachment #8964821 - Flags: review?(michael.l.comella)
Attachment #8964821 - Attachment is obsolete: false
Jean, I've sent for review a new patch for sending "E_Resumed_From_Background" when the app is resumed from background.

Please note that "E_Launch_Browser" is sent everytime the app starts, is just that "E_Launch_But_Not_Default" is also sent if Fennec is not default.
So to regard a user as "high active" it would be sufficient to just check :
> Recent occurrences of E_Launch_Browser OR E_Resumed_From_Background in the last 7 days is more than X sessions
And to regard him as "low active":
> Recent occurrences of E_Launch_Browser OR E_Resumed_From_Background in the last 7 days is less than X sessions
Attachment #8965263 - Flags: review?(sdaswani) → review?(michael.l.comella)
Petru, thanks for the patch. Did you test you are seeing the intended behavior you describe in the LP event log for this device?
Flags: needinfo?(petru.lingurar)
iiuc, it sounds like what Jean wants for this bug is to be able to create targeting segments for "high active" user and "low active" user (comment 18).

Jean, it sounds like Last Active fits this use case: it is equivalent to "E_Launch_Browser OR E_Launch_But_Not_Default OR E_Resumed_From_Background". Why doesn't it help with targeting?

That being said, as Petru describes in comment 20, this patch will add an additional probe, E_Resumed_From_Background, that lets us identify "high active" and "low active" users in a similar way to what Jean suggests. I'll review it and after Jean responds, we can figure out if the additional probe is necessary.
Flags: needinfo?(jcollings)
Hi Michael,

"Last Active" targeting doesn't let you add "in how many days". If I wanted to see a high active or low active user based on the number of sessions in the last x days, unfortunately "Last Active" won't help with that.
Flags: needinfo?(jcollings)
These two patches are correct together, however, mozreview doesn't consider them a patch series so I can't land them as is.

Petru, can you squash the two patches so I can r+ and land?

For future reference, you'll want to use hg bookmarks, or a similar mechanism, to make changesets in sequence and push these bookmarks to mozreview as a single unit, e.g.:

* Bug 1423046 part 2 <- bookmark
* Bug 1423036 part 1
* ...                <- mozilla-central / master

Our official mercurial guide [1] probably contains the details but there's a lot of information in there.

[1]: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/index.html
Petru, please also add this probe to the documentation, as per bug 1452173.

Chenxia, does this need data review?
Flags: needinfo?(liuche)
Kat, I see that Jean has deferred Leanplum to you, can you update the Leanplum wiki with documentation of this? And also fill out a data review request (this should be pretty simple: https://github.com/mozilla/data-review/blob/master/request.md )
Flags: needinfo?(liuche) → needinfo?(kplam)
(In reply to Michael Comella (:mcomella) [needinfo or I won't see it] from comment #25)
> Petru, please also add this probe to the documentation, as per bug 1452173.

Updated project's LP documentation to mention the new attribute as per bug 1452173. Thanks!
Flags: needinfo?(petru.lingurar)
Attachment #8965263 - Attachment is obsolete: true
Attachment #8965263 - Flags: review?(michael.l.comella)
Attachment #8966654 - Flags: review?(sdaswani) → review?(michael.l.comella)
Comment on attachment 8966654 [details]
Bug 1423046 - Drop an event when User opens the Firefox app (from Leanplum contextual hints);

https://reviewboard.mozilla.org/r/235358/#review241154

lgtm but needs documentation: I filed bug 1453154 to land this so we can keep rolling. Please take care of it when you get a chance!
Attachment #8966654 - Flags: review?(michael.l.comella) → review+
Pushed by michael.l.comella@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5f20da3f845d
Drop an event when User opens the Firefox app (from Leanplum contextual hints); r=mcomella
Kat, should we be blocking landing these until Data Review is complete? The reason I chose to land it now is that I'm the only one with commit access and it'd be time consuming for me to constantly check back in on data review status in order to land: if we need to block until data review completes, please let me know if you have any ideas on how we can solve this issue!
https://hg.mozilla.org/mozilla-central/rev/5f20da3f845d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment on attachment 8966654 [details]
Bug 1423046 - Drop an event when User opens the Firefox app (from Leanplum contextual hints);

Approval Request Comment
[Feature/Bug causing the regression]: We did instrument LP to drop an event when the user opens the app from cold start, but this didn't cover the case where they open the up from the background / warm start.
[User impact if declined]: They may not get the most appropriate messaging since we don't distinguish between the two launch profiles.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Devs have confirmed it on dev, I have NI'ed Bogdan to verify on Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: Pretty simple - launch from warm start with LP enabled and make sure the event shows up.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It just drops an event at a specified juncture through LP - it doesn't modify or add UX / processing.
[String changes made/needed]:
Flags: needinfo?(bogdan.surd)
Attachment #8966654 - Flags: approval-mozilla-beta?
(In reply to Michael Comella (:mcomella) [needinfo or I won't see it] from comment #31)
> Kat, should we be blocking landing these until Data Review is complete? The
> reason I chose to land it now is that I'm the only one with commit access
> and it'd be time consuming for me to constantly check back in on data review
> status in order to land: if we need to block until data review completes,
> please let me know if you have any ideas on how we can solve this issue!

I apologize for the delay. I asked several people for help on this yesterday and I didn't get anywhere. I not clear on what I'm supposed to do and haven't been able to get any clarity on next steps.
Flags: needinfo?(kplam)
Devyani (:dgupta) reached out to me yesterday and we discussed what the steps are so feel free to ask her (or me!) if you need more info.
Vlad can you get the docs updated on this one ASAP? Also request data review via https://github.com/mozilla/data-review/blob/master/request.md
Flags: needinfo?(vlad.baicu)
Submitted bug 1453964 for the needed data collection review.
Flags: needinfo?(vlad.baicu)
Comment on attachment 8966654 [details]
Bug 1423046 - Drop an event when User opens the Firefox app (from Leanplum contextual hints);

Leanplum patch needed for product reasons. Approved for 60.0b13.
Attachment #8966654 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Thanks Ryan!
Copying the data review request and duping bug here. (Data reviews should be comments in the bug adding the data probe, not a separate bug.)

>1) What questions will you answer with this data?
When the app has been resumed from background.

>2) Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? 
Provide essential information needed for better MMA targeting (based on user engagement with the app: low engaged vs mid engaged vs high engaged)
    
>3) What alternative methods did you consider to answer these questions? Why were they not sufficient?
The proposed implementation is the only way to get the needed info.

>4) Can current instrumentation answer these questions?
No, this is the first time we are logging this kind of data.

>5) List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories on the Mozilla wiki.
 - Measurement Description: We only need to know when the application is resumed from background.	
 - Data Collection Category: Category 1 “Technical data”  	
 - Tracking Bug #1423046
		
>6) How long will this data be collected? Choose one of the following:
I want to permanently monitor this data.

>7) What populations will you measure?
Firefox for Android users which have the LeanPlum feature available and have enabled Firefox Health Report.
 - Which release channels?: All
 - Which locales?: LeanPlum feature is currently available for users using "eng|zho|deu|fra|ita|ind|por|spa|pol|rus"

>8) If this data collection is default on, what is the opt-out mechanism for users?
Data collection is default on if the users have the LeanPlum feature available.
Users can opt-out of this by turning off Firefox Health Report.

>9) Please provide a general description of how you will analyze this data.
Data will be used to better understand users engagement with the app and for better MMA targeting.

>10) Where do you intend to share the results of your analysis?
Data will be available only to specific persons within Mozilla with LeanPlum dashboard access.
data review+

> 1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Pending documentation, I'm in conversation with :kplam and :dgupta about it.
https://wiki.mozilla.org/Leanplum_Contextual_Hints

> 2) Is there a control mechanism that allows the user to turn the data collection on and off? (Note, for data collection not needed for security purposes, Mozilla provides such a control mechanism) Provide details as to the control mechanism available.

Yes, Firefox data controls

> 3) If the request is for permanent data collection, is there someone who will monitor the data over time?**

Permanent, to track engagement. Jean Collings (jcollings@mozilla.com) will monitor over time. :kplam will monitor in the meantime.

There is a hypothesis, but currently no campaign that will immediately use this data. :kplam and :dgupta have it in their roadmap to add it to a campaign.

> 4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Type 2, user resuming app

> 5) Is the data collection request for default-on or default-off?

Default on

> 6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No

> 7) Is the data collection covered by the existing Firefox privacy notice? **If unsure: escalate to legal if:**

Yes

> 8) Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**

No, follows other LeanPlum campaign/data collection
Device:
 - Samsung Galaxy S8 (Android 8.0);

Hello,

Tested this in the latest Nightly (2018-04-17) and Beta (60.0b13) builds and everything works as expected. Marking as verified.
Status: RESOLVED → VERIFIED
Flags: needinfo?(bogdan.surd)
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

Created:
Updated:
Size: