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

Add Zero Non Deal Bids Warning Only in Debug #3522

Merged
merged 2 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ private Future<AuctionContext> runAuction(AuctionContext receivedContext) {
storedAuctionResponses,
bidRequest.getImp(),
context.getBidRejectionTrackers()))
.map(auctionParticipations -> dropZeroNonDealBids(auctionParticipations, debugWarnings))
.map(auctionParticipations -> dropZeroNonDealBids(
auctionParticipations, debugWarnings, context.getDebugContext().isDebugEnabled()))
.map(auctionParticipations ->
bidsAdjuster.validateAndAdjustBids(auctionParticipations, context, aliases))
.map(auctionParticipations -> updateResponsesMetrics(auctionParticipations, account, aliases))
Expand Down Expand Up @@ -1269,15 +1270,18 @@ private BidderResponse rejectBidderResponseOrProceed(HookStageExecutionResult<Bi
}

private List<AuctionParticipation> dropZeroNonDealBids(List<AuctionParticipation> auctionParticipations,
List<String> debugWarnings) {
List<String> debugWarnings,
boolean isDebugEnabled) {

return auctionParticipations.stream()
.map(auctionParticipation -> dropZeroNonDealBids(auctionParticipation, debugWarnings))
.map(auctionParticipation -> dropZeroNonDealBids(auctionParticipation, debugWarnings, isDebugEnabled))
.toList();
}

