Kargo Analytics Adapter: Bid response time logging #8510
Conversation
…Xml response (prebid#8426) * kargo adapter - adding mediaType to bid response, conditionally set vastXml field * kargo adapter - updating tests
patmmccann
requested review from
robertrmartinez
and removed request for
ChrisHuie
|
@robertrmartinez do you need anything from our end to get started on the review? |
|
So with this code it looks like => On a given page load Any bid response from kargo after that WILL NOT be sent. What if a pub runs many aucitons on a given page? Do you still only want just one bidResponse event sent to your data pipeline? |
|
@robertrmartinez thanks for pointing that out. I've updated the code to reset the bool on auction end. We don't want to send multiple events for the same auction, but this should reset it for any additional auctions on the page. |
|
For majority of use cases this is prob fine. If pub runs multiple auctions at a time, then you can get weird results where only one event will be sent depending on when auction end fires per auction. But seems like you do not really care too much about this. If you did then what you could do is set a map of bools instead of just one global one. Offset by the auctionId so you only set it once per auctionId found. No big deal though. |
modules/kargoAnalyticsAdapter.js
Outdated
| } | ||
|
|
||
| _logBidResponseData.auctionId = bidResponse.auctionId; | ||
| _logBidResponseData.responseTime = bidResponse.responseTimestamp - bidResponse.requestTimestamp; |
There was a problem hiding this comment.
I think prebid core sets a param on this object called timeToRespond which is essentially this.
Also, you want to only track when your exchange does a VALID bid response? Not when it no-bids or errors?
There was a problem hiding this comment.
Updated to use timeToResponse - thanks for pointing that out.
Confirmed - for now we want to just track valid bid responses.
There was a problem hiding this comment.
Looks like this still is using response - request timestamps.
It is really up to you, just noting that there is timeToRespond available which is essentially the same thing.
|
Great idea on using a map to store the auction IDs - I decided just to use an array of auction IDs to keep it more simple. Is that okay or would you prefer the map? |
modules/kargoAnalyticsAdapter.js
Outdated
| } | ||
|
|
||
| _logBidResponseData.auctionId = bidResponse.auctionId; | ||
| _logBidResponseData.responseTime = bidResponse.responseTimestamp - bidResponse.requestTimestamp; |
There was a problem hiding this comment.
Looks like this still is using response - request timestamps.
It is really up to you, just noting that there is timeToRespond available which is essentially the same thing.
|
Ahh - looks like I didn't push the last change using |
| } | ||
|
|
||
| _logBidResponseData.auctionId = bidResponse.auctionId; | ||
| _logBidResponseData.responseTime = bidResponse.timeToRespond; |
There was a problem hiding this comment.
so having this object at module scope we could possibly see issues if two auctions are running at same time.
The right thing would prob be to just create a local object based off your wanted params here.
Even the auctionTimeout part may change between auctions.
Now it is probably fine majority of the time. So I can be fine with leaving it if you want.
Let me know.
* Kargo Bid Adapter: Use currency from Bid Response * Kargo Bid Adapter: Fix failed test * Kargo Bid Adapter: adding media type to bid response, supporting vastXml response (prebid#8426) * kargo adapter - adding mediaType to bid response, conditionally set vastXml field * kargo adapter - updating tests * Kargo Bid Adapter: onTimeout Support (#6) * Adding additional param * Adding response time function * Remove debug * Updating response time log to be set by bid response * Adding screen width/height to request * Test fix * Test fix * Removing interpretResponse signaling * Simplifying send data function * Kargo Analytics Adapter: Update with bid response time support * Renaming event route for auction data * Reset bid response data sent bool * Using array to store logged auctions * Update to use timeToResponse * Test fix Co-authored-by: Wei Wong <wwong@kargo.com> Co-authored-by: Andy Rusiecki <andy.rusiecki@gmail.com>
* Kargo Bid Adapter: Use currency from Bid Response * Kargo Bid Adapter: Fix failed test * Kargo Bid Adapter: adding media type to bid response, supporting vastXml response (#8426) * kargo adapter - adding mediaType to bid response, conditionally set vastXml field * kargo adapter - updating tests * Kargo Bid Adapter: onTimeout Support (#6) * Adding additional param * Adding response time function * Remove debug * Updating response time log to be set by bid response * Adding screen width/height to request * Test fix * Test fix * Removing interpretResponse signaling * Simplifying send data function * Kargo Analytics Adapter: Update with bid response time support * Renaming event route for auction data * Reset bid response data sent bool * Using array to store logged auctions * Update to use timeToResponse * Test fix Co-authored-by: Wei Wong <wwong@kargo.com> Co-authored-by: Andy Rusiecki <andy.rusiecki@gmail.com>
* Kargo Bid Adapter: Use currency from Bid Response * Kargo Bid Adapter: Fix failed test * Kargo Bid Adapter: adding media type to bid response, supporting vastXml response (prebid#8426) * kargo adapter - adding mediaType to bid response, conditionally set vastXml field * kargo adapter - updating tests * Kargo Bid Adapter: onTimeout Support (#6) * Adding additional param * Adding response time function * Remove debug * Updating response time log to be set by bid response * Adding screen width/height to request * Test fix * Test fix * Removing interpretResponse signaling * Simplifying send data function * Kargo Analytics Adapter: Update with bid response time support * Renaming event route for auction data * Reset bid response data sent bool * Using array to store logged auctions * Update to use timeToResponse * Test fix Co-authored-by: Wei Wong <wwong@kargo.com> Co-authored-by: Andy Rusiecki <andy.rusiecki@gmail.com>
Type of change
Description of change
Adds support for bid response time logging for Kargo bids
Other information
Related: prebid/prebid.github.io#3818