Splice strictly only allows certain fields/values, so for related tiles, we'll want to allow type="related" and related=["sites", "list"]. We'll also want to store those values to get served. See also bug 1126182 where Firefox extracts the data. This doesn't really block the Firefox bug as Firefox can implement the functionality while the server keeps serving directory tiles with no related tiles.

Originally I was thinking we would just treat the related attribute, e.g., title, url. Just like how changing a url would result in a new tile id, so would changing the list of relatedness. However, if we expect adding/removing related sites (or other related criteria down the line) to be common, this would generate a lot of tile ids. I'm not sure if we care to differentiate tile {url: urlX, related: [siteA, siteB] } and tile {url: urlX, related: [siteB, siteC] } if all other attributes on the tile are the same. If that's the case, it seems that we'll want to separately track the relatedness. It also looks like we'll want to have a separate attribute for the ad product type here. Currently type can be organic, affiliate, sponsored. Originally I suggested we add a type of "related" but each related tile could be one of organic, affiliate, or sponsored as well. This adtype is also mentioned in splice 1.2 bug 1129138. tspurway, do you have suggestions on how we should store the relatedness of a tile? It would only apply to adtype=related tiles and not directory/enhanced tiles.

The case for generating a new tile_id for each change in the 'related:' field would be to better support A/B testing at the tile level. Another consideration is if we want the same related criteria to be applied to multiple tiles. Our problem is not dissimilar to Google's AdWords, and so maybe a quick review of their data model would be useful (https://support.google.com/adwords/answer/1704395) Essentially the hierarchy is (1--* means one-to-many): Advertiser 1--* Campaigns 1--* Ad Groups 1--* Ads Campaign attributes: budget, pacing, geo and language targeting Ad Group attributes: keywords (and/or related sites, in our case), bids/weights Ads = Tiles This model offers a number of benefits: - clear separation of concerns (who -> budget -> targeting -> tile) - allows easy A/B testing of creatives (different tiles in the same ad group) - allows for separation of targeting optimization (modifying an ad group) from A/B testing of targeting strategies (clone and modify an ad group) - provides a familiar (industry standard?) approach to solving the problem Note that our model may differ in significant ways. For example, in AdWords, geo and locale targeting is done at the Campaign level, where we may find it more useful to target geo at the ad group level, and locale at the tile level as it is currently done.

I want to propose two new tables to the Tiles schema to support this: adgroups = (id, ?locale?) adgroup_sites = (id, adgroup_id, site) In addition, we will need a tiles.adgroup_id field. This will allow us to create the Ad Group 1--* Tiles relationship in addition to allowing us to create the required Sites 1--* Ad Group to support multiple related sites. In addition, this paves the way to create additional future targeting schemes to hang off Ad Groups (eg. Keyword 1--* Ad Group). At the API level, I think we should maintain backward compatibility with our existing splice input and fetch JSON formats, effectively hiding the adgroup abstraction until we start building a proper Splice UI in 1.x One question we should consider is if we move/deprecate tiles.locale up to adgroups.locale

Given the splice interface of taking in distribution.json files, how do we decide when to create an adgroup? { "US/en-US": [ { "title": "related 1", "related": ["siteA", "siteB"] }, { "title": "related 2", "related": ["siteA", "siteB"] } ], "CA/en-US": [ { "title": "related 1", "related": ["siteA", "siteB"] }, { "title": "related 3", "related": ["siteB"] } ] } It seems that we could end up with the following if we assume related 1 and related 2 shared an adgroup adgroup: (1, "en-US") adgroup_site: (1, 1, "siteA") adgroup_site: (2, 1, "siteB") adgroup: (2, "en-US") adgroup_site: (3, 2, "siteB") or the following if we assume related 1/2/3 are all separate adgroups adgroup: (1, "en-US") adgroup_site: (1, 1, "siteA") adgroup_site: (2, 1, "siteB") adgroup: (2, "en-US") adgroup_site: (3, 2, "siteA") adgroup_site: (4, 2, "siteB") adgroup: (3, "en-US") adgroup_site: (5, 3, "siteB") Doing the former requires us to look at the related list of an incoming tile then check if there's an existing adgroup with the same set of sites. Doing the latter could lead to an explosion of adgroups every time we upload a new distribution.json (even though the relatedness might be unchanged). I suppose we could treat adgroups as immutable for now, but we'll want to avoid problems of coalescing same-looking related lists into the same adgroup that would result in accidentally changing the relatedness of an unrelated tile. E.g., related 1 adding siteC maybe shouldn't cause related 2 to also get siteC. Although on the flip side, if related 1 initially targeted 2 tax related sites that related 2 also targeted and we improved our targeting by adding siteC.. we might want related 2 to also target siteC anyway. ?

