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

Adds implementation for DebugWarnings #1626

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b197e53
adds implementation for DebugWarnings
yevhenii-viktorov Dec 10, 2021
cdfac74
New error, warning, debug handling mechanism
yevhenii-viktorov Dec 29, 2021
c5da9f0
add new error, warning, debug handling mechanism
yevhenii-viktorov Dec 29, 2021
251489d
Merge remote-tracking branch 'Prebid/refactoring/debug-warnings' into…
yevhenii-viktorov Dec 29, 2021
c6b3f42
make tests compile
yevhenii-viktorov Dec 30, 2021
d3c484d
fix some integration tests
yevhenii-viktorov Jan 4, 2022
99ad7ee
fix tests and minor refactoring
yevhenii-viktorov Dec 30, 2021
0f1e1e1
Merge remote-tracking branch 'Prebid/refactoring/debug-warnings' into…
yevhenii-viktorov Jan 5, 2022
b4745b0
Merge remote-tracking branch 'Prebid/master' into refactoring/debug-w…
yevhenii-viktorov Jan 5, 2022
e4050e5
fix master merge
yevhenii-viktorov Jan 5, 2022
e99b993
fix flaky test
yevhenii-viktorov Jan 5, 2022
94c8927
change warnings and errors logic
yevhenii-viktorov Jan 18, 2022
c483d8c
pass audit now
yevhenii-viktorov Jan 18, 2022
7de333d
pass audit now for sure
yevhenii-viktorov Jan 18, 2022
5bdd4e9
Merge remote-tracking branch 'Prebid/master' into refactoring/debug-w…
yevhenii-viktorov Jan 18, 2022
8086d9b
merge with master
yevhenii-viktorov Jan 18, 2022
7ec600f
fix CR messages
yevhenii-viktorov Jan 19, 2022
b92ae2b
resolve another comment from CR
yevhenii-viktorov Jan 19, 2022
0b4fbf3
adds warnings to warnings pool
yevhenii-viktorov Jan 20, 2022
322fec6
Update tests for debug errors, warnings (#1695)
mtuchkova Jan 21, 2022
c4ef3e2
remove DebugLog, fix CR comments
yevhenii-viktorov Jan 21, 2022
b55a9cc
Merge remote-tracking branch 'Prebid/refactoring/debug-warnings' into…
yevhenii-viktorov Jan 21, 2022
be1315f
change error code in CcpaSpec (#1699)
yevhenii-viktorov Jan 21, 2022
9ba7262
change another error code in CcpaSpec and CR comments fix
yevhenii-viktorov Jan 21, 2022
2b9b402
change another error code in CcpaSpec
yevhenii-viktorov Jan 21, 2022
8d062b3
rework how PBS working with warnings/errors
yevhenii-viktorov Feb 2, 2022
659dc98
Merge remote-tracking branch 'Prebid/master' into refactoring/debug-w…
yevhenii-viktorov Feb 2, 2022
713f3cc
Merge remote-tracking branch 'Prebid/master' into refactoring/debug-w…
yevhenii-viktorov Apr 22, 2022
e4f9f8a
merge master, adjust new logic to work with prebidlog additions
yevhenii-viktorov Apr 26, 2022
9a472d0
Return a deleted import
Apr 27, 2022
47a6bf8
Merge remote-tracking branch 'Prebid/master' into refactoring/debug-w…
yevhenii-viktorov Apr 27, 2022
b31d19a
Merge remote-tracking branch 'Prebid/refactoring/debug-warnings' into…
yevhenii-viktorov Apr 27, 2022
16a2734
pass code audit
yevhenii-viktorov Apr 27, 2022
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
32 changes: 17 additions & 15 deletions src/main/java/org/prebid/server/auction/BidResponseCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.prebid.server.auction.model.CategoryMappingResult;
import org.prebid.server.auction.model.DebugContext;
import org.prebid.server.auction.model.MultiBidConfig;
import org.prebid.server.auction.model.PrebidLog;
import org.prebid.server.auction.model.PrebidMessage;
import org.prebid.server.auction.model.TargetingInfo;
import org.prebid.server.bidder.BidderCatalog;
import org.prebid.server.bidder.model.BidderBid;
Expand Down Expand Up @@ -247,7 +249,7 @@ private Bid updateBid(Bid bid,
String lineItemId) {

final Account account = auctionContext.getAccount();
final List<String> debugWarnings = auctionContext.getDebugWarnings();
final PrebidLog prebidLog = auctionContext.getPrebidLog();

final String generatedBidId = bidIdGenerator.getType() != IdGeneratorType.none
? bidIdGenerator.generateId()
Expand All @@ -261,7 +263,7 @@ private Bid updateBid(Bid bid,
account,
eventsContext,
effectiveBidId,
debugWarnings,
prebidLog,
lineItemId))
.ext(updateBidExt(
bid,
Expand All @@ -282,7 +284,7 @@ private String updateBidAdm(Bid bid,
Account account,
EventsContext eventsContext,
String effectiveBidId,
List<String> debugWarnings,
PrebidLog prebidLog,
String lineItemId) {

final String bidAdm = bid.getAdm();
Expand All @@ -294,7 +296,7 @@ private String updateBidAdm(Bid bid,
effectiveBidId,
account.getId(),
eventsContext,
debugWarnings,
prebidLog,
lineItemId)
: bidAdm;
}
Expand Down Expand Up @@ -467,8 +469,7 @@ private Future<CategoryMappingResult> createCategoryMapping(AuctionContext aucti
private static CategoryMappingResult addCategoryMappingErrors(CategoryMappingResult categoryMappingResult,
AuctionContext auctionContext) {

auctionContext.getPrebidErrors()
.addAll(CollectionUtils.emptyIfNull(categoryMappingResult.getErrors()));
auctionContext.getPrebidLog().merge(categoryMappingResult.getPrebidLog());

return categoryMappingResult;
}
Expand Down Expand Up @@ -770,8 +771,9 @@ private static ExtResponseDebug toExtResponseDebug(List<BidderResponseInfo> bidd

final BidRequest bidRequest = debugEnabled ? auctionContext.getBidRequest() : null;

final ExtDebugPgmetrics extDebugPgmetrics = debugEnabled ? toExtDebugPgmetrics(
auctionContext.getTxnLog()) : null;
final ExtDebugPgmetrics extDebugPgmetrics = debugEnabled
? toExtDebugPgmetrics(auctionContext.getTxnLog())
: null;
final ExtDebugTrace extDebugTrace = deepDebugLog.isDeepDebugEnabled() ? toExtDebugTrace(deepDebugLog) : null;

return ObjectUtils.anyNotNull(httpCalls, bidRequest, extDebugPgmetrics, extDebugTrace)
Expand Down Expand Up @@ -974,7 +976,8 @@ private static Map<String, List<ExtBidderError>> extractPrebidErrors(VideoStored
AuctionContext auctionContext) {

final List<ExtBidderError> storedErrors = extractStoredErrors(videoStoredDataResult);
final List<ExtBidderError> contextErrors = extractContextErrors(auctionContext);
final List<ExtBidderError> contextErrors =
toBidderErrors(auctionContext.getPrebidLog().error().getAllMessages());
if (storedErrors.isEmpty() && contextErrors.isEmpty()) {
return Collections.emptyMap();
}
Expand All @@ -1000,9 +1003,9 @@ private static List<ExtBidderError> extractStoredErrors(VideoStoredDataResult vi
/**
* Returns a list of {@link ExtBidderError}s of auction context prebid errors.
*/
yevhenii-viktorov marked this conversation as resolved.
Show resolved Hide resolved
private static List<ExtBidderError> extractContextErrors(AuctionContext auctionContext) {
return auctionContext.getPrebidErrors().stream()
.map(message -> ExtBidderError.of(BidderError.Type.generic.getCode(), message))
private static List<ExtBidderError> toBidderErrors(List<PrebidMessage> prebidMessages) {
return prebidMessages.stream()
.map(prebidMessage -> ExtBidderError.of(prebidMessage.getCode(), prebidMessage.getMessage()))
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -1042,9 +1045,8 @@ private static Map<String, List<ExtBidderError>> toExtBidderWarnings(AuctionCont
}

private static Map<String, List<ExtBidderError>> extractContextWarnings(AuctionContext auctionContext) {
final List<ExtBidderError> contextWarnings = auctionContext.getDebugWarnings().stream()
.map(message -> ExtBidderError.of(BidderError.Type.generic.getCode(), message))
.collect(Collectors.toList());
final List<ExtBidderError> contextWarnings =
toBidderErrors(auctionContext.getPrebidLog().warning().getAllMessages());

return contextWarnings.isEmpty()
? Collections.emptyMap()
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/org/prebid/server/auction/DebugResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ private boolean isDebugEnabled(AuctionContext auctionContext) {
final boolean debugAllowedByAccount = isDebugAllowedByAccount(auctionContext.getAccount());

if (debugEnabledForRequest && !debugOverride && !debugAllowedByAccount) {
auctionContext.getDebugWarnings()
.add("Debug turned off for account");
auctionContext.getPrebidLog().warning().accountLevelDebugDisabled("Debug turned off for account");
}

return debugOverride || (debugEnabledForRequest && debugAllowedByAccount);
Expand Down Expand Up @@ -82,8 +81,8 @@ public boolean resolveDebugForBidder(AuctionContext auctionContext, String bidde
final boolean debugAllowedByBidder = bidderCatalog.isDebugAllowed(bidder);

if (debugEnabled && !debugOverride && !debugAllowedByBidder) {
auctionContext.getDebugWarnings()
.add(String.format("Debug turned off for bidder: %s", bidder));
auctionContext.getPrebidLog().warning().bidderLevelDebugDisabled(
String.format("Debug turned off for bidder: %s", bidder));
}

return debugOverride || (debugEnabled && debugAllowedByBidder);
Expand Down
50 changes: 27 additions & 23 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.prebid.server.auction.model.BidderRequest;
import org.prebid.server.auction.model.BidderResponse;
import org.prebid.server.auction.model.MultiBidConfig;
import org.prebid.server.auction.model.PrebidLog;
import org.prebid.server.auction.model.StoredResponseResult;
import org.prebid.server.auction.model.Tuple2;
import org.prebid.server.bidder.Bidder;
Expand Down Expand Up @@ -237,14 +238,14 @@ private Future<AuctionContext> runAuction(AuctionContext receivedContext) {
final BidRequest bidRequest = receivedContext.getBidRequest();
final Timeout timeout = receivedContext.getTimeout();
final Account account = receivedContext.getAccount();
final List<String> debugWarnings = receivedContext.getDebugWarnings();
final PrebidLog prebidLog = receivedContext.getPrebidLog();
final MetricName requestTypeMetric = receivedContext.getRequestTypeMetric();

final List<SeatBid> storedAuctionResponses = new ArrayList<>();
final BidderAliases aliases = aliases(bidRequest);
final String publisherId = account.getId();
final BidRequestCacheInfo cacheInfo = bidRequestCacheInfo(bidRequest);
final Map<String, MultiBidConfig> bidderToMultiBid = bidderToMultiBids(bidRequest, debugWarnings);
final Map<String, MultiBidConfig> bidderToMultiBid = bidderToMultiBids(bidRequest, prebidLog);

return storedResponseProcessor.getStoredResponseResult(bidRequest.getImp(), timeout)
.map(storedResponseResult -> populateStoredResponse(storedResponseResult, storedAuctionResponses))
Expand All @@ -268,7 +269,7 @@ private Future<AuctionContext> runAuction(AuctionContext receivedContext) {

.map(auctionParticipations -> storedResponseProcessor.mergeWithBidderResponses(
auctionParticipations, storedAuctionResponses, bidRequest.getImp()))
.map(auctionParticipations -> dropZeroNonDealBids(auctionParticipations, debugWarnings))
.map(auctionParticipations -> dropZeroNonDealBids(auctionParticipations, prebidLog))
.map(auctionParticipations -> validateAndAdjustBids(auctionParticipations, receivedContext, aliases))
.map(auctionParticipations -> updateMetricsFromResponses(auctionParticipations, publisherId, aliases))

Expand Down Expand Up @@ -352,7 +353,8 @@ private static ExtRequestPrebid extRequestPrebid(BidRequest bidRequest) {
return requestExt != null ? requestExt.getPrebid() : null;
}

private static Map<String, MultiBidConfig> bidderToMultiBids(BidRequest bidRequest, List<String> debugWarnings) {
private static Map<String, MultiBidConfig> bidderToMultiBids(BidRequest bidRequest,
PrebidLog prebidLog) {
final ExtRequestPrebid extRequestPrebid = extRequestPrebid(bidRequest);
final Collection<ExtRequestPrebidMultiBid> multiBids = extRequestPrebid != null
? CollectionUtils.emptyIfNull(extRequestPrebid.getMultibid())
Expand All @@ -366,25 +368,26 @@ private static Map<String, MultiBidConfig> bidderToMultiBids(BidRequest bidReque
final String codePrefix = prebidMultiBid.getTargetBidderCodePrefix();

if (bidder != null && CollectionUtils.isNotEmpty(bidders)) {
debugWarnings.add(String.format("Invalid MultiBid: bidder %s and bidders %s specified. "
prebidLog.warning().multibid(String.format("Invalid MultiBid: bidder %s and bidders %s specified. "
+ "Only bidder %s will be used.", bidder, bidders, bidder));

tryAddBidderWithMultiBid(bidder, maxBids, codePrefix, bidderToMultiBid, debugWarnings);
tryAddBidderWithMultiBid(bidder, maxBids, codePrefix, bidderToMultiBid, prebidLog);
continue;
}

if (bidder != null) {
tryAddBidderWithMultiBid(bidder, maxBids, codePrefix, bidderToMultiBid, debugWarnings);
tryAddBidderWithMultiBid(bidder, maxBids, codePrefix, bidderToMultiBid, prebidLog);
} else if (CollectionUtils.isNotEmpty(bidders)) {
if (codePrefix != null) {
debugWarnings.add(String.format("Invalid MultiBid: CodePrefix %s that was specified for bidders %s "
prebidLog.warning().multibid(String.format("Invalid MultiBid: CodePrefix %s "
+ "that was specified for bidders %s "
+ "will be skipped.", codePrefix, bidders));
}

bidders.forEach(currentBidder ->
tryAddBidderWithMultiBid(currentBidder, maxBids, null, bidderToMultiBid, debugWarnings));
tryAddBidderWithMultiBid(currentBidder, maxBids, null, bidderToMultiBid, prebidLog));
} else {
debugWarnings.add("Invalid MultiBid: Bidder and bidders was not specified.");
prebidLog.warning().multibid("Invalid MultiBid: Bidder and bidders was not specified.");
}
}

Expand All @@ -395,14 +398,15 @@ private static void tryAddBidderWithMultiBid(String bidder,
Integer maxBids,
String codePrefix,
Map<String, MultiBidConfig> bidderToMultiBid,
List<String> debugWarnings) {
PrebidLog prebidLog) {
if (bidderToMultiBid.containsKey(bidder)) {
debugWarnings.add(String.format("Invalid MultiBid: Bidder %s specified multiple times.", bidder));
prebidLog.warning().multibid(
String.format("Invalid MultiBid: Bidder %s specified multiple times.", bidder));
return;
}

if (maxBids == null) {
debugWarnings.add(String.format("Invalid MultiBid: MaxBids for bidder %s is not specified and "
prebidLog.warning().multibid(String.format("Invalid MultiBid: MaxBids for bidder %s is not specified and "
+ "will be skipped.", bidder));
return;
}
Expand Down Expand Up @@ -532,7 +536,7 @@ private Future<List<AuctionParticipation>> makeAuctionParticipation(
.mask(context, bidderToUser, bidders, aliases)
.map(bidderToPrivacyResult ->
getAuctionParticipation(bidderToPrivacyResult, bidRequest, impBidderToStoredResponse, imps,
bidderToMultiBid, biddersToConfigs, aliases, context.getDebugWarnings()));
bidderToMultiBid, biddersToConfigs, aliases, context.getPrebidLog()));
}

private Map<String, ExtBidderConfigOrtb> getBiddersToConfigs(ExtRequestPrebid prebid) {
Expand Down Expand Up @@ -741,7 +745,7 @@ private List<AuctionParticipation> getAuctionParticipation(
Map<String, MultiBidConfig> bidderToMultiBid,
Map<String, ExtBidderConfigOrtb> biddersToConfigs,
BidderAliases aliases,
List<String> debugWarnings) {
PrebidLog prebidLog) {

final Map<String, JsonNode> bidderToPrebidBidders = bidderToPrebidBidders(bidRequest);

Expand All @@ -758,7 +762,7 @@ private List<AuctionParticipation> getAuctionParticipation(
biddersToConfigs,
bidderToPrebidBidders,
aliases,
debugWarnings))
prebidLog))
// Can't be removed after we prepare workflow to filter blocked
.filter(auctionParticipation -> !auctionParticipation.isRequestBlocked())
.collect(Collectors.toList());
Expand Down Expand Up @@ -799,7 +803,7 @@ private AuctionParticipation createAuctionParticipation(
Map<String, ExtBidderConfigOrtb> biddersToConfigs,
Map<String, JsonNode> bidderToPrebidBidders,
BidderAliases bidderAliases,
List<String> debugWarnings) {
PrebidLog prebidLog) {

final boolean blockedRequestByTcf = bidderPrivacyResult.isBlockedRequestByTcf();
final boolean blockedAnalyticsByTcf = bidderPrivacyResult.isBlockedAnalyticsByTcf();
Expand All @@ -821,7 +825,7 @@ private AuctionParticipation createAuctionParticipation(
final App app = bidRequest.getApp();
final Site site = bidRequest.getSite();
if (app != null && site != null) {
debugWarnings.add("BidRequest contains app and site. Removed site object");
prebidLog.warning().bidRequestContainsAppAndSite("BidRequest contains app and site. Removed site object");
}
final Site resolvedSite = app == null ? site : null;

Expand Down Expand Up @@ -1211,15 +1215,15 @@ private BidderResponse rejectBidderResponseOrProceed(HookStageExecutionResult<Bi
}

private List<AuctionParticipation> dropZeroNonDealBids(List<AuctionParticipation> auctionParticipations,
List<String> debugWarnings) {
PrebidLog prebidLog) {

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

private AuctionParticipation dropZeroNonDealBids(AuctionParticipation auctionParticipation,
List<String> debugWarnings) {
PrebidLog prebidLog) {
final BidderResponse bidderResponse = auctionParticipation.getBidderResponse();
final BidderSeatBid seatBid = bidderResponse.getSeatBid();
final List<BidderBid> bidderBids = seatBid.getBids();
Expand All @@ -1228,10 +1232,10 @@ private AuctionParticipation dropZeroNonDealBids(AuctionParticipation auctionPar
for (BidderBid bidderBid : bidderBids) {
final Bid bid = bidderBid.getBid();
if (isZeroNonDealBids(bid.getPrice(), bid.getDealid())) {
metrics.updateAdapterRequestErrorMetric(bidderResponse.getBidder(), MetricName.unknown_error);
debugWarnings.add(String.format(
prebidLog.warning().invalidBidPrice(String.format(
"Dropped bid '%s'. Does not contain a positive (or zero if there is a deal) 'price'",
bid.getId()));
metrics.updateAdapterRequestErrorMetric(bidderResponse.getBidder(), MetricName.unknown_error);
} else {
validBids.add(bidderBid);
}
Expand Down
Loading