Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add API to return sponsored add-ons for the homepage shelf #7966

Closed
diox opened this issue Oct 1, 2020 · 16 comments · Fixed by mozilla/addons-server#15670
Closed

Add API to return sponsored add-ons for the homepage shelf #7966

diox opened this issue Oct 1, 2020 · 16 comments · Fixed by mozilla/addons-server#15670

Comments

@diox
Copy link
Member

diox commented Oct 1, 2020

We need an API to return sponsored add-ons for the homepage shelf. It needs to call Adzerk API to decide which add-ons to show, using the metadata returned to find add-on ids.

On top of the regular add-on serializer info returned, we need to return 2 different additional URLs:

Both are returned by adzerk but needs to be modified to be proxied through our servers (hiding client IPs from Adzerk and letting us implement rate limiting, integrating with the fraud pipeline etc later if needed).

PRD: https://docs.google.com/document/d/19LgWE3Bs_UUE6uV18rwhJOKaMHyPGEoPvg5zJ7gsZwc/edit?pli=1

@bobsilverberg
Copy link
Contributor

If we decide to allow frontend to make just one API call back to addons-server to record impressions, then the impression URL should include all of the data required to make all of the subsequent impression calls to Adzerk. Therefore, we should probably decide on which approach we want to take for the impression callback prior to implementing this.

@eviljeff
Copy link
Member

eviljeff commented Oct 2, 2020

I'm going to focus on the Adzerk integration in this first issue. The serializer will likely just return dummy values for the click and impression urls until #7967 and #7968 are done and we know what the api url will look like.

@bobsilverberg
Copy link
Contributor

I was going to request dummy values for the click and impression URLs so I can use them in mozilla/addons-frontend#9722. The way the code is currently structured, it's actually less changes for the frontend to send one request per add-on for impressions, but I still think that's probably not the way to go.

Because the API is going to return click and impression URLs, and the frontend needs to decide what to do with them, it does seem like we should decide now whether the frontend is going to call a single URL for shelf impressions, or multiple, different URLs (one for each add-on).

What are your thoughts on this, @eviljeff?

@eviljeff
Copy link
Member

eviljeff commented Oct 2, 2020

we should decide now whether the frontend is going to call a single URL for shelf impressions, or multiple, different URLs (one for each add-on).

What are your thoughts on this, @eviljeff?

A single url call will be fine I think. We can work out the format in #7967 though I guess the payload will be some processed variation of 6 * the url adzerk provides*

*e.g. https://e-10521.adzerk.net/i.gif?e=eyJ2IjoiMS42IiwiYXYiOjg2NjU3NCwiYXQiOjUsImJ0IjowLCJjbSI6MTY5MzQ2NiwiY2giOjUwNjA2LCJjayI6e30sImNyIjoxODAyMzY3OCwiZGkiOiI5M*********************************ZDkzIiwic3AiOjE2NjgzLCJzdCI6MTEzMTI0NCwidHIiOnRydWUsInVrIjoidWUxLWVmMmMxNmE1YWU3ZTQ0NTA4NTA3NWEzZWY0MGMxMWEwIiwidHMiOjE2MDE2NDUxODI3NTMsInBuIjoiZGl2MCIsImdjIjpmYWxzZSwiZ0MiOmZhbHNlLCJnaSI6dHJ1ZSwiZ3MiOiJub25lIiwidHoiOiJVVEMiLCJiYSI6MSwiZnEiOjB9&s=-7tSYu0BRk-LGNKoVX2dBLtgBug (* to partially redact it)

@bobsilverberg
Copy link
Contributor

Yeah, the way I see it, the API will return to the frontend a single URL for the entire shelf, and then the frontend will call that when the shelf is displayed.

So the data being returned by this API call will include a single impressionURL, and also multiple clickURLs - one per add-on.

Can we store something on the server so that it can just pass a URL with an id in it to frontend via the API, and then when frontend calls that proxy API the server can look up the actual URL(s) that need to be called for Adzerk? We can't really have the server return the actual Adzerk URLs via the API because that would kind of defeat the point of having the proxy.

@eviljeff
Copy link
Member

eviljeff commented Oct 2, 2020

discussion around impression proxy -> #7967

@eviljeff
Copy link
Member

eviljeff commented Oct 5, 2020

