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 bid size and secure markup validations, controlled by host through configuration #830

Merged
merged 22 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
17ef672
Add bid size and secure markup validations, controlled by host throug…
Jul 27, 2020
e17bff5
Add account-level bid validations overrides
Jul 28, 2020
74753f3
Apply account level overrides for bid validations
Jul 28, 2020
fa64a7c
Represent sizes list as string
Jul 28, 2020
5c5a46e
Update validation error metrics
Jul 28, 2020
390b11f
Include new configuration properties in documentation
Sep 1, 2020
366cdcd
Merge branch 'master' into bid-attributes-validation
Sep 1, 2020
8518608
Fix broken test
Sep 1, 2020
ba2f9bb
Make use of BidderAliases to correctly resolve aliases on metrics rep…
Sep 1, 2020
db58124
Change validation logic for banners: validate max size if enforced by…
Sep 2, 2020
c5e3a2c
Change validation logic for secure creative: look for secure markers …
Sep 2, 2020
5b49449
Add warnings support for bids validation
Sep 2, 2020
f73ab5d
Merge branch 'master' into bid-attributes-validation
Sep 2, 2020
bb744cd
Fix compilation error after merge
Sep 2, 2020
e0769e7
Update validation metrics
Oct 1, 2020
a9affdc
Merge branch 'master' into bid-attributes-validation
Oct 1, 2020
7852223
Merge branch 'master' into bid-attributes-validation
Oct 20, 2020
0f92f8f
Merge branch 'master' into bid-attributes-validation
Nov 13, 2020
abf13fb
Fix documentation and remove obsolete class
Nov 13, 2020
ab37d9b
Merge branch 'master' into bid-attributes-validation
Dec 9, 2020
7063718
Merge branch 'master' into bid-attributes-validation
Dec 14, 2020
6db8896
Merge branch 'master' into bid-attributes-validation
Dec 17, 2020
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
18 changes: 14 additions & 4 deletions docs/application-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ There are two ways to configure application settings: database and file. This do
- `truncate-target-attr` - Maximum targeting attributes size. Values between 1 and 255.
- `default-integration` - Default integration to assume.
- `analytics-config.auction-events.<channel>` - defines which channels are supported by analytics for this account
- `bid-validations.banner-creative-max-size` - Overrides creative max size validation for banners.

```
Purpose | Purpose goal | Purpose meaning for PBS (n\a - not affected)
Expand Down Expand Up @@ -187,6 +188,7 @@ Query to create accounts_account table:
`truncate_target_attr` tinyint(3) unsigned DEFAULT NULL,
`default_integration` varchar(64) DEFAULT NULL,
`analytics_config` varchar(512) DEFAULT NULL,
`bid_validations` json DEFAULT NULL,
`status` enum('active','inactive') DEFAULT 'active',
`updated_by` int(11) DEFAULT NULL,
`updated_by_user` varchar(64) DEFAULT NULL,
Expand All @@ -198,16 +200,16 @@ ENGINE=InnoDB DEFAULT CHARSET=utf8'

where tcf_config column is json with next format

```
```json
{
"enabled": true,
"integration-enabled": {
"video": true,
"web": true,
"app": true,
"amp": true
}
"purpose-one-treatment-interpretation": "ignore"
},
"purpose-one-treatment-interpretation": "ignore",
"purposes": {
"p1": {
"enforce-purpose": "full",
Expand Down Expand Up @@ -309,9 +311,17 @@ where tcf_config column is json with next format
}
```

and bid_validations column is json with next format

```json
{
"banner-creative-max-size": "enforce"
}
```

