-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
I2I - updates and new logic for sizeConfig feature #4129
Comments
@harpere and I dug into this and while we appreciate that quite a bit of work was put into the proposal, we have a couple of observations and reservations. First, this approach isn't any easier to understand than the current system. One of the complaints people had about the original design was that they didn't get it. The other complaint was lack of flexibility. In both cases, a root problem was the assumption that a global sizeConfig would fit. This model still uses a global sizeConfig, and adds invisible functionality to every adunit (label or no) that then requires awkward flags (like VIP) to control edge cases. Second, the approach as documented isn't backwards compatible. Everyone who has an existing sizeconfig would have to change, in a major and fairly complicated way, completely flipping the sense of the "join" between sizeConfig and adunits. Related, the proposal as documented also flips the definition of One modification that would address both of the previous concerns is to keep But even doing that, we don't think this proposal does what #3226 asks for, which is to provide straightforward controls similar to a certain well-known ad server. So here's an alternate proposal, which achieves full backwards-compatibility and infinite control using a paradigm already familiar to everyone managing scenarios complex enough to complain about v1. It also opens the door to supporting the concept for video.
Here's an example showing this approach is pretty straightforward to read, since all the relevant info is together -- no scrolling up to another data structure:
Advantages:
Drawbacks:
Here's your full Example 1 converted to the alternate approach:
|
Thanks for the feedback and consideration. I do understand your points against the original proposal in its attempts 'to stay the same and be different'. The suggested alternative has merit, though I need to digest it a bit more. A few initial questions:
|
|
Agree with what's been said:
|
Just to confirm, I understand the desire to not impact existing users too much by proposing that we keep the existing function intact while adding this new logic. But how long would we plan to maintain the 'current' logic? Are we eventually going to deprecate the I wanted to explicitly clarify this point, since the feedback so far is that it appears to be staying for good. If so, why? The new logic appears to cover the use-cases of the current logic; there doesn't appear to be anything uniquely done by the 'current' approach that won't be possible with the new approach. Maintaining both approaches makes sense if we're going to eventually deprecate the one (as we can setup the proper logic in the code and present the proper message in documentation). But if they're both going to stay, what would be the reason publishers want to use the 'current' logic over the 'new' logic? |
It's a good question, but one I don't think we have enough data on. It's not clear to me what proportion of publishers are happily using the current approach vs those struggling with complex use cases. I propose we leave the global setConfig approach until we get feedback from pubs. I've set up a meeting with the 3 pubs that are Prebid.org members. |
@bretg I spoke to three people here who work with our config builder and our publishers to manage their configs. The consensus was the more explicit, purpose built, number two option would be preferred. Making it similar to that other ad server is good from the standpoint of applying an understandable config in a complex site. However, there was explicit feedback that:
|
Thanks for the thoughtful response @GLStephen .
Then maybe we could extend AdUnit processing resolve macros. e.g.
And to limit the performance impact, the scanning for macros is only turned on by the existence of any defined macros. Overall macros seem like a pretty high-end feature -- do you have any simpler ideas in mind?
|
@bretg's proposal has my preference overall. A few remarks:
|
Thanks @benjaminclot - I think we got you covered.
The remaining 2 questions:
|
I am still willing to make the changes for sizeMapping. I'm trying to consolidate the feedback provided here (thanks all for the varied perspectives, please keep it coming) to iron out what should be included and key scenarios to verify. My rough plans are to start working on the proposed changes in the next week or so. |
@bretg I may not have been clear with (3): I still want to be able to disable an adunit whatever its mediatype is, not only if it's a banner. Having a sizeconfig per mediatype is nice, but there should also be the possibility to define one for the adunit itself. |
In looking overall at the collected feedback, there are points about the scope of feature in how it interacts with an adUnit and its mediaType(s) that need to be clarified. Imagine if you had the following type of adUnit using the new logic:
How should the different mediaTypes work together (or work separately) when one qualifies and the other doesn't (especially around For instance:
As a potential suggestion to 1, perhaps we could setup a Tangent Thought The main idea here would be to put a focus on the size tiers organizing and driving the logic. If you for example create a tier that had no mediaTypes/sizes defined, that adUnit would not show at all. Alternatively - if you include only the banner/native mediaTypes for a small-sized tier and all 3 mediaTyps for larger tiers, then it know to include/exclude those types as needed. What are your thoughts about this type of structure? It may clash a bit more with the existing |
@jsnellbaker could you provide an example of what you're thinking in the Tangent Thought section? |
Agree with Eric that an example counter-counter proposal would be helpful. Let me see if I understand the scenario:
Honestly, I'm not qualified to talk about Native use cases in particular, but will just generalize and assume that every mediatype and every edge case are precious. Back to basics, there are really two completely separate configurations we're talking about here:
I think #1 is covered by what we've defined already. One way to achieve #2 (and eliminate the confusion over bidder-specific sizes) would be to have an explicit
|
Based on the latest blog post, are we to understand that this feature (sadly) won't make it to 3.0? |
@benjaminclot It's been scoped out of 3.0 as the new design being discussed wouldn't be considered a breaking change. It will instead be part of a regular release as a new feature. It should be coming sooner rather than later, so I hope that helps. @bretg @harpere |
@bretg @harpere I still think we should have some flag for the native field to allow some control how that's handled. If we're continuing with the existing logic - the native part of the request would always be included unless the label check failed. Given the requests made earlier in the thread to be able to shut-off the adUnit under certain screen-sizes, I'm not sure the existing logic would be something to easily work with here. In regards to the other thought I mentioned, I was thinking something along the lines of this:
The main idea was to group the mediaTypes that should be active within a specific tier; their inclusion into the tier would set the logic for when that mediaType should/shouldn't be used. Working with the example:
|
I don't understand why we'd want to make size primary over mediatype. The sense I got from this rather long thread is that we shouldn't be making any assumptions that pubs line up screen size behavior in a clean way. If they did, then the current global approach should work just fine. i.e. it's not necessarily the case that banner/video/native treatments all fall on the same neat screen size boundaries. Instead, maybe they want banner sizes to differ in the ranges 0-621px / 622-864px / 865+px while the video playerSize range is 0-703px / 704+px For that reason, I pretty strongly prefer
I don't see that as a problem in the
Again, I don't understand what controls native needs. If it's just a boolean, then a simple
Else, if different components of native need to vary, then it's a little more complicated, but maybe something like this would work:
(note: the Prebid.org native doc shows sizes as a single array rather than an array of arrays. That sounds like a mistake?) So putting it all together in a single example:
|
I'm good with @bretg's suggestions. |
Thanks @bretg for the feedback and putting together the updated example. I agree with your points and we'll start to work on the changes going with this model. |
Closing -- this was the issue for the 'Advanced Size Mapping' module. |
This I2I documents the (currently) planned updates to the sizeConfig feature for Prebid.js 3.0. It is open to feedback, so please share any thoughts/concerns/ideas/questions in regards to the outlined approach.
Table of Contents
Status of sizeConfig feature
Context for change:
mediaQuery
params. This type of setup caused the wrong positions to be used for the wrong ad-slots due the merged configs.adUnitCode
) so that one would be used in one environment (such as desktop) while another would be used in another (such as mobile)Proposed updates on sizeConfig feature
Primary Goals:
Details on change:
setConfig
and adUnit/bid levels remains mostly unchanged (see last point).sizeConfig
objects, rather than themediaQuery
alone be responsible for activating a (or multiple) config(s).disable
,off
, 'pass', 'VIP'When using a config with a
label
:mediaQuery
s for the current screen-size are not included in the filtering process for that adUnit/bid.mediaQuery
s for those label configs match the current screen-sizedesktop
andxl
, then their two sizesSupported fields would be combined into one list with the following sizes[[728, 90], [728, 450], [1000, 90], [1000, 450]]
.When using a config without a label:
mediaQuery
for those no-label configs match the current screen-sizeStatus of other aspects of the sizeConfig feature
labelAny
andlabelAll
fields in the adUnit/bid objectsand
/or
relationship requirements of the labels for that adUnit/bid. If it fails, then the adUnit/bid is not included in the auction.labels
in thepbjs.requestBids
call and use it to allow/block adUnits/bidssetConfig
configs to filter-out sizes.banner
type ads, since they're the only media type where the sizes play a pivotal role.Outstanding Concerns:
setConfig
since the primary configuration point is an array.setConfig
settings), but this would force a change to publishers to update their setups.Examples of Updated Approach
Example 1: Complex rules working together
sample setup of config object and adUnit(s):
As a result of this config and assuming screen-width is 1500px, the following would happen to each adUnit:
[[300, 250], [300, 600]]
bidderD
's bid for this adUnit is included in the auctionleaderboard-desktop
config activatesbidderA
's',bidderB
's', andbidderC
's' bids for this adUnit are included in the auctionmediaQuery
doesn't match the screen-size, that specific bid is excluded from the auctionbidderC
's bid is included in the auction and the sizes remain untouchedVIP
label, this adUnit is exempt from the sizeConfig featurebidderA
's andbidderB
's bids for this adUnitare always included in the auctionExample 2 - No more duplicating adUnits as a workaround
Example taken from #3226 (comment)
Some context on the setup:
In the example noted below, notice the duplicated the adUnit
banner
. Per the client, they made a copy of thebanner
adUnit in order to prevent the 300x250 size from being included in screens >992px.In the old logic, the sizeConfig would have merged the
w992
with either thew768
or thew1200
config. If there was just onebanner
that listed all 3 sizes, then the300x250
would have been included in the request since the merged configs listed 300x250 as a valid size.By splitting the sizes across duplicate adUnits, it ensures that even if the configs are merged together at the larger screen-sizes - the 300x250 wouldn't be part of the active
banner
adUnit (since that size belongs to the other copy of thebanner
adUnit).With the new approach, the
banner
adUnit can be 1 complete adUnit without the risk of the 300x250 size being included accidentally (see below). The correct size is ensured due to the new logic incorporating the label that's currently used by that adUnit and looking only at that label's config.This setup allows a cleaner set of adUnits; by allevating the complexity that came from managing the sizes and the labels across duplicated adUnits, the previously noted workaround should no longer be necessary.
The text was updated successfully, but these errors were encountered: