From 9a1f92659d6da4a252343718b00f351b655828bc Mon Sep 17 00:00:00 2001 From: Anton Babak <76536883+AntoxaAntoxic@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:17:08 +0100 Subject: [PATCH] Add module-execution config on the host level (#3594) --- docs/config-app.md | 7 +- .../server/hooks/execution/GroupExecutor.java | 2 +- .../hooks/execution/HookStageExecutor.java | 8 +- .../spring/config/HooksConfiguration.java | 8 ++ .../container/PrebidServerContainer.groovy | 1 - .../tests/module/GeneralModuleSpec.groovy | 53 ++++++++--- .../execution/HookStageExecutorTest.java | 88 +++++++++++++++++-- 7 files changed, 141 insertions(+), 26 deletions(-) diff --git a/docs/config-app.md b/docs/config-app.md index 5a88317bc14..0f0897437fa 100644 --- a/docs/config-app.md +++ b/docs/config-app.md @@ -370,9 +370,6 @@ contain 'WHERE last_updated > ?' for MySQL and 'WHERE last_updated > $1' for Pos For targeting available next options: - `settings.targeting.truncate-attr-chars` - set the max length for names of targeting keywords (0 means no truncation). -For modules: -- `settings.modules.require-config-to-invoke` - when enabled it requires a runtime config to exist for a module. - ## Host Cookie - `host-cookie.optout-cookie.name` - set the cookie name for optout checking. - `host-cookie.optout-cookie.value` - set the cookie value for optout checking. @@ -443,6 +440,10 @@ If not defined in config all other Health Checkers would be disabled and endpoin - `analytics.pubstack.buffers.count` - threshold in events count for buffer to send events - `analytics.pubstack.buffers.report-ttl-ms` - max period between two reports. +## Modules +- `hooks.admin.module-execution` - a key-value map, where a key is a module name and a value is a boolean, that defines whether modules hooks should/should not be always executed; if the module is not specified it is executed by default when it's present in the execution plan +- `settings.modules.require-config-to-invoke` - when enabled it requires a runtime config to exist for a module. + ## Debugging - `debug.override-token` - special string token for overriding Prebid Server account and/or adapter debug information presence in the auction response. diff --git a/src/main/java/org/prebid/server/hooks/execution/GroupExecutor.java b/src/main/java/org/prebid/server/hooks/execution/GroupExecutor.java index e8d2266d630..18d52b64c99 100644 --- a/src/main/java/org/prebid/server/hooks/execution/GroupExecutor.java +++ b/src/main/java/org/prebid/server/hooks/execution/GroupExecutor.java @@ -81,7 +81,7 @@ public Future> execute() { Future> groupFuture = Future.succeededFuture(initialGroupResult); for (final HookId hookId : group.getHookSequence()) { - if (!modulesExecution.getOrDefault(hookId.getModuleCode(), true)) { + if (!modulesExecution.get(hookId.getModuleCode())) { continue; } diff --git a/src/main/java/org/prebid/server/hooks/execution/HookStageExecutor.java b/src/main/java/org/prebid/server/hooks/execution/HookStageExecutor.java index c5f3c81894a..c9e55e389ae 100644 --- a/src/main/java/org/prebid/server/hooks/execution/HookStageExecutor.java +++ b/src/main/java/org/prebid/server/hooks/execution/HookStageExecutor.java @@ -81,6 +81,7 @@ public class HookStageExecutor { private final ExecutionPlan hostExecutionPlan; private final ExecutionPlan defaultAccountExecutionPlan; + private final Map hostModuleExecution; private final HookCatalog hookCatalog; private final TimeoutFactory timeoutFactory; private final Vertx vertx; @@ -90,6 +91,7 @@ public class HookStageExecutor { private HookStageExecutor(ExecutionPlan hostExecutionPlan, ExecutionPlan defaultAccountExecutionPlan, + Map hostModuleExecution, HookCatalog hookCatalog, TimeoutFactory timeoutFactory, Vertx vertx, @@ -105,10 +107,12 @@ private HookStageExecutor(ExecutionPlan hostExecutionPlan, this.clock = clock; this.mapper = mapper; this.isConfigToInvokeRequired = isConfigToInvokeRequired; + this.hostModuleExecution = hostModuleExecution; } public static HookStageExecutor create(String hostExecutionPlan, String defaultAccountExecutionPlan, + Map hostModuleExecution, HookCatalog hookCatalog, TimeoutFactory timeoutFactory, Vertx vertx, @@ -122,6 +126,7 @@ public static HookStageExecutor create(String hostExecutionPlan, return new HookStageExecutor( parseAndValidateExecutionPlan(hostExecutionPlan, mapper, hookCatalog), parseAndValidateExecutionPlan(defaultAccountExecutionPlan, mapper, hookCatalog), + hostModuleExecution, hookCatalog, Objects.requireNonNull(timeoutFactory), Objects.requireNonNull(vertx), @@ -185,7 +190,7 @@ public Future> executeEntrypointStag .withHookProvider(hookProviderForEntrypointStage(context)) .withInitialPayload(EntrypointPayloadImpl.of(queryParams, headers, body)) .withInvocationContextProvider(invocationContextProvider(endpoint)) - .withModulesExecution(Collections.emptyMap()) + .withModulesExecution(DefaultedMap.defaultedMap(hostModuleExecution, true)) .withRejectAllowed(true) .execute(); } @@ -374,6 +379,7 @@ private Map modulesExecutionForAccount(Account account) { .forEach(module -> resultModulesExecution.computeIfAbsent(module, key -> true)); } + resultModulesExecution.putAll(hostModuleExecution); return DefaultedMap.defaultedMap(resultModulesExecution, !isConfigToInvokeRequired); } diff --git a/src/main/java/org/prebid/server/spring/config/HooksConfiguration.java b/src/main/java/org/prebid/server/spring/config/HooksConfiguration.java index 64534d119aa..5a05ccb8c8e 100644 --- a/src/main/java/org/prebid/server/spring/config/HooksConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/HooksConfiguration.java @@ -8,6 +8,7 @@ import org.prebid.server.hooks.execution.HookStageExecutor; import org.prebid.server.hooks.v1.Module; import org.prebid.server.json.JacksonMapper; +import org.prebid.server.settings.model.HooksAdminConfig; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -16,6 +17,8 @@ import java.time.Clock; import java.util.Collection; +import java.util.Collections; +import java.util.Optional; @Configuration public class HooksConfiguration { @@ -38,6 +41,9 @@ HookStageExecutor hookStageExecutor(HooksConfigurationProperties hooksConfigurat return HookStageExecutor.create( hooksConfiguration.getHostExecutionPlan(), hooksConfiguration.getDefaultAccountExecutionPlan(), + Optional.ofNullable(hooksConfiguration.getAdmin()) + .map(HooksAdminConfig::getModuleExecution) + .orElseGet(Collections::emptyMap), hookCatalog, timeoutFactory, vertx, @@ -60,5 +66,7 @@ private static class HooksConfigurationProperties { String hostExecutionPlan; String defaultAccountExecutionPlan; + + HooksAdminConfig admin; } } diff --git a/src/test/groovy/org/prebid/server/functional/testcontainers/container/PrebidServerContainer.groovy b/src/test/groovy/org/prebid/server/functional/testcontainers/container/PrebidServerContainer.groovy index 5629826942b..b3f938a7ca0 100644 --- a/src/test/groovy/org/prebid/server/functional/testcontainers/container/PrebidServerContainer.groovy +++ b/src/test/groovy/org/prebid/server/functional/testcontainers/container/PrebidServerContainer.groovy @@ -91,7 +91,6 @@ class PrebidServerContainer extends GenericContainer { private static String normalizeProperty(String property) { property.replace(".", "_") - .replace("-", "") .replace("[", "_") .replace("]", "_") } diff --git a/src/test/groovy/org/prebid/server/functional/tests/module/GeneralModuleSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/module/GeneralModuleSpec.groovy index 15aab0a3e2f..1ce411e8d36 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/module/GeneralModuleSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/module/GeneralModuleSpec.groovy @@ -15,7 +15,6 @@ import org.prebid.server.functional.model.request.auction.TraceLevel import org.prebid.server.functional.model.response.auction.InvocationResult import org.prebid.server.functional.service.PrebidServerService import org.prebid.server.functional.util.PBSUtils -import spock.lang.PendingFeature import static org.prebid.server.functional.model.ModuleName.ORTB2_BLOCKING import static org.prebid.server.functional.model.ModuleName.PB_RICHMEDIA_FILTER @@ -293,7 +292,6 @@ class GeneralModuleSpec extends ModuleBaseSpec { modulesConfig << [null, new PbsModulesConfig()] } - @PendingFeature def "PBS should call all modules without account config when modules enabled in module-execution host config"() { given: "PBS service with module-execution config" def pbsConfig = MULTI_MODULE_CONFIG + ENABLED_INVOKE_CONFIG + @@ -333,6 +331,37 @@ class GeneralModuleSpec extends ModuleBaseSpec { pbsServiceFactory.removeContainer(pbsConfig) } + def "PBS shouldn't call any module without account config when modules disabled in module-execution host config"() { + given: "PBS service with module-execution config" + def pbsConfig = MULTI_MODULE_CONFIG + ENABLED_INVOKE_CONFIG + + [("hooks.admin.module-execution.${ORTB2_BLOCKING.code}".toString()): 'false'] + def pbsServiceWithMultipleModules = pbsServiceFactory.getService(pbsConfig) + + and: "Default bid request with verbose trace" + def bidRequest = defaultBidRequest.tap { + ext.prebid.trace = TraceLevel.VERBOSE + } + + and: "Flush metrics" + flushMetrics(pbsServiceWithMultipleModules) + + when: "PBS processes auction request" + def response = pbsServiceWithMultipleModules.sendAuctionRequest(bidRequest) + + then: "PBS response shouldn't include trace information about no-called modules" + assert !response?.ext?.prebid?.modules?.trace?.stages?.outcomes?.groups?.invocationResults?.flatten() + + and: "Ortb2blocking module call metrics shouldn't be updated" + def metrics = pbsServiceWithMultipleModuleWithRequireInvoke.sendCollectedMetricsRequest() + assert !metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] + assert !metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] + assert !metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] + assert !metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + def "PBS should call module without account config when default-account module-execution config enabled module"() { given: "PBS service with module-execution and default account configs" def defaultAccountConfigSettings = AccountConfig.defaultAccountConfig.tap { @@ -358,19 +387,15 @@ class GeneralModuleSpec extends ModuleBaseSpec { when: "PBS processes auction request" def response = pbsServiceWithMultipleModules.sendAuctionRequest(bidRequest) - then: "PBS response should include trace information about called modules" - verifyAll(response?.ext?.prebid?.modules?.trace?.stages?.outcomes?.groups?.invocationResults?.flatten() as List) { - it.status == [SUCCESS, SUCCESS] - it.action == [NO_ACTION, NO_ACTION] - it.hookId.moduleCode.sort() == [ORTB2_BLOCKING, ORTB2_BLOCKING].code.sort() - } + then: "PBS response shouldn't include trace information about no-called modules" + assert !response?.ext?.prebid?.modules?.trace?.stages?.outcomes?.groups?.invocationResults?.flatten() - and: "Ortb2blocking module call metrics should be updated" - def metrics = pbsServiceWithMultipleModules.sendCollectedMetricsRequest() - assert metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] == 1 - assert metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] == 1 - assert metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] == 1 - assert metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] == 1 + and: "Ortb2blocking module call metrics shouldn't be updated" + def metrics = pbsServiceWithMultipleModuleWithRequireInvoke.sendCollectedMetricsRequest() + assert !metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] + assert !metrics[CALL_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] + assert !metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, BIDDER_REQUEST.metricValue, ORTB2_BLOCKING_BIDDER_REQUEST.code)] + assert !metrics[NOOP_METRIC.formatted(ORTB2_BLOCKING.code, RAW_BIDDER_RESPONSE.metricValue, ORTB2_BLOCKING_RAW_BIDDER_RESPONSE.code)] and: "RB-Richmedia-Filter module call metrics shouldn't be updated" assert !metrics[CALL_METRIC.formatted(PB_RICHMEDIA_FILTER.code, ALL_PROCESSED_BID_RESPONSES.metricValue, PB_RICHMEDIA_FILTER_ALL_PROCESSED_RESPONSES.code)] diff --git a/src/test/java/org/prebid/server/hooks/execution/HookStageExecutorTest.java b/src/test/java/org/prebid/server/hooks/execution/HookStageExecutorTest.java index ea9ad000891..30736df5845 100644 --- a/src/test/java/org/prebid/server/hooks/execution/HookStageExecutorTest.java +++ b/src/test/java/org/prebid/server/hooks/execution/HookStageExecutorTest.java @@ -91,6 +91,7 @@ import java.time.Clock; import java.time.ZoneOffset; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -411,6 +412,70 @@ public void shouldBypassEntrypointHooksWhenNoPlanForStage(VertxTestContext conte })); } + @Test + public void shouldBypassEntrypointHooksThatAreDisabled(VertxTestContext context) { + // given + givenEntrypointHook( + "module-alpha", + "hook-a", + immediateHook(InvocationResultUtils.succeeded( + payload -> EntrypointPayloadImpl.of( + payload.queryParams(), payload.headers(), payload.body() + "-abc"), + "moduleAlphaContext"))); + + givenEntrypointHook( + "module-alpha", + "hook-b", + delayedHook(InvocationResultUtils.succeeded(payload -> EntrypointPayloadImpl.of( + payload.queryParams(), payload.headers(), payload.body() + "-def")), 40)); + + givenEntrypointHook( + "module-beta", + "hook-a", + delayedHook(InvocationResultUtils.succeeded(payload -> EntrypointPayloadImpl.of( + payload.queryParams(), payload.headers(), payload.body() + "-ghi")), 80)); + + givenEntrypointHook( + "module-beta", + "hook-b", + immediateHook(InvocationResultUtils.succeeded( + payload -> EntrypointPayloadImpl.of( + payload.queryParams(), payload.headers(), payload.body() + "-jkl"), + "moduleBetaContext"))); + + final HookStageExecutor executor = HookStageExecutor.create( + executionPlan(singletonMap( + Endpoint.openrtb2_auction, + EndpointExecutionPlan.of(singletonMap(Stage.entrypoint, execPlanTwoGroupsTwoHooksEach())))), + null, + Map.of("module-alpha", false), + hookCatalog, + timeoutFactory, + vertx, + clock, + jacksonMapper, + false); + + final HookExecutionContext hookExecutionContext = HookExecutionContext.of(Endpoint.openrtb2_auction); + + // when + final Future> future = executor.executeEntrypointStage( + CaseInsensitiveMultiMap.empty(), + CaseInsensitiveMultiMap.empty(), + "body", + hookExecutionContext); + + // then + future.onComplete(context.succeeding(result -> { + assertThat(result).isNotNull(); + assertThat(result.isShouldReject()).isFalse(); + assertThat(result.getPayload()).isNotNull().satisfies(payload -> + assertThat(payload.body()).isEqualTo("body-ghi-jkl")); + + context.completeNow(); + })); + } + @Test public void shouldExecuteEntrypointHooksToleratingMisbehavingHooks(VertxTestContext context) { // given @@ -1341,12 +1406,14 @@ public void shouldNotExecuteRawAuctionRequestHooksWhenAccountConfigIsNotRequired 200L, asList( HookId.of("module-alpha", "hook-a"), - HookId.of("module-beta", "hook-a"))), + HookId.of("module-beta", "hook-a"), + HookId.of("module-epsilon", "hook-a"))), ExecutionGroup.of( 200L, asList( HookId.of("module-gamma", "hook-b"), - HookId.of("module-delta", "hook-b"))))); + HookId.of("module-delta", "hook-b"), + HookId.of("module-zeta", "hook-b"))))); final String hostExecutionPlan = executionPlan(singletonMap( Endpoint.openrtb2_auction, @@ -1355,6 +1422,7 @@ public void shouldNotExecuteRawAuctionRequestHooksWhenAccountConfigIsNotRequired final HookStageExecutor executor = HookStageExecutor.create( hostExecutionPlan, null, + Map.of("module-epsilon", true, "module-zeta", false), hookCatalog, timeoutFactory, vertx, @@ -1375,11 +1443,13 @@ public void shouldNotExecuteRawAuctionRequestHooksWhenAccountConfigIsNotRequired null, Map.of("module-alpha", mapper.createObjectNode(), "module-beta", mapper.createObjectNode(), - "module-gamma", mapper.createObjectNode()), + "module-gamma", mapper.createObjectNode(), + "module-zeta", mapper.createObjectNode()), HooksAdminConfig.builder() .moduleExecution(Map.of( "module-alpha", true, - "module-beta", false)) + "module-beta", false, + "module-epsilon", false)) .build())) .build()) .hookExecutionContext(hookExecutionContext) @@ -1394,6 +1464,7 @@ public void shouldNotExecuteRawAuctionRequestHooksWhenAccountConfigIsNotRequired .at(1) .id("id") .tmax(1000L) + .site(Site.builder().build()) .build())); assertThat(hookExecutionContext.getStageOutcomes()) @@ -1468,6 +1539,7 @@ public void shouldExecuteRawAuctionRequestHooksWhenAccountConfigIsRequired(Vertx final HookStageExecutor executor = HookStageExecutor.create( hostExecutionPlan, null, + Map.of("module-epsilon", true, "module-zeta", false), hookCatalog, timeoutFactory, vertx, @@ -1488,11 +1560,13 @@ public void shouldExecuteRawAuctionRequestHooksWhenAccountConfigIsRequired(Vertx null, Map.of("module-alpha", mapper.createObjectNode(), "module-beta", mapper.createObjectNode(), - "module-gamma", mapper.createObjectNode()), + "module-gamma", mapper.createObjectNode(), + "module-zeta", mapper.createObjectNode()), HooksAdminConfig.builder() .moduleExecution(Map.of( "module-alpha", true, - "module-beta", false)) + "module-beta", false, + "module-epsilon", false)) .build())) .build()) .hookExecutionContext(hookExecutionContext) @@ -1506,6 +1580,7 @@ public void shouldExecuteRawAuctionRequestHooksWhenAccountConfigIsRequired(Vertx assertThat(payload.bidRequest()).isEqualTo(BidRequest.builder() .at(1) .id("id") + .site(Site.builder().build()) .build())); assertThat(hookExecutionContext.getStageOutcomes()) @@ -3298,6 +3373,7 @@ private HookStageExecutor createExecutor(String hostExecutionPlan, String defaul return HookStageExecutor.create( hostExecutionPlan, defaultAccountExecutionPlan, + Collections.emptyMap(), hookCatalog, timeoutFactory, vertx,