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

Split use of auctionid and transactionid in bidders for source.tid #8573

Closed
patmmccann opened this issue Jun 16, 2022 · 20 comments · Fixed by #9916
Closed

Split use of auctionid and transactionid in bidders for source.tid #8573

patmmccann opened this issue Jun 16, 2022 · 20 comments · Fixed by #9916

Comments

@patmmccann
Copy link
Collaborator

patmmccann commented Jun 16, 2022

Type of issue

Bug

Description

Some bidders set auctionId, others set transactionId. Source.tid should always be from the same field. Also, the PBS adapter is missing this field.

Discussed in the Prebid.js committee meeting. Here's the proposal:

  1. Prebid will recognize 3 different IDs:
    1a) auctionId (ORTB $.id). This should be unique for each request, so an auction with 5 bidders should have 5 $.id
    1b) global transactionId (ORTB $.source.tid). This should be set to the auctionId
    1c) adunit-level transactionId (ORTB $.imp[n].ext.tid).
  2. All three IDs can be overridden by the publisher:
    2a) auctionId - override already implemented as pbjs.requestBids as auctionId.
    2b) global transactionId - override with setConfig({ortb});
    2c) adunit-level transactionId - new override to be implemented as AdUnit.transactionId.

If the community agrees with this proposal:

  • Prebid-core work will be done to support the reading, generation, and propagation of these 3 IDs.
  • The pbsBidAdapter will be updated
  • Docs will be updated
  • We will update bid adapters that refer to any of these IDs and ask the maintainers to approve.

Related

#8543
#3190

Some bidders that appear to remain unsolved:

tid: bidRequest.transactionId,

@BizzClick @devBizzclick

tid: bidRequest.transactionId

@musikele

tid: bidRequest.transactionId

@supportGothamad

tid: bidRequest.transactionId

@adxpremium

deepSetValue(ozoneRequestSingle, 'source.tid', imp.ext[whitelabelBidder].transactionId);// RTB 2.5 : tid is Transaction ID that must be common across all participants in this bid request (e.g., potentially multiple exchanges).

@AskRupert-DM

deepSetValue(payload, 'source.tid', conf.transactionId);

@haibau

deepSetValue(payload, 'source.tid', conf.transactionId);
@pm-azhar-mulla

tid: context.bidRequests[0].transactionId,
@jbartek25

deepSetValue(npRequestSingle, 'source.tid', imp.ext['newspassid'].transactionId);// RTB 2.5 : tid is Transaction ID that must be common across all participants in this bid request (e.g., potentially multiple exchanges).
@AskRupert-DM

tid: validBidRequest.transactionId,
@rikdru

@dgirardi
Copy link
Collaborator

How is this issue separate from #8543 - and why is it a bug? It doesn't look like we have established enough of a consensus on what source.tid should be to decide that some adapters are doing it wrong.

@patmmccann
Copy link
Collaborator Author

In 3190 and in doc i believe it was established it should be transaction id

#8543 is about implementing the new community extension, not source.tid

@patmmccann
Copy link
Collaborator Author

patmmccann commented Jun 23, 2022

Committee discussion outcome:

set imp.ext.tid as the adunit transaction id --done
Pass imp.ext.tid it to prebid server --done
Pass the auction id as source.tid on client side adapters, not first imp transaction id
Pass auction id to PBS adapter in source.tid, sanity test the auction id that is in source.tid,
Publisher override behavior of auction id and impid tbd

@patmmccann
Copy link
Collaborator Author

patmmccann commented Jun 23, 2022

According to #7585 ; prebid server adapter is now generating its own tid because the auction id is insufficient. The first transaction id in the array seems like the way to go

@robertrmartinez

@maphe
Copy link
Contributor

maphe commented Aug 18, 2022

@patmmccann From a bidder standpoint, what's the "right" way to implement that?

source.tid = bidderRequest.auctionId;

or

source.tid = deepAccess(bidRequests[0], 'transactionId'),

