Skip to content
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

No floor price set in prebidServerBidAdapter #8307

Closed
rmattis opened this issue Apr 20, 2022 · 8 comments · Fixed by #8309
Closed

No floor price set in prebidServerBidAdapter #8307

rmattis opened this issue Apr 20, 2022 · 8 comments · Fixed by #8309
Assignees

Comments

@rmattis
Copy link

rmattis commented Apr 20, 2022

Type of issue

Bug

Description

We encountered a problem with the prebidServerBidAdapter where imp.bidfloor and imp.bidfloorcur in the request is not set.
We debugged this and found that the bug was added with version 6.17.0.
If the first bidder in a request is not configured for all adUnits, the bidfloor and bidfloorcur is not set for all adUnits where this bidder is not configured.

if (bid.bidder == null) return acc;

This line is probably what causes the bug and was added with #8154

The main problem is that only the first prebid request is used to receive the floor price for all bidders:

const firstBidRequest = bidRequests[0];

const getFloorBid = find(firstBidRequest.bids, bid => bid.adUnitCode === adUnit.code && typeof bid.getFloor === 'function');
if (getFloorBid) {
let floorInfo;
try {
floorInfo = getFloorBid.getFloor({
currency: config.getConfig('currency.adServerCurrency') || DEFAULT_S2S_CURRENCY,
});
} catch (e) {
logError('PBS: getFloor threw an error: ', e);
}
if (floorInfo && floorInfo.currency && !isNaN(parseFloat(floorInfo.floor))) {
imp.bidfloor = parseFloat(floorInfo.floor);
imp.bidfloorcur = floorInfo.currency
}
}

Steps to reproduce

  1. Add 2 adUnits
  2. Configure the first bidder only for the first ad unit
  3. Configure the first bidder as server side
const adUnit1 = {
  mediaType: { banner: { sizes: [[300, 250]] } },
  ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-1' } } } },
  floors: {....},
  bids: [ { bidder: 'Bidder A', params { }, bidder: 'Bidder B', params { } }  ]
}

const adUnit2 = {
  mediaType: { banner: { sizes: [[300, 250]] } },
  ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-2' } } } },
  floors: {....},
  bids: [ { bidder: 'Bidder B', params { } }  ]
}

const s2sConfig = {
  // ...
  bidders: [ 'Bidder A']
}

Test page

None

Expected results

ad-slot-2 prebid server request should contain bidfloor and bidfloorcur

Actual results

ad-slot-2 prebid server request doesn't contain bidfloor and bidfloorcur

Platform details

prebid.js version > 6.17.0

Other information

Probably changes from #8154 cause this bug

@dgirardi
Copy link
Collaborator

This needs fixing regardless, but I'm surprised that bidfloor in the request matters if you're using a stored request; I'm worried that there may be similar issues with other request parameters if the contents of bids affects the behavior of a stored request.

@muuki88
Copy link
Collaborator

muuki88 commented Apr 20, 2022

Thanks @dgirardi for the immediate feedback 😊

The bidfloor is determined in the client and passed to all bidders regardless if server or client side.

@dgirardi
Copy link
Collaborator

I am unable to reproduce - using this setup:

<!--
  This page calls a single bidder for a single ad slot. It can be considered a "hello world" example for using
  Prebid with the Google Publisher Tag.
  It also makes a good test page for new adapter PR submissions. Simply set your server's Bid Params object in the
  bids array inside the adUnits, and it will use your adapter to load an ad.
  NOTE that many ad servers won't send back an ad if the URL is localhost... so you might need to
  set an alias in your /etc/hosts file so that you can load this page from a different domain.
-->