Query used to get an account:
```
SELECT uuid, price_granularity, banner_cache_ttl, video_cache_ttl, events_enabled, enforce_ccpa, tcf_config, analytics_sampling_factor, truncate_target_attr, default_integration, analytics_config
SELECT uuid, price_granularity, banner_cache_ttl, video_cache_ttl, events_enabled, enforce_ccpa, tcf_config, analytics_sampling_factor, truncate_target_attr, default_integration, analytics_config, bid_validations
FROM accounts_account where uuid = ?
LIMIT 1

Expand Down
2 changes: 2 additions & 0 deletions docs/config-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ Removes and downloads file again if depending service cant process probably corr
- `auction.cache.only-winning-bids` - if equals to `true` only the winning bids would be cached. Has lower priority than request-specific flags.
- `auction.generate-bid-id` - whether to generate seatbid[].bid[].ext.prebid.bidid in the OpenRTB response.
- `auction.id-generator-type` - if generate-bid-id is on, then this defines how the ID should be generated. Currently onlye `uuid` is supported.
- `auction.validations.banner-creative-max-size` - enables creative max size validation for banners. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.
- `auction.validations.secure-markup` - enables secure markup validation. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.

## Amp (OpenRTB)
- `amp.default-timeout-ms` - default operation timeout for OpenRTB Amp requests.
Expand Down
4 changes: 4 additions & 0 deletions docs/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,14 @@ where `[DATASOURCE]` is a data source name, `DEFAULT_DS` by defaul.
- `adapter.<bidder-name>.(openrtb2-web|openrtb-app|amp|legacy).tcf.geo_masked` - number of requests made to `<bidder-name>` that required geo information removed as a result of TCF enforcement for that bidder
- `adapter.<bidder-name>.(openrtb2-web|openrtb-app|amp|legacy).tcf.request_blocked` - number of requests made to `<bidder-name>` that were blocked as a result of TCF enforcement for that bidder
- `adapter.<bidder-name>.(openrtb2-web|openrtb-app|amp|legacy).tcf.analytics_blocked` - number of requests made to `<bidder-name>` that required analytics blocked as a result of TCF enforcement for that bidder
- `adapter.<bidder-name>.response.validation.size.(warn|err)` - number of banner bids received from the `<bidder-name>` that had invalid size
- `adapter.<bidder-name>.response.validation.secure.(warn|err)` - number of bids received from the `<bidder-name>` that had insecure creative while in secure context

## Auction per-account metrics
Following metrics are collected and submitted if account is configured with `basic` verbosity:
- `account.<account-id>.requests` - number of requests received from account with `<account-id>`
- `account.<account-id>.response.validation.size.(warn|err)` - number of banner bids received from account with `<account-id>` that had invalid size
- `account.<account-id>.response.validation.secure.(warn|err)` - number of bids received from account with `<account-id>` that had insecure creative while in secure context

Following metrics are collected and submitted if account is configured with `detailed` verbosity:
- `account.<account-id>.requests.type.(openrtb2-web,openrtb-app,amp,legacy)` - number of requests received from account with `<account-id>` broken down by type of incoming request
Expand Down
66 changes: 42 additions & 24 deletions src/main/java/org/prebid/server/auction/ExchangeService.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,19 @@ public Future<BidResponse> holdAuction(AuctionContext context) {
.compose(impsRequiredRequest -> extractBidderRequests(context, impsRequiredRequest, aliases))
.map(bidderRequests -> updateRequestMetric(
bidderRequests, uidsCookie, aliases, publisherId, context.getRequestTypeMetric()))
.compose(bidderRequests -> CompositeFuture.join(bidderRequests.stream()
.map(bidderRequest -> requestBids(
bidderRequest,
auctionTimeout(timeout, cacheInfo.isDoCaching()),
debugEnabled,
aliases))
.collect(Collectors.toList())))
.compose(bidderRequests -> CompositeFuture.join(
bidderRequests.stream()
.map(bidderRequest -> requestBids(
bidderRequest,
auctionTimeout(timeout, cacheInfo.isDoCaching()),
debugEnabled,
aliases))
.collect(Collectors.toList())))
// send all the requests to the bidders and gathers results
.map(CompositeFuture::<BidderResponse>list)
.map(bidderResponses -> storedResponseProcessor.mergeWithBidderResponses(
bidderResponses, storedResponse, bidRequest.getImp()))
.map(bidderResponses -> validateAndAdjustBids(bidRequest, bidderResponses))
.map(bidderResponses -> validateAndAdjustBids(bidderResponses, context, aliases))
.map(bidderResponses -> updateMetricsFromResponses(bidderResponses, publisherId, aliases))
// produce response from bidder results
.compose(bidderResponses -> bidResponseCreator.create(
Expand Down Expand Up @@ -811,8 +812,10 @@ private static BigDecimal bidAdjustmentForBidder(BidRequest bidRequest, String b
* Passes the request to a corresponding bidder and wraps response in {@link BidderResponse} which also holds
* recorded response time.
*/
private Future<BidderResponse> requestBids(
BidderRequest bidderRequest, Timeout timeout, boolean debugEnabled, BidderAliases aliases) {
private Future<BidderResponse> requestBids(BidderRequest bidderRequest,
Timeout timeout,
boolean debugEnabled,
BidderAliases aliases) {

final String bidderName = bidderRequest.getBidder();
final Bidder<?> bidder = bidderCatalog.bidderByName(aliases.resolveBidder(bidderName));
Expand All @@ -822,10 +825,12 @@ private Future<BidderResponse> requestBids(
.map(seatBid -> BidderResponse.of(bidderName, seatBid, responseTime(startTime)));
}

private List<BidderResponse> validateAndAdjustBids(BidRequest bidRequest, List<BidderResponse> bidderResponses) {
private List<BidderResponse> validateAndAdjustBids(
List<BidderResponse> bidderResponses, AuctionContext auctionContext, BidderAliases aliases) {

return bidderResponses.stream()
.map(bidderResponse -> validBidderResponse(bidderResponse, bidRequest.getCur()))
.map(bidderResponse -> applyBidPriceChanges(bidderResponse, bidRequest))
.map(bidderResponse -> validBidderResponse(bidderResponse, auctionContext, aliases))
.map(bidderResponse -> applyBidPriceChanges(bidderResponse, auctionContext.getBidRequest()))
.collect(Collectors.toList());
}

Expand All @@ -836,35 +841,48 @@ private List<BidderResponse> validateAndAdjustBids(BidRequest bidRequest, List<B
* <p>
* Returns input argument as the result if no errors found or creates new {@link BidderResponse} otherwise.
*/
private BidderResponse validBidderResponse(BidderResponse bidderResponse, List<String> requestCurrencies) {
final BidderSeatBid seatBid = bidderResponse.getSeatBid();
final List<BidderBid> bids = seatBid.getBids();
private BidderResponse validBidderResponse(
BidderResponse bidderResponse, AuctionContext auctionContext, BidderAliases aliases) {

final List<BidderBid> validBids = new ArrayList<>(bids.size());
final BidRequest bidRequest = auctionContext.getBidRequest();
final BidderSeatBid seatBid = bidderResponse.getSeatBid();
final List<BidderError> errors = new ArrayList<>(seatBid.getErrors());

final List<String> requestCurrencies = bidRequest.getCur();
if (requestCurrencies.size() > 1) {
errors.add(BidderError.badInput(
String.format("Cur parameter contains more than one currency. %s will be used",
requestCurrencies.get(0))));
}

for (BidderBid bid : bids) {
final ValidationResult validationResult = responseBidValidator.validate(bid);
final List<BidderBid> bids = seatBid.getBids();
final List<BidderBid> validBids = new ArrayList<>(bids.size());

for (final BidderBid bid : bids) {
final ValidationResult validationResult =
responseBidValidator.validate(bid, bidderResponse.getBidder(), auctionContext, aliases);

if (validationResult.hasWarnings()) {
addAsBidderErrors(validationResult.getWarnings(), errors);
}

if (validationResult.hasErrors()) {
for (String error : validationResult.getErrors()) {
errors.add(BidderError.generic(error));
}
} else {
validBids.add(bid);
addAsBidderErrors(validationResult.getErrors(), errors);
continue;
}

validBids.add(bid);
}

return errors.isEmpty()
? bidderResponse
: bidderResponse.with(BidderSeatBid.of(validBids, seatBid.getHttpCalls(), errors));
}

private void addAsBidderErrors(List<String> messages, List<BidderError> errors) {
messages.stream().map(BidderError::generic).forEach(errors::add);
}

/**
* Performs changes on {@link Bid}s price depends on different between adServerCurrency and bidCurrency,
* and adjustment factor. Will drop bid if currency conversion is needed but not possible.
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/prebid/server/metric/AccountMetrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class AccountMetrics extends UpdatableMetrics {
private final Map<MetricName, RequestTypeMetrics> requestTypeMetrics;
private final RequestMetrics requestsMetrics;
private final CacheMetrics cacheMetrics;
private final ResponseMetrics responseMetrics;

AccountMetrics(MetricRegistry metricRegistry, CounterType counterType, String account) {
super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
Expand All @@ -32,6 +33,7 @@ class AccountMetrics extends UpdatableMetrics {
requestTypeMetrics = new HashMap<>();
requestsMetrics = new RequestMetrics(metricRegistry, counterType, createPrefix(account));
cacheMetrics = new CacheMetrics(metricRegistry, counterType, createPrefix(account));
responseMetrics = new ResponseMetrics(metricRegistry, counterType, createPrefix(account));
}

private static String createPrefix(String account) {
Expand All @@ -57,4 +59,8 @@ RequestMetrics requests() {
CacheMetrics cache() {
return cacheMetrics;
}

ResponseMetrics response() {
return responseMetrics;
}
}
7 changes: 7 additions & 0 deletions src/main/java/org/prebid/server/metric/AdapterMetrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class AdapterMetrics extends UpdatableMetrics {
private final RequestMetrics requestMetrics;
private final Function<String, BidTypeMetrics> bidTypeMetricsCreator;
private final Map<String, BidTypeMetrics> bidTypeMetrics;
private final ResponseMetrics responseMetrics;

AdapterMetrics(MetricRegistry metricRegistry, CounterType counterType, String adapterType) {
super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
Expand All @@ -29,6 +30,7 @@ class AdapterMetrics extends UpdatableMetrics {
requestTypeMetrics = new HashMap<>();
requestMetrics = new RequestMetrics(metricRegistry, counterType, createAdapterPrefix(adapterType));
bidTypeMetrics = new HashMap<>();
responseMetrics = new ResponseMetrics(metricRegistry, counterType, createAdapterPrefix(adapterType));
}

AdapterMetrics(MetricRegistry metricRegistry, CounterType counterType, String account, String adapterType) {
Expand All @@ -44,6 +46,7 @@ class AdapterMetrics extends UpdatableMetrics {
requestTypeMetrics = null;
bidTypeMetricsCreator = null;
bidTypeMetrics = null;
responseMetrics = null;
}

private static String createAdapterPrefix(String adapterType) {
Expand All @@ -69,4 +72,8 @@ RequestMetrics request() {
BidTypeMetrics forBidType(String bidType) {
return bidTypeMetrics.computeIfAbsent(bidType, bidTypeMetricsCreator);
}

ResponseMetrics response() {
return responseMetrics;
}
}
3 changes: 3 additions & 0 deletions src/main/java/org/prebid/server/metric/MetricName.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public enum MetricName {
err,
networkerr,

// bids validation
warn,

// cookie sync
cookie_sync_requests,
opt_outs,
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/prebid/server/metric/Metrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,16 @@ public void updateAdapterRequestErrorMetric(String bidder, MetricName errorMetri
forAdapter(resolveMetricsBidderName(bidder)).request().incCounter(errorMetric);
}

public void updateSizeValidationMetrics(String bidder, String accountId, MetricName type) {
forAdapter(resolveMetricsBidderName(bidder)).response().validation().size().incCounter(type);
forAccount(accountId).response().validation().size().incCounter(type);
}

public void updateSecureValidationMetrics(String bidder, String accountId, MetricName type) {
forAdapter(resolveMetricsBidderName(bidder)).response().validation().secure().incCounter(type);
forAccount(accountId).response().validation().secure().incCounter(type);
}

public void updateUserSyncOptoutMetric() {
userSync().incCounter(MetricName.opt_outs);
}
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/org/prebid/server/metric/ResponseMetrics.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.prebid.server.metric;

import com.codahale.metrics.MetricRegistry;

import java.util.Objects;
import java.util.function.Function;

/**
* Request metrics support.
*/
class ResponseMetrics extends UpdatableMetrics {

private final ValidationMetrics validationMetrics;

ResponseMetrics(MetricRegistry metricRegistry, CounterType counterType, String prefix) {
super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
nameCreator(createPrefix(Objects.requireNonNull(prefix))));

validationMetrics = new ValidationMetrics(metricRegistry, counterType, createPrefix(prefix));
}

private static Function<MetricName, String> nameCreator(String prefix) {
return metricName -> String.format("%s.%s", prefix, metricName.toString());
}

private static String createPrefix(String prefix) {
return String.format("%s.response", prefix);
}

ValidationMetrics validation() {
return validationMetrics;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.prebid.server.metric;

import com.codahale.metrics.MetricRegistry;

import java.util.Objects;
import java.util.function.Function;

/**
* Request metrics support.
*/
class SpecificValidationMetrics extends UpdatableMetrics {

SpecificValidationMetrics(
MetricRegistry metricRegistry, CounterType counterType, String prefix, String validation) {

super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
nameCreator(createPrefix(Objects.requireNonNull(prefix), Objects.requireNonNull(validation))));
}

private static Function<MetricName, String> nameCreator(String prefix) {
return metricName -> String.format("%s.%s", prefix, metricName.toString());
}

private static String createPrefix(String prefix, String validation) {
return String.format("%s.%s", prefix, validation);
}
}
41 changes: 41 additions & 0 deletions src/main/java/org/prebid/server/metric/ValidationMetrics.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.prebid.server.metric;

import com.codahale.metrics.MetricRegistry;

import java.util.Objects;
import java.util.function.Function;

/**
* Request metrics support.
*/
class ValidationMetrics extends UpdatableMetrics {

private final SpecificValidationMetrics sizeValidationMetrics;
private final SpecificValidationMetrics secureValidationMetrics;

ValidationMetrics(MetricRegistry metricRegistry, CounterType counterType, String prefix) {
super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
nameCreator(createPrefix(Objects.requireNonNull(prefix))));

sizeValidationMetrics = new SpecificValidationMetrics(
metricRegistry, counterType, createPrefix(prefix), "size");
secureValidationMetrics = new SpecificValidationMetrics(
metricRegistry, counterType, createPrefix(prefix), "secure");
}

private static Function<MetricName, String> nameCreator(String prefix) {
return metricName -> String.format("%s.%s", prefix, metricName.toString());
}

private static String createPrefix(String prefix) {
return String.format("%s.validation", prefix);
}

SpecificValidationMetrics size() {
return sizeValidationMetrics;
}

SpecificValidationMetrics secure() {
return secureValidationMetrics;
}
}
Loading