google/openrtb

Feedback concerning upgrading to OpenRTB 2.3 JSON / Protobuf protocol

Crystark opened this issue · 2 comments

Hi Osvaldo,

I was integrating with the new OpenRTB protocols and I wanted to report some troubles I had so you can improve the wiki.
Just as a reminder, there is currently 3 major protocols:
1- Adx Protobuf
2- OpenRTB Protobuf
3- OpenRTB JSON

What I understood from this library is that it handles the OpenRTB part whereas openrtb-doubleclick helps integrate with Adx Protobuf in an OpenRTB fashion.

The major problem I had is the billing_id which needs to be provided in the answer as the cid. In the OpenRTB JSON format there's no problem but with the OpenRTB protobuf, one needs to expressly declare the extension. In terms of code, this doesn't work:

return OpenRtb.BidRequest.parseFrom(bytes).getImp(0).getExtension(AdxExt.imp).getBillingIdCount(); 
#0

But this does:

ExtensionRegistry reg = ExtensionRegistry.newInstance();
reg.add(AdxExt.imp);
return OpenRtb.BidRequest.parseFrom(bytes,reg).getImp(0).getExtension(AdxExt.imp).getBillingIdCount();
#1

That's not enough: if you want to convert your protobuf object to json, you also need to configure the OpenRtbJsonFactory with the extensions and that requires the following:

return AdxExtUtils.registerAdxExt(OpenRtbJsonFactory.create());

Once all that is done, everything seems to work fine.
There's one thing though that troubles me still but it may be normal: I was expecting to be able to part from the openrtb-doubleclick library once I've moved to the OpenRTB protocols. However, AdxExt and AdxExtUtils are part of it and are required to handle the billing_id so even though you're integrating OpenRTB protocols, you need the Adx lib. This may be because the OpenRTB protocols aren't yet final but I thought I'd bring it up.

I hope all this can help improve the wiki.
Thanks again for this library.

Thanks for the feedback! On the need to register the extension, you are right, the problem is that the wiki is not a full tutorial so it doesn't show how to use the ExtensionRegistry. This is a problem because Protobuf extensions are not a feature that I should assume everyone will be familiar with and this registry is not an obvious step. You don't need to register extensions individually, do this instead:

AdxExt.registerAllExtensions(reg);

Also, even if you don't do this, the extension values are present in he proto model, they just aren't parsed but they can be accessed like this:

AdxExt.ImpExt.parseFrom(
    imp.getUnknownFields().getField(AdxExt.imp.getNumber())
        .getLengthDelimitedList().get(0));

Admittedly, this code is horrible :) even though it's simplified to assume that there will be exactly one ImpExt value. This is only good for advanced uses that need dynamic discovery of extensions, or lazy parsing for big extensions that are rarely used, etc. I will update the wiki with the registry step.

On the dependency to doubleclick-core, that's inevitable because the AdxExt is AdX-specific so there's no way I could move this to the openrtb-core library; everything here needs to be really exchange-neutral. Modularity is the whole point of using extensions in the first place so the protos can be independent and also associated utilities like JSON support. However I'm planning to split doubleclick-core in two pieces, moving the AdX->OpenRTB mapper to a separate library since now this is not useful for users that work with AdX's new on-wire OpenRTB protocols.

Wiki updated.