I understand the purpose of this value is to be consistent across all bidders, making transactionId more explicit that it should match the open RTB tid aka "Transaction ID". But you seem to imply that Auction ID should be used instead, isn't there a risk that bidders get inconsistent with each other at some point?

@patmmccann
Copy link
Collaborator Author

@maphe my intention is that these shortly resolve to the same value. Planning to discuss in committee

@patmmccann
Copy link
Collaborator Author

patmmccann commented Aug 31, 2022

In short, this proposal is not popular. Instead; an alternate proposal adopted in committee, to be expanded upon [update, the description reflects the committee outcome], is that prebid will identify some value as source.tid and everyone that is reading some other value as the source.tid will be moved over to it.

@maphe in the meantime, I recommend source.tid = bidderRequest.auctionId;

@maphe
Copy link
Contributor

maphe commented Aug 31, 2022

Ok, perfect, thank you.

@maphe
Copy link
Contributor

maphe commented Aug 31, 2022

Follow up question, related to what I've read in this community extension doc, does it have any value in your opinion to implement something like

imp.ext.tid = deepAccess(bidRequest, 'ortb2Imp.ext.tid');

in addition to source.tid?

Adding more context to my questions, TheTradeDesk is reaching out to all their Prebid partners asking them to implement the transaction ID is their respective adapter/exchange, so our goal here is to implement it in a way where we're confident the other adapters will be consistent with.

@dgirardi
Copy link
Collaborator

IMO every adapter should do the equivalent of mergeDeep(imp, bidRequest.ortb2Imp) - that is, everything from ortb2Imp should be carried over, and if your endpoint speaks ORTB you might as well copy it.

maphe pushed a commit to sharethrough/Prebid.js-inventory-growth that referenced this issue Aug 31, 2022
`source.tid` is transaction id at the request level
`bidReq.ortb2Imp` may carry a transaction id at the impression level (`imp.ext.tid`)

Followed recommendations provided by contributors here: prebid#8573

PGE-178206904
@patmmccann
Copy link
Collaborator Author

My understanding from TTD is they plan to prefer imp.ext.tid to source.tid

@maphe
Copy link
Contributor

maphe commented Aug 31, 2022

Ok, that's good to know, I was not able to get this information internally. Anyway I'm covering all the bases by implementing both source.tid and imp.ext.tid (the way @dgirardi suggested) so we should be fine either way, assuming all other adapters will align...

@patmmccann
Copy link
Collaborator Author

one thing of note: source.tid || gpid will have the same cardinality as imp.ext.tid if and only if there are no repeated gpids per page.

@bretg
Copy link
Collaborator

bretg commented Sep 6, 2022

FYI - updated the issue description with the proposal we discussed in last week's meeting.

@maphe
Copy link
Contributor

maphe commented Sep 6, 2022

@bretg that looks different from what was said earlier in this thread, can you confirm it is not advised anymore to set

source.tid = bidderRequest.auctionId;

but it is advised instead to do

source.tid = bidderRequest.transactionId;

Also is it safe to assume that we should stop using generateUUID() for $.id and prefer bidderRequest.auctionId instead?

Finally I'm not quite clear on the timeline and what can be implemented in the adapter now vs later. I'm being asked to implement transaction id following TheTradeDesk demand, but it looks like it's not even possible quite yet.

@patmmccann
Copy link
Collaborator Author

Here is what ttd did https://github.com/prebid/Prebid.js/pull/8679/files

@bretg
Copy link
Collaborator

bretg commented Sep 7, 2022

can you confirm it is not advised anymore

The intention of this issue is to have a community discussion. Some of us believe the current ID structure is sub-optimal, specifically:

  • it's odd (some say bad) that the global source.tid is being set to the same value as the first imp.ext.tid
  • publishers do not currently have a way to override all the values to coordinate with other tech on their pages
  • there's inconsistency in the way that different adapters have approached IDs.

If the community agrees on the approach above, we would need a coordinated effort to make the core and adapter changes.