Right. It seems that we would need a less ambiguous input format for splice ad groups, while keeping ad group data *out* of onyx /fetch output. Here's a suggested format: { "adgroups": [ "turbotax-related": { "locale": "en-US", "sites": ["irs.gov", "hrblock.com", "yourtaxsite.com"]}], "tiles": [ "US/en-US": [ { "title": "related 1", "adgroup": "turbotax-related" }, { "title": "unrelated 2"} ], .... ] } We could use the string "turbotax-related" as the primary key in the table, or we could just store it as the 'name' of the adgroup, and used it to look up the adgroup on ingest (I'm leaning this way). The adgroup_sites table would then be populated by having a composite primary key -> (adgroup_id, url). We could easily remove old adgroups and adgroup_sites on ingest, or in a separate (cron driven?) process.

Looks like a solution that will work. The adgroups definition could be optional if they already exist in the database. Also, in the model above, the same creative (image, title, url, etc.) with different adgroups becomes a new Ad. That would allow for A/B testing and tracking of the different targeting rules. >> One question we should consider is if we move/deprecate tiles.locale up to adgroups.locale Perhaps not? Since adgroups are optional. Note that this implementation depends on Splice 1.2 being deployed. RedShift does not support constraints and/or primary keys. >> We could easily remove old adgroups and adgroup_sites on ingest, or in a separate (cron driven?) process. Maybe I'm not getting this right. When would we remove old adgroups and adgroup_sites? Also, should we allow adgroups/adgroup_sites set to be modifiable? Making them write-once would make record-keeping better (in that we can have perfect history), but it could make code more complex. Instead of removing an adgroup/adgroup_site, we should have some validity parameter. e.g. start/end dates and/or a boolean column. The reason is so that we can keep an audit trail, and also data to do our reporting.

