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
428 stars 732 forks source link

Feature Request: Per-Impression Transaction IDs (BidRequest.imp[].ext.tid) #2381

Closed robhazan closed 1 year ago

robhazan commented 2 years ago

IAB recently approved a new Community Extension to communicate transaction IDs in OpenRTB bid requests: https://github.com/InteractiveAdvertisingBureau/openrtb/blob/master/extensions/community_extensions/per-imp-tids.md

Historically, source.tid is intended to represent a transaction ID (i.e. an ID referring to a specific impression opportunity that a seller/buyer may transact on). The problem with source.tid is that it lives at the BidRequest level - i.e. there’s only one per bid request. However, a bid request can have an array of impressions in it, and thus an array of transactions may occur.

Hanging tid off of imp.ext.tid acknowledges this reality and allows buyers and sellers to communicate about multiple transactions within one bid request. We should add support for per-impression transaction IDs to Prebid Server. It's been added to Prebid.JS per https://github.com/prebid/Prebid.js/issues/8543

SyntaxNode commented 2 years ago

PBS already has support for the imp[].ext.tid extension. However, there is an option where PBS will generate a source.tid if it's missing on the request. It sounds like we should extend that behavior to imp[].ext.tid.

bsardo commented 2 years ago

@bretg, I'd like to clarify the expected behavior here. I've reviewed the extension spec which introduces imp.ext.tid and refers to how source.tid is the fallback for any imps that don't have imp.ext.tid set.

PBS-Go currently sets source.tid to a randomly generated UUID if source.tid is not set on the request.

There's an open PBS-Go PR that changes this logic to the following:

if source.tid is not set:
    for each imp:
        if imp.ext.tid is not set:
           set imp.ext.tid to a randomly generated UUID
    if at least one imp did not have imp.ext.tid set:
        set source.tid to the request ID

Is this correct, particularly the part about setting source.tid to the request id (which I think is synonymous with transactionID in prebid.js)?

bretg commented 2 years ago

What was discussed for Prebid.js is that all of the various IDs are separate. The algorithm above seems ties the setting of imp.ext.tid to source.tid not being there and I don't understand "if at least one imp did not have imp.ext.tid set: set source.tid to the request ID". Further, this approach doesn't take into account that imp.ext.tid may be defined statically in stored requests. See https://github.com/prebid/prebid-server/issues/1507

I would recommend something more comprehensive for each of the 4 IDs

$.id - existing behavior from https://github.com/prebid/prebid-server/issues/1507

if host config generate-storedrequest-bidrequest-id config is true
    if $.id is not set, generate a random value
    if the storedrequest is from AMP or from a top-level stored request (ext.prebid.storedrequest), then replace any existing $.id with a random value
if $.id contains "{{UUID}}", replace that macro with a random value

$.source.tid - There was much debate in PBJS circles about setting source.tid to .imp[0].tid. Several of us disagreed with this though, so I propose expanding the behavior slightly to overwrite existing values that might be in the static storedrequests tying the new behavior to the same config flag:

if source.tid is not set:
   set source.tid to a random UUID  // existing behavior
// new behavior
if host config generate-storedrequest-bidrequest-id config is true
    if the storedrequest is from AMP or from a top-level stored request (ext.prebid.storedrequest), then replace any existing $.source.tid with a random value
if $.source.tid contains "{{UUID}}", replace that macro with a random value

$.imp[].id - from https://github.com/prebid/prebid-server/issues/1507

if host config generate-storedrequest-bidrequest-id config is true
    if any $.imp[].id is missing, set it to a random 16-digit string. (Note: this wasn't in issue 1507)
    if the storedrequest is from AMP **or** from a top-level stored request (ext.prebid.storedrequest), confirm that all $.imp[].id fields in the request are different. If not different, re-number them all starting from "1".

$.imp[].ext.tid - extending the "empty" scenario to cover scenarios where a static value may be set in an AMP/app storedrequest, again tying that to the host config

for each imp:
   if imp[n].ext.tid is not set:
       set imp[n].ext.tid to a randomly generated UUID
   if host config generate-storedrequest-bidrequest-id config is true
       if the storedrequest is from AMP or from a top-level stored request (ext.prebid.storedrequest), then replace any existing $.imp[n].ext.tid with a random value
  if $.imp[n].ext.tid contains "{{UUID}}", replace that macro with a random value
SyntaxNode commented 1 year ago

Further, this approach doesn't take into account that imp.ext.tid may be defined statically in stored requests.

There is no need to take this into account. The transaction id should not be statically defined. That defeats its purpose. If this is done today, we should discourage it.

if at least one imp did not have imp.ext.tid set: set source.tid to the request ID

I believe this was suggested by @robhazan in the Prebid Server Committee meeting. I do not recall the reason why source.tix and imp.ext.tid are related.

Generally, I'ld like to match PBJS's behavior. There was a discussion in https://github.com/prebid/Prebid.js/issues/8543 that source.tid should be used as a fallback if imp.ext.tid is not defined, so perhaps that's where link came from?

bretg commented 1 year ago

There is no need to take this into account. The transaction id should not be statically defined.

We need to define the edge case even if not encouraged. In the PBS committee we did talk about supporting a {{UUID}} macro.

Generally, I'ld like to match PBJS's behavior

This is the PBJS PR -- https://github.com/prebid/Prebid.js/pull/8591 . I don't see any logic about setting source.tid.

So are we good here?

bretg commented 1 year ago

@robhazan commented in Prebid slack that random IDs will be ok. Flagging as ready-for-dev

ccorbo commented 1 year ago

Thanks all, will be implementing this logic as an update to PR: https://github.com/prebid/prebid-server/pull/2412

bretg commented 1 year ago

The PBS-Java team pointed out that there's an existing config auction.generate-source-tid. This config should be obsoleted - would prefer a deprecation period where existence of the config won't stop PBS startup but generate a warning that this config is now ignored.

bretg commented 1 year ago

since imp.id is a string, the PBS-Java team noted that taking the numeric value could be problematic or expensive.

So we propose instead that missing imp.id would be random:

...
if $.imp[].id is missing, set it to a random 16-digit string
...

Heads up @ccorbo

bretg commented 1 year ago

Done in PBS-Java 1.107

SyntaxNode commented 1 year ago

Implemented In PBS-Go 0.238.0.