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

Proposal - Improve Prebid Native when sendAllBids is set to true #5149

Closed
leonardlabat opened this issue Apr 21, 2020 · 18 comments · Fixed by #5411
Closed

Proposal - Improve Prebid Native when sendAllBids is set to true #5149

leonardlabat opened this issue Apr 21, 2020 · 18 comments · Fixed by #5411
Assignees
Labels
intent to implement pinned won't be closed by stalebot

Comments

@leonardlabat
Copy link
Contributor

leonardlabat commented Apr 21, 2020

Context

Currently, publishers that want to use the Prebid Native along with the sendAllBids parameter set to true (meaning that Prebid will send the targeting keys of all bidders to the ad server) are facing an issue due to a limitation on the targeting key length. Indeed, native targeting keys are using sometimes long name like hb_native_title_biddercode which are above the limit and ends truncated.

Phase 1 - PBJS stop sending all the native keys to the adserver if sendAllBids is set to true

Goal : Provide an easy to implement solution that won't require any update in bidder adapters

We think that the following steps should able to mitigate to the current situation. The main idea is that PBJS is no more sending the native keys to the adserver. Instead, we will rely on the existing placeholder mecanism to fetch the assets, except that we won't limit the process to fetch only the assets that have been marked with a placeholder, but we will fetch all of them.

  1. In PBJS, in the getKeyValueTargetingPairs function from auction.js (https://github.com/prebid/Prebid.js/blob/master/src/auction.js#L688), if sendAllBids is set to true, then do not send any targeting keys.

  2. In the Prebid Universal Creative code (https://github.com/prebid/prebid-universal-creative/blob/26e959305c6b4ce0a97b777a90a2bace1f8228f7/src/nativeAssetManager.js#L166), if no native targeting keys have been received, then send the assetRequest command without specifying any keys.

  3. In PBJS, in the getAssetMessage function of native.js (https://github.com/prebid/Prebid.js/blob/master/src/native.js#L183), if an asset list is provided in parameter, then only fill the output with these assets (that's the current behaviour), otherwise fill the output with all the available assets

Phase 2 - Add support for assetUrl

Goal : Provide a long term solution aligned with what's done in OpenRTB

This is based on the asumption that Phase 1 has already been developed, meaning that PBJS already no longer sends all the native keys to the adserver.

  1. In PBJS, in the interpretResponse function, when forging bid objects, the bidder could provide an assetUrl property (if he decides to support the assetUrl mode)

  2. In PBJS, in the getAssetMessage of native.js (https://github.com/prebid/Prebid.js/blob/master/src/native.js#L183), depending if the adObject has an assetUrl property filled

    a. If it does ; then PBJS sends an ajax call to the assetUrl. Once the response is available, it is fowarded to a new method exposed on the bidder interface (something that could be called interpretAssetResponse ?). This method is responsible to parse the response and fill an object with the native assets. Then, PBJS takes the output of the interpretAssetResponse and send it to PUC.

    b. If it doesn't ; then continue to apply the existing logic

Depending on your feedbacks, we could implement the phase 1 on the next few days. Phase 2 is a bit more complex, especially if we intent to change the bidder interface, but perhaps it could be a nice candidate for Prebid 4.

@wqi1972
Copy link
Collaborator

wqi1972 commented Apr 22, 2020

Want to confirm the change won't impact prebid native setup using native template http://prebid.org/adops/setting-up-prebid-native-in-dfp.html
And I am a bit confused, http://prebid.org/overview/prebid-universal-creative.html mentions Prebid Universal Creative is for banner and video only, why native bidding change need change code for Universal Creative?

@leonardlabat
Copy link
Contributor Author

Prebid Universal Creative offers some support for native (that's actually what is used in this sample : http://prebid.org/adops/setting-up-prebid-native-in-dfp.html).

The changes proposed should not have any breaking impact on existing creative (as it only changes a bit the way assets are exchanged between the creative and PBJS).

@wqi1972
Copy link
Collaborator

wqi1972 commented Apr 22, 2020

Got it, really appreciate the explanation. I've never notice that we can add tracker by universal native-trk.js, we use inline js code to fire impression and click tracker inside native template.

@bretg
Copy link
Collaborator

bretg commented Apr 28, 2020

@robertrmartinez is also working on this. Will let you guys compare notes.

@robertrmartinez
Copy link
Collaborator

Hello all.

Here is a link to a google doc we at Rubicon have written up.

Native Proposal - DRAFT

Looking forward to collaborating and coming up with a unified proposal!

@jsnellbaker
Copy link
Collaborator

As a question to the 1st posted proposal (have to read the Rubicon proposal) - if the keys are not sent and the creative template doesn't change, how will PUC be able to do the find/replace for the assets inside the creative code?

I encountered this issue before when trying to work on this type of feature before, but if the existing targeting keys are left in place in the GAM creative - they will be removed entirely by GAM when the code is initially set on the page (as the key did not exist in GAM ad URL). When the native-trk.js script tries to execute to fetch the assets from PBJS, it won't know where to insert them because the 'anchor points' in the creative code are missing.

The current sendId feature works because we inject our own placeholder token into the creative code for the script to latch onto/update with the proper asset value.

If the asset placeholder in the creative doesn't change, I'm not sure this type of setup described in Phase 1 will work - unless I'm missing something.

@wqi1972
Copy link
Collaborator

wqi1972 commented May 1, 2020

As publisher, we appreciate @leonardlabat and @robertrmartinez's effort in provide better and decent native solution.
We currently don't use native-trk.js, but now seriously consider switch to that solution, since it provides a better solution to handle special char and max char limit case than the one we currently use.
Somehow current native-trk.js has some issues for our current native setup, which I post as (prebid/prebid-universal-creative#103 and prebid/prebid-universal-creative#102). Though we can fix both issue with current implementation, I think @robertrmartinez 's Native Proposal - DRAFT provides a better framework to solve both issues fundamentally.
So as a publisher, NewsCorp votes for @robertrmartinez's proposal.

@jaiminpanchal27
Copy link
Collaborator

Hi Guys,

Here is the native proposal we have been working at Xandr. Please have a look at let us know your thoughts
https://docs.google.com/document/d/1MYRPTll6bFXbzxh2fp6gQ0p5_E0jlV-MZnDYi2e6FOw/edit?usp=sharing

Xandr's proposal has some overlapping feature which are already shared above but with some differences in how we render the creative and how native style is defined.

@wqi1972
Copy link
Collaborator

wqi1972 commented May 5, 2020

I checked Xandr's proposal, and this proposal looks also good from a publisher point of view.

Actually Rubicon and Xandr's proposal are pretty similar, 1) using a new field to control if send target for certain asset; 2) use post message to get native assets value from memory; 3) covers tag loading 404 error, though use different implementation; 4) both pass adId as a parameter into native tag data object, so that we can refactor the way how to trigger impression tracking and click tracking in existing code;

@gglas
Copy link

gglas commented May 5, 2020

Hey All - I just pinged the slack channel as we're scheduling a call for this Friday 5/8 at 1130AM EST to discuss the proposals outlined here and decide on a path forward. Please ping me on slack or reach out to me directly if you'd like to be included - thanks!

@leonardlabat
Copy link
Contributor Author

Thanks for replying

We totally agree with your proposal @robertrmartinez. Moreover, this isn't incompatible with the use of an asseturl in the future. Please tell us if we can help implementing it.

@robertrmartinez
Copy link
Collaborator

Thanks everyone for participating!

After the call last Friday I have the task of consolidating the three proposals into one Spec / Intent to Implement.

The basic outline will be supporting three new "modes" for native. All which will be suppressing all the native keys into the adServer (with the ability to toggle individual assets to be sent if wanted)

I will try to get this doc posted here as well as on the Prebid Org Google drive by end of the week.

@robertrmartinez
Copy link
Collaborator

https://docs.google.com/document/d/1IgyZqUVh6ms3Tlf-IrFUAsMi8gV7n9lV9vM1zwp4Pr0/edit?usp=sharing

Here is a consolidated spec with the inputs from all three proposals.

Please leave comments and provide any feedback!

@gglas
Copy link

gglas commented May 26, 2020

Hello Everyone! We're planning on moving forward with the consolidated proposal at the end of this week. If anyone has any questions or comments please try to have them in by Friday May 29th.

@stale
Copy link

stale bot commented Jun 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 23, 2020
@robertrmartinez robertrmartinez linked a pull request Jun 23, 2020 that will close this issue
10 tasks
@stale stale bot closed this as completed Jul 2, 2020
@bretg bretg reopened this Jul 2, 2020
@stale stale bot removed the stale label Jul 2, 2020
@stale
Copy link

stale bot commented Jul 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 18, 2020
@bretg bretg removed the stale label Jul 18, 2020
@stale
Copy link

stale bot commented Aug 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 1, 2020
@jsnellbaker jsnellbaker removed the stale label Aug 3, 2020
@jsnellbaker jsnellbaker added the pinned won't be closed by stalebot label Aug 3, 2020
@robertrmartinez
Copy link
Collaborator

Assigning to @jaiminpanchal27 to close once Prebid.js and PUC changes are merged after his review.

Prebid.js: #5411

PUC: prebid/prebid-universal-creative#106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intent to implement pinned won't be closed by stalebot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants