prebid / prebid-server

Open-source solution for running real-time advertising auctions in the cloud.
https://prebid.org/product-suite/prebid-server/
Apache License 2.0
431 stars 739 forks source link

Support a flag to allow VAST modification for unknown bidders #1945

Open bretg opened 3 years ago

bretg commented 3 years ago

PBS (Java at least, and probably Go too) supports a flag to disable VAST modification on a per-bidder basis. This was a safety net implemented back when we first dipped our toe into the wild world of screwing with bidder-supplied VAST. We didn't know if we would run across scenarios where PBS broke the VAST so we engineered a config to back out of the modification on a per-bidder basis should something go wrong. Thankfully, nothing's gone wrong in the years that this has been done.

There's a new wrinkle in this story though, which is that PBS defaults to assuming that VAST cannot be modified if the bidder is unknown. There's now a scenario where client-side video bids are coming into the /vtrack endpoint with biddercodes that PBS doesn't recognize. That VAST is dutifully being stored in PBC, but's not being modified to have the tag because the bidder is unknown.

The proposal here is to add a new host-company config vtrack.modify-vast-for-unknown-bidder so that we don't make breaking change. Without this flag, PBS would start modifying VAST upon upgrade in scenarios where it wasn't before. The existence of this flag allows host companies to review and control the timing of the change.

Here's a new summary of what should happen when a /vtrack request comes in:

1) If what is being cached is type other than 'xml', then reject the request with HTTP 400 and a warning "vtrack only accepts type xml". 2) verify that the value contains "<vast" anywhere in the string (case insensitive) otherwise reject the request with HTTO 400 and a warning "vtrack content must be vast" 3) If the 'bidder' is known by PBS, look in the bidder.yaml file for modifying-vast-xml-allowed. If allowed, then modify the VAST to add the impression tag. 4) If the bidder is unknown and vtrack.allow-unknown-bidder is true, check the new vtrack.modify-vast-for-unknown-bidder config. If that's true (or not specified), then modify the VAST to add the impression tag.

Note that checks 1 and 2 above are also new. PBJS only ever sends /vtrack requests with type: xml, so we should not be concerned with a breaking change here.

SyntaxNode commented 3 years ago

I don't understand this use case. Why would Prebid Server receive a vtrack call for an unknown bidder?

bretg commented 3 years ago

Why would Prebid Server receive a vtrack call for an unknown bidder?

PBJS can be configured to cache client-side bidder VAST results in Prebid Server. Those bidders may not have server-side equivalents or they may not be enabled in PBS.

hhhjort commented 3 years ago

Is this to enable a PBS analytics module to be used by PBJS to track bidders unavailable on PBS?

SyntaxNode commented 3 years ago

Gotcha. Two further questions from the committee discussion:

bretg commented 3 years ago

What would happen if a PBS Analytics Adapter is presented with an unknown bidder? Is this a breaking change?

Analytics adapters should not have any concept of what bidders are enabled in a given Prebid Server instance. They get the biddercode from the request. If any of them do validation, I suppose they might log a warning, but it's unclear what they would validate against... since they don't have a coded list of enabled adapters, I guess they might have analytics config? Anyhow, bidder validation is definitely not a part of the analytics adapters I'm aware of. Would be willing to update https://docs.prebid.org/prebid-server/developers/pbs-build-an-analytics-adapter.html

Does this begin to position PBS as a generic analytics engine?

PBS has been the only reliable source of recording video impression events for years. The Prebid.js Instream Tracking module relies on a condition that Magnite deems unsafe for a key performance indicator, so PBS is how we track video imps, whether server-side or client-side.

So while I wouldn't say it's a "generic analytics engine" by any means, I would agree that PBS supports Prebid video events.

bretg commented 3 years ago

Clarified the proposed /vtrack algorithm to better cover known and unknown scenarios and define what to do when checks fail.

bretg commented 3 years ago

I'm told that PBS-Java already updated the VAST for /vtrack requests so vtrack.modify-vast-for-unknown-bidder is being questioned.

I'm proposing that to avoid breaking changes we flip the default -- that vtrack.modify-vast-for-unknown-bidder is still added but defaults to true.

bretg commented 3 years ago

Released with PBS-Java 1.71

stale[bot] commented 2 years ago

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.