thinking failure cases:

  1. Adzerk doesn't respond before timeout; returns an error; returns an empty response
  2. Adzerk responds with less than 6 addon ids, after removing duplicates
  3. Adzerk responds with 6 addon ids, but at least one is not currently sponsored on AMO (deleted/disabled/no approved for sponsored versions)

for 1. would the fallback be to a fixed set of 6 (sponsored) addons? Or a random selection of 6?
for 2. would we back-fill from a fixed list (or randomly select them)? What if there are less than 6 available currently - should we back-fill with non-sponsored promoted addons? (e.g. verified or recommended or line)
3. is a variation on 2, but is more tricky (we won't know there are less than 6 until we run the ES query) - Ideally treat the same?

@jvillalobos thoughts?

@bobsilverberg
Copy link
Contributor

In terms of how the frontend would deal with these cases, ideally it won't need to care about any of them. I think we can achieve this by having the server return an empty impressionURL in the case where the add-ons are not coming from Adzerk. If we need to get complicated on the server and sometimes return a set of add-ons where some are from Adzerk and some are not, in that case the server can return data for the impressionURL that only includes ids from add-ons known to have been returned from Adzerk. Something similar can be done for the clickURL, where the server can return an empty clickURL if the add-on has not come from Adzerk.

@eviljeff WDYT?

@jvillalobos
Copy link

for 1. would the fallback be to a fixed set of 6 (sponsored) addons? Or a random selection of 6?

We should not display the shelf in that case. I believe the frontend already knows not to show it if the backend doesn't return any add-ons.

for 2. would we back-fill from a fixed list (or randomly select them)?

We shouldn't try to back-fill the response. The homepage shelf should be able to show fewer than 6 add-ons. I think the minimum is 2.

  1. is a variation on 2, but is more tricky

Same.

Overall, we shouldn't be showing anything that doesn't come from Adzerk because (1) it can hide content problems that we should address quickly, (2) it leads to impressions and clicks we aren't accounting for and could throw our analytics off, and (3) there might be a future where AMO doesn't care if an add-on is Sponsored and only cares about the Verified badge, and back-filling would be harder then. I think Adzerk should be treated as the sole source of truth and plan the fallback with that in mind.

@eviljeff
Copy link
Member

eviljeff commented Oct 5, 2020

thanks, not having to return fallback addons makes things more straightforward.

@bobsilverberg
Copy link
Contributor

for 1. would the fallback be to a fixed set of 6 (sponsored) addons? Or a random selection of 6?

We should not display the shelf in that case. I believe the frontend already knows not to show it if the backend doesn't return any add-ons.

Correct, if no add-ons are returned we hide the entire shelf.

for 2. would we back-fill from a fixed list (or randomly select them)?

We shouldn't try to back-fill the response. The homepage shelf should be able to show fewer than 6 add-ons. I think the minimum is 2.

The minimum is 1, because we hide the shelf if there are no add-ons, but there is no explicit minimum set other than that. We do only display 3 if we receive 4 or 5, so we don't end up with 3 on the first row and less than 3 on the second row, but we will display only 1 or 2 if that's what we get.

@ioanarusiczki
Copy link

@eviljeff
I made several requests for each of the APIs below.
Results:

The addons displayed in the responses are sponsored active addons. The addon ids were: 496379, 620119 or 620127.

Let me know if that's the expected result. When I started testing, I personally expected something else, such as seeing 6 addons ("count":6) for https://addons-dev.allizom.org/api/v4/shelves/sponsored/ but using less than 6 for the page_size revealed the results above.

  • "click_url", "click_data" are available.
  • "impression_url", "impression_data" are available at the end of the response.

@eviljeff
Copy link
Member

I was expecting 6 too tbh. @bobsilverberg was planning to add/change 6 sponsored addons from https://addons-dev.allizom.org/api/v4/addons/search/?promoted=sponsored but I'm not sure what the status of that is (there were also super-long caching issues we noticed)

@bobsilverberg
Copy link
Contributor

I was expecting 6 too tbh. @bobsilverberg was planning to add/change 6 sponsored addons from addons-dev.allizom.org/api/v4/addons/search/?promoted=sponsored but I'm not sure what the status of that is (there were also super-long caching issues we noticed)

When we noticed we were having issues getting the new ones returned I stopped changing them, so there may not be 6 on dev right now that have ids that match AMO.

@eviljeff
Copy link
Member

@ioanarusiczki there's 6 showing now - things seem to be cached for a long time.

@ioanarusiczki
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants