google / openrtb

OpenRTB model for Java and other languages via protobuf; Helper OpenRTB libraries for Java including JSON serialization
Apache License 2.0
398 stars 190 forks source link

Version Specific BidRequest Writers #109

Open mzgupta opened 7 years ago

mzgupta commented 7 years ago

@opinali This library does not provide version specific writer. I mean if I want to send RTB 2.3 BidRequest to DSP1 and RTB 2.4 BidRequest to DSP2, the what is the best way to do it ?

opinali commented 7 years ago

Filtering messages by version is not a supported feature, i.e. I guess you want to populate a request with all signals you have but then automatically exclude 2.4+ specific fields for a DSP that supports 2.3, right? In theory this shouldn't be necessary, those DSPs that use older versions should just ignore the fields they don't understand, after all this is one of the advantages of a dynamic-typed format like JSON. Nobody should be parsing these messages with code that's strict enough to fail if it encounters unknown fields, if this happens it's a bug. However I also see the motivation of saving bandwidth, for that purpose this could be a nice feature... but it's some significant work to do right, it's not just exclude/include fields (lots of them) but many fields are deprecated, some of them had incompatible changes of type (actually spec clarifications), so the decision so far has been to avoid the whole problem and rely on SSPs/DSPs to be well-behaved with higher-spec messages.

mzgupta commented 7 years ago

@opinali solution I was thinking of keeping 2.3 and 2.4 schema both in the application and having separate reader and writer for each version.

You are right, in ideal case DSP should ignore the field. So if I go with your suggestion we should set event deprecated field. What if new version of open rtb remove deprecated fields ? You also mentioned there are incompatible changes too, what should we do for that ?

opinali commented 7 years ago

@mzgupta I think it wouldn't be hard to keep multiple versions of this entire library, using for example the Maven shade or jarjar plugins, but then you need different code to access each version because the generated protobuf classes will be incompatible so this may be only viable if you are using them only in some limited code (e.g some interop stuff that needs separate code per SSP/DSP anyway), but not viable to write core bidding logic that you want to reuse across platforms.

Having a single RTB model and associated utilities that work perfectly for all OpenRTB versions is possible, but it's a big hassle. At Google we do that in systems that needs to integrate with partner platforms, some of them not well behaved with the spec (e.g. extension fields without the ext.* containers). So the internal OpenRTB proto we use for that is much messier than this one, in fact this public proto is a "cleaned up" version of that internal proto.

One of the tricks you can use is this: declare multiple proto fields that will map to the same JSON field, for example have both companionad and companionad_21, the latter for the 2.1 spec which was incompatibly changed at 2.2+. Then have a JSON serializer that can be configured to map either of these proto fields to the companionad JSON field, and then also the app code that processes companion ads must have some extra code to handle both cases.

But the goal of this library is having an OpenRTB model that's as clean as possible, privileging compliance with the latest standard, and exchange-neutral (we keep all AdX-specific support in the separate openrtb-doubleclick library). So we're explicitly not supporting any hacks, not even for AdX. If you need to support special needs of some DSP or incompatible traits of ancient versions of the spec, you can fork the library to add the necessary hacks, then keep the fork in sync as the parent changes.

mzgupta commented 7 years ago

@opinali Thanks for your suggestion, how about extending the OpenRtbJsonWriter.java and overrides the methods and write according to versions

For V23 OpenRtbJsonWriterV23 extends OpenRtbJsonWriter

For V21 OpenRtbJsonWriterV21 extends OpenRtbJsonWriter

I also have to extend OpenRtbJsonFactory.