private AuctionParticipation dropZeroNonDealBids(AuctionParticipation auctionParticipation,
List<String> debugWarnings) {
List<String> debugWarnings,
boolean isDebugEnabled) {

final BidderResponse bidderResponse = auctionParticipation.getBidderResponse();
final BidderSeatBid seatBid = bidderResponse.getSeatBid();
final List<BidderBid> bidderBids = seatBid.getBids();
Expand All @@ -1287,8 +1291,11 @@ private AuctionParticipation dropZeroNonDealBids(AuctionParticipation auctionPar
final Bid bid = bidderBid.getBid();
if (isZeroNonDealBids(bid.getPrice(), bid.getDealid())) {
metrics.updateAdapterRequestErrorMetric(bidderResponse.getBidder(), MetricName.unknown_error);
debugWarnings.add("Dropped bid '%s'. Does not contain a positive (or zero if there is a deal) 'price'"
.formatted(bid.getId()));
if (isDebugEnabled) {
debugWarnings.add(
"Dropped bid '%s'. Does not contain a positive (or zero if there is a deal) 'price'"
.formatted(bid.getId()));
}
} else {
validBids.add(bidderBid);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import groovy.transform.EqualsAndHashCode
import groovy.transform.ToString
import org.prebid.server.functional.model.Currency

import static org.prebid.server.functional.model.request.auction.DebugCondition.ENABLED
import static org.prebid.server.functional.model.request.auction.DistributionChannel.APP
import static org.prebid.server.functional.model.request.auction.DistributionChannel.DOOH
import static org.prebid.server.functional.model.request.auction.DistributionChannel.SITE
Expand All @@ -22,7 +23,7 @@ class BidRequest {
Dooh dooh
Device device
User user
Integer test
DebugCondition test
Integer at
Long tmax
List<String> wseat
Expand Down Expand Up @@ -63,7 +64,7 @@ class BidRequest {
regs = Regs.defaultRegs
id = UUID.randomUUID()
tmax = 2500
ext = new BidRequestExt(prebid: new Prebid(debug: 1))
ext = new BidRequestExt(prebid: new Prebid(debug: ENABLED))
if (channel == SITE) {
site = Site.defaultSite
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.prebid.server.functional.model.request.auction

import com.fasterxml.jackson.annotation.JsonValue

enum DebugCondition {

DISABLED(0), ENABLED(1)

@JsonValue
final int value

private DebugCondition(int value) {
this.value = value
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.prebid.server.functional.model.bidder.BidderName
@ToString(includeNames = true, ignoreNulls = true)
class Prebid {

Integer debug
DebugCondition debug
Boolean returnAllBidStatus
Map<String, BidderName> aliases
Map<String, Integer> aliasgvlids
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import spock.lang.PendingFeature
import java.time.Instant

import static org.prebid.server.functional.model.bidder.BidderName.GENERIC
import static org.prebid.server.functional.model.request.auction.DebugCondition.DISABLED
import static org.prebid.server.functional.model.request.auction.DebugCondition.ENABLED
import static org.prebid.server.functional.model.request.auction.DistributionChannel.DOOH
import static org.prebid.server.functional.util.HttpUtil.REFERER_HEADER

Expand Down Expand Up @@ -133,7 +135,7 @@ class BidValidationSpec extends BaseSpec {
dooh.id = null
dooh.venueType = null
}
bidDoohRequest.ext.prebid.debug = 1
bidDoohRequest.ext.prebid.debug = ENABLED

when: "PBS processes auction request"
defaultPbsService.sendAuctionRequest(bidDoohRequest)
Expand All @@ -148,7 +150,7 @@ class BidValidationSpec extends BaseSpec {
given: "Default basic BidRequest"
def bidRequest = BidRequest.defaultBidRequest
bidRequest.site = new Site(id: null, name: PBSUtils.randomString, page: null)
bidRequest.ext.prebid.debug = 1
bidRequest.ext.prebid.debug = ENABLED

when: "PBS processes auction request"
defaultPbsService.sendAuctionRequest(bidRequest)
Expand All @@ -159,9 +161,9 @@ class BidValidationSpec extends BaseSpec {
}

def "PBS should treat bids with 0 price as valid when deal id is present"() {
given: "Default basic BidRequest with generic bidder"
given: "Default basic BidRequest with generic bidder and enabled debug"
def bidRequest = BidRequest.defaultBidRequest
bidRequest.ext.prebid.debug = 1
bidRequest.ext.prebid.debug = ENABLED

and: "Bid response with 0 price bid"
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
Expand All @@ -183,16 +185,21 @@ class BidValidationSpec extends BaseSpec {
}

def "PBS should drop invalid bid and emit debug error when bid price is #bidPrice and deal id is #dealId"() {
given: "Default basic BidRequest with generic bidder"
def bidRequest = BidRequest.defaultBidRequest
bidRequest.ext.prebid.debug = 1
given: "Default basic BidRequest with generic bidder and enabled debug"
def bidRequest = BidRequest.defaultBidRequest.tap {
it.ext.prebid.debug = debug
it.test = test
}

and: "Bid response"
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
def bid = bidResponse.seatbid.first().bid.first()
bid.dealid = dealId
bid.price = bidPrice
def bidId = bid.id
def bidId = PBSUtils.randomString
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest).tap {
it.seatbid.first.bid.first.tap {
id = bidId
dealid = dealId
price = bidPrice
}
}

and: "Set bidder response"
bidder.setResponse(bidRequest.id, bidResponse)
Expand All @@ -201,13 +208,61 @@ class BidValidationSpec extends BaseSpec {
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Invalid bid should be deleted"
assert response.seatbid.size() == 0
assert !response.seatbid
assert !response.ext.seatnonbid

and: "PBS should emit an error"
assert response.ext?.warnings[ErrorType.PREBID]*.code == [999]
assert response.ext?.warnings[ErrorType.PREBID]*.message ==
["Dropped bid '$bidId'. Does not contain a positive (or zero if there is a deal) 'price'" as String]

where:
debug | test | bidPrice | dealId
DISABLED | ENABLED | PBSUtils.randomNegativeNumber | null
DISABLED | ENABLED | PBSUtils.randomNegativeNumber | PBSUtils.randomNumber
DISABLED | ENABLED | 0 | null
DISABLED | ENABLED | null | PBSUtils.randomNumber
DISABLED | ENABLED | null | null
ENABLED | DISABLED | PBSUtils.randomNegativeNumber | null
ENABLED | DISABLED | PBSUtils.randomNegativeNumber | PBSUtils.randomNumber
ENABLED | DISABLED | 0 | null
ENABLED | DISABLED | null | PBSUtils.randomNumber
ENABLED | DISABLED | null | null
}

def "PBS should drop invalid bid without debug error when request debug disabled and bid price is #bidPrice and deal id is #dealId"() {
given: "Default basic BidRequest with generic bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
test = DISABLED
ext.prebid.debug = DISABLED
}

and: "Bid response"
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest).tap {
it.seatbid.first.bid.first.tap {
dealid = dealId
price = bidPrice
}
}

and: "Set bidder response"
bidder.setResponse(bidRequest.id, bidResponse)

when: "PBS processes auction request"
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Invalid bid should be deleted"
assert !response.seatbid
assert !response.ext.seatnonbid

and: "PBS shouldn't emit an error"
assert !response.ext?.warnings
assert !response.ext?.warnings

and: "PBS should call bidder"
def bidderRequests = bidder.getBidderRequests(bidResponse.id)
assert bidderRequests.size() == 1

where:
bidPrice | dealId
PBSUtils.randomNegativeNumber | null
Expand All @@ -220,7 +275,7 @@ class BidValidationSpec extends BaseSpec {
def "PBS should only drop invalid bid without discarding whole seat"() {
given: "Default basic BidRequest with generic bidder"
def bidRequest = BidRequest.defaultBidRequest
bidRequest.ext.prebid.debug = 1
bidRequest.ext.prebid.debug = ENABLED
bidRequest.ext.prebid.multibid = [new MultiBid(bidder: GENERIC, maxBids: 2)]

and: "Bid response with 2 bids"
Expand All @@ -239,14 +294,61 @@ class BidValidationSpec extends BaseSpec {
when: "PBS processes auction request"
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Invalid bids should be deleted"
then: "Bid response contains only valid bid"
assert response.seatbid?.first()?.bid*.id == [validBidId]

and: "PBS should emit an error"
assert response.ext?.warnings[ErrorType.PREBID]*.code == [999]
assert response.ext?.warnings[ErrorType.PREBID]*.message ==
["Dropped bid '$invalidBid.id'. Does not contain a positive (or zero if there is a deal) 'price'" as String]

where:
debug | test | bidPrice | dealId
0 | 1 | PBSUtils.randomNegativeNumber | null
0 | 1 | PBSUtils.randomNegativeNumber | PBSUtils.randomNumber
0 | 1 | 0 | null
0 | 1 | null | PBSUtils.randomNumber
0 | 1 | null | null
1 | 0 | PBSUtils.randomNegativeNumber | null
1 | 0 | PBSUtils.randomNegativeNumber | PBSUtils.randomNumber
1 | 0 | 0 | null
1 | 0 | null | PBSUtils.randomNumber
1 | 0 | null | null
}

def "PBS should only drop invalid bid without discarding whole seat without debug error when request debug disabled "() {
given: "Default basic BidRequest with generic bidder"
def bidRequest = BidRequest.defaultBidRequest.tap {
test = DISABLED
ext.prebid.tap {
debug = DISABLED
multibid = [new MultiBid(bidder: GENERIC, maxBids: 2)]
}
}

and: "Bid response with 2 bids"
def bidResponse = BidResponse.getDefaultBidResponse(bidRequest)
bidResponse.seatbid[0].bid << Bid.getDefaultBid(bidRequest.imp.first())

and: "One of the bids is invalid"
def invalidBid = bidResponse.seatbid.first().bid.first()
invalidBid.dealid = dealId
invalidBid.price = bidPrice
def validBidId = bidResponse.seatbid.first().bid.last().id

and: "Set bidder response"
bidder.setResponse(bidRequest.id, bidResponse)

when: "PBS processes auction request"
def response = defaultPbsService.sendAuctionRequest(bidRequest)

then: "Bid response contains only valid bid"
assert response.seatbid?.first()?.bid*.id == [validBidId]

and: "PBS shouldn't emit an error"
assert !response.ext?.warnings
assert !response.ext?.warnings

where:
bidPrice | dealId
PBSUtils.randomNegativeNumber | null
Expand All @@ -257,10 +359,7 @@ class BidValidationSpec extends BaseSpec {
}

def "PBS should update 'adapter.generic.requests.bid_validation' metric when bid validation error appears"() {
given: "Initial 'adapter.generic.requests.bid_validation' metric value"
def initialMetricValue = getCurrentMetricValue(defaultPbsService, "adapter.generic.requests.bid_validation")

and: "Bid request"
given: "Bid request"
def bidRequest = BidRequest.defaultBidRequest

and: "Set invalid bid response"
Expand Down
Loading
Loading