<html>
<head>
    <script async src="../../build/dev/prebid.js"></script>
    <script async src="https://www.googletagservices.com/tag/js/gpt.js"></script>
    <script>
        var FAILSAFE_TIMEOUT = 10000;
        var PREBID_TIMEOUT = 10000;

        const adUnit1 = {
            code: 'test-1',
            mediaTypes: { banner: { sizes: [[300, 250]] } },
            ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-1' } } } },
            floors: {
                currency: 'USD',
                schema: {
                    delimiter: '|',
                    fields: [ 'mediaType', 'size' ]
                },
                values: {
                    'banner|*': 1.35,
                    'video|*': 2.00
                }
            },
            bids: [
                {
                    bidder: 'appnexus',
                    params: {
                        placementId: 13144370
                    }
                },
                {
                    bidder: 'rubicon',
                    params: {
                        accountId: 14062,
                        siteId: 70608,
                        zoneId: 498816
                    }
                }
            ],
        }

        const adUnit2 = {
            code: 'test-2',
            mediaTypes: { banner: { sizes: [[300, 250]] } },
            ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-2' } } } },
            floors: {
                currency: 'USD',
                schema: {
                    delimiter: '|',
                    fields: ['mediaType', 'size']
                },
                values: {
                    'banner|*': 1.35,
                    'video|*': 2.00
                }
            },
            bids: [
                {
                    bidder: 'rubicon',
                    params: {
                        accountId: 14062,
                        siteId: 70608,
                        zoneId: 498816
                    }
                }
            ],

        }

        var adUnits = [
            adUnit1,
            adUnit2
        ];

        var pbjs = pbjs || {};
        pbjs.que = pbjs.que || [];

    </script>

    <script>
        var googletag = googletag || {};
        googletag.cmd = googletag.cmd || [];
        googletag.cmd.push(function() {
            googletag.pubads().disableInitialLoad();
        });

        pbjs.que.push(function() {
            pbjs.setConfig({
                debug: true,
                bidderTimeout: 10000,
                floors: {},
                s2sConfig: {
                    accountId: '1',
                    enabled: true, //default value set to false
                    defaultVendor: 'appnexus',
                    bidders: ['appnexus'],
                    timeout: 10000, //default value is 1000
                    adapter: 'prebidServer', //if we have any other s2s adapter, default value is s2s
                },
            });
            pbjs.addAdUnits(adUnits);

            pbjs.requestBids({
                bidsBackHandler: sendAdserverRequest,
                timeout: PREBID_TIMEOUT
            });
        });

        function sendAdserverRequest() {
            if (pbjs.adserverRequestSent) return;
            pbjs.adserverRequestSent = true;
            googletag.cmd.push(function() {
                pbjs.que.push(function() {
                    pbjs.setTargetingForGPTAsync();
                    googletag.pubads().refresh();
                });
            });
        }

        setTimeout(function() {
            sendAdserverRequest();
        }, FAILSAFE_TIMEOUT);

    </script>

    <script>
        googletag.cmd.push(function () {
            googletag.defineSlot('/19968336/header-bid-tag-0', [[300, 250], [300, 600]], 'div-gpt-ad-1460505748561-0').addService(googletag.pubads());

            googletag.pubads().enableSingleRequest();
            googletag.enableServices();
        });
    </script>
</head>

<body>
<h2>Prebid.js Test</h2>
<h5>Div-1</h5>
<div id='div-gpt-ad-1460505748561-0'>
    <script type='text/javascript'>
        googletag.cmd.push(function() { googletag.display('div-gpt-ad-1460505748561-0'); });
    </script>
</div>
</body>
</html>

bidfloor does get populated:

bidfloor

@dgirardi
Copy link
Collaborator

I did find what I think is an unrelated issue, the way the PBS calls getFloor does not appear to work if you try to specify any filter other than currency (size in my example has to be '*'):

floorInfo = getFloorBid.getFloor({
currency: config.getConfig('currency.adServerCurrency') || DEFAULT_S2S_CURRENCY,
});

@ChrisHuie, is this a known issue?

@rmattis
Copy link
Author

rmattis commented Apr 20, 2022

Thanks for your fast feedback :)
Here is the setup reproducing the bug (second bidder also needs to be server side):

<!--
  This page calls a single bidder for a single ad slot. It can be considered a "hello world" example for using
  Prebid with the Google Publisher Tag.
  It also makes a good test page for new adapter PR submissions. Simply set your server's Bid Params object in the
  bids array inside the adUnits, and it will use your adapter to load an ad.
  NOTE that many ad servers won't send back an ad if the URL is localhost... so you might need to
  set an alias in your /etc/hosts file so that you can load this page from a different domain.
-->

