Native ortb - Githubissues

FilipStamenkovic commented 2 years ago

This pull request is based on initiative to change prebid native to support openRTB standard.

Changes are backwards compatible, meaning with these changes puc can work with openRTB assets and with prebid 'standard' assets.

Included in this PR:

don't fire impression and click trackers until rendering ad is finished (rendering can fail, for instance loading script hosted under rendererUrl may fail, in that case we don't want to fire events)
in case ortb format is received in assetResponse, handle impression trackers and click trackers from inside the PUC, no need to be sending post messages to the pbjs so that pbjs can trigger pixels
added feature for handling clicktrackers defined on asset level (previously that was not possible with pbjs + puc).
if renderer (loaded from rendererUrl) has defined window.postRenderAd function call it after ad is rendered.

These changes are related to the following pbjs PR: native ortb

Fawke commented 2 years ago

Hi @FilipStamenkovic,

Thank you for putting up this PR. I've gone through most of your change and I had a few questions/comments.

[ ] Do we need to update the macros? Specifically, over here, when we are trying to gather all the placeholder values to request them from Prebid.js, I don't see the new ORTB 2.5 values/placeholders being forwarded.
[ ] Will this work when the publisher defines the native template in the adUnit or in the Ad server?
[ ] It'll really be helpful if you can provide a working test page. I know you had linked one here, but I don't think it'll run locally on my machine, because you're loading up a file used on your local machine, check here. Also, while testing locally, did you try to see if the old way, using Prebid.js Native Asset names, that still works. Again a test page using the old way would be highly appreciated.
[ ] A unit test case for the support for ortb asset in nativeAssetManager.js would be good to have.
[ ] We would need a docs PR.

FilipStamenkovic commented 2 years ago

Hi @Fawke

Thanks for your review.
To answer your questions:

it looks like this is related when there isn't pbNativeTagData.requestAllAssets = true;. I'm looking at the docs how puc should behave in this situation and I couldn't find any docs or examples. In all docs that I looked we always suggest using requestAllAssets = true. So, I'm not sure if this is some legacy stuff, or how is it used.
I don't think it will work. I forgot that use case, I'll update my code to support adTemplate
That link I attached was just to show changes for some random bidder how their bid adapter code would change when we switch to openRTB. I will create some test page to demonstrate openRTB changes.
Sure, I'll add tests for that file
I opened docs pr

FilipStamenkovic commented 2 years ago

Hi @Fawke I added example page how to test native ortb.

You can find page here

On the same branch there is also a renderer, which can render ortb native ads and 'legacy pbjs' native ads. It's located here.

To test PUC with pbjs changes I suggest using requestly to replace native-render.js that comes from GAM with script you build from this PR.

Also, I prepared python2 script that can be used as 'fake prebid server bidder' for native ads. You can find it bellow. fake_pbs_python2.txt

musikele commented 2 years ago

Hi @dgirardi , is there anything else that has to be fixed on this PR ?

dgirardi commented 2 years ago

To test this, I am using the "fake" native response from the test server you gave me @musikele. Here's the test page; it expects PUC to live at localhost:9990.

I am getting Uncaught TypeError: ortb.assets.forEach is not a function, because the fake response packages assets in an object and not an array. I don't know if it's a problem with the response or if that's legitimate.


<html>

<head>
    <link rel="icon" type="image/png" href="/favicon.png">
    <script async src="//www.googletagservices.com/tag/js/gpt.js"></script>
    <script async src="../../build/dev/prebid.js"></script>

    <script>
        var googletag = googletag || {};
        googletag.cmd = googletag.cmd || [];

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

        var date = new Date().getTime();

        function initAdserver() {
            if (pbjs.initAdserverSet) return;

            googletag.cmd.push(function () {
                pbjs.que.push(function () {
                    pbjs.setTargetingForGPTAsync();
                    googletag.pubads().refresh();
                });
            });

            pbjs.initAdserverSet = true;
        }

        // Load GPT when timeout is reached.
        // setTimeout(initAdserver, PREBID_TIMEOUT);

        pbjs.que.push(function () {
            pbjs.setConfig({
                debug: true,
                bidderTimeout: 10000,
                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
                },
            });

            var adUnits = [{
                code: 'div-3',
                // sizes: [[1, 1]],
                mediaTypes: {
                    native: {
                        sendTargetingKeys: false,
                        //adTemplate: "<div class=\"sponsored-post\">\r\n  <div class=\"thumbnail\" style=\"background-image: url(##hb_native_image##);\"><\/div>\r\n  <div class=\"content\">\r\n  <h1>\r\n    <a href=\"%%CLICK_URL_UNESC%%##hb_native_linkurl##\" target=\"_blank\" class=\"pb-click\">\r\n  ##hb_native_title##\r\n    <\/a>\r\n   <\/h1>\r\n    <p>##hb_native_body##<\/p>\r\n    \t<div class=\"attribution\">\r\n \t##hb_native_brand##\r\n           \t<\/div>\r\n\t<\/div>\r\n<\/div>",
                        //rendererUrl: window.rendererUrl,
                        title: {
                            required: true,
                            len: 800
                        },
                        image: {
                            required: true,
                            sizes: [989, 742],
                        },
                        sponsoredBy: {
                            required: true
                        }
                    }
                },
                bids: [{
                    bidder: 'appnexus',
                    params: {
                        placementId: '13232354'
                    }
                }]
            }];

            pbjs.setConfig({
                debugging: {
                    enabled: true,
                    intercept: [{
                        when: {
                            bidder: 'appnexus'
                        },
                        then: function() { return {
                            mediaType: 'NATIVE',
                            native: {
                                ortb: JSON.parse(`{"ver": "1.2", "eventtrackers": [{"url": "http://localhost:5500/event?type=impression&component=card", "event": 1, "method": 1}, {"url": "http://localhost:5500/event?type=v50&component=card", "event": 2, "method": 1}], "link": {"url": "https://rtk.io", "clicktrackers": ["http://localhost:5500/event?type=click1&component=card", "http://localhost:5500/event?type=click2&component=card"]}, "assets": [{"data": {"value": "Read More"}, "id": 6}, {"data": {"value": "This is a Prebid Native Creative2."}, "id": 5}, {"id": 1, "img": {"url": "https:\\/\\/vcdn.adnxs.com\\/p\\/creative-image\\/94\\/22\\/cd\\/0f\\/9422cd0f-f400-45d3-80f5-2b92629d9257.jpg", "h": 2250, "w": 3000}}, {"id": 2, "img": {"url": "https:\\/\\/vcdn.adnxs.com\\/p\\/creative-image\\/bd\\/59\\/a6\\/c6\\/bd59a6c6-0851-411d-a16d-031475a51312.png", "h": 83, "w": 127}}, {"id": 4, "title": {"text": "This is a Prebid Native Creative"}}, {"data": {"value": "Prebid.org"}, "id": 3}]}`)
                            }
                        }}
                    }]
                }
            })

            pbjs.addAdUnits(adUnits);

            pbjs.requestBids({
                bidsBackHandler: function (bidResponses) {
                    initAdserver();
                }
            });
        });

    </script>

    <script>
        // GPT setup
        googletag.cmd.push(function () {
            googletag.defineSlot('/41758329/dgirardi-test', ['fluid'], 'div-3').setTargeting('liselect', '4').addService(googletag.pubads());

            googletag.pubads().disableInitialLoad();
            googletag.pubads().enableSingleRequest();
            googletag.enableServices();
        });

    </script>
