ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.9k stars 3.89k forks source link

✨Tracking issue for new story ads placement algorithm #33147

Open calebcordry opened 3 years ago

calebcordry commented 3 years ago

We are exploring options to improve the ad placement logic in <amp-story-auto-ads> especially to help monetize shorter stories or shorter sessions, while still maintaining user experience goals.

Pre-work

New work

cc/ @ampproject/wg-monetization

kristoferbaxter commented 3 years ago

This appears to be a great topic for design review. Could you bring the idea there?

dcombe commented 2 years ago

@calebcordry @kristoferbaxter Any update on this ?

End I had a question, is the following function really currently used in LTS to determine if stories can display ads where they have more than 4 pages (in the merged to LTS in https://github.com/ampproject/amphtml/pull/33377). It seems the function is defined but not used in the getNumberOfAds. isStoryEligible() { const storyLength = this.storyPageIds_.length; return storyLength > BEGINNING_OF_STORY_BUFFER + END_OF_STORY_BUFFER; }

Moreover, as far as I understand, according to the code, it is not >=4 pages (as mentioned here: https://github.com/ampproject/amphtml/issues/33547) but >=5pages.

EDIT: After checking other source files, I found https://github.com/ampproject/amphtml/blob/114e330e5036f59e1e604aba9eb05091dd2fb75d/extensions/amp-story-auto-ads/0.1/algorithm-utils.js, that seems to only use on experimental branch the algorithm that permit >=5 pages (algorithm-predetermined). It would significate that currently we are still using the older one (>=8) . Am I right ?

calebcordry commented 2 years ago

Hi @dcombe you are correct, this is not launched yet and we are still running it on a small portion of traffic. We are working on getting it launched but it may be another few weeks. I will update this thread when it is fully launched.

dcombe commented 2 years ago

Ok @calebcordry thanks. And am I right it will be >= 5 and not >=4 as mentioned here ?

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.