<html>
<head>
    <script async src="../../build/dev/prebid.js"></script>
    <script async src="https://www.googletagservices.com/tag/js/gpt.js"></script>
    <script>
        var FAILSAFE_TIMEOUT = 10000;
        var PREBID_TIMEOUT = 10000;

        const adUnit1 = {
            code: 'test-1',
            mediaTypes: { banner: { sizes: [[300, 250]] } },
            ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-1' } } } },
            floors: {
                currency: 'USD',
                schema: {
                    delimiter: '|',
                    fields: [ 'mediaType', 'size' ]
                },
                values: {
                    'banner|*': 1.35,
                    'video|*': 2.00
                }
            },
            bids: [
                {
                    bidder: 'appnexus',
                    params: {
                        placementId: 13144370
                    }
                },
                {
                    bidder: 'rubicon',
                    params: {}
                }
            ],
        }

        const adUnit2 = {
            code: 'test-2',
            mediaTypes: { banner: { sizes: [[300, 250]] } },
            ortb2Imp: { ext: { prebid: { storedrequest: { id: 'ad-slot-2' } } } },
            floors: {
                currency: 'USD',
                schema: {
                    delimiter: '|',
                    fields: ['mediaType', 'size']
                },
                values: {
                    'banner|*': 1.35,
                    'video|*': 2.00
                }
            },
            bids: [
                {
                    bidder: 'rubicon',
                    params: {}
                }
            ],

        }

        var adUnits = [
            adUnit1,
            adUnit2
        ];

        var pbjs = pbjs || {};
        pbjs.que = pbjs.que || [];

    </script>

    <script>
        var googletag = googletag || {};
        googletag.cmd = googletag.cmd || [];
        googletag.cmd.push(function() {
            googletag.pubads().disableInitialLoad();
        });

        pbjs.que.push(function() {
            pbjs.setConfig({
                debug: true,
                bidderTimeout: 10000,
                floors: {},
                s2sConfig: {
                    accountId: '1',
                    enabled: true, //default value set to false
                    defaultVendor: 'appnexus',
                    bidders: ['appnexus', 'rubicon'],
                    timeout: 10000, //default value is 1000
                    adapter: 'prebidServer', //if we have any other s2s adapter, default value is s2s
                },
            });
            pbjs.addAdUnits(adUnits);

            pbjs.requestBids({
                bidsBackHandler: sendAdserverRequest,
                timeout: PREBID_TIMEOUT
            });
        });

        function sendAdserverRequest() {
            if (pbjs.adserverRequestSent) return;
            pbjs.adserverRequestSent = true;
            googletag.cmd.push(function() {
                pbjs.que.push(function() {
                    pbjs.setTargetingForGPTAsync();
                    googletag.pubads().refresh();
                });
            });
        }

        setTimeout(function() {
            sendAdserverRequest();
        }, FAILSAFE_TIMEOUT);

    </script>

    <script>
        googletag.cmd.push(function () {
            googletag.defineSlot('/19968336/header-bid-tag-0', [[300, 250], [300, 600]], 'div-gpt-ad-1460505748561-0').addService(googletag.pubads());

            googletag.pubads().enableSingleRequest();
            googletag.enableServices();
        });
    </script>
</head>

<body>
<h2>Prebid.js Test</h2>
<h5>Div-1</h5>
<div id='div-gpt-ad-1460505748561-0'>
    <script type='text/javascript'>
        googletag.cmd.push(function() { googletag.display('div-gpt-ad-1460505748561-0'); });
    </script>
</div>
</body>
</html>

The floor price for ad-slot-2 is not set:
Bildschirmfoto 2022-04-20 um 21 35 03

@dgirardi
Copy link
Collaborator

@rmattis, I do see the issue now, thanks! however, I don't think it's related to any recent change - I still get the same behavior in 6.16.0

The problem is that it looks for a bid from the first bidder for the second adUnit - when the first bidder did not bid on it. Are you confident this is what you were seeing, or do you think you might have seen something else related to that if (bid.bidder == null) check?

@rmattis
Copy link
Author

rmattis commented Apr 20, 2022

@dgirardi Yeah you're right. It seems the if (bid.bidder == null) is not related to this issue.
However, I just tested it with our production system and anyhow the order of the bidRequests in the prebidServerBidAdapter changed with prebid version 6.17.0

Version 6.16.0:
Bildschirmfoto 2022-04-20 um 22 10 25

Version 6.17.0:
Bildschirmfoto 2022-04-20 um 22 11 27

In our case smartadserver is the bidder which is not configured for all ad units but is used as firstBidRequest here:
https://github.com/rmattis/Prebid.js/blob/d4c057a914ef79a1508db1daa68d27b1a3002610/modules/prebidServerBidAdapter/index.js#L537

With version 6.16.0, adf was the firstBidRequest and didn't cause any problems cause it's configured for all ad units

dgirardi added a commit to dgirardi/Prebid.js that referenced this issue Apr 20, 2022
This addresses prebid#8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)
@dgirardi
Copy link
Collaborator

#8309 should address the issue in here.

Regarding my previous comment about size floor filters not working, as it turns out, it does not work for any request - it's not related to the PBS adapter. I am not sure about the details, but it seems like floors are evaluated both on requests and reponses; and not all filters will work on both - size only looks at responses.

patmmccann pushed a commit that referenced this issue Apr 22, 2022
…est (#8309)

* PBS adapter: fix bug with priceFloors sometimes not being set in request

This addresses #8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)

* Do not send pricefloor if any bid cannot provide one

* Test errors from currency conversion

* Revert whitespace changes

* fix spepelling
Repository owner moved this from In progress to Done in Prebid.js Tactical Issues table Apr 22, 2022
JoelPM pushed a commit to JoelPM/Prebid.js that referenced this issue Apr 25, 2022
…est (prebid#8309)

* PBS adapter: fix bug with priceFloors sometimes not being set in request

This addresses prebid#8307 by picking what's likely to be the minimum floor for an adUnit (instead of just looking at the floor defined by the first request it finds, which may not be there)

* Do not send pricefloor if any bid cannot provide one

* Test errors from currency conversion

* Revert whitespace changes

* fix spepelling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

3 participants