</head>

<body>

<h2>Native Normal</h2>
<div id='div-3'>
    <script>
        googletag.cmd.push(function() { googletag.display('div-3'); });
    </script>
</div>

</body>

</html>

musikele commented 2 years ago

@dgirardi
I can't observe your same problem. Here's some things I noted about your example:

I don't see any error in puc being logged in console.
Is debugging module really working? Because I see the call being made to appnexus prebid server; shouldn't it be mocked? Also, the response seens to be sent to the ad unit.
appnexus server call is failing because publisherId is a string instead of a number... And even if you change the placementId to be a number, you won't get any response
Were you testing the template-inside-adunit scenario ? Because I see rendererUrl and adTemplate keys being commented out. Also, I guess you were testing this scenario to check if it's retro-compatible?
You will not test prebid-universal-creative if you don't load an ad unit with the puc in it. If you want, I can provide you with one.

So, it would be very helpful to know what you wanted to exactly achieve and test.

musikele commented 2 years ago

Hi @dgirardi In the end I found out why I was not seeing debugging working - I was building my pbjs without debugging module in it. Also, there is another debugging.js file that seems to be unrelated, but I was caught off guard.

Do you need anything else to move forward with this?

dgirardi commented 2 years ago

@musikele I'd like a working example, and for release probably also documentation about the new template format. (Or just the docs and I can try building an example myself).

musikele commented 2 years ago

Hi @dgirardi . I changed this PR to send messages for tracking to PBJS. Instead of firing trackers, we're now sending a message to PBJS. In PBJS there's this companion PR that will help asset clicktrackers to be fired: https://github.com/prebid/Prebid.js/pull/8905

patmmccann commented 2 years ago

@musikele merging 171 created a conflict here

musikele commented 2 years ago

@dgirardi I added some changes to native-trk.js . There's no documentation on how it should work, so my changes are just an educated guess. I think this file was reccomended with the very first version of native, the one that had the template in the ad server creative.

musikele commented 2 years ago

@jsnellbaker thanks for catching this ! I'm going to remove the editor files from the PR

jsnellbaker commented 2 years ago

@musikele Can you take a look at the conflicts here, additionally can you double-check the errors in the circleci test? There are some references to some unsupported functions that may be an issue?

Update This branch may just need to be rebased/synced with master. I saw there was another PR recently merged (#178) that updated the browsers.json file to not include IE11 as part of the test suite; the failures were largely in IE11, though there was a test failure that occurred in the other browsers related to:

no placeholders found but requests all assets flag set - adTemplate - openRTB
    TypeError: Object doesn't support property or method 'replaceAll'
       at replaceORTBAssetsAndLinks (webpack:///src/nativeAssetManager.js:414:5 <- creative.js:18569:5)
       at replace (webpack:///src/nativeAssetManager.js:435:7 <- creative.js:18592:7)
       at replaceAssets (webpack:///src/nativeAssetManager.js:327:9 <- creative.js:18452:9)
       at windowListener (webpack:///src/messaging.js:38:17 <- creative.js:17850:9)
       at Anonymous function (webpack:///test/spec/nativeAssetManager_spec.js:58:5 <- creative.js:31860:5)
       at sendMessage (webpack:///src/messaging.js:24:13 <- creative.js:17836:7)
       at requestAllAssets (webpack:///src/nativeAssetManager.js:252:5 <- creative.js:18377:5)
       at loadAssets (webpack:///src/nativeAssetManager.js:199:7 <- creative.js:18324:7)
       at Anonymous function (webpack:///test/spec/nativeAssetManager_spec.js:396:7 <- creative.js:32146:7)

musikele commented 2 years ago

Hi @jsnellbaker, just completed the rebase and yes, the configuration for CircleCI was updated.

prebid / prebid-universal-creative

Native ortb #150