Tercept Analytics Adapter: single request per auction, concurrent auction isolation & payload optimisation#14679
Open
mdusmanalvi wants to merge 7 commits intoprebid:masterfrom
Open
Conversation
- Replace shared events object with per-auctionId Map to fix data loss when multiple auctions run concurrently - Accumulate BID_WON, AD_RENDER_SUCCEEDED, AD_RENDER_FAILED and BIDDER_ERROR into the bids array; flush once 1.5s after AUCTION_END - Use navigator.sendBeacon on visibilitychange so data survives page exit - Add disableAnalytics to clear pending timers and maps on teardown - Add try/catch around track() to prevent adapter errors disrupting page - Remove ad, native and adUrl fields from bid payloads to reduce size - Limit auctionInit.bidderRequests to first entry (device/site data is identical across all bidder requests) - Add is_pl flag (true on first bid of first auction per page load)
Replace tests that assumed multiple requests per auction and separate top-level keys (bidWon, adRenderSucceeded, etc.) with tests matching the new single-request-per-auction design. Covers: - Request timing: no send before 1.5s timer, one request per auction - Payload structure: top-level shape, bidderRequests trimmed to first, host/path/search attached at send time, ad/native/adUrl absent - All event handlers: BID_REQUESTED, BID_RESPONSE, BID_TIMEOUT, NO_BID, BID_WON, AD_RENDER_SUCCEEDED, AD_RENDER_FAILED, BIDDER_ERROR - is_pl flag: true on first bid of first auction only, reset on disable - Concurrent auction isolation: separate bids and auctionInit per id - visibilitychange flush via sendBeacon - disableAnalytics: cancels timers, clears maps, resets firstSent - Error resilience: unknown auctionIds and undefined args do not throw
|
Tread carefully! This PR adds 81 linter errors (possibly disabled through directives):
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a55a159699
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Collaborator
Coverage Report for CI Build 24229648318Coverage increased (+0.003%) to 96.347%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions2 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
…mport - Fix 81 indent errors by properly indenting the try block body - Replace window.addEventListener with document.addEventListener for visibilitychange — the event is non-bubbling and fires on document, so window never received it in Firefox/Safari - Replace navigator.sendBeacon with the ajax.js sendBeacon wrapper as required by Prebid.js linting rules - Update test to stub ajax.sendBeacon and dispatch event on document
Split multi-property spread objects onto separate lines to satisfy the @stylistic/object-property-newline ESLint rule.
Contributor
Author
|
@jefftmahoney Had already resolved the codex review comments. Would it be possible to perhaps expedite the review? |
Contributor
Author
|
@patmmccann was really hoping this could be part of the next release, would it be possible to have this reviewed sooner? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change
Description of change
Refactored the Tercept Analytics Adapter to fix a data-loss bug with concurrent auctions, reduce request volume from ~4 per auction to 1, and trim payload size.
Concurrent auction isolation (bug fix):
eventsobject with aMapkeyed byauctionIdAUCTION_INITwould reset shared state, silently dropping data from any in-flight auction; each auction is now fully isolatedSingle request per auction:
BID_WON,AD_RENDER_SUCCEEDED,AD_RENDER_FAILED,BIDDER_ERROR) are now accumulated into the bids array and flushed in one POST ~1.5s afterAUCTION_ENDPage-exit reliability via
sendBeacon:visibilitychangehandler flushes pending auctions usingnavigator.sendBeaconinstead ofajax, guaranteeing delivery when the user navigates away before the 1.5s timer firesPayload size reduction:
ad(full creative markup, 10–50 KB per bid),nativepayload, andadUrlfrom bid objectsauctionInit.bidderRequeststrimmed to first entry only — device and site data is identical across all bidder requestsResilience and cleanup:
try/catcharoundtrack()so any unexpected error does not disrupt the publisher pagedisableAnalyticsto cancel pending timers and clear maps on teardown, preventing state leaking across SPA navigations