Update Criteo bid adapter to Prebid 1.x #2370

Spark-NF · 2018-04-09T13:54:43Z

Type of change

New bidder adapter
Official adapter submission

Description of change

Test parameters for validating bids

{
    code: '...',
    sizes: [[300, 250], [728, 90]],
    bids: [{
        bidder: 'criteo',
        params: {
            zoneId: 497747
        }
    }]
}

Adapter maintainers: pi-direct@criteo.com, n.faure@criteo.com, a.hubert@criteo.com
The bidders params documentation already exists and does not require an update as the params didn't change

Other information

The old Prebid 0.x adapter can be found here on the legacy branch

mkendall07 · 2018-04-09T21:54:11Z

modules/criteoBidAdapter.js

+  'amp': 1,
+};
+const PROFILE_ID = 207;
+const PUBLISHER_TAG_URL = '//static.criteo.net/js/ld/publishertag.prebid.js';


Can you put a comment here pointing to https://github.com/Prebid-org/prebid-js-external-js-criteo/blob/master/dist/prod.js as source?

mkendall07 · 2018-04-12T19:21:17Z

@Spark-NF
Not seeing any test bid responses. Anything I need to do to get that to work?

mkendall07 · 2018-04-12T19:21:54Z

request:

http://bidder.criteo.com/cdb?ptv=48&profileId=207&av=4&cb=59440494674

Response

{"exd":{"genimpids":["5acfb1914eb96c31e554aa689b681713"],"slots":[{"imp_id":"5acfb1914eb96c31e554aa689b681713","ad_unit_id":"div-gpt-ad-1460505748561-0","zone_id":497747}]}}

mkendall07 · 2018-04-12T19:34:14Z

modules/criteoBidAdapter.js

