diff --git a/docs/config-app.md b/docs/config-app.md index c037a74c482..51dd71cec1a 100644 --- a/docs/config-app.md +++ b/docs/config-app.md @@ -302,6 +302,10 @@ Preconfigured application settings can be obtained from multiple data sources co Warning! Application will not start in case of no one data source is defined and you'll get an exception in logs. +For requests validation mode available next options: +- `settings.fail-on-unknown-bidders` - fail with validation error or just make warning for unknown bidders. +- `settings.fail-on-disabled-bidders` - fail with validation error or just make warning for disabled bidders. + For filesystem data source available next options: - `settings.filesystem.settings-filename` - location of file settings. - `settings.filesystem.stored-requests-dir` - directory with stored requests. diff --git a/docs/metrics.md b/docs/metrics.md index 85df92bc269..fa540bc36e1 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -45,6 +45,8 @@ where `[DATASOURCE]` is a data source name, `DEFAULT_DS` by defaul. - `imps_video` - number of video impressions - `imps_native` - number of native impressions - `imps_audio` - number of audio impressions +- `disabled_bidder` - number of disabled bidders received within requests +- `unknown_bidder` - number of unknown bidders received within requests - `requests.(ok|badinput|err|networkerr|blocklisted_account|blocklisted_app).(openrtb2-web|openrtb-app|amp|legacy)` - number of requests broken down by status and type - `bidder-cardinality..requests` - number of requests targeting `` of bidders - `connection_accept_errors` - number of errors occurred while establishing HTTP connection @@ -92,6 +94,8 @@ Following metrics are collected and submitted if account is configured with `det - `account..requests.type.(openrtb2-web,openrtb-app,amp,legacy)` - number of requests received from account with `` broken down by type of incoming request - `account..debug_requests` - number of requests received from account with `` broken down by type of incoming request (when debug mode is enabled) - `account..requests.rejected` - number of rejected requests caused by incorrect `accountId` +- `account..requests.disabled_bidder` - number of disabled bidders received within requests from account with `` +- `account..requests.unknown_bidder` - number of unknown bidder names received within requests from account with `` - `account..adapter..request_time` - timer tracking how long did it take to make a request to `` when incoming request was from `` - `account..adapter..bids_received` - number of bids received from `` when incoming request was from `` - `account..adapter..requests.(gotbids|nobid)` - number of requests made to `` broken down by result status when incoming request was from `` diff --git a/src/main/java/org/prebid/server/metric/Metrics.java b/src/main/java/org/prebid/server/metric/Metrics.java index 4c001af4a3a..b4f3a5af9c3 100644 --- a/src/main/java/org/prebid/server/metric/Metrics.java +++ b/src/main/java/org/prebid/server/metric/Metrics.java @@ -336,19 +336,19 @@ public void updateAdapterRequestErrorMetric(String bidder, MetricName errorMetri forAdapter(bidder).request().incCounter(errorMetric); } - public void updateAdapterRequestDisabledBidderMetric(String bidder, Account account) { - forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + public void updateDisabledBidderMetric(Account account) { + incCounter(MetricName.disabled_bidder); if (accountMetricsVerbosityResolver.forAccount(account) .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { - forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.disabled_bidder); + forAccount(account.getId()).requests().incCounter(MetricName.disabled_bidder); } } - public void updateAdapterRequestUnknownBidderMetric(String bidder, Account account) { - forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + public void updateUnknownBidderMetric(Account account) { + incCounter(MetricName.unknown_bidder); if (accountMetricsVerbosityResolver.forAccount(account) .isAtLeast(AccountMetricsVerbosityLevel.detailed)) { - forAccount(account.getId()).adapter().forAdapter(bidder).request().incCounter(MetricName.unknown_bidder); + forAccount(account.getId()).requests().incCounter(MetricName.unknown_bidder); } } diff --git a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java index 24522e4cdd6..82a2d7dcee0 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -1033,7 +1033,9 @@ RequestValidator requestValidator( Metrics metrics, JacksonMapper mapper, @Value("${logging.sampling-rate:0.01}") double logSamplingRate, - @Value("${auction.strict-app-site-dooh:false}") boolean enabledStrictAppSiteDoohValidation) { + @Value("${auction.strict-app-site-dooh:false}") boolean enabledStrictAppSiteDoohValidation, + @Value("${settings.fail-on-disabled-bidders}") boolean failOnDisabledBidders, + @Value("${settings.fail-on-unknown-bidders}") boolean failOnUnknownBidders) { return new RequestValidator( bidderCatalog, @@ -1041,7 +1043,9 @@ RequestValidator requestValidator( metrics, mapper, logSamplingRate, - enabledStrictAppSiteDoohValidation); + enabledStrictAppSiteDoohValidation, + failOnDisabledBidders, + failOnUnknownBidders); } @Bean diff --git a/src/main/java/org/prebid/server/validation/RequestValidator.java b/src/main/java/org/prebid/server/validation/RequestValidator.java index 66c19983197..c810227889e 100644 --- a/src/main/java/org/prebid/server/validation/RequestValidator.java +++ b/src/main/java/org/prebid/server/validation/RequestValidator.java @@ -75,6 +75,8 @@ public class RequestValidator { private final JacksonMapper mapper; private final double logSamplingRate; private final boolean enabledStrictAppSiteDoohValidation; + private final boolean failOnDisabledBidders; + private final boolean failOnUnknownBidders; /** * Constructs a RequestValidator that will use the BidderParamValidator passed in order to validate all critical @@ -84,7 +86,9 @@ public RequestValidator(BidderCatalog bidderCatalog, ImpValidator impValidator, Metrics metrics, JacksonMapper mapper, double logSamplingRate, - boolean enabledStrictAppSiteDoohValidation) { + boolean enabledStrictAppSiteDoohValidation, + boolean failOnDisabledBidders, + boolean failOnUnknownBidders) { this.bidderCatalog = Objects.requireNonNull(bidderCatalog); this.impValidator = Objects.requireNonNull(impValidator); @@ -92,6 +96,8 @@ public RequestValidator(BidderCatalog bidderCatalog, this.mapper = Objects.requireNonNull(mapper); this.logSamplingRate = logSamplingRate; this.enabledStrictAppSiteDoohValidation = enabledStrictAppSiteDoohValidation; + this.failOnDisabledBidders = failOnDisabledBidders; + this.failOnUnknownBidders = failOnUnknownBidders; } /** @@ -514,13 +520,25 @@ private void validateAliases(Map aliases, List warnings, final String alias = aliasToBidder.getKey(); final String coreBidder = aliasToBidder.getValue(); if (!bidderCatalog.isValidName(coreBidder)) { - warnings.add( - "request.ext.prebid.aliases.%s refers to unknown bidder: %s".formatted(alias, coreBidder)); - metrics.updateAdapterRequestUnknownBidderMetric(coreBidder, account); + metrics.updateUnknownBidderMetric(account); + + final String message = String.format("request.ext.prebid.aliases.%s refers to unknown bidder: %s", + alias, coreBidder); + if (failOnUnknownBidders) { + throw new ValidationException(message); + } else { + warnings.add(message); + } } else if (!bidderCatalog.isActive(coreBidder)) { - warnings.add( - "request.ext.prebid.aliases.%s refers to disabled bidder: %s".formatted(alias, coreBidder)); - metrics.updateAdapterRequestDisabledBidderMetric(coreBidder, account); + metrics.updateDisabledBidderMetric(account); + + final String message = String.format("request.ext.prebid.aliases.%s refers to disabled bidder: %s", + alias, coreBidder); + if (failOnDisabledBidders) { + throw new ValidationException(message); + } else { + warnings.add(message); + } } if (alias.equals(coreBidder)) { diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 31efd30851b..4bd8ab75332 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -158,6 +158,8 @@ currency-converter: settings: generate-storedrequest-bidrequest-id: false enforce-valid-account: false + fail-on-unknown-bidders: true + fail-on-disabled-bidders: true database: pool-size: 20 idle-connection-timeout: 300 diff --git a/src/test/java/org/prebid/server/metric/MetricsTest.java b/src/test/java/org/prebid/server/metric/MetricsTest.java index 77e9fe0956d..f40731dcb3c 100644 --- a/src/test/java/org/prebid/server/metric/MetricsTest.java +++ b/src/test/java/org/prebid/server/metric/MetricsTest.java @@ -48,6 +48,7 @@ public class MetricsTest { private static final String RUBICON = "rubicon"; private static final String CONVERSANT = "conversant"; private static final String ACCOUNT_ID = "accountId"; + private static final String ACCOUNT_ID_1 = "accountId1"; private static final String ANALYTIC_CODE = "analyticCode"; private MetricRegistry metricRegistry; @@ -615,37 +616,34 @@ public void updateAdapterBidMetricsShouldUpdateMetrics() { } @Test - public void updateAdapterRequestUnknownBidderMetricsShouldIncrementMetrics() { + public void updateUnknownBidderMetricsShouldIncrementMetrics() { // when - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID_1)); // then - assertThat(metricRegistry.counter("adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + assertThat(metricRegistry.counter("unknown_bidder").getCount()).isEqualTo(3); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isOne(); + "account.accountId.requests.unknown_bidder").getCount()).isEqualTo(2); assertThat(metricRegistry.counter( - "adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); - assertThat(metricRegistry.counter( - "account.accountId.adapter.conversant.requests.unknown_bidder").getCount()).isEqualTo(2); + "account.accountId1.requests.unknown_bidder").getCount()).isEqualTo(1); } @Test - public void updateAdapterRequestDisabledBidderMetricsShouldIncrementMetrics() { + public void updateDisabledBidderMetricsShouldIncrementMetrics() { // when - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestDisabledBidderMetric(CONVERSANT, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID_1)); // then - assertThat(metricRegistry.counter("adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isOne(); + "disabled_bidder").getCount()).isEqualTo(3); assertThat(metricRegistry.counter( - "adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + "account.accountId.requests.disabled_bidder").getCount()).isEqualTo(2); assertThat(metricRegistry.counter( - "account.accountId.adapter.conversant.requests.disabled_bidder").getCount()).isEqualTo(2); + "account.accountId1.requests.disabled_bidder").getCount()).isEqualTo(1); } @Test @@ -999,8 +997,8 @@ public void shouldNotUpdateAccountMetricsIfVerbosityIsNone() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isZero(); @@ -1030,8 +1028,8 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsBasic() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); @@ -1061,8 +1059,8 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsDetailed() { metrics.updateAdapterRequestNobidMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterRequestGotbidsMetrics(RUBICON, Account.empty(ACCOUNT_ID)); metrics.updateAdapterBidMetrics(RUBICON, Account.empty(ACCOUNT_ID), 1234L, true, "banner"); - metrics.updateAdapterRequestDisabledBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); - metrics.updateAdapterRequestUnknownBidderMetric(RUBICON, Account.empty(ACCOUNT_ID)); + metrics.updateDisabledBidderMetric(Account.empty(ACCOUNT_ID)); + metrics.updateUnknownBidderMetric(Account.empty(ACCOUNT_ID)); // then assertThat(metricRegistry.counter("account.accountId.requests").getCount()).isOne(); @@ -1079,9 +1077,9 @@ public void shouldUpdateAccountRequestsMetricOnlyIfVerbosityIsDetailed() { assertThat(metricRegistry.counter("account.accountId.adapter.rubicon.bids_received").getCount()) .isEqualTo(1); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.unknown_bidder").getCount()).isEqualTo(1); + "unknown_bidder").getCount()).isEqualTo(1); assertThat(metricRegistry.counter( - "account.accountId.adapter.rubicon.requests.disabled_bidder").getCount()).isEqualTo(1); + "account.accountId.requests.disabled_bidder").getCount()).isEqualTo(1); } @Test diff --git a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java index b45b074717f..e2eb6a0d205 100644 --- a/src/test/java/org/prebid/server/validation/RequestValidatorTest.java +++ b/src/test/java/org/prebid/server/validation/RequestValidatorTest.java @@ -84,7 +84,7 @@ public void setUp() { given(bidderCatalog.isValidName(eq(RUBICON))).willReturn(true); given(bidderCatalog.isActive(eq(RUBICON))).willReturn(true); - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, true, true); } @Test @@ -356,7 +356,7 @@ public void validateShouldReturnValidationMessageWhenRequestAppAndRequestSiteBot @Test public void validateShouldFailWhenDoohSiteAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .app(App.builder().build()) @@ -374,7 +374,7 @@ public void validateShouldFailWhenDoohSiteAndAppArePresentInRequestAndStrictVali @Test public void validateShouldFailWhenSiteAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .app(App.builder().build()) .site(Site.builder().build()) @@ -391,7 +391,7 @@ public void validateShouldFailWhenSiteAndAppArePresentInRequestAndStrictValidati @Test public void validateShouldFailWhenDoohAndSiteArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .site(Site.builder().build()) @@ -408,7 +408,7 @@ public void validateShouldFailWhenDoohAndSiteArePresentInRequestAndStrictValidat @Test public void validateShouldFailWhenDoohAndAppArePresentInRequestAndStrictValidationIsEnabled() { // when - target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true); + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, true, true, true); final BidRequest invalidRequest = validBidRequestBuilder() .dooh(Dooh.builder().build()) .app(App.builder().build()) @@ -1184,7 +1184,7 @@ public void validateShouldReturnValidationMessageWhenAliasNameEqualsToBidderItPo } @Test - public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderName() { + public void validateShouldReturnValidationErrorMessageWhenAliasPointOnNotValidBidderName() { // given final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() .aliases(singletonMap("alias", "fake")) @@ -1194,13 +1194,48 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnNotValidBidderN // when final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + // then + assertThat(result.getErrors()).hasSize(1) + .containsOnly("request.ext.prebid.aliases.alias refers to unknown bidder: fake"); + } + + @Test + public void validateShouldReturnValidationWarningMessageWhenAliasPointOnNotValidBidderName() { + // given + final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("alias", "fake")) + .build()); + final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); + + // when + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, true, false); + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + // then assertThat(result.getWarnings()).hasSize(1) .containsOnly("request.ext.prebid.aliases.alias refers to unknown bidder: fake"); } @Test - public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder() { + public void validateShouldReturnValidationErrorMessageWhenAliasPointOnDisabledBidder() { + // given + final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() + .aliases(singletonMap("alias", "appnexus")) + .build()); + final BidRequest bidRequest = validBidRequestBuilder().ext(ext).build(); + given(bidderCatalog.isValidName("appnexus")).willReturn(true); + given(bidderCatalog.isActive("appnexus")).willReturn(false); + + // when + final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); + + // then + assertThat(result.getErrors()).hasSize(1) + .containsOnly("request.ext.prebid.aliases.alias refers to disabled bidder: appnexus"); + } + + @Test + public void validateShouldReturnValidationWarningMessageWhenAliasPointOnDisabledBidder() { // given final ExtRequest ext = ExtRequest.of(ExtRequestPrebid.builder() .aliases(singletonMap("alias", "appnexus")) @@ -1210,6 +1245,7 @@ public void validateShouldReturnValidationMessageWhenAliasPointOnDisabledBidder( given(bidderCatalog.isActive("appnexus")).willReturn(false); // when + target = new RequestValidator(bidderCatalog, impValidator, metrics, jacksonMapper, 0.01, false, false, true); final ValidationResult result = target.validate(Account.empty(ACCOUNT_ID), bidRequest, null, null); // then