(In reply to Olivier Yiptong [:oyiptong] from comment #6) > Looks like a solution that will work. > > The adgroups definition could be optional if they already exist in the > database. > Yep. > Also, in the model above, the same creative (image, title, url, etc.) with > different adgroups becomes a new Ad. That would allow for A/B testing and > tracking of the different targeting rules. > Yes. > >> One question we should consider is if we move/deprecate tiles.locale up to adgroups.locale > > Perhaps not? Since adgroups are optional. > The other way to look at it is that all targeting is contained in the adgroup, and the decision to show a tile combines all of the targeting data (locale, country, sites, keywords, etc). In this scheme, adgroups are mandatory for all tiles, but may be implicitly created during our 'ingest' process. Here are some ingestion use cases: 1. AdGroup supplied in ingest (remember adgroups contain locale and suggested sites). - the adgroup is created if it doesn't exist (using it's name for lookup). - if the adgroup does exist, we update the set of adgroup_sites associated: - if the ingest lists an adgroup_site that doesn't exist in the db, it adds it - if the ingest lists an adgroup_site that exists in the db, and the adgroup_site is 'inactive', then activate it - if an adgroup_site exists that *isn't* listed in the ingest, then mark it as 'inactive' - this implies a data model like: adgroups(%id, name, locale) 1--* adgroup_sites(%adgroup_id, %url, active) # % indicated PK 2. Tile supplied in ingest without an adgroup name. - if a tile exists (using the current method) in the db with an adgroup reference, ignore the tile - if a tile exists in the db without an adgroup or the tile doesn't exist, create a new tile with a new adgroup (with a generated unique name) and link them 3. Tile supplied in ingest with an adgroup name specified - if the tile exists in the db with the same adgroup as specified in the ingest, ignore the tile - if the tile exists in the db without an adgroup or with a different adgroup as the one specified, create a new tile with a new adgroup (with the specified name) and link them - if the tile doesn't exist in the db and the adgroup does, create a new tile and link it to this adgroup - if neither the tile or the adgroup exist in the db, create a new tile and adgroup (with the specified name) and link them > Note that this implementation depends on Splice 1.2 being deployed. RedShift > does not support constraints and/or primary keys. Well, it certainly doesn't make our job easier, but it's not impossible to manage this 'manually' in RedShift. Maybe we could lock the entire tiles, adgroups and adgroup_sites tables during ingest? > > >> We could easily remove old adgroups and adgroup_sites on ingest, or in a separate (cron driven?) process. > > Maybe I'm not getting this right. When would we remove old adgroups and > adgroup_sites? Ignore my previous comment. I think the usecases above

I say we fry the goldfish and put country_code in the adgroup as well!

After much gnashing of teeth and waving of arms (and coding of code), I feel like the best way to implement the new ingest format it to simply extend the existing one. I am proposing that we modify the rules in comments 5 and 7 thusly: 1. There will be no "adgroups" section or "tiles" section at the top level of the document. There will be a list of geo/locales as it is currently implemented. 2. Every tile will be in exactly one adgroup, either explicitly or implicitly. 3. Explicit adgroup definition. If a tile has an 'adgroup_name' attribute, that tile will use that existing adgroup (fetched from the db), or a new adgroup with that name will be created for that tile (if it doesn't exist), with the specified country_code and locale. It is an error to specify an adgroup_name for a tile that has a different country_code and/or locale than the adgroup it names. If a tile specifies a different adgroup_name than what exists in the database for it, a new tile is created. Note that if the tile specifies an adgroup name, and that adgroup has related_sites, the tile doesn't have to specify those sites again in the ingest (see 5.) 4. Implicit adgroup definition. If a tile doesn't specify an adgroup_name attribute, then it will use the adgroup that was previously defined for the tile. If the tile is new, or no adgroup was assigned to that tile, a new adgroup will be created with the country_code and locale, and a new name will be generated for this adgroup. 5. The related_sites attribute. The tile may also provide a list of sites to be the related sites for this tile. We will determine the adgroup for the tile using 3 and/or 4 and will set the adgroup's related sites to this list. Note that an adgroup's site may be 'active' or 'inactive'. If there are existing sites, then we will 'de-activate' the sites in the set difference (E - N) and append or activate the set intersection (E & N) where E = existing sites, and N = new sites specified in the attribute. To support clients that have old site lists, we will never delete a site from an adgroup, only de-activate it.

Do we want a separate bug for the creation of json files to be served by onyx? Or is this processed during injestion?

Changes to model: - for this version, we are going to force a 1:1 relationship between Adgroup and Tile. This simplifies the implementation, and removes the 'adgroup_name' attribute from the ingest format - change the name of the adgroup.related_sites attribute to adgroup.frecent_sites - when a tile does not specify a set of frecent_sites, then we will disable all frecent_sites for that tile's adgroup

:Mardak, we can handle the distribution/onyx json file format on this bug.

To enable A/B testing of frecent sites, we are adding the following rule/edge case: We will use the existing tile comparison function to decide on whether we create a new tile during ingest, except we will also track all tile_ids parsed. If we encounter a subsequent identical tile on the ingest, we will also consider the list of frecent sites as part of the criteria for the comparison. This will enable us to create copies of tiles with different targeting criteria. For example: { "US/en-US": [ { "title": "related 1", "frecent_sites": ["siteA", "siteB"]}, { "title": "related 1", "frecent_sites": ["siteA", "siteB"]}, { "title": "related 2", "frecent_sites": ["siteC", "siteD"]}, { "title": "related 2", "frecent_sites": ["siteX"]} ] } Would create three tiles for distribution. The two 'related 1' tiles resolve to the same underlying tile, because the related sites are identical. The two 'related 2' tiles will create two tiles for ingestion because they are different.

For the distribution format, we are going to keep the existing format, and add an additional attribute 'frecent_sites'. This attribute is a list of two element lists (tuples). These tuples contain (SITE, site_id) where SITE is the URL string of the site, and the site_id is an integer uniquely identifying that site on the server side. It is assumed that the site_id will be used to report which frecent site caused the impression or click.

Splice currently uploads 4 types of files to S3: * images * distribution file, i.e. the validated ingestion input, with tile ID's assigned (JSON) * geo/locale specific distributions (JSON) * channel index file, optionally, when deployment is requested (JSON) After suggested tiles, we'll need one addition JSON file type: * geo/local specific distributions, with adgroups (JSON) This will contain the new data format, to be consumed by Firefox 38+. The channel index file will need to be modified to provide the choice of the two formats. Here's a proposed format: { "US/en-US": { "legacy": "https://d1nlr0v7fq4juc.cloudfront.net/desktop/US/en-US.2bc87107238e2bd7106d3d2f769dc8d04f27db27.json", "ag": "https://d1nlr0v7fq4juc.cloudfront.net/desktop/US/en-US.ag.[SOMEHASH].json" }, ... }

Reopnening bug, pending code review on github

The hardcoded buckets in Firefox is expecting an exact string match: https://hg.mozilla.org/releases/mozilla-beta/rev/e66ad17db13f#l3.12 The site strings in Firefox are sorted, so a simple fix could be to make sure to frecent_sites is sorted.

The latest code pushed to splice PR 49 adds sorted frecent_sites and a test case to prove it: https://github.com/oyiptong/splice/tree/bug-1126191-related-tiles

Commit pushed to master at https://github.com/mozilla/splice https://github.com/mozilla/splice/commit/c6110270808b82c898b27f72901dbfd23f7a1d71 Related Tiles feature merge closes bug 1126191 closes #49 Squashed commit of the following: commit e51fb12809380ef5e89c384fc93bec8ace40c1ca Author: tspurway <tspurway@gmail.com> Date: Thu Apr 23 11:35:46 2015 -0400 fixed issues with index naming of v3 interface commit c28fe9d4032775b842a898cd6b9dd95706a87461 Author: tspurway <tspurway@gmail.com> Date: Wed Apr 22 10:26:27 2015 -0400 fixed @oyiptong 's issues with PR#49, which were mostly commented code nits commit 03ffd018d58f380e29a44a8d04a3019ed9eac2f0 Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 11:40:13 2015 -0400 adding a 'ver' version key to the index file. this helps onyx be backward compatible commit de01e4369f1de253558615a809b95357680baf1c Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 11:34:38 2015 -0400 adding a 'ver' version key to the index file. this helps onyx be backward compatible commit 151ced04cb12c3ac0b70b1f434ae6c0a11ba06bc Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 09:54:25 2015 -0400 ensuring that frecent_sites are sorted and deduplicated commit 4a4f96afe9343bc91a59e06687bd39cc01892be0 Author: tspurway <tspurway@gmail.com> Date: Mon Apr 20 10:54:58 2015 -0400 fixing tests for local postgres. hopefully will work in CI environments! @oyiptong r? commit 07cbcb08f559782b50be52ef8a63396702d37f59 Merge: 9c1557b 4581fd5 Author: tspurway <tspurway@gmail.com> Date: Fri Apr 17 11:13:26 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles Conflicts: tests/base.py tests/fixtures/tiles.csv commit 9c1557b7e832576e608fa8ad93cc04e0b4bab41c Merge: b91295c 4b597fb Author: Olivier Yiptong <olivier@olivieryiptong.com> Date: Thu Apr 2 16:01:57 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit b91295c2f1d1eeb29004e569e1a43a51d3435de0 Author: tspurway <tspurway@gmail.com> Date: Thu Apr 2 14:56:33 2015 -0400 implemented v2 and v3 separate distribution files, update tile_index to point to appropriate files @oyiptong r? commit cc306209ca82663345fc318250c1eda1e991b61d Merge: 694a3ae a7f14eb Author: tspurway <tspurway@gmail.com> Date: Mon Mar 30 10:50:59 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit 694a3aebda3badbd96f4f27c62d931451e806ea8 Author: tspurway <tspurway@gmail.com> Date: Sat Mar 28 17:46:19 2015 -0400 oops mistakenly committed my local settings commit 17e878b49c7791c6a2502b509e30ae0e7afcdee0 Author: tspurway <tspurway@gmail.com> Date: Sat Mar 28 15:27:46 2015 -0400 done ingest and related tests. commit f7d1a1c0281beebda40c21c02c47578c83f96bef Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 22:56:47 2015 -0400 done ingest and related tests. commit 2454c544b6399a99e0eda2bcd5c2bdd29ef68666 Merge: 5408c67 4d58c40 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 21:02:23 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit 5408c67f1db2582a54d2c026b3f63bc7258679e6 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 20:59:29 2015 -0400 refactored country_code out of adgroup for now. all unit tests pass. need to add some adgroup specific tests, and merge in master commit b780aeec3b1f092ad427db736f1839527cb8ff9a Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 15:32:11 2015 -0400 getting tests up-to-snuff. going to try to refactor country_code out of adgroup. commit 7615edd2194ba446532ef84ca7ff49decd5dc4ff Author: tspurway <tspurway@gmail.com> Date: Mon Mar 23 12:50:09 2015 -0400 got ingest working for frecent_sites. still need to get unit tests up-to-date commit dc7e03be43b79974524c665ff218bbdbba9df749 Author: tspurway <tspurway@gmail.com> Date: Thu Mar 5 15:35:44 2015 -0500 adding migrations for adgroups commit 64d2069fe458cf4e06bdc656a36e6ea676eb1e1b Merge: 4bbc7b1 c153bd7 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 3 11:05:43 2015 -0500 Merge branch 'master' into bug-1126191-related-tiles Conflicts: splice/web/api/authoring.py commit 4bbc7b13fb9cc368a6ed7b6805a97be63737c9aa Author: tspurway <tspurway@gmail.com> Date: Tue Mar 3 11:03:38 2015 -0500 changing the ingest format to reflect new related tile feature commit a938b39ed16b821cb2cd2003ce177f11a0a7e6fd Author: tspurway <tspurway@gmail.com> Date: Thu Feb 19 11:32:28 2015 -0500 adding related tiles to ingestion routines/tests

Commit pushed to master at https://github.com/mozilla/splice https://github.com/mozilla/splice/commit/1f65ab1765bd2d896bfc76b2d24fcc403b5109dd Related Tiles feature merge closes bug 1126191 closes #49 Squashed commit of the following: commit e51fb12809380ef5e89c384fc93bec8ace40c1ca Author: tspurway <tspurway@gmail.com> Date: Thu Apr 23 11:35:46 2015 -0400 fixed issues with index naming of v3 interface commit c28fe9d4032775b842a898cd6b9dd95706a87461 Author: tspurway <tspurway@gmail.com> Date: Wed Apr 22 10:26:27 2015 -0400 fixed @oyiptong 's issues with PR#49, which were mostly commented code nits commit 03ffd018d58f380e29a44a8d04a3019ed9eac2f0 Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 11:40:13 2015 -0400 adding a 'ver' version key to the index file. this helps onyx be backward compatible commit de01e4369f1de253558615a809b95357680baf1c Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 11:34:38 2015 -0400 adding a 'ver' version key to the index file. this helps onyx be backward compatible commit 151ced04cb12c3ac0b70b1f434ae6c0a11ba06bc Author: tspurway <tspurway@gmail.com> Date: Tue Apr 21 09:54:25 2015 -0400 ensuring that frecent_sites are sorted and deduplicated commit 4a4f96afe9343bc91a59e06687bd39cc01892be0 Author: tspurway <tspurway@gmail.com> Date: Mon Apr 20 10:54:58 2015 -0400 fixing tests for local postgres. hopefully will work in CI environments! @oyiptong r? commit 07cbcb08f559782b50be52ef8a63396702d37f59 Merge: 9c1557b 4581fd5 Author: tspurway <tspurway@gmail.com> Date: Fri Apr 17 11:13:26 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles Conflicts: tests/base.py tests/fixtures/tiles.csv commit 9c1557b7e832576e608fa8ad93cc04e0b4bab41c Merge: b91295c 4b597fb Author: Olivier Yiptong <olivier@olivieryiptong.com> Date: Thu Apr 2 16:01:57 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit b91295c2f1d1eeb29004e569e1a43a51d3435de0 Author: tspurway <tspurway@gmail.com> Date: Thu Apr 2 14:56:33 2015 -0400 implemented v2 and v3 separate distribution files, update tile_index to point to appropriate files @oyiptong r? commit cc306209ca82663345fc318250c1eda1e991b61d Merge: 694a3ae a7f14eb Author: tspurway <tspurway@gmail.com> Date: Mon Mar 30 10:50:59 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit 694a3aebda3badbd96f4f27c62d931451e806ea8 Author: tspurway <tspurway@gmail.com> Date: Sat Mar 28 17:46:19 2015 -0400 oops mistakenly committed my local settings commit 17e878b49c7791c6a2502b509e30ae0e7afcdee0 Author: tspurway <tspurway@gmail.com> Date: Sat Mar 28 15:27:46 2015 -0400 done ingest and related tests. commit f7d1a1c0281beebda40c21c02c47578c83f96bef Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 22:56:47 2015 -0400 done ingest and related tests. commit 2454c544b6399a99e0eda2bcd5c2bdd29ef68666 Merge: 5408c67 4d58c40 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 21:02:23 2015 -0400 Merge branch 'master' into bug-1126191-related-tiles commit 5408c67f1db2582a54d2c026b3f63bc7258679e6 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 20:59:29 2015 -0400 refactored country_code out of adgroup for now. all unit tests pass. need to add some adgroup specific tests, and merge in master commit b780aeec3b1f092ad427db736f1839527cb8ff9a Author: tspurway <tspurway@gmail.com> Date: Tue Mar 24 15:32:11 2015 -0400 getting tests up-to-snuff. going to try to refactor country_code out of adgroup. commit 7615edd2194ba446532ef84ca7ff49decd5dc4ff Author: tspurway <tspurway@gmail.com> Date: Mon Mar 23 12:50:09 2015 -0400 got ingest working for frecent_sites. still need to get unit tests up-to-date commit dc7e03be43b79974524c665ff218bbdbba9df749 Author: tspurway <tspurway@gmail.com> Date: Thu Mar 5 15:35:44 2015 -0500 adding migrations for adgroups commit 64d2069fe458cf4e06bdc656a36e6ea676eb1e1b Merge: 4bbc7b1 c153bd7 Author: tspurway <tspurway@gmail.com> Date: Tue Mar 3 11:05:43 2015 -0500 Merge branch 'master' into bug-1126191-related-tiles Conflicts: splice/web/api/authoring.py commit 4bbc7b13fb9cc368a6ed7b6805a97be63737c9aa Author: tspurway <tspurway@gmail.com> Date: Tue Mar 3 11:03:38 2015 -0500 changing the ingest format to reflect new related tile feature commit a938b39ed16b821cb2cd2003ce177f11a0a7e6fd Author: tspurway <tspurway@gmail.com> Date: Thu Feb 19 11:32:28 2015 -0500 adding related tiles to ingestion routines/tests