+  try {
+    const fastBid = localStorage.getItem('criteo_fast_bid');
+    if (fastBid !== null) {
+      eval(fastBid); // eslint-disable-line no-eval


is there an alternative to using eval here? What's being done?

Our adapter relies on an external script (publishertag.js). Once loaded for the first time we put it into the local storage and eval it during the next calls to not reload it from the network and have a better performances.

very interesting. Going to discuss with the prebid team

Thinking about this some more, I don't think this is safe since you are blindly reading in content from localStorage, which can be overwritten by any script on page. Please remove.

As discussed by mail the is the only way we can store our library in the local storage and then reuse it.

@ahubertcriteo
Do you have any stats that show this is faster than a modern browser caching of the parsed code? It doesn't really make sense to eval (parse) it twice.

Yes, we made a lot of testing with our publishers, and results are very positive.

The time between the call to buildRequests/callBids and the call to the PublisherTag entry point (in this adapter new Criteo.PubTag.Adapters.Prebid) was reduced by 80 to 95%, going from a few hundreds milliseconds (sometimes more than 1s depending on the publisher) to a few milliseconds (usually around 10ms).

The number of timeouts as detected by Prebid was also halved.

Even if the file was cached by the browser, the fact that we're simply adding a <script> causes it to have a pretty low priority in the loading chain. Plus, it's asynchronous as opposed to the localStorage that is synchronous (and fast).

If you'd rather have the localStorage.setItem more explicit, we can update the adapter to do so. However, we'd need an API to do a GET and return the file's contents (the ajax one seems like it could do the job).

mkendall07 · 2018-04-12T19:35:09Z

modules/criteoBidAdapter.js

+  window.criteo_prebid_native_slots[id] = { callback, payload };
+
+  // The creative is in an iframe so we have to get the callback and payload
+  // from the parent window (doesn't work with safeframes)


do you have a postMessage solution that would work in safeFrames?

Maybe, we will study this possibility and push it in a second iteration if feasable.

that's fine

mkendall07 · 2018-04-12T19:37:15Z

modules/criteoBidAdapter.js

+
+      // Reload the PublisherTag after the timeout to ensure FastBid is up-to-date and tracking done properly
+      setTimeout(() => {
+        loadScript(PUBLISHER_TAG_URL);


We are going to update this function signature to include module/bidder code so we can track who is loading external JS. Will reply shortly with the changes.

@ahubertcriteo @Spark-NF
Please use loadExternalScript method instead.
This will enforce a whitelist of which adapters are allowed to load external JS:
https://github.com/prebid/Prebid.js/blob/master/src/adloader.js#L9

mkendall07 · 2018-04-12T19:38:22Z

modules/criteoBidAdapter.js

+import { parse } from 'src/url';
+import * as utils from 'src/utils';
+
+var events = require('src/events');


prefer const.

Will be remove if we remove the event listenening below.

mkendall07 · 2018-04-12T19:47:43Z

modules/criteoBidAdapter.js

+
+  registeredEvents = true;
+
+  events.on(EVENTS.BID_WON, (bid) => {


we currently have a policy that bidder adapters cannot consume internal prebid.js events. You'll need to remove these event listeners.

Can I ask what is the rational of this policy?

On our side we wanted to track those events to help publishers with their integrations when they encounter a lot of timeouts
(some of them have inconsistencies in their timeouts configuration between the one passed in requestBids call and the one in the global setTimeout)

As documented here (http://prebid.org/dev-docs/publisher-api-reference.html#module_pbjs.onEvent), can we replace the call to "events.on" by onEvent ?

If not what would be the right alternative way to collect those informations ?

Reason is that we don't want vendors inspecting other vendor bids. I realize that you are not doing that but we want to make this preventable in the code.

The correct way is not fully implemented yet, but you could start with spec.onTimeout function.
http://prebid.org/dev-docs/bidder-adaptor.html#registering-on-timeout

Other events can be added to the spec in a separate PR if desired. Thanks

We will remove this and use the onTimeout instead for the timeout event.
However we wanted to catch those events to spot publishers with timeout issues (inconsistencies between the timeout given in requestBids and the global one triggering dfp). Have you solutions on your side which will allow the bidders to known which timeout will be triggered first ?

mkendall07 · 2018-04-16T21:31:08Z

@Spark-NF @ahubertcriteo
If you can make the requested changes (drop eval and event) we can merge this for now and work on further enhancements in new PRs.

finished

…such

… conditions

In some cases, the buildCdbRequest function might return a falsy value, in case of error in creating the request or if we know in advance that this request will return a no-bid. In this case, the buildRequests() method should not return a request, causing a no-bid.

GDPR support in Criteo adapter

bretg · 2018-05-04T20:50:22Z

@Spark-NF and @ahubertcriteo - any further progress on this PR?

cwbeck · 2018-05-22T13:42:36Z

Would be nice to get this merged in...

benjaminclot · 2018-05-25T08:33:15Z

How did this get committed with eval still there? 🙁

piwanczak · 2018-06-05T14:52:21Z

How about the convention "No loading of external libraries: All code must be present in the adapter, not loaded at runtime."?
Should it be signed as redundant?

benjaminclot · 2018-06-05T15:15:20Z

@mkendall07 Are you going to enforce rules on this adapter or is Criteo going to get a free pass?

mafaigna · 2018-06-06T14:58:01Z

Dear @benjaminclot , @piwanczak ,

Thank you for your interest in Criteo Direct Bidder.

On the question above (usage of external library), Criteo follows the policies put in place by Prebid.org. Our adapter indeed makes use of our external library, so as to ensure that you can smoothly benefit from some of our latest updates. In doing so, we are following a review process from Prebid.org and comply with the the instructions and policy described by Prebid’s external JS repository mechanism (see link). Would you have any question, feel free to reach out your Criteo representative.

On Fastbid/eval: the function is being used to ensure correct working of our adapter in Prebid 1.0 and doesn’t represent a risk per se. In parallel, we are working on alternative versions, which will soon be submitted to our Prebid repo. In the meantime, would you have any concern, please contact your Criteo representative so as to address it.

Kind regards,
Maxence

piwanczak · 2018-06-11T13:39:59Z

@mkendall07 Could you please state the official approach by Prebid team?
My key points are:

I've seen a few external js used in the branch 0.X, but only Criteo in 1.X
prebid-js-external-js-template seems pretty lonely with 14 watches and zero stars. This seems to point that it's nowhere to be found in official Prebid docs
on the contrary - official docs forbid external JS's in adapters
non-minified extenal code seems not-up-to-date or unavailable:
https://github.com/Prebid-org/prebid-js-external-js-criteo/blob/master/dist/prod.js returns 404

As I strongly believe in "open" in opensource I feel it would be great for everyone to know the very same thing and be able to use the same tools.

@mafaigna Thank you for your input. Unfortunately this does not answer my question. I'm glad to hear that you work on alternatives, though.

mkendall07 reviewed Apr 9, 2018

View reviewed changes

snapwich assigned mkendall07 Apr 10, 2018

mkendall07 requested changes Apr 12, 2018

View reviewed changes

mkendall07 added the needs update label Apr 12, 2018

Spark-NF and others added 23 commits April 26, 2018 15:49

Convert Criteo adapter to bidderFactory

b6534c4

Add documentation for Prebid 1.0 Criteo adapter

1f7d0fa

Add support for zone-matching bids on Prebid 1.0 Criteo adapter

dc36bd3

Add unit tests to the Prebid 1.0 Criteo adapter

f780bc5

Explicit the fact that Criteo bids are net revenue

dd664a7

Pass currency in Criteo 1.0 adapter

85b65ed

Update Criteo adapter to use PublisherTag if present

ae7b2ca

Implement fastbid in prebid 1.0 criteo adapter

fa076c8

Pass the bid requests to the Criteo interpret method

761a203

Add missing ttl and creativeId fields to Criteo bids

6a8ea52

Add 'native' support to the Criteo adapter

d4ff98e

Check that the Criteo adapter returned by PublisherTag is not empty

b7d85e7

Fix 'assign to const' IE errors in Criteo native adapter

675d3ea

Update criteo prebid adapter to reload publisher tag once auction is

ac73344

finished

Disable the PublisherTag event queue

78b01ec

Fix Criteo adapter on older Prebid versions not using response.body

4bfa905

Fix TypeError if FastBid is outdated

fa787ba

Remove the success variable in tryGetCriteoFastBid

1b1a191

Fix events being overwritten with FastBid

6e34a4c

Update PublisherTag loading comment

36dec3b

Use adUnitCode as impid

14090c8

Add events handlers in Criteo adapter to fix timeouts not treated as …

fc2f2ac

…such

Add handler for setTargeting event

de434bd

Spark-NF added 5 commits April 26, 2018 15:49

Move the registeredEvents set higher up to reduce the chances of race…

d62e9a5

… conditions

Fix UTests following recent Criteo adapter changes

5e3534f

Add comment linking to the PublisherTag unminified source

c64ac12

Use loadExternalScript instead of loadScript

6d81ed4

Spark-NF force-pushed the criteo-1.0 branch from 8d222b5 to 6d81ed4 Compare April 26, 2018 13:50

Spark-NF and others added 3 commits April 26, 2018 15:57

Use spec.onTimeout instead of registering an event handler

ebf7dbc

GDPR support in Criteo adapter (#4)

5d7fdc4

GDPR support in Criteo adapter

Remove BID_WON and SET_TARGETING events from Criteo adapter

167f09a

Update adapter version

5b9c1cd

mkendall07 added the on hold label May 17, 2018

Add support for multi-size in Criteo adapter

47654b7

danconte72 mentioned this pull request May 21, 2018

Upgrade to latest prebid.js version meumobi/ams-hb#24

Closed

Fix support for multi size in Criteo adapter

dcea248

jsfaure force-pushed the criteo-1.0 branch from fbb9236 to dcea248 Compare May 24, 2018 09:49

Update adapterVersion to 7

597d8f7

mkendall07 merged commit ad1507b into prebid:master May 24, 2018

benjaminclot mentioned this pull request Oct 10, 2018

Prebid 1.27.0 increased 30% in filesize #3169

Closed

ghost mentioned this pull request Jul 26, 2019

Criteo violates the required adapter conventions by loading external libraries #4031

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Criteo bid adapter to Prebid 1.x #2370

Update Criteo bid adapter to Prebid 1.x #2370

Spark-NF commented Apr 9, 2018

mkendall07 Apr 9, 2018 •

edited

Loading

Spark-NF Apr 10, 2018

mkendall07 commented Apr 12, 2018

mkendall07 commented Apr 12, 2018

mkendall07 Apr 12, 2018

ahubertcriteo Apr 16, 2018

mkendall07 Apr 16, 2018

mkendall07 Apr 16, 2018

ahubertcriteo Apr 18, 2018

mkendall07 Apr 30, 2018

Spark-NF May 3, 2018 •

edited

Loading

mkendall07 Apr 12, 2018

ahubertcriteo Apr 16, 2018

mkendall07 Apr 16, 2018

mkendall07 Apr 12, 2018

ahubertcriteo Apr 16, 2018

mkendall07 Apr 24, 2018

mkendall07 Apr 12, 2018

ahubertcriteo Apr 16, 2018

mkendall07 Apr 12, 2018

ahubertcriteo Apr 16, 2018

mkendall07 Apr 16, 2018

ahubertcriteo Apr 18, 2018

mkendall07 commented Apr 16, 2018

bretg commented May 4, 2018

cwbeck commented May 22, 2018

benjaminclot commented May 25, 2018

piwanczak commented Jun 5, 2018

benjaminclot commented Jun 5, 2018

mafaigna commented Jun 6, 2018

piwanczak commented Jun 11, 2018


		registeredEvents = true;

		events.on(EVENTS.BID_WON, (bid) => {

Update Criteo bid adapter to Prebid 1.x #2370

Update Criteo bid adapter to Prebid 1.x #2370

Conversation

Spark-NF commented Apr 9, 2018

Type of change

Description of change

Other information

mkendall07 Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkendall07 commented Apr 12, 2018

mkendall07 commented Apr 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spark-NF May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkendall07 commented Apr 16, 2018

bretg commented May 4, 2018

cwbeck commented May 22, 2018

benjaminclot commented May 25, 2018

piwanczak commented Jun 5, 2018

benjaminclot commented Jun 5, 2018

mafaigna commented Jun 6, 2018

piwanczak commented Jun 11, 2018

mkendall07 Apr 9, 2018 •

edited

Loading

Spark-NF May 3, 2018 •

edited

Loading