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

Price Foors: support imp-level floorMin

Open bretg opened this issue 2 years ago • 7 comments

Type of issue

Enhancement

Description

Turns out that the client-side floors module already supports imp-level floorMin, but we found two problems:

  1. The ORTB path used client and server-side is different. It was implemented client-side as ext.prebid.floorMin, while server-side it's ext.prebid.floors.floorMin. Server-side is correct. This has been discussed in committee and we agreed that client-side should be updated to prefer ext.prebid.floors.floorMin while still accepting ext.prebid.floorMin for a transition period.
  2. The client-side module isn't supporting floorMinCur.

Original Description

Prebid Server recently released an imp-level floorMin feature. The client-side floors module would benefit from this as well.

The server-side interface is imp[].ext.prebid.floors.floorMin, so the proposed client-side interface is

pbjs.addAdUnits({
    code: "mycode",
    ...
    ortb2Imp: {
      ext: {
        prebid: {
          floors: {
            floorMin: 1.0,
            floorMinCur: "USD"
          }
        }
      }
    }
});

The behavior would be as defined in the Prebid Server Price Floor PRD section 4.3.1.2.

  1. After a rule (and thus floor value) is selected for each imp, the Floors feature must validate the chosen floorRuleValue against any provided floorMin:
    1. Look for a floorMin. First check for imp[].ext.prebid.floors.floorMin. If that doesn't exist, check ext.prebid.floors.floorMin.
    2. If a floorMin was found, next, find the floorMin currency: First check for imp[].ext.prebid.floors.floorMinCur. If that doesn't exist, check for ext.prebid.floors.floorMinCur. If that doesn't exist, check the floors data for data.currency or data.modelGroups[].currency
      1. If both imp[].ext.prebid.floors.floorMinCur and ext.prebid.floors.floorMinCur exist and they're different, emit a warning in debug mode.
    3. If the floorMinCur is different from the floor currency, convert the floorMin value.
    4. If a currency conversion is required but not available, the system should not override imp.bidfloor. It should log a warning in debug mode and to the error log at N% sampling.
    5. If the floorRuleValue is less than the floorMin, then floorValue is set to the converted floorMin. Else floorValue is set to floorRuleValue.
    6. Floor values should be rounded to 4 digits of precision.

bretg avatar Sep 01 '22 14:09 bretg

Dupe of #7626 ?

patmmccann avatar Sep 01 '22 15:09 patmmccann

If floorMin comes with a floorMinCur, does that mean that https://github.com/prebid/Prebid.js/pull/8865 needs to be revised? It's currently ignoring currency.

dgirardi avatar Sep 01 '22 15:09 dgirardi

https://github.com/prebid/Prebid.js/pull/8396 ; the solution to the suspected dupe, also doesn't consider currency

patmmccann avatar Sep 01 '22 16:09 patmmccann

Also it looks at adunit.ortb2Imp.ext.prebid.floorMin not adunit.ortb2Imp.ext.prebid.floors.floorMin

patmmccann avatar Sep 01 '22 17:09 patmmccann

Summary of offline discussion:

  • https://github.com/prebid/Prebid.js/pull/8396 was a partial implementation of this (ignores currency), but looks for ext.prebid.floorMin instead of ext.prebid.floors.floorMin. It should be modified to pick up currency and look for ext.prebid.floors.floorMin falling back to ext.prebid.floorMin, with the fallback to be deprecated and removed in some future major version.

  • https://github.com/prebid/Prebid.js/pull/8865 was written with the assumption that PBS may not recognize imp-level floorMins, and thus called for logic that aggregated them into a single request-wide floorMin (but also ignoring currency). That logic may need to be updated to aggregate across currencies, or it may not be needed at all, now that server understands imp-level floorMins.

dgirardi avatar Sep 01 '22 18:09 dgirardi

Updated description to reflect the conversations we've had in meetings.

bretg avatar Sep 12 '22 18:09 bretg

@dgirardi regarding currency, are further changes required for https://github.com/prebid/Prebid.js/pull/8865 or are things ok as is?

jlquaccia avatar Sep 15 '22 18:09 jlquaccia

Closing since https://github.com/prebid/Prebid.js/pull/8865 was merged.

dgirardi avatar Oct 20 '22 14:10 dgirardi