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
434 stars 742 forks source link

PBS video impression tracking #1015

Closed bretg closed 2 years ago

bretg commented 5 years ago

As a followup to the event tracking issue #800, we're ready to implement the injection of additional tracking into VAST XML. Initially this is just to count impressions, but will be expanded to additional video events at some point. There are two scenarios, both of which are required and could be happening at the same time:

1) Server-side VAST - this scenario covers the video XML is returned in the response from a server-side adapter. If that bidder allows PBS to modify their VAST, the server then injects an tag into the VAST, pointing to a prebid server event string, and then caches the modified results.

Here's a flow diagram showing how tracking will work. Changed items are in red.

Screen Shot 2019-08-27 at 11 34 35 AM

2) Client-side VAST - this scenario occurs when the video XML is returned in the response from a client-side adapter. PBJS forwards the XML to PBS on a new endpoint that instructs PBS to update the XML and cache it. If that bidder allows PBS to modify their VAST, the server then injects an tag into the VAST, pointing to a prebid server event string, caches the modified results and responds to the client.

Screen Shot 2019-08-27 at 9 08 02 PM

Proposed modifications

1) Add a new bidder-level flag in PBS that allows a given bidder to turn off our ability to modify their VAST. e.g. if bidderA doesn't want us to modify their VAST, the host company won't have complete analytics.

We propose a new bidder configuration property modifyingVastXmlAllowed, with the default being false.

2) When PBS receives VAST from each bidder, it checks the bidder flag and whether the account supports events. If so, it inserts an \<impression> tag before storing to PBC

The contents of the \<impression> tag are pulled from a new event.url-template property that has macros that need to be resolved. e.g.

event:
    url-template: "/event?t=imp&b=%s&f=b&a=%s"

where b=BIDID, a=ACCOUNT

3) Algorithm for updating the VAST. (originally this was over-simplified. Changed May 13, 2021)

Scan the 'adm' on video bids:

Once we know what type of VAST is present:

4) Client-Side VAST Caching, including PBJS changes

Unlike server-side video, the VAST XML coming into the browser didn't go through Prebid Server, so will not have the tracking strings added. Prebid.js has configuration that allows the publisher to initiate "client-side caching". In order to modify the VAST XML, the page will refer to a new end point on Prebid Server that will perform the same modifications as if it had come through the server.

pbjs.setConfig({
    cache: {
        url: "https://prebid-server.rubiconproject.com/vtrack?a=ACCOUNT",
        vasttrack: true
    }
});

Where /vtrack is a new endpoint that causes the request to go to PBS where the steps 1-3 above will be applied.

If the vasttrack parameter is true, Prebid.js will add a couple of parameters to the POST XML to the specified endpoint with this JSON

{"puts":[{
    "bidid": BIDID,             // new parameter
    "bidder": "BIDDER",    // new parameter
    "type":"xml",
    "value":"<VAST…/VAST>",
    "ttlseconds":3600
}]}

PBS doesn't currently have a /vtrack endpoint -- we shouldn't use /cache because PBC implements that one. The new endpoint would use a similar code path as for server-side VAST:

PBS should validate the arguments supplied with /vtrack: if ACCOUNT, BIDID, or BIDDER aren't supplied, it should reject the /vtrack request with 400. This should get caught when the page is being tested.

If the ACCOUNT doesn't exist and PBS is enforcing accounts, that should also result in a 400.

PBS will process /vtrack similar to the server-side VAST response, inserting impression tracking when:

Closed Item:

bretg commented 5 years ago

For load balancer reasons, I'm starting to prefer a separate endpoint /vtrack.

rpanchyk commented 5 years ago

Hi @bretg ! From PBS configuration point of view i propose to use instead of

events:
    urltemplate: "https://prebid-server.rubiconproject.com/event?t=imp&b=%s&f=b&a=%s"

next:

event:
    url-template: "/event?t=imp&b=%s&f=b&a=%s"

So, the changes:

  1. prefix for the property - event.* since PBS-Java has /event endpoint, so can be more acceptable.
  2. name for the property - url-template to be more readable.
  3. value for the property - includes path with query string since both PBS-Go and PBS-Java has ExternalURL configuration property, so it can be simply joined to obtain full url.
