Prebid.js icon indicating copy to clipboard operation
Prebid.js copied to clipboard

Refine coordination of PBS+PBJS Floors

Open bretg opened this issue 2 years ago • 3 comments

Type of issue

enhancement

Description

The original integration between PBJS floors and PBS floors was implemented in the pbsBidAdapter to turn off server-side floors if client-side floors was active. This approach was taken to preserve the view of client-side analytics which wouldn't have access to decisions made server-side.

However, @patmmccann brought up a powerful use case: server-side floors may be better than client-side floors because they can incorporate country and possibly other aspects. So we came up with this alternate behavior:

If a publisher wants PBS floor logic to have a chance to generate a higher floor, they would explicitly enable server-side floors with either of these two approaches:

pbjs.setConfig({
    s2sConfig: {
       ...
       extPrebid: {
           floors: {
               enabled: true
           }
       }
    }
});

OR

pbjs.setConfig({
    ortb2: {
       ext: {
           prebid: {
               floors: {
                   enabled: true
               }
           }
       }
    }
});

After merging s2sConfig.extPrebid and ortb2, pbsBidAdapter should do something like this:

   if ext.prebid.floors.enabled exists and is true
       loop through imps
           if imp[n].bidfloor exists, copy it to imp[n].ext.prebid.floors.floorMin
           keep track of the lowest floor value
       request.ext.prebid.floors.floorMin = lowestFloorValue

    // else perform the existing logic
    **else** if (typeof deepAccess(firstBidRequest, 'bids.0.floorData') === 'object') {
      request.ext.prebid.floors = { enabled: false };
    }

This will set the floorMins and enable server-side floors-processing.

bretg avatar Jul 27 '22 18:07 bretg

I am perplexed by the last part, about setting imp[].ext.prebid.floors.floorMin. Why isn't server picking it up from imp[].bidfloor ? isn't that the purpose of that field? I also don't see it documented in the server docs - it mentions it only at the request level.

dgirardi avatar Jul 27 '22 22:07 dgirardi

The design on the server side is that imp[].bidfloor is the default floor, not the minFloor.

imp-level minFloor is not yet support server-side, but is in progress.

bretg avatar Jul 28 '22 13:07 bretg

isn't a floor the minimum by definition? the logic would be, as you said, to copy the value from bidfloor. I don't see why they would ever be different. But it's not that important, copying is simple enough.

dgirardi avatar Jul 28 '22 23:07 dgirardi

hey @patmmccann, heads up the first pr i created turned out to be out of sync with the remote pbjs repo, so i closed it and created a new second pr just now (pr #8865 is the one to go with)

jlquaccia avatar Aug 18 '22 19:08 jlquaccia

isn't a floor the minimum by definition?

PBS ignores imp.bidfloor if server-side floor data is found. floorMin is a failsafe the publisher can use to put a fence around the server-side floor data.

@jlquaccia wrote in slack:

if a pub has already set ortb2Imp.ext.prebid.floors.floorMin should imp[n].ext.prebid.floors.floorMin be set imp[n].bidfloor or ortb2Imp.ext.prebid.floors.floorMin ?

Good Q - generally what's already there takes precedence.

And if set to ortb2Imp.ext.prebid.floors.floorMin , should request.ext.prebid.floors.floorMin also be set to the value of ortb2Imp.ext.prebid.floors.floorMin ?

And this is a reasonable followup. I've updated the description with two clauses:

   if ext.prebid.floors.enabled exists and is true
       loop through imps
           if imp[n].bidfloor exists, **and imp[n].ext.prebid.floors.floorMin doesn't exist,** copy it to imp[n].ext.prebid.floors.floorMin
           keep track of the lowest floor value **in imp[n].ext.prebid.floors.floorMin (whether from imp[n].bidfloor or ortb2Imp)**
       request.ext.prebid.floors.floorMin = lowestFloorValue

bretg avatar Aug 29 '22 14:08 bretg

Thanks @bretg sounds good, just refactored my PR based on your answer above.

jlquaccia avatar Aug 30 '22 00:08 jlquaccia

I was asked to re-explain/re-justify this.

Here are the assumptions:

  1. imp.bidfloor is a default floor. Not a minFloor. Not the actual floor. It's only used by PBS if no better value can be found.
  2. If some bidders are client-side, and some server-side, floors are best handled client-side because every bidder has client-side presence. The floor logic will be more consistent if the same dynamic data is applied to all bidders. So the first-cut floor is always decided client-side when both options are present.
  3. However, additional data available on the server side should be allowed to raise the floor.

Rather than building out some new server-side feature to be juggle #2, this proposal just takes a short cut and utilizes existing powerful config to achieve the same results. I've updated the pbsBidAdapter algorithm with some more comments that hopefully help.

bretg avatar Sep 08 '22 20:09 bretg

@bretg, should we worry about floor currencies here? specifically in light of the proposal in https://github.com/prebid/Prebid.js/issues/8938 . Does it make sense to ignore it when calculating the request-wide ext.prebid.floors.floorMin ?

If it was up to me I'd say we should convert all of the imp-level floorMin into the same currency. We already have some similar logic for aggregating multiple bid requests (that can theoretically have floors in different currencies) into a single imp.bidfloor.

dgirardi avatar Sep 15 '22 19:09 dgirardi

Sorry, I don't understand the question @dgirardi . Are you suggesting that we remove the ability for pubs to define the currency on imp-level floors? Would we assume that currency was defined somewhere else?

My preference is that we get the Prebid world used to always specifying currency when they mention a floor.

we should convert all of the imp-level floorMin into the same currency

I'm not interested in defining the implementation, just the requirements. It would be an abuse for a pub to define one imp's floor in USD and the next imp floor in EUR, so I am open to having PBJS reject that kind of scenario.

bretg avatar Sep 27 '22 14:09 bretg

@bretg, you did define an implementation - that is specific about how to calculate the request-wide floorMin, with an algorithm that ignores currency. If that was unintentional I think it's correct to convert everything down to one currency.

dgirardi avatar Sep 27 '22 14:09 dgirardi

it's correct to convert everything down to one currency

agreed

bretg avatar Sep 27 '22 14:09 bretg