Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
robhazan opened this issue Sep 14, 2022 · 11 comments
Closed
Labels
PBS-Go Ready For Dev Feature specification is ready to be developed.

Comments

@robhazan
Copy link
Contributor

IAB recently approved a new Community Extension to communicate transaction IDs in OpenRTB bid requests: https:/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 prebid/Prebid.js#8543

@SyntaxNode
Copy link
Contributor

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
Copy link
Collaborator

bsardo commented Oct 18, 2022

@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
Copy link
Contributor

bretg commented Oct 19, 2022

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 #1507

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

$.id - existing behavior from #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 #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
Copy link
Contributor

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 prebid/Prebid.js#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
Copy link
Contributor

bretg commented Nov 1, 2022

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 -- prebid/Prebid.js#8591 . I don't see any logic about setting source.tid.

So are we good here?

@bretg
Copy link
Contributor

bretg commented Nov 2, 2022

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

@bretg bretg added the Ready For Dev Feature specification is ready to be developed. label Nov 2, 2022
@ccorbo
Copy link
Contributor

ccorbo commented Nov 3, 2022

Thanks all, will be implementing this logic as an update to PR: #2412

@bretg
Copy link
Contributor

bretg commented Nov 30, 2022

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
Copy link
Contributor

bretg commented Dec 20, 2022

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
Copy link
Contributor

bretg commented Jan 11, 2023

Done in PBS-Java 1.107

@bretg bretg added the PBS-Go label Jan 11, 2023
@SyntaxNode
Copy link
Contributor

Implemented In PBS-Go 0.238.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PBS-Go Ready For Dev Feature specification is ready to be developed.
Projects
Status: Done
Development

No branches or pull requests

5 participants