patmmccann pushed a commit that referenced this issue Sep 14, 2022
* [PGE-178206904] Send Transaction ID to STX

`source.tid` is transaction id at the request level
`bidReq.ortb2Imp` may carry a transaction id at the impression level (`imp.ext.tid`)

Followed recommendations provided by contributors here: #8573

PGE-178206904

* Bump adapter version

* Fix Transaction ID + Be more conscious about what we put in `imp.ext`
@patmmccann
Copy link
Collaborator Author

reconfiming this is resolved tpowards auctionid not imp[0].transactionid

@patmmccann
Copy link
Collaborator Author

flagging as possible #8539 enforcement

JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this issue Feb 9, 2023
* [PGE-178206904] Send Transaction ID to STX

`source.tid` is transaction id at the request level
`bidReq.ortb2Imp` may carry a transaction id at the impression level (`imp.ext.tid`)

Followed recommendations provided by contributors here: prebid#8573

PGE-178206904

* Bump adapter version

* Fix Transaction ID + Be more conscious about what we put in `imp.ext`
@patmmccann
Copy link
Collaborator Author

#9862 should make this much easier for everyone in the 8.0 branch! instead of auction id, just point to bidderRequest.ortb2.source.tid

jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this issue May 23, 2023
* [PGE-178206904] Send Transaction ID to STX

`source.tid` is transaction id at the request level
`bidReq.ortb2Imp` may carry a transaction id at the impression level (`imp.ext.tid`)

Followed recommendations provided by contributors here: prebid#8573

PGE-178206904

* Bump adapter version

* Fix Transaction ID + Be more conscious about what we put in `imp.ext`
AskRupert-DM added a commit to AskRupert-DM/ozone-2.8.0 that referenced this issue Jul 17, 2023
Ozone 2.9.0 updates

- changes required for PB 8 prebid#8573 - removed transaction IDs
- added our own id (as per ortb spec & required by ozone server)
- added support for batching 10 impression objects with a config switch 
- adding newspass as an alias
- update the spec tests for this
AskRupert-DM added a commit to AskRupert-DM/ozone-2.8.0 that referenced this issue Jul 17, 2023
- changes required for PB 8 prebid#8573 - removed transaction IDs
- added support for batching/limiting to 10 impression objects in a single request with a config switch - onpage and also in the adapter. Off by default. In the config use ozone.batchRequests: true
- added our own id as required by ozone server & also openrtb spec
- adding newspass as an alias
- update the spec tests
- adding support for gptPreAuction adapter (this is on by default when included in the build)
patmmccann pushed a commit that referenced this issue Jul 19, 2023
* Ozone: Adapter Updates - 2.9.0

- changes required for PB 8 #8573 - removed transaction IDs
- added support for batching/limiting to 10 impression objects in a single request with a config switch - onpage and also in the adapter. Off by default. In the config use ozone.batchRequests: true
- added our own id as required by ozone server & also openrtb spec
- adding newspass as an alias
- update the spec tests
- adding support for gptPreAuction adapter (this is on by default when included in the build)

* Update ozoneBidAdapter.js

fixes per feedback

* Add files via upload

removing alias support (added in by accident)

* Add files via upload

updated tests following removal of aliases
santii7395 pushed a commit to themaven-net/Prebid.js that referenced this issue Aug 28, 2023
* Ozone: Adapter Updates - 2.9.0

- changes required for PB 8 prebid#8573 - removed transaction IDs
- added support for batching/limiting to 10 impression objects in a single request with a config switch - onpage and also in the adapter. Off by default. In the config use ozone.batchRequests: true
- added our own id as required by ozone server & also openrtb spec
- adding newspass as an alias
- update the spec tests
- adding support for gptPreAuction adapter (this is on by default when included in the build)

* Update ozoneBidAdapter.js

fixes per feedback

* Add files via upload

removing alias support (added in by accident)

* Add files via upload

updated tests following removal of aliases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

5 participants