Prebid Core: switch native assets to ortb2 format #7847

FilipStamenkovic · 2021-12-10T13:38:00Z

Type of change

Feature

Description of change

Change from prebid 'standard' of defining native assets to openRTB 1.2 standard.
Instead of sponsoredBy, image, icon, we would have openRTB equivalents:

{ data: { type: 1 }}
{ img: { type: 3 }}
{ img: { type: 1 }}

This will help standardize prebid native. Also, a lot of bidders are sending ortb2 requests on their back end, so at the moment, they are converting bid request from prebid to openRTB standard and later they are converting bid responses from openRTB to prebid standard. With this approach a lot of duplicated code between the bidders can be removed.

Backwards compatibility

When this PR is merged, we will still have backwards compatibility. Which mean, nothing will break for the existing native bidders. That will give us some time until all bidders adjust their code

Other information

This PR resolves #7830 issue.

FilipStamenkovic · 2021-12-10T13:41:10Z

I also created an example how this will look like on the test page. I used pubwise bidder, because that's the only bidder I found that is always returning to me test native bid.

Code how that would look like can be found on my local repo

Fawke · 2022-01-03T11:45:53Z

src/native.js

+
+  const match = requiredAssetIds.every(assetId => includes(returnedAssetIds, assetId));
+  if (!match) {
+    logError(`didn't receive a bit with all required assets. Required ids: ${requiredAssetIds}, but received ids in response: ${returnedAssetIds}`);


Typo: bit should be bid.

I fixed this

Fawke

Hi,

The changes look fine at a first glance, but I do not see any docs PR associated with it. If Native now supports OpenRTB params, then I think we should document it with proper examples.

Also, I do see an associated PR for Prebid Universal Creative. Does that take of the rendering if new ortb fields are used?

FilipStamenkovic · 2022-01-04T08:36:04Z

@Fawke

Thanks for the review.
I didn't open any docs PR, my plan was to open first changes to the code, so that reviewers can take a look at proposed approach how to switch to openRTB for native ads. If everything is ok I will also open docs PR.

bretg · 2022-01-05T17:25:39Z

@FilipStamenkovic - we're going to need an update to the prebidServerBidAdapter. Currently it's mapping the prebid-specific attributes to ortb2. It should check for the existence of this new format first and merge them in. Thanks.

FilipStamenkovic · 2022-01-06T08:41:13Z

@bretg I was thinking to do pbs adapter changes in separate PR not in this one. But, if you think it's ok to add pbs changes in this PR I'll do it then?

bretg · 2022-01-06T13:17:29Z

I'd say go for it @FilipStamenkovic

FilipStamenkovic · 2022-01-10T11:56:14Z

@bretg I included pbs changes in the PR. I didn't implement 'merge strategy' between old way and ortb format.
I decided to go with approach: if mediaTypes.native.ortb is present use it, if not fallback to the current approach.

Rothalack · 2022-01-18T18:42:07Z

test/spec/unit/pbjs_api_spec.js

-          body: {required: false},
-          icon: {required: false},
-        });
+        expect(nativeRequest.ortb.assets).to.deep.equal([


Wouldn't we want to have a test case of old native request as well? It should be able to accept either, if ortb is set go with that if not use old, right? Maybe it's too much to ask or it is already covered in another test.

It's already covered in another test.
Basically all existing native tests rely on 'old native way'.
But, if you feel like this PR needs more tests, I'll add them?

If it's covered in another test, then that works!

Rothalack · 2022-01-19T14:29:17Z

I think this just needs your approval for the Docs PR change request @Fawke

dgirardi · 2022-01-25T12:52:28Z

src/secureCreatives.js

@@ -63,6 +63,11 @@ export function receiveMessage(ev) {
      } else if (data.action === 'allAssetRequest') {
        const message = getAllAssetsMessage(data, adObject);
        ev.source.postMessage(JSON.stringify(message), ev.origin);
+        // if there is an ortb object inside native, puc won't send postMessage to trigger impression tracker, in that case marking bid as winning


I think it'd be better to move this to the top, if any message with an adId comes in, it means that adId has won the auction, no? you'll need some logic to avoid duplicating the event but IMO that's better than two different ways to mark the bid won (or three if you count the non-native case).

I can't move it to the top because of backwards compatibility.
This code needs to work with 'old native' and with ORTB native.
If I move it to the top, in case of 'old native' we would have duplicated events, because in 'old native' handleNativeRequest() function is executed on many different occasions:

native ad was rendered

native ad needs to be resized

native ad was clicked on

etc.

Once native ORTB is fully adopted in the prebid we can then remove a lot of duplicated code that is simply there because of backwards compatibility.

P.S.
When I say 'old native' I mean what is currently there on master branch for native ads.

I mean something like -

const BID_WONS = new WeakMap(); function receiveMessage(ev) { // ... if !BID_WONS.has(adObject) { BID_WONS.set(adObject, true); // ... trigger event } // message handling }

the issue I have with a "temporary" solution like the one here is that 1) it will be a long long time before we can remove the "legacy" protocol and 2) it's easy to break by accident during that time... if for example the safeframe decides to request single assets instead of all, or some new tracker message is added for the native ortb2 case, the committer is probably not thinking about the bid won event. And to me it seems easy to decide "if a frame has the adId, that always means it has won the auction, we can just mark the bid won right at the beginning".

code updated

ChrisHuie · 2022-01-25T15:28:08Z

Docs PR -> prebid/prebid.github.io#3497

osazos

It is ok for me for the code review.
I added a comment in the doc PR.

bretg · 2022-02-01T19:55:46Z

I hope I'm wrong here, but I think this PR is just the first step in a long journey. I believe that all 74 bid adapters that support native need to be updated to support this new format, right? i.e. in addition to looking in mediatypes.native.title, they need to look in mediatypes.native.ortb2.assets.title, right?

If that's the case, publishers can't change their pages until all the adapters they use support the new syntax.

FilipStamenkovic · 2022-02-01T21:08:03Z

Yeah, this is just a first step, adapters would need to update their code.

It would not be mediaTypes.native.ortb2.assets.title.
It's ortb not ortb2 (because this PR is based on openRTB 1.2 Native specs, there is a link in the description). Also, assets contains array of objects, so to get titles (in theory one native ad can have multiple titles) you would need to do something like:

let titles = mediaTypes.native.ortb.assets.filter(asset => asset.hasOwnProperty('title'));

bretg · 2022-02-02T20:27:47Z

Ok, discussed in today's PBJS meeting. We propose handling this native change in a way similar to how we handled the first party data change last year:

Add some transition code (e.g. toOrtbNative) that maps old-style AdUnit native definitions to new-style.
Create a transition function (e.g. fromOrtbNative) that bid adapters can use to translate the new-style native definition back to the old-style. This is a lossy transition and would only support whatever fields are currently covered by the proprietary format.
Update all 74 native-capable bid adapters to call fromOrtbNative() at the top of buildRequests. Then we communicate with them to start the process of converting their adapters to not need fromOrtbNative().

FWIW, I poked around a couple of adapters: one had a complicated bunch of code for mapping native params and the other seemed to wrap it all up and parse server-side. So this transition may take a while.

Documentation-wise, the proposal is:

produce a new document that is the recommended way that new native implementations should be done.
leave the current doc in place but warn that new implementations should be done in the ortb-way
hide the old-old doc away, making it more clear that no one should be using that (sendId) approach anymore.

musikele · 2022-02-18T11:41:34Z

Closing this one and continuing the work in #8086 .

FilipStamenkovic added feature needs review native needs 2nd review Core module updates require two approvals from the core team labels Dec 10, 2021

ChrisHuie requested a review from Fawke December 14, 2021 11:00

ChrisHuie assigned Fawke Dec 14, 2021

Fawke reviewed Jan 3, 2022

View reviewed changes

Fawke requested changes Jan 3, 2022

View reviewed changes

Fawke added the needs docs label Jan 3, 2022

FilipStamenkovic mentioned this pull request Jan 11, 2022

Docs for native openRTB prebid/prebid.github.io#3497

Merged

FilipStamenkovic force-pushed the native_assets_ortb2 branch from 186ba06 to f4ed600 Compare January 12, 2022 09:30

ChrisHuie requested review from dgirardi and Rothalack and removed request for dgirardi January 14, 2022 22:38

ChrisHuie assigned Rothalack Jan 14, 2022

Rothalack removed the needs docs label Jan 18, 2022

Rothalack reviewed Jan 18, 2022

View reviewed changes

Rothalack approved these changes Jan 19, 2022

View reviewed changes

FilipStamenkovic requested a review from Fawke January 24, 2022 15:21

FilipStamenkovic mentioned this pull request Jan 25, 2022

Native ortb prebid/prebid-universal-creative#150

Merged

dgirardi reviewed Jan 25, 2022

View reviewed changes

FilipStamenkovic force-pushed the native_assets_ortb2 branch from d9ebecc to 4ceed0f Compare January 25, 2022 13:13

ChrisHuie requested a review from osazos January 26, 2022 16:29

ChrisHuie assigned osazos Jan 26, 2022

osazos approved these changes Feb 1, 2022

View reviewed changes

ChrisHuie removed the request for review from Fawke February 3, 2022 14:03

ChrisHuie unassigned Fawke Feb 3, 2022

FilipStamenkovic added 9 commits February 7, 2022 10:50

switch native assets to ortb2 format

875cd1d

put rendererUrl, adTemplate back in message

ef4e43b

rename ortb2 to ortb

9cadec5

typo fix

5ca528a

changes to support ortb for native media type

6e118df

handle bid won case

ace6902

mark bid as won on any post message for native

10a736c

fix tests for bid_won event for native

9756a47

add missing imports

a649f89

FilipStamenkovic force-pushed the native_assets_ortb2 branch from e9e7049 to a649f89 Compare February 7, 2022 10:04

musikele mentioned this pull request Feb 18, 2022

Prebid Core: switch native assets to ortb2 format #8086

Merged

1 task

musikele closed this Feb 18, 2022

FilipStamenkovic deleted the native_assets_ortb2 branch August 4, 2023 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prebid Core: switch native assets to ortb2 format #7847

Prebid Core: switch native assets to ortb2 format #7847

FilipStamenkovic commented Dec 10, 2021

FilipStamenkovic commented Dec 10, 2021

Fawke Jan 3, 2022

FilipStamenkovic Jan 4, 2022

Fawke left a comment •

edited

Loading

FilipStamenkovic commented Jan 4, 2022

bretg commented Jan 5, 2022

FilipStamenkovic commented Jan 6, 2022

bretg commented Jan 6, 2022

FilipStamenkovic commented Jan 10, 2022

Rothalack Jan 18, 2022

FilipStamenkovic Jan 19, 2022

Rothalack Jan 19, 2022

Rothalack commented Jan 19, 2022

dgirardi Jan 25, 2022

FilipStamenkovic Jan 25, 2022

dgirardi Jan 25, 2022 •

edited

Loading

FilipStamenkovic Jan 25, 2022 •

edited

Loading

ChrisHuie commented Jan 25, 2022

osazos left a comment

bretg commented Feb 1, 2022 •

edited

Loading

FilipStamenkovic commented Feb 1, 2022

bretg commented Feb 2, 2022

musikele commented Feb 18, 2022

Prebid Core: switch native assets to ortb2 format #7847

Prebid Core: switch native assets to ortb2 format #7847

Conversation

FilipStamenkovic commented Dec 10, 2021

Type of change

Description of change

Backwards compatibility

Other information

FilipStamenkovic commented Dec 10, 2021

Fawke Jan 3, 2022

Choose a reason for hiding this comment

FilipStamenkovic Jan 4, 2022

Choose a reason for hiding this comment

Fawke left a comment • edited Loading

Choose a reason for hiding this comment

FilipStamenkovic commented Jan 4, 2022

bretg commented Jan 5, 2022

FilipStamenkovic commented Jan 6, 2022

bretg commented Jan 6, 2022

FilipStamenkovic commented Jan 10, 2022

Rothalack Jan 18, 2022

Choose a reason for hiding this comment

FilipStamenkovic Jan 19, 2022

Choose a reason for hiding this comment

Rothalack Jan 19, 2022

Choose a reason for hiding this comment

Rothalack commented Jan 19, 2022

dgirardi Jan 25, 2022

Choose a reason for hiding this comment

FilipStamenkovic Jan 25, 2022

Choose a reason for hiding this comment

dgirardi Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

FilipStamenkovic Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

ChrisHuie commented Jan 25, 2022

osazos left a comment

Choose a reason for hiding this comment

bretg commented Feb 1, 2022 • edited Loading

FilipStamenkovic commented Feb 1, 2022

bretg commented Feb 2, 2022

musikele commented Feb 18, 2022

Fawke left a comment •

edited

Loading

dgirardi Jan 25, 2022 •

edited

Loading

FilipStamenkovic Jan 25, 2022 •

edited

Loading

bretg commented Feb 1, 2022 •

edited

Loading