Distribution referrer information is not sent to Adjust

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Firefox 51
Points:
---

Firefox Tracking Flags

(firefox49+ fixed, firefox50+ fixed, firefox51+ fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Currently when we receive an INSTALL_REFERRER, we only report it to Adjust if it is not a distribution.

See:

https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/distribution/ReferrerReceiver.java#60

This means that we are not getting any distribution referrer information into Adjust.

We should send it to Adjust as well as handling it internally.
This is the best patch I could come up with. Alternative was just to put the adjust call with the distribution.

I have no idea if we should be invoking Adjust when we do propagateMozillaCampaign

Also as a side note, BLP is not a common acronym as far as I know. We should probably replace it.
Attachment #8780706 - Flags: review?(rnewman)
Comment on attachment 8780706 [details] [diff] [review]
Send Adjust info on distribution as well

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

I believe this code is this way because of Bug 1143888 Comment 2 -- we didn't want to track distribution installs.

If that's no longer the case (and I don't have context to answer that question), then just move that block above the early return so that the Adjust code always runs.

::: mobile/android/base/java/org/mozilla/gecko/distribution/ReferrerReceiver.java
@@ +57,5 @@
>          // Track the referrer object for distribution handling.
>          ReferrerDescriptor referrer = new ReferrerDescriptor(intent.getStringExtra("referrer"));
>  
> +        if (!TextUtils.equals(referrer.source, MOZILLA_UTM_SOURCE) ||
> +            (TextUtils.equals(referrer.source, MOZILLA_UTM_SOURCE) &&

The first half of this `&&` is redundant. But if your goal is to make Adjust tracking always fire, why not just move the entire `try..catch` block out to before the conditional?
Attachment #8780706 - Flags: review?(rnewman) → review-
> I believe this code is this way because of Bug 1143888 Comment 2 -- we didn't want to track distribution installs.

Do you have any background for why that was believed?

The reason I'm turning this on is because we're getting explicit requests from partners to know when their OTA distribution are coming through the store.

> The first half of this `&&` is redundant. But if your goal is to make Adjust tracking always fire, why not just move the entire `try..catch` block out to before the conditional?

I thought about that, but I wasn't sure if Adjust should fire in the other two cases. Was there a reason for the early return?
(In reply to Mike Kaply [:mkaply] from comment #3)

> Do you have any background for why that was believed?

This was 18 months ago, so it's hazy. Presumably because our default is to not subject users to tracking, and we would have other mechanisms for recording installs for partners… our only reason for using Adjust was to track marketing spend through social networks.


> > The first half of this `&&` is redundant. But if your goal is to make Adjust tracking always fire, why not just move the entire `try..catch` block out to before the conditional?
> 
> I thought about that, but I wasn't sure if Adjust should fire in the other
> two cases. Was there a reason for the early return?

The previous logic was:

  * If we have a non-Mozilla referrer source, do nothing.
  * If we have a Mozilla source:
    * If this is an OTA distribution, install it.
    * Otherwise, this must be a regular Mozilla link campaign; give the referrer to Gecko to include with our main blocklist ping stats.

When Adjust was added, that became:

  * It's a Mozilla source for a distro: install it.
  * It's a Mozilla source for a campaign: track it in BLP.
  * It's a non-Mozilla source (typically an organic or social media install); track through Adjust.

If you now want to track all sources, and you're confident that you won't be perturbing anyone's analysis or going beyond our privacy remit, then simply lift the Adjust call to the top level (perhaps in a try..finally block so we do our other stuff first).
Status: NEW → ASSIGNED
My concern was the last path. I think we would end up calling Adjust for our tests, wouldn't we?

The only source I'm trying to add to our tracking is distros...

As a side note, why are we tracking Mozilla sources through BLP instead of using Adjust?
(In reply to Mike Kaply [:mkaply] from comment #5)
> My concern was the last path. I think we would end up calling Adjust for our
> tests, wouldn't we?

That's a problem in general: e.g., Bug 1270191.

Worth looking into.


> The only source I'm trying to add to our tracking is distros...

Can you go into more detail?

If you're interested in tracking preinstall distros, not just OTA downloads, then this change is probably not enough -- I don't think they get INSTALL_REFERRER at all.

If you're trying to track successful OTA installs, then you should probably leave only a comment in this code and touch the distro downloader.

If you're trying to track all referred installs, then do this at the top level, and make sure Adjust doesn't run in tests.


> As a side note, why are we tracking Mozilla sources through BLP instead of
> using Adjust?

Our ADI figures, including distributions and campaigns, on all platforms (last I checked) come from BLP. When Adjust was integrated, it was entirely to track installs, not ADIs -- we made sure it only ever sent one ping. We're generally interested in use of distributed Firefoxes over time, but Adjust is/was entirely targeted at maximizing ad spend-to-install conversion.
Unfortunately it's not easy to call Adjust from the distribution code because I need the intent and we are only passing the referrer.

What about something simple like this?

Just adding an extra Adjust call in the distribution case only.

I know it duplicates the Adjust call above, but it seems the most simple
Attachment #8782167 - Flags: review?(rnewman)
Comment on attachment 8782167 [details] [diff] [review]
Call Adjust in the distribution case as well

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

Sure, but let's add that call *after* we trigger the Distribution code, not before.

And please test the URL decoding change with real Play store referrer installs before this rolls into Aurora and Beta!
Attachment #8782167 - Flags: review?(rnewman) → review+
This was the checked in patch. It was only the Adjust change, moved as requested by rnewman.

I'm preemptively asking for this for mozilla-beta because we need this for partner tracking.

Approval Request Comment
[Feature/regressing bug #]: Send distribution campaign information to Adjust
[User impact if declined]: Unable to track distribution campaigns through play store.
[Describe test coverage new/current, TreeHerder]: No additional test coverage.
[Risks and why]: Low. Adds additional tracking in a try/catch.
[String/UUID change made/needed]: None
Attachment #8780706 - Attachment is obsolete: true
Attachment #8782167 - Attachment is obsolete: true
Attachment #8785310 - Flags: review+
Attachment #8785310 - Flags: approval-mozilla-beta?
How do we test this on aurora or beta as rnewman suggests in comment 8? And who tests it... Mike or Ioana/someone else from QE?     If you can let me know over the weekend, I can check back Sunday night to get this into Monday morning's beta 8 build.
Flags: needinfo?(mozilla)
Flags: needinfo?(ioana.chiorean)
Tracking and marking affected for 49/50 since this sounds like an urgent last minute change.
And I apologize profusely. Tried to make it not last minute.

To be clear, what rnewman is suggesting testing is not this bug, but it's bug 1294763 which I accidentally mingled in the patch, not in the checkin. So this patch cannot break google play.

This patch is simply an additional datasend.

The only way to test it is to install a distribution and then check our Adjust data.
Flags: needinfo?(mozilla)
https://hg.mozilla.org/mozilla-central/rev/25b297ea1c36
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Also uplifting to aurora?
Flags: needinfo?(mozilla)
Yes, Aurora as well. Thanks.
Flags: needinfo?(mozilla)
Comment on attachment 8785310 [details] [diff] [review]
Patch as checked in

Fix for distribution reporting, please uplift to aurora and beta. 
Sounds like we have to test this live.
Attachment #8785310 - Flags: approval-mozilla-beta?
Attachment #8785310 - Flags: approval-mozilla-beta+
Attachment #8785310 - Flags: approval-mozilla-aurora+
Liz, as mentioned yesterday we never tested distribution builds. 
If we have some clear steps, I can look at it nevertheless.
Flags: needinfo?(ioana.chiorean)
> Liz, as mentioned yesterday we never tested distribution builds. 
If we have some clear steps, I can look at it nevertheless.

Sure.

The easiest way to do it is to follow the instructions here:

https://wiki.mozilla.org/Mobile/Distribution_Files#Testing_distribution_download

You can run that adb command and it will install a test distribution bundle.

For this to work, Firefox must have never been started or be started for the very first time (clear the app data).
You need to log in before you can comment on or make changes to this bug.