bretg commented 5 years ago

Thanks @rpanchyk - updated.

bretg commented 5 years ago

Discussed and approved in PBS PMC

bretg commented 5 years ago

This is done in PBS-Java. Assigning to @hhhjort for PBS-Go implementation.

bretg commented 4 years ago

For the record, here are the PBS-Java PRs that implemented events:

https://github.com/rubicon-project/prebid-server-java/pull/296 https://github.com/rubicon-project/prebid-server-java/pull/298 https://github.com/rubicon-project/prebid-server-java/pull/302 (partially) https://github.com/rubicon-project/prebid-server-java/pull/403 https://github.com/rubicon-project/prebid-server-java/pull/436 https://github.com/rubicon-project/prebid-server-java/pull/437 https://github.com/rubicon-project/prebid-server-java/pull/450 https://github.com/rubicon-project/prebid-server-java/pull/459 (refactoring) https://github.com/rubicon-project/prebid-server-java/pull/635 https://github.com/rubicon-project/prebid-server-java/pull/654 https://github.com/rubicon-project/prebid-server-java/pull/804 (doc)

SyntaxNode commented 4 years ago

This is largely complete in PBS-Go. @laurb9 noted two remaining things to implement:

@danielguedesb Are you able to continue contributing to add those areas of functionality?

laurb9 commented 4 years ago

This older ticket did not define the entire functionality needed for events and I can't find another one that does (edit: or maybe #1470 ), so I will add it here from https://docs.google.com/document/d/1ry0X4C2EV-R0pMrm1IQk9BstxaT395UCl3KKqTGa5c8/edit# so we are all on the same page. Let me know if this should be another ticket. I'm starting work on this.

  1. If the account doesn't support events, we're done - no need for any type of event logic.
  2. Otherwise, if the bid request was video and the response is VAST XML:
    1. If we're allowed to modify the bidder's VAST, then inject an tag with this URL -- https://PBS_HOST/event?t=imp&b=BIDID&f=b&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION
    2. If ext.prebid.cache.vastXml is specified, then cache the VAST in PBS
  3. else // not video
    1. If bid caching is turned on (ext.prebid.cache.bids)
      1. If the event configuration is on for this account/channel combination or if the ext.prebid.events object is defined in the original request, then add wurl to bids cached in PBC. URL is https://PBS_HOST/event?t=win&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION
      2. else if it's a PG bid, then add wurl to bids cached in PBC but with x=0 https://PBS_HOST/event?t=win&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION&x=0
    2. Finally, consider setting seatbid[].bid[].ext.prebid.events.win and seatbid[].bid[].ext.prebid.events.imp
      1. If the event configuration is on for this account/channel combination or if the ext.prebid.events object is defined in the original request, then add response.seatbid[].bid[].ext.prebid.events.{win,imp} to the openrtb output -- https://PBS_HOST/event?t=win&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION and https://PBS_HOST/event?t=imp&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION
      2. else if it's a PG bid, then add response.seatbid[].bid[].ext.prebid.events.{win,imp} to the openrtb output -- https://PBS_HOST/event?t=win&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION&x=0 and https://PBS_HOST/event?t=imp&b=BIDID&f=i&a=ACCOUNT&ts=TIMESTAMP&bidder=BIDDER&int=INTEGRATION&x=0
SyntaxNode commented 4 years ago

Let me know if this should be another ticket.

I think we're good with continuing the discussion here.

I'm starting work on this.

Sweet. Thank you.

laurb9 commented 3 years ago

I've implemented most of the logic in #1597 . The goal was to get the unit tests to a point where we can validate the behavior, then do some refactoring. Some of the tests are currently failing in a non-significant way because of timestamp mismatch (until #1584 is merged) but are covering the behavior we need.

I see a few differences between what PBS-Java does and the algorithm above:

bsardo commented 3 years ago

Let's discuss the revised algorithm presented by @laurb9 above at the prebid.org committee meeting this Friday.

bretg commented 3 years ago

