Update bidding_auction_for_debugging_only.md #236
Conversation
mittalprakhar
commented
Apr 7, 2025
- Added rejectReason as a supported placeholder
- Added sequence diagram for device-orchestrated multi-seller auction
- Added sequence diagram for downsampling
- Clarified downsampling flags
- Clarified MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB only applies to urls sent over the network between B&A services
There was a problem hiding this comment.
Hello @mittalprakhar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request updates the bidding_auction_for_debugging_only.md document to include several clarifications and additions related to the forDebuggingOnly API within the Protected Audience framework. These updates cover topics such as supported placeholders, device-orchestrated multi-seller auctions, downsampling, and size limitations for debug URLs. The goal is to provide a more comprehensive and up-to-date guide for ad techs using the Bidding and Auction Services for debugging purposes.
Highlights
- Placeholder Support: The pull request adds
rejectReasonas a supported placeholder for fDO URLs, allowing ad techs to understand why a bid was rejected. - Multi-Seller Auctions: Sequence diagrams are added to illustrate the flow for device-orchestrated multi-seller auctions, clarifying how debug reporting works in this context.
- Downsampling Clarification: The pull request clarifies the downsampling flags and their impact on fDO URL pings, including the conditions under which URLs are dropped due to cooldown or lockout.
- Size Limit Clarification: The pull request clarifies that
MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KBonly applies to URLs sent over the network between B&A services, not to the total size of all debug URLs generated.
Changelog
Click here to see the changelog
- bidding_auction_for_debugging_only.md
- Added Prakhar Mittal as an author.
- Updated the status table to use the correct capitalization for 'device-orchestrated multi-seller auctions'.
- Changed the note about the fDO API to be bolded.
- Replaced 'adtech' with 'the ad tech' for better readability.
- Added
rejectReasonto the list of allowed placeholder keywords. - Updated the detailed flow for DSP and SSP to clarify how fDO URLs are handled in single-seller and device-orchestrated multi-seller auctions.
- Added sequence diagrams for single-seller and device-orchestrated multi-seller auctions.
- Clarified the meaning of the
enable_sampled_debug_reportingandin_cooldown_or_lockoutfields inProtectedAuctionInputrelated to downsampling. - Clarified that
MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KBapplies to URLs sent between B&A services. - Updated the recommendation for
MAX_ALLOWED_SIZE_DEBUG_URL_BYTESto 64 KB andMAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KBto 3 MB.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A doc's gentle update,
Brings clarity to the code's gate,
Debug paths now clear.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request updates the bidding_auction_for_debugging_only.md file to include new features and clarifications related to the forDebuggingOnly API within the Protected Audience framework. The changes include adding a new placeholder, sequence diagrams, clarifying downsampling flags, and specifying the scope of the MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB parameter. Overall, the changes seem well-structured and contribute to a better understanding of the debugging API.
Summary of Findings
- Inconsistent terminology: The document uses both 'adtech' and 'ad tech'. Consistency in terminology improves readability and maintainability. It's recommended to choose one and stick with it throughout the document.
- Clarity on SFE functionality: The description of SFE's role in placeholder replacement and URL pinging could be more explicit, especially regarding the distinction between single-seller and device-orchestrated multi-seller auctions.
- Downsampling explanation: The explanation of downsampling could benefit from a clearer distinction between the enable_sampled_debug_reporting and in_cooldown_or_lockout flags, and their respective impacts on fDO URL handling.
Merge Readiness
The pull request introduces valuable updates and clarifications to the documentation. However, addressing the identified inconsistencies and ambiguities would further enhance the document's quality and clarity. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issue should be addressed before merging.
| * Buyer can set the [MAX_ALLOWED_SIZE_DEBUG_URL_BYTES and MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB](https://github.com/privacysandbox/bidding-auction-servers/blob/722e1542c262dddc3aaf41be7b6c159a38cefd0a/production/deploy/gcp/terraform/environment/demo/buyer/buyer.tf#L125C1-L127C1) in the Terraform configuration for Bidding service. | ||
| * Seller can set the[ MAX_ALLOWED_SIZE_DEBUG_URL_BYTES and MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB ](https://github.com/privacysandbox/bidding-auction-servers/blob/722e1542c262dddc3aaf41be7b6c159a38cefd0a/production/deploy/gcp/terraform/environment/demo/seller/seller.tf#L126C1-L127C5) in the Terraform configuration for Auction Service. | ||
|
|
||
| Any fDO URL exceeding `MAX_ALLOWED_SIZE_DEBUG_URL_BYTES` will be dropped. Once the cumulative size of the fDO URLs to be sent from one B&A service to another exceeds `MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB`, the remaining fDO URLs will be dropped and not sent. |
There was a problem hiding this comment.
This sentence could be clarified to explicitly state that this size limit applies to the data being sent between B&A services, not to the overall size of URLs generated by ad techs.
| Any fDO URL exceeding `MAX_ALLOWED_SIZE_DEBUG_URL_BYTES` will be dropped. Once the cumulative size of the fDO URLs to be sent from one B&A service to another exceeds `MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB`, the remaining fDO URLs will be dropped and not sent. | |
| Any fDO URL exceeding `MAX_ALLOWED_SIZE_DEBUG_URL_BYTES` will be dropped. Once the cumulative size of the fDO URLs to be sent from one B&A service to another exceeds `MAX_ALLOWED_SIZE_ALL_DEBUG_URLS_KB`, the remaining fDO URLs will be dropped and not sent. |