Only the cached VAST is modified, the VAST returned in adm field is not, in returned or cached bids. Is this intentional ? I have copied this behavior in the PR above.

The algorithm says "If ext.prebid.cache.vastXml is specified, then cache the VAST in PBS. Otherwise, just return the modified VAST." So this is a PBS-Java bug. Opened an internal ticket to resolve. Heads up @rpanchyk

However, I did notice that the algorithm summary didn't check that events were enabled for video before adding the tag. Updated to note that "If events are enabled for this account". See https://docs.google.com/document/d/1ry0X4C2EV-R0pMrm1IQk9BstxaT395UCl3KKqTGa5c8/edit#

bid caching and vast modification are not exclusive as the else above suggests. PBS-Java will change both VAST and cached bids if enabled. This made sense to me, so that's what I did too.

Algorithm summary fixed.

PBS-Java adds wurl to cached bid if both account and request enabled it. The algorithm says either suffices, I implemented the algo.

Another PBS-Java ticket opened to confirm. Thanks.

int (integration) does not get a value, so I didn't add it yet.

Integration is defined in https://github.com/prebid/prebid-server/issues/1428

laurb9 commented 3 years ago

The algorithm says "If ext.prebid.cache.vastXml is specified, then cache the VAST in PBS. Otherwise, just return the modified VAST." So this is a PBS-Java bug.

I'm going to work on updating the VAST everywhere in #1597 as well, in that case. To do that, I think I might need some clarification on expected behavior for the case below:

when bid.adm is empty, PBS-Go makes up a VAST with <VASTAdTagURI><![CDATA[' + bid.NURL + ']]></VASTAdTagURI> just for VAST cache, but it still caches and returns the bid with empty adm.

What should happen when events are enabled and bid.adm is empty ? I feel should generate the VAST, include the Impression events and return the same VAST everywhere (vast, cached bid, returned bid). That is inconsistent with the above behavior, so maybe we should return the same thing all the time, generated or not ?

laurb9 commented 3 years ago

More questions:

Should ext.prebid.events and vast rewriting apply to:

  1. only on winning bids and only when targeting is enabled ? (current implementation)
  2. only on winning bids always ?
  3. all bids ?
  4. other ?
bretg commented 3 years ago

Thanks for pushing this @laurb9 - this is quite intricate. I've revised (somewhat heavily) the Events doc. I don't think the algorithm has changed much, but trying to structure to document to make the various constraints more clear.

So specifically answering your questions:

What should happen when events are enabled and bid.adm is empty ? I agree with you: generate the wrapper and operate as if that wrapper is what had come from the bidder in the first place.

Should ext.prebid.events and vast rewriting apply to

3, because we always return all bidresponses and don't know for sure which one will be chosen by the client.

@rpanchyk - while researching this in PBS-Java code, I reviewed the "analytics_config" database column as implemented and have questions.

1) what is the format of this DB field? Based on a brief tour in the code, I believe it an example would be:

{"auction-events": {"amp": true, "app": true, "web": true}}

But our database refers to auction_events -- note the underscore rather than the dash. Which is correct?

2) Does the auction-events JSON also define the win and imp events? i.e. are all 3 types of events tied together? It appears so.

rpanchyk commented 3 years ago

hi, @bretg

  1. "auction-events" is the name PBS expects in DB.
  2. bid.ext.prebid.events.{win,imp} is added to response only if account has enabled events AND incoming request allows events (defined appropriate channel OR has ext.prebid.events field).
laurb9 commented 3 years ago

Thank you @bretg and @rpanchyk for the updates. I'm working on implementing the changes and updating the PR.

laurb9 commented 3 years ago

I've tried to make a truth table to clarify some of the situations. How does this look ?

bidType == video account events account events channel ext prebid events bid ~dealID~ is PG bidder vastEnable vast modify bid ext prebid events bid.wurl, if caching  
     
    x=0
    x=0
         
       
       
       
  x=0
      x=0
       
laurb9 commented 3 years ago

@bretg I've had a few more questions while working on adding the unit tests based on the truth table above:

  1. from doc

    Make the request.ext.prebid.events object consistent and work for VAST as well as banner/native.

I do not see where this happened, maybe I am looking at a cached doc version ? request.ext.prebid.events only impacts non-video bids as far as I can tell.

  1. from the doc

    Video impression Events - always generated if events are enabled for the account

I assume that is a qualified "always" because of this in the algorithm

If we're allowed to modify the bidder's VAST

  1. How do we define a PG bid ? bid.dealID being set ?

  2. What is LINEID ? Is it the GAM lineitem (imp.ID), the bid.dealID or something else ?

  3. from the doc

    PG always needs to get notified about bidsWon in order to pace.

Does this override the account settings ? Should this be host-controlled, if they implemented pacing in some way since it's not part of PBS ?

  1. In what scenarios should request (.ext.prebid.events) override account (.events_enabled) ? Because PBS will receive events for an account that has events disabled, and that can be confusing.
laurb9 commented 3 years ago

I've updated the PR to what I think is a potentially releasable functionality. I see exchange code is a refactor target, so I'd like to defer the additional PG and channels functionality as an incremental update for later, to avoid more PR conflicts

bretg commented 3 years ago

@laurb9 - created a somewhat modified version of your truth table at the bottom of https://docs.google.com/document/d/1ry0X4C2EV-R0pMrm1IQk9BstxaT395UCl3KKqTGa5c8/edit# . The only comment for your version:

Responding to questions in your comment:

request.ext.prebid.events only impacts non-video bids as far as I can tell

right - that was the original set up -- the proposal here is that request.ext.prebid.events overrides the server account event/analytics config for video as well as banner/native.

assume that is a qualified "always"

Removed the word "always".

  1. How do we define a PG bid ? bid.dealID being set ?
  2. What is LINEID ? Is it the GAM lineitem (imp.ID), the bid.dealID or something else ?
  3. from the doc

PG not supported in PBS-Go.

  1. In what scenarios should request (.ext.prebid.events) override account (.events_enabled) ? Because PBS will receive events for an account that has events disabled, and that can be confusing.

The idea is that request.ext.prebid.events is a request-level override to the server-side config.

defer the additional PG and channels functionality as an incremental update for later, to avoid more PR conflicts

PG may not be built in PBS-Go. That would be a fairly major effort and ought to be driven by demand for the feature.

Channels -- PBS-Java has new account level "analytics_config" that defines analytics on a per-channel basis. Unfortunately I don't think the JSON attribute very well named. Currently it's "auction-events", but it's really more a generic flag for analytics support by channel. Will open a separate discussion on whether we want to make changes there. https://github.com/prebid/prebid-server/issues/1470

laurb9 commented 3 years ago

I updated the code and tests so ext.prebid.events is sufficient to trigger VAST modification as well as banner events response. I've left out the channels and PG.

Please review the unit tests at exchange/exchangetest/events-bid-account-off-request-off.json exchange/exchangetest/events-bid-account-off-request-on.json exchange/exchangetest/events-bid-account-on-request-off.json exchange/exchangetest/events-vast-account-off-request-off.json exchange/exchangetest/events-vast-account-off-request-on.json exchange/exchangetest/events-vast-account-on-request-off.json

cached wurl tests are in exchange/eventscachetest

SyntaxNode commented 3 years ago

@laurb9 We've merged your events PR. Would you say that PBS-Go now implements this feature or are there still gaps?

laurb9 commented 3 years ago

Out of the things specced in the document, we are only missing PG and integration channels.

PG was said to be not implemented yet in PBS-Go, so that part can be added when PG functionality is worked on.

Integration channels: PBS-Go has a per-account enable flag for events. PBS-Java has that, plus three additional subflags that can control it for amp, app and web individually for each account if set. This was mentioned here. This, I have not implemented because I was left the impression that channel mapping needed further discussion (#1428, #1470), and the channel wasn't readily available in the auction for me to use.

bretg commented 3 years ago

Updated VAST updating algorithm

AlexBVolcy commented 2 years ago

This issue will be closed, as everything requested here has been implemented besides PG. And since PG is a big feature, we decided to branch that off into it's own issue https://github.com/prebid/prebid-server/issues/2312